[PATCH v2 0/2] Initial support for Exynos850 SoC in ACPM clk driver

[PATCH v2 0/2] Initial support for Exynos850 SoC in ACPM clk driver

[Alexey Klimov]

This series adds support for the Exynos850 SoC to the Samsung ACPM clock
driver. 

Currently, the driver hardcodes the GS101 clock variants directly within
its probe() routine. To cleanly support Exynos850 (and potentially other
SoCs that utilize the ACPM protocol in future), this series first
refactors the driver a little bit to be SoC-agnostic, and then wires up
the new Exynos850 data.

First patch introduces `driver_data` to dynamically extract clock lists,
number of clocks, names of clocks, and mailbox channel id, removing the
GS101 hardcoding.

Second patch introduces the `exynos850_acpm_clks` array and
"exynos850-acpm-clk" device ID.

This version also tries to handle missing ->recaclk_rate() implementation
in ACPM firmware. Because the Exynos850 ACPM firmware does not support
dynamic rate reads over ACPM IPC, the second patch introduces
a ->get_rate() callback mechanism. For Exynos850, this falls back to
parent CMU clock topology, registering parent data via pdata.fw_name.

Tested with cpufreq for E850, with OPPs and cpufreq_dt on Winlink E850-96
board.

I decided not to add Tudor's trailer for the second patch because it seems
that rework is quite important.
The dt-bindings for clocks will be added as a separate commit in another
series.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
Changes in v2:
- changed commit message of second patch that adds support for Exynos850;
- introduced ->get_rate() callback for Exynos850;
  Rework of second patch and implementation of get_rate from parent clock;
  ACPM clocks for Exynos850 now rely on clock topology and need parent
  clocks to be provided;
- reworked acpm_clk_recalc_rate() to use driver data calback if it is
  available;
- struct acpm_clk now has pointer to drv_data;
- rebased and added changes associated with rebase;
- added mod_devicetable.h as Tudor suggested, added trailer in the first
  commit;
- Link to v1: https://lore.kernel.org/r/20260512-exynos850-acpm-clk-v1-0-837532ddbf38@linaro.org

---
Alexey Klimov (2):
      clk: samsung: acpm: introduce driver data for SoC-specific clocks
      clk: samsung: acpm: add Exynos850 support

 drivers/clk/samsung/clk-acpm.c | 62 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 6 deletions(-)
---
base-commit: 1a1de54f7369cd2b5bac0f265910e60ad3a6b4c3
change-id: 20260512-exynos850-acpm-clk-bed1c23b66eb

Best regards,
-- 
Alexey Klimov <alexey.klimov@linaro.org>

[PATCH v2 1/2] clk: samsung: acpm: introduce driver data for SoC-specific clocks

[Alexey Klimov]

Currently, the ACPM clock driver hardcodes the GS101 clock variant
arrays and counts directly in probe() routine. To support additional
other SoCs (for instance Exynos850 SoC) that use the same ACPM protocol
but have different clock trees, the driver needs to be more flexible.

Introduce driver data attached to the platform_device_id table.
Update the probe function to dynamically extract the clock lists,
number of clocks, its names and mailbox channel id based on the matching
device ID.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/clk/samsung/clk-acpm.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/samsung/clk-acpm.c b/drivers/clk/samsung/clk-acpm.c
index 7680bc8c5275..4b68b44fe615 100644
--- a/drivers/clk/samsung/clk-acpm.c
+++ b/drivers/clk/samsung/clk-acpm.c
@@ -12,6 +12,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/firmware/samsung/exynos-acpm-protocol.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/types.h>
@@ -101,6 +102,8 @@ static int acpm_clk_register(struct device *dev, struct acpm_clk *aclk,
 
 static int acpm_clk_probe(struct platform_device *pdev)
 {
+	const struct acpm_clk_driver_data *drv_data;
+	const struct platform_device_id *id;
 	struct acpm_handle *acpm_handle;
 	struct clk_hw_onecell_data *clk_data;
 	struct clk_hw **hws;
@@ -114,8 +117,14 @@ static int acpm_clk_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(acpm_handle),
 				     "Failed to get acpm handle\n");
 
-	count = acpm_clk_gs101.nr_clks;
-	mbox_chan_id = acpm_clk_gs101.mbox_chan_id;
+	id = platform_get_device_id(pdev);
+	if (!id || !id->driver_data)
+		return -ENODEV;
+
+	drv_data  = (const struct acpm_clk_driver_data *)id->driver_data;
+
+	count = drv_data->nr_clks;
+	mbox_chan_id = drv_data->mbox_chan_id;
 
 	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, count),
 				GFP_KERNEL);
