[RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

[RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

[Tommaso Merciai]

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(-)

-- 
2.25.1

[RFC PATCH v2 1/3] imx8m: drop env_get_location for imx8mn and imx8mp

[Tommaso Merciai]

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)
 {
-- 
2.25.1

[RFC PATCH v2 2/3] imx: imx8mn_evk: override env_get_location

[Tommaso Merciai]

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;
-- 
2.25.1

[RFC PATCH v2 3/3] imx: imx8mp_evk: override env_get_location

[Tommaso Merciai]

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;
-- 
2.25.1

Re: [RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

[Peng Fan (OSS)]

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(-)
> 

Re: [RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

[Tommaso Merciai]

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(-)
> > 

Re: [RFC PATCH v2 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

[Tommaso Merciai]

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(-)
> > >