>-----Original Message----- >From: Marek Vasut [mailto:ma...@denx.de] >Sent: Monday, June 06, 2016 6:15 PM >To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >Cc: york sun <york....@nxp.com>; albert.u.b...@aribaud.net; Rajesh Bhagat ><rajesh.bha...@nxp.com> >Subject: Re: [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A- >008751 > >On 06/06/2016 06:24 AM, Sriram Dash wrote: >>> -----Original Message----- >>> From: Marek Vasut [mailto:ma...@denx.de] >>> Sent: Thursday, June 02, 2016 6:31 PM >>> To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >>> Cc: york sun <york....@nxp.com>; albert.u.b...@aribaud.net; Rajesh >>> Bhagat <rajesh.bha...@nxp.com> >>> Subject: Re: [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB >>> erratum A- >>> 008751 >>> >>> On 06/02/2016 08:54 AM, Sriram Dash wrote: >>>> This patch is doing the following: >>>> 1. Implementing the errata for LS2080. >>>> 2. Adding fixup for fdt for LS2080. >>>> >>>> Signed-off-by: Sriram Dash <sriram.d...@nxp.com> >>>> Signed-off-by: Rajesh Bhagat <rajesh.bha...@nxp.com> >>>> --- >>>> Changes in v2: >>>> - Reworked for changes done in errata checking code. >>> >>> [...] >>> >>>> diff --git a/drivers/usb/common/fsl-dt-fixup.c >>>> b/drivers/usb/common/fsl-dt-fixup.c >>>> index 7c039cb..fd85439 100644 >>>> --- a/drivers/usb/common/fsl-dt-fixup.c >>>> +++ b/drivers/usb/common/fsl-dt-fixup.c >>>> @@ -125,6 +125,7 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd) >>>> int usb_erratum_a007075_off = -1; >>>> int usb_erratum_a007792_off = -1; >>>> int usb_erratum_a005697_off = -1; >>>> + int usb_erratum_a008751_off = -1; >>> >>> Will there be an ever-growing list of unused variables here ? >>> >> >> Same as in Patch 2/5 code cleanup for device tree fixup for fsl usb >> controllers >usb_erratum_aNNNNNN_off variable are keeping track of the device tree fixup for >errata NNNNNN. The code checks errata applicability for each controller and >tries >to fix the device tree accordingly. During this time, the variable >usb_erratum_aNNNNNN_off is updated to the offset of device tree, if the device >tree is fixed. Now, for the second controller, when it tries to fix the device >tree, it >checks from the same offset obtained. As the API for fdt_setprop is such that >the >fixup will occur as soon as the API finds first match, if this variable >usb_erratum_aNNNNNN_off is not maintained, the errata will be fixed always for >the first usb controller it comes across the device tree. So, we need this >variable. > >What will happen if you have two different controllers in the system and each >of >them has different set of erratas ? Will this code fail at handling them by >applying >wrong sets of erratas ? >
Yes, you are right. For the different controllers with different erratas, I am planning to handle it by passing the controller type(ex: fsl-usb2-dr) from the fdt_fixup_erratum function, for which , the errata would be applied, and decided in fdt_fixup_usb_erratum function in V3. >btw Can you please fix your mailer to break lines at 80 chars ? > >>>> int usb_mode_off = -1; >>>> int usb_phy_off = -1; >>>> char str[5]; >>>> @@ -190,6 +191,8 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd) >>>> "a007792", has_erratum_a007792); >>>> fdt_fixup_erratum(&usb_erratum_a005697_off, blob, >>>> "a005697", has_erratum_a005697); >>>> + fdt_fixup_erratum(&usb_erratum_a008751_off, blob, >>>> + "a008751", has_erratum_a008751); >>>> >>>> } >>>> } >>>> diff --git a/drivers/usb/common/fsl-errata.c >>>> b/drivers/usb/common/fsl-errata.c index 95918fc..ebe60a8 100644 >>>> --- a/drivers/usb/common/fsl-errata.c >>>> +++ b/drivers/usb/common/fsl-errata.c >>>> @@ -175,4 +175,19 @@ bool has_erratum_a004477(void) >>>> return false; >>>> } >>>> >>>> +bool has_erratum_a008751(void) >>>> +{ >>>> + u32 svr = get_svr(); >>>> + u32 soc = SVR_SOC_VER(svr); >>>> + >>>> + switch (soc) { >>>> +#ifdef CONFIG_ARM64 >>>> + case SVR_LS2080: >>>> + case SVR_LS2085: >>>> + return IS_SVR_REV(svr, 1, 0); >>>> +#endif >>>> + } >>>> + return false; >>>> +} >>>> + >>>> #endif >>>> diff --git a/drivers/usb/host/xhci-fsl.c >>>> b/drivers/usb/host/xhci-fsl.c index 05f09d7..d55ed87 100644 >>>> --- a/drivers/usb/host/xhci-fsl.c >>>> +++ b/drivers/usb/host/xhci-fsl.c >>>> @@ -15,6 +15,8 @@ >>>> #include <linux/usb/xhci-fsl.h> >>>> #include <linux/usb/dwc3.h> >>>> #include "xhci.h" >>>> +#include <fsl_errata.h> >>>> +#include <fsl_usb.h> >>>> >>>> /* Declare global data pointer */ >>>> DECLARE_GLOBAL_DATA_PTR; >>>> @@ -27,6 +29,31 @@ __weak int __board_usb_init(int index, enum >>>> usb_init_type >>> init) >>>> return 0; >>>> } >>>> >>>> +static inline bool erratum_a008751(void) >>> >>> Drop the inline, let compiler decide. Also, make this an int instead >>> of bool and return 0 on success, 1 on error. >>> >> >> Ok. Will drop inline and make static int from static bool. >> Return 0 on success instead of true. >> Return 1 on error instead of false. >> >>>> +{ >>>> +#if defined(CONFIG_TARGET_LS2080AQDS) || >>> defined(CONFIG_TARGET_LS2080ARDB) >>>> + u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE; >>>> + writel(SCFG_USB3PRM1CR_INIT, scfg + SCFG_USB3PRM1CR / 4); >>>> + return true; >>>> +#endif >>>> + return false; >>>> +} >>>> + >>>> +#define APPLY_ERRATUM(id) \ >>>> +do { >>>> \ >>>> + bool ret; \ >>>> + if (has_erratum_##id()) { \ >>>> + ret = erratum_##id(); \ >>>> + if (ret <= 0) \ >>>> + puts("Failed to apply erratum " #id "\n"); \ >>>> + } \ >>>> +} while (0) >>> >>> Turn this into a function. >>> >> >> On a second thought, I guess I will drop the MACRO and directly call >> the >> has_erratum_a008751() etc from fsl_apply_xhci_errata, instead of going for a >function. >> What do you say? > >Fine > >>>> +static void fsl_apply_xhci_errata(void) { >>>> + APPLY_ERRATUM(a008751); >>>> +} >>>> + >>>> static int fsl_xhci_core_init(struct fsl_xhci *fsl_xhci) { >>>> int ret = 0; >>>> @@ -69,6 +96,8 @@ int xhci_hcd_init(int index, struct xhci_hccr >>>> **hccr, struct >>> xhci_hcor **hcor) >>>> return ret; >>>> } >>>> >>>> + fsl_apply_xhci_errata(); >>>> + >>>> ret = fsl_xhci_core_init(ctx); >>>> if (ret < 0) { >>>> puts("Failed to initialize xhci\n"); diff --git >>>> a/include/fsl_usb.h b/include/fsl_usb.h index d183349..fc72fb9 >>>> 100644 >>>> --- a/include/fsl_usb.h >>>> +++ b/include/fsl_usb.h >>>> @@ -94,5 +94,6 @@ bool has_erratum_a007798(void); bool >>>> has_erratum_a007792(void); bool has_erratum_a005697(void); bool >>>> has_erratum_a004477(void); >>>> +bool has_erratum_a008751(void); >>>> #endif >>>> #endif /*_ASM_FSL_USB_H_ */ >>>> >>> >>> >>> -- >>> Best regards, >>> Marek Vasut > > >-- >Best regards, >Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot