[PATCH v3 13/15] firmware: arm_scmi: Use bound iterators to minimize discovered rates

[Geert Uytterhoeven]

On Tue, 5 May 2026 at 11:59, Geert Uytterhoeven <geert at linux-m68k.org> wrote:
> On Tue, 28 Apr 2026 at 22:17, Cristian Marussi <cristian.marussi at arm.com> wrote:
> > Clock rates are guaranteed to be returned in ascending order for SCMI clock
> > protocol versions greater than 1.0: in such a case, use bounded iterators
> > to minimize the number of message exchanges needed to discover min and max
> > rate.
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi at arm.com>
>
> Thanks for your patch!
>
> > +static int
> > +scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph,
> > +                             u32 clk_id, struct clock_info *cinfo)
> > +{
> > +       struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id];
> > +       int ret;
> > +
> > +       /*
> > +        * Since only after SCMI Clock v1.0 the returned rates are guaranteed to
> > +        * be discovered in ascending order, lazy enumeration cannot be use for
> > +        * SCMI Clock v1.0 protocol.
> > +        */
> > +       if (PROTOCOL_REV_MAJOR(ph->version) > 0x1)
> > +               ret = scmi_clock_describe_rates_get_lazy(ph, clkd);
> > +       else
> > +               ret = scmi_clock_describe_rates_get_full(ph, clkd);
> > +
> > +       if (ret)
> > +               return ret;
> > +
> > +       clkd->info.min_rate = clkd->rates[RATE_MIN];
> >         if (!clkd->rate_discrete) {
> >                 clkd->info.max_rate = clkd->rates[RATE_MAX];
> >                 dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
> >                         clkd->rates[RATE_MIN], clkd->rates[RATE_MAX],
> >                         clkd->rates[RATE_STEP]);
> >         } else {
> > -               sort(clkd->rates, clkd->num_rates,
> > -                    sizeof(clkd->rates[0]), rate_cmp_func, NULL);
> >                 clkd->info.max_rate = clkd->rates[clkd->num_rates - 1];
> > +               dev_dbg(ph->dev, "Clock:%s DISCRETE:%d -> Min %llu Max %llu\n",
> > +                       clkd->info.name, clkd->rate_discrete,
> > +                       clkd->info.min_rate, clkd->info.max_rate);
>
> Printing clkd->rate_discrete is futile, as it is always 1.
> It would be more useful to print clkd->r.num_rates instead, although

clkd->tot_rates (sorry, my local tree still had your v2)

> that may still be lower than the actual value, due to lazy handling.

clkd->tot_rates is the actual value, so there is no such issue.

>
> >         }
> > -       clkd->info.min_rate = clkd->rates[RATE_MIN];
> >
> >         return 0;
> >  }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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