Re: [PATCH V2] clk: scmi: add is_prepared hook

[Dhruva Gole]

On Jul 26, 2024 at 21:10:07 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
> 
> 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@xxxxxxx>
> ---
> 
> 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@xxxxxx>


-- 
Best regards,
Dhruva