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
