On Wed, Mar 03, 2021 at 05:03:25PM +0800, Chien Kun Niu wrote:

Hi , Greg

What tool will "catch" this?  Where is that code located at?
=> I prepare merge the code to Android phone , so I used Android HLOS
to catch this uevent.

Very odd quoting style, perhaps you might want to read up on how to do
this properly at:
	https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

uevents are not for stuff like this, you are trying to send "error
conditions" to userspace, please use the "proper" interfaces like this
and not abuse existing ones.
=> Sorry , I am not sure what is the "proper" interfaces your mean.
     Could you please give me more description?

How does the kernel normally send error conditions that it detects in
hardware to userspace?

You just created a whole new sysfs class with no Documentation/ABI/
update?
=> Yes, I will add it.

Wait, how did you even test this code?  This will not work if you have
more than one hub in the system at a single time, right?
=> I build the test build which flash in Android phone and connect
several hubs to try it.
     When the new hub which met MAX_TOPO_LEVEL connected , it sent
notify to user space.

Phone ------↓
                 hub ------↓
                             hub ------↓
                                           ...------↓
                                                    hub

     if (hdev->level == MAX_TOPO_LEVEL) {
                dev_err(&intf->dev,
                        "Unsupported bus topology: hub nested too deep\n");
                hub_over_tier();
                return -E2BIG;
     }

But you only have a single hub variable, and a huge memory leak, did you
not detect that in your testing?

So, proof that this works?  How did you test this?
=> I use the Pixel phone to verify the code , the framework received
the uevent when the last connected hub over "MAX_TOPO_LEVEL".

Try it on a desktop as well, with many hubs and see what happens :(

Also, you have a memory leak in this submission :(
=> Do you mean I should add device_destroy here ?

What do you think should be done?

hub_device =
device_create(hub_class, NULL, MKDEV(0, 0), NULL, "usb_hub");
+if (IS_ERR(hub_device))
+               return PTR_ERR(hub_device);

void usb_hub_cleanup(void)
{
+if (!IS_ERR(hub_device))
+device_destroy(hub_class, hub_device->devt);

if (!IS_ERR(hub_class))
class_destroy(hub_class);

I don't think you are understanding that you can have multiple hubs in
the system at the same time :(

thanks,

greg k-h