[U-Boot] [PATCH v5 0/4] Make usb device-tree fixup independent of USB controller

[U-Boot] [PATCH v5 0/4] Make usb device-tree fixup independent of USB controller

[Sriram Dash]

Makes usb device-tree fixup independent of Controller type. This enables the
usage of device-tree fixup as a common framework for EHCI and XHCI controllers

Sriram Dash (4):
  drivers:usb:common:fsl-dt-fixup: Move device-tree fixup framework to
    common file
  drivers:usb:common:fsl-dt-fixup: Remove code duplication for
    fdt_usb_get_node_type
  drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for
    xhci controller
  drivers:usb:common:fsl-dt-fixup: fix return value of
    fdt_usb_get_node_type

 Makefile                          |   1 +
 drivers/usb/common/Makefile       |   7 ++
 drivers/usb/common/fsl-dt-fixup.c | 203 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-fsl.c       | 195 ------------------------------------
 include/fdt_support.h             |   4 +-
 5 files changed, 213 insertions(+), 197 deletions(-)
 create mode 100644 drivers/usb/common/Makefile
 create mode 100644 drivers/usb/common/fsl-dt-fixup.c

-- 
2.1.0

[U-Boot] [PATCH v5 1/4] drivers:usb:common:fsl-dt-fixup: Move device-tree fixup framework to common file

[Sriram Dash]

Move usb device-tree fixup framework from ehci-fsl.c to common place so
that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
---
Changes in v5:
  - No update
Changes in v4:
  - Retain copyright info
  - Remove #include from fsl-dt-fixup.c which are not used currently.
Changes in v3:
  - git commit -M -C to generate patches
  - Break the patch 1(Moving dt fix up and removing code duplication) into 2 patches
Changes in v2:
  - Remove the #defines from the patch

 Makefile                                           |   1 +
 drivers/usb/common/Makefile                        |   6 +
 .../usb/{host/ehci-fsl.c => common/fsl-dt-fixup.c} | 157 -----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 4 files changed, 7 insertions(+), 352 deletions(-)
 create mode 100644 drivers/usb/common/Makefile
 copy drivers/usb/{host/ehci-fsl.c => common/fsl-dt-fixup.c} (54%)

diff --git a/Makefile b/Makefile
index 53569e8..97a78d3 100644
--- a/Makefile
+++ b/Makefile
@@ -648,6 +648,7 @@ libs-$(CONFIG_SYS_FSL_DDR) += drivers/ddr/fsl/
 libs-$(CONFIG_ALTERA_SDRAM) += drivers/ddr/altera/
 libs-y += drivers/serial/
 libs-y += drivers/usb/dwc3/
+libs-y += drivers/usb/common/
 libs-y += drivers/usb/emul/
 libs-y += drivers/usb/eth/
 libs-y += drivers/usb/gadget/
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
new file mode 100644
index 0000000..a38ee4a
--- /dev/null
+++ b/drivers/usb/common/Makefile
@@ -0,0 +1,6 @@
+# (C) Copyright 2016 Freescale Semiconductor, Inc.
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
+
+obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/common/fsl-dt-fixup.c
similarity index 54%
copy from drivers/usb/host/ehci-fsl.c
copy to drivers/usb/common/fsl-dt-fixup.c
index 97b7f14..92adb46 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -9,172 +9,16 @@
  */
 
 #include <common.h>
-#include <pci.h>
 #include <usb.h>
 #include <asm/io.h>
-#include <usb/ehci-fsl.h>
 #include <hwconfig.h>
 #include <fsl_usb.h>
 #include <fdt_support.h>
 
-#include "ehci.h"
-
 #ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
