2015-07-31 06:10:47

by Badola Nikhil

[permalink] [raw]
Subject: [PATCH] drivers: usb: fsl: Workaround for USB erratum-A005275

Incoming packets in high speed are randomly corrupted by h/w
resulting in multiple errors. This workaround makes FS as
default mode in all affected socs by disabling HS chirp
signalling.This errata does not affect FS and LS mode.

Forces all HS devices to connect in FS mode for all socs
affected by this erratum:
P3041 and P2041 rev 1.0 and 1.1
P5020 and P5010 rev 1.0 and 2.0
P5040, P1010 and T4240 rev 1.0

Signed-off-by: Ramneek Mehresh <[email protected]>
Signed-off-by: Nikhil Badola <[email protected]>
---
drivers/usb/host/ehci-fsl.c | 4 ++++
drivers/usb/host/ehci-hub.c | 7 +++++++
drivers/usb/host/ehci.h | 12 ++++++++++++
drivers/usb/host/fsl-mph-dr-of.c | 4 ++++
include/linux/fsl_devices.h | 1 +
5 files changed, 28 insertions(+)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 202dafb..3b6eb21 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -278,6 +278,10 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
out_be32(non_ehci + FSL_SOC_USB_SNOOP2, 0x80000000 | SNOOP_SIZE_2GB);
}

+ /* Deal with USB erratum A-005275 */
+ if (pdata->has_fsl_erratum_a005275 == 1)
+ ehci->has_fsl_hs_errata = 1;
+
if ((pdata->operating_mode == FSL_USB2_DR_HOST) ||
(pdata->operating_mode == FSL_USB2_DR_OTG))
if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0))
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 22abb68..fb381cf 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -1222,6 +1222,13 @@ int ehci_hub_control(
ehci->reset_done [wIndex] = jiffies
+ msecs_to_jiffies (50);
}
+
+ /* Force full-speed connect for FSL high-speed erratum;
+ * disable HS Chirp by setting PFSC bit
+ */
+ if (ehci_has_fsl_hs_errata(ehci))
+ temp |= (1 << PORTSC_FSL_PFSC);
+
ehci_writel(ehci, temp, status_reg);
break;

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index f700157..c232838 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -215,6 +215,7 @@ struct ehci_hcd { /* one per controller */
/* SILICON QUIRKS */
unsigned no_selective_suspend:1;
unsigned has_fsl_port_bug:1; /* FreeScale */
+ unsigned has_fsl_hs_errata:1; /* Freescale HS quirk */
unsigned big_endian_mmio:1;
unsigned big_endian_desc:1;
unsigned big_endian_capbase:1;
@@ -675,6 +676,17 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
#define ehci_port_speed(ehci, portsc) USB_PORT_STAT_HIGH_SPEED
#endif

+#define PORTSC_FSL_PFSC 24 /* Port Force Full-Speed Connect */
+
+#if defined(CONFIG_PPC_85xx)
+/* Some Freescale processors have an erratum (USB A-005275) in which
+ * incoming packets get corrupted in HS mode
+ */
+#define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata)
+#else
+#define ehci_has_fsl_hs_errata(e) (0)
+#endif
+
/*-------------------------------------------------------------------------*/

#ifdef CONFIG_PPC_83xx
diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index 9f73141..534c4c5 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -221,6 +221,10 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device *ofdev)
pdata->has_fsl_erratum_a007792 = 1;
else
pdata->has_fsl_erratum_a007792 = 0;
+ if (of_get_property(np, "fsl,usb-erratum-a005275", NULL))
+ pdata->has_fsl_erratum_a005275 = 1;
+ else
+ pdata->has_fsl_erratum_a005275 = 0;

/*
* Determine whether phy_clk_valid needs to be checked
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index cebdbbb..f291291 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -99,6 +99,7 @@ struct fsl_usb2_platform_data {
unsigned suspended:1;
unsigned already_suspended:1;
unsigned has_fsl_erratum_a007792:1;
+ unsigned has_fsl_erratum_a005275:1;
unsigned check_phy_clk_valid:1;

/* register save area for suspend/resume */
--
2.1.0


2015-07-31 16:33:55

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] drivers: usb: fsl: Workaround for USB erratum-A005275

On Fri, 31 Jul 2015, Nikhil Badola wrote:

> Incoming packets in high speed are randomly corrupted by h/w
> resulting in multiple errors. This workaround makes FS as
> default mode in all affected socs by disabling HS chirp
> signalling.This errata does not affect FS and LS mode.
>
> Forces all HS devices to connect in FS mode for all socs
> affected by this erratum:
> P3041 and P2041 rev 1.0 and 1.1
> P5020 and P5010 rev 1.0 and 2.0
> P5040, P1010 and T4240 rev 1.0

Ooh, that's a really bad bug. People will be pretty annoyed that they
can't use high speed connections.

> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -1222,6 +1222,13 @@ int ehci_hub_control(
> ehci->reset_done [wIndex] = jiffies
> + msecs_to_jiffies (50);
> }
> +
> + /* Force full-speed connect for FSL high-speed erratum;
> + * disable HS Chirp by setting PFSC bit
> + */
> + if (ehci_has_fsl_hs_errata(ehci))
> + temp |= (1 << PORTSC_FSL_PFSC);
> +

