[PATCH v4 00/15] SCMI Clock rates discovery rework

Cristian Marussi posted 15 patches 1 month ago
drivers/clk/clk-scmi.c                |  48 +---
drivers/firmware/arm_scmi/clock.c     | 301 ++++++++++++++++++++------
drivers/firmware/arm_scmi/driver.c    |  80 +++++--
drivers/firmware/arm_scmi/protocols.h |  13 +-
include/linux/scmi_protocol.h         |  30 +--
5 files changed, 324 insertions(+), 148 deletions(-)
[PATCH v4 00/15] SCMI Clock rates discovery rework
Posted by Cristian Marussi 1 month ago
Hi,

it was a known limitation, in the SCMI Clock protocol support, the lack of
dynamic allocation around per-clock rates discovery: fixed size statically
per-clock rates arrays did not scale and was increasingly a waste of memory
(see [1]).

This series aim at solving this in successive steps:

 - simplify and reduce to the minimum possible the rates data info exposed
   to the SCMI driver by scmi_clock_info
 - move away from static fixed allocation of per-clock rates arrays in
   favour of a completely dynamic runtime allocation: just allocate what
   is needed based on the effectively discovered

This is done in patches 2-6.

A further bigger optimization suggested in a past series [2] by Etienne
would be, whenever allowed by the spec, to limit upfront the number of
queries in order to simply retrieve min and max rate, that are indeed the
only rates needed by the CLK SCMI driver.

The approach proposed in [1] was open coding and duplicating some of the
functionalities already provided by SCMI iterators, though.

Patch 7-14 implement such optimization instead by:

 - reworking core SCMI iterators to support bound enumerations
 - use such new bound iterators to perform the minimum number of queries
   in order to only retrieve min an max rate

As a final result now the rates enumeration triggered by the CLK SCMI
driver, while still allocating for all the existent rates, miminize the
number of SCMI CLK_DESCRIBE_RATE messages needed to obtain min and max.

Finally, patch 15 introduces a new clock protocol operation to be able to
trigger anytime on demand a full enumeration and obtain the full list of
rates when needed, not only min/max: this latter method is really only used
currently by some dowstream SCMI Test driver of mine.

Most notably in V3 I had:

 - picked up Geert fixes on V2: these could have been squashed in the
   original series while maintaining proper Geert's authorship but as of
   now I have simply picked them up and changed their order to be near the
   commit they fix

 - dropped the "Harden Clock protocol initialization" patch that caused a
   number of out-of-spec vendor FW to break

Based on v7.1-rc2. 
Tested on ARM/JUNO, RADXA/ROCK5B and an emulated environment.

Any feeback welcome.

Thanks,
Cristian

[1]: https://lore.kernel.org/arm-scmi/aZsX-oplR6fiLBBN@pluto/T/#t
[2]: https://lore.kernel.org/20241203173908.3148794-2-etienne.carriere@foss.st.com
---
v3 -->v4
 - Rebased on v7.1-rc2
 - Removed unused info.rate_discrete [Geert]
 - Made dev_dbg() more meaningful by printing tot_rates [Geert]
 - Fixed build bisectability by renaming properly to iter_response_bound_cleanup()

v2 --> v3
 - Rebased on v7.1-rc1
 - Picked up Geert fixes
 - Dropped patch breaking out-of-spec FW (Harden Clock Protocol init)
 - Collected Reviewed tags

v1 --> v2
 - Rebased on v7.0-rc3
 - Added a Fixes patch to rectify bug in rounding algo
 - Removed useless parenthesis in macros
 - Collected a few Reviewed-by tags
 - Clarified commit message


Cristian Marussi (12):
  clk: scmi: Fix clock rate rounding
  firmware: arm_scmi: Add clock determine_rate operation
  clk: scmi: Use new determine_rate clock operation
  firmware: arm_scmi: Simplify clock rates exposed interface
  clk: scmi: Use new simplified per-clock rate properties
  firmware: arm_scmi: Drop unused clock rate interfaces
  firmware: arm_scmi: Make clock rates allocation dynamic
  firmware: arm_scmi: Harden clock parents discovery
  firmware: arm_scmi: Refactor iterators internal allocation
  firmware: arm_scmi: Add bound iterators support
  firmware: arm_scmi: Use bound iterators to minimize discovered rates
  firmware: arm_scmi: Introduce all_rates_get clock operation

