Re: [PATCH v2 3/3] can: etas_es58x: report the firmware version through ethtool

[Vincent MAILHOL]

On Sun. 6 Nov. 2022 at 20:25, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Nov 05, 2022 at 08:45:10PM -0400, Alan Stern wrote:
> > On Sat, Nov 05, 2022 at 06:38:35PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Nov 06, 2022 at 02:21:11AM +0900, Vincent MAILHOL wrote:
> > > > On Sat. 5 Nov. 2022 at 18:27, Vincent MAILHOL
> > > > <mailhol.vincent@xxxxxxxxxx> wrote:
> > > > > On Sat. 5 Nov. 2022 at 17:36, Greg Kroah-Hartman
> > > > > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > It's late right now, and I can't remember the whole USB spec, but I
> > > think the device provides a list of the string ids that are valid for
> > > it.  If so, we can add that to sysfs for any USB device out there, no
> > > matter the string descriptor number.
> >
> > No, there is no such list.
>
> Yeah, my fault, nevermind about that, sorry.
>
> > > If not, maybe we can just iterate the 255 values and populate sysfs
> > > files if they are present?  I'll dig up the USB spec tomorrow...
> >
> > Yes, we could do that.  But the filename would have to be the string
> > id, which is not meaningful.  We wouldn't be able to have labels like
> > "product-info" unless somehow a driver could provide the label.
>
> We could have a directory of strings/ with the individual descriptors in
> there as files with the name being the string id.
>
> But that might take a long time to populate, as it can take a few tries
> to get the string from a device, and to do that 256 times might be
> noticable at device insertion time.
>
> > Also, there's the matter of language.  Devices can have string
> > descriptors in multiple languages; which one should we show in sysfs?
> > All of them?  Right now we use just the default language for the strings
> > that we put in sysfs.
> >
> > > I say do this at the USB core level, that way it works for any USB
> > > device, and you don't have a device-specific sysfs file and custom
> > > userspace code just for this.
> >
> > This is unavoidable to some extent.  Without device-specific information
> > or userspace code, there is no way to know which string descriptor
> > contains the data you want.
>
> Agreed.
>
> Ok, for this specific instance, adding the "we know this string id
> should be here" as a device-specific sysfs file seems to be the easiest
> way forward.
>
> Vincent, want to try that instead?

OK for me. Will do that and remove the kernel log spam and replace it
by a sysfs entry.

I have two questions:

1/ Can I still export and use usb_cache_string()? In other terms, does
the first patch of this series still apply? This looks like the most
convenient function to retrieve that custom string to me.

2/ Is it a no-go to also populate ethtool_drvinfo::fw_version? Some
users might look for the value in sysfs, while some might use ethtool.
If the info is not available in ethtool, people might simply assume it
is not available at all. So, I would like to provide both.


Yours sincerely,
Vincent Mailhol