[U-Boot] [PATCH v2 0/5] Supporting ARM v8 USB errata for FSL

[U-Boot] [PATCH v2 0/5] Supporting ARM v8 USB errata for FSL

[Sriram Dash]

The patch-set does the following :

1. Adds support for ARM for USB Erratum Checking code for implementing the 
USB Erratum for fsl.
2. Performs code cleanup to reduce redundancy when adding fsl device
tree fixup.
3. Implements Erratum A008751 for LS2 platform.

Sriram Dash (5):
  arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper
  usb: xhci: fsl: code cleanup for device tree fixup for fsl usb
    controllers
  fsl: usb: make errata function common for PPC and ARM
  armv8/ls2080: Remove workaround for erratum A008751
  usb: xhci: fsl: Add workaround for USB erratum A-008751

 arch/arm/cpu/armv8/fsl-layerscape/cpu.c            |   7 +
 arch/arm/cpu/armv8/fsl-layerscape/soc.c            |  10 --
 .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |   2 +
 .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |   4 +
 arch/arm/include/asm/arch-fsl-layerscape/soc.h     |   2 +
 board/freescale/ls2080aqds/ls2080aqds.c            |   2 +
 board/freescale/ls2080ardb/ls2080ardb.c            |   2 +
 drivers/usb/common/Makefile                        |   4 +-
 drivers/usb/common/fsl-dt-fixup.c                  |  62 ++++---
 .../fsl_usb.h => drivers/usb/common/fsl-errata.c   | 152 ++++------------
 drivers/usb/host/xhci-fsl.c                        |  29 ++++
 include/fsl_usb.h                                  | 192 +--------------------
 12 files changed, 124 insertions(+), 344 deletions(-)
 copy include/fsl_usb.h => drivers/usb/common/fsl-errata.c (50%)

-- 
2.1.0

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Sriram Dash]

Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v2:
  - No update

 arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
 arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
 arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
 arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index 9a5a6b5..9c575c1 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
 	return -1;      /* cannot identify the cluster */
 }
 
+uint get_svr(void)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	return gur_in32(&gur->svr);
+}
+
 #ifdef CONFIG_DISPLAY_CPUINFO
 int print_cpuinfo(void)
 {
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
index 57b99d4..4151994 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
@@ -592,4 +592,6 @@ struct ccsr_cci400 {
 #define SCR0_CLIENTPD_MASK		0x00000001
 #define SCR0_USFCFG_MASK		0x00000400
 
+uint get_svr(void);
+
 #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 65b3357..e48bbaf 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -319,4 +319,7 @@ struct ccsr_reset {
 	u32 ip_rev1;			/* 0xbf8 */
 	u32 ip_rev2;			/* 0xbfc */
 };
+
+uint get_svr(void);
+
 #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
index 831d817..3f1a0a8 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
@@ -52,6 +52,8 @@ struct cpu_type {
 #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
 #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
 #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
+#define IS_SVR_REV(svr, maj, min) \
+		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
 
 /* ahci port register default value */
 #define AHCI_PORT_PHY_1_CFG    0xa003fffe
-- 
2.1.0

[U-Boot] [PATCH v2 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

[Sriram Dash]

Performs code cleanup for device tree fixup for fsl usb controllers by
making functions to handle these similar errata checking code.

Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
---
Changes in v2:
  - added patch description
  - remove the MACRO and use fdt_fixup_erratum function instead


 drivers/usb/common/fsl-dt-fixup.c | 58 +++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
index 6f31932..cbb008b 100644
--- a/drivers/usb/common/fsl-dt-fixup.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -99,6 +99,23 @@ static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 	return node_offset;
 }
 
+void fdt_fixup_erratum(int *usb_erratum_off, void *blob,
+		       char *str, bool (*has_erratum)(void))
+{
+	char buf[32] = {0};
+
+	snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
+	if (has_erratum()) {
+		*usb_erratum_off =  fdt_fixup_usb_erratum
+				   (blob,
+				    buf,
+				    *usb_erratum_off);
+		if (*usb_erratum_off < 0)
+			return;
+		debug("Adding USB erratum %s\n", str);
+	}
+}
+
 void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 {
 	static const char * const modes[] = { "host", "peripheral", "otg" };
@@ -164,39 +181,14 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 		if (usb_phy_off < 0)
 			return;
 
-		if (has_erratum_a006261()) {
-			usb_erratum_a006261_off =  fdt_fixup_usb_erratum
-						   (blob,
-						    "fsl,usb-erratum-a006261",
-						    usb_erratum_a006261_off);
-			if (usb_erratum_a006261_off < 0)
-				return;
-		}
-
-		if (has_erratum_a007075()) {
-			usb_erratum_a007075_off =  fdt_fixup_usb_erratum
-						   (blob,
-						    "fsl,usb-erratum-a007075",
-						    usb_erratum_a007075_off);
-			if (usb_erratum_a007075_off < 0)
-				return;
-		}
+		fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
+				  "a006261", has_erratum_a006261);
+		fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
+				  "a007075", has_erratum_a007075);
+		fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
+				  "a007792", has_erratum_a007792);
+		fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
+				  "a005697", has_erratum_a005697);
 
-		if (has_erratum_a007792()) {
-			usb_erratum_a007792_off =  fdt_fixup_usb_erratum
-						   (blob,
-						    "fsl,usb-erratum-a007792",
-						    usb_erratum_a007792_off);
-			if (usb_erratum_a007792_off < 0)
-				return;
-		}
-		if (has_erratum_a005697()) {
-			usb_erratum_a005697_off =  fdt_fixup_usb_erratum
-						   (blob,
-						    "fsl,usb-erratum-a005697",
-						    usb_erratum_a005697_off);
-			if (usb_erratum_a005697_off < 0)
-				return;
-		}
 	}
 }
