Re: [PATCH v2 3/4] xhci: dbc: allow to set product name through sysfs

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 10/8/25 00:39, Łukasz Bartosik wrote:
> From: Łukasz Bartosik <ukaszb@chromium.org>
> 
> Add code which allows to set product name of a DbC
> device through sysfs.
> 
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> ---
>   .../testing/sysfs-bus-pci-drivers-xhci_hcd    | 11 ++++++
>   drivers/usb/host/xhci-dbgcap.c                | 36 +++++++++++++++++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> index 071688dbd969..57ba37606f79 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> @@ -98,3 +98,14 @@ Description:
>                  The default value is "0001".
>                  The field length can be from 1 to 63 characters.
>   
> +What:          /sys/bus/pci/drivers/xhci_hcd/.../dbc_iProduct
> +Date:          October 2025
> +Contact:       Łukasz Bartosik <ukaszb@chromium.org>
> +Description:
> +               The dbc_iProduct attribute allows to change the iProduct 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 "Linux USB Debug Target".
> +               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 7ad83548ba4d..bc782f6b533e 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_iProduct_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.product);
> +}
> +
> +static ssize_t dbc_iProduct_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");
> +	if (!len)
> +		return -EINVAL;
> +
> +	if (len > USB_MAX_STRING_LEN/2)
> +		return -E2BIG;
> +
> +	memcpy(dbc->str.product, buf, len);
> +	dbc->str.product[len] = '\0';
> +
> +	return size;
> +}
> +
>   static ssize_t dbc_iSerial_show(struct device *dev,
>   			    struct device_attribute *attr,
>   			    char *buf)
> @@ -1321,6 +1355,7 @@ static DEVICE_ATTR_RW(dbc);
>   static DEVICE_ATTR_RW(dbc_idVendor);
>   static DEVICE_ATTR_RW(dbc_idProduct);
>   static DEVICE_ATTR_RW(dbc_bcdDevice);
> +static DEVICE_ATTR_RW(dbc_iProduct);
>   static DEVICE_ATTR_RW(dbc_iSerial);
>   static DEVICE_ATTR_RW(dbc_bInterfaceProtocol);
>   static DEVICE_ATTR_RW(dbc_poll_interval_ms);
> @@ -1330,6 +1365,7 @@ static struct attribute *dbc_dev_attrs[] = {
>   	&dev_attr_dbc_idVendor.attr,
>   	&dev_attr_dbc_idProduct.attr,
>   	&dev_attr_dbc_bcdDevice.attr,
> +	&dev_attr_dbc_iProduct.attr,

Small naming thing again.

"dbc_idProduct" and dbc_iProduct" are a bit too similar.

Usb core uses "product" and "manufacturer" sysfs entries to show these strings.
I think dbc should use similar "dbc_product" and "dbc_manufacturer" naming.

the "iProduct" is a byte in device descriptor telling the index of the product string
descriptor.

Thanks
Mathias