This hunk was added in the wrong place. It should come before the
closing '}' above, not after. That's because you don't want to force a
full-speed connection if the device is already using low speed.

> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -215,6 +215,7 @@ struct ehci_hcd { /* one per controller */
> /* SILICON QUIRKS */
> unsigned no_selective_suspend:1;
> unsigned has_fsl_port_bug:1; /* FreeScale */
> + unsigned has_fsl_hs_errata:1; /* Freescale HS quirk */
> unsigned big_endian_mmio:1;
> unsigned big_endian_desc:1;
> unsigned big_endian_capbase:1;
> @@ -675,6 +676,17 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
> #define ehci_port_speed(ehci, portsc) USB_PORT_STAT_HIGH_SPEED
> #endif
>
> +#define PORTSC_FSL_PFSC 24 /* Port Force Full-Speed Connect */
> +
> +#if defined(CONFIG_PPC_85xx)
> +/* Some Freescale processors have an erratum (USB A-005275) in which
> + * incoming packets get corrupted in HS mode
> + */
> +#define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata)
> +#else
> +#define ehci_has_fsl_hs_errata(e) (0)
> +#endif
> +
> /*-------------------------------------------------------------------------*/

I would prefer it if you add your new hunk after this /*-----...---*/
dividing line (that is, next to the ehci_has_fsl_portno_bug() code)
instead of before the dividing line.

Otherwise this looks okay.

Alan Stern

2015-08-03 05:59:49

by Badola Nikhil

[permalink] [raw]
Subject: RE: [PATCH] drivers: usb: fsl: Workaround for USB erratum-A005275

> -----Original Message-----
> From: Alan Stern [mailto:[email protected]]
> Sent: Friday, July 31, 2015 10:04 PM
> To: Badola Nikhil-B46172
> Cc: [email protected]; [email protected];
> [email protected]; Mehresh Ramneek-B31383
> Subject: Re: [PATCH] drivers: usb: fsl: Workaround for USB erratum-A005275
>
> On Fri, 31 Jul 2015, Nikhil Badola wrote:
>
> > Incoming packets in high speed are randomly corrupted by h/w resulting
> > in multiple errors. This workaround makes FS as default mode in all
> > affected socs by disabling HS chirp signalling.This errata does not
> > affect FS and LS mode.
> >
> > Forces all HS devices to connect in FS mode for all socs affected by
> > this erratum:
> > P3041 and P2041 rev 1.0 and 1.1
> > P5020 and P5010 rev 1.0 and 2.0
> > P5040, P1010 and T4240 rev 1.0
>
> Ooh, that's a really bad bug. People will be pretty annoyed that they can't
> use high speed connections.
>
> > --- a/drivers/usb/host/ehci-hub.c
> > +++ b/drivers/usb/host/ehci-hub.c
> > @@ -1222,6 +1222,13 @@ int ehci_hub_control(
> > ehci->reset_done [wIndex] = jiffies
> > + msecs_to_jiffies (50);
> > }
> > +
> > + /* Force full-speed connect for FSL high-speed
> erratum;
> > + * disable HS Chirp by setting PFSC bit
> > + */
> > + if (ehci_has_fsl_hs_errata(ehci))
> > + temp |= (1 << PORTSC_FSL_PFSC);
> > +
>
> This hunk was added in the wrong place. It should come before the closing '}'
> above, not after. That's because you don't want to force a full-speed
> connection if the device is already using low speed.

I agree that the hunk must be added before '}' i.e. in else block but reason being
our controllers have tt . Please correct me if I am wrong.
However the code executes "else" block in case of high speed as well as low speed device,
and low speed device is working fine in that case (I checked a Dell USB Keyboard).

>
> > --- a/drivers/usb/host/ehci.h
> > +++ b/drivers/usb/host/ehci.h
> > @@ -215,6 +215,7 @@ struct ehci_hcd { /* one per
> controller */
> > /* SILICON QUIRKS */
> > unsigned no_selective_suspend:1;
> > unsigned has_fsl_port_bug:1; /* FreeScale */
> > + unsigned has_fsl_hs_errata:1; /* Freescale HS quirk
> */
> > unsigned big_endian_mmio:1;
> > unsigned big_endian_desc:1;
> > unsigned big_endian_capbase:1;
> > @@ -675,6 +676,17 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned
> int portsc)
> > #define ehci_port_speed(ehci, portsc)
> USB_PORT_STAT_HIGH_SPEED
> > #endif
> >
> > +#define PORTSC_FSL_PFSC 24 /* Port Force Full-Speed Connect */
> > +
> > +#if defined(CONFIG_PPC_85xx)
> > +/* Some Freescale processors have an erratum (USB A-005275) in which
> > + * incoming packets get corrupted in HS mode */
> > +#define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata)
> > +#else
> > +#define ehci_has_fsl_hs_errata(e) (0)
> > +#endif
> > +
> >
> > /*--------------------------------------------------------------------
> > -----*/
>
> I would prefer it if you add your new hunk after this /*-----...---*/ dividing
> line (that is, next to the ehci_has_fsl_portno_bug() code) instead of before
> the dividing line.

Agreed. Will make changes in next version.