linux-kernel - Re: [PATCH V2] 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: <20240726134414.2ctbtt53sd3lyfjl@dhruva>
Date: Fri, 26 Jul 2024 19:14:14 +0530
From: Dhruva Gole <d-gole@...com>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
CC: <sudeep.holla@....com>, <cristian.marussi@....com>,
        <mturquette@...libre.com>, <sboyd@...nel.org>,
        <linux-clk@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <arm-scmi@...r.kernel.org>,
        Peng Fan <peng.fan@....com>
Subject: Re: [PATCH V2] clk: scmi: add is_prepared hook

On Jul 26, 2024 at 21:10:07 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@....com>
> 
> Some clks maybe default enabled by hardware, so add is_prepared hook
> for non-atomic clk_ops 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>
> ---
> 
> V2:
>  Provider helper __scmi_clk_is_enabled for atomic and non-atomic usage
>  Move is_prepared hook out of SCMI_CLK_STATE_CTRL_SUPPORTED
> 
>  drivers/clk/clk-scmi.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
> index d86a02563f6c..15510c2ff21c 100644
> --- a/drivers/clk/clk-scmi.c
> +++ b/drivers/clk/clk-scmi.c
> @@ -156,13 +156,13 @@ static void scmi_clk_atomic_disable(struct clk_hw *hw)
>  	scmi_proto_clk_ops->disable(clk->ph, clk->id, ATOMIC);
>  }
>  
> -static int scmi_clk_atomic_is_enabled(struct clk_hw *hw)
> +static int __scmi_clk_is_enabled(struct clk_hw *hw, bool atomic)

I think we can combine other atomic/non atomic in the same way no?
Let me know if I should send a follow up patch based on this to make
__scmi_clk_enable(hw,atomic) and __scmi_clk_disable(hw,atomic)

I'd be more than happy to do so.

>  {
>  	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, ATOMIC);
> +	ret = scmi_proto_clk_ops->state_get(clk->ph, clk->id, &enabled, atomic);
>  	if (ret)
>  		dev_warn(clk->dev,
>  			 "Failed to get state for clock ID %d\n", clk->id);
> @@ -170,6 +170,16 @@ static int scmi_clk_atomic_is_enabled(struct clk_hw *hw)
>  	return !!enabled;
>  }
>  
> +static int scmi_clk_atomic_is_enabled(struct clk_hw *hw)
> +{
> +	return __scmi_clk_is_enabled(hw, ATOMIC);
> +}
> +
> +static int scmi_clk_is_enabled(struct clk_hw *hw)
> +{
> +	return __scmi_clk_is_enabled(hw, NOT_ATOMIC);
> +}
> +
>  static int scmi_clk_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
>  {
>  	int ret;
> @@ -285,6 +295,8 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
>  
>  	if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED))
>  		ops->is_enabled = scmi_clk_atomic_is_enabled;
> +	else
> +		ops->is_prepared = scmi_clk_is_enabled;

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


-- 
Best regards,
Dhruva