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

Marek Vasut
2016-03-23T08:41:34Z

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