Re: [RFC PATCH 3/3] board: raspberrypi: Add Pico 2 support

Hi Bhargav,

On 2026-07-26T11:56:45, Bhargav Das <[email protected]> wrote:
> board: raspberrypi: Add Pico 2 support
>
> Add board support for the Raspberry Pi Pico 2.
>
> UART1 console is functional as of now.
>
> misc_init_r() sets a fixed value for the "serial#" environment variable.
> This runs after relocation, so a successful env_set() here confirms env
> works post-relocation.
>
> Signed-off-by: Bhargav Das <[email protected]>
>
> board/raspberrypi/pico2/Kconfig     | 15 +++++++
>  board/raspberrypi/pico2/MAINTAINERS |  9 ++++
>  board/raspberrypi/pico2/Makefile    |  3 ++
>  board/raspberrypi/pico2/pico2.c     | 83 +++++++++++++++++++++++++++++++++++++
>  configs/rpi_pico2_defconfig         | 29 +++++++++++++
>  include/configs/rpi_pico2.h         | 48 +++++++++++++++++++++
>  6 files changed, 187 insertions(+)

> diff --git a/configs/rpi_pico2_defconfig b/configs/rpi_pico2_defconfig
> @@ -0,0 +1,29 @@
> +CONFIG_CMD_TIMER=y
> +CONFIG_OF_CONTROL=y
> +# CONFIG_OF_UPSTREAM is not set
> +CONFIG_OF_EMBED=y
> +CONFIG_NO_NET=y
> +CONFIG_SPECIFY_CONSOLE_INDEX=y
> +CONFIG_DM_SERIAL=y
> +CONFIG_PL01X_SERIAL=y
> +CONFIG_TIMER=y

This enables CONFIG_TIMER (the DM timer framework) and
CONFIG_CMD_TIMER, but there is no timer node in the device tree (patch
2) and no DM timer driver selected here. The armv7m SysTick driver in
arch/arm/cpu/armv7m/systick-timer.c is gated on CONFIG_SYS_ARCH_TIMER,
not CONFIG_TIMER, so it is not built either. Just to check - how does
get_timer() work at runtime? I suspect you want CONFIG_SYS_ARCH_TIMER
for the SysTick, or a proper DM timer with a matching DT node. As it
stands the timer config looks inconsistent.

> diff --git a/include/configs/rpi_pico2.h b/include/configs/rpi_pico2.h
> @@ -0,0 +1,48 @@
> +/*
> + * Serial / Console
> + * RP2350 UART1 is PL011 compatible
> + * GPIO4 = TX, GPIO5 = RX
> + * Clock: XOSC 12MHz
> + */
> +#ifdef CONFIG_PL01X_SERIAL
> +#define CFG_PL011_CLOCK                      12000000
> +#define CFG_SYS_SERIAL0                      RP2350_UART1_BASE
> +#define CFG_PL01x_PORTS                      {(void *)CFG_SYS_SERIAL0}
> +#endif

This is inconsistent with the rest of the series. The early UART runs
CLK_PERI at 150MHz (patch 1 uses IBRD=81/FBRD=24 for that), and the DT
gives clock = <150000000>. Since you use DM_SERIAL with OF_CONTROL,
the pl01x driver reads plat->clock from the DT 'clock' property, so
CFG_PL011_CLOCK is only the fallback and the 12MHz value here is never
used - and the "Clock: XOSC 12MHz" comment is wrong. CFG_SYS_SERIAL0
and CFG_PL01x_PORTS are for the non-DM platdata path, which you do not
use. Please drop these, or at least fix the clock value so it does not
mislead.

> diff --git a/board/raspberrypi/pico2/pico2.c b/board/raspberrypi/pico2/pico2.c
> @@ -0,0 +1,83 @@
> +#ifdef CONFIG_MISC_INIT_R
> +int misc_init_r(void)
> +{
> +     /* Set a default serial number, rpi pico 2 part no */
> +     if (!env_get("serial#"))
> +             env_set("serial#", "SC1631");
> +
> +     return 0;
> +}
> +#endif

misc_init_r() is only called when CONFIG_MISC_INIT_R is set, so the
#ifdef guard is redundant - please drop it. BTW the commit message
says the chip has "a unique chip ID we can use as serial number", but
this writes a fixed string; that reads as a contradiction. Since it is
a placeholder, I would say so in the commit message.

Regards,
Simon