Re: [PATCH 03/11] firmware: arm_scmi: Simplify clock rates exposed interface

[Geert Uytterhoeven]

Hi Cristian,

Thanks for your patch!

On Fri, 27 Feb 2026 at 16:33, Cristian Marussi <cristian.marussi@xxxxxxx> wrote:
> Move needlessly exposed fields away from scmi_clock_info into the new
> internal struct scmi_clock_desc while keeping exposed only the two new
> min_rate and max_rate fields for each clock.
>
> No functional change.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>

> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -157,13 +157,27 @@ struct scmi_clock_rate_notify_payld {
>         __le32 rate_high;
>  };
>
> +struct scmi_clock_desc {
> +       u32 id;
> +       bool rate_discrete;
> +       unsigned int num_rates;
> +       u64 rates[SCMI_MAX_NUM_RATES];
> +#define        RATE_MIN        0
> +#define        RATE_MAX        1
> +#define        RATE_STEP       2

Any specific reason you are not using a union here, like in
scmi_clock_info?

> +       struct scmi_clock_info info;
> +};

> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -51,6 +51,8 @@ struct scmi_clock_info {
>         bool rate_ctrl_forbidden;
>         bool parent_ctrl_forbidden;
>         bool extended_config;
> +       u64 min_rate;
> +       u64 max_rate;
>         union {
>                 struct {
>                         int num_rates;

You patch description read like the actual rates would be moved
from scmi_clock_info to scmi_clock_desc, i.e. _removed_ here?

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