* [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
@ 2017-12-16 3:45 Thang Q. Nguyen
2018-01-04 5:17 ` Thang Q. Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Thang Q. Nguyen @ 2017-12-16 3:45 UTC (permalink / raw)
To: Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel
Cc: Thang Nguyen, Tung Nguyen, Phong Vo, Loc Ho, patches
From: Tung Nguyen <tunguyen@apm.com>
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@apm.com>
Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
---
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)) {
+ 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");
@@ -861,8 +875,8 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
if (!hcd->state)
return 0;
- if (hcd->state != HC_STATE_SUSPENDED ||
- xhci->shared_hcd->state != HC_STATE_SUSPENDED)
+ if (hcd->state != HC_STATE_SUSPENDED || (xhci->shared_hcd &&
+ xhci->shared_hcd->state != HC_STATE_SUSPENDED))
return -EINVAL;
xhci_dbc_suspend(xhci);
@@ -875,12 +889,15 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
xhci_dbg(xhci, "%s: stopping port polling.\n", __func__);
clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
del_timer_sync(&hcd->rh_timer);
- clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
- del_timer_sync(&xhci->shared_hcd->rh_timer);
+ if (xhci->shared_hcd) {
+ clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+ del_timer_sync(&xhci->shared_hcd->rh_timer);
+ }
spin_lock_irq(&xhci->lock);
clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
- clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+ if (xhci->shared_hcd)
+ clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
/* step 1: stop endpoint */
/* skipped assuming that port suspend has done */
@@ -961,7 +978,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
msleep(100);
set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
- set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+ if (xhci->shared_hcd)
+ set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
spin_lock_irq(&xhci->lock);
if (xhci->quirks & XHCI_RESET_ON_RESUME)
@@ -998,7 +1016,9 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* Let the USB core know _both_ roothubs lost power. */
usb_root_hub_lost_power(xhci->main_hcd->self.root_hub);
- usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
+ if (xhci->shared_hcd)
+ usb_root_hub_lost_power(
+ xhci->shared_hcd->self.root_hub);
xhci_dbg(xhci, "Stop HCD\n");
xhci_halt(xhci);
@@ -1039,7 +1059,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
retval = xhci_run(secondary_hcd);
}
hcd->state = HC_STATE_SUSPENDED;
- xhci->shared_hcd->state = HC_STATE_SUSPENDED;
+ if (xhci->shared_hcd)
+ xhci->shared_hcd->state = HC_STATE_SUSPENDED;
goto done;
}
@@ -1068,7 +1089,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* Resume root hubs only when have pending events. */
status = readl(&xhci->op_regs->status);
if (status & STS_EINT) {
- usb_hcd_resume_root_hub(xhci->shared_hcd);
+ if (xhci->shared_hcd)
+ usb_hcd_resume_root_hub(xhci->shared_hcd);
usb_hcd_resume_root_hub(hcd);
}
}
@@ -1087,8 +1109,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* Re-enable port polling. */
xhci_dbg(xhci, "%s: starting port polling.\n", __func__);
- set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
- usb_hcd_poll_rh_status(xhci->shared_hcd);
+ if (xhci->shared_hcd) {
+ set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+ usb_hcd_poll_rh_status(xhci->shared_hcd);
+ }
set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
usb_hcd_poll_rh_status(hcd);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
2017-12-16 3:45 [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled Thang Q. Nguyen
@ 2018-01-04 5:17 ` Thang Q. Nguyen
2018-01-16 14:50 ` Mathias Nyman
0 siblings, 1 reply; 5+ messages in thread
From: Thang Q. Nguyen @ 2018-01-04 5:17 UTC (permalink / raw)
To: Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel
Cc: Thang Nguyen, Tung Nguyen, Phong Vo, Loc Ho, patches
Hi,
On Sat, Dec 16, 2017 at 10:45 AM, Thang Q. Nguyen <tqnguyen@apm.com> wrote:
> From: Tung Nguyen <tunguyen@apm.com>
>
> 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@apm.com>
> Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
> ---
> 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)) {
> + 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");
>
> @@ -861,8 +875,8 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
> if (!hcd->state)
> return 0;
>
> - if (hcd->state != HC_STATE_SUSPENDED ||
> - xhci->shared_hcd->state != HC_STATE_SUSPENDED)
> + if (hcd->state != HC_STATE_SUSPENDED || (xhci->shared_hcd &&
> + xhci->shared_hcd->state != HC_STATE_SUSPENDED))
> return -EINVAL;
>
> xhci_dbc_suspend(xhci);
> @@ -875,12 +889,15 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
> xhci_dbg(xhci, "%s: stopping port polling.\n", __func__);
> clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> del_timer_sync(&hcd->rh_timer);
> - clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
> - del_timer_sync(&xhci->shared_hcd->rh_timer);
> + if (xhci->shared_hcd) {
> + clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
> + del_timer_sync(&xhci->shared_hcd->rh_timer);
> + }
>
> spin_lock_irq(&xhci->lock);
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> - clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
> + if (xhci->shared_hcd)
> + clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
> /* step 1: stop endpoint */
> /* skipped assuming that port suspend has done */
>
> @@ -961,7 +978,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> msleep(100);
>
> set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> - set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
> + if (xhci->shared_hcd)
> + set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
>
> spin_lock_irq(&xhci->lock);
> if (xhci->quirks & XHCI_RESET_ON_RESUME)
> @@ -998,7 +1016,9 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>
> /* Let the USB core know _both_ roothubs lost power. */
> usb_root_hub_lost_power(xhci->main_hcd->self.root_hub);
> - usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
> + if (xhci->shared_hcd)
> + usb_root_hub_lost_power(
> + xhci->shared_hcd->self.root_hub);
>
> xhci_dbg(xhci, "Stop HCD\n");
> xhci_halt(xhci);
> @@ -1039,7 +1059,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> retval = xhci_run(secondary_hcd);
> }
> hcd->state = HC_STATE_SUSPENDED;
> - xhci->shared_hcd->state = HC_STATE_SUSPENDED;
> + if (xhci->shared_hcd)
> + xhci->shared_hcd->state = HC_STATE_SUSPENDED;
> goto done;
> }
>
> @@ -1068,7 +1089,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> /* Resume root hubs only when have pending events. */
> status = readl(&xhci->op_regs->status);
> if (status & STS_EINT) {
> - usb_hcd_resume_root_hub(xhci->shared_hcd);
> + if (xhci->shared_hcd)
> + usb_hcd_resume_root_hub(xhci->shared_hcd);
> usb_hcd_resume_root_hub(hcd);
> }
> }
> @@ -1087,8 +1109,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>
> /* Re-enable port polling. */
> xhci_dbg(xhci, "%s: starting port polling.\n", __func__);
> - set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
> - usb_hcd_poll_rh_status(xhci->shared_hcd);
> + if (xhci->shared_hcd) {
> + set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
> + usb_hcd_poll_rh_status(xhci->shared_hcd);
> + }
> set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> usb_hcd_poll_rh_status(hcd);
>
> --
> 1.8.3.1
>
Do you have any comment on this patch?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
2018-01-04 5:17 ` Thang Q. Nguyen
@ 2018-01-16 14:50 ` Mathias Nyman
2018-01-18 7:27 ` Tung Vuong Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Mathias Nyman @ 2018-01-16 14:50 UTC (permalink / raw)
To: Thang Q. Nguyen, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
linux-kernel
Cc: Tung Nguyen, Phong Vo, Loc Ho, patches
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@apm.com> wrote:
>> From: Tung Nguyen <tunguyen@apm.com>
>>
>> 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@apm.com>
>> Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
2018-01-16 14:50 ` Mathias Nyman
@ 2018-01-18 7:27 ` Tung Vuong Nguyen
2018-01-18 15:02 ` Mathias Nyman
0 siblings, 1 reply; 5+ messages in thread
From: Tung Vuong Nguyen @ 2018-01-18 7:27 UTC (permalink / raw)
To: Mathias Nyman
Cc: Thang Q. Nguyen, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
linux-kernel, Phong Vo, Loc Ho, patches
On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
<mathias.nyman@linux.intel.com> 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@apm.com> wrote:
>>>
>>> From: Tung Nguyen <tunguyen@apm.com>
>>>
>>> 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@apm.com>
>>> Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
>>> ---
>>> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled
2018-01-18 7:27 ` Tung Vuong Nguyen
@ 2018-01-18 15:02 ` Mathias Nyman
0 siblings, 0 replies; 5+ messages in thread
From: Mathias Nyman @ 2018-01-18 15:02 UTC (permalink / raw)
To: Tung Vuong Nguyen
Cc: Thang Q. Nguyen, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
linux-kernel, Phong Vo, Loc Ho, patches
On 18.01.2018 09:27, Tung Vuong Nguyen wrote:
> On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
> <mathias.nyman@linux.intel.com> 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@apm.com> wrote:
>>>>
>>>> From: Tung Nguyen <tunguyen@apm.com>
>>>>
>>>> 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@apm.com>
>>>> Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
>>>> ---
>>>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-18 14:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-16 3:45 [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled Thang Q. Nguyen
2018-01-04 5:17 ` Thang Q. Nguyen
2018-01-16 14:50 ` Mathias Nyman
2018-01-18 7:27 ` Tung Vuong Nguyen
2018-01-18 15:02 ` Mathias Nyman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox