> Le 2 sept. 2026 à 12:30, Alex Bennée <[email protected]> a écrit :
>
> Gilles Grimaud <[email protected]> writes:
>
>> From: gilles grimaud <[email protected]>
>>
>> Model the RP2040 system and USB PLL register blocks and their clock outputs.
>> Add the clock mux, divider, selected-source and frequency-counter registers,
>> then route clk-sys to the Cortex-M0+ and clk-peri to both UARTs. Include
>> focused qtests for PLL configuration, clock switching and frequency
>> measurement.
>>
>> Signed-off-by: gilles grimaud <[email protected]>
>> ---
>> hw/arm/Kconfig | 2 +
> <snip>
>> +
>> +#define ATOMIC_ALIAS_MASK 0x3000
>> +#define ATOMIC_XOR 0x1000
>> +#define ATOMIC_SET 0x2000
>> +#define ATOMIC_CLR 0x3000
>
> These seem to be common defines - why not have a common 2040 header that
> they all use.
Agreed. I will add a common RP2040 header for the APB atomic aliases and
use its inline helper throughout the peripheral models in v3.
>
> <snip>
>> diff --git a/hw/misc/rp2040_pll.c b/hw/misc/rp2040_pll.c
>> new file mode 100644
>> index 0000000000..6fe2be6608
>> --- /dev/null
>> +++ b/hw/misc/rp2040_pll.c
>> @@ -0,0 +1,234 @@
> <snip>
>> +
>> +#define ATOMIC_ALIAS_MASK 0x3000
>> +#define ATOMIC_XOR 0x1000
>> +#define ATOMIC_SET 0x2000
>> +#define ATOMIC_CLR 0x3000
>> +
>> +static uint32_t rp2040_pll_apply_alias(uint32_t old, uint32_t value,
>> + hwaddr alias)
>> +{
>> + switch (alias) {
>> + case ATOMIC_XOR:
>> + return old ^ value;
>> + case ATOMIC_SET:
>> + return old | value;
>> + case ATOMIC_CLR:
>> + return old & ~value;
>> + default:
>> + return value;
>> + }
>
> This is repeated throughout the code, sometimes inline sometimes not.
> This screams for a common inline helper in the headers.
Yes, I will use a common inline helper throughout the RP2040 peripheral
models in v3.
>
>> +}
>> +
>> +static bool rp2040_pll_locked(RP2040PllState *s)
>> +{
>> + return !(s->pwr & (PLL_PWR_PD | PLL_PWR_VCOPD));
>> +}
>> +
>> +static unsigned rp2040_pll_output_hz(RP2040PllState *s)
>> +{
>> + uint32_t refdiv = s->cs & PLL_CS_REFDIV_MASK;
>> + uint32_t fbdiv = s->fbdiv_int & PLL_FBDIV_MASK;
>> + uint32_t postdiv1 = extract32(s->prim, 16, 3);
>> + uint32_t postdiv2 = extract32(s->prim, 12, 3);
>> + uint64_t hz;
>> +
>> + if (!rp2040_pll_locked(s) || (s->pwr & PLL_PWR_POSTDIVPD)) {
>> + return 0;
>> + }
>> +
>> + refdiv = refdiv ? refdiv : 1;
>> + if (s->cs & PLL_CS_BYPASS) {
>> + return XOSC_HZ / refdiv;
>> + }
>> +
>> + if (!fbdiv || !postdiv1 || !postdiv2) {
>> + return s->fallback_hz;
>> + }
>> +
>> + hz = XOSC_HZ;
>> + hz = hz * fbdiv / refdiv / postdiv1 / postdiv2;
>> + return hz;
>> +}
>> +
>> +static void rp2040_pll_update_clock(RP2040PllState *s)
>> +{
>> + clock_update_hz(s->clk, rp2040_pll_output_hz(s));
>> +}
>> +
>> +static uint64_t rp2040_pll_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> + RP2040PllState *s = opaque;
>> + hwaddr offset = addr & 0xfff;
>> + const char *name = s->trace_name ? s->trace_name : "rp2040.pll";
>> + uint64_t value;
>> +
>> + switch (offset) {
>> + case PLL_CS:
>> + value = s->cs & ~PLL_CS_LOCK;
>> + if (rp2040_pll_locked(s)) {
>> + value |= PLL_CS_LOCK;
>> + }
>> + break;
>> + case PLL_PWR:
>> + value = s->pwr;
>> + break;
>> + case PLL_FBDIV_INT:
>> + value = s->fbdiv_int;
>> + break;
>> + case PLL_PRIM:
>> + value = s->prim;
>> + break;
>> + default:
>> + value = 0;
>> + rp2040_log_unimplemented_read(name, size, s->base + addr, offset,
>> + value);
>> + break;
>> + }
>> +
>> + return value;
>> +}
>> +
>> +static void rp2040_pll_write(void *opaque, hwaddr addr,
>> + uint64_t value64, unsigned size)
>> +{
>> + RP2040PllState *s = opaque;
>> + hwaddr alias = addr & ATOMIC_ALIAS_MASK;
>> + hwaddr offset = addr & 0xfff;
>> + const char *name = s->trace_name ? s->trace_name : "rp2040.pll";
>> + uint32_t value = value64;
>> +
>> + switch (offset) {
>> + case PLL_CS:
>> + s->cs = rp2040_pll_apply_alias(s->cs, value, alias) &
>> + (PLL_CS_BYPASS | PLL_CS_REFDIV_MASK);
>> + break;
>> + case PLL_PWR:
>> + s->pwr = rp2040_pll_apply_alias(s->pwr, value, alias) &
>> PLL_PWR_MASK;
>> + break;
>> + case PLL_FBDIV_INT:
>> + s->fbdiv_int = rp2040_pll_apply_alias(s->fbdiv_int, value, alias) &
>> + PLL_FBDIV_MASK;
>> + break;
>> + case PLL_PRIM:
>> + s->prim = rp2040_pll_apply_alias(s->prim, value, alias) &
>> + PLL_PRIM_MASK;
>> + break;
>> + default:
>> + rp2040_log_unimplemented_write(name, size, s->base + addr, offset,
>> + value64);
>> + break;
>> + }
>> +
>> + rp2040_pll_update_clock(s);
>> +}
>> +
>> +static const MemoryRegionOps rp2040_pll_ops = {
>> + .read = rp2040_pll_read,
>> + .write = rp2040_pll_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> + .valid = {
>> + .min_access_size = 4,
>> + .max_access_size = 4,
>> + },
>> +};
>> +
>> +static void rp2040_pll_reset(DeviceState *dev)
>> +{
>> + RP2040PllState *s = RP2040_PLL(dev);
>> +
>> + s->cs = 1;
>> + s->pwr = PLL_PWR_MASK;
>> + s->fbdiv_int = 0;
>> + s->prim = (7 << 16) | (7 << 12);
>
> #defines for magic numbers please.
Agreed. I will replace these with named PLL post-divider fields and a
named reset value.
>
>> +
>> + rp2040_pll_update_clock(s);
>> +}
>> +
>> +static void rp2040_pll_init(Object *obj)
>> +{
>> + RP2040PllState *s = RP2040_PLL(obj);
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + s->clk = qdev_init_clock_out(dev, "clk");
>> + memory_region_init_io(&s->iomem, obj, &rp2040_pll_ops, s,
>> + "rp2040.pll", RP2040_PLL_SIZE);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
>> +}
>> +
>> +static const VMStateDescription rp2040_pll_vmstate = {
>> + .name = TYPE_RP2040_PLL,
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .fields = (const VMStateField[]) {
>> + VMSTATE_UINT32(cs, RP2040PllState),
>> + VMSTATE_UINT32(pwr, RP2040PllState),
>> + VMSTATE_UINT32(fbdiv_int, RP2040PllState),
>> + VMSTATE_UINT32(prim, RP2040PllState),
>> + VMSTATE_CLOCK(clk, RP2040PllState),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static const Property rp2040_pll_properties[] = {
>> + DEFINE_PROP_STRING("trace-name", RP2040PllState, trace_name),
>> + DEFINE_PROP_UINT32("base", RP2040PllState, base, 0),
>> + DEFINE_PROP_UINT32("fallback-hz", RP2040PllState, fallback_hz, 0),
>> +};
>> +
>> +static void rp2040_pll_class_init(ObjectClass *klass, const void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + device_class_set_legacy_reset(dc, rp2040_pll_reset);
>> + device_class_set_props(dc, rp2040_pll_properties);
>> + dc->vmsd = &rp2040_pll_vmstate;
>> +}
>> +
>> +static const TypeInfo rp2040_pll_info = {
>> + .name = TYPE_RP2040_PLL,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(RP2040PllState),
>> + .instance_init = rp2040_pll_init,
>> + .class_init = rp2040_pll_class_init,
>> +};
>> +
>> +static void rp2040_pll_register_types(void)
>> +{
>> + type_register_static(&rp2040_pll_info);
>> +}
>> +type_init(rp2040_pll_register_types)
>> diff --git a/include/hw/arm/rp2040.h b/include/hw/arm/rp2040.h
>> index 47a8cb2236..abdd2f006c 100644
>> --- a/include/hw/arm/rp2040.h
>> +++ b/include/hw/arm/rp2040.h
>> @@ -13,6 +13,8 @@
>> #include "hw/char/pl011.h"
>> #include "hw/core/clock.h"
>> #include "hw/core/sysbus.h"
>> +#include "hw/misc/rp2040_clocks.h"
>> +#include "hw/misc/rp2040_pll.h"
>> #include "hw/misc/rp2040_rosc.h"
>> #include "hw/misc/rp2040_syscfg.h"
>> #include "hw/misc/rp2040_sysinfo.h"
>> @@ -40,6 +42,9 @@ struct RP2040State {
>>
>> ARMv7MState armv7m;
>> PL011State uart[2];
>> + RP2040ClocksState clocks;
>> + RP2040PllState pll_sys;
>> + RP2040PllState pll_usb;
>> RP2040SysCfgState syscfg;
>> RP2040SysInfoState sysinfo;
>> RP2040RoscState rosc;
>> diff --git a/include/hw/misc/rp2040_clocks.h
>> b/include/hw/misc/rp2040_clocks.h
>> new file mode 100644
>> index 0000000000..ebbb14e3fa
>> --- /dev/null
>> +++ b/include/hw/misc/rp2040_clocks.h
>> @@ -0,0 +1,36 @@
>> +/*
>> + * RP2040 clocks emulation
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HW_MISC_RP2040_CLOCKS_H
>> +#define HW_MISC_RP2040_CLOCKS_H
>> +
>> +#include "hw/core/clock.h"
>> +#include "hw/core/sysbus.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_RP2040_CLOCKS "rp2040-clocks"
>> +OBJECT_DECLARE_SIMPLE_TYPE(RP2040ClocksState, RP2040_CLOCKS)
>> +
>> +#define RP2040_CLOCKS_BASE 0x40008000
>> +#define RP2040_CLOCKS_SIZE 0x4000
>> +
>> +struct RP2040ClocksState {
>> + SysBusDevice parent_obj;
>> +
>> + MemoryRegion iomem;
>> + Clock *clk_ref;
>> + Clock *clk_sys;
>> + Clock *clk_peri;
>> + Clock *clk_usb;
>> + Clock *clk_adc;
>> + Clock *clk_rtc;
>> + Clock *pll_sys;
>> + Clock *pll_usb;
>> +
>> + uint32_t regs[0x100 / 4];
>> +};
>> +
>> +#endif
>> diff --git a/include/hw/misc/rp2040_pll.h b/include/hw/misc/rp2040_pll.h
>> new file mode 100644
>> index 0000000000..ad188aa46e
>> --- /dev/null
>> +++ b/include/hw/misc/rp2040_pll.h
>> @@ -0,0 +1,37 @@
>> +/*
>> + * RP2040 PLL emulation
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HW_MISC_RP2040_PLL_H
>> +#define HW_MISC_RP2040_PLL_H
>> +
>> +#include "hw/core/clock.h"
>> +#include "hw/core/sysbus.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_RP2040_PLL "rp2040-pll"
>> +OBJECT_DECLARE_SIMPLE_TYPE(RP2040PllState, RP2040_PLL)
>> +
>> +#define RP2040_PLL_SYS_BASE 0x40028000
>> +#define RP2040_PLL_USB_BASE 0x4002c000
>> +#define RP2040_PLL_SIZE 0x4000
>> +
>> +struct RP2040PllState {
>> + SysBusDevice parent_obj;
>> +
>> + MemoryRegion iomem;
>> + Clock *clk;
>> +
>> + char *trace_name;
>> + uint32_t base;
>> + uint32_t fallback_hz;
>> +
>> + uint32_t cs;
>> + uint32_t pwr;
>> + uint32_t fbdiv_int;
>> + uint32_t prim;
>> +};
>> +
>> +#endif
>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>> index 441f6f294a..416d90ae94 100644
>> --- a/tests/qtest/meson.build
>> +++ b/tests/qtest/meson.build
>> @@ -256,6 +256,7 @@ qtests_arm = \
>> (config_all_devices.has_key('CONFIG_VEXPRESS') ? ['test-arm-mptimer'] :
>> []) + \
>> (config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) +
>> \
>> (config_all_devices.has_key('CONFIG_RASPI_PICO') ?
>> ['rp2040-sysinfo-syscfg-test',
>> + 'rp2040-clocks-test',
>> 'rp2040-rosc-test',
>> 'rp2040-tbman-test',
>> 'rp2040-vreg-test'] :
>> []) + \
>> diff --git a/tests/qtest/rp2040-clocks-test.c
>> b/tests/qtest/rp2040-clocks-test.c
>> new file mode 100644
>> index 0000000000..39e49eb57f
>> --- /dev/null
>> +++ b/tests/qtest/rp2040-clocks-test.c
>> @@ -0,0 +1,107 @@
>> +/*
>> + * QTest testcase for the RP2040 clocks block.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqtest.h"
>> +#include "qemu/bitops.h"
>> +
>> +#define CLOCKS_BASE 0x40008000
>> +#define CLK_SYS_CTRL 0x3c
>> +#define CLK_SYS_DIV 0x40
>> +#define CLK_PERI_CTRL 0x48
>> +#define CLK_PERI_DIV 0x4c
>> +#define CLK_USB_CTRL 0x54
>> +#define CLK_USB_DIV 0x58
>> +#define CLK_ADC_CTRL 0x60
>> +#define CLK_ADC_DIV 0x64
>> +#define CLK_RTC_CTRL 0x6c
>> +#define CLK_RTC_DIV 0x70
>> +#define FC0_SRC 0x94
>> +#define FC0_STATUS 0x98
>> +#define FC0_RESULT 0x9c
>> +
>> +#define PLL_SYS_BASE 0x40028000
>> +#define PLL_USB_BASE 0x4002c000
>> +#define PLL_CS 0x00
>> +#define PLL_PWR 0x04
>> +#define PLL_FBDIV_INT 0x08
>> +#define PLL_PRIM 0x0c
>> +
>> +#define PLL_PWR_BITS (BIT(5) | BIT(3) | BIT(2) | BIT(0))
>> +#define CLK_CTRL_ENABLE BIT(11)
>> +#define FC0_STATUS_DONE BIT(4)
>
> Again you can re-use include definitions.
Yes.
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
