Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
From: Tung Vuong Nguyen
Date: Thu Jan 18 2018 - 02:28:36 EST
On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
<mathias.nyman@xxxxxxxxxxxxxxx> wrote:
>
> Hi, Sorry about the delay
>
>
> On 04.01.2018 07:17, Thang Q. Nguyen wrote:
>>
>> Hi,
>>
>> On Sat, Dec 16, 2017 at 10:45 AM, Thang Q. Nguyen <tqnguyen@xxxxxxx> wrote:
>>>
>>> From: Tung Nguyen <tunguyen@xxxxxxx>
>>>
>>> Currently, hcd->shared_hcd always creates and registers to the usb-core.
>>> If, for some reasons, USB3 downstream port is disabled, no roothub port for
>>> USB3.0 is found. This causes kernel to display an error:
>>> hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19)
>>> This patch checks, creates and registers shared_hcd if USB3.0 downstream
>>> port is available.
>>>
>>> Signed-off-by: Tung Nguyen <tunguyen@xxxxxxx>
>>> Signed-off-by: Thang Q. Nguyen <tqnguyen@xxxxxxx>
>>> ---
>>> drivers/usb/host/xhci-mem.c | 2 +-
>>> drivers/usb/host/xhci-plat.c | 26 +++++++++++----------
>>> drivers/usb/host/xhci.c | 54 ++++++++++++++++++++++++++++++++------------
>>> 3 files changed, 54 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>>> index 554a8a5..157d1e7 100644
>>> --- a/drivers/usb/host/xhci-mem.c
>>> +++ b/drivers/usb/host/xhci-mem.c
>>> @@ -1067,7 +1067,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
>>> struct usb_device *top_dev;
>>> struct usb_hcd *hcd;
>>>
>>> - if (udev->speed >= USB_SPEED_SUPER)
>>> + if (udev->speed >= USB_SPEED_SUPER && xhci->shared_hcd)
>>> hcd = xhci->shared_hcd;
>>> else
>>> hcd = xhci->main_hcd;
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 6f03830..e812e3d 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -253,12 +253,6 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>
>>> xhci->clk = clk;
>>> xhci->main_hcd = hcd;
>>> - xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
>>> - dev_name(&pdev->dev), hcd);
>>> - if (!xhci->shared_hcd) {
>>> - ret = -ENOMEM;
>>> - goto disable_clk;
>>> - }
>>>
>>> if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
>>> xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>> @@ -290,12 +284,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>> if (ret)
>>> goto disable_usb_phy;
>>>
>>> - if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> - xhci->shared_hcd->can_do_streams = 1;
>>> -
>>> - ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> - if (ret)
>>> - goto dealloc_usb2_hcd;
>>> + if (xhci->num_usb3_ports > 0) {
>>> + xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
>>> + dev_name(&pdev->dev), hcd);
>>> + if (!xhci->shared_hcd) {
>>> + ret = -ENOMEM;
>>> + goto disable_clk;
>>> + }
>>> + if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> + xhci->shared_hcd->can_do_streams = 1;
>>> +
>>> + ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> + if (ret)
>>> + goto dealloc_usb2_hcd;
>>> + }
>>>
>>> device_enable_async_suspend(&pdev->dev);
>>> pm_runtime_put_noidle(&pdev->dev);
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 05104bd..4824bf6 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -417,12 +417,14 @@ static void compliance_mode_recovery(struct timer_list *t)
>>> i + 1);
>>> xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
>>> "Attempting compliance mode recovery");
>>> - hcd = xhci->shared_hcd;
>>> + if (xhci->shared_hcd) {
>>> + hcd = xhci->shared_hcd;
>>>
>>> - if (hcd->state == HC_STATE_SUSPENDED)
>>> - usb_hcd_resume_root_hub(hcd);
>>> + if (hcd->state == HC_STATE_SUSPENDED)
>>> + usb_hcd_resume_root_hub(hcd);
>>>
>>> - usb_hcd_poll_rh_status(hcd);
>>> + usb_hcd_poll_rh_status(hcd);
>>> + }
>>> }
>>> }
>>>
>>> @@ -611,6 +613,18 @@ int xhci_run(struct usb_hcd *hcd)
>>> if (ret)
>>> xhci_free_command(xhci, command);
>>> }
>>> + /*
>>> + * Execute xhci_start() in case xhci->shared_hcd is not registered.
>>> + * If the xhci->shared_hcd doesn't exist, no one triggers to start
>>> + * the xhci which should be done before exitting run function
>>> + */
>>> + if (!xhci->shared_hcd) {
>>> + if (xhci_start(xhci)) {
>
>
> This probably won't work as primary hcd was added before shared_hcd was created.
> usb_add_hcd(hcd) calls xhci_run() before xhci->shared_hcd exists, so this will
> cause the xHC to start before the shared_hcd is created or setup.
>
> -Mathias
Hi Mathias
I ran the test and saw the patch works fine in 2 cases:
xhci->num_usb3_ports = 0 and xhci->num_usb3_ports > 0.
For the case that we don't have the usb 3.0, the standalone
primary_hcd should work without the share_hcd. This case the root hubs
run at speed 480M.
If we have xhci->num_usb3_ports > 0 then we have 2 times to trigger to
start xHC in xhci_run. Once by the primary_hcd and once by share_hcd.
We will have 5000M root hubs and following by 480M root hubs when
triggering by the share_hcd and as properly.
So the xHC should works when we only have primary_hcd or we have both
primary hcd and share_hcd
Here is the brief of the scenario for this patch:
+ create the primary hcd in xhci-plat
+ execute usb_add_hcd(hcd), execute xhci_run and start the xHC (for primary hcd)
+ if the xhci->num_usb3_ports > 0 then:
+ create the share_hcd structure
+ execute usb_add_hcd(share_hcd), execute xhci_run and start
the xHC again (for share_hcd)
+ The root hub run at 5000M
else if xhci->num_usb3_ports == 0 then:
+ don't create the share_hcd structure
+ don't usb_add_hcd(share_hcd)
+ the xHC can run with root hub 480M
Please share your ideas for something should be changed.