From: "Wasim Khan (OSS)" <wasim.khan@oss.nxp.com>
To: Tom Rini <trini@konsulko.com>,
"Wasim Khan (OSS)" <wasim.khan@oss.nxp.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>,
Varun Sethi <V.Sethi@nxp.com>,
"u-boot@lists.denx.de" <u-boot@lists.denx.de>
Subject: RE: [PATCH] board/freescale/vid : move platform specific definitions
Date: Fri, 24 Sep 2021 20:27:43 +0000 [thread overview]
Message-ID: <DU2PR04MB8726D6105F63C9CED279E47190A49@DU2PR04MB8726.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20210924115336.GA31748@bill-the-cat>
Hi Tom,
> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> Sent: Friday, September 24, 2021 5:24 PM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; u-
> boot@lists.denx.de
> Subject: Re: [PATCH] board/freescale/vid : move platform specific definitions
>
> On Fri, Sep 24, 2021 at 08:36:45AM +0000, Wasim Khan (OSS) wrote:
> > Hi Tom,
> >
> > > -----Original Message-----
> > > From: Tom Rini <trini@konsulko.com>
> > > Sent: Tuesday, September 21, 2021 8:14 PM
> > > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > > Cc: Priyanka Jain <priyanka.jain@nxp.com>; Varun Sethi
> > > <V.Sethi@nxp.com>; u- boot@lists.denx.de; Wasim Khan
> > > <wasim.khan@nxp.com>
> > > Subject: Re: [PATCH] board/freescale/vid : move platform specific
> > > definitions
> > >
> > > On Tue, Sep 21, 2021 at 04:24:57PM +0200, Wasim Khan wrote:
> > >
> > > > From: Wasim Khan <wasim.khan@nxp.com>
> > > >
> > > > VID is a common driver. Move platform specific definitions to
> > > > platform specific header files
> > > >
> > > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > > ---
> > > > board/freescale/common/vid.h | 10 ----------
> > > > include/configs/lx2160a_common.h | 7 +++++++
> > > > 2 files changed, 7 insertions(+), 10 deletions(-)
> > >
> > > NAK. Things need to move out of include/configs/ and not in to
> > > them, please find another common header file to use.
> > >
> > > --
> > > Tom
> >
> > Thank you so much for review.
> > Header files 'include/configs/' are auto picked for platform we are using.
> > I find it useful especially for common drivers like VID to auto pick required
> values for underneath platform from
> 'include/configs/<CONFIG_SYS_CONFIG_NAME>.h' and we don't need changes
> in common driver.
> >
> > (arch/Kconfig)
> > config SYS_CONFIG_NAME
> > string
> > help
> > This option should contain the base name of board header file.
> > The header file include/configs/<CONFIG_SYS_CONFIG_NAME>.h
> > should be included from include/config.h.
> >
> >
> > (Same is recommended for add/remove boards: doc/README.kconfig)
> > Define CONFIG_SYS_CONFIG_NAME="target" to include
> > include/configs/<target>.h
> >
> >
> > Currently all NXP platforms (except LX2 series) are using
> CONFIG_SYS_CONFIG_NAME to include platform specific header file for VID
> driver. I extended the support for LX2 and because the changes are common for
> lx2160ardb, lx2160aqds and lx2162aqds , I added them to lx2160a_common.h.
> >
> > Do you want me to move changes from ' include/configs/lx2160a_common.h'
> to ' include/configs/lx2160ardb.h', ' include/configs/lx2160aqds.h' and '
> include/configs/lx2162aqds.h' ?
> >
> > Or you want me to avoid adding anything to
> 'include/configs/<CONFIG_SYS_CONFIG_NAME>.h ' files ? Is there any reason to
> do so ?
>
> I want you to find a place outside of include/config/ for these values to reside,
> yes. The long term goal of moving everything to Kconfig means that we will not
> have include/config/ headers anymore. Further, values like this which are not
> actually user-configurable should neither be CONFIG-prefixed (which these are
> not, good!) nor reside in include/config/ at all. There should be some
> appropriate header perhaps under arch/arm/include/asm/arch-fsl-layerscape/
> to put these values.
> And if they need to be shared a bit wider, a fsl-layerscape-common or similar
> directory could be used (similar to arch/arm/include/asm/ti-common/). Does
> this make sense? Thanks.
>
> --
> Tom
Thanks for explaining it to me.
Because VID driver is applicable for arm/x86 platforms, I need to check what would be best place for the header file , but I understood your point.
Thanks,
Wasim
prev parent reply other threads:[~2021-09-24 20:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 14:24 [PATCH] board/freescale/vid : move platform specific definitions Wasim Khan
2021-09-21 14:43 ` Tom Rini
2021-09-24 8:36 ` Wasim Khan (OSS)
2021-09-24 11:53 ` Tom Rini
2021-09-24 20:27 ` Wasim Khan (OSS) [this message]
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=DU2PR04MB8726D6105F63C9CED279E47190A49@DU2PR04MB8726.eurprd04.prod.outlook.com \
--to=wasim.khan@oss.nxp.com \
--cc=V.Sethi@nxp.com \
--cc=priyanka.jain@nxp.com \
--cc=trini@konsulko.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).