[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:43:25Z

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 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++;
> }

Or even use for(i = 0; compat_usb_fsl[i]; i++) { do something }

--
Best regards,
Marek Vasut