Re: [PATCH v3 0/3] can: etas_es58x: report firmware, bootloader and hardware version

[Vincent MAILHOL <mailhol.vincent@wanadoo.fr>]

On Mon. 14 Nov. 2022 at 02:03, Andrew Lunn <andrew@lunn.ch> wrote:
> On Sun, Nov 13, 2022 at 01:01:05PM +0900, Vincent Mailhol wrote:
> > The goal of this series is to report the firmware version, the
> > bootloader version and the hardware revision of ETAS ES58x
> > devices.
> >
> > These are already reported in the kernel log but this isn't best
> > practise. Remove the kernel log and instead export all these in
> > sysfs. In addition, the firmware version is also reported through
> > ethtool.
>
> Sorry to only comment on version 3, rather than version 1. I don't
> normally look at CAN patches.

Actually, I only started to CC linux-usb mailing from version 2.
Regardless, thanks a lot, this is a valuable feedback.

> Have you considered using devlink?
>
> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-info.html

I have not thought about this (I simply did not know the existence of
this feature). A first quick look makes me think it is a good idea. I
will continue to investigate.

> fw and asic.id would cover two of your properties. Maybe talk to Jiri
> about the bootloader. It might make sense to add it is a new common
> property, or to use a custom property.

I will try to report the firmware version and the hardware version in
a first step and then see what we can do for the bootloader.

> devlink has the advantage of being a well defined, standardised API,
> rather than just random, per device sys files.

ACK.

> There might also be other interesting features in devlink, once you
> have basic support. Many Ethernet switch drivers use devlink regions
> to dump all the registers, for example.

I am aware of ethtool_drvinfo (which I implemented in the last patch
of this series to report the firmware version).
Do you have any reference of how to dump the other registers?

> Since there is a bootloader, i
> assume the firmware is upgradeable? devlink supports that.

True, it is upgradeable, however, I do not have an environment to test
for upgrades so there are no plans right now to develop an upgrade
feature.


Yours sincerely,
Vincent Mailhol