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

[Geert Uytterhoeven]

Hi Cristian,

On Fri, 27 Feb 2026 at 16:33, Cristian Marussi <cristian.marussi@xxxxxxx> wrote:
> 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
> 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@xxxxxxx>

Thanks for your patch!

> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -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) {
> +               *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);

step_size is u64, while do_div() truncates it to 32-bit.

> +
> +       *rate = ftmp * clk->range.step_size + fmin;
> +
> +       return 0;
> +}
> +
>  static int
>  scmi_clock_config_set(const struct scmi_protocol_handle *ph, u32 clk_id,
>                       enum clk_state state,

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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