Hi Bryan, On 2026-08-04T21:55:02, Bryan Brattlof <[email protected]> wrote: > arm: mach-k3: Add initial am62lx SoC support > > Add initial mach-k3 files needed to support boards based on the AM62L > family of SoCs
The commit message is very thin for a patch that adds a new SoC family. Please expand it to describe what the AM62L is (ARM64 Cortex-A53), the boot flow being wired up (SCMI/PSCI selects, SPL entry, boot-device decoding, ATF reserved region sizing), and why this SoC gets its own subdirectory rather than reusing am62x. That gives future readers the context to make sense of the sizeable Kconfig re-shuffle in the same patch. > > Tested-by: Anshul Dalal <[email protected]> > Reviewed-by: Anshul Dalal <[email protected]> > Signed-off-by: Bryan Brattlof <[email protected]> > > arch/arm/mach-k3/Kconfig | 17 +- > arch/arm/mach-k3/Makefile | 2 + > arch/arm/mach-k3/am62lx/Kconfig | 23 ++ > arch/arm/mach-k3/am62lx/Makefile | 7 + > arch/arm/mach-k3/am62lx/am62l3_fdt.c | 12 + > arch/arm/mach-k3/am62lx/am62l3_init.c | 58 ++++ > arch/arm/mach-k3/am62lx/boot.c | 83 +++++ > arch/arm/mach-k3/am62lx/schema.yaml | 427 > +++++++++++++++++++++++++ > arch/arm/mach-k3/include/mach/am62l_hardware.h | 48 +++ > arch/arm/mach-k3/include/mach/am62l_spl.h | 41 +++ > arch/arm/mach-k3/include/mach/hardware.h | 4 + > arch/arm/mach-k3/include/mach/spl.h | 4 + > include/configs/am62l3_evm.h | 14 + > 13 files changed, 736 insertions(+), 4 deletions(-) > diff --git a/arch/arm/mach-k3/am62lx/Kconfig b/arch/arm/mach-k3/am62lx/Kconfig > @@ -0,0 +1,23 @@ > +config TARGET_AM62L3_EVM > + bool "TI K3 based AM62L5 EVM" Typo - AM62L5 should be AM62L3 to match the symbol name and the rest of the series. > diff --git a/arch/arm/mach-k3/am62lx/am62l3_init.c > b/arch/arm/mach-k3/am62lx/am62l3_init.c > @@ -0,0 +1,58 @@ > + case BOOT_DEVICE_MMC: > + if (bootmode_cfg & MAIN_DEVSTAT_PRIMARY_MMC_FS_RAW_MASK) > + return MMCSD_MODE_RAW; > + default: > + return MMCSD_MODE_FS; > + } > +} > + I would expect the fall-through from BOOT_DEVICE_MMC into default to warn - can you add an explicit fallthrough; ? Also please drop the trailing blank line at end-of-file. > diff --git a/arch/arm/mach-k3/am62lx/Makefile > b/arch/arm/mach-k3/am62lx/Makefile > @@ -0,0 +1,7 @@ > +obj-$(CONFIG_OF_SYSTEM_SETUP) += am62l3_fdt.o > +obj-$(CONFIG_SPL_BUILD) += am62l3_init.o > +obj-y += boot.o boot.c only exports get_boot_device(), which is called from spl_boot_device() in am62l3_init.c - i.e. SPL only. Shouldn't this be gated on CONFIG_SPL_BUILD as well, rather than obj-y? > diff --git a/arch/arm/mach-k3/include/mach/am62l_spl.h > b/arch/arm/mach-k3/include/mach/am62l_spl.h > @@ -0,0 +1,41 @@ > +#define BOOT_DEVICE_USB 0x0A > +#define BOOT_DEVICE_DFU 0x0A Lower-case hex. But just to check - on am62p and am62a these are distinct values (USB=0x2a, DFU=0x0a). Is the collapse to a single value correct for the AM62L ROM, or a copy-paste from the aliasing block below? If it really is one value on this SoC, please add a short comment saying so, otherwise nothing stops someone adding a 'case BOOT_DEVICE_USB:' later and hitting a duplicate-case-label build error. > +/* Invalid Choices for the AM62Lx */ > +#define BOOT_DEVICE_MMC2_2 0x1F > +#define BOOT_DEVICE_ETHERNET 0x1F Same concern for these two - please note explicitly that they exist only to satisfy references in common code and are not real boot modes on AM62L. > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig > @@ -81,11 +90,9 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX > - default 0x43c3f290 if SOC_K3_AM625 > + default 0x43c3f290 if SOC_K3_AM625 || SOC_K3_AM62L3 || SOC_K3_AM62P5 || > SOC_K3_J722S > default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R > default 0x7000f290 if SOC_K3_AM62A7 && ARM64 > - default 0x43c4f290 if SOC_K3_AM62P5 > - default 0x43c7f290 if SOC_K3_J722S This coalescing changes the boot-param table index for AM62P5 (was 0x43c4f290) and J722S (was 0x43c7f290) to 0x43c3f290. That is a behavioural change for those SoCs, not just an AM62L addition, and it ideally belongs in a separate, clearly-justified patch - the commit message should at minimum explain why the old per-SoC addresses were wrong. Regards, Simon
