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

[Rajesh Bhagat]

> -----Original Message-----
> From: york sun
> Sent: Monday, June 27, 2016 8:58 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/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.
> >	

Hello York,

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

Are you planning to pick this?

> 
> York

Best Regards,
Rajesh Bhagat