Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 於 2021年3月3日 週三 下午5:10寫道： > > 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? > I will create a sysfs attribute to record the hub status. If there is a new hub with over 6 USB TOPO layer connected, I will use the sysfs_notify to send the "error conditions" to userspace. Is it a proper interfaces to delivery "error conditions"? > > 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 Thanks, Chien Kun Niu
