Re: [PATCH v2 2/5] clk: scmi: Add support for state control restricted clocks

Cristian Marussi
2024-04-08T18:26:14Z

On Sun, Apr 07, 2024 at 09:48:59PM -0700, Stephen Boyd wrote:
> Quoting Cristian Marussi (2024-03-25 14:00:22)
> > Some exposed SCMI Clocks could be marked as non-supporting state changes.
> > Configure a clk_ops descriptor which does not provide the state change
> > callbacks for such clocks when registering with CLK framework.
> >
> > CC: Michael Turquette <mturquette@baylibre.com>
> > CC: Stephen Boyd <sboyd@kernel.org>
> > CC: linux-clk@vger.kernel.org
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > drivers/clk/clk-scmi.c | 22 +++++++++++++++-------
> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
> > index d5d369b052bd..fc9603988d91 100644
> > --- a/drivers/clk/clk-scmi.c
> > +++ b/drivers/clk/clk-scmi.c
> > @@ -18,6 +18,7 @@
> >
> > enum scmi_clk_feats {
> > SCMI_CLK_ATOMIC_SUPPORTED,
> > + SCMI_CLK_STATE_CTRL_FORBIDDEN,
>
> Can it be positive, i.e. SCMI_CLK_STATE_CTRL_SUPPORTED?

Yes of course.

>
> > SCMI_CLK_MAX_FEATS
> > };
> >
> > @@ -230,15 +231,19 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
> > * only the prepare/unprepare API, as allowed by the clock framework
> > * when atomic calls are not available.
> > */
> > - if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED)) {
> > - ops->enable = scmi_clk_atomic_enable;
> > - ops->disable = scmi_clk_atomic_disable;
> > - ops->is_enabled = scmi_clk_atomic_is_enabled;
> > - } else {
> > - ops->prepare = scmi_clk_enable;
> > - ops->unprepare = scmi_clk_disable;
> > + if (!(feats_key & BIT(SCMI_CLK_STATE_CTRL_FORBIDDEN))) {
> > + if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED)) {
> > + ops->enable = scmi_clk_atomic_enable;
> > + ops->disable = scmi_clk_atomic_disable;
> > + } else {
> > + ops->prepare = scmi_clk_enable;
> > + ops->unprepare = scmi_clk_disable;
> > + }
> > }
> >
> > + if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED))
> > + ops->is_enabled = scmi_clk_atomic_is_enabled;
> > +
> > /* Rate ops */
> > ops->recalc_rate = scmi_clk_recalc_rate;
> > ops->round_rate = scmi_clk_round_rate;
> > @@ -288,6 +293,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
> > if (atomic_capable && ci->enable_latency <= atomic_threshold)
> > feats_key |= BIT(SCMI_CLK_ATOMIC_SUPPORTED);
> >
> > + if (ci->state_ctrl_forbidden)
>
> Then this is negated.
>

I will rework accordingly

Thanks,
Cristian