On 15.11.2017 11:46, Thang Q. Nguyen wrote:
On Wed, Nov 1, 2017 at 4:17 PM, Thang Q. Nguyen <tqnguyen@xxxxxxx> wrote:
Hi,
On Mon, Oct 23, 2017 at 5:27 PM, Mathias Nyman
<mathias.nyman@xxxxxxxxxxxxxxx> wrote:
On 23.10.2017 12:37, Thang Q. Nguyen wrote:
Hi,
In our latest ARM64-based CPU, we use the DesignWare USB which is
xHCI-compatible. For some reasons, we disable USB3.0 support so remove the
unnecessary USB3.0 capability structure information as specified in the
xHCI specification 1.1, section 7.2:
"At least one of these capability structures is required for all xHCI
implementations. More than one may be defined for implementations that
support more that one bus protocol."
So there is no supported protocol capability with revision major == 3, in
your case.
When booting kernel, linux kernel displays an error message to claim about
no USB port on the USB3.0 roothub:
[ 3.497767] hub 2-0:1.0: config failed, hub doesn't have any ports!
(err -19)
Although the error message does not affect USB functionality, error
message will make users confused.
Looking into the XHCI driver, there are always two USB hub ports
implemented. This implements the xHCI specification as in 4.19.7:
"In a USB3 hub, two independently addressable hub ports exist for each
physical down stream connector; a USB2 compatible port accessed through
the USB2 connection and a USB3 compatible port accessed through the
SuperSpeed connection. The Root Hub of the xHCI emulates this operation by
defining a Root Hub PORTSC register for each connection type; USB2
(Low-/Full-/High-Speed) or USB3 (SuperSpeed)."
But the problem is that, xhci-hcd always pass both USB hub ports to
usb-core driver. In case any of USB2 or USB3 compatible port is missing,
there is no port on the corresponding hub and kernel will displays the
error message.
Below are some approaches we suggest to overcome the issue:
Approach 1: Allow roothub has no port. The change will look like:
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b5c7336..995d62d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1347,6 +1347,9 @@ static int hub_configure(struct usb_hub
*hub,
ret = -ENODEV;
goto fail;
} else if (hub->descriptor->bNbrPorts == 0) {
+ if (hdev->parent)
+ return -ENODEV;
+
message = "hub doesn't have any
ports!";
ret = -ENODEV;
goto fail;
Approach 2: do not pass a USB hub port to usb-core if it has no port.
Don't know if this approach is feasible or not.
Approach 3: implement maximum-speed attribute support and this will be set
to high-speed in case of missing USB3 support. But this works only for the
case of USB3.0 disabled.
Can you help review and suggest what approach should be used to fix this
issue?
Current xhci driver design assumes there are two hcds available.
Several functions assume a shared hcd exists.
Creating a hcd with a roothub with zero ports is probably a faster way to
get
things running, but I think we should redesign parts of the xhci driver to
work with only one hcd.
So best would be to never add the secodary hcd if there are no USB 3 ports,
meaning no:
xhci->shared_hcd = usb_create_shared_hcd(..);
usb_add_hcd(xhci->shared_hcd, dev->irq,
Below code is to not add shared_hcd if no USB 3 port:
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 1cb6eae..0881fb5 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -285,12 +285,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto disable_usb_phy;
if no usb3 ports found here maybe also remove the:
xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
dev_name(&pdev->dev), hcd);
- if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
- xhci->shared_hcd->can_do_streams = 1;
+ if (xhci->num_usb3_ports > 0) {
+ 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;
+ 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);
Then, the xhci_run should execute xhci_start() in case no USB 3 port
(xhci->shared_hcd->run() will execute xhci_run_finished() which run
xhci_start().
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 51535ba..3ad41aa 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -630,6 +630,16 @@ 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 (xhci->num_usb3_ports > 0) {
if (!xhci->shared_hcd) or if (xhci->num_usb3_ports == 0)
checking !xhci->shared_hcd somehow feels more correct here.
+ if (xhci_start(xhci)) {
+ xhci_halt(xhci);
+ return -ENODEV;
+ }
+ xhci->cmd_ring_state = CMD_RING_STATE_RUNNING;
+ }
+
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Finished xhci_run for USB2 roothub");
return 0;
Instead we need to make sure xhci->shared_hcd is set to NULL,
and modify at least the following functions to work with only one xhci hcd:
Do we really need to set xhci->shared_hcd to NULL? when
xhci->shared_hcd is not registered to usb_core, no operation work on
it.
Just to make it easier to check if xhci->shared_hcd exists, and to catch all
issues while developing this change.
xhci_run()
xhci_stop()
xhci_suspend()
xhci_resume()
xhci_gen_setup()
More changes are very likely needed, but this is a place to start
-Mathias
- ThangQ.
Hi Mathias,
Do you have any comment on my approach?
xhci_plat_remove() needs work as well, like the previous functions I mentioned.
But yes, to me the approach looks good
-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
