* [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
@ 2013-06-26 8:58 George Cherian
2013-06-26 9:02 ` Felipe Balbi
2013-06-26 16:53 ` Sarah Sharp
0 siblings, 2 replies; 6+ messages in thread
From: George Cherian @ 2013-06-26 8:58 UTC (permalink / raw)
To: linux-usb; +Cc: balbi, linux-kernel, gregkh, sarah.a.sharp, George Cherian
Synopsis xhci controllers with hci_version > 0.96 gives spurious success
events on short packet completion. During webcam capture the
"ERROR Transfer event TRB DMA ptr not part of current TD" was observed.
The same application works fine with synopsis controllers hci_version 0.96.
Signed-off-by: George Cherian <george.cherian@ti.com>
---
drivers/usb/host/xhci-plat.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 93ad67e..e63c6d3 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
* dev struct in order to setup MSI
*/
xhci->quirks |= XHCI_BROKEN_MSI;
+
+ /*
+ * In some xhci controllers which follows xhci 1.0 spec gives a spurious
+ * success event after a short transfer. This quirk will ignore such
+ * spurious event. Hit this issue in synopsis xhci controllers with
+ * hci_version > 0.96
+ */
+
+ if (xhci->hci_version > 0x96)
+ xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
}
/* called during probe() after chip reset completes */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
2013-06-26 8:58 [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat George Cherian
@ 2013-06-26 9:02 ` Felipe Balbi
2013-06-26 12:16 ` George Cherian
2013-06-26 16:53 ` Sarah Sharp
1 sibling, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2013-06-26 9:02 UTC (permalink / raw)
To: George Cherian; +Cc: linux-usb, balbi, linux-kernel, gregkh, sarah.a.sharp
[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]
Hi,
On Wed, Jun 26, 2013 at 02:28:57PM +0530, George Cherian wrote:
> Synopsis xhci controllers with hci_version > 0.96 gives spurious success
> events on short packet completion. During webcam capture the
> "ERROR Transfer event TRB DMA ptr not part of current TD" was observed.
> The same application works fine with synopsis controllers hci_version 0.96.
>
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
> drivers/usb/host/xhci-plat.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 93ad67e..e63c6d3 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> * dev struct in order to setup MSI
> */
> xhci->quirks |= XHCI_BROKEN_MSI;
> +
> + /*
> + * In some xhci controllers which follows xhci 1.0 spec gives a spurious
> + * success event after a short transfer. This quirk will ignore such
> + * spurious event. Hit this issue in synopsis xhci controllers with
> + * hci_version > 0.96
> + */
> +
> + if (xhci->hci_version > 0x96)
> + xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> }
doesn't look like the correct way to do this. What if enabling that
quirk on hosts which don't have the quirk cause problems ?
I would suggest adding a platform_data which (in our case) dwc3 will
pass to xhci-plat. Then you can do proper revision detection of the
synopsys controller and set the quirk only on the failing hosts.
BTW, do you have the STARS number for this errata ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
2013-06-26 9:02 ` Felipe Balbi
@ 2013-06-26 12:16 ` George Cherian
2013-06-26 12:25 ` Felipe Balbi
0 siblings, 1 reply; 6+ messages in thread
From: George Cherian @ 2013-06-26 12:16 UTC (permalink / raw)
To: balbi; +Cc: linux-usb, linux-kernel, gregkh, sarah.a.sharp
On 6/26/2013 2:32 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Jun 26, 2013 at 02:28:57PM +0530, George Cherian wrote:
>> Synopsis xhci controllers with hci_version > 0.96 gives spurious success
>> events on short packet completion. During webcam capture the
>> "ERROR Transfer event TRB DMA ptr not part of current TD" was observed.
>> The same application works fine with synopsis controllers hci_version 0.96.
>>
>> Signed-off-by: George Cherian<george.cherian@ti.com>
>> ---
>> drivers/usb/host/xhci-plat.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 93ad67e..e63c6d3 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>> * dev struct in order to setup MSI
>> */
>> xhci->quirks |= XHCI_BROKEN_MSI;
>> +
>> + /*
>> + * In some xhci controllers which follows xhci 1.0 spec gives a spurious
>> + * success event after a short transfer. This quirk will ignore such
>> + * spurious event. Hit this issue in synopsis xhci controllers with
>> + * hci_version > 0.96
>> + */
>> +
>> + if (xhci->hci_version > 0x96)
>> + xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>> }
> doesn't look like the correct way to do this. What if enabling that
> quirk on hosts which don't have the quirk cause problems ?
For a controller which does not have this issue will never get a
spurious success for short packet ( and that too for only ISOCH).
per this commit ad808333d Intel xhci: Ignore spurious successful event.
This spurious successful event behavior isn't technically disallowed by
the xHCI specification, so make the xHCI driver just ignore the spurious
completion event.
> I would suggest adding a platform_data which (in our case) dwc3 will
> pass to xhci-plat. Then you can do proper revision detection of the
> synopsys controller and set the quirk only on the failing hosts.
>
> BTW, do you have the STARS number for this errata ?
No STARS number yet.
Regards
-George
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
2013-06-26 12:16 ` George Cherian
@ 2013-06-26 12:25 ` Felipe Balbi
0 siblings, 0 replies; 6+ messages in thread
From: Felipe Balbi @ 2013-06-26 12:25 UTC (permalink / raw)
To: George Cherian; +Cc: balbi, linux-usb, linux-kernel, gregkh, sarah.a.sharp
[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]
Hi,
On Wed, Jun 26, 2013 at 05:46:29PM +0530, George Cherian wrote:
> >>@@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> >> * dev struct in order to setup MSI
> >> */
> >> xhci->quirks |= XHCI_BROKEN_MSI;
> >>+
> >>+ /*
> >>+ * In some xhci controllers which follows xhci 1.0 spec gives a spurious
> >>+ * success event after a short transfer. This quirk will ignore such
> >>+ * spurious event. Hit this issue in synopsis xhci controllers with
> >>+ * hci_version > 0.96
> >>+ */
> >>+
> >>+ if (xhci->hci_version > 0x96)
> >>+ xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >> }
> >doesn't look like the correct way to do this. What if enabling that
> >quirk on hosts which don't have the quirk cause problems ?
> For a controller which does not have this issue will never get a
> spurious success for short packet ( and that too for only ISOCH).
>
> per this commit ad808333d Intel xhci: Ignore spurious successful event.
>
> This spurious successful event behavior isn't technically disallowed by
> the xHCI specification, so make the xHCI driver just ignore the spurious
> completion event.
still we don't want to enable quirks for devices which aren't quirky :-)
Now how Sarah correctly enables the quirk flag only for the known "bad"
Panther Point device.
> >I would suggest adding a platform_data which (in our case) dwc3 will
> >pass to xhci-plat. Then you can do proper revision detection of the
> >synopsys controller and set the quirk only on the failing hosts.
> >
> >BTW, do you have the STARS number for this errata ?
> No STARS number yet.
once we have it, let's make sure to follow what we do on the dwc3 and
list it in a comment. You already that you found the bug with a Synopsys
controller anyway, might as well point to the fact that there is a
ticket with synopsys to track this issue.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
2013-06-26 8:58 [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat George Cherian
2013-06-26 9:02 ` Felipe Balbi
@ 2013-06-26 16:53 ` Sarah Sharp
2013-06-30 21:00 ` Sarah Sharp
1 sibling, 1 reply; 6+ messages in thread
From: Sarah Sharp @ 2013-06-26 16:53 UTC (permalink / raw)
To: George Cherian; +Cc: linux-usb, balbi, linux-kernel, gregkh
On Wed, Jun 26, 2013 at 02:28:57PM +0530, George Cherian wrote:
> Synopsis xhci controllers with hci_version > 0.96 gives spurious success
> events on short packet completion. During webcam capture the
> "ERROR Transfer event TRB DMA ptr not part of current TD" was observed.
> The same application works fine with synopsis controllers hci_version 0.96.
It's a known issue. The xHCI 1.0 spec changed how hardware handles
short packets. The HW will notify SW of the TRB where the short packet,
and it will also give a successful status for the last TRB in a TD (the
one with the IOC flag set). On the second successful status, that
warning will be triggered in the driver.
Software is now supposed to not assume the TD as done until it gets that
last successful status. That means we have a slight race condition,
although it should have little practical impact. This patch papers over
that issue.
I will take this patch, but will add the note that is on my long-term
to-do list to fix this issue.
Sarah Sharp
>
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
> drivers/usb/host/xhci-plat.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 93ad67e..e63c6d3 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> * dev struct in order to setup MSI
> */
> xhci->quirks |= XHCI_BROKEN_MSI;
> +
> + /*
> + * In some xhci controllers which follows xhci 1.0 spec gives a spurious
> + * success event after a short transfer. This quirk will ignore such
> + * spurious event. Hit this issue in synopsis xhci controllers with
> + * hci_version > 0.96
> + */
> +
> + if (xhci->hci_version > 0x96)
> + xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> }
>
> /* called during probe() after chip reset completes */
> --
> 1.8.1.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat
2013-06-26 16:53 ` Sarah Sharp
@ 2013-06-30 21:00 ` Sarah Sharp
0 siblings, 0 replies; 6+ messages in thread
From: Sarah Sharp @ 2013-06-30 21:00 UTC (permalink / raw)
To: George Cherian; +Cc: linux-usb, balbi, linux-kernel, gregkh
On Wed, Jun 26, 2013 at 09:53:34AM -0700, Sarah Sharp wrote:
> On Wed, Jun 26, 2013 at 02:28:57PM +0530, George Cherian wrote:
> > Synopsis xhci controllers with hci_version > 0.96 gives spurious success
> > events on short packet completion. During webcam capture the
> > "ERROR Transfer event TRB DMA ptr not part of current TD" was observed.
> > The same application works fine with synopsis controllers hci_version 0.96.
>
> It's a known issue. The xHCI 1.0 spec changed how hardware handles
> short packets. The HW will notify SW of the TRB where the short packet,
> and it will also give a successful status for the last TRB in a TD (the
> one with the IOC flag set). On the second successful status, that
> warning will be triggered in the driver.
>
> Software is now supposed to not assume the TD as done until it gets that
> last successful status. That means we have a slight race condition,
> although it should have little practical impact. This patch papers over
> that issue.
>
> I will take this patch, but will add the note that is on my long-term
> to-do list to fix this issue.
Actually, since it applies to both xhci-plat and xhci-pci hosts, can you
move setting this quirk into xhci_gen_setup?
Sarah Sharp
> > Signed-off-by: George Cherian <george.cherian@ti.com>
> > ---
> > drivers/usb/host/xhci-plat.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index 93ad67e..e63c6d3 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -25,6 +25,16 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> > * dev struct in order to setup MSI
> > */
> > xhci->quirks |= XHCI_BROKEN_MSI;
> > +
> > + /*
> > + * In some xhci controllers which follows xhci 1.0 spec gives a spurious
> > + * success event after a short transfer. This quirk will ignore such
> > + * spurious event. Hit this issue in synopsis xhci controllers with
> > + * hci_version > 0.96
> > + */
> > +
> > + if (xhci->hci_version > 0x96)
> > + xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > }
> >
> > /* called during probe() after chip reset completes */
> > --
> > 1.8.1.4
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-30 21:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-26 8:58 [PATCH] usb: host: xhci-plat: Enable XHCI_SPURIOUS_SUCCESS quirk for xhci-plat George Cherian
2013-06-26 9:02 ` Felipe Balbi
2013-06-26 12:16 ` George Cherian
2013-06-26 12:25 ` Felipe Balbi
2013-06-26 16:53 ` Sarah Sharp
2013-06-30 21:00 ` Sarah Sharp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.