[U-Boot] [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms

From: Ramneek Mehresh <ramneek.mehresh@nxp.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
Date: Wed, 27 Jan 2016 08:26:08 +0000	[thread overview]
Message-ID: <VI1PR04MB1264D2D249B2D7EFF0E2FDC2EAD90@VI1PR04MB1264.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <201601270835.29959.marex@denx.de>


> -----Original Message-----
> From: Marek Vasut [mailto:marex at 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 at 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 at 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 at 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 at denx.de]
> > > > > Sent: Tuesday, January 26, 2016 4:58 PM
> > > > > To: Ramneek Mehresh <ramneek.mehresh@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@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.