On Monday, February 08, 2016 at 12:07:29 PM, Ramneek Mehresh wrote:
-----Original Message----- From: Ramneek Mehresh Sent: Thursday, January 28, 2016 5:45 PM To: 'Marek Vasut' marex@denx.de Cc: Ramneek Mehresh ramneek.mehresh@freescale.com; u- boot@lists.denx.de; Simon Glass sjg@chromium.org Subject: RE: [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, January 28, 2016 5:04 PM To: Ramneek Mehresh ramneek.mehresh@nxp.com Cc: Ramneek Mehresh ramneek.mehresh@freescale.com; u- boot@lists.denx.de; Simon Glass sjg@chromium.org Subject: Re: [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
On Thursday, January 28, 2016 at 12:05:29 PM, Ramneek Mehresh wrote:
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Wednesday, January 27, 2016 5:13 PM To: Ramneek Mehresh ramneek.mehresh@nxp.com Cc: Ramneek Mehresh ramneek.mehresh@freescale.com; u- boot@lists.denx.de; Simon Glass sjg@chromium.org Subject: Re: [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
On Wednesday, January 27, 2016 at 12:33:04 PM, Ramneek Mehresh
wrote:
> -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: Wednesday, January 27, 2016 1:57 PM > To: Ramneek Mehresh ramneek.mehresh@nxp.com > Cc: Ramneek Mehresh ramneek.mehresh@freescale.com; u- > boot@lists.denx.de; Simon Glass sjg@chromium.org > Subject: Re: [PATCH 2/2] include:configs: Add usb device-tree > fixup for all fsl platforms > > On Wednesday, January 27, 2016 at 09:26:08 AM, Ramneek
Mehresh
wrote:
> > > -----Original Message----- > > > From: Marek Vasut [mailto:marex@denx.de] > > > Sent: Wednesday, January 27, 2016 1:05 PM > > > To: Ramneek Mehresh ramneek.mehresh@nxp.com > > > Cc: Ramneek Mehresh ramneek.mehresh@freescale.com; u- > > > boot@lists.denx.de; Simon Glass sjg@chromium.org > > > Subject: Re: [PATCH 2/2] include:configs: Add usb > > > device-tree fixup for all fsl platforms > > > > > > On Wednesday, January 27, 2016 at 05:30:51 AM, Ramneek > > > Mehresh > > wrote: > > > > > -----Original Message----- > > > > > From: Marek Vasut [mailto:marex@denx.de] > > > > > Sent: Wednesday, January 27, 2016 9:57 AM > > > > > To: Ramneek Mehresh ramneek.mehresh@nxp.com > > > > > Cc: Ramneek Mehresh
ramneek.mehresh@freescale.com;
u-
> > > > > boot@lists.denx.de; Simon Glass sjg@chromium.org > > > > > Subject: Re: [PATCH 2/2] include:configs: Add usb > > > > > device-tree fixup for all fsl platforms > > > > > > > > > > On Wednesday, January 27, 2016 at 05:14:00 AM, Ramneek
Mehresh
> > > wrote: > > > > > > > -----Original Message----- > > > > > > > From: Marek Vasut [mailto:marex@denx.de] > > > > > > > Sent: Tuesday, January 26, 2016 4:58 PM > > > > > > > To: Ramneek Mehresh
> > > > > > > Cc: u-boot@lists.denx.de > > > > > > > Subject: Re: [PATCH 2/2] include:configs: Add usb > > > > > > > device-tree fixup for all fsl platforms > > > > > > > > > > > > > > On Tuesday, January 26, 2016 at 12:36:58 PM, > > > > > > > Ramneek
Mehresh
> > > wrote: > > > > > > > > Add usb device-tree fixup for all relevant fsl > > > > > > > > ppc and arm platforms > > > > > > > > > > > > > > > > Signed-off-by: Ramneek Mehresh > > > > > > > > > > ramneek.mehresh@freescale.com > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > board/freescale/b4860qds/b4860qds.c | 2 > > > > > > > > +- board/freescale/bsc9131rdb/bsc9131rdb.c > > > > > > > > | 2 ++ board/freescale/bsc9132qds/bsc9132qds.c > > > > > > > > | 2 ++ > > > > > > > > board/freescale/corenet_ds/corenet_ds.c | 4 > > > > > > > > ++++ board/freescale/ls2080aqds/ls2080aqds.c > > > > > > > > | 4 ++++ > > > > > > > > board/freescale/ls2080ardb/ls2080ardb.c | 4 > > > > > > > > ++++ board/freescale/mpc8308rdb/mpc8308rdb.c > > > > > > > > | 4
++++
> > > > > > > > board/freescale/mpc8315erdb/mpc8315erdb.c | 2 > > > > > > > > ++ board/freescale/mpc837xemds/mpc837xemds.c > > > > > > > > | 2
++
> > > > > > > > board/freescale/mpc837xerdb/mpc837xerdb.c | 2 > > > > > > > > ++ board/freescale/mpc8536ds/mpc8536ds.c > > > > > > > > | 2 +- board/freescale/p1010rdb/p1010rdb.c > > > > > > > > | 2 +- board/freescale/p1022ds/p1022ds.c > > > > > > > > | 2 +- > > > > > > > > board/freescale/p1023rdb/p1023rdb.c | 2 > > > > > > > > +- board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c > > > > > > > > | 2 +- board/freescale/p1_twr/p1_twr.c > > > > > > > > | 3 +++ board/freescale/p2041rdb/p2041rdb.c > > > > > > > > | 2 +- > > > > > > > > board/freescale/t102xqds/t102xqds.c | 2 > > > > > > > > +- board/freescale/t102xrdb/t102xrdb.c > > > > > > > > | 3 +++ board/freescale/t1040qds/t1040qds.c > > > > > > > > | 2 +- board/freescale/t104xrdb/t104xrdb.c > > > > > > > > | 2 +- > > > > > > > > board/freescale/t208xqds/t208xqds.c | 3 > > > > > > > > +++ board/freescale/t208xrdb/t208xrdb.c > > > > > > > > | 3 +++ board/freescale/t4qds/t4240emu.c > > > > > > > > | 3 +++ board/freescale/t4qds/t4240qds.c > > > > > > > > | 3 +++ > > > > > > > > board/freescale/t4rdb/t4240rdb.c | 3 > > > > > > > > +++ include/configs/B4860QDS.h > > > > > > > > | 1 + include/configs/BSC9131RDB.h > > > > > > > > | 1 + include/configs/BSC9132QDS.h > > > > > > > > | 3 ++- include/configs/MPC8308RDB.h > > > > > > > > | 3 +++ include/configs/MPC8315ERDB.h > > > > > > > > | 1 + include/configs/MPC837XEMDS.h > > > > > > > > | 3 ++- > > > > > > > > include/configs/MPC837XERDB.h | 1 > > > > > > > > + include/configs/MPC8536DS.h | > > > > > > > > 1 + include/configs/P1010RDB.h > > > > > > > > | 1 + include/configs/P1022DS.h > > > > > > > > | 1 + include/configs/P1023RDB.h > > > > > > > > | 1 + include/configs/P2041RDB.h > > > > > > > > | 1 + include/configs/T102xQDS.h > > > > > > > > | 1 + include/configs/T102xRDB.h > > > > > > > > | 1 + include/configs/T1040QDS.h > > > > > > > > | 1 + include/configs/T104xRDB.h > > > > > > > > | 1 + include/configs/T208xQDS.h > > > > > > > > | 1 + > > > > > > > > include/configs/T208xRDB.h | 1 > > > > > > > > + include/configs/T4240QDS.h | > > > > > > > > 1 + include/configs/corenet_ds.h > > > > > > > > | 1 + include/configs/ls2080aqds.h > > > > > > > > | 1 + include/configs/ls2080ardb.h > > > > > > > > | 1 + include/configs/p1_p2_rdb_pc.h > > > > > > > > | 1 + include/configs/p1_twr.h > > > > > > > > | 1 + 50 files changed, 85 > > > > > > > > insertions(+), 12 > > > > > > > > > > > > > > > > deletions(-) > > > > > > > > > > > > > > Each such new macro must be documented. What is > > > > > > > the point
of
> > > > > > > this bulk rename anyway? > > > > > > > > > > > > Yes, I'll document the new MACRO defined for usb > > > > > > device-tree > > fixup. > > > > > > > However, this is not bulk rename. I just modified > > > > > > all these files to include usb device-tree fixup for > > > > > > all fsl ppc platforms. Most of these platforms were > > > > > > already using this mechanism (some via different > > > > > > ways), > > > > > > > > > > but > > > > > > > > > > > now its consistent across them. > > > > > > > > > > Wouldn't it make more sense to make this generic then > > > > > instead of patching zillion files ? > > > > > > > > The patch set is to make this support generic, but I do > > > > need to make these platforms use this generic support in > > > > consistent way via single macro inclusion in their > > > > configs... > > > > > > You can also just modify the function itself instead of > > > million board files. Adding ifdef-endif combo all around > > > the place is just not gonna work, sorry. > > > > > > Also, the macro should probably contain _OF_ instead of > > > DEVTREE ... > > > > Ok, in this case, I'll use the already defined macros used > > by these platforms. However, this approach will not make > > sure that all these platforms are using this feature in a > > consistent/similar
way.
> Why not ?
All the platforms files are calling fdt_fixup_dr_usb() under following conditions: 1. no macro usage, direct call 2. Using CONFIG_HAS_FSL_DR_USB macro (for socs having DR controller) 2. Using CONFIG_HAS_FSL_MPH_USB (for socs having MPH controller)
Using CONFIG_HAS_FSL_XHCI_USB (for socs having XHCI controller)
This is why I wanted a single macro (CONFIG_USB_DEVTREE_FIXUP) for invocation of fdt_fixup_dr_usb() from all platforms. One macro --> one feature
That'd be fine, but you're adding ifdefs all around the place and that is not fine. You can solve this without ifdefs too, for example by having a __weak empty version of the function where
applicable.
Ok, so you're suggesting me to have multiple definitions of this function...the function Itself being __weak...one for ehci and another for xhci. In this case, we'll be having problem with boards of an soc having both ehci and xhci controller...then how do we handle that situation?
I only see one implementation of fdt_fixup_dr_usb(blob, bd); being invoked throughout this patch. Where are these "multiple"
implementations ? The only implementation right now was in drivers/usb/host/ehci-fsl.c which I moved to boards/freescale/common/usb.c This was done so as not to have another definition of this function in drivers/usb/host/xhci-fsl.c.
Hi Marek...a kind reminder for the above discussion...please let me know if you're ok with the above patch or shall I resend the patch with #defines removed. Please suggest.
The later, patch which does not add billion #defines .