* [PATCH] board/freescale/vid : move platform specific definitions
@ 2021-09-21 14:24 Wasim Khan
2021-09-21 14:43 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Wasim Khan @ 2021-09-21 14:24 UTC (permalink / raw)
To: trini, priyanka.jain, v.sethi; +Cc: u-boot, Wasim Khan
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(-)
diff --git a/board/freescale/common/vid.h b/board/freescale/common/vid.h
index b34c080b4b..32e68fceb2 100644
--- a/board/freescale/common/vid.h
+++ b/board/freescale/common/vid.h
@@ -59,16 +59,6 @@
#define PMBUS_CMD_VOUT_COMMAND 0x21
#define PMBUS_CMD_PAGE_PLUS_WRITE 0x05
-#if defined(CONFIG_TARGET_LX2160AQDS) || defined(CONFIG_TARGET_LX2162AQDS) || \
-defined(CONFIG_TARGET_LX2160ARDB)
-/* Voltage monitor on channel 2*/
-#define I2C_VOL_MONITOR_BUS_V_OFFSET 0x2
-#define I2C_VOL_MONITOR_BUS_V_OVF 0x1
-#define I2C_VOL_MONITOR_BUS_V_SHIFT 3
-#define I2C_VOL_MONITOR_ADDR 0x63
-#define I2C_MUX_CH_VOL_MONITOR 0xA
-#endif
-
int adjust_vdd(ulong vdd_override);
u16 soc_get_fuse_vid(int vid_index);
diff --git a/include/configs/lx2160a_common.h b/include/configs/lx2160a_common.h
index 1ae7d37dd9..c2c4a8c7a1 100644
--- a/include/configs/lx2160a_common.h
+++ b/include/configs/lx2160a_common.h
@@ -86,6 +86,13 @@
#define CONFIG_SYS_LS_MC_DRAM_DPL_OFFSET 0x00F20000
#define CONFIG_SYS_LS_MC_BOOT_TIMEOUT_MS 5000
+/* Voltage monitor on channel 2*/
+#define I2C_VOL_MONITOR_BUS_V_OFFSET 0x2
+#define I2C_VOL_MONITOR_BUS_V_OVF 0x1
+#define I2C_VOL_MONITOR_BUS_V_SHIFT 3
+#define I2C_VOL_MONITOR_ADDR 0x63
+#define I2C_MUX_CH_VOL_MONITOR 0xA
+
/* Define phy_reset function to boot the MC based on mcinitcmd.
* This happens late enough to properly fixup u-boot env MAC addresses.
*/
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] board/freescale/vid : move platform specific definitions
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)
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2021-09-21 14:43 UTC (permalink / raw)
To: Wasim Khan; +Cc: priyanka.jain, v.sethi, u-boot, Wasim Khan
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] board/freescale/vid : move platform specific definitions
2021-09-21 14:43 ` Tom Rini
@ 2021-09-24 8:36 ` Wasim Khan (OSS)
2021-09-24 11:53 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Wasim Khan (OSS) @ 2021-09-24 8:36 UTC (permalink / raw)
To: Tom Rini, Wasim Khan (OSS); +Cc: Priyanka Jain, Varun Sethi, u-boot
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 ?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] board/freescale/vid : move platform specific definitions
2021-09-24 8:36 ` Wasim Khan (OSS)
@ 2021-09-24 11:53 ` Tom Rini
2021-09-24 20:27 ` Wasim Khan (OSS)
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2021-09-24 11:53 UTC (permalink / raw)
To: Wasim Khan (OSS); +Cc: Priyanka Jain, Varun Sethi, u-boot
[-- Attachment #1: Type: text/plain, Size: 3199 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] board/freescale/vid : move platform specific definitions
2021-09-24 11:53 ` Tom Rini
@ 2021-09-24 20:27 ` Wasim Khan (OSS)
0 siblings, 0 replies; 5+ messages in thread
From: Wasim Khan (OSS) @ 2021-09-24 20:27 UTC (permalink / raw)
To: Tom Rini, Wasim Khan (OSS); +Cc: Priyanka Jain, Varun Sethi, u-boot
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-24 20:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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.