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

[Marek Vasut]

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 at nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun at nxp.com>; Ramneek Mehresh
>> <ramneek.mehresh at nxp.com>; Rajesh Bhagat <rajesh.bhagat at 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 at nxp.com>
>>> Signed-off-by: Sriram Dash <sriram.dash at 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