On 9/5/20 12:52 PM, Jakub Kicinski wrote:
On Thu,  3 Sep 2020 17:05:34 -0700 Shannon Nelson wrote:
+       devlink_flash_update_status_notify(dl, "Downloading", NULL, 0, 
fw->size);
+       offset = 0;
+       next_interval = fw->size / IONIC_FW_INTERVAL_FRACTION;
+       while (offset < fw->size) {
+               copy_sz = min_t(unsigned int, buf_sz, fw->size - offset);
+               mutex_lock(&ionic->dev_cmd_lock);
+               memcpy_toio(&idev->dev_cmd_regs->data, fw->data + offset, 
copy_sz);
+               ionic_dev_cmd_firmware_download(idev,
+                                               offsetof(union 
ionic_dev_cmd_regs, data),
+                                               offset, copy_sz);
+               err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
+               mutex_unlock(&ionic->dev_cmd_lock);
+               if (err) {
+                       netdev_err(netdev,
+                                  "download failed offset 0x%x addr 0x%lx len 
0x%x\n",
+                                  offset, offsetof(union ionic_dev_cmd_regs, 
data),
+                                  copy_sz);
+                       NL_SET_ERR_MSG_MOD(extack, "Segment download failed");
+                       goto err_out;
+               }
+               offset += copy_sz;
+
+               if (offset > next_interval) {
+                       devlink_flash_update_status_notify(dl, "Downloading",
+                                                          NULL, offset, 
fw->size);
+                       next_interval = offset + (fw->size / 
IONIC_FW_INTERVAL_FRACTION);
+               }
+       }
+       devlink_flash_update_status_notify(dl, "Downloading", NULL, 1, 1);
This is quite awkward. You send a notification with a different size,
and potentially an unnecessary one if last iteration of the loop
triggered offset > next_interval.

Please just add || offset == fw->size to the condition at the end of
the loop and it will always trigger, with the correct length.

Or maybe make that last one look like
    devlink_flash_update_status_notify(dl, "Downloading", NULL, fw->size, fw->size); to be less awkward and to keep the style of using a final status_notify at the end of the block, as done in the Install and Select blocks further along?

sln






Reply via email to