>-----Original Message-----
>From: Marek Vasut [mailto:ma...@denx.de]
>Sent: Wednesday, March 02, 2016 3:55 AM
>To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de
>Cc: york sun <york....@nxp.com>; Ramneek Mehresh
><ramneek.mehr...@nxp.com>; Rajesh Bhagat <rajesh.bha...@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.mehr...@nxp.com>
>> Signed-off-by: Sriram Dash <sriram.d...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
