Re: [PATCH v2 2/4] xhci: dbc: allow to set serial number through sysfs

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Oct 07, 2025 at 09:39:00PM +0000, Łukasz Bartosik wrote:
> From: Łukasz Bartosik <ukaszb@chromium.org>
> 
> Add code which allows to set serial number of a DbC
> device through sysfs.
> 
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> ---
>  .../testing/sysfs-bus-pci-drivers-xhci_hcd    | 13 +++++++
>  drivers/usb/host/xhci-dbgcap.c                | 36 +++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> index fc82aa4e54b0..071688dbd969 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> @@ -85,3 +85,16 @@ Description:
>  		up to 5000. The default value is 64 ms.
>  		This polling interval is used while DbC is enabled but has no
>  		active data transfers.
> +
> +What:          /sys/bus/pci/drivers/xhci_hcd/.../dbc_iSerial
> +Date:          October 2025
> +Contact:       Łukasz Bartosik <ukaszb@chromium.org>
> +Description:
> +               The dbc_iSerial attribute allows to change the iSerial field
> +               presented in the USB device descriptor by xhci debug device.
> +               Value can only be changed while debug capability (DbC) is in
> +               disabled state to prevent USB device descriptor change while
> +               connected to a USB host.
> +               The default value is "0001".
> +               The field length can be from 1 to 63 characters.
> +
> diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
> index c2fecaffd6f3..7ad83548ba4d 100644
> --- a/drivers/usb/host/xhci-dbgcap.c
> +++ b/drivers/usb/host/xhci-dbgcap.c
> @@ -1200,6 +1200,40 @@ static ssize_t dbc_bcdDevice_store(struct device *dev,
>  	return size;
>  }
>  
> +static ssize_t dbc_iSerial_show(struct device *dev,
> +			    struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct xhci_hcd	*xhci = hcd_to_xhci(dev_get_drvdata(dev));
> +	struct xhci_dbc	*dbc = xhci->dbc;
> +
> +	return sysfs_emit(buf, "%s\n", dbc->str.serial);
> +}
> +
> +static ssize_t dbc_iSerial_store(struct device *dev,
> +			     struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct xhci_hcd	*xhci = hcd_to_xhci(dev_get_drvdata(dev));
> +	struct xhci_dbc	*dbc = xhci->dbc;
> +	size_t len;
> +
> +	if (dbc->state != DS_DISABLED)
> +		return -EBUSY;
> +
> +	len = strcspn(buf, "\n");

So you are requiring the \n to be there?  Why?  What tool will write it?
You don't document this in up in the documentation :)

> +	if (!len)
> +		return -EINVAL;
> +
> +	if (len > USB_MAX_STRING_LEN/2)
> +		return -E2BIG;
> +
> +	memcpy(dbc->str.serial, buf, len);
> +	dbc->str.serial[len] = '\0';

As you know that buf is zero terminated, you can do a strcpy instead of
this two-step process.  Ah, but that \n character, why not just zap it
out if it is present?

thanks,

greg k-h