From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms
Date: Thu, 28 Jan 2016 12:34:26 +0100 [thread overview]
Message-ID: <201601281234.26669.marex@denx.de> (raw)
In-Reply-To: <DB5PR04MB1255A3BBC886163C3E5C2A64EADA0@DB5PR04MB1255.eurprd04.prod.outlook.com>
On Thursday, January 28, 2016 at 12:05:29 PM, Ramneek Mehresh wrote:
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at 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 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 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@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 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@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.
> > > >
> > > > 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?
I only see one implementation of fdt_fixup_dr_usb(blob, bd); being invoked
throughout this patch. Where are these "multiple" implementations ?
next prev parent reply other threads:[~2016-01-28 11:34 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-26 11:36 [U-Boot] [PATCH 1/2] fsl:usb: Make fsl usb device-tree fixup arch independent Ramneek Mehresh
2016-01-26 11:27 ` Marek Vasut
2016-01-26 11:36 ` [U-Boot] [PATCH 2/2] include:configs: Add usb device-tree fixup for all fsl platforms Ramneek Mehresh
2016-01-26 11:28 ` Marek Vasut
2016-01-27 4:14 ` Ramneek Mehresh
2016-01-27 4:27 ` Marek Vasut
2016-01-27 4:30 ` Ramneek Mehresh
2016-01-27 7:35 ` Marek Vasut
2016-01-27 8:26 ` Ramneek Mehresh
2016-01-27 8:27 ` Marek Vasut
2016-01-27 11:33 ` Ramneek Mehresh
2016-01-27 11:42 ` Marek Vasut
2016-01-28 11:05 ` Ramneek Mehresh
2016-01-28 11:34 ` Marek Vasut [this message]
2016-01-28 12:15 ` Ramneek Mehresh
2016-02-08 11:07 ` Ramneek Mehresh
2016-02-08 15:26 ` Marek Vasut
2016-02-24 4:44 ` [U-Boot] [PATCH v2 0/2] Make usb device-tree fixup independent of USB controller Sriram Dash
2016-02-24 4:44 ` [U-Boot] [PATCH v2 1/2] board:freescale:common: Move device-tree fixup framework to common file Sriram Dash
2016-02-25 17:58 ` Marek Vasut
2016-02-26 16:07 ` Sriram Dash
2016-02-26 16:21 ` Marek Vasut
2016-02-24 4:44 ` [U-Boot] [PATCH v2 2/2] board:freescale:usb: Add device-tree fixup support for xhci controller Sriram Dash
2016-02-25 18:00 ` Marek Vasut
2016-02-26 16:19 ` Sriram Dash
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201601281234.26669.marex@denx.de \
--to=marex@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.