Hi Tom, > -----Original Message----- > From: U-Boot <[email protected]> On Behalf Of Tom Rini > Sent: Friday, September 24, 2021 5:24 PM > To: Wasim Khan (OSS) <[email protected]> > Cc: Priyanka Jain <[email protected]>; Varun Sethi <[email protected]>; u- > [email protected] > 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 <[email protected]> > > > Sent: Tuesday, September 21, 2021 8:14 PM > > > To: Wasim Khan (OSS) <[email protected]> > > > Cc: Priyanka Jain <[email protected]>; Varun Sethi > > > <[email protected]>; u- [email protected]; Wasim Khan > > > <[email protected]> > > > 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 <[email protected]> > > > > > > > > VID is a common driver. Move platform specific definitions to > > > > platform specific header files > > > > > > > > Signed-off-by: Wasim Khan <[email protected]> > > > > --- > > > > 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
