linux-kernel - RE: [PATCH] clk: scmi: add is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB8459D7FF7393CEF70105CC2F88B42@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Fri, 26 Jul 2024 12:35:00 +0000
From: Peng Fan <peng.fan@....com>
To: Cristian Marussi <cristian.marussi@....com>, "Peng Fan (OSS)"
	<peng.fan@....nxp.com>
CC: "sudeep.holla@....com" <sudeep.holla@....com>, "mturquette@...libre.com"
	<mturquette@...libre.com>, "sboyd@...nel.org" <sboyd@...nel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "arm-scmi@...r.kernel.org"
	<arm-scmi@...r.kernel.org>
Subject: RE: [PATCH] clk: scmi: add is_prepared hook

> Subject: Re: [PATCH] clk: scmi: add is_prepared hook
> 
> On Thu, Jul 25, 2024 at 05:07:41PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@....com>
> >
> 
> .... one more remark..
> 
> > Some clks maybe default enabled by hardware, so add is_prepared
> hook
> > to get the status of the clk. Then when disabling unused clks, those
> > unused clks but default hardware on clks could be in off state to save
> > power.
> >
> > Signed-off-by: Peng Fan <peng.fan@....com>
> > ---
> >  drivers/clk/clk-scmi.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index
> > d86a02563f6c..d2d370337ba5 100644
> > --- a/drivers/clk/clk-scmi.c
> > +++ b/drivers/clk/clk-scmi.c
> > @@ -142,6 +142,20 @@ static void scmi_clk_disable(struct clk_hw
> *hw)
> >  	scmi_proto_clk_ops->disable(clk->ph, clk->id, NOT_ATOMIC);  }
> >
> > +static int scmi_clk_is_enabled(struct clk_hw *hw) {
> > +	int ret;
> > +	bool enabled = false;
> > +	struct scmi_clk *clk = to_scmi_clk(hw);
> > +
> > +	ret = scmi_proto_clk_ops->state_get(clk->ph, clk->id, &enabled,
> NOT_ATOMIC);
> > +	if (ret)
> > +		dev_warn(clk->dev,
> > +			 "Failed to get state for clock ID %d\n", clk-
> >id);
> > +
> > +	return !!enabled;
> > +}
> > +
> >  static int scmi_clk_atomic_enable(struct clk_hw *hw)  {
> >  	struct scmi_clk *clk = to_scmi_clk(hw); @@ -280,6 +294,7
> @@
> > scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
> >  		} else {
> >  			ops->prepare = scmi_clk_enable;
> >  			ops->unprepare = scmi_clk_disable;
> > +			ops->is_prepared = scmi_clk_is_enabled;
> 
> ... you should NOT add the is_prepared ops here, since this would
> mean
> that you will have the is_prepared ops available only when
> SCMI_CLK_STATE_CTRL_SUPPORTED, WHILE you most probably want
> to be able
> to check (non atomically) the current enabled-status for a clock EVEN
> IF
> you are allowed to change its state...please add the is_prepared in the
> else-branch down below where the ATOMIC is_enabled is added

Will use this in v2. And together try to avoid duplicate code.
        if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED))
                ops->is_enabled = scmi_clk_atomic_is_enabled;
+       else
+               ops->is_prepared = scmi_clk_is_enabled;
 
        /* Rate ops */

Thanks,
Peng.

> 
> Thanks,
> Cristian