[Intel-wired-lan] [PATCH iwl-net] ice: implement AQ download pkg retry
Wojciech Drewek
wojciech.drewek at intel.com
Fri May 17 07:49:04 UTC 2024
On 16.05.2024 18:36, Brett Creeley wrote:
>
>
> On 5/16/2024 7:04 AM, Wojciech Drewek wrote:
>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>
>>
>> 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.
>>
>> Reviewed-by: Michal Swiatkowski <michal.swiatkowski at linux.intel.com>
>> Signed-off-by: Wojciech Drewek <wojciech.drewek at intel.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_ddp.c | 19 +++++++++++++++++--
>> 1 file changed, 17 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..19e2111fcf08 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,22 @@ 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 (try_cnt < 5) {
>> + 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)
>
> Are these the only 2 sporadic errors that FW will return?
Yes, that's right. We don't want to retry in case of other errors since those might be valid.
>
>> + break;
>> +
>> + try_cnt++;
>> + msleep(20);
>> + }
>> +
>> + if (try_cnt)
>> + dev_dbg(ice_hw_to_dev(hw),
>> + "ice_aq_download_pkg failed, number of retries: %d\n",
>> + try_cnt);
>
> If try_cnt is non-zero it doesn't mean the last download failed, it just means one or more attempts to download failed right? Maybe just "ice_aq_download_pkg number of retries: %d" since the if (status) check below will print on failure?
Sounds reasonable, we want this log only because we want to know if we hit this sporadic failure.
>
>>
>> /* Save AQ status from download package */
>> if (status) {
>> --
>> 2.40.1
>>
>>
More information about the Intel-wired-lan
mailing list