[PATCH RFC 0/2] clk: scmi: DT support for SCMI clock rate rounding modes (per‑clock policy)

[PATCH RFC 0/2] clk: scmi: DT support for SCMI clock rate rounding modes (per‑clock policy)

[Peng Fan (OSS)]

The ARM SCMI specification (DEN0056E) defines rounding‑mode flags for the
CLOCK_RATE_SET command, allowing a client to request that the firmware
round a requested clock rate down, up, or autonomously choose the
closest achievable rate.
This series introduces DT support in the SCMI clock provider to carry a
per‑clock rounding policy from the device tree into the SCMI protocol.

Patch 1 adds dt‑bindings constants for rounding modes:
ROUND_DOWN, ROUND_UP, ROUND_AUTO.

Patch 2 extends the SCMI clock provider to optionally support
"#clock-cells = <2>", where the second cell encodes the rounding mode.
The first consumer that references a given clock latches the per‑clock
policy. Subsequent consumers of the same clock must specify the same
mode; otherwise, the request is rejected to avoid non‑deterministic
behavior. The selected mode is passed through to the SCMI Clock protocol
and mapped to the corresponding CLOCK_SET_* flag.

Patch 2 includes changes to drivers/clk/clk-scmi.c and drivers/firmware
arm_scmi/clock.c, it is hard to separate the changes without breaking,
so I put the changes in one patch.

This design adopts a per‑clock policy model, not per‑consumer. The rounding
mode is applied by the provider per clock (index).
All consumers of the same clock must agree on the rounding mode.
Conflicting per‑consumer requests for the same clock are invalid and
are rejected during phandle translation.

This avoids silent clobbering and preserves deterministic behavior.

Existing device trees using #clock-cells = <1> continue to work and
default to ROUND_DOWN, exactly as before.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      dt-bindings: clock: Add SCMI clock rounding mode declarations
      clk: scmi: Add support for two #clock-cells to pass rate rounding mode

 drivers/clk/clk-scmi.c            | 62 +++++++++++++++++++++++++++++++++++++--
 drivers/firmware/arm_scmi/clock.c | 15 ++++++++--
 include/dt-bindings/clock/scmi.h  | 13 ++++++++
 include/linux/scmi_protocol.h     |  8 ++++-
 4 files changed, 92 insertions(+), 6 deletions(-)
---
base-commit: 3f9cd19e764b782706dbaacc69e502099cb014ba
change-id: 20260306-scmi-clk-round-1615258cf0fa

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH RFC 1/2] dt-bindings: clock: Add SCMI clock rounding mode declarations

[Peng Fan (OSS)]

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

The ARM System Control and Management Interface (SCMI) specification
(DEN0056E) defines the CLOCK_RATE_SET command, which allows clients to
specify how the platform should round the requested clock rate.

Bits[3:2] define the rounding behaviour:
  - If Bit[3] is set, the platform autonomously selects the closest
    achievable rate, ignoring Bit[2].
  - If Bit[3] is clear, Bit[2] selects the direction:
        0 = round down
        1 = round up

Add dt-bindings definitions for these rounding modes so that device tree
agent nodes can specify the desired rounding behaviour to the firmware.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 include/dt-bindings/clock/scmi.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/dt-bindings/clock/scmi.h b/include/dt-bindings/clock/scmi.h
new file mode 100644
index 0000000000000000000000000000000000000000..bd97d5aba53d042659e8e00ed96c6a054a2f9d98
--- /dev/null
+++ b/include/dt-bindings/clock/scmi.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright 2026 NXP
+ */
+
+#ifndef __SCMI_CLOCK_H
+#define __SCMI_CLOCK_H
+
+#define ROUND_DOWN	0
+#define ROUND_UP	1
+#define ROUND_AUTO	2
+
+#endif

-- 
2.37.1

[PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Peng Fan (OSS)]

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

SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
when setting a clock rate. The previously added dt-bindings header
defines three modes:

  ROUND_DOWN / ROUND_UP / ROUND_AUTO

To enable device tree clients to select a rounding mode, extend the
SCMI clock provider to support "#clock-cells = <2>", where the second
cell encodes the desired rounding mode. The default remains
ROUND_DOWN for backwards compatibility with existing device trees.

When two cells are used, scmi_clk_two_cells_get() extracts the rounding
mode and stores it per clock. The SCMI clk driver then passes this
value to the SCMI Clock protocol, which maps it to the appropriate
CLOCK_SET_* flag.

