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:

quoted

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.

quoted

Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download")
Reviewed-by: Michal Swiatkowski <redacted>
Signed-off-by: Wojciech Drewek <redacted>
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.

quoted

            /* Save AQ status from download package */
          if (status) {

Kind regards,

Paul