-- 
2.1.0

[U-Boot] [PATCH v2 3/5] fsl: usb: make errata function common for PPC and ARM

[Sriram Dash]

This patch does the following things:
1. Makes the errata checking code common for PPC and ARM
2. Moves all the static inline functions into a dedicated C file

Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v2:
  - Moves all the static inline functions into a dedicated C file

 drivers/usb/common/Makefile       |   4 +-
 drivers/usb/common/fsl-dt-fixup.c |   1 +
 drivers/usb/common/fsl-errata.c   | 178 +++++++++++++++++++++++++++++++++++
 include/fsl_usb.h                 | 191 ++------------------------------------
 4 files changed, 189 insertions(+), 185 deletions(-)
 create mode 100644 drivers/usb/common/fsl-errata.c

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 2f46d38..aee7e32 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,5 @@
 #
 
 obj-$(CONFIG_DM_USB) += common.o
-obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
-obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
+obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o fsl-errata.o
+obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o fsl-errata.o
diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
index cbb008b..7c039cb 100644
--- a/drivers/usb/common/fsl-dt-fixup.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -12,6 +12,7 @@
 #include <usb.h>
 #include <asm/io.h>
 #include <hwconfig.h>
+#include <fsl_errata.h>
 #include <fsl_usb.h>
 #include <fdt_support.h>
 
