Supporting ARM v8 USB errata for FSL

Adds get_svr and IS_SVR_REV helpers for ARMv8 platforms, similar to PPC and ARMv7.

Signed-off-by Sriram Dash <[email protected]>

Signed-off-by Rajesh Bhagat <[email protected]>

---

Changes in v1:
- Added commit message

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

arch/arm/cpu/armv8/fsl-layerscape/cpu.c +7 -0

index 9a5a6b5..9c575c1 100644
@@ -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)
 {

arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h +2 -0

index e98e055..8b8a7c1 100644
@@ -596,4 +596,6 @@ struct ccsr_cci400 {
 #define SCR0_CLIENTPD_MASK		0x00000001
 #define SCR0_USFCFG_MASK		0x00000400
 
+uint get_svr(void);
+
 #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/

arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +3 -0

index 65b3357..e48bbaf 100644
@@ -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_ */

arch/arm/include/asm/arch-fsl-layerscape/soc.h +0 -0

index 02ecc62..f244904 100644

arch/arm/include/asm/arch-fsl-layerscape/soc.h +0 -0

 /* ahci port register default value */
 #define AHCI_PORT_PHY_1_CFG    0xa003fffe
 

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 6:58 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

Quoted 7 lines · Sriram Dash

Adds get_svr and IS_SVR_REV helpers for ARMv8 platforms, similar to PPC and ARMv7.

Signed-off-by Sriram Dash <[email protected]>

Signed-off-by Rajesh Bhagat <[email protected]>

---

Quoted 5 lines

Changes in v1:
- Added commit message

Changes in v2:
- No update

So what changed in V3 here ?

Quoted 57 lines

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

arch/arm/cpu/armv8/fsl-layerscape/cpu.c +7 -0

index 9a5a6b5..9c575c1 100644
@@ -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)
 {

arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h +2 -0

index e98e055..8b8a7c1 100644
@@ -596,4 +596,6 @@ struct ccsr_cci400 {
 #define SCR0_CLIENTPD_MASK		0x00000001
 #define SCR0_USFCFG_MASK		0x00000400
 
+uint get_svr(void);
+
 #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/

arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +3 -0

index 65b3357..e48bbaf 100644
@@ -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 02ecc62..f244904 100644

arch/arm/include/asm/arch-fsl-layerscape/soc.h +0 -0

@@ -53,6 +53,8 @@ struct cpu_type {
 #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)

arch/arm/include/asm/arch-fsl-layerscape/soc.h struct cpu_type { · line 57

Quoted 4 lines

arch/arm/include/asm/arch-fsl-layerscape/soc.h +2 -0

 #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))

This is susceptible to macro expansion problems, you need parenthesis around all the macro args:
((SVR_MAJ(svr) == (maj)) && (SVR_MIN(svr) == (min)))

arch/arm/include/asm/arch-fsl-layerscape/soc.h struct cpu_type {

Quoted 3 lines

arch/arm/include/asm/arch-fsl-layerscape/soc.h +0 -0

 /* ahci port register default value */
 #define AHCI_PORT_PHY_1_CFG    0xa003fffe
 

--
Best regards,
Marek Vasut

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 <[email protected]>

Signed-off-by Sriram Dash <[email protected]>

---

Changes in v3:
- Inverted the condition for has_erratum for better readability
- If fdt_fixup_usb_erratum fails, return ENOSPC and exit the fixup
- Added logic for handling the condition with different controllers with different erratas
- first check if the errata is applicable for the SoC
- then check if it is applicable for the controller
- if both are successful, then fix dt.

Changes in v2:
- Added patch description
- Removed the MACRO and use fdt_fixup_erratum function instead

 drivers/usb/common/fsl-dt-fixup.c | 88 ++++++++++++++++++++++-----------------
 include/fsl_usb.h                 |  6 +++
 2 files changed, 56 insertions(+), 38 deletions(-)

drivers/usb/common/fsl-dt-fixup.c +12 -4

index 6f31932..9cbd9d2 100644
@@ -20,9 +20,9 @@
 #endif
 
 static const char * const compat_usb_fsl[] = {
-	"fsl-usb2-mph",
-	"fsl-usb2-dr",
-	"snps,dwc3",
+	FSL_USB2_MPH,
+	FSL_USB2_DR,
+	SNPS_DWC3,
 	NULL
 };
 

@@ -80,16 +80,24 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 }
 
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
-				 int start_offset)
+				 const char *controller_type, int start_offset)
 {
 	int node_offset, err;
 	const char *node_type = NULL;
+	const char *node_name = NULL;
 
 	err = fdt_usb_get_node_type(blob, start_offset,
 				    &node_offset, &node_type);
 	if (err < 0)
 		return err;
 
+	if (!strcmp(node_type, FSL_USB2_MPH) || !strcmp(node_type, FSL_USB2_DR))
+		node_name = USB2_CI;
+	else
+		node_name = node_type;
+	if (strcmp(node_name, controller_type))
+		return err;
+
 	err = fdt_setprop(blob, node_offset, prop_erratum, NULL, 0);
 	if (err < 0) {
 		printf("ERROR: could not set %s for %s: %s.\n",

@@ -99,6 +107,23 @@ static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 	return node_offset;

drivers/usb/common/fsl-dt-fixup.c +30 -34

+		return -EINVAL;
+	*usb_erratum_off = fdt_fixup_usb_erratum(blob, buf, controller_type,
+						 *usb_erratum_off);
+	if (*usb_erratum_off < 0)
+		return -ENOSPC;
+	debug("Adding USB erratum %s\n", str);
+	return 0;
+}
+
 void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 {
 	static const char * const modes[] = { "host", "peripheral", "otg" };

@@ -111,6 +136,7 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 	int usb_phy_off = -1;
 	char str[5];
 	int i, j;
+	int ret;
 
 	for (i = 1; i <= CONFIG_USB_MAX_CONTROLLER_COUNT; i++) {
 		const char *dr_mode_type = NULL;

@@ -164,39 +190,25 @@ 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;
-		}
-
-		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;
-		}
+		ret = fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
+					USB2_CI, "a006261",
+					has_erratum_a006261);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
+					USB2_CI, "a007075",
+					has_erratum_a007075);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
+					USB2_CI, "a007792",
+					has_erratum_a007792);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
+					USB2_CI, "a005697",
+					has_erratum_a005697);
+		if (ret == -ENOSPC)
+			return;
 	}
 }