@@ -142,8 +151,7 @@ static int acpm_clk_probe(struct platform_device *pdev)
 
 		hws[i] = &aclk->hw;
 
-		err = acpm_clk_register(dev, aclk,
-					acpm_clk_gs101.clks[i].name);
+		err = acpm_clk_register(dev, aclk, drv_data->clks[i].name);
 		if (err)
 			return dev_err_probe(dev, err,
 					     "Failed to register clock\n");
@@ -154,7 +162,7 @@ static int acpm_clk_probe(struct platform_device *pdev)
 }
 
 static const struct platform_device_id acpm_clk_id[] = {
-	{ .name = "gs101-acpm-clk" },
+	{ .name = "gs101-acpm-clk", (kernel_ulong_t)&acpm_clk_gs101 },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, acpm_clk_id);

-- 
2.51.0

[PATCH v2 2/2] clk: samsung: acpm: add Exynos850 support

[Alexey Klimov]

The Exynos850 SoC utilizes the APM co-processor for clock management
via ACPM, similar to the GS101 SoC.

Add the Exynos850-specific clock variant array (including mif, int,
cpucl0, cpucl1, g3d, aud, cam, disp, and cp) and wire it up to the
driver data infrastructure. Add the "exynos850-acpm-clk" device ID
to enable dynamic instantiation of these clocks during probe().

