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

[Marek Vasut <marex@denx.de>]

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