On Wed, Sep 2, 2026 at 4:52 PM Tim Ouyang <[email protected]> wrote:
>
> On Mon, Aug 24, 2026 at 04:50:36PM +0800, Eric Chung wrote:
> > Add logic to determine the current boot device by reading the
> > SoC's configuration registers, rather than using a hardcoded
> > default.
> >
> > Signed-off-by: Eric Chung <[email protected]>
> > Reviewed-by: Yao Zi <[email protected]>
> > ---
> > v9:
> > - Enhance detection on booting from SD card.
> > v5:
> > - Add blank lines around the switch statement.
> > v2:
> > - Use FIELD_GET() to parse boot strap mode.
> > - Remove comments on hacking. Since the sequence between eMMC
> >   and SD device is exchanged in the upstream DTS. So it isn't
> >   a hacking any more.
> > ---
> >  board/spacemit/k1/spl.c | 95 
> > ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 94 insertions(+), 1 deletion(-)
> >
> > diff --git a/board/spacemit/k1/spl.c b/board/spacemit/k1/spl.c
> > index d749e21a2d57..b70eed14e6ed 100644
> > --- a/board/spacemit/k1/spl.c
> > +++ b/board/spacemit/k1/spl.c
> > @@ -13,6 +13,7 @@
> >  #include <dm/device.h>
> >  #include <dm/uclass.h>
> >  #include <i2c.h>
> > +#include <linux/bitfield.h>
> >  #include <linux/ctype.h>
> >  #include <linux/delay.h>
> >  #include <log.h>
> > @@ -22,6 +23,20 @@
> >  #include <tlv_eeprom.h>
> >  #include "tlv_codes.h"
> >
> > +/* boot mode configs */
> > +#define BOOT_DEV_FLAG_REG      0xd4282d10
> > +#define BOOT_PIN_SEL_REG       0xd4282c20
> > +
> > +#define BOOT_STRAP_MODE_OFFSET 9
> > +#define BOOT_STRAP_MODE_MASK   3
> > +#define BOOT_STRAP_MODE_EMMC   0
> > +#define BOOT_STRAP_MODE_SPI    1
> > +#define BOOT_STRAP_MODE_NAND   2
> > +#define BOOT_STRAP_MODE_SD     3
> > +
> > +#define STORAGE_API_P_ADDR     0xc0838498
> > +#define SDCARD_API_ENTRY       0xffe0a548
> > +
> >  #define MUX_MODE4              4
> >  #define EDGE_NONE              BIT(6)
> >  #define PULL_UP                        (6 << 13)       /* bit[15:13] 110 */
> > @@ -46,6 +61,17 @@ typedef void (*puts_func_t)(const char *s);
> >  typedef int (*ddr_init_func_t)(u64 ddr_base, u32 cs_num, u32 data_rate,
> >                                puts_func_t puts);
> >
> > +enum board_boot_mode {
> > +       BOOT_MODE_NONE = 0,
> > +       BOOT_MODE_USB = 0x55a,
> > +       BOOT_MODE_EMMC,
> > +       BOOT_MODE_NAND,
> > +       BOOT_MODE_SPI,
> > +       BOOT_MODE_SD,
> > +       BOOT_MODE_SHELL = 0x55f,
> > +       BOOT_MODE_BOOTSTRAP,
> > +};
> > +
> >  struct ddr_cfg {
> >         u32     data_rate;
> >         u32     cs_num;
> > @@ -58,6 +84,69 @@ binman_sym_declare(ulong, ddr_fw, size);
> >
> >  char product_name[I2C_BUF_SIZE] = "k1";
> >
> > +u32 read_boot_mode(void)
> > +{
> > +       void __iomem *boot_dev = (void __iomem *)BOOT_DEV_FLAG_REG;
> > +       void __iomem *boot_strap = (void __iomem *)BOOT_PIN_SEL_REG;
> > +       void __iomem *storage_api = (void __iomem *)STORAGE_API_P_ADDR;
> > +       u32 mode, sel, ret = 0;
> > +
> > +       mode = readl(storage_api);
> > +       if (mode == SDCARD_API_ENTRY)
> > +               return BOOT_MODE_SD;
> > +
> > +       mode = readl(boot_dev);
> > +       if (mode == BOOT_MODE_NONE || mode > BOOT_MODE_SD) {
> > +               sel = FIELD_GET(BOOT_STRAP_MODE_MASK << 
> > BOOT_STRAP_MODE_OFFSET,
> > +                               readl(boot_strap));
> > +               switch (sel) {
> > +               case BOOT_STRAP_MODE_EMMC:
> > +                       mode = BOOT_MODE_EMMC;
> > +                       break;
> > +               case BOOT_STRAP_MODE_NAND:
> > +                       mode = BOOT_MODE_NAND;
> > +                       break;
> > +               case BOOT_STRAP_MODE_SPI:
> > +                       mode = BOOT_MODE_SPI;
> > +                       break;
> > +               case BOOT_STRAP_MODE_SD:
> > +               default:
> > +                       mode = BOOT_MODE_SD;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       switch (mode) {
> > +       case BOOT_MODE_EMMC:
> > +               ret = BOOT_DEVICE_MMC1;
> > +               break;
> > +       case BOOT_MODE_NAND:
> > +               ret = BOOT_DEVICE_NAND;
> > +               break;
> > +       case BOOT_MODE_SPI:
> > +               ret = BOOT_DEVICE_SPI;
> > +               break;
> > +       case BOOT_MODE_USB:
> > +               ret = BOOT_DEVICE_USB;
> > +               break;
> > +       case BOOT_MODE_SD:
> > +               ret = BOOT_DEVICE_MMC2;
> > +               break;
> > +       default:
> > +               ret = BOOT_DEVICE_MMC1;
> > +               break;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +void write_boot_mode(u32 mode)
> > +{
> > +       void __iomem *boot_dev = (void __iomem *)BOOT_DEV_FLAG_REG;
> > +
> > +       writel(mode, boot_dev);
> > +}
> > +
> >  static void i2c_early_init(void)
> >  {
> >         struct udevice *bus;
> > @@ -327,8 +416,12 @@ void nor_early_init(void)
> >
> >  void board_init_f(ulong dummy)
> >  {
> > +       u32 mode;
> >         int ret;
> >
> > +       mode = read_boot_mode();
> > +       write_boot_mode(mode);
> > +
> >         ret = spl_early_init();
> >         if (ret)
> >                 panic("spl_early_init() failed:%d\n", ret);
> > @@ -354,7 +447,7 @@ void board_init_f(ulong dummy)
> >
> >  u32 spl_boot_device(void)
> >  {
> > -       return BOOT_DEVICE_SPI;
> > +       return read_boot_mode();
> >  }
> >
> >  void spl_board_init(void)
>
> Hi Eric,
>
> I previously said that I found no obvious issues, but after taking
> another look, I noticed that read_boot_mode() is called twice: first
> from board_init_f() and later from spl_boot_device().
>
> The first call returns a BOOT_DEVICE_* value, which board_init_f()
> writes back to BOOT_DEV_FLAG_REG. The second call then incorrectly
> interprets that value as a BOOT_MODE_* value.
>
> For example, BOOT_DEVICE_SPI is written as 8, which does not match
> BOOT_MODE_SPI (0x55d), so the second call falls back to MMC1.
>
> Please keep the BOOT_MODE_* and BOOT_DEVICE_* encodings separate.
> Sorry for missing this in my initial review.
> --
> Best regards,
> Tim Ouyang

Hi Tim,

Thanks for catching this issue. I've fixed it and submitted a new
version. Please review and merge the new version.

Best Regards
Eric

Reply via email to