* [PATCH AUTOSEL 6.17-6.12] clk: scmi: Add duty cycle ops only when duty cycle is supported
[not found] <20251026144958.26750-1-sashal@kernel.org>
@ 2025-10-26 14:48 ` Sasha Levin
2025-10-26 14:49 ` [PATCH AUTOSEL 6.17-6.12] clk: scmi: migrate round_rate() to determine_rate() Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2025-10-26 14:48 UTC (permalink / raw)
To: patches, stable
Cc: Jacky Bai, Sudeep Holla, Stephen Boyd, Sasha Levin, mturquette,
arm-scmi, linux-arm-kernel, linux-clk
From: Jacky Bai <ping.bai@nxp.com>
[ Upstream commit 18db1ff2dea0f97dedaeadd18b0cb0a0d76154df ]
For some of the SCMI based platforms, the oem extended config may be
supported, but not for duty cycle purpose. Skip the duty cycle ops if
err return when trying to get duty cycle info.
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
YES – this is a low-risk bug fix that prevents the driver from
advertising duty-cycle support on firmware that actually rejects the
operation, avoiding real user-visible failures.
- `scmi_clk_ops_alloc()` wires up `get_duty_cycle`/`set_duty_cycle`
whenever the duty-cycle feature bit is set (`drivers/clk/clk-
scmi.c:311`). Before this patch any clock with `extended_config =
true` populated that bit, so consumers believed the duty-cycle API
worked even when firmware returned `-EOPNOTSUPP`.
- In practice, a refused call bubbles up to drivers that rely on the
feature. For example, `clk_set_duty_cycle()` in the AXG TDM interface
aborts audio setup if the clock op fails (`sound/soc/meson/axg-tdm-
interface.c:249`), so misreporting support breaks real hardware.
- The commit now probes firmware once at registration time and only sets
`SCMI_CLK_DUTY_CYCLE_SUPPORTED` when
`config_oem_get(...SCMI_CLOCK_CFG_DUTY_CYCLE...)` succeeds
(`drivers/clk/clk-scmi.c:349` and `drivers/clk/clk-scmi.c:372-377`).
This simply reuses the existing accessor (`drivers/clk/clk-
scmi.c:187`) and has no side effects beyond skipping the bogus ops.
- Change is tiny, localized to the SCMI clock driver, and introduces no
ABI or architectural churn; the new call is already required whenever
the duty-cycle helpers are invoked, so risk is minimal.
- Stable branches need to carry the duty-cycle support addition (`clk:
scmi: Add support for get/set duty_cycle operations`, commit
87af9481af53) beforehand; with that prerequisite satisfied,
backporting this fix prevents firmware that only supports other OEM
configs from breaking consumers.
Given it fixes a regression introduced with duty-cycle support and keeps
the driver from lying about capabilities, it fits stable backport
criteria.
drivers/clk/clk-scmi.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index 78dd2d9c7cabd..6b286ea6f1218 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -346,6 +346,8 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
unsigned int atomic_threshold_us,
const struct clk_ops **clk_ops_db, size_t db_size)
{
+ int ret;
+ u32 val;
const struct scmi_clock_info *ci = sclk->info;
unsigned int feats_key = 0;
const struct clk_ops *ops;
@@ -367,8 +369,13 @@ 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 (ci->extended_config) {
+ ret = scmi_proto_clk_ops->config_oem_get(sclk->ph, sclk->id,
+ SCMI_CLOCK_CFG_DUTY_CYCLE,
+ &val, NULL, false);
+ if (!ret)
+ feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
+ }
if (WARN_ON(feats_key >= db_size))
return NULL;
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.17-6.12] clk: scmi: migrate round_rate() to determine_rate()
[not found] <20251026144958.26750-1-sashal@kernel.org>
2025-10-26 14:48 ` [PATCH AUTOSEL 6.17-6.12] clk: scmi: Add duty cycle ops only when duty cycle is supported Sasha Levin
@ 2025-10-26 14:49 ` Sasha Levin
2025-10-26 23:16 ` Brian Masney
1 sibling, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2025-10-26 14:49 UTC (permalink / raw)
To: patches, stable
Cc: Brian Masney, Sudeep Holla, Peng Fan, Sasha Levin, mturquette,
sboyd, arm-scmi, linux-arm-kernel, linux-clk
From: Brian Masney <bmasney@redhat.com>
[ Upstream commit 80cb2b6edd8368f7e1e8bf2f66aabf57aa7de4b7 ]
This driver implements both the determine_rate() and round_rate() clk
ops, and the round_rate() clk ops is deprecated. When both are defined,
clk_core_determine_round_nolock() from the clk core will only use the
determine_rate() clk ops.
The existing scmi_clk_determine_rate() is a noop implementation that
lets the firmware round the rate as appropriate. Drop the existing
determine_rate implementation and convert the existing round_rate()
implementation over to determine_rate().
scmi_clk_determine_rate() was added recently when the clock parent
support was added, so it's not expected that this change will regress
anything.
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com> #i.MX95-19x19-EVK
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
YES The patch restores the SCMI clock driver's ability to return a valid
rounded rate when the framework asks for it.
- With the regression-introducing stub in `scmi_clk_determine_rate()`
every request fell through without touching `req->rate`, so
`clk_core_determine_round_nolock()` would return the caller’s original
value whenever both ops were present (`drivers/clk/clk.c:1596-1613`),
making `clk_round_rate()` lie about the hardware outcome on platforms
that advertise min/max/step limits.
- The new implementation in `drivers/clk/clk-scmi.c:57-90` moves the
logic that used to live in `.round_rate()` into `.determine_rate()`,
clamping to `min_rate`/`max_rate` and quantising by `step_size`,
exactly reproducing the behaviour that worked before the noop
`determine_rate()` was introduced.
- Discrete-rate clocks remain unchanged—the function still bails out
early (`drivers/clk/clk-scmi.c:63-71`), matching the old behaviour—and
the ops table simply stops advertising the deprecated `.round_rate()`
callback (`drivers/clk/clk-scmi.c:299-304`), so risk is minimal and
confined to SCMI clocks.
- The bug has shipped since the recent parent-support work (first seen
in v6.10), so stable kernels carrying that change are returning
incorrect values to consumers today.
Because this is a regression fix with low risk and no architectural
churn, it is a good candidate for backporting to every stable series
that contains the broken noop `determine_rate()`.
drivers/clk/clk-scmi.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index d2408403283fc..78dd2d9c7cabd 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -54,8 +54,8 @@ static unsigned long scmi_clk_recalc_rate(struct clk_hw *hw,
return rate;
}
-static long scmi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long *parent_rate)
+static int scmi_clk_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
{
u64 fmin, fmax, ftmp;
struct scmi_clk *clk = to_scmi_clk(hw);
@@ -67,20 +67,27 @@ static long scmi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
* running at then.
*/
if (clk->info->rate_discrete)
- return rate;
+ return 0;
fmin = clk->info->range.min_rate;
fmax = clk->info->range.max_rate;
- if (rate <= fmin)
- return fmin;
- else if (rate >= fmax)
- return fmax;
+ if (req->rate <= fmin) {
+ req->rate = fmin;
+
+ return 0;
+ } else if (req->rate >= fmax) {
+ req->rate = fmax;
- ftmp = rate - fmin;
+ return 0;
+ }
+
+ ftmp = req->rate - fmin;
ftmp += clk->info->range.step_size - 1; /* to round up */
do_div(ftmp, clk->info->range.step_size);
- return ftmp * clk->info->range.step_size + fmin;
+ req->rate = ftmp * clk->info->range.step_size + fmin;
+
+ return 0;
}
static int scmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -119,15 +126,6 @@ static u8 scmi_clk_get_parent(struct clk_hw *hw)
return p_idx;
}
-static int scmi_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
-{
- /*
- * Suppose all the requested rates are supported, and let firmware
- * to handle the left work.
- */
- return 0;
-}
-
static int scmi_clk_enable(struct clk_hw *hw)
{
struct scmi_clk *clk = to_scmi_clk(hw);
@@ -300,7 +298,6 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
/* Rate ops */
ops->recalc_rate = scmi_clk_recalc_rate;
- ops->round_rate = scmi_clk_round_rate;
ops->determine_rate = scmi_clk_determine_rate;
if (feats_key & BIT(SCMI_CLK_RATE_CTRL_SUPPORTED))
ops->set_rate = scmi_clk_set_rate;
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread