On Thu, May 25, 2023 at 5:50 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, May 25, 2023 at 01:31:17PM -0700, Roy Luo wrote:
> > On Thu, May 25, 2023 at 12:10 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, May 25, 2023 at 11:46:23AM -0700, Roy Luo wrote:
> > > > Alan, thanks for the quick response!
> > > > Yes, port_dev->state is indeed the same as port_dev->child->state. However,
> > > > I still add port_dev->state because port_dev->child won't be assigned until
> > > > the corresponding usb_device is in ADDRESS state.
> > > > I wish I can assign get port_dev->child assigned earlier, but I think
> > > > the current design - assign port_dev->child and device_add() after ADDRESS
> > > > state - also makes sense because there are many ways that the enumeration
> > > > could fail in the early stage. By adding port_dev->state, I can link
> > > > usb_device->state to usb_port as soon as the usb_device is created to get
> > > > around the limitation of port_dev->child.
> > > > I would be very happy to hear other ideas.
> > >
> > > Is there any real reason not to set port_dev->child as soon as the
> > > usb_device structure is created? If enumeration fails, the pointer can
> > > be cleared.
> > >
> > > Alan Stern
> >
> > Currently the usb core assumes the usb_device that port_dev->child points
> > to is enumerated and port_dev->child->dev is registered when
> > port_dev->child is present. Setting port_dev->child early would break this
> > fundamental assumption, hence I'm a bit reluctant to go this way.
>
> Well, you could remove that assumption by adding a "child_is_registered"
> flag and explicitly checking it.
>
> Alan Stern
Agree that's doable, with the following overheads:
1. We can no longer safely access port_dev->child without checking
"child_is_registered", and there are plenty of places in the usb core that
touch port_dev->child. The implicit assumption could also hurt code
maintainability.
2. In the worst case where enumeration keeps failing, the retry loop in
hub_port_connect() would frequently hold device_state_lock in order
to link/unlink the usb_device to port_dev->child.
This would definitely make sense if more places need port_dev->child early.
However, we only need port_dev->child->state at this point, so it does not
seem like a good deal to me.
On Fri, May 26, 2023 at 12:42 AM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, May 25, 2023 at 05:38:18PM +0000, Roy Luo wrote:
> > + * @work: workqueue for sysfs_notify()
>
> Do you really need this? This should be possible to call in any context
> as kernfs_notify() says that it is safe to do that, right?
>
Thanks for pointing this out!
Yes, kernfs_notify() or sysfs_notify_dirent() should work, will take
that into the next patch.
> Also, what userspace code is now calling poll() on your new sysfs file?
>
> thanks,
>
> greg k-h
We are looking at adding the code to the generic userspace components
if not hardware abstraction layer in the userspace.
Thanks,
Roy