diff --git a/drivers/usb/common/fsl-errata.c b/drivers/usb/common/fsl-errata.c
new file mode 100644
index 0000000..95918fc
--- /dev/null
+++ b/drivers/usb/common/fsl-errata.c
@@ -0,0 +1,178 @@
+/*
+ * Freescale USB Controller
+ *
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <fsl_errata.h>
+#include<fsl_usb.h>
+
+/* USB Erratum Checking code */
+#if defined(CONFIG_PPC) || defined(CONFIG_ARM)
+bool has_dual_phy(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_T1023:
+	case SVR_T1024:
+	case SVR_T1013:
+	case SVR_T1014:
+		return IS_SVR_REV(svr, 1, 0);
+	case SVR_T1040:
+	case SVR_T1042:
+	case SVR_T1020:
+	case SVR_T1022:
+	case SVR_T2080:
+	case SVR_T2081:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
+	case SVR_T4240:
+	case SVR_T4160:
+	case SVR_T4080:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+#endif
+	}
+
+	return false;
+}
+
+bool has_erratum_a006261(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_P1010:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_P2041:
+	case SVR_P2040:
+		return IS_SVR_REV(svr, 1, 0) ||
+			IS_SVR_REV(svr, 1, 1) || IS_SVR_REV(svr, 2, 1);
+	case SVR_P3041:
+		return IS_SVR_REV(svr, 1, 0) ||
+			IS_SVR_REV(svr, 1, 1) ||
+			IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 2, 1);
+	case SVR_P5010:
+	case SVR_P5020:
+	case SVR_P5021:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_T4240:
+	case SVR_T4160:
+	case SVR_T4080:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_T1040:
+		return IS_SVR_REV(svr, 1, 0);
+	case SVR_T2080:
+	case SVR_T2081:
+		return IS_SVR_REV(svr, 1, 0);
+	case SVR_P5040:
+		return IS_SVR_REV(svr, 1, 0);
+#endif
+	}
+
+	return false;
+}
+
+bool has_erratum_a007075(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_B4860:
+	case SVR_B4420:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_P1010:
+		return IS_SVR_REV(svr, 1, 0);
+	case SVR_P4080:
+		return IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 3, 0);
+#endif
+	}
+	return false;
+}
+
+bool has_erratum_a007798(void)
+{
+#ifdef CONFIG_PPC
+	return SVR_SOC_VER(get_svr()) == SVR_T4240 &&
+		IS_SVR_REV(get_svr(), 2, 0);
+#endif
+	return false;
+}
+
+bool has_erratum_a007792(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_T4240:
+	case SVR_T4160:
+	case SVR_T4080:
+		return IS_SVR_REV(svr, 2, 0);
+	case SVR_T1024:
+	case SVR_T1023:
+		return IS_SVR_REV(svr, 1, 0);
+	case SVR_T1040:
+	case SVR_T1042:
+	case SVR_T1020:
+	case SVR_T1022:
+	case SVR_T2080:
+	case SVR_T2081:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
+#endif
+	}
+	return false;
+}
+
+bool has_erratum_a005697(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_9131:
+	case SVR_9132:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
+#endif
+	}
+	return false;
+}
+
+bool has_erratum_a004477(void)
+{
+	u32 svr = get_svr();
+	u32 soc = SVR_SOC_VER(svr);
+
+	switch (soc) {
+#ifdef CONFIG_PPC
+	case SVR_P1010:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_P1022:
+	case SVR_9131:
+	case SVR_9132:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
+	case SVR_P2020:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0) ||
+			IS_SVR_REV(svr, 2, 1);
+	case SVR_B4860:
+	case SVR_B4420:
+		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
+	case SVR_P4080:
+		return IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 3, 0);
+#endif
+	}
+
+	return false;
+}
+
+#endif
diff --git a/include/fsl_usb.h b/include/fsl_usb.h
index 187e384..d183349 100644
--- a/include/fsl_usb.h
+++ b/include/fsl_usb.h
@@ -86,188 +86,13 @@ struct ccsr_usb_phy {
 #endif
 
 /* USB Erratum Checking code */
-#ifdef CONFIG_PPC
-static inline bool has_dual_phy(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_T1023:
-	case SVR_T1024:
-	case SVR_T1013:
-	case SVR_T1014:
-		return IS_SVR_REV(svr, 1, 0);
-	case SVR_T1040:
-	case SVR_T1042:
-	case SVR_T1020:
-	case SVR_T1022:
-	case SVR_T2080:
-	case SVR_T2081:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
-	case SVR_T4240:
-	case SVR_T4160:
-	case SVR_T4080:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	}
-
-	return false;
-}
-
-static inline bool has_erratum_a006261(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_P1010:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_P2041:
-	case SVR_P2040:
-		return IS_SVR_REV(svr, 1, 0) ||
-			IS_SVR_REV(svr, 1, 1) || IS_SVR_REV(svr, 2, 1);
-	case SVR_P3041:
-		return IS_SVR_REV(svr, 1, 0) ||
-			IS_SVR_REV(svr, 1, 1) ||
-			IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 2, 1);
-	case SVR_P5010:
-	case SVR_P5020:
-	case SVR_P5021:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_T4240:
-	case SVR_T4160:
-	case SVR_T4080:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_T1040:
-		return IS_SVR_REV(svr, 1, 0);
-	case SVR_T2080:
-	case SVR_T2081:
-		return IS_SVR_REV(svr, 1, 0);
-	case SVR_P5040:
-		return IS_SVR_REV(svr, 1, 0);
-	}
-
-	return false;
-}
-
-static inline bool has_erratum_a007075(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_B4860:
-	case SVR_B4420:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_P1010:
-		return IS_SVR_REV(svr, 1, 0);
-	case SVR_P4080:
-		return IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 3, 0);
-	}
-	return false;
-}
-
-static inline bool has_erratum_a007798(void)
-{
-	return SVR_SOC_VER(get_svr()) == SVR_T4240 &&
-		IS_SVR_REV(get_svr(), 2, 0);
-}
-
-static inline bool has_erratum_a007792(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_T4240:
-	case SVR_T4160:
-	case SVR_T4080:
-		return IS_SVR_REV(svr, 2, 0);
-	case SVR_T1024:
-	case SVR_T1023:
-		return IS_SVR_REV(svr, 1, 0);
-	case SVR_T1040:
-	case SVR_T1042:
-	case SVR_T1020:
-	case SVR_T1022:
-	case SVR_T2080:
-	case SVR_T2081:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
-	}
-	return false;
-}
-
-static inline bool has_erratum_a005697(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_9131:
-	case SVR_9132:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
-	}
-	return false;
-}
-
-static inline bool has_erratum_a004477(void)
-{
-	u32 svr = get_svr();
-	u32 soc = SVR_SOC_VER(svr);
-
-	switch (soc) {
-	case SVR_P1010:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_P1022:
-	case SVR_9131:
-	case SVR_9132:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1);
-	case SVR_P2020:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0) ||
-			IS_SVR_REV(svr, 2, 1);
-	case SVR_B4860:
-	case SVR_B4420:
-		return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 2, 0);
-	case SVR_P4080:
-		return IS_SVR_REV(svr, 2, 0) || IS_SVR_REV(svr, 3, 0);
-	}
-
-	return false;
-}
-#else
-static inline bool has_dual_phy(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a006261(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a007075(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a007798(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a007792(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a005697(void)
-{
-	return false;
-}
-
-static inline bool has_erratum_a004477(void)
-{
-	return false;
-}
+#if defined(CONFIG_PPC) || defined(CONFIG_ARM)
+bool has_dual_phy(void);
+bool has_erratum_a006261(void);
+bool has_erratum_a007075(void);
+bool has_erratum_a007798(void);
+bool has_erratum_a007792(void);
+bool has_erratum_a005697(void);
+bool has_erratum_a004477(void);
 #endif
 #endif /*_ASM_FSL_USB_H_ */
-- 
2.1.0

[U-Boot] [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum A008751

[Sriram Dash]

Remove the errata workaround from soc specific file for LS2080

Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v2:
  - No update


 arch/arm/cpu/armv8/fsl-layerscape/soc.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 0fb5c7f..5636531 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -122,15 +122,6 @@ void erratum_a009635(void)
 }
 #endif	/* CONFIG_SYS_FSL_ERRATUM_A009635 */
 
-static void erratum_a008751(void)
-{
-#ifdef CONFIG_SYS_FSL_ERRATUM_A008751
-	u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE;
-
-	writel(0x27672b2a, scfg + SCFG_USB3PRM1CR / 4);
-#endif
-}
-
 static void erratum_rcw_src(void)
 {
 #if defined(CONFIG_SPL)
@@ -187,7 +178,6 @@ void bypass_smmu(void)
 }
 void fsl_lsch3_early_init_f(void)
 {
-	erratum_a008751();
 	erratum_rcw_src();
 	init_early_memctl_regs();	/* tighten IFC timing */
 	erratum_a009203();
-- 
2.1.0

[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751

[Sriram Dash]

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.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v2:
  - Reworked for changes done in errata checking code.


 .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
 board/freescale/ls2080aqds/ls2080aqds.c            |  2 ++
 board/freescale/ls2080ardb/ls2080ardb.c            |  2 ++
 drivers/usb/common/fsl-dt-fixup.c                  |  3 +++
 drivers/usb/common/fsl-errata.c                    | 15 +++++++++++
 drivers/usb/host/xhci-fsl.c                        | 29 ++++++++++++++++++++++
 include/fsl_usb.h                                  |  1 +
 7 files changed, 53 insertions(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index e48bbaf..9b60bd3 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -139,6 +139,7 @@
 /* Supplemental Configuration */
 #define SCFG_BASE		0x01fc0000
 #define SCFG_USB3PRM1CR			0x000
+#define SCFG_USB3PRM1CR_INIT		0x27672b2a
 
 #define TP_ITYP_AV		0x00000001	/* Initiator available */
 #define TP_ITYP_TYPE(x)	(((x) & 0x6) >> 1)	/* Initiator Type */
diff --git a/board/freescale/ls2080aqds/ls2080aqds.c b/board/freescale/ls2080aqds/ls2080aqds.c
index b3bd40a..97892f8 100644
--- a/board/freescale/ls2080aqds/ls2080aqds.c
+++ b/board/freescale/ls2080aqds/ls2080aqds.c
@@ -296,6 +296,8 @@ int ft_board_setup(void *blob, bd_t *bd)
 
 	fdt_fixup_memory_banks(blob, base, size, 2);
 
+	fdt_fixup_dr_usb(blob, bd);
+
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fixup_board_enet(blob);
 	err = fsl_mc_ldpaa_exit(bd);
diff --git a/board/freescale/ls2080ardb/ls2080ardb.c b/board/freescale/ls2080ardb/ls2080ardb.c
index fb39af6..39b8ea3 100644
--- a/board/freescale/ls2080ardb/ls2080ardb.c
+++ b/board/freescale/ls2080ardb/ls2080ardb.c
@@ -275,6 +275,8 @@ int ft_board_setup(void *blob, bd_t *bd)
 
 	fdt_fixup_memory_banks(blob, base, size, 2);
 
+	fdt_fixup_dr_usb(blob, bd);
+
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fixup_board_enet(blob);
 	err = fsl_mc_ldpaa_exit(bd);
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;
 	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)
+{
+#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)
+
+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_ */
-- 
2.1.0

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Marek Vasut]

On 06/02/2016 08:54 AM, Sriram Dash wrote:
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2:
>   - No update
> 
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index 9a5a6b5..9c575c1 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>  	return -1;      /* cannot identify the cluster */
>  }
>  
> +uint get_svr(void)

Should be u32 . The function should have some more recognizable name,
fsl_get_svr() or such.

> +{
> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +
> +	return gur_in32(&gur->svr);
> +}
> +
>  #ifdef CONFIG_DISPLAY_CPUINFO
>  int print_cpuinfo(void)
>  {
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> index 57b99d4..4151994 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>  #define SCR0_CLIENTPD_MASK		0x00000001
>  #define SCR0_USFCFG_MASK		0x00000400
>  
> +uint get_svr(void);
> +
>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> index 65b3357..e48bbaf 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -319,4 +319,7 @@ struct ccsr_reset {
>  	u32 ip_rev1;			/* 0xbf8 */
>  	u32 ip_rev2;			/* 0xbfc */
>  };
> +
> +uint get_svr(void);
> +
>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> index 831d817..3f1a0a8 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> @@ -52,6 +52,8 @@ struct cpu_type {
>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
> +#define IS_SVR_REV(svr, maj, min) \
> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>  
>  /* ahci port register default value */
>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Marek Vasut]

On 06/02/2016 08:54 AM, Sriram Dash wrote:

Commit message is missing, please fix

> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2:
>   - No update
> 
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index 9a5a6b5..9c575c1 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>  	return -1;      /* cannot identify the cluster */
>  }
>  
> +uint get_svr(void)
> +{
> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +
> +	return gur_in32(&gur->svr);
> +}
> +
>  #ifdef CONFIG_DISPLAY_CPUINFO
>  int print_cpuinfo(void)
>  {
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> index 57b99d4..4151994 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>  #define SCR0_CLIENTPD_MASK		0x00000001
>  #define SCR0_USFCFG_MASK		0x00000400
>  
> +uint get_svr(void);
> +
>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> index 65b3357..e48bbaf 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -319,4 +319,7 @@ struct ccsr_reset {
>  	u32 ip_rev1;			/* 0xbf8 */
>  	u32 ip_rev2;			/* 0xbfc */
>  };
> +
> +uint get_svr(void);
> +
>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> index 831d817..3f1a0a8 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
> @@ -52,6 +52,8 @@ struct cpu_type {
>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
> +#define IS_SVR_REV(svr, maj, min) \
> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>  
>  /* ahci port register default value */
>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

[Marek Vasut]

On 06/02/2016 08:54 AM, Sriram Dash wrote:
> Performs code cleanup for device tree fixup for fsl usb controllers by
> making functions to handle these similar errata checking code.
> 
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> ---
> Changes in v2:
>   - added patch description
>   - remove the MACRO and use fdt_fixup_erratum function instead
> 
> 
>  drivers/usb/common/fsl-dt-fixup.c | 58 +++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
> index 6f31932..cbb008b 100644
> --- a/drivers/usb/common/fsl-dt-fixup.c
> +++ b/drivers/usb/common/fsl-dt-fixup.c
> @@ -99,6 +99,23 @@ static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
>  	return node_offset;
>  }
>  
> +void fdt_fixup_erratum(int *usb_erratum_off, void *blob,
> +		       char *str, bool (*has_erratum)(void))
> +{
> +	char buf[32] = {0};
> +
> +	snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
> +	if (has_erratum()) {

Invert the condition here so you won't have the indentation problems below:

if (!(has_erratum())
	return;
... other stuff.


> +		*usb_erratum_off =  fdt_fixup_usb_erratum
> +				   (blob,
> +				    buf,
> +				    *usb_erratum_off);
> +		if (*usb_erratum_off < 0)
> +			return;

If fdt_fixup_usb_erratum() fails, this function will return, but the
code below will continue. This changes the logic of the code to the
worse, so fix this.

> +		debug("Adding USB erratum %s\n", str);
> +	}
> +}
> +
>  void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>  {
>  	static const char * const modes[] = { "host", "peripheral", "otg" };
> @@ -164,39 +181,14 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>  		if (usb_phy_off < 0)
>  			return;
>  
> -		if (has_erratum_a006261()) {
> -			usb_erratum_a006261_off =  fdt_fixup_usb_erratum
> -						   (blob,
> -						    "fsl,usb-erratum-a006261",
> -						    usb_erratum_a006261_off);
> -			if (usb_erratum_a006261_off < 0)
> -				return;
> -		}
> -
> -		if (has_erratum_a007075()) {
> -			usb_erratum_a007075_off =  fdt_fixup_usb_erratum
> -						   (blob,
> -						    "fsl,usb-erratum-a007075",
> -						    usb_erratum_a007075_off);
> -			if (usb_erratum_a007075_off < 0)
> -				return;
> -		}
> +		fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
> +				  "a006261", has_erratum_a006261);
> +		fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
> +				  "a007075", has_erratum_a007075);
> +		fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
> +				  "a007792", has_erratum_a007792);
> +		fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
> +				  "a005697", has_erratum_a005697);

Are these usb_erratum_aNNNNNN_off variables needed at all ? Why are they
even passed into the function ? I think they can be local to the function.

> -		if (has_erratum_a007792()) {
> -			usb_erratum_a007792_off =  fdt_fixup_usb_erratum
> -						   (blob,
> -						    "fsl,usb-erratum-a007792",
> -						    usb_erratum_a007792_off);
> -			if (usb_erratum_a007792_off < 0)
> -				return;
> -		}
> -		if (has_erratum_a005697()) {
> -			usb_erratum_a005697_off =  fdt_fixup_usb_erratum
> -						   (blob,
> -						    "fsl,usb-erratum-a005697",
> -						    usb_erratum_a005697_off);
> -			if (usb_erratum_a005697_off < 0)
> -				return;
> -		}
>  	}
>  }
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum A008751

[Marek Vasut]

On 06/02/2016 08:54 AM, Sriram Dash wrote:
> Remove the errata workaround from soc specific file for LS2080

The commit message does not explain why this change is legal or needed.

> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> Changes in v2:
>   - No update
> 
> 
>  arch/arm/cpu/armv8/fsl-layerscape/soc.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> index 0fb5c7f..5636531 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> @@ -122,15 +122,6 @@ void erratum_a009635(void)
>  }
>  #endif	/* CONFIG_SYS_FSL_ERRATUM_A009635 */
>  
> -static void erratum_a008751(void)
> -{
> -#ifdef CONFIG_SYS_FSL_ERRATUM_A008751
> -	u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE;
> -
> -	writel(0x27672b2a, scfg + SCFG_USB3PRM1CR / 4);
> -#endif
> -}
> -
>  static void erratum_rcw_src(void)
>  {
>  #if defined(CONFIG_SPL)
> @@ -187,7 +178,6 @@ void bypass_smmu(void)
>  }
>  void fsl_lsch3_early_init_f(void)
>  {
> -	erratum_a008751();
>  	erratum_rcw_src();
>  	init_early_memctl_regs();	/* tighten IFC timing */
>  	erratum_a009203();
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751

[Marek Vasut]

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.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 ?

>  	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.

> +{
> +#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.

> +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

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 02, 2016 6:18 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>helper
>
>On 06/02/2016 08:54 AM, Sriram Dash wrote:
>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> ---
>> Changes in v2:
>>   - No update
>>
>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>  4 files changed, 14 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> index 9a5a6b5..9c575c1 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>  	return -1;      /* cannot identify the cluster */
>>  }
>>
>> +uint get_svr(void)
>
>Should be u32 . The function should have some more recognizable name,
>fsl_get_svr() or such.
>

Hello Marek,

As done in PPC for all the Socs for FSL, in arch/powerpc/cpu/mpc83xx/start.S , and ARM v7 LS1021, in arch/arm/cpu/armv7/ls102xa/cpu.c, this uint get_svr(void) function finds the svr number for the SoC. Now, as i have to make the code generic for PPC and ARM (in patch 3/5 namely make errata function common for PPC and ARM), I am currently having the same name and return type used.

Regards,
Sriram

>> +{
>> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
>> +
>> +	return gur_in32(&gur->svr);
>> +}
>> +
>>  #ifdef CONFIG_DISPLAY_CPUINFO
>>  int print_cpuinfo(void)
>>  {
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> index 57b99d4..4151994 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>>  #define SCR0_CLIENTPD_MASK		0x00000001
>>  #define SCR0_USFCFG_MASK		0x00000400
>>
>> +uint get_svr(void);
>> +
>>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> index 65b3357..e48bbaf 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> @@ -319,4 +319,7 @@ struct ccsr_reset {
>>  	u32 ip_rev1;			/* 0xbf8 */
>>  	u32 ip_rev2;			/* 0xbfc */
>>  };
>> +
>> +uint get_svr(void);
>> +
>>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */ diff --git
>> a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> index 831d817..3f1a0a8 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> @@ -52,6 +52,8 @@ struct cpu_type {
>>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
>> +#define IS_SVR_REV(svr, maj, min) \
>> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>>
>>  /* ahci port register default value */
>>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
>>
>
>
>--
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 02, 2016 6:18 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>helper
>
>On 06/02/2016 08:54 AM, Sriram Dash wrote:
>
>Commit message is missing, please fix
>

Adds get_svr and IS_SVR_REV helpers for ARMv8 platforms, similar to PPC and ARMv7.
Will take care in V3.

>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> ---
>> Changes in v2:
>>   - No update
>>
>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>  4 files changed, 14 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> index 9a5a6b5..9c575c1 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>  	return -1;      /* cannot identify the cluster */
>>  }
>>
>> +uint get_svr(void)
>> +{
>> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
>> +
>> +	return gur_in32(&gur->svr);
>> +}
>> +
>>  #ifdef CONFIG_DISPLAY_CPUINFO
>>  int print_cpuinfo(void)
>>  {
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> index 57b99d4..4151994 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>>  #define SCR0_CLIENTPD_MASK		0x00000001
>>  #define SCR0_USFCFG_MASK		0x00000400
>>
>> +uint get_svr(void);
>> +
>>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> index 65b3357..e48bbaf 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> @@ -319,4 +319,7 @@ struct ccsr_reset {
>>  	u32 ip_rev1;			/* 0xbf8 */
>>  	u32 ip_rev2;			/* 0xbfc */
>>  };
>> +
>> +uint get_svr(void);
>> +
>>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */ diff --git
>> a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> index 831d817..3f1a0a8 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>> @@ -52,6 +52,8 @@ struct cpu_type {
>>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
>> +#define IS_SVR_REV(svr, maj, min) \
>> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>>
>>  /* ahci port register default value */
>>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
>>
>
>
>--
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v2 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 02, 2016 6:25 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v2 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl
>usb controllers
>
>On 06/02/2016 08:54 AM, Sriram Dash wrote:
>> Performs code cleanup for device tree fixup for fsl usb controllers by
>> making functions to handle these similar errata checking code.
>>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> ---
>> Changes in v2:
>>   - added patch description
>>   - remove the MACRO and use fdt_fixup_erratum function instead
>>
>>
>>  drivers/usb/common/fsl-dt-fixup.c | 58
>> +++++++++++++++++----------------------
>>  1 file changed, 25 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>> b/drivers/usb/common/fsl-dt-fixup.c
>> index 6f31932..cbb008b 100644
>> --- a/drivers/usb/common/fsl-dt-fixup.c
>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>> @@ -99,6 +99,23 @@ static int fdt_fixup_usb_erratum(void *blob, const char
>*prop_erratum,
>>  	return node_offset;
>>  }
>>
>> +void fdt_fixup_erratum(int *usb_erratum_off, void *blob,
>> +		       char *str, bool (*has_erratum)(void)) {
>> +	char buf[32] = {0};
>> +
>> +	snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
>> +	if (has_erratum()) {
>
>Invert the condition here so you won't have the indentation problems below:
>
>if (!(has_erratum())
>	return;
>... other stuff.
>

Thank you for the suggestion. Looks better in a single line. Will change in V3.

>
>> +		*usb_erratum_off =  fdt_fixup_usb_erratum
>> +				   (blob,
>> +				    buf,
>> +				    *usb_erratum_off);
>> +		if (*usb_erratum_off < 0)
>> +			return;
>
>If fdt_fixup_usb_erratum() fails, this function will return, but the code below will
>continue. This changes the logic of the code to the worse, so fix this.
>

Yes. You are correct. I will now return ENOSPC in case of failure and check that in
fdt_fixup_dr_usb function to maintain the same logic as before.

>> +		debug("Adding USB erratum %s\n", str);
>> +	}
>> +}
>> +
>>  void fdt_fixup_dr_usb(void *blob, bd_t *bd)  {
>>  	static const char * const modes[] = { "host", "peripheral", "otg" };
>> @@ -164,39 +181,14 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>>  		if (usb_phy_off < 0)
>>  			return;
>>
>> -		if (has_erratum_a006261()) {
>> -			usb_erratum_a006261_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a006261",
>> -						    usb_erratum_a006261_off);
>> -			if (usb_erratum_a006261_off < 0)
>> -				return;
>> -		}
>> -
>> -		if (has_erratum_a007075()) {
>> -			usb_erratum_a007075_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a007075",
>> -						    usb_erratum_a007075_off);
>> -			if (usb_erratum_a007075_off < 0)
>> -				return;
>> -		}
>> +		fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
>> +				  "a006261", has_erratum_a006261);
>> +		fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
>> +				  "a007075", has_erratum_a007075);
>> +		fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
>> +				  "a007792", has_erratum_a007792);
>> +		fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
>> +				  "a005697", has_erratum_a005697);
>
>Are these usb_erratum_aNNNNNN_off variables needed at all ? Why are they even
>passed into the function ? I think they can be local to the function.
>

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.
Now, we cannot locally deal with the variable, reason being, in function fdt_fixup_erratum, To make it keep track of the offset, I have again generate multiple variables for that many erratas.
Also, I have to make each variable static, in order to keep track of the controller specific errata Applicability.

>> -		if (has_erratum_a007792()) {
>> -			usb_erratum_a007792_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a007792",
>> -						    usb_erratum_a007792_off);
>> -			if (usb_erratum_a007792_off < 0)
>> -				return;
>> -		}
>> -		if (has_erratum_a005697()) {
>> -			usb_erratum_a005697_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a005697",
>> -						    usb_erratum_a005697_off);
>> -			if (usb_erratum_a005697_off < 0)
>> -				return;
>> -		}
>>  	}
>>  }
>>
>
>
>--
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum A008751

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 02, 2016 6:28 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum
>A008751
>
>On 06/02/2016 08:54 AM, Sriram Dash wrote:
>> Remove the errata workaround from soc specific file for LS2080
>
>The commit message does not explain why this change is legal or needed.
>