Geert Uytterhoeven (3):
  firmware: arm_scmi: Fix bound iterators returning too many items
  firmware: arm_scmi: Use proper iter_response_bound_cleanup() name
  firmware: arm_scmi: Fix OOB in scmi_clock_describe_rates_get_lazy()

 drivers/clk/clk-scmi.c                |  48 +---
 drivers/firmware/arm_scmi/clock.c     | 301 ++++++++++++++++++++------
 drivers/firmware/arm_scmi/driver.c    |  80 +++++--
 drivers/firmware/arm_scmi/protocols.h |  13 +-
 include/linux/scmi_protocol.h         |  30 +--
 5 files changed, 324 insertions(+), 148 deletions(-)

-- 
2.53.0
Re: [PATCH v4 00/15] SCMI Clock rates discovery rework
Posted by Geert Uytterhoeven 1 month ago
Hi Cristian,

On Fri, 8 May 2026 at 17:33, Cristian Marussi <cristian.marussi@arm.com> wrote:
> it was a known limitation, in the SCMI Clock protocol support, the lack of
> dynamic allocation around per-clock rates discovery: fixed size statically
> per-clock rates arrays did not scale and was increasingly a waste of memory
> (see [1]).

[...]

> v3 -->v4
>  - Rebased on v7.1-rc2
>  - Removed unused info.rate_discrete [Geert]
>  - Made dev_dbg() more meaningful by printing tot_rates [Geert]
>  - Fixed build bisectability by renaming properly to iter_response_bound_cleanup()

Thanks for the update!

I believe you still have a possible runtime bisectability issue
between "[PATCH v4 04/15] firmware: arm_scmi: Simplify clock
rates exposed interface" and "[PATCH v4 05/15] clk: scmi: Use new
simplified per-clock rate properties": 04/15 removes the last setter
of scmi_clock_info.rate_discrete, before 05/15 removes the last getter.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH v4 00/15] SCMI Clock rates discovery rework
Posted by Sudeep Holla 1 month ago
On Fri, May 08, 2026 at 07:25:49PM +0200, Geert Uytterhoeven wrote:
> Hi Cristian,
> 
> On Fri, 8 May 2026 at 17:33, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > it was a known limitation, in the SCMI Clock protocol support, the lack of
> > dynamic allocation around per-clock rates discovery: fixed size statically
> > per-clock rates arrays did not scale and was increasingly a waste of memory
> > (see [1]).
> 
> [...]
> 
> > v3 -->v4
> >  - Rebased on v7.1-rc2
> >  - Removed unused info.rate_discrete [Geert]
> >  - Made dev_dbg() more meaningful by printing tot_rates [Geert]
> >  - Fixed build bisectability by renaming properly to iter_response_bound_cleanup()
> 
> Thanks for the update!
> 
> I believe you still have a possible runtime bisectability issue
> between "[PATCH v4 04/15] firmware: arm_scmi: Simplify clock
> rates exposed interface" and "[PATCH v4 05/15] clk: scmi: Use new
> simplified per-clock rate properties": 04/15 removes the last setter
> of scmi_clock_info.rate_discrete, before 05/15 removes the last getter.
> 

I have fixed this up by adding some initialisation in 04/15 and removing it
in 06/15. Cristian, if possible can you check if the functionality will
remain intact after 05/15 ?

