Re: [PATCH 1/3] arm: mach-k3: Rework spl/get_boot_device()

Wed, 29 Oct 2025 08:39:48 -0700 

Hi Anshul,


On 10/7/25 1:32 PM, Anshul Dalal wrote:

Hi Wadim, I have noted a few comments below but overall the patch set
looks good to me.

On Thu Oct 2, 2025 at 6:34 PM IST, Wadim Egorov wrote:

Introduce a shared boot-device helper that translates DEVSTAT boot
modes via SoC-specific lookup tables. This replaces the per SoC
get_boot_device()/get_*_bootmedia() implementations and centralizes
the logic in boot-device.c while keeping the mapping data SoC-owned.

Start with am62x as a reference implementation.

Signed-off-by: Wadim Egorov <[email protected]>
---
  arch/arm/mach-k3/Makefile                     |   2 +-
  arch/arm/mach-k3/am62x/boot.c                 | 141 ++++++------------
  arch/arm/mach-k3/boot-device.c                |  90 +++++++++++
  arch/arm/mach-k3/boot-device.h                |  34 +++++
  arch/arm/mach-k3/include/mach/am62_hardware.h |   1 +
  5 files changed, 169 insertions(+), 99 deletions(-)
  create mode 100644 arch/arm/mach-k3/boot-device.c
  create mode 100644 arch/arm/mach-k3/boot-device.h

diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile
index b2fd5810b67..7dc77328e5c 100644
--- a/arch/arm/mach-k3/Makefile
+++ b/arch/arm/mach-k3/Makefile
@@ -6,7 +6,7 @@
  obj-$(CONFIG_ARM64) += arm64/
  obj-$(CONFIG_CPU_V7R) += r5/
  obj-$(CONFIG_OF_LIBFDT) += common_fdt.o
-obj-y += common.o security.o k3-ddr.o
+obj-y += common.o security.o k3-ddr.o boot-device.o
  obj-$(CONFIG_SOC_K3_AM62A7) += am62ax/
  obj-$(CONFIG_SOC_K3_AM62P5) += am62px/
  obj-$(CONFIG_SOC_K3_AM625) += am62x/
diff --git a/arch/arm/mach-k3/am62x/boot.c b/arch/arm/mach-k3/am62x/boot.c
index a3a6cda6bdb..06bd04da99f 100644
--- a/arch/arm/mach-k3/am62x/boot.c
+++ b/arch/arm/mach-k3/am62x/boot.c
@@ -3,104 +3,7 @@
  #include <asm/arch/hardware.h>
  #include <asm/arch/am62_spl.h>
