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

[Marek Vasut]

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 ?