On Tue, Nov 12, 2024 at 08:55:14AM +0100, Stanislaw Gruszka wrote:
> For diagnostics purpose read firmware version during probe and add
> sysfs attribute to make that information available.
>
> Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@xxxxxxxxxxxxxxx>
> ---
> v3->v4: use sysfs attribute to provide firmware version,
> drop tags from Hans since the code changed
> v2->v3: use dev_dbg() for printing the firmware version
> v1->v2: fix 80-chars line wrap
>
> .../ABI/testing/sysfs-bus-usb-devices-ljca | 9 ++++
> drivers/usb/misc/usb-ljca.c | 53 +++++++++++++++++++
> 2 files changed, 62 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
> new file mode 100644
> index 000000000000..f5eb38bf99a8
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
> @@ -0,0 +1,9 @@
> +What: /sys/bus/usb/devices/.../ljca_version
> +Date: November 2024
> +KernelVersion: 6.13
> +Contact: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> +Description:
> + Provides the firmware version of LJCA device.
> + The format is Major.Minor.Patch.Build, where
> + Major, Minor, Patch, and Build are decimal numbers.
> + For example: 1.0.0.256
> diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
> index d9c21f783055..4b5bd5b27b2a 100644
> --- a/drivers/usb/misc/usb-ljca.c
> +++ b/drivers/usb/misc/usb-ljca.c
> @@ -43,6 +43,7 @@ enum ljca_client_type {
>
> /* MNG client commands */
> enum ljca_mng_cmd {
> + LJCA_MNG_GET_VERSION = 1,
> LJCA_MNG_RESET = 2,
> LJCA_MNG_ENUM_GPIO = 4,
> LJCA_MNG_ENUM_I2C = 5,
> @@ -68,6 +69,13 @@ struct ljca_msg {
> u8 data[] __counted_by(len);
> } __packed;
>
> +struct ljca_fw_version {
> + u8 major;
> + u8 minor;
> + __le16 patch;
> + __le16 build;
> +} __packed;
> +
> struct ljca_i2c_ctr_info {
> u8 id;
> u8 capacity;
> @@ -152,8 +160,10 @@ struct ljca_adapter {
> struct mutex mutex;
>
> struct list_head client_list;
> + struct ljca_fw_version fw_version;
>
> bool disconnect;
> + bool fw_version_valid;
>
> u32 reset_id;
> };
> @@ -740,6 +750,24 @@ static int ljca_enumerate_clients(struct ljca_adapter *adap)
> return ret;
> }
>
> +static void ljca_read_fw_version(struct ljca_adapter *adap)
> +{
> + struct ljca_fw_version version;
> + int ret;
> +
> + ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_GET_VERSION, NULL, 0,
> + (u8 *)&version, sizeof(version), true,
> + LJCA_WRITE_ACK_TIMEOUT_MS);
> +
> + if (ret != sizeof(version)) {
> + dev_err(adap->dev, "Get version failed, ret: %d\n", ret);
> + return;
> + }
> +
> + adap->fw_version = version;
> + adap->fw_version_valid = true;
> +}
> +
> static int ljca_probe(struct usb_interface *interface,
> const struct usb_device_id *id)
> {
> @@ -811,6 +839,8 @@ static int ljca_probe(struct usb_interface *interface,
> if (ret)
> goto err_free;
>
> + ljca_read_fw_version(adap);
> +
> /*
> * This works around problems with ov2740 initialization on some
> * Lenovo platforms. The autosuspend delay, has to be smaller than
> @@ -874,6 +904,28 @@ static int ljca_resume(struct usb_interface *interface)
> return usb_submit_urb(adap->rx_urb, GFP_KERNEL);
> }
>
> +static ssize_t ljca_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct usb_interface *intf = to_usb_interface(dev);
> + struct ljca_adapter *adap = usb_get_intfdata(intf);
> + struct ljca_fw_version ver = adap->fw_version;
> +
> + if (!adap->fw_version_valid)
> + return -ENODATA;
Why are you showing this sysfs attribute if there is no firmware
version? Please only create the file if it is possible to read a valid
value from it.
thanks,
greg k-h
