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

Re: [PATCH v2 1/3] driver core: add device_enumeration_failure_notify() helper

[Greg KH]

On Wed, Dec 24, 2025 at 11:58:06AM +0000, Akshay Gujar wrote:
> Hotpluggable buses can detect that a device is physically present, but
> enumeration may still fail early due to protocol-level errors. Today,
> such failures are only reported via kernel log messages, with no
> structured userspace notification.
> 
> Introduce device_enumeration_failure_notify(), a generic helper that
> emits a KOBJ_CHANGE uevent containing:
> 
>     DEVICE_ENUMERATION_FAILURE=<identifier>
> 
> The <identifier> string is provided by the bus layer and identifies the
> failing port or device instance in a bus-defined format.
> 
> This allows userspace to correlate repeated enumeration failures with
> specific ports or connectors without relying solely on kernel logs.
> 
> Signed-off-by: Akshay Gujar <Akshay.Gujar@harman.com>
> ---
>  drivers/base/core.c    | 30 ++++++++++++++++++++++++++++++
>  include/linux/device.h | 12 ++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 40de2f51a1b1..4c70d9a6dc69 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3747,6 +3747,36 @@ int device_add(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(device_add);
>  
> +/**
> + * device_enumeration_failure_notify - send uevent for enumeration failure
> + * @parent: the device to send the uevent from
> + * @id_name: textual identifier for the failing device
> + *
> + * Emits a KOBJ_CHANGE uevent with:
> + *
> + *    DEVICE_ENUMERATION_FAILURE=<id_name>
> + *
> + * Buses such as USB/PCI may use this helper when hardware is detected
> + * but enumeration cannot proceed.
> + */
> +void device_enumeration_failure_notify(struct device *parent, const char *id_name)
> +{
> +	char *envp[2] = { NULL, NULL };
> +
> +	if (!parent || !id_name)
> +		return;
> +
> +	envp[0] = kasprintf(GFP_KERNEL,
> +			    "DEVICE_ENUMERATION_FAILURE=%s",
> +			    id_name);
> +	if (!envp[0])
> +		return;
> +
> +	kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, envp);
> +	kfree(envp[0]);
> +}
> +EXPORT_SYMBOL_GPL(device_enumeration_failure_notify);
> +
>  /**
>   * device_register - register a device with the system.
>   * @dev: pointer to the device structure
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..dedc5e9e0ade 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1197,4 +1197,16 @@ static inline bool device_link_test(const struct device_link *link, u32 flags)
>  #define MODULE_ALIAS_CHARDEV_MAJOR(major) \
>  	MODULE_ALIAS("char-major-" __stringify(major) "-*")
>  
> +/**
> + * device_enumeration_failure_notify - notify userspace about enumeration failure
> + * @parent: device to emit the uevent from

Why is this called "parent"?  SHouldn't this just be the device that
caused the failure?

> + * @id_name: textual identifier for the failed endpoint/device instance

Any hints on what this is going to contain?  Should this be documented
somewhere?

thanks,

greg k-h

Re: [PATCH v2 0/3] Generic device enumeration failure notification

[Greg KH]

On Wed, Dec 24, 2025 at 11:58:05AM +0000, Akshay Gujar wrote:
> This series is a revised version of the earlier patch, updated in response
> to review feedback in the discussion starting at:
> 
>   <2025100805-resisting-target-419a@gregkh>
> 
> The intent is to provide a small, generic mechanism for notifying
> userspace when device enumeration fails after hardware is detected but
> before enumeration completes.
> 
> Some devices may be detected electrically but fail to enumerate due to
> protocol-level errors or invalid responses. Today, such failures are
> reported only via kernel log messages. This series introduces a
> structured uevent notification that allows userspace to observe these
> failures without embedding policy in the kernel.
> 
> Patch overview:
> 
>   1/3 driver-core: add device_enumeration_failure_notify() helper
>       Adds a generic helper in the driver core to emit a KOBJ_CHANGE
>       uevent containing DEVICE_ENUMERATION_FAILURE=<identifier>.
> 
>   2/3 Documentation: ABI: document DEVICE_ENUMERATION_FAILURE uevent
>       Documents the new uevent and includes an example captured on USB.
> 
>   3/3 usb: hub: send enumeration failure uevent
>       Uses the generic helper in the USB hub enumeration failure path.
> 
> The USB change is intentionally minimal and serves as an initial user of
> the generic helper. Other subsystems may use the helper independently if
> needed.
> 
> Akshay Gujar (3):
>   driver core: add device_enumeration_failure_notify() helper
>   Documentation: ABI: document DEVICE_ENUMERATION_FAILURE uevent
>   usb: hub: send enumeration failure uevent
> 
>  Documentation/ABI/testing/sysfs-uevent | 22 +++++++++++++++++++
>  drivers/base/core.c                    | 30 ++++++++++++++++++++++++++
>  drivers/usb/core/hub.c                 |  5 ++++-
>  include/linux/device.h                 | 12 +++++++++++
>  4 files changed, 68 insertions(+), 1 deletion(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v2 2/3] Documentation: ABI: document DEVICE_ENUMERATION_FAILURE uevent

[Greg KH]

On Wed, Dec 24, 2025 at 11:58:07AM +0000, Akshay Gujar wrote:
> Document the DEVICE_ENUMERATION_FAILURE uevent emitted when device
> enumeration fails, including a minimal example captured on USB.
> 
> Signed-off-by: Akshay Gujar <Akshay.Gujar@harman.com>
> ---
>  Documentation/ABI/testing/sysfs-uevent | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-uevent b/Documentation/ABI/testing/sysfs-uevent
> index 0b6227706b35..1f1573c8be96 100644
> --- a/Documentation/ABI/testing/sysfs-uevent
> +++ b/Documentation/ABI/testing/sysfs-uevent
> @@ -49,3 +49,25 @@ Description:
>  
>  Users:
>                  udev, userspace tools generating synthetic uevents
> +
> +What:		DEVICE_ENUMERATION_FAILURE
> +Date:		Dec 2025

It's not December anymore :)

> +KernelVersion:	6.x

Are you sure this will happen?  And the major number means nothing,
let's pick the real release number please.

thanks,

greg k-h

Re: [PATCH v2 3/3] usb: hub: send enumeration failure uevent

[Greg KH]

On Wed, Dec 24, 2025 at 11:58:08AM +0000, Akshay Gujar wrote:
> Use the device_enumeration_failure_notify() helper when USB device
> enumeration fails. This supplements the existing kernel log message with
> a structured userspace-visible notification identifying the affected
> port.
> 
> Signed-off-by: Akshay Gujar <Akshay.Gujar@harman.com>
> ---
>  drivers/usb/core/hub.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index be50d03034a9..c1963b1bb9fb 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5613,9 +5613,12 @@ 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");
> +			device_enumeration_failure_notify(port_dev->dev.parent,

Why use the parent here?  Normally devices can NOT access their parent
pointer "safely" so this feels odd.  Why not have the device itself be
the one that emits this, as that's what dev_err() just gave us.

> +							  dev_name(&port_dev->dev));

Why not just get the name directly from the device you are going to pass
in?  It shouldn't then be needed again, right?

thanks,

greg k-h