| Message ID | 20220624161209.1644097-1-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | e008e5132f7e160320dcc0ffb8fe7fef5264aecd |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | [v2] sunxi: fix initial environment loading without MMC | expand |
On Fri, 24 Jun 2022 17:12:09 +0100 Andre Przywara <andre.przywara@arm.com> wrote: > From: Samuel Holland <samuel@sholland.org> > > Commit e42dad4168fe ("sunxi: use boot source for determining environment > location") changed our implementation of env_get_location() and enabled > it for every board, even those without MMC support (like the C.H.I.P. > boards). However the default fallback location of ENVL_FAT requires MMC > support compiled in, so the board hangs when trying to initially load > the environment. > > Change the algorithm to only return configured environment locations, > and improve the fallback algorithm on the way. > > The env_init() routine calling this function here does not behave well > if the return value is ENVL_UNKNOWN on the very first call: it will make > U-Boot proper silently hang very early. > Work around this issue by making sure we return some configured (dummy) > environment location when prio is 0. This for instance happens when > booting via FEL. > > This fixes U-Boot loading on the C.H.I.P. boards. > > Fixes: e42dad4168fe ("sunxi: use boot source for determining environment location") > Reported-by: Chris Morgan <macroalpha82@gmail.com> > Signed-off-by: Samuel Holland <samuel@sholland.org> > [Andre: fix FEL boot case by not returning ENVL_UNKNOWN when prio==0] > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Applied to sunxi/master, for v2022.07. Cheers, Andre > --- > Hi Samuel, > > I cheekily added your Signed-off-by:, as I made this your patch (since > you came up with it in that email reply some weeks ago). I hope that's > fine. > I tested this briefly on a Pine64-LTS, booting from FEL, SD card, eMMC > and SPI. I also simulated a CHIP board, by just defining ENV_IS_NOWHERE, > then booting, as this was the case that broke. > If no one complains, I would like to push this ASAP, to make it into > the 2022.07 release still. > > Cheers, > Andre > > board/sunxi/board.c | 49 +++++++++++++++++++++++++++------------------ > 1 file changed, 29 insertions(+), 20 deletions(-) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 806e3bcb69..21a2407e06 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -128,26 +128,37 @@ void i2c_init_board(void) > * 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. > + * When booting from NAND we try UBI first, then NAND directly. > * SPI flash falls back to FAT (on SD card). > */ > enum env_location env_get_location(enum env_operation op, int prio) > { > - enum env_location boot_loc = ENVL_FAT; > + if (prio > 1) > + return ENVL_UNKNOWN; > > - gd->env_load_prio = prio; > + /* NOWHERE is exclusive, no other option can be defined. */ > + if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) > + return ENVL_NOWHERE; > > switch (sunxi_get_boot_device()) { > case BOOT_DEVICE_MMC1: > case BOOT_DEVICE_MMC2: > - boot_loc = ENVL_FAT; > + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) > + return ENVL_FAT; > + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > + return ENVL_MMC; > break; > case BOOT_DEVICE_NAND: > + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_UBI)) > + return ENVL_UBI; > if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) > - boot_loc = ENVL_NAND; > + return ENVL_NAND; > break; > case BOOT_DEVICE_SPI: > - if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > - boot_loc = ENVL_SPI_FLASH; > + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > + return ENVL_SPI_FLASH; > + if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) > + return ENVL_FAT; > break; > case BOOT_DEVICE_BOARD: > break; > @@ -155,21 +166,19 @@ enum env_location env_get_location(enum env_operation op, int prio) > 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: > + /* > + * If we come here for the first time, we *must* return a valid > + * environment location other than ENVL_UNKNOWN, or the setup sequence > + * in board_f() will silently hang. This is arguably a bug in > + * env_init(), but for now pick one environment for which we know for > + * sure to have a driver for. For all defconfigs this is either FAT > + * or UBI, or NOWHERE, which is already handled above. > + */ > + if (prio == 0) { > + if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) > return ENVL_FAT; > - case ENVL_FAT: > - if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > - return ENVL_MMC; > - break; > - default: > - break; > - } > + if (IS_ENABLED(CONFIG_ENV_IS_IN_UBI)) > + return ENVL_UBI; > } > > return ENVL_UNKNOWN;
Hi Andre, On 6/24/22 11:12 AM, Andre Przywara wrote: > From: Samuel Holland <samuel@sholland.org> > > Commit e42dad4168fe ("sunxi: use boot source for determining environment > location") changed our implementation of env_get_location() and enabled > it for every board, even those without MMC support (like the C.H.I.P. > boards). However the default fallback location of ENVL_FAT requires MMC > support compiled in, so the board hangs when trying to initially load > the environment. > > Change the algorithm to only return configured environment locations, > and improve the fallback algorithm on the way. > > The env_init() routine calling this function here does not behave well > if the return value is ENVL_UNKNOWN on the very first call: it will make > U-Boot proper silently hang very early. > Work around this issue by making sure we return some configured (dummy) > environment location when prio is 0. This for instance happens when > booting via FEL. > > This fixes U-Boot loading on the C.H.I.P. boards. > > Fixes: e42dad4168fe ("sunxi: use boot source for determining environment location") > Reported-by: Chris Morgan <macroalpha82@gmail.com> > Signed-off-by: Samuel Holland <samuel@sholland.org> > [Andre: fix FEL boot case by not returning ENVL_UNKNOWN when prio==0] > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi Samuel, > > I cheekily added your Signed-off-by:, as I made this your patch (since > you came up with it in that email reply some weeks ago). I hope that's > fine. Yes, this is fine with me. Regards, Samuel
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 806e3bcb69..21a2407e06 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -128,26 +128,37 @@ void i2c_init_board(void) * 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. + * When booting from NAND we try UBI first, then NAND directly. * SPI flash falls back to FAT (on SD card). */ enum env_location env_get_location(enum env_operation op, int prio) { - enum env_location boot_loc = ENVL_FAT; + if (prio > 1) + return ENVL_UNKNOWN; - gd->env_load_prio = prio; + /* NOWHERE is exclusive, no other option can be defined. */ + if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) + return ENVL_NOWHERE; switch (sunxi_get_boot_device()) { case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2: - boot_loc = ENVL_FAT; + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) + return ENVL_FAT; + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) + return ENVL_MMC; break; case BOOT_DEVICE_NAND: + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_UBI)) + return ENVL_UBI; if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) - boot_loc = ENVL_NAND; + return ENVL_NAND; break; case BOOT_DEVICE_SPI: - if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) - boot_loc = ENVL_SPI_FLASH; + if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) + return ENVL_SPI_FLASH; + if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) + return ENVL_FAT; break; case BOOT_DEVICE_BOARD: break; @@ -155,21 +166,19 @@ enum env_location env_get_location(enum env_operation op, int prio) 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: + /* + * If we come here for the first time, we *must* return a valid + * environment location other than ENVL_UNKNOWN, or the setup sequence + * in board_f() will silently hang. This is arguably a bug in + * env_init(), but for now pick one environment for which we know for + * sure to have a driver for. For all defconfigs this is either FAT + * or UBI, or NOWHERE, which is already handled above. + */ + if (prio == 0) { + if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) return ENVL_FAT; - case ENVL_FAT: - if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) - return ENVL_MMC; - break; - default: - break; - } + if (IS_ENABLED(CONFIG_ENV_IS_IN_UBI)) + return ENVL_UBI; } return ENVL_UNKNOWN;