Re: [PATCH v10 2/5] USB: misc: Add onboard_usb_hub driver

[Matthias Kaehlcke <mka@chromium.org>]

On Wed, May 19, 2021 at 10:43:56AM -0400, Alan Stern wrote:
> On Tue, May 18, 2021 at 02:40:47PM -0700, Matthias Kaehlcke wrote:
> > 
> > Could you also have a look at "[4/5] usb: host: xhci-plat:
> > Create platform device for onboard hubs in probe()"
> > (https://lore.kernel.org/patchwork/patch/1425453/)? It's a
> > relatively short patch that creates the platform device for
> > the driver from xhci-plat as you suggested in the v4
> > discussion.
> 
> I'm not the maintainer for xhci-related drivers.
> 
> However, there is at least one thing about this patch which looks 
> suspicious: Adding the onboard_hub_dev pointer to struct usb_hcd instead 
> of to struct xhci_plat_priv, where it would make a lot more sense.

I can move it to struct usb_hcd if that's preferred

> It's also worth mentioning that this approach won't work at all when the 
> onboard hub is not at the top level (its parent isn't the root hub),

Yes, this limitation is mentioned in the commit message of '[2/5] USB:
misc: Add onboard_usb_hub driver'. It shouldn't be hard to add support
for nested hubs, however I currently have no such configuration for
testing, so I prefer to defer it until the need actually arises and
it can be tested.

> or when more than one onboard hubs are connected to the same root hub.

Right, currently that isn't supported. xhci-plat could iterate over the
ports and have a list of the platform devices it owns. It would also
require some logic to make sure only one platform device is created per
hub. Much of this code could probably live in the onboard_hub driver
and would also be used if support for non-root hubs is added.