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

Sriram Dash
2016-03-23T08:30:56Z

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