Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1524623 > unrolled thread

[PATCH 1/3] usb: xhci: add quirk flag for broken PED bits

Started byRoger Quadros <rogerq@ti.com>
First post2016-11-17 19:10 +0100
Last post2016-11-21 14:10 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] usb: xhci: add quirk flag for broken PED bits Roger Quadros <rogerq@ti.com> - 2016-11-17 19:10 +0100
    Re: [PATCH 1/3] usb: xhci: add quirk flag for broken PED bits Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-11-18 11:50 +0100
      Re: [PATCH 1/3] usb: xhci: add quirk flag for broken PED bits Roger Quadros <rogerq@ti.com> - 2016-11-21 13:00 +0100
    [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits Roger Quadros <rogerq@ti.com> - 2016-11-21 13:00 +0100
      Re: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits Felipe Balbi <balbi@kernel.org> - 2016-11-21 13:00 +0100
        Re: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits Roger Quadros <rogerq@ti.com> - 2016-11-21 14:10 +0100
      [PATCH v3 1/3] usb: xhci: add quirk flag for broken PED bits Roger Quadros <rogerq@ti.com> - 2016-11-21 14:10 +0100

#1524623 — [PATCH 1/3] usb: xhci: add quirk flag for broken PED bits

FromRoger Quadros <rogerq@ti.com>
Date2016-11-17 19:10 +0100
Subject[PATCH 1/3] usb: xhci: add quirk flag for broken PED bits
Message-ID<sEzih-48J-1@gated-at.bofh.it>
From: Felipe Balbi <balbi@ti.com>

Some devices from Texas Instruments [1] suffer from
a silicon bug where Port Enabled/Disabled bit
should not be used to silence an erroneous device.

The bug is so that if port is disabled with PED
bit, an IRQ for device removal (or attachment)
will never fire.

Just for the sake of completeness, the actual
problem lies with SNPS USB IP and this affects
all known versions up to 3.00a. A separate
patch will be added to dwc3 to enabled this
quirk flag if version is <= 3.00a.

[1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
Section i896— USB xHCI Port Disable Feature Does Not Work

Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/xhci-hub.c | 6 ++++++
 drivers/usb/host/xhci.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef1690..c3c051d 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 		return;
 	}
 
+	if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
+		xhci_dbg(xhci, "Broken Port Enabled/Disabled, ignoring "
+				"port disable request.\n");
+		return;
+	}
+
 	/* Write 1 to disable the port */
 	writel(port_status | PORT_PE, addr);
 	port_status = readl(addr);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f945380..4f724aa 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1656,6 +1656,9 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
+/* For controller with a broken Port Disable implementation */
+#define XHCI_BROKEN_PORT_PED    (1 << 21)
+
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */
-- 
2.7.4

[toc] | [next] | [standalone]

#1525170

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-11-18 11:50 +0100
Message-ID<sEOU2-602-7@gated-at.bofh.it>
In reply to#1524623
Hello.

On 11/17/2016 6:01 PM, Roger Quadros wrote:

> From: Felipe Balbi <balbi@ti.com>
>
> Some devices from Texas Instruments [1] suffer from
> a silicon bug where Port Enabled/Disabled bit
> should not be used to silence an erroneous device.
>
> The bug is so that if port is disabled with PED
> bit, an IRQ for device removal (or attachment)
> will never fire.
>
> Just for the sake of completeness, the actual
> problem lies with SNPS USB IP and this affects
> all known versions up to 3.00a. A separate
> patch will be added to dwc3 to enabled this
> quirk flag if version is <= 3.00a.
>
> [1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
> Section i896— USB xHCI Port Disable Feature Does Not Work
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/usb/host/xhci-hub.c | 6 ++++++
>  drivers/usb/host/xhci.h     | 3 +++
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 0ef1690..c3c051d 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
>  		return;
>  	}
>
> +	if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
> +		xhci_dbg(xhci, "Broken Port Enabled/Disabled, ignoring "
> +				"port disable request.\n");

    The messages shouldn't be broken up to facilitate grepping.