-static void set_txfifothresh(struct usb_ehci *, u32);
-
-/* Check USB PHY clock valid */
-static int usb_phy_clk_valid(struct usb_ehci *ehci)
-{
-	if (!((in_be32(&ehci->control) & PHY_CLK_VALID) ||
-			in_be32(&ehci->prictrl))) {
-		printf("USB PHY clock invalid!\n");
-		return 0;
-	} else {
-		return 1;
-	}
-}
-
-/*
- * Create the appropriate control structures to manage
- * a new EHCI host controller.
- *
- * Excerpts from linux ehci fsl driver.
- */
-int ehci_hcd_init(int index, enum usb_init_type init,
-		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
-{
-	struct usb_ehci *ehci = NULL;
-	const char *phy_type = NULL;
-	size_t len;
-	char current_usb_controller[5];
-#ifdef CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
-	char usb_phy[5];
-
-	usb_phy[0] = '\0';
-#endif
-	if (has_erratum_a007075()) {
-		/*
-		 * A 5ms delay is needed after applying soft-reset to the
-		 * controller to let external ULPI phy come out of reset.
-		 * This delay needs to be added before re-initializing
-		 * the controller after soft-resetting completes
-		 */
-		mdelay(5);
-	}
-	memset(current_usb_controller, '\0', 5);
-	snprintf(current_usb_controller, 4, "usb%d", index+1);
-
-	switch (index) {
-	case 0:
-		ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB1_ADDR;
-		break;
-	case 1:
-		ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB2_ADDR;
-		break;
-	default:
-		printf("ERROR: wrong controller index!!\n");
-		return -EINVAL;
-	};
-
-	*hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
-	*hcor = (struct ehci_hcor *)((uint32_t) *hccr +
-			HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
-
-	/* Set to Host mode */
-	setbits_le32(&ehci->usbmode, CM_HOST);
-
-	out_be32(&ehci->snoop1, SNOOP_SIZE_2GB);
-	out_be32(&ehci->snoop2, 0x80000000 | SNOOP_SIZE_2GB);
-
-	/* Init phy */
-	if (hwconfig_sub(current_usb_controller, "phy_type"))
-		phy_type = hwconfig_subarg(current_usb_controller,
-				"phy_type", &len);
-	else
-		phy_type = getenv("usb_phy_type");
-
-	if (!phy_type) {
-#ifdef CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
-		/* if none specified assume internal UTMI */
-		strcpy(usb_phy, "utmi");
-		phy_type = usb_phy;
-#else
-		printf("WARNING: USB phy type not defined !!\n");
-		return -1;
-#endif
-	}
-
-	if (!strncmp(phy_type, "utmi", 4)) {
-#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
-		clrsetbits_be32(&ehci->control, CONTROL_REGISTER_W1C_MASK,
-				PHY_CLK_SEL_UTMI);
-		clrsetbits_be32(&ehci->control, CONTROL_REGISTER_W1C_MASK,
-				UTMI_PHY_EN);
-		udelay(1000); /* delay required for PHY Clk to appear */
-#endif
-		out_le32(&(*hcor)->or_portsc[0], PORT_PTS_UTMI);
-		clrsetbits_be32(&ehci->control, CONTROL_REGISTER_W1C_MASK,
-				USB_EN);
-	} else {
-		clrsetbits_be32(&ehci->control, CONTROL_REGISTER_W1C_MASK,
-				PHY_CLK_SEL_ULPI);
-		clrsetbits_be32(&ehci->control, UTMI_PHY_EN |
-				CONTROL_REGISTER_W1C_MASK, USB_EN);
-		udelay(1000); /* delay required for PHY Clk to appear */
-		if (!usb_phy_clk_valid(ehci))
-			return -EINVAL;
-		out_le32(&(*hcor)->or_portsc[0], PORT_PTS_ULPI);
-	}
-
-	out_be32(&ehci->prictrl, 0x0000000c);
-	out_be32(&ehci->age_cnt_limit, 0x00000040);
-	out_be32(&ehci->sictrl, 0x00000001);
-
-	in_le32(&ehci->usbmode);
-
-	if (has_erratum_a007798())
-		set_txfifothresh(ehci, TXFIFOTHRESH);
-
-	if (has_erratum_a004477()) {
-		/*
-		 * When reset is issued while any ULPI transaction is ongoing
-		 * then it may result to corruption of ULPI Function Control
-		 * Register which eventually causes phy clock to enter low
-		 * power mode which stops the clock. Thus delay is required
-		 * before reset to let ongoing ULPI transaction complete.
-		 */
-		udelay(1);
-	}
-	return 0;
-}
-
-/*
- * Destroy the appropriate control structures corresponding
- * the the EHCI host controller.
- */
-int ehci_hcd_stop(int index)
-{
-	return 0;
-}
-
-/*
- * Setting the value of TXFIFO_THRESH field in TXFILLTUNING register
- * to counter DDR latencies in writing data into Tx buffer.
- * This prevents Tx buffer from getting underrun
- */
-static void set_txfifothresh(struct usb_ehci *ehci, u32 txfifo_thresh)
-{
-	u32 cmd;
-	cmd = ehci_readl(&ehci->txfilltuning);
-	cmd &= ~TXFIFO_THRESH_MASK;
-	cmd |= TXFIFO_THRESH(txfifo_thresh);
-	ehci_writel(&ehci->txfilltuning, cmd);
-}
-
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
 static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 				       const char *phy_type, int start_offset)
 {
@@ -367,4 +211,3 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
 		}
 	}
 }