Existing DTs using "#clock-cells = <1>" are also being supported.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-scmi.c            | 62 +++++++++++++++++++++++++++++++++++++--
 drivers/firmware/arm_scmi/clock.c | 15 ++++++++--
 include/linux/scmi_protocol.h     |  8 ++++-
 3 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index 6b286ea6f1218c802d0ebb782c75a19057581c20..16547a1fa1a0f1595323b0f89753b38315743150 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2018-2024 ARM Ltd.
  */
 
+#include <dt-bindings/clock/scmi.h>
 #include <linux/bits.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -32,6 +33,8 @@ static const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
 
 struct scmi_clk {
 	u32 id;
+	u32 round;
+	bool round_set;	/* policy latched once */
 	struct device *dev;
 	struct clk_hw hw;
 	const struct scmi_clock_info *info;
@@ -94,8 +97,20 @@ static int scmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 			     unsigned long parent_rate)
 {
 	struct scmi_clk *clk = to_scmi_clk(hw);
+	u32 round;
+
+	switch (clk->round) {
+	case ROUND_UP:
+		round = SCMI_CLOCK_ROUND_UP;
+		break;
+	case ROUND_AUTO:
+		round = SCMI_CLOCK_ROUND_AUTO;
+		break;
+	default:
+		round = SCMI_CLOCK_ROUND_DOWN;
+	}
 
-	return scmi_proto_clk_ops->rate_set(clk->ph, clk->id, rate);
+	return scmi_proto_clk_ops->rate_set(clk->ph, clk->id, round, rate);
 }
 
 static int scmi_clk_set_parent(struct clk_hw *hw, u8 parent_index)
@@ -396,6 +411,41 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
 	return ops;
 }
 
