[PATCH 4/7] drivers: clk: add support for RPMI clocks

[Charles Perry]

On Sat, Jun 27, 2026 at 01:47:17AM +0000, Yao Zi wrote:
> On Fri, Jun 26, 2026 at 01:15:45PM -0700, Charles Perry wrote:
> > The RISC-V Platform Management Interface (RPMI) defines a service group
> > for control and monitoring of clocks [1]. This can be exposed as a
> > UCLASS_CLK driver.
> > 
> > [1]: https://github.com/riscv-non-isa/riscv-rpmi (chapter 4.8)
> > 
> > Signed-off-by: Charles Perry <charles.perry at microchip.com>
> > ---
> >  MAINTAINERS            |   1 +
> >  drivers/clk/Kconfig    |   7 +
> >  drivers/clk/Makefile   |   1 +
> >  drivers/clk/clk_rpmi.c | 346 +++++++++++++++++++++++++++++++++++++++++
> >  include/rpmi_proto.h   |  39 +++++
> >  5 files changed, 394 insertions(+)
> >  create mode 100644 drivers/clk/clk_rpmi.c
> 
> ...
> 
> > diff --git a/drivers/clk/clk_rpmi.c b/drivers/clk/clk_rpmi.c
> > new file mode 100644
> > index 000000000000..d2efb057600b
> > --- /dev/null
> > +++ b/drivers/clk/clk_rpmi.c
> > @@ -0,0 +1,346 @@
> 
> ...
> 
> > +struct rpmi_clk_def {
> > +	u32 num_rates;
> > +	u32 transition_latency;
> 
> rpmi_clk_def.transition_latency only gets assigned in
> rpmi_clk_get_attrs() and never used. Since U-Boot's clock infrastructure
> doesn't care about it at all, maybe we could drop the field?

Ack.

> 
> > +	enum rpmi_clock_type type;
> > +	char name[RPMI_CLK_NAME_LEN + 1];
> > +};
> 
> ...
> 
> > +static int _rpmi_clk_enable_disable(struct rpmi_clk_priv *priv, u32 clkid,
> > +				    bool ena)
> > +{
> > +	struct rpmi_set_config_tx tx = {
> > +		.clkid = cpu_to_le32(clkid),
> > +		.config = cpu_to_le32(ena ? RPMI_CLK_STATE_ENABLED :
> > +					    RPMI_CLK_STATE_DISABLED),
> > +	};
> > +	__le32 rx;
> > +	int ret, status;
> > +
> > +	ret = rpmi_send_with_resp(&priv->chan, RPMI_CLK_SRV_SET_CONFIG, &tx,
> > +				  sizeof(tx), &rx, sizeof(rx), NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	status = le32_to_cpu(rx);
> > +	if (status)
> > +		return rpmi_to_linux_error(status);
> > +
> > +	return 0;
> 
> Please return rpmi_to_linux_error(status) and drop the if above.

Ack.

> 
> > +}
> > +
> > +static ulong _rpmi_clk_get_rate(struct rpmi_clk_priv *priv, u32 clkid)
> > +{
> > +	__le32 tx = cpu_to_le32(clkid);
> > +	struct rpmi_get_rate_rx rx;
> > +	int ret, status;
> > +
> > +	ret = rpmi_send_with_resp(&priv->chan, RPMI_CLK_SRV_GET_RATE, &tx,
> > +				  sizeof(tx), &rx, sizeof(rx), NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	status = le32_to_cpu(rx.status);
> > +	if (status)
> > +		return rpmi_to_linux_error(status);
> > +
> > +	return (((u64)(le32_to_cpu(rx.hi)) << 32) | (u32)(le32_to_cpu(rx.lo)));
> > +}
> 
> ...
> 
> > +static int _rpmi_clk_set_rate(struct rpmi_clk_priv *priv, u32 clkid,
> > +			      unsigned long rate)
> 
> This function is declared to return int, but...
> 
> > +{
> > +	struct rpmi_set_rate_tx tx = {
> > +		.clkid = cpu_to_le32(clkid),
> > +		.flags = 0,
> > +		.lo = cpu_to_le32(lower_32_bits(rate)),
> > +		.hi = cpu_to_le32(upper_32_bits(rate)),
> > +	};
> > +	__le32 rx;
> > +	int ret, status;
> > +
> > +	ret = rpmi_send_with_resp(&priv->chan, RPMI_CLK_SRV_SET_RATE, &tx,
> > +				  sizeof(tx), &rx, sizeof(rx), NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	status = le32_to_cpu(rx);
> > +	if (status)
> > +		return rpmi_to_linux_error(status);
> > +
> > +	return _rpmi_clk_get_rate(priv, clkid);
> 
> _rpmi_clk_get_rate() returns ulong. This unnecessarily truncates the
> rate on 64bit platform when it's above approximately 2.1GHz.

Good catch.

I'll keep the return code as int here and put the call to
_rpmi_clk_get_rate() inside rpmi_clk_set_rate() instead.

Thanks,
Charles