[PATCH v3 2/2] armv8: lx2162aqds: Add support for LX2162AQDS platform

[Tom Rini]

On Wed, Sep 09, 2020 at 06:42:10AM +0000, Meenakshi Aggarwal wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini <trini at konsulko.com>
> > Sent: Monday, September 7, 2020 6:43 PM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>
> > Cc: u-boot at lists.denx.de; Priyanka Jain <priyanka.jain at nxp.com>; Varun Sethi
> > <V.Sethi at nxp.com>; Ioana Ciornei <ioana.ciornei at nxp.com>; Qiang Zhao
> > <qiang.zhao at nxp.com>; Hui Song <hui.song_1 at nxp.com>; Manish Tomar
> > <manish.tomar at nxp.com>; Vikas Singh <vikas.singh at nxp.com>
> > Subject: Re: [PATCH v3 2/2] armv8: lx2162aqds: Add support for LX2162AQDS
> > platform
> > 
> > On Mon, Sep 07, 2020 at 03:42:07PM +0530, meenakshi.aggarwal at nxp.com
> > wrote:
> > 
> > > From: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>
> > >
> > > This patch add base support for LX2162AQDS board.
> > > LX2162AQDS board supports LX2162A family SoCs.
> > > This patch add basic support of platform.
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei at nxp.com>
> > > Signed-off-by: Zhao Qiang <qiang.zhao at nxp.com>
> > > Signed-off-by: hui.song <hui.song_1 at nxp.com>
> > > Signed-off-by: Manish Tomar <manish.tomar at nxp.com>
> > > Signed-off-by: Vikas Singh <vikas.singh at nxp.com>
> > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>
> > > ---
> > >  arch/arm/Kconfig                               |  12 +
> > >  arch/arm/dts/Makefile                          |   6 +-
> > >  arch/arm/dts/fsl-lx2160a-qds.dts               |   3 -
> > >  arch/arm/dts/fsl-lx2160a-qds.dtsi              |  22 +-
> > >  arch/arm/dts/fsl-lx2162a-qds-17-x.dts          |  17 +
> > >  arch/arm/dts/fsl-lx2162a-qds-18-x.dts          |  17 +
> > >  arch/arm/dts/fsl-lx2162a-qds-20-x.dts          |  17 +
> > >  arch/arm/dts/fsl-lx2162a-qds-sd1-17.dtsi       |  58 ++
> > >  arch/arm/dts/fsl-lx2162a-qds-sd1-18.dtsi       |  61 ++
> > >  arch/arm/dts/fsl-lx2162a-qds-sd1-20.dtsi       |  26 +
> > >  arch/arm/dts/fsl-lx2162a-qds.dts               |  34 +
> > 
> > Most of these new dts* files are not added to MAINTAINERS entries and it
> > would be good to do so.
> [Meenakshi Aggarwal] Will add in next version
> > 
> > >  board/freescale/common/vid.c                   |   3 +-
> > >  board/freescale/lx2160a/Kconfig                |  16 +
> > >  board/freescale/lx2160a/MAINTAINERS            |  10 +
> > >  board/freescale/lx2160a/Makefile               |   1 +
> > >  board/freescale/lx2160a/README                 | 132 ++++
> > 
> > This README also should get updated to rST and moved.
> [Meenakshi Aggarwal] OK, will plan it

Thanks.

> > [snip]
> > > +/*
> > > + * Need to override existing (lx2160a) with lx2162aqds so
> > > +set_board_info will
> > > + * use proper prefix when creating full board_name (SYS_BOARD + type)
> > > +*/ #undef CONFIG_SYS_BOARD
> > > +#define CONFIG_SYS_BOARD                "lx2162aqds"
> > 
> > SYS_BOARD is under Kconfig, please just fix it there.
> [Meenakshi Aggarwal] LX2162 uses same die as of LX2160 so we are re-using LX2160 code. I have added SYS_BOARD (=lx2160a) in Kconfig but I will have to reassign(=lx2162a) it here 
> else in environment variable it will show as lx2160.

It needs to be handled within Kconfig.  That may mean you need to change
the board/.../Kconfig file to take into account what the target is.  You
cannot do this here.

> > > +#undef CONFIG_SYS_NXP_SRDS_3
> > > +
> > > +/* Qixis */
> > > +#define QIXIS_XMAP_MASK			0x07
> > 
> > This is the start of adding tons of non-CONFIG information to the config header.
> > Don't do this, it's going to make full migration to Kconfig harder.  Please audit
> > the whole file and anything that's not CONFIG_xxx needs to be somewhere else
> > really.  Thanks.
> [Meenakshi Aggarwal] Will prefer to do this in a separate patch as it will impact all three LX2 based boards, because all use similar macros.

I'm sorry I didn't catch this (and also all of the headers being used
globally via the config.h file, but that's another topic) earlier.  But
I really want to see this addressed before we make the problem bigger.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200909/e3a1d98f/attachment.sig>