* [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output
@ 2025-07-06 14:27 Daniele Barcella
2025-07-07 8:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Daniele Barcella @ 2025-07-06 14:27 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Daniele Barcella, linux-usb, linux-kernel
The patch improves the usb modalias sysfs output by including the manufacturer string ath the end in the format 'mnf%s' after filtering the string, like how it is done for the dmi id device.
For example usb:v0483p5740d0100dcEFdsc02dp01ic02isc02ip00in00mnfFlipperDevicesInc.
This change allows hwdb rules to target devices with the same vid+pid but different manufacturer.
For example, the STMicroelectronics Virtual COM Port (0483:5740) is widely used in both prototyping and production devices that don't have a unique vendor ID.
For further context, refer to the related discussion on systemd's GitHub: https://github.com/systemd/systemd/pull/24869.
Signed-off-by: Daniele Barcella <daniele.barcella@pm.me>
---
drivers/usb/core/sysfs.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 23f3cb1989f4..dc191fa94372 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -1154,20 +1154,39 @@ static ssize_t interface_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(interface);
+static void ascii_filter(char *d, const char *s)
+{
+ /* Filter out characters we don't want to see in the modalias string */
+ for (; *s; s++)
+ if (*s > ' ' && *s < 127 && *s != ':')
+ *(d++) = *s;
+
+ *d = 0;
+}
+
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct usb_interface *intf;
struct usb_device *udev;
struct usb_host_interface *alt;
+ char *manufacturer;
+ int emit;
intf = to_usb_interface(dev);
udev = interface_to_usbdev(intf);
alt = READ_ONCE(intf->cur_altsetting);
- return sysfs_emit(buf,
+ if (udev->manufacturer) {
+ manufacturer = kmalloc(strlen(udev->manufacturer) + 1, GFP_KERNEL);
+ ascii_filter(manufacturer, udev->manufacturer);
+ } else {
+ manufacturer = kstrdup("", GFP_KERNEL);
+ }
+
+ emit = sysfs_emit(buf,
"usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02X"
- "ic%02Xisc%02Xip%02Xin%02X\n",
+ "ic%02Xisc%02Xip%02Xin%02Xmnf%s\n",
le16_to_cpu(udev->descriptor.idVendor),
le16_to_cpu(udev->descriptor.idProduct),
le16_to_cpu(udev->descriptor.bcdDevice),
@@ -1177,7 +1196,11 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
alt->desc.bInterfaceClass,
alt->desc.bInterfaceSubClass,
alt->desc.bInterfaceProtocol,
- alt->desc.bInterfaceNumber);
+ alt->desc.bInterfaceNumber,
+ manufacturer
+ );
+ kfree(manufacturer);
+ return emit;
}
static DEVICE_ATTR_RO(modalias);
--
2.47.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output
2025-07-06 14:27 [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output Daniele Barcella
@ 2025-07-07 8:00 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2025-07-07 8:00 UTC (permalink / raw)
To: Daniele Barcella; +Cc: linux-usb, linux-kernel
On Sun, Jul 06, 2025 at 02:27:26PM +0000, Daniele Barcella wrote:
> The patch improves the usb modalias sysfs output by including the manufacturer string ath the end in the format 'mnf%s' after filtering the string, like how it is done for the dmi id device.
>
> For example usb:v0483p5740d0100dcEFdsc02dp01ic02isc02ip00in00mnfFlipperDevicesInc.
>
> This change allows hwdb rules to target devices with the same vid+pid but different manufacturer.
> For example, the STMicroelectronics Virtual COM Port (0483:5740) is widely used in both prototyping and production devices that don't have a unique vendor ID.
Nit, please use 72 columns for your changelog.
>
> For further context, refer to the related discussion on systemd's GitHub: https://github.com/systemd/systemd/pull/24869.
As random links can disappear at any time, and the text in a kernel
changelog will endure for forever, please put all relevant information
in the changelog itself, don't require people to go look elesewhere and
hope it is still relevant.
>
> Signed-off-by: Daniele Barcella <daniele.barcella@pm.me>
> ---
> drivers/usb/core/sysfs.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> index 23f3cb1989f4..dc191fa94372 100644
> --- a/drivers/usb/core/sysfs.c
> +++ b/drivers/usb/core/sysfs.c
> @@ -1154,20 +1154,39 @@ static ssize_t interface_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(interface);
>
> +static void ascii_filter(char *d, const char *s)
> +{
> + /* Filter out characters we don't want to see in the modalias string */
> + for (; *s; s++)
> + if (*s > ' ' && *s < 127 && *s != ':')
> + *(d++) = *s;
> +
> + *d = 0;
> +}
We don't have a common kernel function for this?
> +
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct usb_interface *intf;
> struct usb_device *udev;
> struct usb_host_interface *alt;
> + char *manufacturer;
> + int emit;
>
> intf = to_usb_interface(dev);
> udev = interface_to_usbdev(intf);
> alt = READ_ONCE(intf->cur_altsetting);
>
> - return sysfs_emit(buf,
> + if (udev->manufacturer) {
> + manufacturer = kmalloc(strlen(udev->manufacturer) + 1, GFP_KERNEL);
> + ascii_filter(manufacturer, udev->manufacturer);
> + } else {
> + manufacturer = kstrdup("", GFP_KERNEL);
> + }
> +
> + emit = sysfs_emit(buf,
> "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02X"
> - "ic%02Xisc%02Xip%02Xin%02X\n",
> + "ic%02Xisc%02Xip%02Xin%02Xmnf%s\n",
> le16_to_cpu(udev->descriptor.idVendor),
> le16_to_cpu(udev->descriptor.idProduct),
> le16_to_cpu(udev->descriptor.bcdDevice),
> @@ -1177,7 +1196,11 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> alt->desc.bInterfaceClass,
> alt->desc.bInterfaceSubClass,
> alt->desc.bInterfaceProtocol,
> - alt->desc.bInterfaceNumber);
> + alt->desc.bInterfaceNumber,
> + manufacturer
> + );
> + kfree(manufacturer);
> + return emit;
I really do not like this change, sorry. Vendors are supposed to go and
get their own product id if they are a vendor-specific device, they
should NOT be attempting to trigger off of USB device strings as those
are not guaranteed to relate to anything at all.
This adds a new user/kernel api, just to work around a userspace tool
that can be easily modified to handle this one specific broken device
that is lying about its vendor/product id. Please go fix this in
userspace with a simple script that udev can run to determine what the
device string contains, and don't require the kernel to do this work
instead.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-07-07 8:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-06 14:27 [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output Daniele Barcella
2025-07-07 8:00 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).