-#endif
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 97b7f14..ec6b8fe 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -173,198 +173,3 @@ static void set_txfifothresh(struct usb_ehci *ehci, u32 txfifo_thresh)
 	cmd |= TXFIFO_THRESH(txfifo_thresh);
 	ehci_writel(&ehci->txfilltuning, cmd);
 }
-
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
-	const char *node_type = NULL;
-	int node_offset;
-	int err;
-
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
-		}
-		node_type = compat_dr;
-	} else {
-		node_type = compat_mph;
-	}
-
-	if (mode) {
-		err = fdt_setprop(blob, node_offset, prop_mode, mode,
-				  strlen(mode) + 1);
-		if (err < 0)
-			printf("WARNING: could not set %s for %s: %s.\n",
-			       prop_mode, node_type, fdt_strerror(err));
-	}
-
-	if (phy_type) {
-		err = fdt_setprop(blob, node_offset, prop_type, phy_type,
-				  strlen(phy_type) + 1);
-		if (err < 0)
-			printf("WARNING: could not set %s for %s: %s.\n",
-			       prop_type, node_type, fdt_strerror(err));
-	}
-
-	return node_offset;
-}
-
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
-static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
-				 int start_offset)
-{
-	int node_offset, err;
-	const char *node_type = NULL;
-
-	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
-	if (!node_type)
-		return -1;
-
-	err = fdt_setprop(blob, node_offset, prop_erratum, NULL, 0);
-	if (err < 0) {
-		printf("ERROR: could not set %s for %s: %s.\n",
-		       prop_erratum, node_type, fdt_strerror(err));
-	}
-
-	return node_offset;
-}
-
-void fdt_fixup_dr_usb(void *blob, bd_t *bd)
-{
-	static const char * const modes[] = { "host", "peripheral", "otg" };
-	static const char * const phys[] = { "ulpi", "utmi", "utmi_dual" };
-	int usb_erratum_a006261_off = -1;
-	int usb_erratum_a007075_off = -1;
-	int usb_erratum_a007792_off = -1;
-	int usb_erratum_a005697_off = -1;
-	int usb_mode_off = -1;
-	int usb_phy_off = -1;
-	char str[5];
-	int i, j;
-
-	for (i = 1; i <= CONFIG_USB_MAX_CONTROLLER_COUNT; i++) {
-		const char *dr_mode_type = NULL;
-		const char *dr_phy_type = NULL;
-		int mode_idx = -1, phy_idx = -1;
-
-		snprintf(str, 5, "%s%d", "usb", i);
-		if (hwconfig(str)) {
-			for (j = 0; j < ARRAY_SIZE(modes); j++) {
-				if (hwconfig_subarg_cmp(str, "dr_mode",
-							modes[j])) {
-					mode_idx = j;
-					break;
-				}
-			}
-
-			for (j = 0; j < ARRAY_SIZE(phys); j++) {
-				if (hwconfig_subarg_cmp(str, "phy_type",
-							phys[j])) {
-					phy_idx = j;
-					break;
-				}
-			}
-
-			if (mode_idx < 0 && phy_idx < 0) {
-				printf("WARNING: invalid phy or mode\n");
-				return;
-			}
-
-			if (mode_idx > -1)
-				dr_mode_type = modes[mode_idx];
-
-			if (phy_idx > -1)
-				dr_phy_type = phys[phy_idx];
-		}
-
-		if (has_dual_phy())
-			dr_phy_type = phys[2];
-
-		usb_mode_off = fdt_fixup_usb_mode_phy_type(blob,
-							   dr_mode_type, NULL,
-							   usb_mode_off);
-
-		if (usb_mode_off < 0)
-			return;
-
-		usb_phy_off = fdt_fixup_usb_mode_phy_type(blob,
-							  NULL, dr_phy_type,
-							  usb_phy_off);
-
-		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;
-		}
-	}
-}
-#endif
-- 
2.1.0

