| Message ID | 20221113040108.68249-1-mailhol.vincent@wanadoo.fr |
|---|---|
| Headers | show |
| Series | can: etas_es58x: report firmware, bootloader and hardware version | expand |
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. Have you considered using devlink? https://www.kernel.org/doc/html/latest/networking/devlink/devlink-info.html 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. devlink has the advantage of being a well defined, standardised API, rather than just random, per device sys files. 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. Since there is a bootloader, i assume the firmware is upgradeable? devlink supports that. Andrew
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
> Do you have any reference of how to dump the other registers?
Have a look at drivers/net/dsa/mv88e6xxx/devlink.c, all the code with
mv88e6xxx_region_. This ethernet switch chip has multiple banks of
registers, one per port of the switch, and two global. It also has a
few other tables which can be interesting to dump in their raw format.
There is also a user space tool to pritty print them.
Andrew
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. * Changelog * v2 -> v3: * patch 2/3: do not spam the kernel log anymore with the product number. Instead parse the product information string, extract the firmware version, the bootloadar version and the hardware revision and export them through sysfs. * patch 2/3: rework the parsing in order not to need additional fields in struct es58x_parameters. * patch 3/3: only populate ethtool_drvinfo::fw_version because since commit edaf5df22cb8 ("ethtool: ethtool_get_drvinfo: populate drvinfo fields even if callback exits"), there is no need to populate ethtool_drvinfo::driver and ethtool_drvinfo::bus_info in the driver. v1 -> v2: * was a single patch. It is now a series of three patches. * add a first new patch to export usb_cache_string(). * add a second new patch to apply usb_cache_string() to existing code. * add missing check on product info string to prevent a buffer overflow. * add comma on the last entry of struct es58x_parameters. Vincent Mailhol (3): USB: core: export usb_cache_string() can: etas_es58x: export firmware, bootloader and hardware versions in sysfs can: etas_es58x: report firmware-version through ethtool drivers/net/can/usb/etas_es58x/Makefile | 2 +- drivers/net/can/usb/etas_es58x/es58x_core.c | 70 ++---- drivers/net/can/usb/etas_es58x/es58x_core.h | 51 ++++ drivers/net/can/usb/etas_es58x/es58x_sysfs.c | 231 +++++++++++++++++++ drivers/usb/core/message.c | 1 + drivers/usb/core/usb.h | 1 - include/linux/usb.h | 1 + 7 files changed, 309 insertions(+), 48 deletions(-) create mode 100644 drivers/net/can/usb/etas_es58x/es58x_sysfs.c