This errata a008751 is applied on Soc specific file currently.
We are moving this errata to a a file where all the errata implementation will take Place for usb for fsl. Implementation of the errata will follow in Patch 5/5.

However, I cannot reference to the driver specific implementation till it is committed.
So, this is just the description of removing the errata from soc specific file.

>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> ---
>> Changes in v2:
>>   - No update
>>
>>
>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c | 10 ----------
>>  1 file changed, 10 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> index 0fb5c7f..5636531 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> @@ -122,15 +122,6 @@ void erratum_a009635(void)  }
>>  #endif	/* CONFIG_SYS_FSL_ERRATUM_A009635 */
>>
>> -static void erratum_a008751(void)
>> -{
>> -#ifdef CONFIG_SYS_FSL_ERRATUM_A008751
>> -	u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE;
>> -
>> -	writel(0x27672b2a, scfg + SCFG_USB3PRM1CR / 4);
>> -#endif
>> -}
>> -
>>  static void erratum_rcw_src(void)
>>  {
>>  #if defined(CONFIG_SPL)
>> @@ -187,7 +178,6 @@ void bypass_smmu(void)  }  void
>> fsl_lsch3_early_init_f(void)  {
>> -	erratum_a008751();
>>  	erratum_rcw_src();
>>  	init_early_memctl_regs();	/* tighten IFC timing */
>>  	erratum_a009203();
>>
>
>
>--
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 02, 2016 6:31 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at 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:
>> 1. Implementing the errata for LS2080.
>> 2. 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.

>>  	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?

>> +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

[U-Boot] [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum A008751

[Marek Vasut]

On 06/06/2016 06:23 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, June 02, 2016 6:28 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
>> <rajesh.bhagat@nxp.com>
>> Subject: Re: [PATCH v2 4/5] armv8/ls2080: Remove workaround for erratum
>> A008751
>>
>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>> Remove the errata workaround from soc specific file for LS2080
>>
>> The commit message does not explain why this change is legal or needed.
>>
> 
> This errata a008751 is applied on Soc specific file currently.
> We are moving this errata to a a file where all the errata implementation will take Place for usb for fsl. Implementation of the errata will follow in Patch 5/5.
> 
> However, I cannot reference to the driver specific implementation till it is committed.
> So, this is just the description of removing the errata from soc specific file.

This explanation should be in the commit message.

>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>> ---
>>> Changes in v2:
>>>   - No update
>>>
>>>
>>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c | 10 ----------
>>>  1 file changed, 10 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> index 0fb5c7f..5636531 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> @@ -122,15 +122,6 @@ void erratum_a009635(void)  }
>>>  #endif	/* CONFIG_SYS_FSL_ERRATUM_A009635 */
>>>
>>> -static void erratum_a008751(void)
>>> -{
>>> -#ifdef CONFIG_SYS_FSL_ERRATUM_A008751
>>> -	u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE;
>>> -
>>> -	writel(0x27672b2a, scfg + SCFG_USB3PRM1CR / 4);
>>> -#endif
>>> -}
>>> -
>>>  static void erratum_rcw_src(void)
>>>  {
>>>  #if defined(CONFIG_SPL)
>>> @@ -187,7 +178,6 @@ void bypass_smmu(void)  }  void
>>> fsl_lsch3_early_init_f(void)  {
>>> -	erratum_a008751();
>>>  	erratum_rcw_src();
>>>  	init_early_memctl_regs();	/* tighten IFC timing */
>>>  	erratum_a009203();
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751

[Marek Vasut]

On 06/06/2016 06:24 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, June 02, 2016 6:31 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at 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:
>>> 1. Implementing the errata for LS2080.
>>> 2. 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 ?

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] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Marek Vasut]