-static u32 __get_backup_bootmedia(u32 devstat) 
-{
-       u32 bkup_bootmode = (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_MASK) >>
-                               MAIN_DEVSTAT_BACKUP_BOOTMODE_SHIFT;
-       u32 bkup_bootmode_cfg =
-                       (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_MASK) >>
-                               MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_SHIFT;
-
-       switch (bkup_bootmode) {
-       case BACKUP_BOOT_DEVICE_UART:
-               return BOOT_DEVICE_UART;
-
-       case BACKUP_BOOT_DEVICE_USB:
-               return BOOT_DEVICE_USB;
-
-       case BACKUP_BOOT_DEVICE_ETHERNET:
-               return BOOT_DEVICE_ETHERNET;
-
-       case BACKUP_BOOT_DEVICE_MMC:
-               if (bkup_bootmode_cfg)
-                       return BOOT_DEVICE_MMC2;
-               return BOOT_DEVICE_MMC1;
-
-       case BACKUP_BOOT_DEVICE_SPI:
-               return BOOT_DEVICE_SPI;
-
-       case BACKUP_BOOT_DEVICE_I2C:
-               return BOOT_DEVICE_I2C;
-
-       case BACKUP_BOOT_DEVICE_DFU:
-               if (bkup_bootmode_cfg & MAIN_DEVSTAT_BACKUP_USB_MODE_MASK)
-                       return BOOT_DEVICE_USB;
-               return BOOT_DEVICE_DFU;
-       };
-
-       return BOOT_DEVICE_RAM;
-}
-
-static u32 __get_primary_bootmedia(u32 devstat)
-{
-       u32 bootmode = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK) >>
-                               MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT;
-       u32 bootmode_cfg = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_MASK) >>
-                               MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_SHIFT;
-
-       switch (bootmode) {
-       case BOOT_DEVICE_OSPI:
-               fallthrough;
-       case BOOT_DEVICE_QSPI:
-               fallthrough;
-       case BOOT_DEVICE_XSPI:
-               fallthrough;
-       case BOOT_DEVICE_SPI:
-               return BOOT_DEVICE_SPI;
-
-       case BOOT_DEVICE_ETHERNET_RGMII:
-               fallthrough;
-       case BOOT_DEVICE_ETHERNET_RMII:
-               return BOOT_DEVICE_ETHERNET;
-
-       case BOOT_DEVICE_EMMC:
-               return BOOT_DEVICE_MMC1;
-
-       case BOOT_DEVICE_MMC:
-               if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_MMC_PORT_MASK) >>
-                               MAIN_DEVSTAT_PRIMARY_MMC_PORT_SHIFT)
-                       return BOOT_DEVICE_MMC2;
-               return BOOT_DEVICE_MMC1;
-
-       case BOOT_DEVICE_DFU:
-               if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_USB_MODE_MASK) >>
-                   MAIN_DEVSTAT_PRIMARY_USB_MODE_SHIFT)
-                       return BOOT_DEVICE_USB;
-               return BOOT_DEVICE_DFU;
-
-       case BOOT_DEVICE_NOBOOT:
-               return BOOT_DEVICE_RAM;
-       }
-
-       return bootmode;
-}
-
-u32 get_boot_device(void)
-{
-       u32 devstat = readl(CTRLMMR_MAIN_DEVSTAT);
-       u32 bootmode = *(u32 *)(K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
-       u32 bootmedia;
-
-       if (bootmode == K3_PRIMARY_BOOTMODE)
-               bootmedia = __get_primary_bootmedia(devstat);
-       else
-               bootmedia = __get_backup_bootmedia(devstat);
-
-       debug("%s: devstat = 0x%x bootmedia = 0x%x bootmode = %d\n",
-             __func__, devstat, bootmedia, bootmode);
-
-       return bootmedia;
-}
+#include "../boot-device.h"
const char *get_reset_reason(void) 
  {
@@ -141,3 +44,45 @@ const char *get_reset_reason(void)
return "UNKNOWN"; 
  }
+
+static const struct k3_boot_map am62_boot_device_primary_table[] = {
+       { BOOT_DEVICE_OSPI, 0, 0, 0, BOOT_DEVICE_SPI },
+       { BOOT_DEVICE_QSPI, 0, 0, 0, BOOT_DEVICE_SPI },
+       { BOOT_DEVICE_XSPI, 0, 0, 0, BOOT_DEVICE_SPI },
+       { BOOT_DEVICE_SPI, 0, 0, 0, BOOT_DEVICE_SPI },
+       { BOOT_DEVICE_ETHERNET_RGMII, 0, 0, 0, BOOT_DEVICE_ETHERNET },
+       { BOOT_DEVICE_ETHERNET_RMII, 0, 0, 0, BOOT_DEVICE_ETHERNET },
+       { BOOT_DEVICE_EMMC, 0, 0, 0, BOOT_DEVICE_MMC1 },
+       { BOOT_DEVICE_MMC, MAIN_DEVSTAT_PRIMARY_MMC_PORT_MASK, 
MAIN_DEVSTAT_PRIMARY_MMC_PORT_SHIFT, 1, BOOT_DEVICE_MMC2 },
+       { BOOT_DEVICE_MMC, MAIN_DEVSTAT_PRIMARY_MMC_PORT_MASK, 
MAIN_DEVSTAT_PRIMARY_MMC_PORT_SHIFT, 0, BOOT_DEVICE_MMC1 },
+       { BOOT_DEVICE_DFU, MAIN_DEVSTAT_PRIMARY_USB_MODE_MASK, 
MAIN_DEVSTAT_PRIMARY_USB_MODE_SHIFT, 1, BOOT_DEVICE_USB },
+       { BOOT_DEVICE_DFU, MAIN_DEVSTAT_PRIMARY_USB_MODE_MASK, 
MAIN_DEVSTAT_PRIMARY_USB_MODE_SHIFT, 0, BOOT_DEVICE_DFU },
+       { BOOT_DEVICE_NOBOOT, 0, 0, 0, BOOT_DEVICE_RAM },
+};
+
+static const struct k3_boot_map am62_boot_device_backup_table[] = {
+       { BACKUP_BOOT_DEVICE_UART, 0, 0, 0, BOOT_DEVICE_UART },
+       { BACKUP_BOOT_DEVICE_USB, 0, 0, 0, BOOT_DEVICE_USB },
+       { BACKUP_BOOT_DEVICE_ETHERNET, 0, 0, 0, BOOT_DEVICE_ETHERNET },
+       { BACKUP_BOOT_DEVICE_MMC, 1, 0, 1, BOOT_DEVICE_MMC2 },
+       { BACKUP_BOOT_DEVICE_MMC, 1, 0, 0, BOOT_DEVICE_MMC1 },
+       { BACKUP_BOOT_DEVICE_SPI, 0, 0, 0, BOOT_DEVICE_SPI },
+       { BACKUP_BOOT_DEVICE_I2C, 0, 0, 0, BOOT_DEVICE_I2C },
+       { BACKUP_BOOT_DEVICE_DFU, MAIN_DEVSTAT_BACKUP_USB_MODE_MASK, 0, 0, 
BOOT_DEVICE_DFU },
+       { BACKUP_BOOT_DEVICE_DFU, MAIN_DEVSTAT_BACKUP_USB_MODE_MASK, 0, 1, 
BOOT_DEVICE_USB },
+};
+
+static const struct k3_boot_device_info am62_boot_device_info = {
+       .boot_device_primary_table = am62_boot_device_primary_table,
+       .boot_device_primary_count = ARRAY_SIZE(am62_boot_device_primary_table),
+       .boot_device_backup_table = am62_boot_device_backup_table,
+       .boot_device_backup_count = ARRAY_SIZE(am62_boot_device_backup_table),
+       .main_devstat_reg = (void __iomem *)CTRLMMR_MAIN_DEVSTAT,
+       .wkup_devstat_reg = NULL,
+       .bootmode_addr = (void __iomem *)K3_BOOT_PARAM_TABLE_INDEX_OCRAM,
+};
+
+const struct k3_boot_device_info *k3_get_boot_device_info(void)
+{
+       return &am62_boot_device_info;
+}
diff --git a/arch/arm/mach-k3/boot-device.c b/arch/arm/mach-k3/boot-device.c
new file mode 100644
index 00000000000..9431dae71fa
--- /dev/null
+++ b/arch/arm/mach-k3/boot-device.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright (C) 2025 PHYTEC Messtechnik GmbH
+ *  Author: Wadim Egorov <[email protected]>
+ */
+
+#include "boot-device.h"
+
+#include <asm/io.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/am62_spl.h>
+


We shouldn't be including an am62x specific header in a mach-k3 file
meant for multiple SoCs. Please refactor the macros used below so it's
no longer required.


Good point, I will update it.



One idea could be to let the SoC's boot.c handle parsing the devstat and
bootmode (primary or not). With get_boot_device in boot-device.c only
receiving the final bootmode and bootmode_cfg as part of the
k3_boot_device_info struct.
This means we will end up with code duplication again, every SoC/boot.c will have identical parsing code. Let's not do this. 



This would also allow Jacinto platforms to easily make use of this
refactor since the handling of wkup and main devstat would be handled as
part of the SoC's boot.c.


I don't think it is a big deal to extend for jacinto here.




+__weak const struct k3_boot_device_info *k3_get_boot_device_info(void)
+{
+       return NULL;
+}
+
+static u32 __get_backup_bootmedia(const struct k3_boot_device_info *info, u32 
devstat)
+{
+       u32 bkup_bootmode = (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_MASK) >>
+                            MAIN_DEVSTAT_BACKUP_BOOTMODE_SHIFT;
+       u32 bkup_bootmode_cfg = (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_MASK) 
>>
+                                MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_SHIFT;
+       unsigned int i;
+
+       for (i = 0; i < info->boot_device_backup_count; i++) {
+               const struct k3_boot_map *m = 
&info->boot_device_backup_table[i];
+
+               if (bkup_bootmode != m->mode)
+                       continue;
+
+               if (m->cfg_mask == 0)
+                       return m->result;
+
+               if (((bkup_bootmode_cfg & m->cfg_mask) >> m->cfg_shift) == 
m->cfg_value)
+                       return m->result;
+       }
+
+       return BOOT_DEVICE_RAM;
+}
+
+static u32 __get_primary_bootmedia(const struct k3_boot_device_info *info, u32 
devstat)
+{
+       u32 bootmode = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK) >>
+                       MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT;
+       u32 bootmode_cfg = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_MASK) >>
+                       MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_SHIFT;
+       unsigned int i;
+
+       for (i = 0; i < info->boot_device_primary_count; i++) {
+               const struct k3_boot_map *m = 
&info->boot_device_primary_table[i];
+
+               if (bootmode != m->mode)
+                       continue;
+
+               if (m->cfg_mask == 0)
+                       return m->result;
+
+               if (((bootmode_cfg & m->cfg_mask) >> m->cfg_shift) == 
m->cfg_value)
+                       return m->result;
+       }
+
+       return bootmode;
+}
+
+u32 __weak get_boot_device(void)
+{


Having a k3 specific get_boot_device called by U-Boot's generic
spl_boot_device from SoC's init file is convoluted.

We should probably just replace get_boot_device with spl_boot_device in
a later patch after this series.
I have introduced get_boot_device() because our board code needs this information in various stages. See board/phytec/common/k3/board.c 





+       const struct k3_boot_device_info *info = k3_get_boot_device_info();
+       u32 bootmode;
+       u32 main_devstat;
+       u32 bootmedia;
+
+       if (!info) {
+               pr_err("%s: no boot-device info for this SoC\n", __func__);
+               return BOOT_DEVICE_NOBOOT;
+       }
+
+       bootmode = readl(info->bootmode_addr);
+       main_devstat = readl(info->main_devstat_reg);
+
+       if (bootmode == K3_PRIMARY_BOOTMODE)
+               bootmedia = __get_primary_bootmedia(info, main_devstat);
+       else
+               bootmedia = __get_backup_bootmedia(info, main_devstat);
+
+       debug("%s: devstat = 0x%x bootmedia = 0x%x bootmode = %d\n",
+             __func__, main_devstat, bootmedia, bootmode);
+
+       return bootmedia;
+}
diff --git a/arch/arm/mach-k3/boot-device.h b/arch/arm/mach-k3/boot-device.h
new file mode 100644
index 00000000000..965a4570710
--- /dev/null
+++ b/arch/arm/mach-k3/boot-device.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright (C) 2025 PHYTEC Messtechnik GmbH
+ *  Author: Wadim Egorov <[email protected]>
+ */
+
+#ifndef _K3_BOOT_DEVICE_H
+#define _K3_BOOT_DEVICE_H
+
+#include <linux/types.h>
+#include <asm/io.h>
+
+/* Descriptor for mode + optional cfg test -> normalized device */
+struct k3_boot_map {
+       u32 mode;       /* Raw PRIMARY_BOOTMODE value */
+       u32 cfg_mask;   /* Bits in devstat-cfg to test (0 if none) */
+       u32 cfg_shift;  /* Shift right before comparing */
+       u32 cfg_value;  /* Desired value after mask+shift */
+       u32 result;     /* Normalized device to return */
+};
+


