Re: [PATCH v2 net-next 2/2] ionic: add devlink firmware update

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 5 Sep 2020 14:47:58 -0700 Shannon Nelson wrote:
> On 9/5/20 12:52 PM, Jakub Kicinski wrote:
> > On Thu,  3 Sep 2020 17:05:34 -0700 Shannon Nelson wrote:  
> >> +		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?
 
That may still generate two notifications at the end tho, no?
Unless the loop one in the loop is && offset != fw->size.