Hi Bhargav, On 2026-07-26T11:56:45, Bhargav Das <[email protected]> wrote: > arm: Add RP2350 architecture support > > Add initial U-Boot architecture support for the RP2350 SoC. > > RP2350 specs: > - CPU: Dual-core Arm Cortex-M33 or dual-core RISC-V Hazard3 @ 150MHz. > - Memory: 520 KB on-chip SRAM, 4 MB on-board QSPI flash. > > Only Arm core support is added here. > > The architecture is added under arch/arm/mach-rp2xxx/. > > This patch implements early SoC bring-up: > - XOSC (12MHz crystal) startup > - PLL_SYS configuration to 150MHz > - CLK_REF/CLK_SYS/CLK_PERI clock switching > - UART1 early console init (115200 8N1) > > Cortex-M33 is an Armv8-M core, but U-Boot does not yet have a CPU_V8M > architecture variant. CPU_V7M's startup and exception handling are close > [...] > > arch/arm/Kconfig | 7 + > arch/arm/Makefile | 2 + > arch/arm/include/asm/arch-rp2xxx/rp2350.h | 307 > ++++++++++++++++++++++++++++++ > arch/arm/mach-rp2xxx/Kconfig | 58 ++++++ > arch/arm/mach-rp2xxx/Makefile | 4 + > arch/arm/mach-rp2xxx/image_def.S | 23 +++ > arch/arm/mach-rp2xxx/rp2350.c | 261 +++++++++++++++++++++++++ > arch/arm/mach-rp2xxx/u-boot.lds | 94 +++++++++ > 8 files changed, 756 insertions(+)
> diff --git a/arch/arm/mach-rp2xxx/rp2350.c b/arch/arm/mach-rp2xxx/rp2350.c > @@ -0,0 +1,261 @@ > + disable_mpu(); > + > + for (i = 0; i < ARRAY_SIZE(rp2350_mpu_config); i++) > + mpu_config(&rp2350_mpu_config[i]); > + > + enable_mpu(); This calls into arch/arm/cpu/armv7m/mpu.c, which programs the PMSAv7 MPU: it writes the region size and attributes into MPU_RASR. The Cortex-M33 is Armv8-M and uses the PMSAv8 MPU, which as I understand it has no RASR - regions are described with a base in MPU_RBAR and a limit in MPU_RLAR, with attributes indexed through MPU_MAIR0/1. So these writes do not configure the MPU as the comment describes; I suspect the region setup is silently ineffective and the board boots only because the default memory map is already usable. The commit message argues CPU_V7M's startup and exception handling are close enough, but that reasoning does not extend to the MPU register model. Please can you either drop the MPU setup for now, or note clearly that it is a no-op pending real Armv8-M support. What do you think? > diff --git a/arch/arm/mach-rp2xxx/rp2350.c b/arch/arm/mach-rp2xxx/rp2350.c > @@ -0,0 +1,261 @@ > + * Region 0: XIP Flash 0x10000000 - executable, read-only, cacheable > + * Region 1: SRAM 0x20000000 - read/write, cacheable > + * Region 2: Peripherals 0x40000000 - read/write, device (non-cacheable) > + */ > + > +int print_cpuinfo(void) he comment documents three regions, but rp2350_mpu_config[] only sets up two (a single 512MB region covering ROM/flash/SRAM, and one for peripherals) - please update the comment to match. Also this block is the header for arch_cpu_init(), yet print_cpuinfo() sits between it and the function it describes. Please move print_cpuinfo() out so the comment sits directly above arch_cpu_init(). > diff --git a/arch/arm/mach-rp2xxx/rp2350.c b/arch/arm/mach-rp2xxx/rp2350.c > @@ -0,0 +1,261 @@ > + writel(0, &pll->pwr); /* Power everything on */ > + writel(PLL_PWR_POSTDIVPD | PLL_PWR_DSMPD, &pll->pwr); /* Keep post > dividers and DSM powered down for now, only VCO on */ Does the first write actually do something useful? The second line is also well over 80 columns - please move the comment above the statement. > diff --git a/arch/arm/mach-rp2xxx/rp2350.c b/arch/arm/mach-rp2xxx/rp2350.c > @@ -0,0 +1,261 @@ > + clrsetbits_le32(clk_base + 0x30, 0x3, CLK_REF_SRC_XOSC); > + while (!(readl(clk_base + 0x38) & BIT(CLK_REF_SRC_XOSC))) > + ; The clocks code pokes bare offsets (0x30, 0x38, 0x3c, 0x44, 0x48) with magic bit positions inline, which is hard to verify against the datasheet. The cover letter notes struct rp2350_clocks_regs is defined but unused; as it stands it does not even contain the CLK_REF/CLK_SYS/CLK_PERI registers used here. Please can you extend that struct to cover these and access them by name, or at least add named offset/field defines in rp2350.h, rather than open-coding the offsets? I don't have one of these boards in my lab but I'm looking forward to it! Regards, Simon
