Re: [PATCH 11/11] firmware: arm_scmi: Introduce all_rates_get clock operation

[Cristian Marussi <cristian.marussi@arm.com>]

On Mon, Mar 02, 2026 at 03:18:30PM +0800, Peng Fan wrote:
> On Sat, Feb 28, 2026 at 10:47:20AM +0000, Cristian Marussi wrote:
> >On Sat, Feb 28, 2026 at 10:49:47AM +0800, Peng Fan wrote:
> >> On Fri, Feb 27, 2026 at 03:32:25PM +0000, Cristian Marussi wrote:
> >> >Add a clock operation to get the whole set of rates available to a specific
> >> >clock: when needed this request could transparently trigger a full rate
> >> >discovery enumeration if this specific clock-rates were previously only
> >> >lazily enumerated.
> >> >

Hi,

> >> >Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >> >---
> >> > drivers/firmware/arm_scmi/clock.c | 85 +++++++++++++++++++++----------
> >> > include/linux/scmi_protocol.h     |  9 ++++
> >> > 2 files changed, 67 insertions(+), 27 deletions(-)

[snip]

> >> > 
> >> >-	if (clkd->rate_discrete)
> >> >-		sort(clkd->rates, clkd->num_rates,
> >> >-		     sizeof(clkd->rates[0]), rate_cmp_func, NULL);
> >> >+	if (clkd->r.rate_discrete && PROTOCOL_REV_MAJOR(ph->version) == 0x1)
> >> 
> >> Not understand well "PROTOCOL_REV_MAJOR(ph->version) == 0x1", I may
> >> get something wrong, should use ">="?
> >
> >I have NOT double checked BUT I think fro Etienne original patch, you
> >can assume that clock rates are returned by the platform in ascending
> >order (already sorted) only after Clock protocol version 0x01, the first
> >ever, so ONLY when teh used version is 0x1 we must perform a full scan
> >(no lazy optimization since we cannot assume that rates[last-1] is max
> >AND we must sort the obtained list of clocks...
> 
> Per https://developer.arm.com/documentation/den0056/c/?lang=en, SCMI version
> 3.0, CLOCK protocol version is 1.0, I see:
> "The clock rates returned by this call should be in numeric ascending order".
> 
> SCMI 2.0 also has it. But SCMI 1.0 does not have above line.
> 
> All the above specs are using CLOCK protocol version 1.0.
> 
> It might be overshoot, should the "PROTOCOL_REV_MAJOR(ph->version) == 0x1"
> be dropped, so always sort the array?

Yes there is a 'glitch' in the specs v1.0/v2.0/v3.0 since all reports
Clock proto version 0x10000 BUT the clarification around the ordering of
the returned rates is NOT always there, so, since we must be absolutely
pedantic to guarantee compatibility with any possible deployed SCMI/FW,
we cannot assume that any 0x10000 protocol return ordered rates: this
means that if proto==0x10000 we MUST sort AND we cannot play smart with
the new bound iterators since we are NOT assure that rates[0] AND
rates[last -1] contain respectively min and max rates...

On the other side for any protocol version != 0x10000 (i.e. v3.1/v3.2/v4.0)
we can assume those rates as ordered and so, on one side, we can run 'lazy'
partial enumerations AND, on the other side, we can avoid the overhead of
sorting with full scans enumerations.

Thanks,
Cristian