[...]
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index f945380..4f724aa 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1656,6 +1656,9 @@ struct xhci_hcd {
>  #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
>  #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
>  #define XHCI_MISSING_CAS	(1 << 24)
> +/* For controller with a broken Port Disable implementation */
> +#define XHCI_BROKEN_PORT_PED    (1 << 21)

    Indent with tabs as above, please.

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1526603

FromRoger Quadros <rogerq@ti.com>
Date2016-11-21 13:00 +0100
Message-ID<sFVqq-Fl-17@gated-at.bofh.it>
In reply to#1525170
On 18/11/16 12:49, Sergei Shtylyov wrote:
> Hello.
> 
> On 11/17/2016 6:01 PM, Roger Quadros wrote:
> 
>> From: Felipe Balbi <balbi@ti.com>
>>
>> Some devices from Texas Instruments [1] suffer from
>> a silicon bug where Port Enabled/Disabled bit
>> should not be used to silence an erroneous device.
>>
>> The bug is so that if port is disabled with PED
>> bit, an IRQ for device removal (or attachment)
>> will never fire.
>>
>> Just for the sake of completeness, the actual
>> problem lies with SNPS USB IP and this affects
>> all known versions up to 3.00a. A separate
>> patch will be added to dwc3 to enabled this
>> quirk flag if version is <= 3.00a.
>>
>> [1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
>> Section i896— USB xHCI Port Disable Feature Does Not Work
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/usb/host/xhci-hub.c | 6 ++++++
>>  drivers/usb/host/xhci.h     | 3 +++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index 0ef1690..c3c051d 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
>>          return;
>>      }
>>
>> +    if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
>> +        xhci_dbg(xhci, "Broken Port Enabled/Disabled, ignoring "
>> +                "port disable request.\n");
> 
>    The messages shouldn't be broken up to facilitate grepping.
> 
> [...]
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index f945380..4f724aa 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1656,6 +1656,9 @@ struct xhci_hcd {
>>  #define XHCI_SSIC_PORT_UNUSED    (1 << 22)
>>  #define XHCI_NO_64BIT_SUPPORT    (1 << 23)
>>  #define XHCI_MISSING_CAS    (1 << 24)
>> +/* For controller with a broken Port Disable implementation */
>> +#define XHCI_BROKEN_PORT_PED    (1 << 21)
> 
>    Indent with tabs as above, please.
> 
> [...]
> 

Thanks. I've fixed both issues and sent a v2.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1526599 — [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits

FromRoger Quadros <rogerq@ti.com>
Date2016-11-21 13:00 +0100
Subject[PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits
Message-ID<sFVqp-Fl-1@gated-at.bofh.it>
In reply to#1524623
From: Felipe Balbi <balbi@ti.com>

Some devices from Texas Instruments [1] suffer from
a silicon bug where Port Enabled/Disabled bit
should not be used to silence an erroneous device.

The bug is so that if port is disabled with PED
bit, an IRQ for device removal (or attachment)
will never fire.

Just for the sake of completeness, the actual
problem lies with SNPS USB IP and this affects
all known versions up to 3.00a. A separate
patch will be added to dwc3 to enabled this
quirk flag if version is <= 3.00a.

[1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
Section i896— USB xHCI Port Disable Feature Does Not Work

Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/xhci-hub.c | 6 ++++++
 drivers/usb/host/xhci.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef1690..1d41637 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 		return;
 	}
 
+	if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
+		xhci_dbg(xhci,
+			 "Broken Port Enabled/Disabled, ignoring port disable request.\n");
+		return;
+	}
+
 	/* Write 1 to disable the port */
 	writel(port_status | PORT_PE, addr);
 	port_status = readl(addr);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f945380..6553903 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1656,6 +1656,9 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
+/* For controller with a broken Port Disable implementation */
+#define XHCI_BROKEN_PORT_PED	(1 << 21)
+
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1526604 — Re: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits

FromFelipe Balbi <balbi@kernel.org>
Date2016-11-21 13:00 +0100
SubjectRe: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits
Message-ID<sFVqq-Fl-19@gated-at.bofh.it>
In reply to#1526599

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index f945380..6553903 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1656,6 +1656,9 @@ struct xhci_hcd {
>  #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
>  #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
>  #define XHCI_MISSING_CAS	(1 << 24)
> +/* For controller with a broken Port Disable implementation */
> +#define XHCI_BROKEN_PORT_PED	(1 << 21)

since back when I first wrote this, bit 21 has been taken. You should be
using bit 25 here.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1526663 — Re: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits

FromRoger Quadros <rogerq@ti.com>
Date2016-11-21 14:10 +0100
SubjectRe: [PATCH v2 1/3] usb: xhci: add quirk flag for broken PED bits
Message-ID<sFWwa-1AV-31@gated-at.bofh.it>
In reply to#1526604

[Multipart message — attachments visible in raw view] — view raw

On 21/11/16 13:56, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index f945380..6553903 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1656,6 +1656,9 @@ struct xhci_hcd {
>>  #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
>>  #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
>>  #define XHCI_MISSING_CAS	(1 << 24)
>> +/* For controller with a broken Port Disable implementation */
>> +#define XHCI_BROKEN_PORT_PED	(1 << 21)
> 
> since back when I first wrote this, bit 21 has been taken. You should be
> using bit 25 here.
> 
My bad. I'll fix this up.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1526654 — [PATCH v3 1/3] usb: xhci: add quirk flag for broken PED bits

FromRoger Quadros <rogerq@ti.com>
Date2016-11-21 14:10 +0100
Subject[PATCH v3 1/3] usb: xhci: add quirk flag for broken PED bits
Message-ID<sFWw9-1AV-7@gated-at.bofh.it>
In reply to#1526599
From: Felipe Balbi <balbi@ti.com>

Some devices from Texas Instruments [1] suffer from
a silicon bug where Port Enabled/Disabled bit
should not be used to silence an erroneous device.

The bug is so that if port is disabled with PED
bit, an IRQ for device removal (or attachment)
will never fire.

Just for the sake of completeness, the actual
problem lies with SNPS USB IP and this affects
all known versions up to 3.00a. A separate
patch will be added to dwc3 to enabled this
quirk flag if version is <= 3.00a.

[1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
Section i896— USB xHCI Port Disable Feature Does Not Work

Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/xhci-hub.c | 6 ++++++
 drivers/usb/host/xhci.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef1690..1d41637 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 		return;
 	}
 
+	if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
+		xhci_dbg(xhci,
+			 "Broken Port Enabled/Disabled, ignoring port disable request.\n");
+		return;
+	}
+
 	/* Write 1 to disable the port */
 	writel(port_status | PORT_PE, addr);
 	port_status = readl(addr);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f945380..585af71 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1656,6 +1656,9 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
+/* For controller with a broken Port Disable implementation */
+#define XHCI_BROKEN_PORT_PED	(1 << 25)
+
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web