[U-Boot] [PATCH v2] include: usb: Rename USB controller base address mapping

[york sun]

On 06/16/2016 08:58 PM, Rajesh Bhagat wrote:
>
>
>> -----Original Message-----
>> From: york sun
>> Sent: Friday, June 17, 2016 9:07 AM
>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; u-boot at lists.denx.de
>> Cc: Qianyu.Gong at freescale.com; Mingkai.hu at freescale.com;
>> prabhakar at freescale.com
>> Subject: Re: [PATCH v2] include: usb: Rename USB controller base address mapping
>>
>> On 06/16/2016 08:28 PM, Rajesh Bhagat wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: york sun
>>>> Sent: Thursday, June 16, 2016 9:13 PM
>>>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; u-boot at lists.denx.de
>>>> Cc: Qianyu.Gong at freescale.com; Mingkai.hu at freescale.com;
>>>> prabhakar at freescale.com
>>>> Subject: Re: [PATCH v2] include: usb: Rename USB controller base
>>>> address mapping
>>>>
>>>> On 06/07/2016 06:29 AM, Rajesh Bhagat wrote:
>>>>> Remove Soc specific defines and use generic chasis specific defines
>>>>> for USB controller base address mapping.
>>>>>
>>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>>>>> ---
>>>>> Changes in v2:
>>>>>     - Rebased patch for latest u-boot
>>>>
>>>>
>>>> <snip>
>>>>
>>>>> diff --git a/include/linux/usb/xhci-fsl.h
>>>>> b/include/linux/usb/xhci-fsl.h index 253eddf..199f366 100644
>>>>> --- a/include/linux/usb/xhci-fsl.h
>>>>> +++ b/include/linux/usb/xhci-fsl.h
>>>>> @@ -51,22 +51,18 @@ struct fsl_xhci {
>>>>>     	struct dwc3 *dwc3_reg;
>>>>>     };
>>>>>
>>>>> -#if defined(CONFIG_LS102XA)
>>>>> -#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
>>>>> CONFIG_SYS_LS102XA_XHCI_USB1_ADDR
>>>>> +#if defined(CONFIG_LS102XA) || defined(CONFIG_LS1012A) #define
>>>>> +CONFIG_SYS_FSL_XHCI_USB1_ADDR CONFIG_SYS_XHCI_USB1_ADDR
>>>>>     #define CONFIG_SYS_FSL_XHCI_USB2_ADDR 0
>>>>>     #define CONFIG_SYS_FSL_XHCI_USB3_ADDR 0
>>>>>     #elif defined(CONFIG_LS2080A)
>>>>> -#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
>>>>> CONFIG_SYS_LS2080A_XHCI_USB1_ADDR -#define
>>>>> CONFIG_SYS_FSL_XHCI_USB2_ADDR
>> CONFIG_SYS_LS2080A_XHCI_USB2_ADDR
>>>>> -#define CONFIG_SYS_FSL_XHCI_USB3_ADDR 0 -#elif
>>>>> defined(CONFIG_LS1043A) || defined(CONFIG_LS1012A) -#define
>>>>> CONFIG_SYS_FSL_XHCI_USB1_ADDR
>> CONFIG_SYS_LS1043A_XHCI_USB1_ADDR
>>>>> -#define CONFIG_SYS_FSL_XHCI_USB2_ADDR
>>>>> CONFIG_SYS_LS1043A_XHCI_USB2_ADDR -#define
>>>>> CONFIG_SYS_FSL_XHCI_USB3_ADDR
>> CONFIG_SYS_LS1043A_XHCI_USB3_ADDR -
>>>> #elif
>>>>> defined(CONFIG_LS1012A) -#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
>>>>> CONFIG_SYS_LS1043A_XHCI_USB1_ADDR -#define
>>>>> CONFIG_SYS_FSL_XHCI_USB2_ADDR 0
>>>>> +#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
>> CONFIG_SYS_XHCI_USB1_ADDR
>>>>> +#define CONFIG_SYS_FSL_XHCI_USB2_ADDR
>> CONFIG_SYS_XHCI_USB2_ADDR
>>>>>     #define CONFIG_SYS_FSL_XHCI_USB3_ADDR 0
>>>>> +#elif defined(CONFIG_LS1043A)
>>>>> +#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
>> CONFIG_SYS_XHCI_USB1_ADDR
>>>>> +#define CONFIG_SYS_FSL_XHCI_USB2_ADDR
>> CONFIG_SYS_XHCI_USB2_ADDR
>>>>> +#define CONFIG_SYS_FSL_XHCI_USB3_ADDR
>> CONFIG_SYS_XHCI_USB3_ADDR
>>>>>     #endif
>>>
>>> Hello York,
>>>
>>>>
>>>> Do you plan to consolidate the above section?
>>>>
>>>
>>> Do you mean making the macro defines common as below?
>>>
>>> #define CONFIG_SYS_FSL_XHCI_USB1_ADDR 0 #define
>>> CONFIG_SYS_FSL_XHCI_USB2_ADDR 0 #define
>> CONFIG_SYS_FSL_XHCI_USB3_ADDR
>>> 0
>>>
>>> #if defined(CONFIG_LS102XA) || defined(CONFIG_LS1012A) ||
>> defined(CONFIG_LS2080A)
>>> 	|| defined(CONFIG_LS2085A) || defined(CONFIG_LS1043A) #undef
>>> CONFIG_SYS_FSL_XHCI_USB1_ADDR #define
>> CONFIG_SYS_FSL_XHCI_USB1_ADDR
>>> CONFIG_SYS_XHCI_USB1_ADDR #endif
>>>
>>> #if defined(CONFIG_LS2080A) || defined(CONFIG_LS2085A) ||
>>> defined(CONFIG_LS1043A) #undef CONFIG_SYS_FSL_XHCI_USB2_ADDR #define
>>> CONFIG_SYS_FSL_XHCI_USB2_ADDR CONFIG_SYS_XHCI_USB2_ADDR #endif
>>>
>>> #if defined(CONFIG_LS1043A)
>>> #undef CONFIG_SYS_FSL_XHCI_USB3_ADDR
>>> #define CONFIG_SYS_FSL_XHCI_USB3_ADDR CONFIG_SYS_XHCI_USB3_ADDR
>> #endif
>>>
>
> Hello York,
>
>>
>> No. I mean to replace this section as
>>
>> #ifndef CONFIG_SYS_FSL_XHCI_USB1_ADDR
>> #define CONFIG_SYS_FSL_XHCI_USB1_ADDR 0
>> #endif
>> #ifndef CONFIG_SYS_FSL_XHCI_USB2_ADD
>> #define CONFIG_SYS_FSL_XHCI_USB2_ADD 0
>> #endif
>> #ifndef CONFIG_SYS_FSL_XHCI_USB3_ADD
>> #define CONFIG_SYS_FSL_XHCI_USB3_ADD 0
>> #endif
>>
>> Then you don't need to check SoC macros at all.
>>
>
> Actually, we are trying to re-use macros defined in chasis specific files for other platform
> of same chasis i.e. LS1012A. For example,
>
> File: arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>
> Original:
> #define CONFIG_SYS_LS1043A_XHCI_USB1_ADDR       (CONFIG_SYS_IMMR + 0x01f00000)
> #define CONFIG_SYS_LS1043A_XHCI_USB2_ADDR       (CONFIG_SYS_IMMR + 0x02000000)
> #define CONFIG_SYS_LS1043A_XHCI_USB3_ADDR       (CONFIG_SYS_IMMR + 0x02100000)
>
> Proposed (Made macros common):
> #define CONFIG_SYS_XHCI_USB1_ADDR               (CONFIG_SYS_IMMR + 0x01f00000)
> #define CONFIG_SYS_XHCI_USB2_ADDR               (CONFIG_SYS_IMMR + 0x02000000)
> #define CONFIG_SYS_XHCI_USB3_ADDR               (CONFIG_SYS_IMMR + 0x02100000)
>
> Now, LS1012A has same address as chasis is same but number of USB controllers in only one.
> Board specific decision for controller count is take in file include/linux/usb/xhci-fsl.h
> in above code.
>
> Hence, if I take implementation suggested by you I need to use Soc specific flags in file
> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h which I was trying to avoid.
>

OK. You need to use ifdef somewhere. You can keep it your way.

York