On 06/06/2016 06:21 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, June 02, 2016 6:18 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
>> <rajesh.bhagat@nxp.com>
>> Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>> helper
>>
>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>> ---
>>> Changes in v2:
>>>   - No update
>>>
>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>>  4 files changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> index 9a5a6b5..9c575c1 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>>  	return -1;      /* cannot identify the cluster */
>>>  }
>>>
>>> +uint get_svr(void)
>>
>> Should be u32 . The function should have some more recognizable name,
>> fsl_get_svr() or such.
>>
> 
> Hello Marek,
> 
> As done in PPC for all the Socs for FSL, in arch/powerpc/cpu/mpc83xx/start.S , and ARM v7 LS1021, in arch/arm/cpu/armv7/ls102xa/cpu.c, this uint get_svr(void) function finds the svr number for the SoC. Now, as i have to make the code generic for PPC and ARM (in patch 3/5 namely make errata function common for PPC and ARM), I am currently having the same name and return type used.

You can rename it now.

> Regards,
> Sriram
> 
>>> +{
>>> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
>>> +
>>> +	return gur_in32(&gur->svr);
>>> +}
>>> +
>>>  #ifdef CONFIG_DISPLAY_CPUINFO
>>>  int print_cpuinfo(void)
>>>  {
>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> index 57b99d4..4151994 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>>>  #define SCR0_CLIENTPD_MASK		0x00000001
>>>  #define SCR0_USFCFG_MASK		0x00000400
>>>
>>> +uint get_svr(void);
>>> +
>>>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> index 65b3357..e48bbaf 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> @@ -319,4 +319,7 @@ struct ccsr_reset {
>>>  	u32 ip_rev1;			/* 0xbf8 */
>>>  	u32 ip_rev2;			/* 0xbfc */
>>>  };
>>> +
>>> +uint get_svr(void);
>>> +
>>>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */ diff --git
>>> a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>> b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>> index 831d817..3f1a0a8 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>> @@ -52,6 +52,8 @@ struct cpu_type {
>>>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>>>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>>>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
>>> +#define IS_SVR_REV(svr, maj, min) \
>>> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>>>
>>>  /* ahci port register default value */
>>>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Monday, June 06, 2016 6:21 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>helper
>
>On 06/06/2016 06:21 AM, Sriram Dash wrote:
>>> -----Original Message-----
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Thursday, June 02, 2016 6:18 PM
>>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>> Bhagat <rajesh.bhagat@nxp.com>
>>> Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and
>>> IS_SVR_REV helper
>>>
>>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>>> ---
>>>> Changes in v2:
>>>>   - No update
>>>>
>>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>>>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>>>  4 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> index 9a5a6b5..9c575c1 100644
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>>>  	return -1;      /* cannot identify the cluster */
>>>>  }
>>>>
>>>> +uint get_svr(void)
>>>
>>> Should be u32 . The function should have some more recognizable name,
>>> fsl_get_svr() or such.
>>>
>>
>> Hello Marek,
>>
>> As done in PPC for all the Socs for FSL, in arch/powerpc/cpu/mpc83xx/start.S ,
>and ARM v7 LS1021, in arch/arm/cpu/armv7/ls102xa/cpu.c, this uint get_svr(void)
>function finds the svr number for the SoC. Now, as i have to make the code generic
>for PPC and ARM (in patch 3/5 namely make errata function common for PPC and
>ARM), I am currently having the same name and return type used.
>
>You can rename it now.
>

Somewhat I agree to the point. It is good to have u32 instead of 
uint and the get_svr() function should have some more recognizable
name. But, this function is already being used in many files and this
would require for some uncalled 120 + changes for PPC and ARM v7,
which is not in the scope of this patchset.
Currently, I am focusing on the errata implementation, which I want
to make generic, by having the same names across PPC, ARMv7 and
ARMv8. So, I am planning to do this change at a later point of time.

>> Regards,
>> Sriram
>>
>>>> +{
>>>> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
>>>> +
>>>> +	return gur_in32(&gur->svr);
>>>> +}
>>>> +
>>>>  #ifdef CONFIG_DISPLAY_CPUINFO
>>>>  int print_cpuinfo(void)
>>>>  {
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> index 57b99d4..4151994 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>>>>  #define SCR0_CLIENTPD_MASK		0x00000001
>>>>  #define SCR0_USFCFG_MASK		0x00000400
>>>>
>>>> +uint get_svr(void);
>>>> +
>>>>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> index 65b3357..e48bbaf 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> @@ -319,4 +319,7 @@ struct ccsr_reset {
>>>>  	u32 ip_rev1;			/* 0xbf8 */
>>>>  	u32 ip_rev2;			/* 0xbfc */
>>>>  };
>>>> +
>>>> +uint get_svr(void);
>>>> +
>>>>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */ diff --git
>>>> a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> index 831d817..3f1a0a8 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> @@ -52,6 +52,8 @@ struct cpu_type {
>>>>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>>>>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>>>>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
>>>> +#define IS_SVR_REV(svr, maj, min) \
>>>> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>>>>
>>>>  /* ahci port register default value */
>>>>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Marek Vasut
>
>
>--
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Monday, June 06, 2016 6:15 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; albert.u.boot at 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 at denx.de]
>>> Sent: Thursday, June 02, 2016 6:31 PM
>>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at 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:
>>>> 1. Implementing the errata for LS2080.
>>>> 2. 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

[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Marek Vasut]

On 06/08/2016 06:12 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Monday, June 06, 2016 6:21 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
>> <rajesh.bhagat@nxp.com>
>> Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>> helper
>>
>> On 06/06/2016 06:21 AM, Sriram Dash wrote:
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Thursday, June 02, 2016 6:18 PM
>>>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>>>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>>> Bhagat <rajesh.bhagat@nxp.com>
>>>> Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and
>>>> IS_SVR_REV helper
>>>>
>>>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>>>> ---
>>>>> Changes in v2:
>>>>>   - No update
>>>>>
>>>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>>>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>>>>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>>>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>>>>  4 files changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>> index 9a5a6b5..9c575c1 100644
>>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>>>>  	return -1;      /* cannot identify the cluster */
>>>>>  }
>>>>>
>>>>> +uint get_svr(void)
>>>>
>>>> Should be u32 . The function should have some more recognizable name,
>>>> fsl_get_svr() or such.
>>>>
>>>
>>> Hello Marek,
>>>
>>> As done in PPC for all the Socs for FSL, in arch/powerpc/cpu/mpc83xx/start.S ,
>> and ARM v7 LS1021, in arch/arm/cpu/armv7/ls102xa/cpu.c, this uint get_svr(void)
>> function finds the svr number for the SoC. Now, as i have to make the code generic
>> for PPC and ARM (in patch 3/5 namely make errata function common for PPC and
>> ARM), I am currently having the same name and return type used.
>>
>> You can rename it now.
>>
> 
> Somewhat I agree to the point. It is good to have u32 instead of 
> uint and the get_svr() function should have some more recognizable
> name. But, this function is already being used in many files and this
> would require for some uncalled 120 + changes for PPC and ARM v7,
> which is not in the scope of this patchset.
> Currently, I am focusing on the errata implementation, which I want
> to make generic, by having the same names across PPC, ARMv7 and
> ARMv8. So, I am planning to do this change at a later point of time.

I really dislike such function names, but OK, it's manged to spread
through the codebase too much.


-- 
Best regards,
Marek Vasut