[PATCH] usb: core: improve handling of hubs with no ports

[Alan Stern]

On Thu, Feb 24, 2022 at 09:16:05PM +0100, Heiner Kallweit wrote:
> On 24.02.2022 21:06, Jack Pham wrote:
> > On Wed, Feb 23, 2022 at 05:13:03PM -0500, Alan Stern wrote:
> >> On Wed, Feb 23, 2022 at 09:58:56PM +0100, Heiner Kallweit wrote:
> >>> On 23.02.2022 15:17, Alan Stern wrote:
> >>>> Don't change register_root_hub().  Just change xhci_plat_probe(); make 
> >>>> it skip the second call to usb_add_hcd() if there are no USB-3 ports.
> > 
> > I believe this had been attempted in the past, but it does not appear
> > that patch was ever accepted:
> > 
> > https://lore.kernel.org/linux-usb/1517221474-19627-1-git-send-email-tqnguyen@apm.com/
> > 
> I also found that xhci at several places relies on a proper shared_hcd,
> even if there are no USB3 ports. Therefore maybe go with the less invasive
> original version of my patch?
> 
> https://www.spinics.net/lists/linux-usb/msg222998.html

The patch that Jack refers to, written by Tung Nguyen, does always 
create the shared_hcd.  It simply avoids registering the shared_hcd 
when there are no USB-3 ports.

You should try that patch and see if it works on your system.

Alan Stern

> > Jack
> > 
> >>> This works on my system. However a consequence is that xhci->shared_hcd
> >>> is NULL.
> >>
> >> Why is that?  xhci->shared_hcd doesn't get set in usb_add_hcd(), so 
> >> skipping that call shouldn't cause it to be NULL.
> >>
> >> Note: If you skip calling usb_add_hcd(), you will also have to skip the 
> >> corresponding call to usb_remove_hcd().  There may be a few more 
> >> subtleties involved as well; like I said before, I'm not an expert on 
> >> this driver.  You should ask the xhci-hcd maintainer for advice.
> >>
> >> Alan Stern
> >>
> >>>  There are a few places like the following in xhci.c where
> >>> this may result in a NPE. Not knowing the USB subsystem in detail
> >>> I can't say whether these places are in any relevant path.
> >>>
> >>> static int xhci_run_finished(struct xhci_hcd *xhci)
> >>> {
> >>>         if (xhci_start(xhci)) {
> >>>                 xhci_halt(xhci);
> >>>                 return -ENODEV;
> >>>         }
> >>>         xhci->shared_hcd->state = HC_STATE_RUNNING;
> >>>
> >>>
> >>>
> >>>> Alan Stern
> >>>>
> >>>>> What I can do: submit my patches as RFC, then there's a better basis
> >>>>> for a discussion.
> >>>>>
> >>>>>> Alan Stern
> >>>>>
> >>>>> Heiner
> >>>
> >>> Heiner
>