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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240726102103.dh6upqdeq2tj4hgn@dhruva>
Date: Fri, 26 Jul 2024 15:51:03 +0530
From: Dhruva Gole <d-gole@...com>
To: Peng Fan <peng.fan@....com>
CC: "Peng Fan (OSS)" <peng.fan@....nxp.com>,
        "sudeep.holla@....com"
	<sudeep.holla@....com>,
        "cristian.marussi@....com"
	<cristian.marussi@....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>,
        "vigneshr@...com" <vigneshr@...com>, "kamlesh@...com" <kamlesh@...com>
Subject: Re: [PATCH] clk: scmi: add is_prepared hook

On Jul 26, 2024 at 09:28:52 +0000, Peng Fan wrote:
> > Subject: Re: [PATCH] clk: scmi: add is_prepared hook
> > 
> > On Jul 25, 2024 at 17:07:41 +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@....com>
> > >
> > > Some clks maybe default enabled by hardware, so add is_prepared
> > hook
> > 
> > Why is_prepared when there is an is_enabled hook?
> 
> This patch is for non-atomic clk ops. The is_enabled hook is
> In atomic clk ops.
> 
> > See in the atomic case we already have something similar:
> > 
> > ops->is_enabled = scmi_clk_atomic_is_enabled;
> > 
> > > 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;
> > 
> > IMO from the decription and what the function is doing is_enabled
> > makes
> > more sense here to me, unless there's a better explanation.
> > 
> > Ref: linux/clk-provider.h
> > is_prepared: Queries the hardware to determine if the clock is prepared
> > vs
> > is_enabled: Queries the hardware to determine if the clock is enabled
> 
> SCMI firmware has no knowledge of prepare/unprepared.
> 
> As wrote in the beginning, this patch is for non atomic clk ops.

OK, I got carried away with the wording of is_prepared a bit but it
seems like prepare/unprepare are inter changeably used to enable/disable
in non atomic cases and so it makes sense to follow suit with is_prepared.

Thanks for clarifying,

Reviewed-by: Dhruva Gole <d-gole@...com>


-- 
Best regards,
Dhruva