[PATCH V3] clk: scmi: add is_prepared hook

[PATCH V3] clk: scmi: add is_prepared hook

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Some clocks maybe default enabled by hardware. For clocks that don't
have users, that will be left in hardware default state, because prepare
count and enable count is zero,if there is no is_prepared hook to get
the hardware state. So add is_prepared hook to detect the hardware
state. Then when disabling the unused clocks, they can be simply
turned OFF to save power during kernel boot.

Reviewed-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V3:
 Update the commit log. See discussion:
 https://lore.kernel.org/all/20240802061234.njlviydzmjbsyteb@lcpd911/
V2:
 Provider helper __scmi_clk_is_enabled for atomic and non-atomic usage
 Move is_prepared hook out of SCMI_CLK_STATE_CTRL_SUPPORTED
 https://lore.kernel.org/all/20240802061234.njlviydzmjbsyteb@lcpd911/

 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)
 {
 	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;
 
 	/* Rate ops */
 	ops->recalc_rate = scmi_clk_recalc_rate;
-- 
2.37.1

RE: [PATCH V3] clk: scmi: add is_prepared hook

[Peng Fan]

Hi Stephen, Sudeep

> Subject: [PATCH V3] clk: scmi: add is_prepared hook

Not sure this patch belongs to clk tree or scmi tree. But please give a look
when you have time.

Thanks,
Peng.

> 
> From: Peng Fan <peng.fan@nxp.com>
> 
> Some clocks maybe default enabled by hardware. For clocks that don't
> have users, that will be left in hardware default state, because prepare
> count and enable count is zero,if there is no is_prepared hook to get
> the hardware state. So add is_prepared hook to detect the hardware
> state. Then when disabling the unused clocks, they can be simply
> turned OFF to save power during kernel boot.
> 
> Reviewed-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V3:
>  Update the commit log. See discussion:
> 
> https://lore.kernel.org/all/20240802061234.njlviydzmjbsyteb@lcpd91
> 1/
> V2:
>  Provider helper __scmi_clk_is_enabled for atomic and non-atomic
> usage  Move is_prepared hook out of
> SCMI_CLK_STATE_CTRL_SUPPORTED
> https://lore.kernel.org/all/20240802061234.njlviydzmjbsyteb@lcpd91
> 1/
> 
>  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)
>  {
>  	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;
> 
>  	/* Rate ops */
>  	ops->recalc_rate = scmi_clk_recalc_rate;
> --
> 2.37.1

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

[Sudeep Holla]

On Tue, Aug 06, 2024 at 10:56:01PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Some clocks maybe default enabled by hardware. For clocks that don't
> have users, that will be left in hardware default state, because prepare
> count and enable count is zero,if there is no is_prepared hook to get
> the hardware state. So add is_prepared hook to detect the hardware
> state. Then when disabling the unused clocks, they can be simply
> turned OFF to save power during kernel boot.
>

LGTM,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

IIUC, there is no dependency on any SCMI changes, so this can go alone
via clk tree.

-- 
Regards,
Sudeep

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

[Stephen Boyd]

Quoting Peng Fan (OSS) (2024-08-06 07:56:01)
> From: Peng Fan <peng.fan@nxp.com>
> 
> Some clocks maybe default enabled by hardware. For clocks that don't
> have users, that will be left in hardware default state, because prepare
> count and enable count is zero,if there is no is_prepared hook to get
> the hardware state. So add is_prepared hook to detect the hardware
> state. Then when disabling the unused clocks, they can be simply
> turned OFF to save power during kernel boot.
> 
> Reviewed-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Applied to clk-next