Subject: Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

From: Jiri Pirko <[email-masked]>
Date: Tue, 19 May 2020 09:30:32 +0200

Tue, May 19, 2020 at 07:43:01AM CEST, [email-masked] wrote:
>On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <[email-masked]> wrote:
>>
>> Tue, May 19, 2020 at 06:31:27AM CEST, [email-masked] wrote:
>> >On Mon, May 18, 2020 at 4:31 PM Jiri Pirko <[email-masked]> wrote:
>> >>
>> >> Mon, May 18, 2020 at 10:27:15AM CEST, [email-masked] wrote:
>> >> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
>> >> >parameter and use it in bnxt_en driver.
>> >> >
>> >> >Also, firmware spec. is updated to 1.10.1.40.
>> >>
>> >> Hi.
>> >>
>> >> We've been discussing this internally for some time.
>> >> I don't like to use params for this purpose.
>> >> We already have "devlink dev flash" and "devlink dev reload" commands.
>> >> Combination of these two with appropriate attributes should provide what
>> >> you want. The "param" you are introducing is related to either "flash"
>> >> or "reload", so I don't think it is good to have separate param, when we
>> >> can extend the command attributes.
>> >>
>> >> How does flash&reload work for mlxsw now:
>> >>
>> >> # devlink flash
>> >> Now new version is pending, old FW is running
>> >> # devlink reload
>> >> Driver resets the device, new FW is loaded
>> >>
>> >> I propose to extend reload like this:
>> >>
>> >>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>> >>    driver-default - means one of following to, according to what is
>> >>                     default for the driver
>> >>    fw-reset - does FW reset and driver entities re-instantiation
>> >>    driver-only - does driver entities re-instantiation only
>> >>    fw-live-patch - does only FW live patching - no effect on kernel
>> >>
>> >> Could be an enum or bitfield. Does not matter. The point is to use
>> >> reload with attribute to achieve what user wants. In your usecase, user
>> >> would do:
>> >>
>> >> # devlink flash
>> >> # devlink reload level fw-live-patch
>> >
>> >Jiri,
>> >
>> >I am adding this param to control the fw hot reset capability of the device.
>>
>> I don't follow, sorry. Could you be more verbose about what you are
>> trying to achieve here?
>As mentioned below, hot_fw_reset is a device capability similar to roce.
>Capability can be enabled or disabled on the device, if the device supports.
>When enabled and if supported firmware and driver are loaded, user can
>utilise the capability to fw_reset or fw_live_patch.

I don't undestand what exactly is this supposed to enable/disable. Could
you be more specific?

>
>So, we need a policy to enable/disable the capability.
>
>Thanks.
>>
>> >Permanent configuration mode will toggle the NVM config space which is
>> >very similar to enable_roce/enable_sriov param and runtime configuration
>> >mode will toggle the driver level knob to avoid/allow fw-live-reset.
>> >
>> >From above. I see that you are suggesting how to trigger the fw hot reset.
>> >This is good to have, but does not serve the purpose of enabling or disabling
>> >of the feature. Our driver is currently using "ethtool --reset" for
>> >triggering fw-reset
>> >or fw-live-patch.
>> >
>> >Thanks,
>> >Vasundhara