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

[Cristian Marussi]

On Mon, Mar 02, 2026 at 01:48:00PM +0100, Geert Uytterhoeven wrote:
> Hi Cristian,
> 
> Thanks for your patch!

Hi,

> 
> 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?
>

No, I will fix in v2.
 
> > +       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?
> 

Yes they will be removed later in the series to avoid breaking the
build...I will add a _removed_ tag for clarity (that TIL that exist :D)

Thanks,
Cristian