On Mon, Oct 26, 2026 at 12:49:39PM +0530, Subrahmanya Lingappa wrote:
> Charles,
>
> On Wed, Oct 14, 2026 at 7:47 PM Charles Perry
> <[email protected]> wrote:
> >
> > Create a new transport layer that sends the messages to a simulated RPMI
> > firmware.
> >
> > For now, this only contains a simulation of the clock service group
> > which is implemented using code from librpmi [1] and adapted.
> >
> > Also add a test in test/dm that makes use of the RPMI clock driver.
> >
> > [1]:
> > https://github.com/riscv-software-src/librpmi/blob/3f3c4ca47339a8795432e260f17e1fc5091c9a3a/lib/rpmi_service_group_clock.c
> >
> > Signed-off-by: Charles Perry <[email protected]>
> > ---
> > Changes in v4: None
> > Changes in v3: None
> >
> > Changes in v2:
> > - Reorder headers
> > ---
> > MAINTAINERS | 1 +
> > arch/sandbox/dts/test.dts | 12 +
> > configs/sandbox_defconfig | 2 +
> > drivers/firmware/rpmi/Kconfig | 3 +-
> > drivers/firmware/rpmi/Makefile | 1 +
> > drivers/firmware/rpmi/rpmi-sandbox-clock.c | 446
> > +++++++++++++++++++++++++++++
> > drivers/firmware/rpmi/rpmi-sandbox.c | 156 ++++++++++
> > drivers/firmware/rpmi/rpmi-sandbox.h | 47 +++
> > test/dm/Makefile | 1 +
> > test/dm/rpmi.c | 48 ++++
> > 10 files changed, 716 insertions(+), 1 deletion(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cf45df9a027a..6ca7709c64b7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1564,6 +1564,7 @@ S: Maintained
> > F: drivers/clk/clk_rpmi.c
> > F: drivers/firmware/rpmi/
> > F: include/rpmi*
> > +F: test/dm/rpmi.c
> >
> > SANDBOX
> > M: Simon Glass <[email protected]>
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index 25c99ef81472..d7998fa0da4a 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -882,6 +882,18 @@
> > sm: secure-monitor {
> > compatible = "sandbox,sm";
> > };
> > +
> > + rpmi: rpmi {
> > + compatible = "sandbox,rpmi-transport";
> > + #mbox-cells = <1>;
> > + };
> > +
> > + rpmi_clock: rpmi-clock {
> > + compatible = "riscv,rpmi-clock";
> > + mboxes = <&rpmi 0x8>;
> > + #clock-cells = <1>;
> > + clocks = <&rpmi_clock 0>, <&rpmi_clock 1>,
> > <&rpmi_clock 2>, <&rpmi_clock 3>;
> > + };
> > };
> >
> > fpga {
> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > index a2708e6fb333..52df7cc08b14 100644
> > --- a/configs/sandbox_defconfig
> > +++ b/configs/sandbox_defconfig
> > @@ -198,6 +198,7 @@ CONFIG_BUTTON_GPIO=y
> > CONFIG_CLK=y
> > CONFIG_CLK_K210=y
> > CONFIG_CLK_K210_SET_RATE=y
> > +CONFIG_CLK_RPMI=y
> > CONFIG_SANDBOX_CLK_CCF=y
> > CONFIG_CLK_SCMI=y
> > CONFIG_CPU=y
> > @@ -214,6 +215,7 @@ CONFIG_FASTBOOT_FLASH=y
> > CONFIG_FASTBOOT_FLASH_MMC_DEV=0
> > CONFIG_ARM_FFA_TRANSPORT=y
> > CONFIG_SCMI_FIRMWARE=y
> > +CONFIG_RPMI_FIRMWARE=y
> > CONFIG_FPGA_ALTERA=y
> > CONFIG_FPGA_STRATIX_II=y
> > CONFIG_FPGA_STRATIX_V=y
> > diff --git a/drivers/firmware/rpmi/Kconfig b/drivers/firmware/rpmi/Kconfig
> > index 421a1fa9deba..b462d3693987 100644
> > --- a/drivers/firmware/rpmi/Kconfig
> > +++ b/drivers/firmware/rpmi/Kconfig
> > @@ -1,6 +1,7 @@
> > config RPMI_FIRMWARE
> > bool "Enable RPMI support"
> > - depends on RISCV && DM && OF_CONTROL
> > + depends on RISCV || SANDBOX
> > + depends on DM && OF_CONTROL
> > select FIRMWARE
> > help
> > RISC-V Platform Management Interface (RPMI) defines an OS-agnostic
> > diff --git a/drivers/firmware/rpmi/Makefile b/drivers/firmware/rpmi/Makefile
> > index 4f4bcb929db4..150e98a72df9 100644
> > --- a/drivers/firmware/rpmi/Makefile
> > +++ b/drivers/firmware/rpmi/Makefile
> > @@ -1,3 +1,4 @@
> > obj-y += rpmi-uclass.o
> > obj-$(CONFIG_RPMI_SBI_MPXY) += rpmi-sbi-mpxy.o
> > obj-$(CONFIG_RPMI_SHMEM) += rpmi-shmem.o
> > +obj-$(CONFIG_SANDBOX) += rpmi-sandbox.o rpmi-sandbox-clock.o
> > diff --git a/drivers/firmware/rpmi/rpmi-sandbox-clock.c
> > b/drivers/firmware/rpmi/rpmi-sandbox-clock.c
> > new file mode 100644
> > index 000000000000..fa205d067b24
> > --- /dev/null
> > +++ b/drivers/firmware/rpmi/rpmi-sandbox-clock.c
> > @@ -0,0 +1,446 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2024 Ventana Micro Systems Inc.
> > + * Copyright (C) 2026 Microchip Technology Inc. All rights reserved.
> > + */
> > +
> > +#define LOG_CATEGORY UCLASS_RPMI
> > +
> > +#include <asm/types.h>
> > +#include <asm/io.h>
> > +#include <asm/byteorder.h>
> > +#include <linux/kernel.h>
> > +#include <linux/string.h>
> > +#include <rpmi.h>
> > +#include "rpmi-sandbox.h"
> > +
> > +struct rpmi_clock {
> > + enum rpmi_clock_type type;
> > + const char *name;
> > + u32 rate_count;
> > + const u64 *rate_array;
> > + u32 transition_latency;
> > + enum rpmi_clock_state state;
> > + u64 rate;
> > +};
> > +
> > +const u64 discrete_rates[] = { 20000000, 333333333, 500000000, 1000000000
> > };
> > +
> > +const u64 linear_rates[] = { 50000000, 100000000, 1000000 };
> > +
> > +#define DISCRETE_CLOCK(name, current_state, current_rate) \
> > + { RPMI_CLK_TYPE_DISCRETE, \
> > + name, \
> > + ARRAY_SIZE(discrete_rates), \
> > + discrete_rates, \
> > + 0, \
> > + current_state, \
> > + current_rate }
> > +
> > +#define LINEAR_CLOCK(name, current_state, current_rate) \
> > + { RPMI_CLK_TYPE_LINEAR, name, ARRAY_SIZE(linear_rates), \
> > + linear_rates, 0, current_state, \
> > + current_rate }
> > +
> > +struct rpmi_clock clock_simulation[] = {
> > + DISCRETE_CLOCK("CLK0", RPMI_CLK_STATE_DISABLED, discrete_rates[0]),
> > + DISCRETE_CLOCK("CLK1", RPMI_CLK_STATE_DISABLED, discrete_rates[1]),
> > + DISCRETE_CLOCK("CLK2", RPMI_CLK_STATE_ENABLED, discrete_rates[2]),
> > + LINEAR_CLOCK("CLK3", RPMI_CLK_STATE_ENABLED, linear_rates[0]),
> > +};
> > +
> > +static u32 clock_count = ARRAY_SIZE(clock_simulation);
> > +
> > +static struct rpmi_clock *rpmi_get_clock(u32 clkid)
> > +{
> > + if (clkid >= clock_count)
> > + return NULL;
> > +
> > + return &clock_simulation[clkid];
> > +}
> > +
> > +static enum rpmi_error_codes
> > +rpmi_clock_set_rate(u32 clkid, enum rpmi_clock_rate_match match, u64 rate)
> > +{
> > + struct rpmi_clock *clk = rpmi_get_clock(clkid);
> > +
> > + if (!clk)
> > + return RPMI_ERR_INVALID_PARAM;
> > +
> > + clk->rate = rate;
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes rpmi_clock_set_state(u32 clkid,
> > + enum rpmi_clock_state
> > state)
> > +{
> > + struct rpmi_clock *clk = rpmi_get_clock(clkid);
> > +
> > + if (!clk)
> > + return RPMI_ERR_INVALID_PARAM;
> > +
> > + clk->state = state;
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes rpmi_clock_get_state(u32 clkid,
> > + enum rpmi_clock_state
> > *state)
> > +{
> > + struct rpmi_clock *clk = rpmi_get_clock(clkid);
> > +
> > + if (!clk || !state)
> > + return RPMI_ERR_INVALID_PARAM;
> > +
> > + *state = clk->state;
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes rpmi_clock_get_rate(u32 clkid, u64 *rate)
> > +{
> > + struct rpmi_clock *clk = rpmi_get_clock(clkid);
> > +
> > + if (!clk || !rate)
> > + return RPMI_ERR_INVALID_PARAM;
> > +
> > + *rate = clk->rate;
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes
> > +rpmi_clock_sg_get_num_clocks(struct rpmi_transport *trans, u16
> > request_datalen,
> > + const u8 *request_data, u16 *response_datalen,
> > + u8 *response_data)
> > +{
> > + u32 *resp = (void *)response_data;
> > +
> > + resp[0] = cpu_to_le32((u32)RPMI_SUCCESS);
> > + resp[1] = cpu_to_le32(clock_count);
> > +
> > + *response_datalen = 2 * sizeof(*resp);
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes
> > +rpmi_clock_sg_get_attributes(struct rpmi_transport *trans, u16
> > request_datalen,
> > + const u8 *request_data, u16 *response_datalen,
> > + u8 *response_data)
> > +{
> > + struct rpmi_clock *clk;
> > + u16 resp_dlen;
> > + u32 flags = 0;
> > +
> > + u32 *resp = (void *)response_data;
> > +
> > + u32 clkid = cpu_to_le32(((const u32 *)request_data)[0]);
> > +
> > + clk = rpmi_get_clock(clkid);
> > + if (!clk) {
> > + resp_dlen = sizeof(*resp);
> > + resp[0] = cpu_to_le32((u32)RPMI_ERR_INVALID_PARAM);
> > + goto done;
> > + }
> > +
> > + /* encode CLOCK_FORMAT */
> > + if (clk->type == RPMI_CLK_TYPE_LINEAR)
> > + flags |= 1;
> > +
> > + resp[3] = cpu_to_le32(clk->transition_latency);
> > + resp[2] = cpu_to_le32(clk->rate_count);
> > + resp[1] = cpu_to_le32(flags);
> > + resp[0] = cpu_to_le32((u32)RPMI_SUCCESS);
> > +
> > + if (clk->name)
> > + strlcpy((char *)&resp[4], clk->name, RPMI_CLK_NAME_LEN);
> > +
> > + resp_dlen = 8 * sizeof(*resp);
> > +
> > +done:
> > + *response_datalen = resp_dlen;
> > +
> > + return RPMI_SUCCESS;
> > +}
> > +
> > +static enum rpmi_error_codes
> > +rpmi_clock_sg_get_supp_rates(struct rpmi_transport *trans, u16
> > request_datalen,
> > + const u8 *request_data, u16 *response_datalen,
> > + u8 *response_data)
> > +{
> > + u32 i = 0, j = 0;
> > + u32 rate_count;
> > + u32 resp_dlen = 0, clk_rate_idx = 0;
> > + u32 max_rates, remaining = 0, returned = 0;
> > + const u64 *rate_array;
> > + struct rpmi_clock *clk;
> > +
> > + u32 *resp = (void *)response_data;
> > +
> > + u32 clkid = cpu_to_le32(((const u32 *)request_data)[0]);
> > +
> > + clk = rpmi_get_clock(clkid);
> > + if (clk) {
> > + resp_dlen = sizeof(*resp);
> > + resp[0] = cpu_to_le32((u32)RPMI_ERR_INVALID_PARAM);
> > + goto done;
> > + }
> This condition is inverted: a valid clock ID returns
> `RPMI_ERR_INVALID_PARAM`, while an invalid ID leaves `clk == NULL` and is
> dereferenced on the next line. Please change this to `if (!clk)`.
Oh, that's bad...
>
> > +
> > + rate_count = clk->rate_count;
> > + rate_array = clk->rate_array;
> > + if (!rate_count || !rate_array) {
> > + resp_dlen = sizeof(*resp);
> > + resp[0] = cpu_to_le32((u32)RPMI_ERR_NOTSUPP);
> > + goto done;
> > + }
...
> > diff --git a/test/dm/Makefile b/test/dm/Makefile
> > index 5dd6b57bbe84..c6372fa40583 100644
> > --- a/test/dm/Makefile
> > +++ b/test/dm/Makefile
> > @@ -118,6 +118,7 @@ obj-$(CONFIG_CMD_RKMTD) += rkmtd.o
> > obj-$(CONFIG_$(PHASE_)DM_RNG) += rng.o
> > obj-$(CONFIG_DM_RTC) += rtc.o
> > obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> > +obj-$(CONFIG_RPMI_FIRMWARE) += rpmi.o
> > obj-$(CONFIG_SCSI) += scsi.o
> > obj-$(CONFIG_DM_SERIAL) += serial.o
> > obj-$(CONFIG_DM_SPI_FLASH) += sf.o
> > diff --git a/test/dm/rpmi.c b/test/dm/rpmi.c
> > new file mode 100644
> > index 000000000000..7cb4fd11a46d
> > --- /dev/null
> > +++ b/test/dm/rpmi.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2026 Microchip Technology Inc. All rights reserved.
> > + */
> > +
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/test.h>
> > +#include <rpmi.h>
> > +#include <stdio.h>
> > +#include <test/ut.h>
> > +
> > +static int dm_test_rpmi_clock(struct unit_test_state *uts)
> > +{
> > + struct udevice *rpmi_dev, *clk_dev;
> > + struct clk clk0, clk1, clk2, clk3;
> > +
> > + ut_assertok(uclass_get_device_by_name(UCLASS_RPMI, "rpmi",
> > &rpmi_dev));
> > +
> > + ut_assertok(
> > + uclass_get_device_by_name(UCLASS_CLK, "rpmi-clock",
> > &clk_dev));
> > +
> > + // Grab clock 0 and retrieve the rate
> > + ut_assertok(clk_get_by_index(clk_dev, 0, &clk0));
> > + ut_asserteq_64(20000000, clk_get_rate(&clk0));
> > +
> > + // Change to 1GHz
> > + ut_asserteq(1000000000, clk_set_rate(&clk0, 1000000000));
>
> The test does not exercise the supported-rates path above. Please add a
> regression test that sends `GET_SUPPORTED_RATES` through the sandbox
> transport
> for both a valid clock ID and an out-of-range ID. The current
> get-rate/set-rate
> test passes without invoking this service. Enable/disable coverage can be
> added separately; shared-memory and MPXY command tests belong with their
> respective transport/command harnesses.
The RPMI clock driver doesn't even use the GET_SUPPORTED_RATES service as
of now. I'll add the ->round_rate() callback for completeness so that we
can test this service.
Thanks,
Charles
>
> Regards,
> Subrahmanya
>
