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

[Heiner Kallweit <hkallweit1@gmail.com>]

On 23.02.2022 15:17, Alan Stern wrote:
> On Wed, Feb 23, 2022 at 01:26:23PM +0100, Heiner Kallweit wrote:
>> On 23.02.2022 03:10, Alan Stern wrote:
>>> On Tue, Feb 22, 2022 at 10:13:09PM +0100, Heiner Kallweit wrote:
>>>>
>>>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>>>> index 83b5aff25..e3f40d1f4 100644
>>>> --- a/drivers/usb/core/hub.c
>>>> +++ b/drivers/usb/core/hub.c
>>>> @@ -1423,9 +1423,8 @@ static int hub_configure(struct usb_hub *hub,
>>>>  		ret = -ENODEV;
>>>>  		goto fail;
>>>>  	} else if (hub->descriptor->bNbrPorts == 0) {
>>>> -		message = "hub doesn't have any ports!";
>>>> -		ret = -ENODEV;
>>>> -		goto fail;
>>>> +		dev_info(hub_dev, "hub has no ports, exiting\n");
>>>> +		return -ENODEV;
>>>>  	}
>>>>  
>>>>  	/*
>>>
>>> How about instead changing xhci-hcd so that it doesn't try to register 
>>> a USB-3 root hub if the controller doesn't have any USB-3 ports?  I 
>>> think that would make more sense.
>>>
>> Right, this would be better. I checked and it seems to be a little bit
>> bigger endeavor. If I let register_root_hub() fail, then this removes
>> the USB3 bus/host (shared hcd), but also the USB2 bus/host.
>> It took an additional change to xhci_plat_probe() to make it work on my
>> system. Not sure what the impact could be on systems not using
>> xhci_plat_probe(). Users may face the same issue like me, and having
>> a USB3 hub with no ports may remove also the USB2 bus/host.
> 
> 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.
> 
This works on my system. However a consequence is that xhci->shared_hcd
is NULL. 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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic