]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
clk: scmi: Handle case where child clocks are initialized before their parents
authorSascha Hauer <s.hauer@pengutronix.de>
Thu, 12 Jun 2025 12:56:57 +0000 (14:56 +0200)
committerStephen Boyd <sboyd@kernel.org>
Fri, 20 Jun 2025 00:30:02 +0000 (17:30 -0700)
The SCMI clock driver currently assumes that parent clocks are always
initialized before their children. However, this assumption can fail if
a child clock is encountered before its parent during probe.

This leads to an issue during initialization of the parent_data array:

    sclk->parent_data[i].hw = hws[sclk->info->parents[i]];

If the parent clock's hardware structure has not been initialized yet,
this assignment results in invalid data.

To resolve this, allocate all struct scmi_clk instances as a contiguous
array at the beginning of the probe and populate the hws[] array
upfront. This ensures that any parent referenced later is already
initialized, regardless of the order in which clocks are processed.

Note that we can no longer free individual scmi_clk instances if
scmi_clk_ops_init() fails which shouldn't be a problem if the SCMI
platform has proper per-agent clock discovery.

Fixes: 65a8a3dd3b95f ("clk: scmi: Add support for clock {set,get}_parent")
Reviewed-by: peng.fan@nxp.com
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Link: https://lore.kernel.org/r/20250612-clk-scmi-children-parent-fix-v3-1-7de52a27593d@pengutronix.de
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
drivers/clk/clk-scmi.c

index 15510c2ff21c0335f5cb30677343bd4ef59c0738..1b1561c84127b9e41bfb6096ceb3626f32c4fee0 100644 (file)
@@ -404,6 +404,7 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
        const struct scmi_handle *handle = sdev->handle;
        struct scmi_protocol_handle *ph;
        const struct clk_ops *scmi_clk_ops_db[SCMI_MAX_CLK_OPS] = {};
+       struct scmi_clk *sclks;
 
        if (!handle)
                return -ENODEV;
@@ -430,18 +431,21 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
        transport_is_atomic = handle->is_transport_atomic(handle,
                                                          &atomic_threshold_us);
 
+       sclks = devm_kcalloc(dev, count, sizeof(*sclks), GFP_KERNEL);
+       if (!sclks)
+               return -ENOMEM;
+
+       for (idx = 0; idx < count; idx++)
+               hws[idx] = &sclks[idx].hw;
+
        for (idx = 0; idx < count; idx++) {
-               struct scmi_clk *sclk;
+               struct scmi_clk *sclk = &sclks[idx];
                const struct clk_ops *scmi_ops;
 
-               sclk = devm_kzalloc(dev, sizeof(*sclk), GFP_KERNEL);
-               if (!sclk)
-                       return -ENOMEM;
-
                sclk->info = scmi_proto_clk_ops->info_get(ph, idx);
                if (!sclk->info) {
                        dev_dbg(dev, "invalid clock info for idx %d\n", idx);
-                       devm_kfree(dev, sclk);
+                       hws[idx] = NULL;
                        continue;
                }
 
@@ -479,13 +483,11 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
                if (err) {
                        dev_err(dev, "failed to register clock %d\n", idx);
                        devm_kfree(dev, sclk->parent_data);
-                       devm_kfree(dev, sclk);
                        hws[idx] = NULL;
                } else {
                        dev_dbg(dev, "Registered clock:%s%s\n",
                                sclk->info->name,
                                scmi_ops->enable ? " (atomic ops)" : "");
-                       hws[idx] = &sclk->hw;
                }
        }