Unlike GS101, the ACPM firmware on Exynos850 does not implement a
dynamic rate read call over ACPM IPC. To return sane clock rate in
->recalc_rate introduce a SoC-specific ->get_rate() driver data
callback. For Exynos850, this falls back to reading the upstream parent
CMU clock rate.
When ->get_rate() is set, populate pdata.fw_name during clock registration
to let clock framework to link the parent clock.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/clk/samsung/clk-acpm.c | 46 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-acpm.c b/drivers/clk/samsung/clk-acpm.c
index 4b68b44fe615..9f46fbdd999d 100644
--- a/drivers/clk/samsung/clk-acpm.c
+++ b/drivers/clk/samsung/clk-acpm.c
@@ -22,6 +22,7 @@ struct acpm_clk {
 	struct clk_hw hw;
 	unsigned int mbox_chan_id;
 	struct acpm_handle *handle;
+	const struct acpm_clk_driver_data *drv_data;
 };
 
 struct acpm_clk_variant {
@@ -32,6 +33,8 @@ struct acpm_clk_driver_data {
 	const struct acpm_clk_variant *clks;
 	unsigned int nr_clks;
 	unsigned int mbox_chan_id;
+	/* For SoCs where ACPM firmware doesn't implement get_rate call */
+	unsigned long (*get_rate)(struct clk_hw *clk, unsigned long parent_rate);
 };
 
 #define to_acpm_clk(clk) container_of(clk, struct acpm_clk, hw)
@@ -41,6 +44,18 @@ struct acpm_clk_driver_data {
 		.name		= cname,		\
 	}
 
+static const struct acpm_clk_variant exynos850_acpm_clks[] = {
+	ACPM_CLK("mif"),
+	ACPM_CLK("int"),
+	ACPM_CLK("cpucl0"),
+	ACPM_CLK("cpucl1"),
+	ACPM_CLK("g3d"),
+	ACPM_CLK("aud"),
+	ACPM_CLK("cam"),
+	ACPM_CLK("disp"),
+	ACPM_CLK("cp"),
+};
+
 static const struct acpm_clk_variant gs101_acpm_clks[] = {
 	ACPM_CLK("mif"),
 	ACPM_CLK("int"),
@@ -58,6 +73,19 @@ static const struct acpm_clk_variant gs101_acpm_clks[] = {
 	ACPM_CLK("bo"),
 };
 
+static unsigned long bypass_acpm_exynos850_get_rate(struct clk_hw *hw,
+						    unsigned long parent_rate)
+{
+	return parent_rate;
+}
+
+static const struct acpm_clk_driver_data acpm_clk_exynos850 = {
+	.clks = exynos850_acpm_clks,
+	.nr_clks = ARRAY_SIZE(exynos850_acpm_clks),
+	.mbox_chan_id = 0,
+	.get_rate = bypass_acpm_exynos850_get_rate,
+};
+
 static const struct acpm_clk_driver_data acpm_clk_gs101 = {
 	.clks = gs101_acpm_clks,
 	.nr_clks = ARRAY_SIZE(gs101_acpm_clks),
@@ -69,6 +97,9 @@ static unsigned long acpm_clk_recalc_rate(struct clk_hw *hw,
 {
 	struct acpm_clk *clk = to_acpm_clk(hw);
 
+	if (clk->drv_data->get_rate)
+		return clk->drv_data->get_rate(hw, parent_rate);
+
 	return clk->handle->ops->dvfs.get_rate(clk->handle, clk->mbox_chan_id,
 					       clk->id);
 }
@@ -89,14 +120,23 @@ static const struct clk_ops acpm_clk_ops = {
 };
 
 static int acpm_clk_register(struct device *dev, struct acpm_clk *aclk,
-			     const char *name)
+			     const char *name, const struct acpm_clk_driver_data *drv_data)
 {
 	struct clk_init_data init = {};
+	struct clk_parent_data pdata = {};
 
 	init.name = name;
 	init.ops = &acpm_clk_ops;
 	aclk->hw.init = &init;
 
+	/* If get_rate is set, the SoC relies on parent topology */
+	if (drv_data->get_rate) {
+		pdata.fw_name = name;
+		init.parent_data = &pdata;
+		init.num_parents = 1;
+		init.flags = CLK_GET_RATE_NOCACHE;
+	}
+
 	return devm_clk_hw_register(dev, &aclk->hw);
 }
 
@@ -145,13 +185,14 @@ static int acpm_clk_probe(struct platform_device *pdev)
 		 * The code assumes the clock IDs start from zero,
 		 * are sequential and do not have gaps.
 		 */
+		aclk->drv_data = drv_data;
 		aclk->id = i;
 		aclk->handle = acpm_handle;
 		aclk->mbox_chan_id = mbox_chan_id;
 
 		hws[i] = &aclk->hw;
 
-		err = acpm_clk_register(dev, aclk, drv_data->clks[i].name);
+		err = acpm_clk_register(dev, aclk, drv_data->clks[i].name, drv_data);
 		if (err)
 			return dev_err_probe(dev, err,
 					     "Failed to register clock\n");
@@ -162,6 +203,7 @@ static int acpm_clk_probe(struct platform_device *pdev)
 }
 
 static const struct platform_device_id acpm_clk_id[] = {
+	{ .name = "exynos850-acpm-clk", (kernel_ulong_t)&acpm_clk_exynos850 },
 	{ .name = "gs101-acpm-clk", (kernel_ulong_t)&acpm_clk_gs101 },
 	{ }
 };

-- 
2.51.0

Re: [PATCH v2 2/2] clk: samsung: acpm: add Exynos850 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clk: samsung: acpm: broken rate tracking and missing CLK_SET_RATE_PARENT
- [High] clk: samsung: acpm: missing of_node causes orphaned clocks and missing DT provider
--

commit c667cadd8b070f358391c7469c777edfe81f6617
Author: Alexey Klimov <alexey.klimov@linaro.org>

clk: samsung: acpm: add Exynos850 support

This commit adds Exynos850 support to the Samsung ACPM clock driver. It
introduces a SoC-specific get_rate() callback because the Exynos850 ACPM
firmware doesn't implement a dynamic rate read call. When get_rate() is
set, it populates pdata.fw_name to link the parent clock.

> diff --git a/drivers/clk/samsung/clk-acpm.c b/drivers/clk/samsung/clk-acpm.c
> index 4b68b44fe6158..9f46fbdd999d4 100644
> --- a/drivers/clk/samsung/clk-acpm.c
> +++ b/drivers/clk/samsung/clk-acpm.c
[ ... ]
> @@ -58,6 +73,19 @@ static const struct acpm_clk_variant gs101_acpm_clks[] = {
>  	ACPM_CLK("bo"),
>  };
>  
> +static unsigned long bypass_acpm_exynos850_get_rate(struct clk_hw *hw,
> +						    unsigned long parent_rate)
> +{
> +	return parent_rate;
> +}

[Severity: High]
If acpm_clk_set_rate() alters the hardware frequency via IPC, doesn't blindly
returning the parent rate here break the clock framework's rate tracking?

Since the clock lacks the CLK_SET_RATE_PARENT flag, the parent's cached rate
isn't updated when the rate is set. It seems clk_get_rate() will return the
stale parent rate even after a successful clk_set_rate().

> +
> +static const struct acpm_clk_driver_data acpm_clk_exynos850 = {
[ ... ]
> @@ -89,14 +120,23 @@ static const struct clk_ops acpm_clk_ops = {
>  };
>  
>  static int acpm_clk_register(struct device *dev, struct acpm_clk *aclk,
> -			     const char *name)
> +			     const char *name, const struct acpm_clk_driver_data *drv_data)
>  {
>  	struct clk_init_data init = {};
> +	struct clk_parent_data pdata = {};
>  
>  	init.name = name;
>  	init.ops = &acpm_clk_ops;
>  	aclk->hw.init = &init;
>  
> +	/* If get_rate is set, the SoC relies on parent topology */
> +	if (drv_data->get_rate) {
> +		pdata.fw_name = name;
> +		init.parent_data = &pdata;
> +		init.num_parents = 1;
> +		init.flags = CLK_GET_RATE_NOCACHE;
> +	}

[Severity: High]
Since the acpm-clocks platform device is manually created by the parent driver
and lacks an of_node, will setting pdata.fw_name fail to resolve the parent?

When core->of_node inherits NULL from dev->of_node, it appears clk_core_get()
skips the of_parse_clkspec() lookup and falls back to legacy clkdev, which
fails. Could this result in permanently orphaned clocks where the parent rate
evaluates to 0?

Also, is CLK_SET_RATE_PARENT needed here to propagate rate changes upstream?

> +
>  	return devm_clk_hw_register(dev, &aclk->hw);
>  }
[ ... ]
> @@ -145,13 +185,14 @@ static int acpm_clk_probe(struct platform_device *pdev)
[ ... ]
> -		err = acpm_clk_register(dev, aclk, drv_data->clks[i].name);
> +		err = acpm_clk_register(dev, aclk, drv_data->clks[i].name, drv_data);
>  		if (err)
>  			return dev_err_probe(dev, err,
>  					     "Failed to register clock\n");
>  	}
>  
>  	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
>  					   clk_data);
>  }

