diff mbox

[U-Boot] drivers:usb:fsl: Implement Erratum A-009116 for XHCI controller

Message ID 1427428074-5535-1-git-send-email-nikhil.badola@freescale.com
State Not Applicable
Delegated to: Marek Vasut
Headers show

Commit Message

Nikhil Badola March 27, 2015, 3:47 a.m. UTC
This adjusts (micro)frame length to appropriate value thus
avoiding USB devices to time out over a longer run

Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
---
Depends on fsl/usb: Add USB XHCI support
http://patchwork.ozlabs.org/patch/373593/

 drivers/usb/host/xhci-fsl.c  |  9 +++++++++
 include/linux/usb/dwc3.h     | 19 +++++++++++++------
 include/linux/usb/xhci-fsl.h |  2 ++
 3 files changed, 24 insertions(+), 6 deletions(-)

Comments

Marek Vasut April 2, 2015, 5:02 p.m. UTC | #1
On Friday, March 27, 2015 at 04:47:54 AM, Nikhil Badola wrote:
> This adjusts (micro)frame length to appropriate value thus
> avoiding USB devices to time out over a longer run
> 
> Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>

Hi,

it seems the XHCI driver from Freescale was never applied, since no
maintainer was ever in CC and there was no effort from the author to
contact anyone after sending it out. That's rather unfortunate.

I briefly reviewed the driver which you linked and it seems like it
is almost exact copy of the xhci-omap.c . We certainly don't want
this level of code duplication.

Would it be possible for you to pick up that driver, work your change
into it, avoid the code duplication and repost the whole thing please ?

Best regards,
Marek Vasut
Nikhil Badola April 6, 2015, 8:59 a.m. UTC | #2
> -----Original Message-----
> From: Marek Vasut [mailto:marex@denx.de]
> Sent: Thursday, April 02, 2015 10:32 PM
> To: Badola Nikhil-B46172
> Cc: u-boot@lists.denx.de
> Subject: Re: [PATCH] drivers:usb:fsl: Implement Erratum A-009116 for XHCI
> controller
> 
> On Friday, March 27, 2015 at 04:47:54 AM, Nikhil Badola wrote:
> > This adjusts (micro)frame length to appropriate value thus avoiding
> > USB devices to time out over a longer run
> >
> > Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
> 
> Hi,
> 
> it seems the XHCI driver from Freescale was never applied, since no
> maintainer was ever in CC and there was no effort from the author to contact
> anyone after sending it out. That's rather unfortunate.

Freescale XHCI driver's author is already talking with HW teams to see if there's some
PHY shutdown sequence available for FSL socs...hence the delay.
> 
> I briefly reviewed the driver which you linked and it seems like it is almost
> exact copy of the xhci-omap.c . We certainly don't want this level of code
> duplication.
>

I agree with the fact that code duplication should be minimized. However the driver for
same controller would follow same initialization steps...until an effort is made in open-source
by someone to write a single driver file with common initialization code, this code
will keep getting duplicated.
 
> Would it be possible for you to pick up that driver, work your change into it,
> avoid the code duplication and repost the whole thing please ?
> 
> Best regards,
> Marek Vasut
Marek Vasut April 6, 2015, 3:02 p.m. UTC | #3
On Monday, April 06, 2015 at 10:59:09 AM, nikhil.badola@freescale.com wrote:
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex@denx.de]
> > Sent: Thursday, April 02, 2015 10:32 PM
> > To: Badola Nikhil-B46172
> > Cc: u-boot@lists.denx.de
> > Subject: Re: [PATCH] drivers:usb:fsl: Implement Erratum A-009116 for XHCI
> > controller
> > 
> > On Friday, March 27, 2015 at 04:47:54 AM, Nikhil Badola wrote:
> > > This adjusts (micro)frame length to appropriate value thus avoiding
> > > USB devices to time out over a longer run
> > > 
> > > Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
> > 
> > Hi,
> > 
> > it seems the XHCI driver from Freescale was never applied, since no
> > maintainer was ever in CC and there was no effort from the author to
> > contact anyone after sending it out. That's rather unfortunate.
> 
> Freescale XHCI driver's author is already talking with HW teams to see if
> there's some PHY shutdown sequence available for FSL socs...hence the
> delay.

Hi, OK.

