[U-Boot] [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

Marek Vasut
2016-06-10T05:33:00Z

On 06/10/2016 07:20 AM, Sriram Dash wrote:
>> From: Marek Vasut [mailto:marex at denx.de]
>> On 06/10/2016 05:47 AM, Sriram Dash wrote:
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Thursday, June 09, 2016 7:01 PM
>>>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>>>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>>> Bhagat <rajesh.bhagat@nxp.com>
>>>> Subject: Re: [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device
>>>> tree fixup for fsl usb controllers
>>
>> Would you please fix your mailer to omit this useless header ?
>>
>> [...]
>>
>
> Will take care next time onwards.
>
>>>>> diff --git a/include/fsl_usb.h b/include/fsl_usb.h index
>>>>> 187e384..882a5f5 100644
>>>>> --- a/include/fsl_usb.h
>>>>> +++ b/include/fsl_usb.h
>>>>> @@ -85,6 +85,12 @@ struct ccsr_usb_phy { #define
>>>>> CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif
>>>>>
>>>>> +/* USB Controllers */
>>>>> +#define FSL_USB2_MPH "fsl-usb2-mph"
>>>>> +#define FSL_USB2_DR "fsl-usb2-dr"
>>>>> +#define USB2_CI "usb2-ci"
>>
>> What is this "usb2-ci" string ? Is this some DT property or DT compatible property or
>> just some random string?
>>
>
> Currently, for chipidea usb 2 controller, there is no generic
> DT compatible property.

Linux kernel documentation says there is "chipidea,usb2":

https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt

> Some names used are "qcom,ci-hdrc",
> "chipidea,usb2","fsl-usb2-dr","fsl-usb2-mph ". This string "usb2-ci"
> is not used currently for DT property.

The "chipidea,usb2" is generic one, all the others are either heritage
from the past or more precise specifiers of the IP block. Do not invent
yet another new compatible property in this case, it makes zero sense.

>>>>> +#define SNPS_DWC3 "snps,dwc3"
>>>>
>>>> Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?
>>>>
>>>
>>> Currently, the macro are only being used for dt fixup for fsl usb.
>>> However, in the near future, they might be needed for some other
>>> purpose, for example, board specific settings.
>>
>> If they will be used for other purpose, then they can be moved. They are local now,
>> so keep them local.
>>
>
> Ok. I will move the global macro to fsl-dt-fixup.c.
>
>>> So, i think i will stick with the global macro.
>>>
>>>>> /* USB Erratum Checking code */
>>>>> #ifdef CONFIG_PPC
>>>>> static inline bool has_dual_phy(void)
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Marek Vasut
>>
>>
>> --
>> Best regards,
>> Marek Vasut

--
Best regards,
Marek Vasut