[U-Boot] [PATCH v5 1/4] drivers:usb:common:fsl-dt-fixup: Move device-tree fixup framework to common file

[Marek Vasut]

On 03/22/2016 08:10 AM, Sriram Dash wrote:
> Move usb device-tree fixup framework from ehci-fsl.c to common place so
> that it can be used by other drivers as well (xhci-fsl.c).
> 
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH v5 2/4] drivers:usb:common:fsl-dt-fixup: Remove code duplication for fdt_usb_get_node_type

[Sriram Dash]

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to
avoid code duplication.

Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v5:
  - reorder the functions and gets rid of the forward declaration
Changes in v4:
  - Make minimal modification to code
Changes in v3:
  - Move the duplication of code to new patch

 drivers/usb/common/fsl-dt-fixup.c | 72 ++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
index 92adb46..eb13f12 100644
--- a/drivers/usb/common/fsl-dt-fixup.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -19,33 +19,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
 
+	return node_type;
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+				       const char *phy_type, int start_offset)
+{
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);
@@ -65,32 +77,6 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 				 int start_offset)
 {
-- 
2.1.0

[U-Boot] [PATCH v5 2/4] drivers:usb:common:fsl-dt-fixup: Remove code duplication for fdt_usb_get_node_type

[Marek Vasut]

On 03/22/2016 08:10 AM, Sriram Dash wrote:
> Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to
> avoid code duplication.
> 
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for xhci controller

[Sriram Dash]

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
---
Changes in v5:
  - Make the array static const
Changes in v4:
  - Use a terminating entry in the array for getting node type for controller
Changes in v3:
  - Modify the Makefile to remove comparison
  - Put the supported controllers in array and checking from array
Changes in v2:
  - Remove the #defines from the patch and adding controller support

 drivers/usb/common/Makefile       |  1 +
 drivers/usb/common/fsl-dt-fixup.c | 33 +++++++++++++++++----------------
 include/fdt_support.h             |  4 ++--
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index a38ee4a..2f3d43d 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,3 +4,4 @@
 #
 
 obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
+obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
index eb13f12..13f9fb8 100644
--- a/drivers/usb/common/fsl-dt-fixup.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -19,27 +19,28 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
+static const char compat_usb_fsl[] = {
+	"fsl-usb2-mph" "\0"
+	"fsl-usb2-dr" "\0"
+	"snps,dwc3" "\0"
+};
+
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
+	const char *node_name, *nxt;
+
+	for (node_name = compat_usb_fsl; *node_name; node_name = nxt + 1) {
+		nxt = node_name;
+		while (*nxt)
+			++nxt;
+		*node_offset = fdt_node_offset_by_compatible
+			       (blob, start_offset, node_name);
+		if (*node_offset >= 0) {
+			node_type = node_name;
+			break;
 		}
-	} else {
-		node_type = compat_mph;
 	}
 
 	return node_type;
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 296add0..d34e959 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -113,11 +113,11 @@ void fdt_fixup_qe_firmware(void *fdt);
  */
 int fdt_fixup_display(void *blob, const char *path, const char *display);
 
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
+#if defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL)
 void fdt_fixup_dr_usb(void *blob, bd_t *bd);
 #else
 static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
-#endif /* defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB) */
+#endif /* defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL) */
 
 #if defined(CONFIG_SYS_FSL_SEC_COMPAT)
 void fdt_fixup_crypto_node(void *blob, int sec_rev);
-- 
2.1.0

[U-Boot] [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for xhci controller

[Marek Vasut]

On 03/22/2016 08:10 AM, Sriram Dash wrote:
> Enables usb device-tree fixup code to incorporate xhci controller
> 
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> ---
> Changes in v5:
>   - Make the array static const
> Changes in v4:
>   - Use a terminating entry in the array for getting node type for controller
> Changes in v3:
>   - Modify the Makefile to remove comparison
>   - Put the supported controllers in array and checking from array
> Changes in v2:
>   - Remove the #defines from the patch and adding controller support
> 
>  drivers/usb/common/Makefile       |  1 +
>  drivers/usb/common/fsl-dt-fixup.c | 33 +++++++++++++++++----------------
>  include/fdt_support.h             |  4 ++--
>  3 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index a38ee4a..2f3d43d 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,3 +4,4 @@
>  #
>  
>  obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
> +obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
> diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
> index eb13f12..13f9fb8 100644
> --- a/drivers/usb/common/fsl-dt-fixup.c
> +++ b/drivers/usb/common/fsl-dt-fixup.c
> @@ -19,27 +19,28 @@
>  #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
>  #endif
>  
> +static const char compat_usb_fsl[] = {
> +	"fsl-usb2-mph" "\0"
> +	"fsl-usb2-dr" "\0"
> +	"snps,dwc3" "\0"
> +};

This is supposed to be static constant array of strings. Can you tell
me, based on your knowledge of the C language, what is wrong with this
construct ?

Best regards,
Marek Vasut

[U-Boot] [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for xhci controller

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Tuesday, March 22, 2016 12:54 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; Ramneek Mehresh
><ramneek.mehresh@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup
>support for xhci controller
>
>On 03/22/2016 08:10 AM, Sriram Dash wrote:
>> Enables usb device-tree fixup code to incorporate xhci controller
>>
>> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> ---
>> Changes in v5:
>>   - Make the array static const
>> Changes in v4:
>>   - Use a terminating entry in the array for getting node type for
>> controller Changes in v3:
>>   - Modify the Makefile to remove comparison
>>   - Put the supported controllers in array and checking from array
>> Changes in v2:
>>   - Remove the #defines from the patch and adding controller support
>>
>>  drivers/usb/common/Makefile       |  1 +
>>  drivers/usb/common/fsl-dt-fixup.c | 33 +++++++++++++++++----------------
>>  include/fdt_support.h             |  4 ++--
>>  3 files changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>> index a38ee4a..2f3d43d 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -4,3 +4,4 @@
>>  #
>>
>>  obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
>> +obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>> b/drivers/usb/common/fsl-dt-fixup.c
>> index eb13f12..13f9fb8 100644
>> --- a/drivers/usb/common/fsl-dt-fixup.c
>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>> @@ -19,27 +19,28 @@
>>  #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
>>
>> +static const char compat_usb_fsl[] = {
>> +	"fsl-usb2-mph" "\0"
>> +	"fsl-usb2-dr" "\0"
>> +	"snps,dwc3" "\0"
>> +};
>
>This is supposed to be static constant array of strings. Can you tell me, based on
>your knowledge of the C language, what is wrong with this construct ?
>

IMO, above construct is correct. Originally, we proposed array of strings but later we changed to 
character array with terminating NULL entry for each element to incorporate your below review 
comment given in v3. 
"My opinion is to use a terminating NULL entry and iterate over the array until you reach it."

>Best regards,
>Marek Vasut

[U-Boot] [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for xhci controller

[Marek Vasut]

On 03/23/2016 09:30 AM, Sriram Dash wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Tuesday, March 22, 2016 12:54 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; Ramneek Mehresh
>> <ramneek.mehresh@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> Subject: Re: [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup
>> support for xhci controller
>>
>> On 03/22/2016 08:10 AM, Sriram Dash wrote:
>>> Enables usb device-tree fixup code to incorporate xhci controller
>>>
>>> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>> ---
>>> Changes in v5:
>>>   - Make the array static const
>>> Changes in v4:
>>>   - Use a terminating entry in the array for getting node type for
>>> controller Changes in v3:
>>>   - Modify the Makefile to remove comparison
>>>   - Put the supported controllers in array and checking from array
>>> Changes in v2:
>>>   - Remove the #defines from the patch and adding controller support
>>>
>>>  drivers/usb/common/Makefile       |  1 +
>>>  drivers/usb/common/fsl-dt-fixup.c | 33 +++++++++++++++++----------------
>>>  include/fdt_support.h             |  4 ++--
>>>  3 files changed, 20 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>>> index a38ee4a..2f3d43d 100644
>>> --- a/drivers/usb/common/Makefile
>>> +++ b/drivers/usb/common/Makefile
>>> @@ -4,3 +4,4 @@
>>>  #
>>>
>>>  obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
>>> +obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
>>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>>> b/drivers/usb/common/fsl-dt-fixup.c
>>> index eb13f12..13f9fb8 100644
>>> --- a/drivers/usb/common/fsl-dt-fixup.c
>>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>>> @@ -19,27 +19,28 @@
>>>  #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
>>>
>>> +static const char compat_usb_fsl[] = {
>>> +	"fsl-usb2-mph" "\0"
>>> +	"fsl-usb2-dr" "\0"
>>> +	"snps,dwc3" "\0"
>>> +};
>>
>> This is supposed to be static constant array of strings. Can you tell me, based on
>> your knowledge of the C language, what is wrong with this construct ?
>>
> 
> IMO, above construct is correct. Originally, we proposed array of strings but later we changed to 
> character array with terminating NULL entry for each element to incorporate your below review 
> comment given in v3. 
> "My opinion is to use a terminating NULL entry and iterate over the array until you reach it."

Do this:

static const char *compat_usb_fsl[] = {
	"fsl-usb2-mph",
	"fsl-usb2-dr",
	"snps,dwc3",
	NULL
};

and then do

i = 0;
while (compat_usb_fsl[i]) {
	... do some stuff ...
	i++;
}

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup support for xhci controller

[Marek Vasut]

On 03/23/2016 09:41 AM, Marek Vasut wrote:
> On 03/23/2016 09:30 AM, Sriram Dash wrote:
>>
>>
>>> -----Original Message-----
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Tuesday, March 22, 2016 12:54 PM
>>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>>> Cc: york sun <york.sun@nxp.com>; Ramneek Mehresh
>>> <ramneek.mehresh@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>> Subject: Re: [PATCH v5 3/4] drivers:usb:common:fsl-dt-fixup: Add device-tree fixup
>>> support for xhci controller
>>>
>>> On 03/22/2016 08:10 AM, Sriram Dash wrote:
>>>> Enables usb device-tree fixup code to incorporate xhci controller
>>>>
>>>> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
>>>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>>>> ---
>>>> Changes in v5:
>>>>   - Make the array static const
>>>> Changes in v4:
>>>>   - Use a terminating entry in the array for getting node type for
>>>> controller Changes in v3:
>>>>   - Modify the Makefile to remove comparison
>>>>   - Put the supported controllers in array and checking from array
>>>> Changes in v2:
>>>>   - Remove the #defines from the patch and adding controller support
>>>>
>>>>  drivers/usb/common/Makefile       |  1 +
>>>>  drivers/usb/common/fsl-dt-fixup.c | 33 +++++++++++++++++----------------
>>>>  include/fdt_support.h             |  4 ++--
>>>>  3 files changed, 20 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>>>> index a38ee4a..2f3d43d 100644
>>>> --- a/drivers/usb/common/Makefile
>>>> +++ b/drivers/usb/common/Makefile
>>>> @@ -4,3 +4,4 @@
>>>>  #
>>>>
>>>>  obj-$(CONFIG_USB_EHCI_FSL) += fsl-dt-fixup.o
>>>> +obj-$(CONFIG_USB_XHCI_FSL) += fsl-dt-fixup.o
>>>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>>>> b/drivers/usb/common/fsl-dt-fixup.c
>>>> index eb13f12..13f9fb8 100644
>>>> --- a/drivers/usb/common/fsl-dt-fixup.c
>>>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>>>> @@ -19,27 +19,28 @@
>>>>  #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
>>>>
>>>> +static const char compat_usb_fsl[] = {
>>>> +	"fsl-usb2-mph" "\0"
>>>> +	"fsl-usb2-dr" "\0"
>>>> +	"snps,dwc3" "\0"
>>>> +};
>>>
>>> This is supposed to be static constant array of strings. Can you tell me, based on
>>> your knowledge of the C language, what is wrong with this construct ?
>>>
>>
>> IMO, above construct is correct. Originally, we proposed array of strings but later we changed to 
>> character array with terminating NULL entry for each element to incorporate your below review 
>> comment given in v3. 
>> "My opinion is to use a terminating NULL entry and iterate over the array until you reach it."
> 
> Do this:
> 
> static const char *compat_usb_fsl[] = {
> 	"fsl-usb2-mph",
> 	"fsl-usb2-dr",
> 	"snps,dwc3",
> 	NULL
> };
> 
> and then do
> 
> i = 0;
> while (compat_usb_fsl[i]) {
> 	... do some stuff ...
> 	i++;
> }

Or even use for(i = 0; compat_usb_fsl[i]; i++) { do something }

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of fdt_usb_get_node_type

[Sriram Dash]

Use int as it is native (and widely used) return type.

Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v5:
  - Modified title and description
  - Using error codes for return type.

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

diff --git a/drivers/usb/common/fsl-dt-fixup.c b/drivers/usb/common/fsl-dt-fixup.c
index 13f9fb8..7183b80 100644
--- a/drivers/usb/common/fsl-dt-fixup.c
+++ b/drivers/usb/common/fsl-dt-fixup.c
@@ -25,11 +25,11 @@ static const char compat_usb_fsl[] = {
 	"snps,dwc3" "\0"
 };
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
+static int fdt_usb_get_node_type(void *blob, int start_offset,
+				 int *node_offset, const char **node_type)
 {
-	const char *node_type = NULL;
 	const char *node_name, *nxt;
+	int ret = -ENOENT;
 
 	for (node_name = compat_usb_fsl; *node_name; node_name = nxt + 1) {
 		nxt = node_name;
@@ -38,12 +38,13 @@ static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 		*node_offset = fdt_node_offset_by_compatible
 			       (blob, start_offset, node_name);
 		if (*node_offset >= 0) {
-			node_type = node_name;
+			*node_type = node_name;
+			ret = 0;
 			break;
 		}
 	}
 
-	return node_type;
+	return ret;
 }
 
 static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
@@ -55,9 +56,10 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 	int node_offset;
 	int err;
 
-	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
-	if (!node_type)
-		return -1;
+	err = fdt_usb_get_node_type(blob, start_offset,
+				    &node_offset, &node_type);
+	if (err < 0)
+		return err;
 
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
@@ -84,9 +86,10 @@ static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 	int node_offset, err;
 	const char *node_type = NULL;
 
-	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
-	if (!node_type)
-		return -1;
+	err = fdt_usb_get_node_type(blob, start_offset,
+				    &node_offset, &node_type);
+	if (err < 0)
+		return err;
 
 	err = fdt_setprop(blob, node_offset, prop_erratum, NULL, 0);
 	if (err < 0) {
-- 
2.1.0

[U-Boot] [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of fdt_usb_get_node_type

[Marek Vasut]

On 03/22/2016 08:10 AM, Sriram Dash wrote:
> Use int as it is native (and widely used) return type.

What do you mean by "int ... is native (and widely used) return type" ?

It's just a signed type, that's all there is to it.

> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>

[...]

Best regards,
Marek Vasut

[U-Boot] [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of fdt_usb_get_node_type

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Tuesday, March 22, 2016 12:56 PM
>To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun@nxp.com>; Ramneek Mehresh
><ramneek.mehresh@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>Subject: Re: [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of
>fdt_usb_get_node_type
>
>On 03/22/2016 08:10 AM, Sriram Dash wrote:
>> Use int as it is native (and widely used) return type.
>
>What do you mean by "int ... is native (and widely used) return type" ?
>
>It's just a signed type, that's all there is to it.
>

Ok that may sound incorrect, Shall I change the description as below:
"Changes the return type of fdt_usb_get_node_type from char* to int"

>> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>
>[...]
>
>Best regards,
>Marek Vasut

[U-Boot] [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of fdt_usb_get_node_type

[Marek Vasut]

On 03/23/2016 09:31 AM, Sriram Dash wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Tuesday, March 22, 2016 12:56 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; Ramneek Mehresh
>> <ramneek.mehresh@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> Subject: Re: [PATCH v5 4/4] drivers:usb:common:fsl-dt-fixup: fix return value of
>> fdt_usb_get_node_type
>>
>> On 03/22/2016 08:10 AM, Sriram Dash wrote:
>>> Use int as it is native (and widely used) return type.
>>
>> What do you mean by "int ... is native (and widely used) return type" ?
>>
>> It's just a signed type, that's all there is to it.
>>
> 
> Ok that may sound incorrect, Shall I change the description as below:
> "Changes the return type of fdt_usb_get_node_type from char* to int"

Yeah, excellent.


-- 
Best regards,
Marek Vasut