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

From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Alan Stern <stern@rowland.harvard.edu>,
	Heiner Kallweit <hkallweit1@gmail.com>
Cc: Jack Pham <quic_jackp@quicinc.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux USB Mailing List <linux-usb@vger.kernel.org>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	Tung Nguyen <tunguyen@apm.com>,
	Mathias Nyman <mathias.nyman@intel.com>
Subject: Re: [PATCH] usb: core: improve handling of hubs with no ports
Date: Thu, 3 Mar 2022 13:50:02 +0200	[thread overview]
Message-ID: <4b0c7f0d-749b-a8a8-9b25-720ed826ae2a@linux.intel.com> (raw)
In-Reply-To: <YhfougN/KOTZeGV7@rowland.harvard.edu>

On 24.2.2022 22.21, Alan Stern wrote:
> 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.

I think we need to start supporting xHC conreollers with just one roothub. 
Only call usb_add_hcd() once in those cases.

Even prepare for special cases where xHCI only has usb3 ports (usb2 pins routed
to a different host controller)

Currently driver reads port capabilities in:
usb_add_hcd()
  hcd->driver->reset   ...-> xhci_gen_setup()
    xhci_gen_setup()
      xhci_init(hcd)
        xhci_mem_init()
          xhci_setup_port_arrays()

Driver makes some assumptions based on if hcd is primary or not early in
xhci_gen_setup(), and initializes values like hcd->speed = HCD_USB2;

Should be doable, changes needed in at least xhci_run(), xhci_stop(),
xhci_resume(), xhci_suspend(), xhci_gen_setup() and probe.

-Mathias

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