[PATCH v2 1/4] clk: scmi: Bulk allocate all sub-driver instance data

[PATCH v2 1/4] clk: scmi: Bulk allocate all sub-driver instance data

[Marek Vasut]

Allocate all sub-driver instance data at once. The amount of data that
have to be allocated is known up front, so is the size of the data, so
there is no need to call malloc() in a loop, mallocate all data at once.

The upside is, less heap fragmentation and fewer malloc() calls overall,
and a faster boot time.

The downside is, if some of the clock fail to register, then the clock
driver cannot free parts of the bulk allocated sub-driver instance data.
Such a failure can only occur if clk_register() were to fail, and if that
happens, the system has more significant problems. Worse, if a core clock
driver fails to probe, the system has even bigger problem.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Alice Guo <alice.guo@nxp.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
V2: Add RB from Peng
---
 drivers/clk/clk_scmi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 37e349b9c78..548bbfe14de 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -283,7 +283,7 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
 
 static int scmi_clk_probe(struct udevice *dev)
 {
-	struct clk_scmi *clk_scmi;
+	struct clk_scmi *clk_scmi_bulk, *clk_scmi;
 	struct scmi_clock_priv *priv = dev_get_priv(dev);
 	size_t num_clocks, i;
 	int ret;
@@ -312,20 +312,23 @@ static int scmi_clk_probe(struct udevice *dev)
 		return ret;
 	}
 