include/fsl_usb.h +0 -0

index 187e384..882a5f5 100644

include/fsl_usb.h +0 -0

 /* USB Erratum Checking code */
 #ifdef CONFIG_PPC
 static inline bool has_dual_phy(void)

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 7:01 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Quoted 49 lines · 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 <[email protected]>

Signed-off-by Sriram Dash <[email protected]>

---

Changes in v3:
- Inverted the condition for has_erratum for better readability
- If fdt_fixup_usb_erratum fails, return ENOSPC and exit the fixup
- Added logic for handling the condition with different controllers with different erratas
- first check if the errata is applicable for the SoC
- then check if it is applicable for the controller
- if both are successful, then fix dt.

Changes in v2:
- Added patch description
- Removed the MACRO and use fdt_fixup_erratum function instead

 drivers/usb/common/fsl-dt-fixup.c | 88 ++++++++++++++++++++++-----------------
 include/fsl_usb.h                 |  6 +++
 2 files changed, 56 insertions(+), 38 deletions(-)

drivers/usb/common/fsl-dt-fixup.c +4 -4

index 6f31932..9cbd9d2 100644
@@ -20,9 +20,9 @@
 #endif
 
 static const char * const compat_usb_fsl[] = {
-	"fsl-usb2-mph",
-	"fsl-usb2-dr",
-	"snps,dwc3",
+	FSL_USB2_MPH,
+	FSL_USB2_DR,
+	SNPS_DWC3,
 	NULL
 };
 

@@ -80,16 +80,24 @@ static int fdt_fixup_usb_mode_phy_type(void *blob,
 const char *mode,  }
 
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
-				 int start_offset)
+				 const char *controller_type, int start_offset)
 {

drivers/usb/common/fsl-dt-fixup.c static int fdt_fixup_usb_mode_phy_type(void *blob, · line 94

Quoted 10 lines

drivers/usb/common/fsl-dt-fixup.c +2 -0

 	int node_offset, err;
 	const char *node_type = NULL;
+	const char *node_name = NULL;
 
 	err = fdt_usb_get_node_type(blob, start_offset,
 				    &node_offset, &node_type);
 	if (err < 0)
 		return err;
 
+	if (!strcmp(node_type, FSL_USB2_MPH) || !strcmp(node_type,

FSL_USB2_DR))

drivers/usb/common/fsl-dt-fixup.c static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,

Quoted 9 lines

drivers/usb/common/fsl-dt-fixup.c +6 -0

+		node_name = USB2_CI;
+	else
+		node_name = node_type;
+	if (strcmp(node_name, controller_type))
+		return err;
+
 	err = fdt_setprop(blob, node_offset, prop_erratum, NULL, 0);
 	if (err < 0) {
 		printf("ERROR: could not set %s for %s: %s.\n", @@ -99,6 +107,23

@@

Quoted 4 lines

static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
return node_offset;
}

drivers/usb/common/fsl-dt-fixup.c static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum, · line 117

Quoted 8 lines

drivers/usb/common/fsl-dt-fixup.c +8 -0

+static int fdt_fixup_erratum(int *usb_erratum_off, void *blob,
+			     const char *controller_type, char *str,
+			     bool (*has_erratum)(void))
+{
+	char buf[32] = {0};
+
+	snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
+	if (!(has_erratum()))

No need for the extra parenthesis around has_erratum() .

Quoted 86 lines

drivers/usb/common/fsl-dt-fixup.c +30 -34

+		return -EINVAL;
+	*usb_erratum_off = fdt_fixup_usb_erratum(blob, buf, controller_type,
+						 *usb_erratum_off);
+	if (*usb_erratum_off < 0)
+		return -ENOSPC;
+	debug("Adding USB erratum %s\n", str);
+	return 0;
+}
+
 void fdt_fixup_dr_usb(void *blob, bd_t *bd)  {
 	static const char * const modes[] = { "host", "peripheral", "otg" };

@@ -111,6 +136,7 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 	int usb_phy_off = -1;
 	char str[5];
 	int i, j;
+	int ret;
 
 	for (i = 1; i <= CONFIG_USB_MAX_CONTROLLER_COUNT; i++) {
 		const char *dr_mode_type = NULL;

@@ -164,39 +190,25 @@ 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;
-		}
-
-		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;
-		}
+		ret = fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
+					USB2_CI, "a006261",
+					has_erratum_a006261);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
+					USB2_CI, "a007075",
+					has_erratum_a007075);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
+					USB2_CI, "a007792",
+					has_erratum_a007792);
+		if (ret == -ENOSPC)
+			return;
+		ret = fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
+					USB2_CI, "a005697",
+					has_erratum_a005697);
+		if (ret == -ENOSPC)
+			return;
 	}
 }

