| Message ID | 20240806145601.1184337-1-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [V3] clk: scmi: add is_prepared hook | expand |
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
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.
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
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;