-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Monday, June 06, 2016 6:15 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; albert.u.boot@aribaud.net; Rajesh Bhagat rajesh.bhagat@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:marex@denx.de] Sent: Thursday, June 02, 2016 6:31 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; albert.u.boot@aribaud.net; Rajesh Bhagat rajesh.bhagat@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:
- Implementing the errata for LS2080.
- Adding fixup for fdt for LS2080.
Signed-off-by: Sriram Dash sriram.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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