| Message ID | 20220111124607.863952-5-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | e42dad4168fe7acae810f759078a8cdfe2cd9086 |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | sunxi: Fix U-Boot proper SPI operation | expand |
On Tue, Jan 11, 2022 at 12:46:04PM +0000, Andre Przywara wrote: > Currently we only support to load the environment from raw MMC or FAT > locations on Allwinner boards. With the advent of SPI flash we probably > also want to support using the environment there, so we need to become > a bit more flexible. > > Change the environment priority function to take the boot source into > account. When booted from eMMC or SD card, we use FAT or MMC, if > configured, as before. > If we are booted from SPI flash, we try to use the environment from > there, if possible. The same is true for NAND flash booting, although > this is somewhat theoretical right now (as untested). > > This way we can use the same image for SD and SPI flash booting, which > allows us to simply copy a booted image from SD card to the SPI flash, > for instance. Unfortunately after a git-bisect I can confirm this patch breaks booting in FEL mode whenever the environment is set to CONFIG_ENV_IS_NOWHERE. Tested and reproducible on an NTC CHIP or a Source Parts Popcorn (both Allwinner R8 chips). The curious thing though is that I could never see env_get_location called, but whenever I either set the environment to something like FAT or comment out this change (which I believe causes the non-board specific version of env_get_location to be used) it would boot again. So while I can confirm this patch breaks booting FEL when the env is set to nowhere, I was not able to figure out why that is. Thank you. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 43 insertions(+), 8 deletions(-) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 2790a0f9e8..2472343d00 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -170,21 +170,56 @@ void i2c_init_board(void) > #endif > } > > -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT) > +/* > + * Try to use the environment from the boot source first. > + * For MMC, this means a FAT partition on the boot device (SD or eMMC). > + * If the raw MMC environment is also enabled, this is tried next. > + * SPI flash falls back to FAT (on SD card). > + */ > enum env_location env_get_location(enum env_operation op, int prio) > { > - switch (prio) { > - case 0: > - return ENVL_FAT; > + enum env_location boot_loc = ENVL_FAT; > > - case 1: > - return ENVL_MMC; > + gd->env_load_prio = prio; > > + switch (sunxi_get_boot_device()) { > + case BOOT_DEVICE_MMC1: > + case BOOT_DEVICE_MMC2: > + boot_loc = ENVL_FAT; > + break; > + case BOOT_DEVICE_NAND: > + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) > + boot_loc = ENVL_NAND; > + break; > + case BOOT_DEVICE_SPI: > + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > + boot_loc = ENVL_SPI_FLASH; > + break; > + case BOOT_DEVICE_BOARD: > + break; > default: > - return ENVL_UNKNOWN; > + break; > + } > + > + /* Always try to access the environment on the boot device first. */ > + if (prio == 0) > + return boot_loc; > + > + if (prio == 1) { > + switch (boot_loc) { > + case ENVL_SPI_FLASH: > + return ENVL_FAT; > + case ENVL_FAT: > + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > + return ENVL_MMC; > + break; > + default: > + break; > + } > } > + > + return ENVL_UNKNOWN; > } > -#endif > > #ifdef CONFIG_DM_MMC > static void mmc_pinmux_setup(int sdc);
On Fri, 15 Apr 2022 12:28:22 -0500 Chris Morgan <macroalpha82@gmail.com> wrote: Hi Chris, > On Tue, Jan 11, 2022 at 12:46:04PM +0000, Andre Przywara wrote: > > Currently we only support to load the environment from raw MMC or FAT > > locations on Allwinner boards. With the advent of SPI flash we probably > > also want to support using the environment there, so we need to become > > a bit more flexible. > > > > Change the environment priority function to take the boot source into > > account. When booted from eMMC or SD card, we use FAT or MMC, if > > configured, as before. > > If we are booted from SPI flash, we try to use the environment from > > there, if possible. The same is true for NAND flash booting, although > > this is somewhat theoretical right now (as untested). > > > > This way we can use the same image for SD and SPI flash booting, which > > allows us to simply copy a booted image from SD card to the SPI flash, > > for instance. > > Unfortunately after a git-bisect I can confirm this patch breaks > booting in FEL mode whenever the environment is set to > CONFIG_ENV_IS_NOWHERE. Tested and reproducible on an NTC CHIP or a > Source Parts Popcorn (both Allwinner R8 chips). > > The curious thing though is that I could never see env_get_location > called, but whenever I either set the environment to something like > FAT or comment out this change (which I believe causes the non-board > specific version of env_get_location to be used) it would boot again. > So while I can confirm this patch breaks booting FEL when the env is > set to nowhere, I was not able to figure out why that is. Alright, thanks for the report. I see the problem: The default fallback for the new env_get_location() is ENVL_FAT, as 157 out of the 160 Allwinner boards define CONFIG_ENV_IS_IN_FAT. However the C.H.I.P. boards are one of the three exceptions, and returning FAT when the FAT environment driver is not compiled in will not end well. I don't have any CHIP board, but simulated this by configuring a normal board with just defining CONFIG_ENV_IS_NOWHERE. I made a patch that tries to choose a matching fallback. It's not very elegant, but seems to fix the problem for me. Please test this once posted. Cheers, Andre > > Thank you. > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 43 insertions(+), 8 deletions(-) > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > > index 2790a0f9e8..2472343d00 100644 > > --- a/board/sunxi/board.c > > +++ b/board/sunxi/board.c > > @@ -170,21 +170,56 @@ void i2c_init_board(void) > > #endif > > } > > > > -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT) > > +/* > > + * Try to use the environment from the boot source first. > > + * For MMC, this means a FAT partition on the boot device (SD or eMMC). > > + * If the raw MMC environment is also enabled, this is tried next. > > + * SPI flash falls back to FAT (on SD card). > > + */ > > enum env_location env_get_location(enum env_operation op, int prio) > > { > > - switch (prio) { > > - case 0: > > - return ENVL_FAT; > > + enum env_location boot_loc = ENVL_FAT; > > > > - case 1: > > - return ENVL_MMC; > > + gd->env_load_prio = prio; > > > > + switch (sunxi_get_boot_device()) { > > + case BOOT_DEVICE_MMC1: > > + case BOOT_DEVICE_MMC2: > > + boot_loc = ENVL_FAT; > > + break; > > + case BOOT_DEVICE_NAND: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) > > + boot_loc = ENVL_NAND; > > + break; > > + case BOOT_DEVICE_SPI: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > > + boot_loc = ENVL_SPI_FLASH; > > + break; > > + case BOOT_DEVICE_BOARD: > > + break; > > default: > > - return ENVL_UNKNOWN; > > + break; > > + } > > + > > + /* Always try to access the environment on the boot device first. */ > > + if (prio == 0) > > + return boot_loc; > > + > > + if (prio == 1) { > > + switch (boot_loc) { > > + case ENVL_SPI_FLASH: > > + return ENVL_FAT; > > + case ENVL_FAT: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > > + return ENVL_MMC; > > + break; > > + default: > > + break; > > + } > > } > > + > > + return ENVL_UNKNOWN; > > } > > -#endif > > > > #ifdef CONFIG_DM_MMC > > static void mmc_pinmux_setup(int sdc);
Will do. Sorry, I stepped away from things for a while but I will test it now that I am back. On Wed, Apr 20, 2022 at 6:34 PM Andre Przywara <andre.przywara@arm.com> wrote: > > On Fri, 15 Apr 2022 12:28:22 -0500 > Chris Morgan <macroalpha82@gmail.com> wrote: > > Hi Chris, > > > On Tue, Jan 11, 2022 at 12:46:04PM +0000, Andre Przywara wrote: > > > Currently we only support to load the environment from raw MMC or FAT > > > locations on Allwinner boards. With the advent of SPI flash we probably > > > also want to support using the environment there, so we need to become > > > a bit more flexible. > > > > > > Change the environment priority function to take the boot source into > > > account. When booted from eMMC or SD card, we use FAT or MMC, if > > > configured, as before. > > > If we are booted from SPI flash, we try to use the environment from > > > there, if possible. The same is true for NAND flash booting, although > > > this is somewhat theoretical right now (as untested). > > > > > > This way we can use the same image for SD and SPI flash booting, which > > > allows us to simply copy a booted image from SD card to the SPI flash, > > > for instance. > > > > Unfortunately after a git-bisect I can confirm this patch breaks > > booting in FEL mode whenever the environment is set to > > CONFIG_ENV_IS_NOWHERE. Tested and reproducible on an NTC CHIP or a > > Source Parts Popcorn (both Allwinner R8 chips). > > > > The curious thing though is that I could never see env_get_location > > called, but whenever I either set the environment to something like > > FAT or comment out this change (which I believe causes the non-board > > specific version of env_get_location to be used) it would boot again. > > So while I can confirm this patch breaks booting FEL when the env is > > set to nowhere, I was not able to figure out why that is. > > Alright, thanks for the report. I see the problem: The default > fallback for the new env_get_location() is ENVL_FAT, as 157 out of the > 160 Allwinner boards define CONFIG_ENV_IS_IN_FAT. > However the C.H.I.P. boards are one of the three exceptions, and > returning FAT when the FAT environment driver is not compiled in will > not end well. I don't have any CHIP board, but simulated this by > configuring a normal board with just defining CONFIG_ENV_IS_NOWHERE. > > I made a patch that tries to choose a matching fallback. It's not very > elegant, but seems to fix the problem for me. Please test this once > posted. > > Cheers, > Andre > > > > > Thank you. > > > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > --- > > > board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 43 insertions(+), 8 deletions(-) > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > > > index 2790a0f9e8..2472343d00 100644 > > > --- a/board/sunxi/board.c > > > +++ b/board/sunxi/board.c > > > @@ -170,21 +170,56 @@ void i2c_init_board(void) > > > #endif > > > } > > > > > > -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT) > > > +/* > > > + * Try to use the environment from the boot source first. > > > + * For MMC, this means a FAT partition on the boot device (SD or eMMC). > > > + * If the raw MMC environment is also enabled, this is tried next. > > > + * SPI flash falls back to FAT (on SD card). > > > + */ > > > enum env_location env_get_location(enum env_operation op, int prio) > > > { > > > - switch (prio) { > > > - case 0: > > > - return ENVL_FAT; > > > + enum env_location boot_loc = ENVL_FAT; > > > > > > - case 1: > > > - return ENVL_MMC; > > > + gd->env_load_prio = prio; > > > > > > + switch (sunxi_get_boot_device()) { > > > + case BOOT_DEVICE_MMC1: > > > + case BOOT_DEVICE_MMC2: > > > + boot_loc = ENVL_FAT; > > > + break; > > > + case BOOT_DEVICE_NAND: > > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) > > > + boot_loc = ENVL_NAND; > > > + break; > > > + case BOOT_DEVICE_SPI: > > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > > > + boot_loc = ENVL_SPI_FLASH; > > > + break; > > > + case BOOT_DEVICE_BOARD: > > > + break; > > > default: > > > - return ENVL_UNKNOWN; > > > + break; > > > + } > > > + > > > + /* Always try to access the environment on the boot device first. */ > > > + if (prio == 0) > > > + return boot_loc; > > > + > > > + if (prio == 1) { > > > + switch (boot_loc) { > > > + case ENVL_SPI_FLASH: > > > + return ENVL_FAT; > > > + case ENVL_FAT: > > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > > > + return ENVL_MMC; > > > + break; > > > + default: > > > + break; > > > + } > > > } > > > + > > > + return ENVL_UNKNOWN; > > > } > > > -#endif > > > > > > #ifdef CONFIG_DM_MMC > > > static void mmc_pinmux_setup(int sdc); >
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 2790a0f9e8..2472343d00 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -170,21 +170,56 @@ void i2c_init_board(void) #endif } -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT) +/* + * Try to use the environment from the boot source first. + * For MMC, this means a FAT partition on the boot device (SD or eMMC). + * If the raw MMC environment is also enabled, this is tried next. + * SPI flash falls back to FAT (on SD card). + */ enum env_location env_get_location(enum env_operation op, int prio) { - switch (prio) { - case 0: - return ENVL_FAT; + enum env_location boot_loc = ENVL_FAT; - case 1: - return ENVL_MMC; + gd->env_load_prio = prio; + switch (sunxi_get_boot_device()) { + case BOOT_DEVICE_MMC1: + case BOOT_DEVICE_MMC2: + boot_loc = ENVL_FAT; + break; + case BOOT_DEVICE_NAND: + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) + boot_loc = ENVL_NAND; + break; + case BOOT_DEVICE_SPI: + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) + boot_loc = ENVL_SPI_FLASH; + break; + case BOOT_DEVICE_BOARD: + break; default: - return ENVL_UNKNOWN; + break; + } + + /* Always try to access the environment on the boot device first. */ + if (prio == 0) + return boot_loc; + + if (prio == 1) { + switch (boot_loc) { + case ENVL_SPI_FLASH: + return ENVL_FAT; + case ENVL_FAT: + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) + return ENVL_MMC; + break; + default: + break; + } } + + return ENVL_UNKNOWN; } -#endif #ifdef CONFIG_DM_MMC static void mmc_pinmux_setup(int sdc);
Currently we only support to load the environment from raw MMC or FAT locations on Allwinner boards. With the advent of SPI flash we probably also want to support using the environment there, so we need to become a bit more flexible. Change the environment priority function to take the boot source into account. When booted from eMMC or SD card, we use FAT or MMC, if configured, as before. If we are booted from SPI flash, we try to use the environment from there, if possible. The same is true for NAND flash booting, although this is somewhat theoretical right now (as untested). This way we can use the same image for SD and SPI flash booting, which allows us to simply copy a booted image from SD card to the SPI flash, for instance. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 8 deletions(-)