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

[Felipe Balbi <balbi@ti.com>]

[-- 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 --]