> > I briefly reviewed the driver which you linked and it seems like it is
> > almost exact copy of the xhci-omap.c . We certainly don't want this
> > level of code duplication.
> 
> I agree with the fact that code duplication should be minimized. However
> the driver for same controller would follow same initialization
> steps...until an effort is made in open-source by someone to write a
> single driver file with common initialization code, this code will keep
> getting duplicated.

I agree. I won't allow such explicitly duplicated code in, so it's up to
you to do the deduplication. Thanks :)

> > Would it be possible for you to pick up that driver, work your change
> > into it, avoid the code duplication and repost the whole thing please ?
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-fsl.c b/drivers/usb/host/xhci-fsl.c
index 1d25084..981cf17 100644
--- a/drivers/usb/host/xhci-fsl.c
+++ b/drivers/usb/host/xhci-fsl.c
@@ -29,6 +29,12 @@  inline int __board_usb_init(int index, enum usb_init_type init)
 int board_usb_init(int index, enum usb_init_type init)
 	__attribute__((weak, alias("__board_usb_init")));
 
+static void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val)
+{
+	setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL |
+		     GFLADJ_30MHZ(val));
+}
+
 static void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)
 {
 	clrsetbits_le32(&dwc3_reg->g_ctl,
@@ -100,6 +106,9 @@  static int fsl_xhci_core_init(struct fsl_xhci *fsl_xhci)
 	/* We are hard-coding DWC3 core to Host Mode */
 	dwc3_set_mode(fsl_xhci->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
 
+	/* Set GFLADJ_30MHZ value as per Erratum A009116 */
+	dwc3_set_fladj(fsl_xhci->dwc3_reg, GFLADJ_30MHZ_DEFAULT);
+
 	return ret;
 }
 
diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h
index 7edc760..13d58e9 100644
--- a/include/linux/usb/dwc3.h
+++ b/include/linux/usb/dwc3.h
@@ -109,7 +109,11 @@  struct dwc3 {					/* offset: 0xC100 */
 
 	u32 g_hwparams8;
 
-	u32 reserved4[63];
+	u32 reserved4[11];
+
+	u32 g_fladj;
+
+	u32 reserved5[51];
 
 	u32 d_cfg;
 	u32 d_ctl;
@@ -118,15 +122,15 @@  struct dwc3 {					/* offset: 0xC100 */
 	u32 d_gcmdpar;
 	u32 d_gcmd;
 
-	u32 reserved5[2];
+	u32 reserved6[2];
 
 	u32 d_alepena;
 
-	u32 reserved6[55];
+	u32 reserved7[55];
 
 	struct d_physical_endpoint d_phy_ep_cmd[32];
 
-	u32 reserved7[128];
+	u32 reserved8[128];
 
 	u32 o_cfg;
 	u32 o_ctl;
@@ -134,7 +138,7 @@  struct dwc3 {					/* offset: 0xC100 */
 	u32 o_evten;
 	u32 o_sts;
 
-	u32 reserved8[3];
+	u32 reserved9[3];
 
 	u32 adp_cfg;
 	u32 adp_ctl;
@@ -143,7 +147,7 @@  struct dwc3 {					/* offset: 0xC100 */
 
 	u32 bc_cfg;
 
-	u32 reserved9;
+	u32 reserved10;
 
 	u32 bc_evt;
 	u32 bc_evten;
@@ -191,4 +195,7 @@  struct dwc3 {					/* offset: 0xC100 */
 #define DWC3_DCTL_CSFTRST			(1 << 30)
 #define DWC3_DCTL_LSFTRST			(1 << 29)
 
+/* Global Frame Length Adjustment Register */
+#define GFLADJ_30MHZ_REG_SEL			(1 << 7)
+#define GFLADJ_30MHZ(n)				((n) & 0x3f)
 #endif /* __DWC3_H_ */
diff --git a/include/linux/usb/xhci-fsl.h b/include/linux/usb/xhci-fsl.h
index 54b7e8b..22a65e9 100644
--- a/include/linux/usb/xhci-fsl.h
+++ b/include/linux/usb/xhci-fsl.h
@@ -10,6 +10,8 @@ 
 #ifndef _ASM_ARCH_XHCI_FSL_H_
 #define _ASM_ARCH_XHCI_FSL_H_
 
+#define GFLADJ_30MHZ_DEFAULT	0x20
+
 /* USBOTGSS_WRAPPER definitions */
 #define USBOTGSS_WRAPRESET	(1 << 17)
 #define USBOTGSS_DMADISABLE (1 << 16)