Add OEM extension for clk-scmi

Add OEM extension for clk-scmi

[Peng Fan]

Hi All,

This is to discuss the spread spectrum support for clk-scmi.  I think
it is hard to provide a generic framework from scmi core level.
Because oem extension varies per protocol. And most of the
code added is in consumer driver side saying clk-scmi.c not
the firmware/arm_scmi/clock.c.

Based on above, I am thinking to follow what arm-smmu
implements(arm_smmu_impl_init), by calling a function
scmi_clk_oem_init in probe.

And define a generic feature macro in enum scmi_clk_feats:
SCMI_CLK_EXT_OEM_SSC_SUPPORTED,

Also define a new structure for OEM stuff:
+struct scmi_clk_plat {
+       int (*query_oem_feats)(const struct scmi_protocol_handle *ph,
+                              u32 id, unsigned long *feats_key);
+       int (*set_spread_spectrum)(struct clk_hw *hw,
+                                  const struct clk_spread_spectrum *ss_conf);
+};
+

Add a new file clk-scmi-oem for up plat hooks implementation.

And add clk-scmi.h to move some code that would be shared
between clk-scmi.c and clk-scmi-oem.c

This is just an idea, more code pieces as below, this does not
pass compiling, just an idea in my mind.

If you think this is feasible, I will do a formal patch.

BTW: I will be on holiday in the following week. I will check
email from time to time, but may not respond in time.
 
drivers/clk/clk-scmi-oem.c
static int scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
                                            struct clk_spread_spectrum *ss_conf)
{
        /* TODO */
        return 0;
}

static int scmi_clk_imx_query_oem_feats(const struct scmi_protocol_handle *ph,
                                        u32 id, unsigned long *feats_key)
{
        /* TODO */
        if ( the clk supports SSC)
              *feats_key |= SCMI_CLK_EXT_OEM_SSC_SUPPORTED;
        return 0;
}

static const struct scmi_clk_plat scmi_clk_imx_plat = {
        .query_ext_oem_feats = scmi_clk_imx_query_oem_feats,
        .set_spread_spectrum = scmi_clk_imx_set_spread_spectrum,
};

int scmi_clk_oem_init(struct scmi_device *dev, struct scmi_clk_plat *data)
{
        if (!strcmp(handle->version->vendor_id, SCMI_IMX_VENDOR) &&
            !strcmp(handle->version->sub_vendor_id, SCMI_IMX_SUBVENDOR)) {
                *data = scmi_clk_imx_plat;
        } else {
                /* Other SoC vendors (TI) */
        }

        return 0;
}

In clk-scmi.c, using the hook to update oem feats
@@ -375,6 +388,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
                                                 &val, NULL, false);
                if (!ret)
                        feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
+
+               if (data->query_ext_oem_feats)
+                       data->query_ext_oem_feats(sclk->ph, sclk->id, &feats_key);
        }

@@ -313,11 +322,15 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
                ops->set_duty_cycle = scmi_clk_set_duty_cycle;
        }
 
+       if (feats_key & BIT(SCMI_CLK_EXT_OEM_SSC_SUPPORTED))
+               ops->set_spread_spectrum = data->set_spread_spectrum;
+
        return ops;
 }

Thanks,
Peng.

Re: Add OEM extension for clk-scmi

[Sebin Francis]

Hi Peng

On 30/09/25 18:52, Peng Fan wrote:
> Hi All,
> 
> This is to discuss the spread spectrum support for clk-scmi.  I think
> it is hard to provide a generic framework from scmi core level.
> Because oem extension varies per protocol. And most of the
> code added is in consumer driver side saying clk-scmi.c not
> the firmware/arm_scmi/clock.c.
> 
> Based on above, I am thinking to follow what arm-smmu
> implements(arm_smmu_impl_init), by calling a function
> scmi_clk_oem_init in probe.
> 
> And define a generic feature macro in enum scmi_clk_feats:
> SCMI_CLK_EXT_OEM_SSC_SUPPORTED,
> 
> Also define a new structure for OEM stuff:
> +struct scmi_clk_plat {
> +       int (*query_oem_feats)(const struct scmi_protocol_handle *ph,
> +                              u32 id, unsigned long *feats_key);
> +       int (*set_spread_spectrum)(struct clk_hw *hw,
> +                                  const struct clk_spread_spectrum *ss_conf);
> +};
> +
> 
> Add a new file clk-scmi-oem for up plat hooks implementation.
> 
> And add clk-scmi.h to move some code that would be shared
> between clk-scmi.c and clk-scmi-oem.c
> 
> This is just an idea, more code pieces as below, this does not
> pass compiling, just an idea in my mind.
> 
> If you think this is feasible, I will do a formal patch.
> 
> BTW: I will be on holiday in the following week. I will check
> email from time to time, but may not respond in time.
>   
> drivers/clk/clk-scmi-oem.c
> static int scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
>                                              struct clk_spread_spectrum *ss_conf)
> {
>          /* TODO */
>          return 0;
> }
> 
> static int scmi_clk_imx_query_oem_feats(const struct scmi_protocol_handle *ph,
>                                          u32 id, unsigned long *feats_key)
> {
>          /* TODO */
>          if ( the clk supports SSC)
>                *feats_key |= SCMI_CLK_EXT_OEM_SSC_SUPPORTED;
>          return 0;
> }
> 
> static const struct scmi_clk_plat scmi_clk_imx_plat = {
>          .query_ext_oem_feats = scmi_clk_imx_query_oem_feats,
>          .set_spread_spectrum = scmi_clk_imx_set_spread_spectrum,
> };
> 
> int scmi_clk_oem_init(struct scmi_device *dev, struct scmi_clk_plat *data)
> {
>          if (!strcmp(handle->version->vendor_id, SCMI_IMX_VENDOR) &&
>              !strcmp(handle->version->sub_vendor_id, SCMI_IMX_SUBVENDOR)) {
>                  *data = scmi_clk_imx_plat;
>          } else {
>                  /* Other SoC vendors (TI) */
>          }
> 
>          return 0;
> }
> 
> In clk-scmi.c, using the hook to update oem feats
> @@ -375,6 +388,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
>                                                   &val, NULL, false);
>                  if (!ret)
>                          feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
> +
> +               if (data->query_ext_oem_feats)
> +                       data->query_ext_oem_feats(sclk->ph, sclk->id, &feats_key);
>          }
> 
> @@ -313,11 +322,15 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
>                  ops->set_duty_cycle = scmi_clk_set_duty_cycle;
>          }
>   
> +       if (feats_key & BIT(SCMI_CLK_EXT_OEM_SSC_SUPPORTED))
> +               ops->set_spread_spectrum = data->set_spread_spectrum;
> +
>          return ops;
>   }
> 
> Thanks,
> Peng.

