[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <43089963-9252-404c-a617-6f8f8e90b978@intel.com>
Date: Mon, 3 Jun 2024 10:41:24 +0200
From: Wojciech Drewek <wojciech.drewek@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: <netdev@...r.kernel.org>, <jacob.e.keller@...el.com>, <kuba@...nel.org>,
<intel-wired-lan@...ts.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v3] ice: implement AQ download
pkg retry
On 31.05.2024 12:36, Paul Menzel wrote:
> Dear Wojciech,
>
>
> Thank you for your patch.
>
> Am 31.05.24 um 11:32 schrieb Wojciech Drewek:
>> ice_aqc_opc_download_pkg (0x0C40) AQ sporadically returns error due
>> to FW issue. Fix this by retrying five times before moving to
>> Safe Mode.
>
> Also mention the delay of 20 ms?
I can do this.
>
> Please elaborate, what firmware version you tested with, and if there are plans to fix this in the firmware.
I tested it with firmware 4.40, I don't know anything about the fix yet.
>
>> Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download")
>> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
>> Reviewed-by: Brett Creeley <brett.creeley@....com>
>> ---
>> v2: remove "failure" from log message
>> v3: don't sleep in the last iteration of the wait loop
>> ---
>> drivers/net/ethernet/intel/ice/ice_ddp.c | 23 +++++++++++++++++++++--
>> 1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
>> index ce5034ed2b24..f182179529b7 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
>> @@ -1339,6 +1339,7 @@ ice_dwnld_cfg_bufs_no_lock(struct ice_hw *hw, struct ice_buf *bufs, u32 start,
>> for (i = 0; i < count; i++) {
>> bool last = false;
>> + int try_cnt = 0;
>> int status;
>> bh = (struct ice_buf_hdr *)(bufs + start + i);
>> @@ -1346,8 +1347,26 @@ ice_dwnld_cfg_bufs_no_lock(struct ice_hw *hw, struct ice_buf *bufs, u32 start,
>> if (indicate_last)
>> last = ice_is_last_download_buffer(bh, i, count);
>> - status = ice_aq_download_pkg(hw, bh, ICE_PKG_BUF_SIZE, last,
>> - &offset, &info, NULL);
>> + while (1) {
>> + status = ice_aq_download_pkg(hw, bh, ICE_PKG_BUF_SIZE,
>> + last, &offset, &info,
>> + NULL);
>> + if (hw->adminq.sq_last_status != ICE_AQ_RC_ENOSEC &&
>> + hw->adminq.sq_last_status != ICE_AQ_RC_EBADSIG)
>> + break;
>> +
>> + try_cnt++;
>> +
>> + if (try_cnt == 5)
>> + break;
>> +
>> + msleep(20);
>> + }
>> +
>> + if (try_cnt)
>> + dev_dbg(ice_hw_to_dev(hw),
>> + "ice_aq_download_pkg number of retries: %d\n",
>> + try_cnt);
>
> Should the firmware be fixed, a warning could be shown asking to update the firmware.
I won't put such info before we actually have a fix in the firmware.
>
>> /* Save AQ status from download package */
>> if (status) {
>
>
> Kind regards,
>
> Paul
Powered by blists - more mailing lists