include/fsl_usb.h index renamed from include/fsl_usb.h +0 -0

187e384..882a5f5 100644

include/fsl_usb.h +0 -0

@@ -85,6 +85,12 @@ struct ccsr_usb_phy {  #define

CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif

include/fsl_usb.h struct ccsr_usb_phy { · line 92

Quoted 5 lines

include/fsl_usb.h +5 -0

+/* USB Controllers */
+#define FSL_USB2_MPH	"fsl-usb2-mph"
+#define FSL_USB2_DR	"fsl-usb2-dr"
+#define USB2_CI		"usb2-ci"
+#define SNPS_DWC3	"snps,dwc3"

Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?

Quoted 4 lines

/* USB Erratum Checking code */
#ifdef CONFIG_PPC
static inline bool has_dual_phy(void)

--
Best regards,
Marek Vasut

Quoted 8 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 7:01 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Quoted 11 lines

include/fsl_usb.h struct ccsr_usb_phy { · line 91

Quoted 11 lines

include/fsl_usb.h index renamed from include/fsl_usb.h +0 -0

187e384..882a5f5 100644

include/fsl_usb.h +0 -0

@@ -85,6 +85,12 @@ struct ccsr_usb_phy {  #define

CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif

include/fsl_usb.h +4 -0

+/* USB Controllers */
+#define FSL_USB2_MPH	"fsl-usb2-mph"
+#define FSL_USB2_DR	"fsl-usb2-dr"
+#define USB2_CI		"usb2-ci"

Quoted 4 lines

Quoted 1 line

+#define SNPS_DWC3 "snps,dwc3"

Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?

Currently, the macro are only being used for dt fixup for fsl usb. However, in the near future, they might be needed for some other purpose, for example, board specific settings.

So, i think i will stick with the global macro.

Quoted 9 lines

Quoted 4 lines

/* USB Erratum Checking code */
#ifdef CONFIG_PPC
static inline bool has_dual_phy(void)

--
Best regards,
Marek Vasut

From: Marek Vasut [mailto:marex at denx.de]

Quoted 8 lines · Sriram Dash

Quoted 8 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 7:01 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Would you please fix your mailer to omit this useless header ?

[...]

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

include/fsl_usb.h index renamed from include/fsl_usb.h +0 -0

187e384..882a5f5 100644

include/fsl_usb.h +0 -0

@@ -85,6 +85,12 @@ struct ccsr_usb_phy {  #define

CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif

Quoted 4 lines

Quoted 4 lines

include/fsl_usb.h struct ccsr_usb_phy { · line 91

Quoted 4 lines

include/fsl_usb.h +4 -0

+/* USB Controllers */
+#define FSL_USB2_MPH	"fsl-usb2-mph"
+#define FSL_USB2_DR	"fsl-usb2-dr"
+#define USB2_CI		"usb2-ci"

What is this "usb2-ci" string ? Is this some DT property or DT compatible property or just some random string?

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

+#define SNPS_DWC3 "snps,dwc3"

Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?

Quoted 3 lines

Currently, the macro are only being used for dt fixup for fsl usb. However, in the near future, they might be needed for some other purpose, for example, board specific settings.

If they will be used for other purpose, then they can be moved. They are local now, so keep them local.

Quoted 11 lines

So, i think i will stick with the global macro.

Quoted 9 lines

Quoted 4 lines

/* USB Erratum Checking code */
#ifdef CONFIG_PPC
static inline bool has_dual_phy(void)

--
Best regards,
Marek Vasut

--
Best regards,
Marek Vasut

Quoted 11 lines

From: Marek Vasut [mailto:marex at denx.de]

Quoted 8 lines · Sriram Dash

Quoted 8 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 7:01 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Quoted 4 lines

Would you please fix your mailer to omit this useless header ?

[...]

Will take care next time onwards.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

include/fsl_usb.h index renamed from include/fsl_usb.h +0 -0

187e384..882a5f5 100644

include/fsl_usb.h +0 -0

@@ -85,6 +85,12 @@ struct ccsr_usb_phy {  #define

CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

include/fsl_usb.h struct ccsr_usb_phy { · line 91

Quoted 4 lines

include/fsl_usb.h +4 -0

+/* USB Controllers */
+#define FSL_USB2_MPH	"fsl-usb2-mph"
+#define FSL_USB2_DR	"fsl-usb2-dr"
+#define USB2_CI		"usb2-ci"

What is this "usb2-ci" string ? Is this some DT property or DT compatible property or just some random string?

Currently, for chipidea usb 2 controller, there is no generic DT compatible property.

Some names used are "qcom,ci-hdrc", "chipidea,usb2","fsl-usb2-dr","fsl-usb2-mph ". This string "usb2-ci" is not used currently for DT property.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

+#define SNPS_DWC3 "snps,dwc3"

Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?

Quoted 7 lines

Quoted 3 lines

Currently, the macro are only being used for dt fixup for fsl usb. However, in the near future, they might be needed for some other purpose, for example, board specific settings.

If they will be used for other purpose, then they can be moved. They are local now, so keep them local.

Ok. I will move the global macro to fsl-dt-fixup.c.

Quoted 16 lines

Quoted 11 lines

So, i think i will stick with the global macro.

Quoted 9 lines

Quoted 4 lines

/* USB Erratum Checking code */
#ifdef CONFIG_PPC
static inline bool has_dual_phy(void)

--
Best regards,
Marek Vasut

--
Best regards,
Marek Vasut

From: Marek Vasut [mailto:marex at denx.de]

Quoted 34 lines · Sriram Dash

Quoted 11 lines

From: Marek Vasut [mailto:marex at denx.de] On 06/10/2016 05:47 AM,

Quoted 8 lines · Sriram Dash

Quoted 8 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, June 09, 2016 7:01 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; albert.u.boot at aribaud.net; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Quoted 4 lines

Would you please fix your mailer to omit this useless header ?

[...]

Will take care next time onwards.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

include/fsl_usb.h index renamed from include/fsl_usb.h +0 -0

187e384..882a5f5 100644

include/fsl_usb.h +0 -0

@@ -85,6 +85,12 @@ struct ccsr_usb_phy {  #define

CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

include/fsl_usb.h struct ccsr_usb_phy { · line 91

Quoted 4 lines

include/fsl_usb.h +4 -0

+/* USB Controllers */
+#define FSL_USB2_MPH	"fsl-usb2-mph"
+#define FSL_USB2_DR	"fsl-usb2-dr"
+#define USB2_CI		"usb2-ci"

What is this "usb2-ci" string ? Is this some DT property or DT compatible property or just some random string?

Quoted 2 lines

Currently, for chipidea usb 2 controller, there is no generic DT compatible property.

Linux kernel documentation says there is "chipidea,usb2":

https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/ci-hdrc-
usb2.txt

Quoted 3 lines

Some names used are "qcom,ci-hdrc", "chipidea,usb2","fsl-usb2-dr","fsl-usb2-mph ". This string "usb2-ci" is not used currently for DT property.

The "chipidea,usb2" is generic one, all the others are either heritage from the past or more precise specifiers of the IP block. Do not invent yet another new compatible property in this case, it makes zero sense.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

+#define SNPS_DWC3 "snps,dwc3"

Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?

Currently, the macro are only being used for dt fixup for fsl usb. However, in the near future, they might be needed for some other purpose, for example, board specific settings.

Quoted 22 lines

Quoted 3 lines

If they will be used for other purpose, then they can be moved. They are local now, so keep them local.

Ok. I will move the global macro to fsl-dt-fixup.c.

Quoted 16 lines

Quoted 11 lines

So, i think i will stick with the global macro.

Quoted 9 lines

Quoted 4 lines

/* USB Erratum Checking code */
#ifdef CONFIG_PPC
static inline bool has_dual_phy(void)

--
Best regards,
Marek Vasut

--
Best regards,
Marek Vasut

--
Best regards,
Marek Vasut