Message-Id: <20221130174658.29282-1-mailhol.vincent@wanadoo.fr>
Date:   Thu,  1 Dec 2022 02:46:51 +0900
From:   Vincent Mailhol <mailhol.vincent@...adoo.fr>
To:     linux-can@...r.kernel.org
Cc:     Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        netdev@...r.kernel.org, linux-usb@...r.kernel.org,
        Saeed Mahameed <saeed@...nel.org>,
        Andrew Lunn <andrew@...n.ch>, Jiri Pirko <jiri@...dia.com>,
        Lukas Magel <lukas.magel@...teo.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>
Subject: [PATCH v5 0/7] can: etas_es58x: report firmware, bootloader and hardware version

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
practice. Remove the kernel log and instead export all these through
devlink. The devlink core automatically exports the firmware and the
bootloader version to ethtool, so no need to implement the
ethtool_ops::get_drvinfo() callback anymore.

Patch one and two implement the core support for devlink (at device
level) and devlink port (at the network interface level).

Patch three export usb_cache_string() and patch four add a new info
attribute to devlink.h. Both are prerequisites for patch five.

Patch five is the actual goal: it parses the product information from
a custom usb string returned by the device and expose them through
devlink.

Patch six removes the product information from the kernel log.

Finally, patch seven add a devlink documentation page with list all
the information attributes reported by the driver.


* Sample outputs following this series *

| $ devlink dev info 
| usb/1-9:1.1:
|   serial_number 0108954
|   versions:
|       fixed:
|         board.rev B012/000
|       running:
|         fw 04.00.01
|         fw.bootloader 02.00.00

| $ devlink port show can0
| usb/1-9:1.1/0: type eth netdev can0 flavour physical port 0 splittable false

| $ ethtool -i can0
| driver: etas_es58x
| version: 6.1.0-rc7+
| firmware-version: 04.00.01 02.00.00 
| expansion-rom-version: 
| bus-info: 1-9:1.1
| supports-statistics: no
| supports-test: no
| supports-eeprom-access: no
| supports-register-dump: no
| supports-priv-flags: no

---
* Changelog *

v4 -> v5:

  * [PATH 2/7] add devlink port support. This extends devlink to the
    network interface.

  * thanks to devlink port, 'ethtool -i' is now able to retrieve the
    firmware version from devlink. No need to implement the
    ethtool_ops::get_drvinfo() callback anymore: remove one patch from
    the series.

  * [PATCH 4/7] A new patch to add a new info attribute for the
    bootloader version in devlink.h. This patch was initially sent as
    a standalone patch here:
      https://lore.kernel.org/netdev/20221129031406.3849872-1-mailhol.vincent@wanadoo.fr/
    Merging it to this series so that it is both added and used at the
    same time.

  * [PATCH 5/7] use the newly info attribute defined in patch 4/7 to
    report the bootloader version instead of the custom string "bl".

  * [PATCH 5/7] because the series does not implement
    ethtool_ops::get_drvinfo() anymore, the two helper functions
    es58x_sw_version_is_set() and es58x_hw_revision_is_set() are only
    used in devlink.c. Move them from es58x_core.h to es58x_devlink.c.

  * [PATCH 5/7] small rework of the helper function
    es58x_hw_revision_is_set(): it is OK to only check the letter (if
    the letter is '\0', it will not be possible to print the next
    numbers).

  * [PATCH 5/7 and 6/7] add reviewed-by Andrew Lunn tag.

  * [PATCH 7/7] Now, 'ethtool -i' reports both the firmware version
    and the bootloader version (this is how the core export the
    information from devlink to ethtool). Update the documentation to
    reflect this fact.

  * Reoder the patches.

v3 -> v4:

  * major rework to use devlink instead of sysfs following Andrew's
    comment.

  * split the series in 6 patches.

  * [PATCH 1/6] add Acked-by: Greg Kroah-Hartman

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 (7):
  can: etas_es58x: add devlink support
  can: etas_es58x: add devlink port support
  USB: core: export usb_cache_string()
  net: devlink: add DEVLINK_INFO_VERSION_GENERIC_FW_BOOTLOADER
  can: etas_es58x: export product information through
    devlink_ops::info_get()
  can: etas_es58x: remove es58x_get_product_info()
  Documentation: devlink: add devlink documentation for the etas_es58x
    driver

 .../networking/devlink/devlink-info.rst       |   5 +
 .../networking/devlink/etas_es58x.rst         |  36 +++
 MAINTAINERS                                   |   1 +
 drivers/net/can/usb/Kconfig                   |   1 +
 drivers/net/can/usb/etas_es58x/Makefile       |   2 +-
 drivers/net/can/usb/etas_es58x/es58x_core.c   |  98 +++-----
 drivers/net/can/usb/etas_es58x/es58x_core.h   |  50 ++++
 .../net/can/usb/etas_es58x/es58x_devlink.c    | 235 ++++++++++++++++++
 drivers/usb/core/message.c                    |   1 +
 drivers/usb/core/usb.h                        |   1 -
 include/linux/usb.h                           |   1 +
 include/net/devlink.h                         |   2 +
 12 files changed, 372 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/networking/devlink/etas_es58x.rst
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_devlink.c

-- 
2.37.4

Powered by blists - more mailing lists