Re: [PATCH] usb: core: notify unrecognized usb device

Re: [PATCH] usb: core: notify unrecognized usb device

[Greg KH]

On Fri, Feb 21, 2025 at 10:29:49AM +0000, Akshay Gujar wrote:
> Description: To send uevent for unrecognized device connected on system.

Odd format here, have you read the documentation of the kernel process
in how to write a changelog?  I recommend a quick glance at the section
"The canonical patch format" in the kernel file,
Documentation/process/submitting-patches.rst for details.

> As per the usb compliance, USB-IF enforces a "no silent failure" rule.
> This means that an implementation of USB must not appear broken to the
> consumer. In configurations where the consumer's expectations are not
> met, either the peripheral or host must provide appropriate and useful
> feedback to the consumer regarding the problem.
> 
> Link: https://compliance.usb.org/index.asp?UpdateFile=Embedded%20Host&Format=Standard#10

Odd, many Linux devices have passed usb-if testing since 2005 when this
was made a "rule", how did that happen?  What recently changed to
suddenly require this be a kernel issue?

And does usb-if even matter these days?  You do know what they think
about Linux overall, right (hint, they kicked us out from
participating...) so why should we follow their "requirements" when they
do not allow us to even participate or provide feedback when they create
them?

> Signed-off-by: Akshay Gujar <Akshay.Gujar@harman.com>
> ---
>  drivers/usb/core/hub.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c3f839637..d00129b59 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5343,6 +5343,26 @@ static int descriptors_changed(struct usb_device *udev,
>  	return changed;
>  }
>  
> +static void unrecognized_usb_device_notify(struct usb_port *port_dev)
> +{
> +	char *envp[2] = { NULL, NULL };
> +	struct device *hub_dev;
> +
> +	hub_dev = port_dev->dev.parent;
> +
> +	if (!hub_dev)
> +		return;

How can this be true?

> +
> +	envp[0] = kasprintf(GFP_KERNEL, "UNRECOGNIZED_USB_DEVICE_ON_PORT=%s",
> +				kobject_name(&port_dev->dev.kobj));

Hint, if a driver ever starts calling into kobject or sysfs functions,
usually something is wrong.  This should just use dev_name(), right?

> +	if (!envp[0])
> +		return;
> +
> +	kobject_uevent_env(&hub_dev->kobj, KOBJ_CHANGE, envp);

Where is this new uevent documented?  What userspace tool will see this
and do something about it?  How was this tested?

> +
> +	kfree(envp[0]);
> +}
> +
>  static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
>  		u16 portchange)
>  {
> @@ -5569,9 +5589,11 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
>  	if (hub->hdev->parent ||
>  			!hcd->driver->port_handed_over ||
>  			!(hcd->driver->port_handed_over)(hcd, port1)) {
> -		if (status != -ENOTCONN && status != -ENODEV)
> +		if (status != -ENOTCONN && status != -ENODEV) {
>  			dev_err(&port_dev->dev,
>  					"unable to enumerate USB device\n");
> +			unrecognized_usb_device_notify(port_dev);

This is only if a hub acts up with talking to a device, it does not mean
the device was not supported at all.  So this isn't going to meet the
standard that you describe above.  Userspace is really the only thing
that can know if a device is "supported" or not, not the kernel.

thanks,

greg k-h

Re: [PATCH] usb: core: notify unrecognized usb device

[Greg KH]

On Tue, Aug 26, 2025 at 04:52:44PM +0000, Akshay Gujar wrote:
> Sorry for delayed response.
> 
> On Fri, Feb 21, 2025 11:53:01AM +0100, Greg KH wrote:

That was many months :)

> >> As per the usb compliance, USB-IF enforces a "no silent failure" rule.
> >> This means that an implementation of USB must not appear broken to the
> >> consumer. In configurations where the consumer's expectations are not
> >> met, either the peripheral or host must provide appropriate and useful
> >> feedback to the consumer regarding the problem.
> >> 
> >> Link: https://compliance.usb.org/index.asp?UpdateFile=Embedded%20Host&Format=Standard#10
> 
> >Odd, many Linux devices have passed usb-if testing since 2005 when this
> >was made a "rule", how did that happen?  What recently changed to
> >suddenly require this be a kernel issue?
> 
> Previously, OEMs handled this with private kernel patches or custom modifications. 
> However, with Android's Generic Kernel Image (GKI) initiative, vendors can no longer make arbitrary kernel modifications. 
> GKI requires using a common upstream kernel, so functionality like this needs to be up streamed rather than handled with vendor-specific patches.
> This patch provides a standard upstream solution for what was previously handled with custom kernel modifications.

That's good, but that does not mean that what you are attempting to do
really is the correct thing to do.  Here you were trying to say that
this is a requirement of USB-IF, but it really is not.  This is just
wanting to add a new feature to the USB core that previously was only
out-of-tree for your devices.  Please be more specific in your
description of the problem and issues involved.

> >And does usb-if even matter these days?  You do know what they think
> >about Linux overall, right (hint, they kicked us out from
> >participating...) so why should we follow their "requirements" when they
> >do not allow us to even participate or provide feedback when they create
> >them?
> 
> I understand your frustration with USB-IF's treatment of Linux.
> Rather than frame this as following USB-IF requirements, this patch addresses a practical Automotive ecosystem need: providing userspace notification of USB enumeration failures.
> However, this patch isn't really about following USB-IF requirements - it's about providing useful functionality.

Then don't say it has anything to do with USB-IF if it does not.

> >> ---
> >>  drivers/usb/core/hub.c | 24 +++++++++++++++++++++++-
> >>  1 file changed, 23 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index c3f839637..d00129b59 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -5343,6 +5343,26 @@ static int descriptors_changed(struct usb_device *udev,
> >>  	return changed;
> >>  }
> >>  
> >> +static void unrecognized_usb_device_notify(struct usb_port *port_dev)
> >> +{
> >> +	char *envp[2] = { NULL, NULL };
> >> +	struct device *hub_dev;
> >> +
> >> +	hub_dev = port_dev->dev.parent;
> >> +
> >> +	if (!hub_dev)
> >> +		return;
> 
> >How can this be true?
> 
> You're absolutely right. This check is unnecessary. I'll remove this in v2.
> 
> >> +
> >> +	envp[0] = kasprintf(GFP_KERNEL, "UNRECOGNIZED_USB_DEVICE_ON_PORT=%s",
> >> +				kobject_name(&port_dev->dev.kobj));
> 
> >Hint, if a driver ever starts calling into kobject or sysfs functions,
> >usually something is wrong.  This should just use dev_name(), right?
> 
> Correct! I'll change this to use dev_name(&port_dev->dev) in v2.
> 
> >> +	if (!envp[0])
> >> +		return;
> >> +
> >> +	kobject_uevent_env(&hub_dev->kobj, KOBJ_CHANGE, envp);
> 
> >Where is this new uevent documented?  What userspace tool will see this
> >and do something about it?  How was this tested?
> 
> I'll add documentation to Documentation/ABI/testing/ describing 
> the uevent format and intended consumers.
> 
> For testing: I used "udevadm monitor --property" to verify uevent 
> generation during enumeration failures.
> 
> For Android usage: Our USB HAL service uses a NetlinkListener to 
> capture these events and provide user feedback for connection issues.
> 
> >> +
> >> +	kfree(envp[0]);
> >> +}
> >> +
> >>  static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
> >>  		u16 portchange)
> >>  {
> >> @@ -5569,9 +5589,11 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
> >>  	if (hub->hdev->parent ||
> >>  			!hcd->driver->port_handed_over ||
> >>  			!(hcd->driver->port_handed_over)(hcd, port1)) {
> >> -		if (status != -ENOTCONN && status != -ENODEV)
> >> +		if (status != -ENOTCONN && status != -ENODEV) {
> >>  			dev_err(&port_dev->dev,
> >>  					"unable to enumerate USB device\n");
> >> +			unrecognized_usb_device_notify(port_dev);
> 
> >This is only if a hub acts up with talking to a device, it does not mean
> >the device was not supported at all.  So this isn't going to meet the
> >standard that you describe above.  Userspace is really the only thing
> >that can know if a device is "supported" or not, not the kernel.
> 
> I mischaracterized this. This detects enumeration failures, not unsupported devices. 

That's a very big difference.  Enumeration failures happen all the time
due to horrible cables and other hardware issues.  If you are now going
to flood userspace with this information, it better be ready to handle
it and do something with it.

But, for an enumeration failure, you can't do anything with it, so why
report it at all?

> Userspace determines device support. I'll rename the function to "usb_enumeration_failure_notify" and update.
> The use case is for USB-IF compliance testing (DevNoResponse tests) where test equipment needs to verify enumeration failure detection. 

There is no such requirement for the kernel to provide this information,
why can't you just do this all in userspace with the information that
you have?  You know if a device is active or bound to a driver properly,
why not just rely on that instead of making the kernel do something
different here?

thanks,

greg k-h

Re: [PATCH] usb: core: notify unrecognized usb device

[Oliver Neukum]

Hi,

On 9/6/25 14:28, Greg KH wrote:
> That's a very big difference.  Enumeration failures happen all the time
> due to horrible cables and other hardware issues.  If you are now going
> to flood userspace with this information, it better be ready to handle
> it and do something with it.
> 
> But, for an enumeration failure, you can't do anything with it, so why
> report it at all?

that is probably not true. For once you can try another cable in many cases.
Currently we'd log this information. That is a worse way to handle this kind
of failure.
If there is an unrecoverable IO error, user space ought to be notified.
Syslog is not the best way to do so. There ought to be a standardized way
of doing this. However, this makes me say that this issue is not really
confined to USB. Other hotpluggable busses have the same issue.

	Regards
		Oliver

Re: [PATCH] usb: core: notify unrecognized usb device

[Greg KH]

On Mon, Sep 08, 2025 at 10:58:35AM +0200, Oliver Neukum wrote:
> Hi,
> 
> On 9/6/25 14:28, Greg KH wrote:
> > That's a very big difference.  Enumeration failures happen all the time
> > due to horrible cables and other hardware issues.  If you are now going
> > to flood userspace with this information, it better be ready to handle
> > it and do something with it.
> > 
> > But, for an enumeration failure, you can't do anything with it, so why
> > report it at all?
> 
> that is probably not true. For once you can try another cable in many cases.
> Currently we'd log this information. That is a worse way to handle this kind
> of failure.
> If there is an unrecoverable IO error, user space ought to be notified.
> Syslog is not the best way to do so. There ought to be a standardized way
> of doing this. However, this makes me say that this issue is not really
> confined to USB. Other hotpluggable busses have the same issue.

Yes, all busses have this type of issue for when devices can not be
enumerated or fail.  We shouldn't make something that only works for
USB.  I think PCI reports this type of thing somehow, so maybe
generalize that api?

thanks,

greg k-h

Re: [PATCH] usb: core: notify unrecognized usb device

[Greg KH]

On Thu, Sep 18, 2025 at 05:23:55PM +0000, Akshay Gujar wrote:
> >On Sat, Sep 6, 2025 14:28:37AM +0200, Greg KH wrote:
> 
> > >>> As per the usb compliance, USB-IF enforces a "no silent failure" rule.
> > >>> This means that an implementation of USB must not appear broken to the
> > >>> consumer. In configurations where the consumer's expectations are not
> > >>> met, either the peripheral or host must provide appropriate and useful
> > >>> feedback to the consumer regarding the problem.
> > >>> 
> > >>> Link: https://compliance.usb.org/index.asp?UpdateFile=Embedded%20Host&Format=Standard#10
> > >
> > >Odd, many Linux devices have passed usb-if testing since 2005 when this
> > >was made a "rule", how did that happen?  What recently changed to
> > >suddenly require this be a kernel issue?
> > > 
> > > Previously, OEMs handled this with private kernel patches or custom modifications. 
> > > However, with Android's Generic Kernel Image (GKI) initiative, vendors can no longer make arbitrary kernel modifications. 
> > > GKI requires using a common upstream kernel, so functionality like this needs to be up streamed rather than handled with vendor-specific patches.
> > > This patch provides a standard upstream solution for what was previously handled with custom kernel modifications.
> 
> > That's good, but that does not mean that what you are attempting to do
> > really is the correct thing to do.  Here you were trying to say that
> > this is a requirement of USB-IF, but it really is not.  This is just
> > wanting to add a new feature to the USB core that previously was only
> > out-of-tree for your devices.  Please be more specific in your
> > description of the problem and issues involved.
> 
> To clarify, this patch targets two needs: 
> it helps with USB-IF compliance testing (where enumeration failures must be visible to userspace), 

Why must it be visible to userspace?  How have we passed testing before
now?

> and it provides a generic mechanism for userspace to be notified of USB enumeration failures, 
> which is currently missing in the USB core. This solution standardizes the notification, 
> especially important for Android GKI and similar environments.

But there is no user for this new user/kernel api anywhere.  Android
does not do this today, what is the chance it really will use it?  Do
you have working patches somewhere that will land if this is added?

And where is any of this being documented? :)

> >> that is probably not true. For once you can try another cable in many cases.
> >> Currently we'd log this information. That is a worse way to handle this kind
> >> of failure.
> >> If there is an unrecoverable IO error, user space ought to be notified.
> >> Syslog is not the best way to do so. There ought to be a standardized way
> >> of doing this. However, this makes me say that this issue is not really
> >> confined to USB. Other hotpluggable busses have the same issue.
> 
> >Yes, all busses have this type of issue for when devices can not be
> >enumerated or fail.  We shouldn't make something that only works for
> >USB.  I think PCI reports this type of thing somehow, so maybe
> >generalize that api?
> 
> Agreed, this is a broader issue across hotpluggable busses. Generalizing the API to cover enumeration failures for USB, PCI, and others would be ideal. 
> For now, this patch addresses USB specifically due to immediate needs.

Great, but again, why not make it generic so that all can use it?

Otherwise we end up with a USB-specific solution that no one else can
ever use in the future.

thanks,

greg k-h