[PATCH] board/freescale/vid : move platform specific definitions
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. */
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.
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 ?
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.
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
participants (3)
-
Tom Rini -
Wasim Khan -
Wasim Khan (OSS)