Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18.01.2018 09:27, Tung Vuong Nguyen wrote:
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.


original xhci_plat probe:
-------------------------
usb_create_hcd(hcd)  // USB2
usb_create_hcd(shared) // USB3 xhci->shared_hcd exists now, and hcd->primary is set for the USB3 hcd
usb_add_hcd(hcd)  // USB2
  hcd->driver->start(hcd)  // = .xhci_run()
   if (!usb_hcd_is_primary_hcd(hcd))   // FALSE, this is the primary USB2
   else
   do other setup stuff
usb_add_hcd(shared)  // USB3
  hcd->driver->start(shared)  // = .xhci_run()
   if (!usb_hcd_is_primary_hcd(hcd))   // TRUE, this is the secondary USB3
     xhci_run_finished()
       xhci_start()
         actually write bit to xHC that starts conrtoller.

so xHC HW is not really processing anyting before everything is ready

xhci_plat probe after this patch:
---------------------------------
usb_create_hcd(hcd)  // USB2
usb_add_hcd(hcd)  // USB2
  hcd->driver->start(hcd)  // = .xhci_run()
  if (!xhci->shared_hcd)   // TRUE, shared HCD is not yet created
    if (xhci_start(xhci))
       actually write bit to xHC that starts controller.  // <------------ THIS IS THE PROBLEM
usb_create_hcd(shared) // USB3 xhci->shared_hcd exists now
usb_add_hcd(shared)  // USB3
  hcd->driver->start(shared)  // = .xhci_run()
  ...

So there is a time when xHC HW is processing but the USB3 side in driver is not properly set up yet.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux