On Thu, 10 Sep 2020 13:59:07 -0700 Jacob Keller wrote: > On 9/9/2020 5:55 PM, Jakub Kicinski wrote: > > On Wed, 9 Sep 2020 15:26:50 -0700 Jacob Keller wrote: > >> The devlink core recently gained support for checking whether the driver > >> supports a flash_update parameter, via `supported_flash_update_params`. > >> However, parameters are specified as function arguments. Adding a new > >> parameter still requires modifying the signature of the .flash_update > >> callback in all drivers. > >> > >> Convert the .flash_update function to take a new `struct > >> devlink_flash_update_params` instead. By using this structure, and the > >> `supported_flash_update_params` bit field, a new parameter to > >> flash_update can be added without requiring modification to existing > >> drivers. > >> > >> As before, all parameters except file_name will require driver opt-in. > >> Because file_name is a necessary field to for the flash_update to make > >> sense, no "SUPPORTED" bitflag is provided and it is always considered > >> valid. All future additional parameters will require a new bit in the > >> supported_flash_update_params bitfield. > > > > I keep thinking we should also make the core do the > > request_firmware_direct(). What else is the driver gonna do with the file > > name.. > > > > But I don't want to drag your series out so: > > > > Reviewed-by: Jakub Kicinski <k...@kernel.org> > > Hmm. What does _direct do? I guess it means it won't fall back to the > userspace helper if it can't find the firmware? It looks like MLX > drivers use it, but others seem to just stick to regular request_firmware.
FWIW _direct() is pretty much meaningless today, I think the kernel support for non-direct is mostly dropped. Systemd doesn't support it either. > This seems like an improvement that we can handle in a follow up series > either way. Thanks for the review! Agreed. Too many pending patches for this area already :S
