Thread (48 messages) 48 messages, 6 authors, 2026-03-03

Re: [PATCH 01/11] firmware: arm_scmi: Add clock determine_rate operation

From: Cristian Marussi <cristian.marussi@arm.com>
Date: 2026-02-28 10:07:51
Also in: arm-scmi, linux-clk, linux-renesas-soc, lkml

On Fri, Feb 27, 2026 at 04:50:09PM +0000, Jonathan Cameron wrote:
On Fri, 27 Feb 2026 15:32:15 +0000
Cristian Marussi [off-list ref] wrote:
quoted
Add a clock operation to help determining the effective rate, closest to
the required one, that a specific clock can support.

Calculation is currently performed kernel side and the logic is taken
directly from the SCMI Clock driver: embedding the determinate rate logic
in the protocol layer enables semplifications in the SCMI Clock protocol
simplifications
quoted
interface and  will more easily accommodate further evolutions where such
determine_rate logic into is optionally delegated to the platform SCMI
server.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Hi Cristian,

Drive by review follows.  It's Friday afternoon an only a few mins to beer
o'clock :)
Thanks for having a look :P
quoted
---
Spoiler alert next SCMI spec will most probably include a new
CLOCK_DETERMINE_RATE command to delegate to the platform such calculations,
so this clock proto_ops will be needed anyway sooner or later
---
 drivers/firmware/arm_scmi/clock.c | 42 +++++++++++++++++++++++++++++++
 include/linux/scmi_protocol.h     |  6 +++++
 2 files changed, 48 insertions(+)
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index ab36871650a1..54e8b59c3941 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/limits.h>
 #include <linux/sort.h>
+#include <asm/div64.h>
 
 #include "protocols.h"
 #include "notify.h"
@@ -624,6 +625,46 @@ static int scmi_clock_rate_set(const struct scmi_protocol_handle *ph,
 	return ret;
 }
 
+static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
+				     u32 clk_id, unsigned long *rate)
+{
+	u64 fmin, fmax, ftmp;
+	struct scmi_clock_info *clk;
+	struct clock_info *ci = ph->get_priv(ph);
+
+	if (!rate)
+		return -EINVAL;
+
+	clk = scmi_clock_domain_lookup(ci, clk_id);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	/*
+	 * If we can't figure out what rate it will be, so just return the
+	 * rate back to the caller.
+	 */
+	if (clk->rate_discrete)
+		return 0;
+
+	fmin = clk->range.min_rate;
+	fmax = clk->range.max_rate;
+	if (*rate <= fmin) {
Does the rate ever end up different by doing this than it would if you
just dropped these short cuts? If not I wonder if this code complexity
is worthwhile vs

	*rate = clamp(*rate, clk->range.min_rate, clk->range.max_rate);

then carry on with the clamping to a step.

The only case I can immediately spot where it would be different would
be if (range.max_rate - range.min_rate) % range.step_size != 0
which smells like an invalid clock and could result in an out of
range rounding up anyway.
Yes indeed, but this patch aimed really ONLY at moving this logic from the
CLK SCMI driver into the SCMI Clock protocol layer since it then enables
more simplications...I have NOT really look into the logic...I suppose I
could optimize this in a following distinct patch...
quoted
+		*rate = fmin;
+		return 0;
+	} else if (*rate >= fmax) {
+		*rate = fmax;
+		return 0;
+	}
+
+	ftmp = *rate - fmin;
+	ftmp += clk->range.step_size - 1; /* to round up */
+	do_div(ftmp, clk->range.step_size);
+
+	*rate = ftmp * clk->range.step_size + fmin;
+
+	return 0;
+}
Thanks,
Cristian
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help