Re: [PATCH v2 3/5] usb: dwc3: Add quirk for Synopsis device disconnection errata

Felipe Balbi
2015-01-19T14:52:01Z

[-- Attachment #1: Type: text/plain, Size: 4743 bytes --]

Hi,

On Mon, Jan 19, 2015 at 03:56:47PM +0800, Sneeker Yeh wrote:
> Synopsis Designware USB3 IP earlier than v3.00a which is configured in silicon
> with DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, would need a specific quirk to prevent
> xhci host controller from dying when device is disconnected.
>
> Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP configuration whose state
> cannot be checked from software in runtime, it has to be enabled via platform
> data or device tree.
>
> Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
> ---
> Documentation/devicetree/bindings/usb/dwc3.txt | 17 +++++++++++++++++
> drivers/usb/dwc3/core.c | 6 ++++++
> drivers/usb/dwc3/core.h | 1 +
> drivers/usb/dwc3/host.c | 4 ++++
> drivers/usb/dwc3/platform_data.h | 1 +
> 5 files changed, 29 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index cd7f045..1b78b29 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -37,6 +37,23 @@ Optional properties:
> - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
> utmi_l1_suspend_n, false when asserts utmi_sleep_n
> - snps,hird-threshold: HIRD threshold
> + - snps,has_suspend_on_disconnect: true when IP is configured in silicon with
> + DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, it can inject a
> + specific quirk to prevent xhci host controller from
> + dying when usb device is disconnected from root hub.
> + Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP
> + configuration whose state cannot be checked from
> + software in runtime, it has to be enabled via platform
> + data or device tree.
> +
> + xhci host dying symptom here is caused by that
> + DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
> + configuration makes IP auto-suspended after PORTCSC is
> + cleared when usb device detached, then an asynchronous
> + disconnection procedure might fail using endpoint
> + command that suspened IP won't have any response to.
> +
> + this issue is fixed when IP version >= 3.00a.
>
> This is usually a subnode to DWC3 glue to which it is connected.
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25ddc39..fbceab1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -838,6 +838,9 @@ static int dwc3_probe(struct platform_device *pdev)
> "snps,tx_de_emphasis_quirk");
> of_property_read_u8(node, "snps,tx_de_emphasis",
> &tx_de_emphasis);
> +
> + dwc->suspend_on_disconnect_quirk = of_property_read_bool(node,
> + "snps,has_suspend_on_disconnect");
> } else if (pdata) {
> dwc->maximum_speed = pdata->maximum_speed;
> dwc->has_lpm_erratum = pdata->has_lpm_erratum;
> @@ -864,6 +867,9 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
> if (pdata->tx_de_emphasis)
> tx_de_emphasis = pdata->tx_de_emphasis;
> +
> + dwc->suspend_on_disconnect_quirk =
> + pdata->has_suspend_on_disconnect;
> }
>
> /* default to superspeed if no maximum_speed passed */
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 8090249..d7458ff 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -832,6 +832,7 @@ struct dwc3 {
>
> unsigned tx_de_emphasis_quirk:1;
> unsigned tx_de_emphasis:2;
> + unsigned suspend_on_disconnect_quirk:1;

you're missing the comment on the structure and these should be
alphabetically sorted.

> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index 12bfd3c..9c42074 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -53,6 +53,10 @@ int dwc3_host_init(struct dwc3 *dwc)
> pdata.usb3_lpm_capable = 1;
> #endif
>
> + if ((dwc->revision < DWC3_REVISION_300A) &&
> + dwc->suspend_on_disconnect_quirk)
> + pdata.delay_portcsc_clear = 1;
> +
> ret = platform_device_add_data(xhci, &pdata, sizeof(pdata));
> if (ret) {
> dev_err(dwc->dev, "couldn't add platform data to xHCI device\n");
> diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
> index a3a3b6d..69562f1 100644
> --- a/drivers/usb/dwc3/platform_data.h
> +++ b/drivers/usb/dwc3/platform_data.h
> @@ -44,4 +44,5 @@ struct dwc3_platform_data {
>
> unsigned tx_de_emphasis_quirk:1;
> unsigned tx_de_emphasis:2;
> + unsigned has_suspend_on_disconnect:1;

needs a comment at the top of this structure and should sorted.

--
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]