[U-Boot] [PATCH v3 0/3] Make usb device-tree fixup independent of USB controller
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 (3): board:freescale:common: Move device-tree fixup framework to common file board:freescale:usb: Remove code duplication for fdt_usb_get_node_type board:freescale:usb: Add device-tree fixup support for xhci controller
board/freescale/common/Makefile | 3 + board/freescale/common/usb.c | 194 +++++++++++++++++++++++++++++++++++++++ drivers/usb/host/ehci-fsl.c | 195 ---------------------------------------- include/fdt_support.h | 4 +- 4 files changed, 199 insertions(+), 197 deletions(-) create mode 100644 board/freescale/common/usb.c
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 --- board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o + ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /* - * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc. - * - * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB - * - * Author: Tor Krill tor@excito.com + * (C) Copyright 2016 Freescale Semiconductor, Inc. * * SPDX-License-Identifier: GPL-2.0+ */ @@ -17,164 +13,11 @@ #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 +210,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
On 03/01/2016 08:03 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
board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
Where is the changelog ?
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /*
- (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- Author: Tor Krill tor@excito.com
- (C) Copyright 2016 Freescale Semiconductor, Inc.
What's with this copyright change here ?
- SPDX-License-Identifier: GPL-2.0+
*/ @@ -17,164 +13,11 @@ #include <fsl_usb.h> #include <fdt_support.h>
-#include "ehci.h"
[...]
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:43 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/01/2016 08:03 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
board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
Where is the changelog ?
Will include changelog for v2 and v3 in v4.
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /*
- (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- Author: Tor Krill tor@excito.com
- (C) Copyright 2016 Freescale Semiconductor, Inc.
What's with this copyright change here ?
It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c copyright information in the new file?
- SPDX-License-Identifier: GPL-2.0+
*/ @@ -17,164 +13,11 @@ #include <fsl_usb.h> #include <fdt_support.h>
-#include "ehci.h"
[...]
-- Best regards, Marek Vasut
Best Regards, Sriram
On 03/03/2016 09:29 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:43 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/01/2016 08:03 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
board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
Where is the changelog ?
Will include changelog for v2 and v3 in v4.
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /*
- (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- Author: Tor Krill tor@excito.com
- (C) Copyright 2016 Freescale Semiconductor, Inc.
What's with this copyright change here ?
It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c copyright information in the new file?
There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?
According to git, it's not a new file:
b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c
so yes, it should retain all copyright info.
And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.
- SPDX-License-Identifier: GPL-2.0+
*/ @@ -17,164 +13,11 @@ #include <fsl_usb.h> #include <fdt_support.h>
-#include "ehci.h"
[...]
-- Best regards, Marek Vasut
Best Regards, Sriram
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, March 03, 2016 3:26 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/03/2016 09:29 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:43 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/01/2016 08:03 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
board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
Where is the changelog ?
Will include changelog for v2 and v3 in v4.
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /*
- (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- Author: Tor Krill tor@excito.com
- (C) Copyright 2016 Freescale Semiconductor, Inc.
What's with this copyright change here ?
It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c
copyright information in the new file?
There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?
Yes
According to git, it's not a new file:
b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c
so yes, it should retain all copyright info.
Ok, We will retain copyright info in v4.
And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.
drivers/usb/host/ was a good option, but we want to make it independent of host and gadget. So, whenever there is a specific requirement for freescale boards, it will use the same from board: freescale: common: usb
Else, another option is to have drivers/usb/common/fsl-dt-fixup.c. What do you say?
- SPDX-License-Identifier: GPL-2.0+
*/ @@ -17,164 +13,11 @@ #include <fsl_usb.h> #include <fdt_support.h>
-#include "ehci.h"
[...]
-- Best regards, Marek Vasut
Best Regards, Sriram
-- Best regards, Marek Vasut
On 03/03/2016 12:10 PM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, March 03, 2016 3:26 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/03/2016 09:29 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:43 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@lists.denx.de Cc: york sun york.sun@nxp.com; Ramneek Mehresh ramneek.mehresh@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Tom Rini trini@konsulko.com Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file
On 03/01/2016 08:03 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
board/freescale/common/Makefile | 2 + .../ehci-fsl.c => board/freescale/common/usb.c | 160 +---------------- drivers/usb/host/ehci-fsl.c | 195 --------------------- 3 files changed, 3 insertions(+), 354 deletions(-) copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
Where is the changelog ?
Will include changelog for v2 and v3 in v4.
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index be114ce..62de45c 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -13,6 +13,8 @@ MINIMAL=y endif endif
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o obj- := __dummy__.o diff --git a/drivers/usb/host/ehci-fsl.c b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/board/freescale/common/usb.c @@ -1,9 +1,5 @@ /*
- (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- Author: Tor Krill tor@excito.com
- (C) Copyright 2016 Freescale Semiconductor, Inc.
What's with this copyright change here ?
It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c
copyright information in the new file?
There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?
Yes
According to git, it's not a new file:
b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c
so yes, it should retain all copyright info.
Ok, We will retain copyright info in v4.
And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.
drivers/usb/host/ was a good option, but we want to make it independent of host and gadget. So, whenever there is a specific requirement for freescale boards, it will use the same from board: freescale: common: usb
Else, another option is to have drivers/usb/common/fsl-dt-fixup.c. What do you say?
That is fine. Moving it to board code would make it problematic to convert to DM afterward, so I want to prevent that.
- SPDX-License-Identifier: GPL-2.0+
*/ @@ -17,164 +13,11 @@ #include <fsl_usb.h> #include <fdt_support.h>
-#include "ehci.h"
[...]
-- Best regards, Marek Vasut
Best Regards, Sriram
-- Best regards, Marek Vasut
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com --- board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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); @@ -64,32 +76,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) {
On 03/01/2016 08:03 AM, Sriram Dash wrote:
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com
board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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;
- return node_type;
The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.
+}
+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);
@@ -64,32 +76,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) {
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:57 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/01/2016 08:03 AM, Sriram Dash wrote:
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com
board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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;
- return node_type;
The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.
This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code duplication.
+}
+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);
@@ -64,32 +76,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) {
-- Best regards, Marek Vasut
On 03/03/2016 09:30 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:57 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/01/2016 08:03 AM, Sriram Dash wrote:
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com
board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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;
- return node_type;
The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.
This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code duplication.
I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.
+}
+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);
@@ -64,32 +76,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) {
-- Best regards, Marek Vasut
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, March 03, 2016 3:29 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/03/2016 09:30 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:57 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/01/2016 08:03 AM, Sriram Dash wrote:
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com
board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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;
- return node_type;
The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.
This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code
duplication.
I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.
I think git diff tool is creating confusion here, I have just moved function fdt_usb_get_node_type above fdt_fixup_usb_mode_phy_type which created all the diff and looked as if new function is added.
Please check below created patch where declaration is added to call fdt_usb_get_node_type in fdt_usb_get_mode_phy_type. We will be sending below in V4.
#################################################################
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..5553225 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,32 +18,21 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char *fdt_usb_get_node_type(void *blob, int start_offset, + int *node_offset); + 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; - } + 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,
#################################################################
+}
+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);
@@ -64,32 +76,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) {
-- Best regards, Marek Vasut
-- Best regards, Marek Vasut
On 03/03/2016 12:11 PM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, March 03, 2016 3:29 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/03/2016 09:30 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:57 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type
On 03/01/2016 08:03 AM, Sriram Dash wrote:
Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.
Signed-off-by: Ramneek Mehresh ramneek.mehresh@nxp.com Signed-off-by: Sriram Dash sriram.dash@nxp.com
board/freescale/common/usb.c | 72 ++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 43 deletions(-)
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,33 +18,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;
- return node_type;
The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.
This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code
duplication.
I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.
I think git diff tool is creating confusion here, I have just moved function fdt_usb_get_node_type above fdt_fixup_usb_mode_phy_type which created all the diff and looked as if new function is added.
Please check below created patch where declaration is added to call fdt_usb_get_node_type in fdt_usb_get_mode_phy_type. We will be sending below in V4.
#################################################################
diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index 85cb1bf..5553225 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,32 +18,21 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char *fdt_usb_get_node_type(void *blob, int start_offset,
int *node_offset);
Ah I see, I am wrong, sorry.
btw can you fix the fdt_usb_get_node_type() in another patch to return int ?
Thanks! Best regards, Marek Vasut
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 --- board/freescale/common/Makefile | 1 + board/freescale/common/usb.c | 30 +++++++++++++----------------- include/fdt_support.h | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index 62de45c..c644896 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -14,6 +14,7 @@ endif endif
obj-$(CONFIG_USB_EHCI_FSL) += usb.o +obj-$(CONFIG_USB_XHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index d815dc1..8e423be 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,29 +18,25 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph", + "fsl-usb2-dr", + "snps,dwc3"}; + 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; + int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]); + int i; + + for (i = 0; i < size ; i++) { + *node_offset = fdt_node_offset_by_compatible + (blob, start_offset, compat_usb_fsl[i]); + if (*node_offset >= 0) { + node_type = compat_usb_fsl[i]; + 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);
On 03/01/2016 08:03 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
Changelog ?
board/freescale/common/Makefile | 1 + board/freescale/common/usb.c | 30 +++++++++++++----------------- include/fdt_support.h | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index 62de45c..c644896 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -14,6 +14,7 @@ endif endif
obj-$(CONFIG_USB_EHCI_FSL) += usb.o +obj-$(CONFIG_USB_XHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index d815dc1..8e423be 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,29 +18,25 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
"fsl-usb2-dr","snps,dwc3"};
This is const char *foo[].
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;
- int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);
Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added. And it wastes space. No, instead, use a terminating entry in the array.
btw use checkpatch before your next submission.
- int i;
- for (i = 0; i < size ; i++) {
*node_offset = fdt_node_offset_by_compatible(blob, start_offset, compat_usb_fsl[i]);if (*node_offset >= 0) {node_type = compat_usb_fsl[i];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);
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:55 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller
On 03/01/2016 08:03 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
Changelog ?
Will include changelog for v2 and v3 in v4.
board/freescale/common/Makefile | 1 + board/freescale/common/usb.c | 30 +++++++++++++----------------- include/fdt_support.h | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index 62de45c..c644896 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -14,6 +14,7 @@ endif endif
obj-$(CONFIG_USB_EHCI_FSL) += usb.o +obj-$(CONFIG_USB_XHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index d815dc1..8e423be 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,29 +18,25 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
"fsl-usb2-dr","snps,dwc3"};This is const char *foo[].
Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.
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;
- int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);
Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added. And it wastes space. No, instead, use a terminating entry in the array.
Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?
btw use checkpatch before your next submission.
- int i;
- for (i = 0; i < size ; i++) {
*node_offset = fdt_node_offset_by_compatible(blob, start_offset, compat_usb_fsl[i]);if (*node_offset >= 0) {node_type = compat_usb_fsl[i];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);
-- Best regards, Marek Vasut
On 03/03/2016 09:32 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:55 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller
On 03/01/2016 08:03 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
Changelog ?
Will include changelog for v2 and v3 in v4.
board/freescale/common/Makefile | 1 + board/freescale/common/usb.c | 30 +++++++++++++----------------- include/fdt_support.h | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index 62de45c..c644896 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -14,6 +14,7 @@ endif endif
obj-$(CONFIG_USB_EHCI_FSL) += usb.o +obj-$(CONFIG_USB_XHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index d815dc1..8e423be 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,29 +18,25 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
"fsl-usb2-dr","snps,dwc3"};This is const char *foo[].
Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.
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;
- int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);
Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added. And it wastes space. No, instead, use a terminating entry in the array.
Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?
My opinion is to use a terminating NULL entry and iterate over the array until you reach it.
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, March 03, 2016 3:33 PM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller
On 03/03/2016 09:32 AM, Sriram Dash wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, March 02, 2016 3:55 AM To: Sriram Dash sriram.dash@nxp.com; u-boot@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 v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller
On 03/01/2016 08:03 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
Changelog ?
Will include changelog for v2 and v3 in v4.
board/freescale/common/Makefile | 1 + board/freescale/common/usb.c | 30 +++++++++++++----------------- include/fdt_support.h | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile index 62de45c..c644896 100644 --- a/board/freescale/common/Makefile +++ b/board/freescale/common/Makefile @@ -14,6 +14,7 @@ endif endif
obj-$(CONFIG_USB_EHCI_FSL) += usb.o +obj-$(CONFIG_USB_XHCI_FSL) += usb.o
ifdef MINIMAL # necessary to create built-in.o diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c index d815dc1..8e423be 100644 --- a/board/freescale/common/usb.c +++ b/board/freescale/common/usb.c @@ -18,29 +18,25 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
"fsl-usb2-dr","snps,dwc3"};This is const char *foo[].
Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.
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;
- int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);
Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer
element is added.
And it wastes space. No, instead, use a terminating entry in the array.
Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?
My opinion is to use a terminating NULL entry and iterate over the array until you reach it.
Accepted. Will do it in v4.
-- Best regards, Marek Vasut
participants (2)
-
Marek Vasut -
Sriram Dash