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