[U-Boot] [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
Ramneek Mehresh
ramneek.mehresh at nxp.com
Thu Jan 28 12:05:29 CET 2016
> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Wednesday, January 27, 2016 5:13 PM
> To: Ramneek Mehresh <ramneek.mehresh at nxp.com>
> Cc: Ramneek Mehresh <ramneek.mehresh at freescale.com>; u-
> boot at lists.denx.de; Simon Glass <sjg at 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 at denx.de]
> > > Sent: Wednesday, January 27, 2016 1:57 PM
> > > To: Ramneek Mehresh <ramneek.mehresh at nxp.com>
> > > Cc: Ramneek Mehresh <ramneek.mehresh at freescale.com>; u-
> > > boot at lists.denx.de; Simon Glass <sjg at 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 at denx.de]
> > > > > Sent: Wednesday, January 27, 2016 1:05 PM
> > > > > To: Ramneek Mehresh <ramneek.mehresh at nxp.com>
> > > > > Cc: Ramneek Mehresh <ramneek.mehresh at freescale.com>; u-
> > > > > boot at lists.denx.de; Simon Glass <sjg at 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 at denx.de]
> > > > > > > Sent: Wednesday, January 27, 2016 9:57 AM
> > > > > > > To: Ramneek Mehresh <ramneek.mehresh at nxp.com>
> > > > > > > Cc: Ramneek Mehresh <ramneek.mehresh at freescale.com>; u-
> > > > > > > boot at lists.denx.de; Simon Glass <sjg at 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 at denx.de]
> > > > > > > > > Sent: Tuesday, January 26, 2016 4:58 PM
> > > > > > > > > To: Ramneek Mehresh <ramneek.mehresh at freescale.com>
> > > > > > > > > Cc: u-boot at 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 at 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)
> > 3. 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?
More information about the U-Boot
mailing list