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@denx.de] Sent: Tuesday, March 22, 2016 12:54 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 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 }