+	clk_scmi_bulk = kzalloc(num_clocks * sizeof(*clk_scmi), GFP_KERNEL);
+	if (!clk_scmi_bulk)
+		return -ENOMEM;
+
 	for (i = 0; i < num_clocks; i++) {
 		char *clock_name;
 		u32 attributes;
 
 		if (!scmi_clk_get_attibute(dev, i, &clock_name, &attributes)) {
-			clk_scmi = kzalloc(sizeof(*clk_scmi), GFP_KERNEL);
-			if (!clk_scmi || !clock_name)
+			clk_scmi = clk_scmi_bulk + i;
+			if (!clock_name)
 				ret = -ENOMEM;
 			else
 				ret = clk_register(&clk_scmi->clk, dev->driver->name,
 						   clock_name, dev->name);
 
 			if (ret) {
-				free(clk_scmi);
 				free(clock_name);
 				return ret;
 			}
-- 
2.51.0

[PATCH v2 2/4] clk: scmi: Factor out clock control flags resolution

[Marek Vasut]

Pull clock control flags resolution into dedicated function and
call it from each site that does access clock control flags. No
functional change.

This is a preparatory patch for deferred issue of SCMI_CLOCK_ATTRIBUTES.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Alice Guo <alice.guo@nxp.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
V2: Add RB from Peng
---
 drivers/clk/clk_scmi.c | 51 +++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 548bbfe14de..1a6a6ae464c 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -163,22 +163,36 @@ static int scmi_clk_gate(struct clk *clk, int enable)
 	return scmi_to_linux_errno(out.status);
 }
 
-static int scmi_clk_enable(struct clk *clk)
+static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
 {
 	struct clk_scmi *clkscmi;
 	struct clk *c;
 	int ret;
 
-	if (!CONFIG_IS_ENABLED(CLK_CCF))
-		return scmi_clk_gate(clk, 1);
-
 	ret = clk_get_by_id(clk->id, &c);
 	if (ret)
 		return ret;
 
 	clkscmi = container_of(c, struct clk_scmi, clk);
 
-	if (clkscmi->ctrl_flags & SUPPORT_CLK_STAT_CONTROL)
+	*ctrl_flags = clkscmi->ctrl_flags;
+
+	return 0;
+}
+
+static int scmi_clk_enable(struct clk *clk)
+{
+	u32 ctrl_flags;
+	int ret;
+
+	if (!CONFIG_IS_ENABLED(CLK_CCF))
+		return scmi_clk_gate(clk, 1);
+
+	ret = scmi_clk_get_ctrl_flags(clk, &ctrl_flags);
+	if (ret)
+		return ret;
+
+	if (ctrl_flags & SUPPORT_CLK_STAT_CONTROL)
 		return scmi_clk_gate(clk, 1);
 
 	/* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */
@@ -188,20 +202,17 @@ static int scmi_clk_enable(struct clk *clk)
 
 static int scmi_clk_disable(struct clk *clk)
 {
-	struct clk_scmi *clkscmi;
-	struct clk *c;
+	u32 ctrl_flags;
 	int ret;
 
 	if (!CONFIG_IS_ENABLED(CLK_CCF))
 		return scmi_clk_gate(clk, 0);
 
-	ret = clk_get_by_id(clk->id, &c);
+	ret = scmi_clk_get_ctrl_flags(clk, &ctrl_flags);
 	if (ret)
 		return ret;
 
-	clkscmi = container_of(c, struct clk_scmi, clk);
-
-	if (clkscmi->ctrl_flags & SUPPORT_CLK_STAT_CONTROL)
+	if (ctrl_flags & SUPPORT_CLK_STAT_CONTROL)
 		return scmi_clk_gate(clk, 0);
 
 	/* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */
@@ -259,20 +270,17 @@ static ulong __scmi_clk_set_rate(struct clk *clk, ulong rate)
 
 static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
 {
-	struct clk_scmi *clkscmi;
-	struct clk *c;
+	u32 ctrl_flags;
 	int ret;
 
 	if (!CONFIG_IS_ENABLED(CLK_CCF))
 		return __scmi_clk_set_rate(clk, rate);
 
-	ret = clk_get_by_id(clk->id, &c);
+	ret = scmi_clk_get_ctrl_flags(clk, &ctrl_flags);
 	if (ret)
 		return ret;
 
-	clkscmi = container_of(c, struct clk_scmi, clk);
-
-	if (clkscmi->ctrl_flags & SUPPORT_CLK_RATE_CONTROL)
+	if (ctrl_flags & SUPPORT_CLK_RATE_CONTROL)
 		return __scmi_clk_set_rate(clk, rate);
 
 	/* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */
@@ -375,20 +383,17 @@ static int __scmi_clk_set_parent(struct clk *clk, struct clk *parent)
 
 static int scmi_clk_set_parent(struct clk *clk, struct clk *parent)
 {
-	struct clk_scmi *clkscmi;
-	struct clk *c;
+	u32 ctrl_flags;
 	int ret;
 
 	if (!CONFIG_IS_ENABLED(CLK_CCF))
 		return __scmi_clk_set_parent(clk, parent);
 
-	ret = clk_get_by_id(clk->id, &c);
+	ret = scmi_clk_get_ctrl_flags(clk, &ctrl_flags);
 	if (ret)
 		return ret;
 
-	clkscmi = container_of(c, struct clk_scmi, clk);
-
-	if (clkscmi->ctrl_flags & SUPPORT_CLK_PARENT_CONTROL)
+	if (ctrl_flags & SUPPORT_CLK_PARENT_CONTROL)
 		return __scmi_clk_set_parent(clk, parent);
 
 	/* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */
-- 
2.51.0

[PATCH v2 3/4] clk: scmi: Postpone clock name resolution

[Marek Vasut]

The clock names are retrived via SCMI_CLOCK_ATTRIBUTES, called for each
clock ID. This may take a lot of time to complete and is not strictly
necessary. Register each clock as "scmi-%zu" instead, and let the first
call of SCMI_CLOCK_ATTRIBUTES fill in the actual clock name.

This has a side effect, which can be considered both an upside and also
a downside. Unused clock are never renamed and retain their placeholder
"scmi-%zu" name, which avoids empty clock names for nameless SCMI clock,
and avoids the name resolution and improves boot time. But for those
SCMI clock which do have name, that name is not listed until the clock
are used.

This is a preparatory patch for deferred issue of SCMI_CLOCK_ATTRIBUTES.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Alice Guo <alice.guo@nxp.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
V2: Add RB from Peng
---
 drivers/clk/clk_scmi.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 1a6a6ae464c..9bb41eddbba 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -17,6 +17,7 @@
 
 struct clk_scmi {
 	struct clk clk;
+	char name[SCMI_CLOCK_NAME_LENGTH_MAX];
 	u32 ctrl_flags;
 };
 
@@ -325,25 +326,21 @@ static int scmi_clk_probe(struct udevice *dev)
 		return -ENOMEM;
 
 	for (i = 0; i < num_clocks; i++) {
-		char *clock_name;
+		clk_scmi = clk_scmi_bulk + i;
+		char *clock_name = clk_scmi->name;
 		u32 attributes;
 
-		if (!scmi_clk_get_attibute(dev, i, &clock_name, &attributes)) {
-			clk_scmi = clk_scmi_bulk + i;
-			if (!clock_name)
-				ret = -ENOMEM;
-			else
-				ret = clk_register(&clk_scmi->clk, dev->driver->name,
-						   clock_name, dev->name);
-
-			if (ret) {
-				free(clock_name);
-				return ret;
-			}
+		snprintf(clock_name, SCMI_CLOCK_NAME_LENGTH_MAX, "scmi-%zu", i);
+
+		ret = clk_register(&clk_scmi->clk, dev->driver->name,
+				   clock_name, dev->name);
+		if (ret)
+			return ret;
 
-			dev_clk_dm(dev, i, &clk_scmi->clk);
-			dev_set_parent_priv(clk_scmi->clk.dev, priv);
+		dev_clk_dm(dev, i, &clk_scmi->clk);
+		dev_set_parent_priv(clk_scmi->clk.dev, priv);
 
+		if (!scmi_clk_get_attibute(dev, i, &clock_name, &attributes)) {
 			if (CLK_HAS_RESTRICTIONS(attributes)) {
 				u32 perm;
 
-- 
2.51.0

[PATCH v2 4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES

[Marek Vasut]

Instead of resolving clock control flags using SCMI_CLOCK_ATTRIBUTES
during probe for each and every clock, resolve the clock control
flags using SCMI_CLOCK_ATTRIBUTES when the clock control flags are
first used. Because most clock are never used by U-Boot, this allows
reducing the amount of SCMI_CLOCK_ATTRIBUTES considerably, and this
improve probe time of the scmi clock driver and U-Boot start up time.

On Renesas X5H, with 1700+ SCMI clock, the boot time improved by 1.7s .

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Alice Guo <alice.guo@nxp.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
V2: Add RB from Peng
---
 drivers/clk/clk_scmi.c | 50 ++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 9bb41eddbba..4073ff895cd 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -19,6 +19,7 @@ struct clk_scmi {
 	struct clk clk;
 	char name[SCMI_CLOCK_NAME_LENGTH_MAX];
 	u32 ctrl_flags;
+	bool attrs_resolved;
 };
 
 struct scmi_clock_priv {
@@ -86,7 +87,7 @@ static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
 	return 0;
 }
 
-static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name,
+static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char *name,
 				 u32 *attr)
 {
 	struct scmi_clock_priv *priv = dev_get_priv(dev);
@@ -110,7 +111,7 @@ static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name,
 		if (ret)
 			return ret;
 
-		*name = strdup(out.clock_name);
+		strncpy(name, out.clock_name, SCMI_CLOCK_NAME_LENGTH_MAX);
 		*attr = out.attributes;
 	} else {
 		struct scmi_clk_attribute_out out;
@@ -127,7 +128,7 @@ static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name,
 		if (ret)
 			return ret;
 
-		*name = strdup(out.clock_name);
+		strncpy(name, out.clock_name, SCMI_CLOCK_NAME_LENGTH_MAX);
 		*attr = out.attributes;
 	}
 
@@ -166,7 +167,9 @@ static int scmi_clk_gate(struct clk *clk, int enable)
 
 static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
 {
+	struct udevice *dev = clk->dev->parent;
 	struct clk_scmi *clkscmi;
+	u32 attributes;
 	struct clk *c;
 	int ret;
 
@@ -176,6 +179,31 @@ static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
 
 	clkscmi = container_of(c, struct clk_scmi, clk);
 
+	if (!clkscmi->attrs_resolved) {
+		char name[SCMI_CLOCK_NAME_LENGTH_MAX];
+		ret = scmi_clk_get_attibute(dev, clk->id & CLK_ID_MSK,
+					    name, &attributes);
+		if (ret)
+			return ret;
+
+		strncpy(clkscmi->name, name, SCMI_CLOCK_NAME_LENGTH_MAX);
+		if (CLK_HAS_RESTRICTIONS(attributes)) {
+			u32 perm;
+
+			ret = scmi_clk_get_permissions(dev, clk->id & CLK_ID_MSK, &perm);
+			if (ret < 0)
+				clkscmi->ctrl_flags = 0;
+			else
+				clkscmi->ctrl_flags = perm;
+		} else {
+			clkscmi->ctrl_flags = SUPPORT_CLK_STAT_CONTROL |
+					      SUPPORT_CLK_PARENT_CONTROL |
+					      SUPPORT_CLK_RATE_CONTROL;
+		}
+
+		clkscmi->attrs_resolved = true;
+	}
+
 	*ctrl_flags = clkscmi->ctrl_flags;
 
 	return 0;
@@ -328,7 +356,6 @@ static int scmi_clk_probe(struct udevice *dev)
 	for (i = 0; i < num_clocks; i++) {
 		clk_scmi = clk_scmi_bulk + i;
 		char *clock_name = clk_scmi->name;
-		u32 attributes;
 
 		snprintf(clock_name, SCMI_CLOCK_NAME_LENGTH_MAX, "scmi-%zu", i);
 
@@ -339,21 +366,6 @@ static int scmi_clk_probe(struct udevice *dev)
 
 		dev_clk_dm(dev, i, &clk_scmi->clk);
 		dev_set_parent_priv(clk_scmi->clk.dev, priv);
-
-		if (!scmi_clk_get_attibute(dev, i, &clock_name, &attributes)) {
-			if (CLK_HAS_RESTRICTIONS(attributes)) {
-				u32 perm;
-
-				ret = scmi_clk_get_permissions(dev, i, &perm);
-				if (ret < 0)
-					clk_scmi->ctrl_flags = 0;
-				else
-					clk_scmi->ctrl_flags = perm;
-			} else {
-				clk_scmi->ctrl_flags = SUPPORT_CLK_STAT_CONTROL | SUPPORT_CLK_PARENT_CONTROL |
-						       SUPPORT_CLK_RATE_CONTROL;
-			}
-		}
 	}
 
 	return 0;
-- 
2.51.0

Re: [PATCH v2 1/4] clk: scmi: Bulk allocate all sub-driver instance data

[Peng Fan (OSS)]

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


On Fri, 07 Nov 2025 04:01:23 +0100, Marek Vasut wrote:
> Allocate all sub-driver instance data at once. The amount of data that
> have to be allocated is known up front, so is the size of the data, so
> there is no need to call malloc() in a loop, mallocate all data at once.
> 
> The upside is, less heap fragmentation and fewer malloc() calls overall,
> and a faster boot time.
> 
> [...]

Applied to fsl-qoriq
ext, thanks!

[1/4] clk: scmi: Bulk allocate all sub-driver instance data
      commit: e8c2051fdc8bc3f59d36b614c3d6a1da04520d9d
[2/4] clk: scmi: Factor out clock control flags resolution
      commit: 624de0d04ae479d59b3061cf93ab77cc06cb2c86
[3/4] clk: scmi: Postpone clock name resolution
      commit: 88b89eaed86d9ba23b440b8a91b69925d74fa6fc
[4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES
      commit: 3ea7acae0d9cf73729cb0fad0056e519d3a2267c

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

Re: [PATCH v2 4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES

[Peng Fan]

Hi Marek,

On Fri, Nov 07, 2025 at 04:01:26AM +0100, Marek Vasut wrote:
>Instead of resolving clock control flags using SCMI_CLOCK_ATTRIBUTES
>during probe for each and every clock, resolve the clock control
>flags using SCMI_CLOCK_ATTRIBUTES when the clock control flags are
>first used. Because most clock are never used by U-Boot, this allows
>reducing the amount of SCMI_CLOCK_ATTRIBUTES considerably, and this
>improve probe time of the scmi clock driver and U-Boot start up time.
>
>On Renesas X5H, with 1700+ SCMI clock, the boot time improved by 1.7s .
>
> 
> static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
> {
>+	struct udevice *dev = clk->dev->parent;

I will change this to "clk->dev", otherwise there is CI failure.

No need to use clk->dev->parent here. The core code will
find out the scmi root device.

No need send v3, I could patch it in tree if no objections.

Thanks,
Peng.

> 	struct clk_scmi *clkscmi;
>+	u32 attributes;
> 	struct clk *c;
> 	int ret;

Re: [PATCH v2 4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES

[Marek Vasut]

On 11/7/25 1:16 PM, Peng Fan wrote:

Hello Peng,

> On Fri, Nov 07, 2025 at 04:01:26AM +0100, Marek Vasut wrote:
>> Instead of resolving clock control flags using SCMI_CLOCK_ATTRIBUTES
>> during probe for each and every clock, resolve the clock control
>> flags using SCMI_CLOCK_ATTRIBUTES when the clock control flags are
>> first used. Because most clock are never used by U-Boot, this allows
>> reducing the amount of SCMI_CLOCK_ATTRIBUTES considerably, and this
>> improve probe time of the scmi clock driver and U-Boot start up time.
>>
>> On Renesas X5H, with 1700+ SCMI clock, the boot time improved by 1.7s .
>>
>>
>> static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
>> {
>> +	struct udevice *dev = clk->dev->parent;
> 
> I will change this to "clk->dev", otherwise there is CI failure.

What kind of CI failure ?

> No need to use clk->dev->parent here. The core code will
> find out the scmi root device.
> 
> No need send v3, I could patch it in tree if no objections.
This change would be wrong, so please do not do that. Apply this patch:

"
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 4e8e5a262c8..99fd9679634 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -168,6 +168,7 @@ static int scmi_clk_gate(struct clk *clk, int enable)
  static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
  {
         struct udevice *dev = clk->dev->parent;
+printf("clk->dev->name=%s clk->dev->parent->name=%s\n", clk->dev->name, 
clk->dev->parent->name);
         struct clk_scmi *clkscmi;
         u32 attributes;
         struct clk *c;
"

When U-Boot starts, the output looks this way:

"
clk->dev->name=scmi-325 clk->dev->parent->name=protocol@14
"

Notice "clk->dev->parent->name=protocol@14" , which is correct, 
therefore the clk->dev->parent is correct.

With the "clk->dev" change applied, the wrong device is used and the 
system starts producing failures:

"
scmi-over-mailbox scmi: Buffer too small: hdr->length:32, out_msg_sz:24
"

What kind of CI failure do you observe ?

RE: [PATCH v2 4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES

[Peng Fan]

> Subject: Re: [PATCH v2 4/4] clk: scmi: Defer issue of
> SCMI_CLOCK_ATTRIBUTES
> >
> > I will change this to "clk->dev", otherwise there is CI failure.
> 
> What kind of CI failure ?

=> ut dm dm_test_scmi_clocks
Test: scmi_clocks: scmi.c
find_scmi_protocol_device() sandbox-scmi_agent scmi: Invalid SCMI device, agent not found
test/dm/scmi.c:426, dm_test_scmi_clocks(): !ret_dev || ret_dev == 1088
Test: scmi_clocks: scmi.c (flat tree)
find_scmi_protocol_device() sandbox-scmi_agent scmi: Invalid SCMI device, agent not found
test/dm/scmi.c:426, dm_test_scmi_clocks(): !ret_dev || ret_dev == 1088
Test 'scmi_clocks' failed 2 times
Tests run: 1, 11 ms, average: 11 ms, failures: 2
exit not allowed from main input shell.

Regards,
Peng.

> 
> > No need to use clk->dev->parent here. The core code will find out the
> > scmi root device.
> >
> > No need send v3, I could patch it in tree if no objections.
> This change would be wrong, so please do not do that. Apply this patch:
> 
> "
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index
> 4e8e5a262c8..99fd9679634 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -168,6 +168,7 @@ static int scmi_clk_gate(struct clk *clk, int
> enable)
>   static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
>   {
>          struct udevice *dev = clk->dev->parent;
> +printf("clk->dev->name=%s clk->dev->parent->name=%s\n", clk->dev-
> >name,
> clk->dev->parent->name);
>          struct clk_scmi *clkscmi;
>          u32 attributes;
>          struct clk *c;
> "
> 
> When U-Boot starts, the output looks this way:
> 
> "
> clk->dev->name=scmi-325 clk->dev->parent->name=protocol@14
> "
> 
> Notice "clk->dev->parent->name=protocol@14" , which is correct,
> therefore the clk->dev->parent is correct.
> 
> With the "clk->dev" change applied, the wrong device is used and the
> system starts producing failures:
> 
> "
> scmi-over-mailbox scmi: Buffer too small: hdr->length:32,
> out_msg_sz:24 "
> 
> What kind of CI failure do you observe ?

Re: [PATCH v2 4/4] clk: scmi: Defer issue of SCMI_CLOCK_ATTRIBUTES

[Marek Vasut]

On 11/8/25 9:02 AM, Peng Fan wrote:

Hello Peng,

>> Subject: Re: [PATCH v2 4/4] clk: scmi: Defer issue of
>> SCMI_CLOCK_ATTRIBUTES
>>>
>>> I will change this to "clk->dev", otherwise there is CI failure.
>>
>> What kind of CI failure ?
> 
> => ut dm dm_test_scmi_clocks
> Test: scmi_clocks: scmi.c
> find_scmi_protocol_device() sandbox-scmi_agent scmi: Invalid SCMI device, agent not found
> test/dm/scmi.c:426, dm_test_scmi_clocks(): !ret_dev || ret_dev == 1088
> Test: scmi_clocks: scmi.c (flat tree)
> find_scmi_protocol_device() sandbox-scmi_agent scmi: Invalid SCMI device, agent not found
> test/dm/scmi.c:426, dm_test_scmi_clocks(): !ret_dev || ret_dev == 1088
> Test 'scmi_clocks' failed 2 times
> Tests run: 1, 11 ms, average: 11 ms, failures: 2
> exit not allowed from main input shell.
It seems in the end, we were both right about the device parent. The 
function has to use dev->parent, but not clk->dev->parent, but the 
c->dev->parent resolved by clk_get_by_id() .

I sent a V3 with this change:

"
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 4e8e5a262c8..538ae054f4b 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -167,8 +167,8 @@ static int scmi_clk_gate(struct clk *clk, int enable)

  static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
  {
-	struct udevice *dev = clk->dev->parent;
  	struct clk_scmi *clkscmi;
+	struct udevice *dev;
  	u32 attributes;
  	struct clk *c;
  	int ret;
@@ -177,6 +177,8 @@ static int scmi_clk_get_ctrl_flags(struct clk *clk, 
u32 *ctrl_flags)
  	if (ret)
  		return ret;

+	dev = c->dev->parent;
+
  	clkscmi = container_of(c, struct clk_scmi, clk);

  	if (!clkscmi->attrs_resolved) {
"