| Message ID | 20211225202554.47853-2-tomm.merciai@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | imx8m: move env_get_location for imx8mn and imx8mp at board level | expand |
Hello Tommaso, Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) would it not make sense to move also env_get_offset() to board level? Regards, Teresa > {
Hi Teresa On Tue, Jan 4, 2022 at 12:04 PM Teresa Remmet <T.Remmet@phytec.de> wrote: > > Hello Tommaso, > > Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) > > would it not make sense to move also env_get_offset() to board level? > Drop it in another patch. This is not reference in uboot Michael > Regards, > Teresa > > > > { > -- > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber | > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE > 149059855
Hello Michael, Am Dienstag, dem 04.01.2022 um 12:06 +0100 schrieb Michael Nazzareno Trimarchi: > Hi Teresa > > On Tue, Jan 4, 2022 at 12:04 PM Teresa Remmet <T.Remmet@phytec.de> > wrote: > > Hello Tommaso, > > > > Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) > > > > would it not make sense to move also env_get_offset() to board > > level? > > > > Drop it in another patch. This is not reference in uboot Ah thanks! Teresa > > Michael > > > Regards, > > Teresa > > > > > > > { > > -- > > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, > > Germany > > > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber > > | > > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. > > 266500608, DE > > 149059855 > >
On Tue, Jan 04, 2022 at 11:04:10AM +0000, Teresa Remmet wrote: > Hello Tommaso, > > Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) > > would it not make sense to move also env_get_offset() to board level? Hi Teresa, I think is better to put this function at board level. In this way others boards that use i.MX8MN/i.MX8MM SOC can customize env_get_location function. For example: maybe one user want store U-Boot env on a device other than the boot device. Tommaso > > Regards, > Teresa > > > > { > -- > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber | > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE > 149059855
Hello Tommaso, Am Samstag, dem 08.01.2022 um 20:08 +0100 schrieb Tommaso Merciai: > On Tue, Jan 04, 2022 at 11:04:10AM +0000, Teresa Remmet wrote: > > Hello Tommaso, > > > > Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) > > > > would it not make sense to move also env_get_offset() to board > > level? > > Hi Teresa, > I think is better to put this function at board level. In this way > others boards that use i.MX8MN/i.MX8MM SOC can customize > env_get_location > function. For example: maybe one user want store U-Boot env on a > device > other than the boot device. Michael send a patch to remove the function. Which I missed. See: https://lore.kernel.org/u-boot/20211117143456.34441-1-michael@amarulasolutions.com/ So everything is fine then. Thanks, Teresa > > Tommaso > > Regards, > > Teresa > > > > > > > { > > -- > > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, > > Germany > > > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber > > | > > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. > > 266500608, DE > > 149059855
On Tue, Jan 11, 2022 at 09:35:54AM +0000, Teresa Remmet wrote: > Hello Tommaso, > > Am Samstag, dem 08.01.2022 um 20:08 +0100 schrieb Tommaso Merciai: > > On Tue, Jan 04, 2022 at 11:04:10AM +0000, Teresa Remmet wrote: > > > Hello Tommaso, > > > > > > Am Samstag, dem 25.12.2021 um 21:25 +0100 schrieb 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) > > > > > > would it not make sense to move also env_get_offset() to board > > > level? > > > > Hi Teresa, > > I think is better to put this function at board level. In this way > > others boards that use i.MX8MN/i.MX8MM SOC can customize > > env_get_location > > function. For example: maybe one user want store U-Boot env on a > > device > > other than the boot device. > > Michael send a patch to remove the function. Which I missed. See: > https://lore.kernel.org/u-boot/20211117143456.34441-1-michael@amarulasolutions.com/ > > So everything is fine then. > > Thanks, > Teresa Hi Teresa, Perfect. Thanks, Tommaso > > > > > Tommaso > > > Regards, > > > Teresa > > > > > > > > > > { > > > -- > > > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, > > > Germany > > > > > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber > > > | > > > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. > > > 266500608, DE > > > 149059855 > -- > PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany > > Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber | > Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE > 149059855
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) {
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(-)