-- 
Regards,
Sudeep
Re: [PATCH v4 00/15] SCMI Clock rates discovery rework
Posted by Cristian Marussi 1 month ago
On Mon, May 11, 2026 at 09:09:40AM +0100, Sudeep Holla wrote:
> On Fri, May 08, 2026 at 07:25:49PM +0200, Geert Uytterhoeven wrote:
> > Hi Cristian,
> > 
> > On Fri, 8 May 2026 at 17:33, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > > it was a known limitation, in the SCMI Clock protocol support, the lack of
> > > dynamic allocation around per-clock rates discovery: fixed size statically
> > > per-clock rates arrays did not scale and was increasingly a waste of memory
> > > (see [1]).
> > 
> > [...]
> > 
> > > v3 -->v4
> > >  - Rebased on v7.1-rc2
> > >  - Removed unused info.rate_discrete [Geert]
> > >  - Made dev_dbg() more meaningful by printing tot_rates [Geert]
> > >  - Fixed build bisectability by renaming properly to iter_response_bound_cleanup()
> > 
> > Thanks for the update!
> > 
> > I believe you still have a possible runtime bisectability issue
> > between "[PATCH v4 04/15] firmware: arm_scmi: Simplify clock
> > rates exposed interface" and "[PATCH v4 05/15] clk: scmi: Use new
> > simplified per-clock rate properties": 04/15 removes the last setter
> > of scmi_clock_info.rate_discrete, before 05/15 removes the last getter.
> > 
> 
> I have fixed this up by adding some initialisation in 04/15 and removing it
> in 06/15. Cristian, if possible can you check if the functionality will
> remain intact after 05/15 ?

LGTM.

Thanks,
Cristian
Re: [PATCH v4 00/15] SCMI Clock rates discovery rework
Posted by Sudeep Holla 3 weeks, 3 days ago
On Fri, 08 May 2026 16:32:45 +0100, Cristian Marussi wrote:
> it was a known limitation, in the SCMI Clock protocol support, the lack of
> dynamic allocation around per-clock rates discovery: fixed size statically
> per-clock rates arrays did not scale and was increasingly a waste of memory
> (see [1]).
> 
> This series aim at solving this in successive steps:
> 
> [...]

Applied to sudeep.holla/linux (for-next/scmi/updates), thanks!

[01/15] clk: scmi: Fix clock rate rounding
        https://git.kernel.org/sudeep.holla/c/d0c81a38d06d
[02/15] firmware: arm_scmi: Add clock determine_rate operation
        https://git.kernel.org/sudeep.holla/c/ecde921eb460
[03/15] clk: scmi: Use new determine_rate clock operation
        https://git.kernel.org/sudeep.holla/c/af86c99170b7
[04/15] firmware: arm_scmi: Simplify clock rates exposed interface
        https://git.kernel.org/sudeep.holla/c/0d76f62613ca
[05/15] clk: scmi: Use new simplified per-clock rate properties
        https://git.kernel.org/sudeep.holla/c/cdcd2fc94936
[06/15] firmware: arm_scmi: Drop unused clock rate interfaces
        https://git.kernel.org/sudeep.holla/c/2e757f71a5ab
[07/15] firmware: arm_scmi: Make clock rates allocation dynamic
        https://git.kernel.org/sudeep.holla/c/62ba967595e0
[08/15] firmware: arm_scmi: Harden clock parents discovery
        https://git.kernel.org/sudeep.holla/c/bda40491e0ce
[09/15] firmware: arm_scmi: Refactor iterators internal allocation
        https://git.kernel.org/sudeep.holla/c/e99ed7267263
[10/15] firmware: arm_scmi: Add bound iterators support
        https://git.kernel.org/sudeep.holla/c/4848d07ea9fc
[11/15] firmware: arm_scmi: Fix bound iterators returning too many items
        https://git.kernel.org/sudeep.holla/c/ae4a088f13de
[12/15] firmware: arm_scmi: Use proper iter_response_bound_cleanup() name
        https://git.kernel.org/sudeep.holla/c/3065e26dac52
[13/15] firmware: arm_scmi: Use bound iterators to minimize discovered rates
        https://git.kernel.org/sudeep.holla/c/26d04d592a47
[14/15] firmware: arm_scmi: Fix OOB in scmi_clock_describe_rates_get_lazy()
        https://git.kernel.org/sudeep.holla/c/4a07036d6159
[15/15] firmware: arm_scmi: Introduce all_rates_get clock operation
        https://git.kernel.org/sudeep.holla/c/d2488ff1a257
--
Regards,
Sudeep