[Severity: High]
Does this fail to expose the clock provider to Device Tree consumers?

If dev->of_node is NULL, devm_of_clk_add_hw_provider() will silently return 0
without actually registering the hardware provider.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-exynos850-acpm-clk-v2-0-ffad4a6b3fb1@linaro.org?part=2

Re: [PATCH v2 0/2] Initial support for Exynos850 SoC in ACPM clk driver

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

Hello,

On Tue, Sep 15, 2026 at 12:05:42AM +0100, Alexey Klimov wrote:
> - added mod_devicetable.h as Tudor suggested, added trailer in the first
>   commit;

Argh, the reasoning in that suggestion was:

	you need to include linux/mod_devicetable.h for kernel_ulong_t.

(https://lore.kernel.org/all/1c41ecc9-d951-45df-b933-1934e1381663@linaro.org/).

This is wrong:

	$ git grep kernel_ulong_t v7.3-rc1:include/linux/mod_devicetable.h || echo void
	void

Please drop the inclusion of <linux/mod_devicetable.h> again, the only
effect of including is that you introduce a bunch of unneeded build
dependencies and I'm fighting to get rid of that file.

Also note that the driver is already using struct platform_device_id
which obviously has a member with type kernel_ulong_t. If you'd need an
additional header for kernel_ulong_t, the driver wouldn't have compiled
already before.

<linux/platform_device.h> already pulls in the definition of struct
platform_device_id and thus also kernel_ulong_t. Please rely on that or
if you want to go full iwyu include <linux/device-id/platformh.>

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/2] Initial support for Exynos850 SoC in ACPM clk driver

[Krzysztof Kozlowski]

On Tue, 15 Sep 2026 00:05:42 +0100, Alexey Klimov wrote:
> This series adds support for the Exynos850 SoC to the Samsung ACPM clock
> driver.
> 
> Currently, the driver hardcodes the GS101 clock variants directly within
> its probe() routine. To cleanly support Exynos850 (and potentially other
> SoCs that utilize the ACPM protocol in future), this series first
> refactors the driver a little bit to be SoC-agnostic, and then wires up
> the new Exynos850 data.
> 
> [...]

Applied, thanks!

[1/2] clk: samsung: acpm: introduce driver data for SoC-specific clocks
      https://git.kernel.org/krzk/linux/c/802a1a97c41dff82a557223f7a43655fc02920bd
[2/2] clk: samsung: acpm: add Exynos850 support
      https://git.kernel.org/krzk/linux/c/13634cde9771811488847ef76cd64a785599fdb5

Best regards,
-- 
Krzysztof Kozlowski <krzk@kernel.org>