[RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level
This series move env_get_location from soc to board level. As suggested by Michael michael@amarulasolutions.com make no sense to define an unique way for multiple board. One board can boot from emmc and having env on spi flash etc.. Anyways, this function is kept in both imx8mn and imx8mp evk boards instead of being completely dropped. (as suggested by Andrey andrey.zhizhikin@leica-geosystems.com)
Tommaso Merciai (3): imx8m: drop env_get_location for imx8mn and imx8mp imx: imx8mn_evk: override env_get_location imx: imx8mp_evk: override env_get_location
arch/arm/mach-imx/imx8m/soc.c | 39 ------------------------- board/freescale/imx8mn_evk/imx8mn_evk.c | 35 ++++++++++++++++++++++ board/freescale/imx8mp_evk/imx8mp_evk.c | 34 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-)
This function defined for two architecture is not really generic and can generate problem when people add a new board.
Signed-off-by: Tommaso Merciai tomm.merciai@gmail.com --- arch/arm/mach-imx/imx8m/soc.c | 39 ----------------------------------- 1 file changed, 39 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 863508776d..f0030a557a 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -1313,45 +1313,6 @@ void do_error(struct pt_regs *pt_regs, unsigned int esr) #endif
#if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP) -enum env_location env_get_location(enum env_operation op, int prio) -{ - enum boot_device dev = get_boot_device(); - enum env_location env_loc = ENVL_UNKNOWN; - - if (prio) - return env_loc; - - switch (dev) { -#ifdef CONFIG_ENV_IS_IN_SPI_FLASH - case QSPI_BOOT: - env_loc = ENVL_SPI_FLASH; - break; -#endif -#ifdef CONFIG_ENV_IS_IN_NAND - case NAND_BOOT: - env_loc = ENVL_NAND; - break; -#endif -#ifdef CONFIG_ENV_IS_IN_MMC - case SD1_BOOT: - case SD2_BOOT: - case SD3_BOOT: - case MMC1_BOOT: - case MMC2_BOOT: - case MMC3_BOOT: - env_loc = ENVL_MMC; - break; -#endif - default: -#if defined(CONFIG_ENV_IS_NOWHERE) - env_loc = ENVL_NOWHERE; -#endif - break; - } - - return env_loc; -} - #ifndef ENV_IS_EMBEDDED long long env_get_offset(long long defautl_offset) {
Override env_get_location function at board level, previously dropped down from soc.c
References: - commit 63cd508ce696cb1ef0867d9ab6e93d36df339e0a
Signed-off-by: Tommaso Merciai tomm.merciai@gmail.com --- Changes since v1: - Fix code indentation using checkpatch as suggested by MBehún
board/freescale/imx8mn_evk/imx8mn_evk.c | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/board/freescale/imx8mn_evk/imx8mn_evk.c b/board/freescale/imx8mn_evk/imx8mn_evk.c index 9a0a0488bf..ec1ab202a6 100644 --- a/board/freescale/imx8mn_evk/imx8mn_evk.c +++ b/board/freescale/imx8mn_evk/imx8mn_evk.c @@ -5,11 +5,46 @@
#include <common.h> #include <env.h> +#include <env_internal.h> #include <init.h> +#include <asm/arch/sys_proto.h> +#include <asm/mach-imx/boot_mode.h> #include <asm/global_data.h>
DECLARE_GLOBAL_DATA_PTR;
+enum env_location env_get_location(enum env_operation op, int prio) +{ + enum boot_device dev = get_boot_device(); + enum env_location env_loc = ENVL_UNKNOWN; + + if (prio) + return env_loc; + + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && dev == QSPI_BOOT) { + env_loc = ENVL_SPI_FLASH; + } else if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND) && dev == NAND_BOOT) { + env_loc = ENVL_NAND; + } else if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) { + switch (dev) { + case SD1_BOOT: + case SD2_BOOT: + case SD3_BOOT: + case MMC1_BOOT: + case MMC2_BOOT: + case MMC3_BOOT: + env_loc = ENVL_MMC; + break; + default: + break; + } + } else if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) { + env_loc = ENVL_MMC; + } + + return env_loc; +} + int board_init(void) { return 0;
Override env_get_location function at board level, previously dropped down from soc.c
References: - commit 63cd508ce696cb1ef0867d9ab6e93d36df339e0a
Signed-off-by: Tommaso Merciai tomm.merciai@gmail.com --- Changes since v1: - Fix code indentation using checkpatch as suggested by MBehún
board/freescale/imx8mp_evk/imx8mp_evk.c | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c b/board/freescale/imx8mp_evk/imx8mp_evk.c index 62096c24fb..8548f606d2 100644 --- a/board/freescale/imx8mp_evk/imx8mp_evk.c +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c @@ -5,6 +5,7 @@
#include <common.h> #include <env.h> +#include <env_internal.h> #include <errno.h> #include <init.h> #include <miiphy.h> @@ -17,6 +18,7 @@ #include <asm/arch/clock.h> #include <asm/arch/sys_proto.h> #include <asm/mach-imx/gpio.h> +#include <asm/mach-imx/boot_mode.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -32,6 +34,38 @@ static iomux_v3_cfg_t const wdog_pads[] = { MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B | MUX_PAD_CTRL(WDOG_PAD_CTRL), };
+enum env_location env_get_location(enum env_operation op, int prio) +{ + enum boot_device dev = get_boot_device(); + enum env_location env_loc = ENVL_UNKNOWN; + + if (prio) + return env_loc; + + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && dev == QSPI_BOOT) { + env_loc = ENVL_SPI_FLASH; + } else if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND) && dev == NAND_BOOT) { + env_loc = ENVL_NAND; + } else if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) { + switch (dev) { + case SD1_BOOT: + case SD2_BOOT: + case SD3_BOOT: + case MMC1_BOOT: + case MMC2_BOOT: + case MMC3_BOOT: + env_loc = ENVL_MMC; + break; + default: + break; + } + } else if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) { + env_loc = ENVL_MMC; + } + + return env_loc; +} + int board_early_init_f(void) { struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
On 2021/12/1 4:17, Tommaso Merciai wrote:
This series move env_get_location from soc to board level. As suggested by Michael michael@amarulasolutions.com make no sense to define an unique way for multiple board. One board can boot from emmc and having env on spi flash etc.. Anyways, this function is kept in both imx8mn and imx8mp evk boards instead of being completely dropped. (as suggested by Andrey andrey.zhizhikin@leica-geosystems.com)
If there are other i.MX8MN/P boards already uses the function, move it to i.mx8mn/p_evk would break other boards. If i.MX8MN/P evk are the other users, it should be ok to move the board code.
Regards, Peng.
Tommaso Merciai (3): imx8m: drop env_get_location for imx8mn and imx8mp imx: imx8mn_evk: override env_get_location imx: imx8mp_evk: override env_get_location
arch/arm/mach-imx/imx8m/soc.c | 39 ------------------------- board/freescale/imx8mn_evk/imx8mn_evk.c | 35 ++++++++++++++++++++++ board/freescale/imx8mp_evk/imx8mp_evk.c | 34 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-)
On Sun, Dec 19, 2021 at 10:11:44AM +0800, Peng Fan (OSS) wrote:
On 2021/12/1 4:17, Tommaso Merciai wrote:
This series move env_get_location from soc to board level. As suggested by Michael michael@amarulasolutions.com make no sense to define an unique way for multiple board. One board can boot from emmc and having env on spi flash etc.. Anyways, this function is kept in both imx8mn and imx8mp evk boards instead of being completely dropped. (as suggested by Andrey andrey.zhizhikin@leica-geosystems.com)
If there are other i.MX8MN/P boards already uses the function, move it to i.mx8mn/p_evk would break other boards. If i.MX8MN/P evk are the other users, it should be ok to move the board code.
Hi Peng, Maybe declare it as __weak in soc.c and ovverride it a board level can be a valid solution? Let me know.
thanks. tommaso
Regards, Peng.
Tommaso Merciai (3): imx8m: drop env_get_location for imx8mn and imx8mp imx: imx8mn_evk: override env_get_location imx: imx8mp_evk: override env_get_location
arch/arm/mach-imx/imx8m/soc.c | 39 ------------------------- board/freescale/imx8mn_evk/imx8mn_evk.c | 35 ++++++++++++++++++++++ board/freescale/imx8mp_evk/imx8mp_evk.c | 34 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-)
On Thu, Dec 23, 2021 at 10:49:38PM +0100, Tommaso Merciai wrote:
On Sun, Dec 19, 2021 at 10:11:44AM +0800, Peng Fan (OSS) wrote:
On 2021/12/1 4:17, Tommaso Merciai wrote:
This series move env_get_location from soc to board level. As suggested by Michael michael@amarulasolutions.com make no sense to define an unique way for multiple board. One board can boot from emmc and having env on spi flash etc.. Anyways, this function is kept in both imx8mn and imx8mp evk boards instead of being completely dropped. (as suggested by Andrey andrey.zhizhikin@leica-geosystems.com)
If there are other i.MX8MN/P boards already uses the function, move it to i.mx8mn/p_evk would break other boards. If i.MX8MN/P evk are the other users, it should be ok to move the board code.
Hi Peng, Maybe declare it as __weak in soc.c and ovverride it a board level can be a valid solution? Let me know.
Hi Peng, Reviewing env_get_location is already declared as __weak, my bad. Then we have to add it in other boards that use imx8mp/imx8mn:
- board/phytec/phycore_imx8mp/phycore-imx8mp.c - board/beacon/imx8mn/imx8mn_beacon.c
I will try to involve the respective maintainers and resend it in v3.
Thanks, Tommaso
thanks. tommaso
Regards, Peng.
Tommaso Merciai (3): imx8m: drop env_get_location for imx8mn and imx8mp imx: imx8mn_evk: override env_get_location imx: imx8mp_evk: override env_get_location
arch/arm/mach-imx/imx8m/soc.c | 39 ------------------------- board/freescale/imx8mn_evk/imx8mn_evk.c | 35 ++++++++++++++++++++++ board/freescale/imx8mp_evk/imx8mp_evk.c | 34 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-)
participants (2)
-
Peng Fan (OSS) -
Tommaso Merciai