Re: [PATCH net-next v1] ethtool: ethtool_get_drvinfo: populate drvinfo fields even if callback exits

[Leon Romanovsky <leon@kernel.org>]

On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote:
> On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote:
> > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote:
> > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote:
> > > > If ethtool_ops::get_drvinfo() callback isn't set,
> > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and
> > > > ethtool_drvinfo::bus_info fields.
> > > >
> > > > However, if the driver provides the callback function, those two
> > > > fields are not touched. This means that the driver has to fill these
> > > > itself.
> > >
> > > Can you please point to such drivers?
> >
> > What you mean by "such drivers" is not clear from the quoted context,
> > at least to me.
> 
> An example:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041
> 
> This driver wants to set fw_version but needs to also fill the driver
> name and bus_info. My patch will enable *such drivers* to only fill
> the fw_version and delegate the rest to the core.

Sorry for being misleading, It looks like I typed only part of the sentence
which I had in my mind. I wanted to see if any driver exists which prints
drv_name and bus_info different from default.

> 
> > > One can argue that they don't need to touch these fields in a first
> > > place and ethtool_drvinfo should always overwrite them.
> >
> > Quite likely most driver prints to .driver and .bus_info can be dropped
> > with this patch in place. Then again, I'm suspecting it's a bit of a
> > chicken and an egg problem with people adding new drivers not having
> > an incentive to add the print in the core and people who want to add
> > the print in the core not having any driver that would benefit.
> > Therefore I'd lean towards accepting Vincent's patch as is even if
> > the submission can likely be more thorough and strict.
> 
> If we can agree that no drivers should ever print .driver and
> .bus_info, then I am fine to send a clean-up patch to remove all this
> after this one gets accepted. However, I am not willing to invest time
> for nothing. So would one of you be ready to sign-off such a  clean-up
> patch?

I will be happy to see such patch and will review it, but can't add sign-off
as I'm not netdev maintainer.

Thanks