+static struct clk_hw *
+scmi_clk_two_cells_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct clk_hw_onecell_data *hw_data = data;
+	unsigned int idx = clkspec->args[0];
+	u32 round = clkspec->args[1];
+	struct scmi_clk *clk;
+	struct clk_hw *hw;
+
+	if (idx >= hw_data->num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (round > ROUND_AUTO) {
+		pr_err("%s: invalid round method %u\n", __func__, round);
+		return ERR_PTR(-EINVAL);
+	}
+
+	hw = hw_data->hws[idx];
+	clk = to_scmi_clk(hw);
+
+	/* per-clock policy: latch on first use, refuse conflicts */
+	if (clk->round_set && clk->round != round) {
+		pr_warn("%s: conflicting rounding mode for clk idx %u: %u != %u\n",
+			__func__, idx, clk->round, round);
+		return ERR_PTR(-EINVAL);
+	}
+
+	clk->round = round;
+	clk->round_set = true;
+
+	return hw;
+}
+
 static int scmi_clocks_probe(struct scmi_device *sdev)
 {
 	int idx, count, err;
@@ -409,6 +459,7 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
 	struct scmi_protocol_handle *ph;
 	const struct clk_ops *scmi_clk_ops_db[SCMI_MAX_CLK_OPS] = {};
 	struct scmi_clk *sclks;
+	u32 cells = 1;
 
 	if (!handle)
 		return -ENODEV;
@@ -456,6 +507,8 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
 		sclk->id = idx;
 		sclk->ph = ph;
 		sclk->dev = dev;
+		sclk->round = ROUND_DOWN;
+		sclk->round_set = false;
 
 		/*
 		 * Note that the scmi_clk_ops_db is on the stack, not global,
@@ -495,8 +548,11 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
 		}
 	}
 
-	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
-					   clk_data);
+	of_property_read_u32(np, "#clock-cells", &cells);
+	if (cells == 2)
+		return devm_of_clk_add_hw_provider(dev, scmi_clk_two_cells_get, clk_data);
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
 }
 
 static const struct scmi_device_id scmi_id_table[] = {
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index ab36871650a1ff890c4cb7f67d3ded2622a72868..1548b6611f7f6c4ac60e740bb36f2377568d06dd 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -570,10 +570,10 @@ scmi_clock_rate_get(const struct scmi_protocol_handle *ph,
 }
 
 static int scmi_clock_rate_set(const struct scmi_protocol_handle *ph,
-			       u32 clk_id, u64 rate)
+			       u32 clk_id, u32 round, u64 rate)
 {
 	int ret;
-	u32 flags = 0;
+	u32 flags;
 	struct scmi_xfer *t;
 	struct scmi_clock_set_rate *cfg;
 	struct clock_info *ci = ph->get_priv(ph);
@@ -590,6 +590,17 @@ static int scmi_clock_rate_set(const struct scmi_protocol_handle *ph,
 	if (ret)
 		return ret;
 
+	switch (round) {
+	case SCMI_CLOCK_ROUND_UP:
+		flags = CLOCK_SET_ROUND_UP;
+		break;
+	case SCMI_CLOCK_ROUND_AUTO:
+		flags = CLOCK_SET_ROUND_AUTO;
+		break;
+	default:
+		flags = 0;
+	}
+
 	if (ci->max_async_req &&
 	    atomic_inc_return(&ci->cur_async_req) < ci->max_async_req)
 		flags |= CLOCK_SET_ASYNC;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index aafaac1496b06a6e4f0ca32eee58a9edf7d4a70f..d0b7186177f49dea9c4b0030927782e6fd819ad0 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -83,6 +83,12 @@ enum scmi_clock_oem_config {
 	SCMI_CLOCK_CFG_OEM_END = 0xFF,
 };
 
+enum scmi_clock_round {
+	SCMI_CLOCK_ROUND_DOWN = 0x0,
+	SCMI_CLOCK_ROUND_UP = 0x1,
+	SCMI_CLOCK_ROUND_AUTO = 0x2,
+};
+
 /**
  * struct scmi_clk_proto_ops - represents the various operations provided
  *	by SCMI Clock Protocol
@@ -107,7 +113,7 @@ struct scmi_clk_proto_ops {
 	int (*rate_get)(const struct scmi_protocol_handle *ph, u32 clk_id,
 			u64 *rate);
 	int (*rate_set)(const struct scmi_protocol_handle *ph, u32 clk_id,
-			u64 rate);
+			u32 round, u64 rate);
 	int (*enable)(const struct scmi_protocol_handle *ph, u32 clk_id,
 		      bool atomic);
 	int (*disable)(const struct scmi_protocol_handle *ph, u32 clk_id,

-- 
2.37.1

Re: [PATCH RFC 0/2] clk: scmi: DT support for SCMI clock rate rounding modes (per‑clock policy)

[Peng Fan]

Hi All,

On Fri, Mar 06, 2026 at 02:20:11PM +0800, Peng Fan (OSS) wrote:
>The ARM SCMI specification (DEN0056E) defines rounding‑mode flags for the
>CLOCK_RATE_SET command, allowing a client to request that the firmware
>round a requested clock rate down, up, or autonomously choose the
>closest achievable rate.

Would you please give a look and give some feedback?

Thanks,
Peng

>This series introduces DT support in the SCMI clock provider to carry a
>per‑clock rounding policy from the device tree into the SCMI protocol.
>
>Patch 1 adds dt‑bindings constants for rounding modes:
>ROUND_DOWN, ROUND_UP, ROUND_AUTO.
>
>Patch 2 extends the SCMI clock provider to optionally support
>"#clock-cells = <2>", where the second cell encodes the rounding mode.
>The first consumer that references a given clock latches the per‑clock
>policy. Subsequent consumers of the same clock must specify the same
>mode; otherwise, the request is rejected to avoid non‑deterministic
>behavior. The selected mode is passed through to the SCMI Clock protocol
>and mapped to the corresponding CLOCK_SET_* flag.
>
>Patch 2 includes changes to drivers/clk/clk-scmi.c and drivers/firmware
>arm_scmi/clock.c, it is hard to separate the changes without breaking,
>so I put the changes in one patch.
>
>This design adopts a per‑clock policy model, not per‑consumer. The rounding
>mode is applied by the provider per clock (index).
>All consumers of the same clock must agree on the rounding mode.
>Conflicting per‑consumer requests for the same clock are invalid and
>are rejected during phandle translation.
>
>This avoids silent clobbering and preserves deterministic behavior.
>
>Existing device trees using #clock-cells = <1> continue to work and
>default to ROUND_DOWN, exactly as before.
>
>Signed-off-by: Peng Fan <peng.fan@nxp.com>
>---
>Peng Fan (2):
>      dt-bindings: clock: Add SCMI clock rounding mode declarations
>      clk: scmi: Add support for two #clock-cells to pass rate rounding mode
>
> drivers/clk/clk-scmi.c            | 62 +++++++++++++++++++++++++++++++++++++--
> drivers/firmware/arm_scmi/clock.c | 15 ++++++++--
> include/dt-bindings/clock/scmi.h  | 13 ++++++++
> include/linux/scmi_protocol.h     |  8 ++++-
> 4 files changed, 92 insertions(+), 6 deletions(-)
>---
>base-commit: 3f9cd19e764b782706dbaacc69e502099cb014ba
>change-id: 20260306-scmi-clk-round-1615258cf0fa
>
>Best regards,
>-- 
>Peng Fan <peng.fan@nxp.com>
>

Re: [PATCH RFC 0/2] clk: scmi: DT support for SCMI clock rate rounding modes (per‑clock policy)

[Brian Masney]

Hi Peng,

On Fri, Mar 06, 2026 at 02:20:11PM +0800, Peng Fan (OSS) wrote:
> The ARM SCMI specification (DEN0056E) defines rounding‑mode flags for the
> CLOCK_RATE_SET command, allowing a client to request that the firmware
> round a requested clock rate down, up, or autonomously choose the
> closest achievable rate.
> This series introduces DT support in the SCMI clock provider to carry a
> per‑clock rounding policy from the device tree into the SCMI protocol.
> 
> Patch 1 adds dt‑bindings constants for rounding modes:
> ROUND_DOWN, ROUND_UP, ROUND_AUTO.
> 
> Patch 2 extends the SCMI clock provider to optionally support
> "#clock-cells = <2>", where the second cell encodes the rounding mode.
> The first consumer that references a given clock latches the per‑clock
> policy. Subsequent consumers of the same clock must specify the same
> mode; otherwise, the request is rejected to avoid non‑deterministic
> behavior. The selected mode is passed through to the SCMI Clock protocol
> and mapped to the corresponding CLOCK_SET_* flag.
> 
> Patch 2 includes changes to drivers/clk/clk-scmi.c and drivers/firmware
> arm_scmi/clock.c, it is hard to separate the changes without breaking,
> so I put the changes in one patch.
> 
> This design adopts a per‑clock policy model, not per‑consumer. The rounding
> mode is applied by the provider per clock (index).
> All consumers of the same clock must agree on the rounding mode.
> Conflicting per‑consumer requests for the same clock are invalid and
> are rejected during phandle translation.
> 
> This avoids silent clobbering and preserves deterministic behavior.
> 
> Existing device trees using #clock-cells = <1> continue to work and
> default to ROUND_DOWN, exactly as before.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

My high level feedback about this:

1) Since you are making changes to the DT schema for the clock-cells,
   does the SCMI DT schema document also need to be updated to allow
   clock-cells to be 1 or 2?

2) For the ROUND_XXX constants, I would prefix them with something
   since the existing ROUND names are fairly generic sounding. Maybe
   CLK_SCMI_?

Brian

Re: [PATCH RFC 0/2] clk: scmi: DT support for SCMI clock rate rounding modes (per‑clock policy)

[Peng Fan]

Hi Brian,
On Mon, Apr 06, 2026 at 11:38:30AM -0400, Brian Masney wrote:
>Hi Peng,
>
>On Fri, Mar 06, 2026 at 02:20:11PM +0800, Peng Fan (OSS) wrote:
>> The ARM SCMI specification (DEN0056E) defines rounding‑mode flags for the
>> CLOCK_RATE_SET command, allowing a client to request that the firmware
>> round a requested clock rate down, up, or autonomously choose the
>> closest achievable rate.
>> This series introduces DT support in the SCMI clock provider to carry a
>> per‑clock rounding policy from the device tree into the SCMI protocol.
>> 
>> Patch 1 adds dt‑bindings constants for rounding modes:
>> ROUND_DOWN, ROUND_UP, ROUND_AUTO.
>> 
>> Patch 2 extends the SCMI clock provider to optionally support
>> "#clock-cells = <2>", where the second cell encodes the rounding mode.
>> The first consumer that references a given clock latches the per‑clock
>> policy. Subsequent consumers of the same clock must specify the same
>> mode; otherwise, the request is rejected to avoid non‑deterministic
>> behavior. The selected mode is passed through to the SCMI Clock protocol
>> and mapped to the corresponding CLOCK_SET_* flag.
>> 
>> Patch 2 includes changes to drivers/clk/clk-scmi.c and drivers/firmware
>> arm_scmi/clock.c, it is hard to separate the changes without breaking,
>> so I put the changes in one patch.
>> 
>> This design adopts a per‑clock policy model, not per‑consumer. The rounding
>> mode is applied by the provider per clock (index).
>> All consumers of the same clock must agree on the rounding mode.
>> Conflicting per‑consumer requests for the same clock are invalid and
>> are rejected during phandle translation.
>> 
>> This avoids silent clobbering and preserves deterministic behavior.
>> 
>> Existing device trees using #clock-cells = <1> continue to work and
>> default to ROUND_DOWN, exactly as before.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>
>My high level feedback about this:
>
>1) Since you are making changes to the DT schema for the clock-cells,
>   does the SCMI DT schema document also need to be updated to allow
>   clock-cells to be 1 or 2?

You are right.

>
>2) For the ROUND_XXX constants, I would prefix them with something
>   since the existing ROUND names are fairly generic sounding. Maybe
>   CLK_SCMI_?

sounds good.

I will wait a few more days to see if there are other comments, then
will post out next version.

Thanks,
Peng

>
>Brian
>

Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Sudeep Holla]

On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
> when setting a clock rate. The previously added dt-bindings header
> defines three modes:
> 
>   ROUND_DOWN / ROUND_UP / ROUND_AUTO
> 
> To enable device tree clients to select a rounding mode, extend the
> SCMI clock provider to support "#clock-cells = <2>", where the second
> cell encodes the desired rounding mode. The default remains
> ROUND_DOWN for backwards compatibility with existing device trees.
> 

Where is the binding update documented ? It's not in 1/2.

Also if it can be static in the device tree, why can't it be
autonomously handled in the platform firmware ? I think I know the
answer for this but I want to make sure it is a valid use-case and
gets documented here as part of binding updates.

-- 
Regards,
Sudeep

Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Peng Fan]

Hi Sudeep,

Thanks for giving a look.

On Wed, Apr 22, 2026 at 02:14:56PM +0100, Sudeep Holla wrote:
>On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
>> when setting a clock rate. The previously added dt-bindings header
>> defines three modes:
>> 
>>   ROUND_DOWN / ROUND_UP / ROUND_AUTO
>> 
>> To enable device tree clients to select a rounding mode, extend the
>> SCMI clock provider to support "#clock-cells = <2>", where the second
>> cell encodes the desired rounding mode. The default remains
>> ROUND_DOWN for backwards compatibility with existing device trees.
>> 
>
>Where is the binding update documented ? It's not in 1/2.

This was missed in this patchset, I will fix in new version, if this
patchset does not have big design flaw.

>
>Also if it can be static in the device tree, why can't it be
>autonomously handled in the platform firmware ? I think I know the

Linux passes ROUND_DOWN, SCMI firmware uses round down for clk calculation.

>answer for this but I want to make sure it is a valid use-case and
>gets documented here as part of binding updates.

Per info from our video software team.
We have some video modes where the best pixel clock rate is slightly above the
nominal rate, and the default round down rule (CLOCK_ROUND_RULE_CEILING in SM
firmware) can cause the resulting clock rate to be much lower than expected.

disp1pix = 96200000 Hz (desired pixel clock rate)

The MIPI DPHY cannot hit the exact frequency of 288600000 Hz needed for this
pixel clock rate, so the next best DPHY PLL frequency is 289000000 Hz. This
corresponds to a pixel clock frequency of 96333333 Hz, which is slightly higher
than the nominal rate of 96200000 Hz the video mode specifies.

Setting the VIDEOPLL (disp1pix parent) to 289000000 Hz should divide down to
the adjusted disp1pix frequency of 96333333 Hz, but here is what happens in the
SM firmware:

quotient = 289000000 / (96200000 + 1) = 3.004 => 3 (notice that the SM always
receives the nominal clock rate, not the adjusted rate)

If the rounding rule is round down (CLOCK_ROUND_RULE_CEILING),
quotient = quotient + 1. Therefore, quotient becomes 4.

disp1pix = 289000000 / 4 = 72250000, which is nowhere close to the target of
96333333.

However, if we can use ROUND_AUTO the SM firmware would select a quotient of 3
in this case, and disp1pix would match our target: 289000000 / 3 = 96333333.

Thanks,
Peng

>
>-- 
>Regards,
>Sudeep

Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Sudeep Holla]

On Wed, Apr 22, 2026 at 10:00:23PM +0800, Peng Fan wrote:
> Hi Sudeep,
> 
> Thanks for giving a look.
> 
> On Wed, Apr 22, 2026 at 02:14:56PM +0100, Sudeep Holla wrote:
> >On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
> >> From: Peng Fan <peng.fan@nxp.com>
> >> 
> >> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
> >> when setting a clock rate. The previously added dt-bindings header
> >> defines three modes:
> >> 
> >>   ROUND_DOWN / ROUND_UP / ROUND_AUTO
> >> 
> >> To enable device tree clients to select a rounding mode, extend the
> >> SCMI clock provider to support "#clock-cells = <2>", where the second
> >> cell encodes the desired rounding mode. The default remains
> >> ROUND_DOWN for backwards compatibility with existing device trees.
> >> 
> >
> >Where is the binding update documented ? It's not in 1/2.
> 
> This was missed in this patchset, I will fix in new version, if this
> patchset does not have big design flaw.
> 
> >
> >Also if it can be static in the device tree, why can't it be
> >autonomously handled in the platform firmware ? I think I know the
> 
> Linux passes ROUND_DOWN, SCMI firmware uses round down for clk calculation.
> 
> >answer for this but I want to make sure it is a valid use-case and
> >gets documented here as part of binding updates.
> 
> Per info from our video software team.
> We have some video modes where the best pixel clock rate is slightly above the
> nominal rate, and the default round down rule (CLOCK_ROUND_RULE_CEILING in SM
> firmware) can cause the resulting clock rate to be much lower than expected.
> 
> disp1pix = 96200000 Hz (desired pixel clock rate)
> 
> The MIPI DPHY cannot hit the exact frequency of 288600000 Hz needed for this
> pixel clock rate, so the next best DPHY PLL frequency is 289000000 Hz. This
> corresponds to a pixel clock frequency of 96333333 Hz, which is slightly higher
> than the nominal rate of 96200000 Hz the video mode specifies.
> 
> Setting the VIDEOPLL (disp1pix parent) to 289000000 Hz should divide down to
> the adjusted disp1pix frequency of 96333333 Hz, but here is what happens in the
> SM firmware:
> 
> quotient = 289000000 / (96200000 + 1) = 3.004 => 3 (notice that the SM always
> receives the nominal clock rate, not the adjusted rate)
> 
> If the rounding rule is round down (CLOCK_ROUND_RULE_CEILING),
> quotient = quotient + 1. Therefore, quotient becomes 4.
> 
> disp1pix = 289000000 / 4 = 72250000, which is nowhere close to the target of
> 96333333.
> 

I do not think this is the correct interpretation of `CLOCK_ROUND_DOWN/UP`.

`CLOCK_ROUND_DOWN/UP` should apply to the requested `disp1pix` rate itself,
not to the divider choice in a way that forces selection of the next integer
divisor and produces a much lower output clock.

Here, the requested `disp1pix` is `96,200,000 Hz`, and the parent rate is
`289,000,000 Hz`. The achievable child rates nearby are:

`289,000,000 / 3 = 96,333,333 Hz`
`289,000,000 / 4 = 72,250,000 Hz`

Given those options, the firmware should be able to round the request
autonomously to the nearest supported `disp1pix` rate, which is `96,333,333
Hz` (`289,000,000 / 3`).

Under that interpretation:

`CLOCK_ROUND_UP` would permit choosing `96,333,333`
`CLOCK_ROUND_AUTO` would also likely choose `96,333,333`
Choosing `/4` and ending up at `72,250,000` does not look like a meaningful
rounding of `96,200,000`

So the issue appears to be that the firmware is applying the rounding rule to
divider selection rather than to the resulting `disp1pix` frequency.

> However, if we can use `ROUND_AUTO` the SM firmware would select a quotient of 3
> in this case, and `disp1pix` would match our target: `289000000 / 3 = 96333333`.

Given the explanation above, I would not support this approach. `ROUND_AUTO`
should be sufficient for this case if the firmware is making a sensible
selection.

-- 
Regards,
Sudeep

Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Peng Fan]

On Wed, Apr 22, 2026 at 07:51:03PM +0100, Sudeep Holla wrote:
>On Wed, Apr 22, 2026 at 10:00:23PM +0800, Peng Fan wrote:
>> Hi Sudeep,
>> 
>> Thanks for giving a look.
>> 
>> On Wed, Apr 22, 2026 at 02:14:56PM +0100, Sudeep Holla wrote:
>> >On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
>> >> From: Peng Fan <peng.fan@nxp.com>
>> >> 
>> >> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
>> >> when setting a clock rate. The previously added dt-bindings header
>> >> defines three modes:
>> >> 
>> >>   ROUND_DOWN / ROUND_UP / ROUND_AUTO
>> >> 
>> >> To enable device tree clients to select a rounding mode, extend the
>> >> SCMI clock provider to support "#clock-cells = <2>", where the second
>> >> cell encodes the desired rounding mode. The default remains
>> >> ROUND_DOWN for backwards compatibility with existing device trees.
>> >> 
>> >
>> >Where is the binding update documented ? It's not in 1/2.
>> 
>> This was missed in this patchset, I will fix in new version, if this
>> patchset does not have big design flaw.
>> 
>> >
>> >Also if it can be static in the device tree, why can't it be
>> >autonomously handled in the platform firmware ? I think I know the
>> 
>> Linux passes ROUND_DOWN, SCMI firmware uses round down for clk calculation.
>> 
>> >answer for this but I want to make sure it is a valid use-case and
>> >gets documented here as part of binding updates.
>> 
>> Per info from our video software team.
>> We have some video modes where the best pixel clock rate is slightly above the
>> nominal rate, and the default round down rule (CLOCK_ROUND_RULE_CEILING in SM
>> firmware) can cause the resulting clock rate to be much lower than expected.
>> 
>> disp1pix = 96200000 Hz (desired pixel clock rate)
>> 
>> The MIPI DPHY cannot hit the exact frequency of 288600000 Hz needed for this
>> pixel clock rate, so the next best DPHY PLL frequency is 289000000 Hz. This
>> corresponds to a pixel clock frequency of 96333333 Hz, which is slightly higher
>> than the nominal rate of 96200000 Hz the video mode specifies.
>> 
>> Setting the VIDEOPLL (disp1pix parent) to 289000000 Hz should divide down to
>> the adjusted disp1pix frequency of 96333333 Hz, but here is what happens in the
>> SM firmware:
>> 
>> quotient = 289000000 / (96200000 + 1) = 3.004 => 3 (notice that the SM always
>> receives the nominal clock rate, not the adjusted rate)
>> 
>> If the rounding rule is round down (CLOCK_ROUND_RULE_CEILING),
>> quotient = quotient + 1. Therefore, quotient becomes 4.
>> 
>> disp1pix = 289000000 / 4 = 72250000, which is nowhere close to the target of
>> 96333333.
>> 
>
>I do not think this is the correct interpretation of `CLOCK_ROUND_DOWN/UP`.
>
>`CLOCK_ROUND_DOWN/UP` should apply to the requested `disp1pix` rate itself,
>not to the divider choice in a way that forces selection of the next integer
>divisor and produces a much lower output clock.
>
>Here, the requested `disp1pix` is `96,200,000 Hz`, and the parent rate is
>`289,000,000 Hz`. The achievable child rates nearby are:
>
>`289,000,000 / 3 = 96,333,333 Hz`
>`289,000,000 / 4 = 72,250,000 Hz`
>
>Given those options, the firmware should be able to round the request
>autonomously to the nearest supported `disp1pix` rate, which is `96,333,333
>Hz` (`289,000,000 / 3`).
>
>Under that interpretation:
>
>`CLOCK_ROUND_UP` would permit choosing `96,333,333`
>`CLOCK_ROUND_AUTO` would also likely choose `96,333,333`
>Choosing `/4` and ending up at `72,250,000` does not look like a meaningful
>rounding of `96,200,000`
>
>So the issue appears to be that the firmware is applying the rounding rule to
>divider selection rather than to the resulting `disp1pix` frequency.

User requests 96.2 MHz with ROUND_DOWN semantics, expecting the closest
achievable frequency that does not meaningfully deviate from the request.

Firmware evaluates the parent rate of 289,000,000 Hz and computes:

    289,000,000 / 3 = 96,333,333 Hz

Since this resulting frequency is slightly higher than the requested
96.2 MHz, the firmware, applying a strict `output <= requested` rule,
rejects divider 3 and selects divider 4 instead, producing:

    289,000,000 / 4 = 72,250,000 Hz

This leads to an output frequency that is much farther from the requested
value.

My question is: if the firmware were to select divider 3 and produce
96,333,333 Hz (only ~0.13% higher than the request), would that be
considered a violation of ROUND_DOWN semantics, or is ROUND_DOWN intended
to select the closest achievable output frequency rather than enforcing
a strict inequality against the requested rate?

Thanks,
Peng

>
>> However, if we can use `ROUND_AUTO` the SM firmware would select a quotient of 3
>> in this case, and `disp1pix` would match our target: `289000000 / 3 = 96333333`.
>
>Given the explanation above, I would not support this approach. `ROUND_AUTO`
>should be sufficient for this case if the firmware is making a sensible
>selection.
>
>-- 
>Regards,
>Sudeep
>

Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode

[Sudeep Holla]

On Thu, Apr 23, 2026 at 09:17:47AM +0800, Peng Fan wrote:
> On Wed, Apr 22, 2026 at 07:51:03PM +0100, Sudeep Holla wrote:
> >On Wed, Apr 22, 2026 at 10:00:23PM +0800, Peng Fan wrote:
> >> Hi Sudeep,
> >> 
> >> Thanks for giving a look.
> >> 
> >> On Wed, Apr 22, 2026 at 02:14:56PM +0100, Sudeep Holla wrote:
> >> >On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
> >> >> From: Peng Fan <peng.fan@nxp.com>
> >> >> 
> >> >> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
> >> >> when setting a clock rate. The previously added dt-bindings header
> >> >> defines three modes:
> >> >> 
> >> >>   ROUND_DOWN / ROUND_UP / ROUND_AUTO
> >> >> 
> >> >> To enable device tree clients to select a rounding mode, extend the
> >> >> SCMI clock provider to support "#clock-cells = <2>", where the second
> >> >> cell encodes the desired rounding mode. The default remains
> >> >> ROUND_DOWN for backwards compatibility with existing device trees.
> >> >> 
> >> >
> >> >Where is the binding update documented ? It's not in 1/2.
> >> 
> >> This was missed in this patchset, I will fix in new version, if this
> >> patchset does not have big design flaw.
> >> 
> >> >
> >> >Also if it can be static in the device tree, why can't it be
> >> >autonomously handled in the platform firmware ? I think I know the
> >> 
> >> Linux passes ROUND_DOWN, SCMI firmware uses round down for clk calculation.
> >> 
> >> >answer for this but I want to make sure it is a valid use-case and
> >> >gets documented here as part of binding updates.
> >> 
> >> Per info from our video software team.
> >> We have some video modes where the best pixel clock rate is slightly above the
> >> nominal rate, and the default round down rule (CLOCK_ROUND_RULE_CEILING in SM
> >> firmware) can cause the resulting clock rate to be much lower than expected.
> >> 
> >> disp1pix = 96200000 Hz (desired pixel clock rate)
> >> 
> >> The MIPI DPHY cannot hit the exact frequency of 288600000 Hz needed for this
> >> pixel clock rate, so the next best DPHY PLL frequency is 289000000 Hz. This
> >> corresponds to a pixel clock frequency of 96333333 Hz, which is slightly higher
> >> than the nominal rate of 96200000 Hz the video mode specifies.
> >> 
> >> Setting the VIDEOPLL (disp1pix parent) to 289000000 Hz should divide down to
> >> the adjusted disp1pix frequency of 96333333 Hz, but here is what happens in the
> >> SM firmware:
> >> 
> >> quotient = 289000000 / (96200000 + 1) = 3.004 => 3 (notice that the SM always
> >> receives the nominal clock rate, not the adjusted rate)
> >> 
> >> If the rounding rule is round down (CLOCK_ROUND_RULE_CEILING),
> >> quotient = quotient + 1. Therefore, quotient becomes 4.
> >> 
> >> disp1pix = 289000000 / 4 = 72250000, which is nowhere close to the target of
> >> 96333333.
> >> 
> >
> >I do not think this is the correct interpretation of `CLOCK_ROUND_DOWN/UP`.
> >
> >`CLOCK_ROUND_DOWN/UP` should apply to the requested `disp1pix` rate itself,
> >not to the divider choice in a way that forces selection of the next integer
> >divisor and produces a much lower output clock.
> >
> >Here, the requested `disp1pix` is `96,200,000 Hz`, and the parent rate is
> >`289,000,000 Hz`. The achievable child rates nearby are:
> >
> >`289,000,000 / 3 = 96,333,333 Hz`
> >`289,000,000 / 4 = 72,250,000 Hz`
> >
> >Given those options, the firmware should be able to round the request
> >autonomously to the nearest supported `disp1pix` rate, which is `96,333,333
> >Hz` (`289,000,000 / 3`).
> >
> >Under that interpretation:
> >
> >`CLOCK_ROUND_UP` would permit choosing `96,333,333`
> >`CLOCK_ROUND_AUTO` would also likely choose `96,333,333`
> >Choosing `/4` and ending up at `72,250,000` does not look like a meaningful
> >rounding of `96,200,000`
> >
> >So the issue appears to be that the firmware is applying the rounding rule to
> >divider selection rather than to the resulting `disp1pix` frequency.
> 
> User requests 96.2 MHz with ROUND_DOWN semantics, expecting the closest
> achievable frequency that does not meaningfully deviate from the request.
> 
> Firmware evaluates the parent rate of 289,000,000 Hz and computes:
> 
>     289,000,000 / 3 = 96,333,333 Hz
> 
> Since this resulting frequency is slightly higher than the requested
> 96.2 MHz, the firmware, applying a strict `output <= requested` rule,
> rejects divider 3 and selects divider 4 instead, producing:
> 
>     289,000,000 / 4 = 72,250,000 Hz
> 
> This leads to an output frequency that is much farther from the requested
> value.
> 
> My question is: if the firmware were to select divider 3 and produce
> 96,333,333 Hz (only ~0.13% higher than the request), would that be
> considered a violation of ROUND_DOWN semantics, or is ROUND_DOWN intended
> to select the closest achievable output frequency rather than enforcing
> a strict inequality against the requested rate?
> 

We can change the driver to default to ROUND_AUTO if that helps. I fully
understand the default ROUND_DOWN is not good but if firmware can't handle
your use case with ROUND_AUTO, it is firmware issue.

-- 
Regards,
Sudeep