Thanks for proposing the new approach. As approach gives each OEM add 
there plat specific handlers, this approach looks good to me.

Thanks
Sebin

Re: Add OEM extension for clk-scmi

[Cristian Marussi]

On Tue, Sep 30, 2025 at 01:22:15PM +0000, Peng Fan wrote:
> Hi All,
> 

Hi,

> This is to discuss the spread spectrum support for clk-scmi.  I think
> it is hard to provide a generic framework from scmi core level.
> Because oem extension varies per protocol. And most of the
> code added is in consumer driver side saying clk-scmi.c not
> the firmware/arm_scmi/clock.c.
> 
> Based on above, I am thinking to follow what arm-smmu
> implements(arm_smmu_impl_init), by calling a function
> scmi_clk_oem_init in probe.
> 
> And define a generic feature macro in enum scmi_clk_feats:
> SCMI_CLK_EXT_OEM_SSC_SUPPORTED,
> 
> Also define a new structure for OEM stuff:
> +struct scmi_clk_plat {
> +       int (*query_oem_feats)(const struct scmi_protocol_handle *ph,
> +                              u32 id, unsigned long *feats_key);
> +       int (*set_spread_spectrum)(struct clk_hw *hw,
> +                                  const struct clk_spread_spectrum *ss_conf);
> +};
> +
> 
> Add a new file clk-scmi-oem for up plat hooks implementation.
> 
> And add clk-scmi.h to move some code that would be shared
> between clk-scmi.c and clk-scmi-oem.c
> 
> This is just an idea, more code pieces as below, this does not
> pass compiling, just an idea in my mind.
> 
> If you think this is feasible, I will do a formal patch.
> 
> BTW: I will be on holiday in the following week. I will check
> email from time to time, but may not respond in time.
>  
> drivers/clk/clk-scmi-oem.c
> static int scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
>                                             struct clk_spread_spectrum *ss_conf)
> {
>         /* TODO */
>         return 0;
> }
> 
> static int scmi_clk_imx_query_oem_feats(const struct scmi_protocol_handle *ph,
>                                         u32 id, unsigned long *feats_key)
> {
>         /* TODO */
>         if ( the clk supports SSC)
>               *feats_key |= SCMI_CLK_EXT_OEM_SSC_SUPPORTED;
>         return 0;
> }
> 
> static const struct scmi_clk_plat scmi_clk_imx_plat = {
>         .query_ext_oem_feats = scmi_clk_imx_query_oem_feats,
>         .set_spread_spectrum = scmi_clk_imx_set_spread_spectrum,
> };
> 
> int scmi_clk_oem_init(struct scmi_device *dev, struct scmi_clk_plat *data)
> {
>         if (!strcmp(handle->version->vendor_id, SCMI_IMX_VENDOR) &&
>             !strcmp(handle->version->sub_vendor_id, SCMI_IMX_SUBVENDOR)) {
>                 *data = scmi_clk_imx_plat;
>         } else {
>                 /* Other SoC vendors (TI) */
>         }
> 
>         return 0;
> }
> 
> In clk-scmi.c, using the hook to update oem feats
> @@ -375,6 +388,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
>                                                  &val, NULL, false);
>                 if (!ret)
>                         feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
> +
> +               if (data->query_ext_oem_feats)
> +                       data->query_ext_oem_feats(sclk->ph, sclk->id, &feats_key);
>         }
> 
> @@ -313,11 +322,15 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
>                 ops->set_duty_cycle = scmi_clk_set_duty_cycle;
>         }
>  
> +       if (feats_key & BIT(SCMI_CLK_EXT_OEM_SSC_SUPPORTED))
> +               ops->set_spread_spectrum = data->set_spread_spectrum;
> +
>         return ops;
>  }

In general seems a sensible approach...once you had a full series we can
reason about it better probably....and see how much more (or less) we
can/should generalize.

Thanks for this,
Cristian

RE: Add OEM extension for clk-scmi

[Peng Fan]

Hi Cristian, Sebin

> Subject: Re: Add OEM extension for clk-scmi
> 
> > +               ops->set_spread_spectrum = data->set_spread_spectrum;
> > +
> >         return ops;
> >  }
> 
> In general seems a sensible approach...once you had a full series we
> can reason about it better probably....and see how much more (or less)
> we can/should generalize.

I reply both here.  I will post the patches together with the clk ssc
enablement in clk-conf.c this week.

Thanks,
Peng.

> 
> Thanks for this,
> Cristian