Nit: This could be better documented following the standard '@' in a
comment above the struct.


Can do




+struct k3_boot_device_info {
+       const struct k3_boot_map *boot_device_primary_table;
+       unsigned int boot_device_primary_count;
+       const struct k3_boot_map *boot_device_backup_table;
+       unsigned int boot_device_backup_count;
+       void __iomem *main_devstat_reg;
+       void __iomem *wkup_devstat_reg;
+       void __iomem *bootmode_addr;
+};
+
+const struct k3_boot_device_info *k3_get_boot_device_info(void);
+
+#endif /* _K3_BOOT_DEVICE_H */
diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h 
b/arch/arm/mach-k3/include/mach/am62_hardware.h
index 2f5655bf24a..084f16bf2d2 100644
--- a/arch/arm/mach-k3/include/mach/am62_hardware.h
+++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
@@ -10,6 +10,7 @@
  #define __ASM_ARCH_AM62_HARDWARE_H
#include <config.h> 
+#include <asm/io.h>


Why do we need this header here?
There was a compile error in a previous version. A separate patch would be better, sorry. But I will drop it since there is no Issue anymore. 

Regards,
Wadim




  #ifndef __ASSEMBLY__
  #include <linux/bitops.h>
  #endif


Acked-by: Anshul Dalal <[email protected]>

Regards,
Anshul

Reply via email to