Re: [PATCH v2 3/3] usb: misc: ljca: print firmware version

[Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>]

On Wed, Nov 06, 2024 at 01:37:58PM +0000, Sakari Ailus wrote:
> Hi Greg,
> 
> On Wed, Nov 06, 2024 at 01:42:33PM +0100, Greg KH wrote:
> > On Wed, Nov 06, 2024 at 01:34:38PM +0100, Stanislaw Gruszka wrote:
> > > For diagnostics purposes read firmware version from device
> > > and print it to dmesg during initialization.
> > 
> > No, sorry, when drivers work properly, they are quiet.  Think about what
> > your kernel log would look like if you did this for every single driver
> > in the tree.

Not single one, but there are plenty of drivers in the tree that
print driver/firmware/hardware version to the log. Few in the usb
subsystem:

drivers/usb/fotg210/fotg210-udc.c:      dev_info(dev, "version %s\n", DRIVER_VERSION);
drivers/usb/gadget/legacy/acm_ms.c:     dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
drivers/usb/gadget/legacy/cdc2.c:       dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
drivers/usb/gadget/legacy/ether.c:      dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
drivers/usb/gadget/legacy/hid.c:        dev_info(&gadget->dev, DRIVER_DESC ", version: " DRIVER_VERSION "\n");
drivers/usb/gadget/udc/fusb300_udc.c:   dev_info(&pdev->dev, "version %s\n", DRIVER_VERSION);
drivers/usb/gadget/udc/lpc32xx_udc.c:   dev_info(udc->dev, "%s version %s\n", driver_name, DRIVER_VERSION);
drivers/usb/gadget/udc/m66592-udc.c:    dev_info(&pdev->dev, "version %s\n", DRIVER_VERSION);
drivers/usb/gadget/udc/net2272.c:       dev_info(dev->dev, "version: %s\n", driver_vers);
drivers/usb/gadget/udc/net2272.c:       dev_info(dev->dev, "RDK2 FPGA version %08x\n",
drivers/usb/gadget/udc/r8a66597-udc.c:  dev_info(dev, "version %s\n", DRIVER_VERSION);
drivers/usb/gadget/udc/renesas_usbf.c:  dev_info(dev, "USBF version: %08x\n",
drivers/usb/host/xhci-mtk.c:    dev_info(mtk->dev, "uwk - reg:0x%x, version:%d\n",
drivers/usb/mtu3/mtu3_core.c:   dev_info(mtu->dev, "IP version 0x%x(%s IP)\n", mtu->hw_version,
drivers/usb/mtu3/mtu3_host.c:   dev_info(ssusb->dev, "uwk - reg:0x%x, version:%d\n",
drivers/usb/typec/ucsi/ucsi_ccg.c:              dev_info(dev, "secondary fw version is too low (< %d)\n",
drivers/usb/typec/ucsi/ucsi_ccg.c:              dev_info(dev, "found primary fw with later version\n");
drivers/usb/typec/ucsi/ucsi_stm32g0.c:          dev_info(g0->dev, "Flashing FW: %08x (%08x cur)\n", fw_info->version, fw_version);
drivers/usb/typec/ucsi/ucsi_stm32g0.c:  dev_info(g0->dev, "Bootloader Version 0x%02x\n", g0->bl_version);

> > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > Tested-by: Hans de Goede <hdegoede@redhat.com> # ThinkPad X1 Yoga Gen 8, ov2740
> > > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> > > ---
> > >  drivers/usb/misc/usb-ljca.c | 29 +++++++++++++++++++++++++++++
<snip>

> > > +	}
> > > +
> > > +	dev_info(adap->dev, "Firmware version: %d.%d.%d.%d\n",
> > > +		 version.major, version.minor,
> > > +		 le16_to_cpu(version.patch), le16_to_cpu(version.build));
> > 
> > Again, sorry, but no.  Feel free to dump this in a sysfs file if you
> > really want to get access to it, but not in the kernel log.
> 
> I guess dev_dbg() should do as well.

I think that's better as we currently do not support sysfs in ljca,
so extra code would be needed.

Regards
Stanislaw