| Message ID | 20240531093206.714632-1-wojciech.drewek@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net,v3] ice: implement AQ download pkg retry | expand |
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? Please elaborate, what firmware version you tested with, and if there are plans to fix this in the firmware. > Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download") > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> > Reviewed-by: Brett Creeley <brett.creeley@amd.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. > > /* Save AQ status from download package */ > if (status) { Kind regards, Paul
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@linux.intel.com> >> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> >> Reviewed-by: Brett Creeley <brett.creeley@amd.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
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Wojciech Drewek > Sent: Friday, May 31, 2024 3:02 PM > To: netdev@vger.kernel.org > Cc: Keller, Jacob E <jacob.e.keller@intel.com>; kuba@kernel.org; intel-wired-lan@lists.osuosl.org > Subject: [Intel-wired-lan] [PATCH iwl-net v3] ice: implement AQ download pkg retry > > 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. > > Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download") > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> > Reviewed-by: Brett Creeley <brett.creeley@amd.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(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
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); /* Save AQ status from download package */ if (status) {
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. Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download") Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> Reviewed-by: Brett Creeley <brett.creeley@amd.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(-)