Hi,
a small series to review how the SCMI Clock driver chooses and sets up the
CLK operations to associate to a clock when registering with CLK framework.
SCMI clocks exposed by the platform sports a growing number of clock
properties since SCMI v3.2: discovered SCMI clocks could be restricted in
terms of capability to set state/rate/parent/duty_cycle and the platform
itself can have a varying support in terms of atomic support.
Knowing upfront which operations are NOT allowed on some clocks helps
avoiding needless message exchanges.
As a result, the SCMI Clock driver, when registering resources with the
CLK framework, aims to provide only the specific clk_ops as known to be
certainly supported by the specific SCMI clock resource.
Using static pre-compiled clk_ops structures to fulfill all the possible
(and possibly growing) combinations of clock features is cumbersome and
error-prone (there are 32 possible combinations as of now to account for
the above mentioned clock features variation).
This rework introduces a dynamic allocation mechanism to be able to
configure the required clk_ops at run-time when the SCMI clocks are
enumerated.
Only one single clk_ops is generated (per driver instance) for each of the
features combinations effectively found in the set of returned SCMI
resources.
Once this preliminary rework is done in 1/5, the following patches use this
new clk_ops schema to introduce a number of restricted clk_ops depending on
the specific retrieved SCMI clocks characteristics.
Based on v6.9-rc1
Thanks,
Cristian
v2 -> v3
- moving scmi_clk_ops_db from being global to a per-instance/per-probe
structure to avoid sharing devm_ allocated clk_ops between different
driver instances.
- using bits.h macros
- fixed a few dox comments
- explicit unit in atomic_threshold_us
- added a runtime size-check before accessing scmi_clk_ops_db using feats_key
- reworked scmi_clk_ops_alloc call to reduce nesting
- using transport_is_atomic instead of is_atomic to be clearer
- using SCMI_<feats>_SUPPORTED instead of SCMI<feats>_FORBIDDEN
v1 -> V2
- rebased on v6.9-rc1
Cristian Marussi (5):
clk: scmi: Allocate CLK operations dynamically
clk: scmi: Add support for state control restricted clocks
clk: scmi: Add support for rate change restricted clocks
clk: scmi: Add support for re-parenting restricted clocks
clk: scmi: Add support for get/set duty_cycle operations
drivers/clk/clk-scmi.c | 249 +++++++++++++++++++++++++++++++++--------
1 file changed, 201 insertions(+), 48 deletions(-)
--
2.44.0
Provide the CLK framework callbacks related to get/set clock duty cycle if
the related SCMI clock supports OEM extended configurations.
CC: Michael Turquette <[email protected]>
CC: Stephen Boyd <[email protected]>
CC: [email protected]
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/clk/clk-scmi.c | 49 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index ce0f26ee632f..d86a02563f6c 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -22,6 +22,7 @@ enum scmi_clk_feats {
SCMI_CLK_STATE_CTRL_SUPPORTED,
SCMI_CLK_RATE_CTRL_SUPPORTED,
SCMI_CLK_PARENT_CTRL_SUPPORTED,
+ SCMI_CLK_DUTY_CYCLE_SUPPORTED,
SCMI_CLK_FEATS_COUNT
};
@@ -169,6 +170,45 @@ static int scmi_clk_atomic_is_enabled(struct clk_hw *hw)
return !!enabled;
}
+static int scmi_clk_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
+{
+ int ret;
+ u32 val;
+ struct scmi_clk *clk = to_scmi_clk(hw);
+
+ ret = scmi_proto_clk_ops->config_oem_get(clk->ph, clk->id,
+ SCMI_CLOCK_CFG_DUTY_CYCLE,
+ &val, NULL, false);
+ if (!ret) {
+ duty->num = val;
+ duty->den = 100;
+ } else {
+ dev_warn(clk->dev,
+ "Failed to get duty cycle for clock ID %d\n", clk->id);
+ }
+
+ return ret;
+}
+
+static int scmi_clk_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
+{
+ int ret;
+ u32 val;
+ struct scmi_clk *clk = to_scmi_clk(hw);
+
+ /* SCMI OEM Duty Cycle is expressed as a percentage */
+ val = (duty->num * 100) / duty->den;
+ ret = scmi_proto_clk_ops->config_oem_set(clk->ph, clk->id,
+ SCMI_CLOCK_CFG_DUTY_CYCLE,
+ val, false);
+ if (ret)
+ dev_warn(clk->dev,
+ "Failed to set duty cycle(%u/%u) for clock ID %d\n",
+ duty->num, duty->den, clk->id);
+
+ return ret;
+}
+
static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk,
const struct clk_ops *scmi_ops)
{
@@ -258,6 +298,12 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
if (feats_key & BIT(SCMI_CLK_PARENT_CTRL_SUPPORTED))
ops->set_parent = scmi_clk_set_parent;
+ /* Duty cycle */
+ if (feats_key & BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED)) {
+ ops->get_duty_cycle = scmi_clk_get_duty_cycle;
+ ops->set_duty_cycle = scmi_clk_set_duty_cycle;
+ }
+
return ops;
}
@@ -312,6 +358,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
if (!ci->parent_ctrl_forbidden)
feats_key |= BIT(SCMI_CLK_PARENT_CTRL_SUPPORTED);
+ if (ci->extended_config)
+ feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
+
if (WARN_ON(feats_key >= db_size))
return NULL;
--
2.44.0
Some exposed SCMI Clocks could be marked as non-supporting rate changes.
Configure a clk_ops descriptors which does not provide the rate change
callbacks for such clocks when registering with CLK framework.
CC: Michael Turquette <[email protected]>
CC: Stephen Boyd <[email protected]>
CC: [email protected]
Signed-off-by: Cristian Marussi <[email protected]>
---
v2 --> v3
- using SCMI_CLK_RATE_CTRL_SUPPORTED instead of _FORBIDDEN
---
drivers/clk/clk-scmi.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index e70708573965..ba234b56f742 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -20,6 +20,7 @@
enum scmi_clk_feats {
SCMI_CLK_ATOMIC_SUPPORTED,
SCMI_CLK_STATE_CTRL_SUPPORTED,
+ SCMI_CLK_RATE_CTRL_SUPPORTED,
SCMI_CLK_FEATS_COUNT
};
@@ -248,7 +249,8 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
ops->recalc_rate = scmi_clk_recalc_rate;
ops->round_rate = scmi_clk_round_rate;
ops->determine_rate = scmi_clk_determine_rate;
- ops->set_rate = scmi_clk_set_rate;
+ if (feats_key & BIT(SCMI_CLK_RATE_CTRL_SUPPORTED))
+ ops->set_rate = scmi_clk_set_rate;
/* Parent ops */
ops->get_parent = scmi_clk_get_parent;
@@ -302,6 +304,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
if (!ci->state_ctrl_forbidden)
feats_key |= BIT(SCMI_CLK_STATE_CTRL_SUPPORTED);
+ if (!ci->rate_ctrl_forbidden)
+ feats_key |= BIT(SCMI_CLK_RATE_CTRL_SUPPORTED);
+
if (WARN_ON(feats_key >= db_size))
return NULL;
--
2.44.0
Quoting Cristian Marussi (2024-04-15 09:36:44)
> Hi,
>
> a small series to review how the SCMI Clock driver chooses and sets up the
> CLK operations to associate to a clock when registering with CLK framework.
Did you want me to merge this through clk tree?
On Fri, Apr 19, 2024 at 07:08:54PM -0700, Stephen Boyd wrote:
> Quoting Cristian Marussi (2024-04-15 09:36:44)
> > Hi,
> >
> > a small series to review how the SCMI Clock driver chooses and sets up the
> > CLK operations to associate to a clock when registering with CLK framework.
>
> Did you want me to merge this through clk tree?
Up to @Sudeep really...
Thanks for the reviewing this series.
Cristian
On Fri, Apr 19, 2024 at 07:08:54PM -0700, Stephen Boyd wrote:
> Quoting Cristian Marussi (2024-04-15 09:36:44)
> > Hi,
> >
> > a small series to review how the SCMI Clock driver chooses and sets up the
> > CLK operations to associate to a clock when registering with CLK framework.
>
> Did you want me to merge this through clk tree?
I am fine either way. You add:
Reviewed-by: Sudeep Holla <[email protected]>
if you prefer to take it. Or else please provide your ack for me to
take it via SCMI tree.
--
Regards,
Sudeep
Quoting Sudeep Holla (2024-04-22 01:06:06)
> On Fri, Apr 19, 2024 at 07:08:54PM -0700, Stephen Boyd wrote:
> > Quoting Cristian Marussi (2024-04-15 09:36:44)
> > > Hi,
> > >
> > > a small series to review how the SCMI Clock driver chooses and sets up the
> > > CLK operations to associate to a clock when registering with CLK framework.
> >
> > Did you want me to merge this through clk tree?
>
> I am fine either way. You add:
>
> Reviewed-by: Sudeep Holla <[email protected]>
>
> if you prefer to take it. Or else please provide your ack for me to
> take it via SCMI tree.
>
Ok I can take it then.
Quoting Cristian Marussi (2024-04-15 09:36:49)
> Provide the CLK framework callbacks related to get/set clock duty cycle if
> the related SCMI clock supports OEM extended configurations.
>
> CC: Michael Turquette <[email protected]>
> CC: Stephen Boyd <[email protected]>
> CC: [email protected]
> Signed-off-by: Cristian Marussi <[email protected]>
> ---
Applied to clk-next
Quoting Cristian Marussi (2024-04-15 09:36:47)
> Some exposed SCMI Clocks could be marked as non-supporting rate changes.
> Configure a clk_ops descriptors which does not provide the rate change
> callbacks for such clocks when registering with CLK framework.
>
> CC: Michael Turquette <[email protected]>
> CC: Stephen Boyd <[email protected]>
> CC: [email protected]
> Signed-off-by: Cristian Marussi <[email protected]>
> ---
Applied to clk-next