Add the Exynos ACPM clock driver. It provides support for clocks that
are controlled by firmware that implements the ACPM interface.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/clk/samsung/Kconfig | 10 ++
drivers/clk/samsung/Makefile | 1 +
drivers/clk/samsung/clk-acpm.c | 203 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig
index 76a494e95027af26272e30876a87ac293bd56dfa..70a8b82a0136b4d0213d8ff95e029c52436e5c7f 100644
--- a/drivers/clk/samsung/Kconfig
+++ b/drivers/clk/samsung/Kconfig
@@ -95,6 +95,16 @@ config EXYNOS_CLKOUT
status of the certains clocks from SoC, but it could also be tied to
other devices as an input clock.
+config EXYNOS_ACPM_CLK
+ tristate "Clock driver controlled via ACPM interface"
+ depends on EXYNOS_ACPM_PROTOCOL || (COMPILE_TEST && !EXYNOS_ACPM_PROTOCOL)
+ help
+ This driver provides support for clocks that are controlled by
+ firmware that implements the ACPM interface.
+
+ This driver uses the ACPM interface to interact with the firmware
+ providing all the clock controlls.
+
config TESLA_FSD_COMMON_CLK
bool "Tesla FSD clock controller support" if COMPILE_TEST
depends on COMMON_CLK_SAMSUNG
diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index b77fe288e4bb484c68d1ff497acc0b83d132ea03..04b63436b12f6f5169575d74f54b105e97bbb052 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynos990.o
obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynosautov9.o
obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynosautov920.o
obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-gs101.o
+obj-$(CONFIG_EXYNOS_ACPM_CLK) += clk-acpm.o
obj-$(CONFIG_S3C64XX_COMMON_CLK) += clk-s3c64xx.o
obj-$(CONFIG_S5PV210_COMMON_CLK) += clk-s5pv210.o clk-s5pv210-audss.o
obj-$(CONFIG_TESLA_FSD_COMMON_CLK) += clk-fsd.o
diff --git a/drivers/clk/samsung/clk-acpm.c b/drivers/clk/samsung/clk-acpm.c
new file mode 100644
index 0000000000000000000000000000000000000000..fe24471c41fcaab43b62b552949c26520b98c1e3
--- /dev/null
+++ b/drivers/clk/samsung/clk-acpm.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Samsung Exynos ACPM protocol based clock driver.
+ *
+ * Copyright 2025 Linaro Ltd.
+ */
+
+#include <dt-bindings/clock/google,gs101-acpm.h>
+#include <linux/array_size.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/firmware/samsung/exynos-acpm-protocol.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+enum acpm_clk_dev_type {
+ GS101_ACPM_CLK_ID,
+};
+
+struct acpm_clk {
+ u32 id;
+ struct clk_hw hw;
+ unsigned int mbox_chan_id;
+ const struct acpm_handle *handle;
+};
+
+struct acpm_clk_variant {
+ unsigned int id;
+ const char *name;
+};
+
+struct acpm_clk_driver_data {
+ const struct acpm_clk_variant *clks;
+ unsigned int nr_clks;
+ unsigned int mbox_chan_id;
+};
+
+#define to_acpm_clk(clk) container_of(clk, struct acpm_clk, hw)
+
+#define ACPM_CLK(_id, cname) \
+ { \
+ .id = _id, \
+ .name = cname, \
+ }
+
+static const struct acpm_clk_variant gs101_acpm_clks[] = {
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_MIF, "mif"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_INT, "int"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_CPUCL0, "cpucl0"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_CPUCL1, "cpucl1"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_CPUCL2, "cpucl2"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_G3D, "g3d"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_G3DL2, "g3dl2"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_TPU, "tpu"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_INTCAM, "intcam"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_TNR, "tnr"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_CAM, "cam"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_MFC, "mfc"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_DISP, "disp"),
+ ACPM_CLK(GS101_CLK_ACPM_DVFS_BO, "b0"),
+};
+
+static const struct acpm_clk_driver_data acpm_clk_gs101 = {
+ .clks = gs101_acpm_clks,
+ .nr_clks = ARRAY_SIZE(gs101_acpm_clks),
+ .mbox_chan_id = 0,
+};
+
+static unsigned long acpm_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct acpm_clk *clk = to_acpm_clk(hw);
+
+ return clk->handle->ops.dvfs_ops.get_rate(clk->handle,
+ clk->mbox_chan_id, clk->id, 0);
+}
+
+static int acpm_clk_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ /*
+ * We can't figure out what rate it will be, so just return the
+ * rate back to the caller. acpm_clk_recalc_rate() will be called
+ * after the rate is set and we'll know what rate the clock is
+ * running at then.
+ */
+ return 0;
+}
+
+static int acpm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct acpm_clk *clk = to_acpm_clk(hw);
+
+ return clk->handle->ops.dvfs_ops.set_rate(clk->handle,
+ clk->mbox_chan_id, clk->id, rate);
+}
+
+static const struct clk_ops acpm_clk_ops = {
+ .recalc_rate = acpm_clk_recalc_rate,
+ .determine_rate = acpm_clk_determine_rate,
+ .set_rate = acpm_clk_set_rate,
+};
+
+static int acpm_clk_ops_init(struct device *dev, struct acpm_clk *aclk,
+ const char *name)
+{
+ struct clk_init_data init = {};
+
+ init.name = name;
+ init.ops = &acpm_clk_ops;
+ aclk->hw.init = &init;
+
+ return devm_clk_hw_register(dev, &aclk->hw);
+}
+
+static int acpm_clk_probe(struct platform_device *pdev)
+{
+ enum acpm_clk_dev_type type = platform_get_device_id(pdev)->driver_data;
+ const struct acpm_clk_driver_data *drv_data;
+ const struct acpm_handle *acpm_handle;
+ struct clk_hw_onecell_data *clk_data;
+ struct clk_hw **hws;
+ struct device *dev = &pdev->dev;
+ struct acpm_clk *aclks;
+ unsigned int mbox_chan_id;
+ int i, err, count;
+
+ switch (type) {
+ case GS101_ACPM_CLK_ID:
+ drv_data = &acpm_clk_gs101;
+ break;
+ default:
+ return dev_err_probe(dev, -EINVAL, "Invalid device type\n");
+ }
+
+ acpm_handle = devm_acpm_get_by_node(dev, dev->parent->of_node);
+ if (IS_ERR(acpm_handle))
+ return dev_err_probe(dev, PTR_ERR(acpm_handle),
+ "Failed to get acpm handle.\n");
+
+ 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);
+ if (!clk_data)
+ return -ENOMEM;
+
+ clk_data->num = count;
+ hws = clk_data->hws;
+
+ aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
+ if (!aclks)
+ return -ENOMEM;
+
+ for (i = 0; i < count; i++) {
+ const struct acpm_clk_variant *variant = &drv_data->clks[i];
+ unsigned int id = variant->id;
+ struct acpm_clk *aclk;
+
+ if (id >= count)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid ACPM clock ID.\n");
+
+ aclk = &aclks[id];
+ aclk->id = id;
+ aclk->handle = acpm_handle;
+ aclk->mbox_chan_id = mbox_chan_id;
+
+ hws[id] = &aclk->hw;
+
+ err = acpm_clk_ops_init(dev, aclk, variant->name);
+ 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);
+}
+
+static const struct platform_device_id acpm_clk_id[] = {
+ { "gs101-acpm-clk", GS101_ACPM_CLK_ID },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, acpm_clk_id);
+
+static struct platform_driver acpm_clk_driver = {
+ .driver = {
+ .name = "acpm-clocks",
+ },
+ .probe = acpm_clk_probe,
+ .id_table = acpm_clk_id,
+};
+module_platform_driver(acpm_clk_driver);
+
+MODULE_AUTHOR("Tudor Ambarus <tudor.ambarus@linaro.org>");
+MODULE_DESCRIPTION("Samsung Exynos ACPM clock driver");
+MODULE_LICENSE("GPL");
--
2.51.0.338.gd7d06c2dae-googOn 03/09/2025 15:56, Tudor Ambarus wrote:
> +
> +static int acpm_clk_probe(struct platform_device *pdev)
> +{
> + enum acpm_clk_dev_type type = platform_get_device_id(pdev)->driver_data;
> + const struct acpm_clk_driver_data *drv_data;
> + const struct acpm_handle *acpm_handle;
> + struct clk_hw_onecell_data *clk_data;
> + struct clk_hw **hws;
> + struct device *dev = &pdev->dev;
> + struct acpm_clk *aclks;
> + unsigned int mbox_chan_id;
> + int i, err, count;
> +
> + switch (type) {
> + case GS101_ACPM_CLK_ID:
> + drv_data = &acpm_clk_gs101;
Just use acpm_clk_gs101 directly (see also further comment).
> + break;
> + default:
> + return dev_err_probe(dev, -EINVAL, "Invalid device type\n");
> + }
> +
> + acpm_handle = devm_acpm_get_by_node(dev, dev->parent->of_node);
> + if (IS_ERR(acpm_handle))
> + return dev_err_probe(dev, PTR_ERR(acpm_handle),
> + "Failed to get acpm handle.\n");
> +
> + 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);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + clk_data->num = count;
> + hws = clk_data->hws;
> +
> + aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
> + if (!aclks)
> + return -ENOMEM;
> +
> + for (i = 0; i < count; i++) {
> + const struct acpm_clk_variant *variant = &drv_data->clks[i];
> + unsigned int id = variant->id;
> + struct acpm_clk *aclk;
> +
> + if (id >= count)
This is not possible. You control the IDs build time, so this must be
either build time check or no check. I vote for no check, because I
don't think the ID is anyhow related to number of clocks. What if (not
recommended but what if) the IDs have a gap and next ID is 1000. I see
your code using ID:
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid ACPM clock ID.\n");
> +
> + aclk = &aclks[id];
> + aclk->id = id;
> + aclk->handle = acpm_handle;
> + aclk->mbox_chan_id = mbox_chan_id;
> +
> + hws[id] = &aclk->hw;
^^^ here, but why do you need it? Why it cannot be hws[i]?
> +
> + err = acpm_clk_ops_init(dev, aclk, variant->name);
> + 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);
> +}
> +
> +static const struct platform_device_id acpm_clk_id[] = {
> + { "gs101-acpm-clk", GS101_ACPM_CLK_ID },
Please drop GS101_ACPM_CLK_ID here and in other places. It's dead code
now. It should be introduced with next users/devices.
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, acpm_clk_id);
Best regards,
Krzysztof
On 9/6/25 1:19 PM, Krzysztof Kozlowski wrote:
> On 03/09/2025 15:56, Tudor Ambarus wrote:
>> +
>> +static int acpm_clk_probe(struct platform_device *pdev)
>> +{
>> + enum acpm_clk_dev_type type = platform_get_device_id(pdev)->driver_data;
>> + const struct acpm_clk_driver_data *drv_data;
>> + const struct acpm_handle *acpm_handle;
>> + struct clk_hw_onecell_data *clk_data;
>> + struct clk_hw **hws;
>> + struct device *dev = &pdev->dev;
>> + struct acpm_clk *aclks;
>> + unsigned int mbox_chan_id;
>> + int i, err, count;
>> +
>> + switch (type) {
>> + case GS101_ACPM_CLK_ID:
>> + drv_data = &acpm_clk_gs101;
>
> Just use acpm_clk_gs101 directly (see also further comment).
okay
>
>> + break;
>> + default:
>> + return dev_err_probe(dev, -EINVAL, "Invalid device type\n");
>> + }
>> +
>> + acpm_handle = devm_acpm_get_by_node(dev, dev->parent->of_node);
>> + if (IS_ERR(acpm_handle))
>> + return dev_err_probe(dev, PTR_ERR(acpm_handle),
>> + "Failed to get acpm handle.\n");
>> +
>> + 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);
>> + if (!clk_data)
>> + return -ENOMEM;
>> +
>> + clk_data->num = count;
>> + hws = clk_data->hws;
>> +
>> + aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
>> + if (!aclks)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < count; i++) {
>> + const struct acpm_clk_variant *variant = &drv_data->clks[i];
>> + unsigned int id = variant->id;
>> + struct acpm_clk *aclk;
>> +
>> + if (id >= count)
>
> This is not possible. You control the IDs build time, so this must be
> either build time check or no check. I vote for no check, because I
using BUILD_BUG_ON_MSG? that would work, see below the why.
> don't think the ID is anyhow related to number of clocks. What if (not
> recommended but what if) the IDs have a gap and next ID is 1000. I see
> your code using ID:
>
>
>> + return dev_err_probe(dev, -EINVAL,
>> + "Invalid ACPM clock ID.\n");
>> +
>> + aclk = &aclks[id];
>> + aclk->id = id;
>> + aclk->handle = acpm_handle;
>> + aclk->mbox_chan_id = mbox_chan_id;
>> +
>> + hws[id] = &aclk->hw;
>
> ^^^ here, but why do you need it? Why it cannot be hws[i]?
so that it works correctly with of_clk_hw_onecell_get() in case the clocks
IDs are not starting from 0 or are reordered when defined. For example let's
consider clock ID 1 is wrongly defined at index 0 in the array. When someone
references clock ID 1 in the device tree, and we use of_clk_hw_onecell_get,
it would get the clock defined at index 1.
In my case the clocks start from index 0 and they are defined in ascending
order with no gaps, so the check is gratuitously made. I wanted to have some
sanity check. Do you still think I shall remove the check and use hws[i]?
>
>> +
>> + err = acpm_clk_ops_init(dev, aclk, variant->name);
>> + 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);
>> +}
>> +
>> +static const struct platform_device_id acpm_clk_id[] = {
>> + { "gs101-acpm-clk", GS101_ACPM_CLK_ID },
>
> Please drop GS101_ACPM_CLK_ID here and in other places. It's dead code
> now. It should be introduced with next users/devices.
okay. Thanks for the review!
>
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, acpm_clk_id);
>
> Best regards,
> Krzysztof
On 08/09/2025 09:39, Tudor Ambarus wrote:
>>> +
>>> + aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
>>> + if (!aclks)
>>> + return -ENOMEM;
>>> +
>>> + for (i = 0; i < count; i++) {
>>> + const struct acpm_clk_variant *variant = &drv_data->clks[i];
>>> + unsigned int id = variant->id;
>>> + struct acpm_clk *aclk;
>>> +
>>> + if (id >= count)
>>
>> This is not possible. You control the IDs build time, so this must be
>> either build time check or no check. I vote for no check, because I
>
> using BUILD_BUG_ON_MSG? that would work, see below the why.
>
>> don't think the ID is anyhow related to number of clocks. What if (not
>> recommended but what if) the IDs have a gap and next ID is 1000. I see
>> your code using ID:
>>
>>
>>> + return dev_err_probe(dev, -EINVAL,
>>> + "Invalid ACPM clock ID.\n");
>>> +
>>> + aclk = &aclks[id];
>>> + aclk->id = id;
>>> + aclk->handle = acpm_handle;
>>> + aclk->mbox_chan_id = mbox_chan_id;
>>> +
>>> + hws[id] = &aclk->hw;
>>
>> ^^^ here, but why do you need it? Why it cannot be hws[i]?
>
> so that it works correctly with of_clk_hw_onecell_get() in case the clocks
Ah true, hws[] has to be indexed by ID.
> IDs are not starting from 0 or are reordered when defined. For example let's
> consider clock ID 1 is wrongly defined at index 0 in the array. When someone
> references clock ID 1 in the device tree, and we use of_clk_hw_onecell_get,
> it would get the clock defined at index 1.
>
> In my case the clocks start from index 0 and they are defined in ascending
> order with no gaps, so the check is gratuitously made. I wanted to have some
> sanity check. Do you still think I shall remove the check and use hws[i]?
Look at some users of of_clk_hw_onecell_get() - they all don't care
about this and do:
441 for (idx = 0; idx < count; idx++) {
442 struct scmi_clk *sclk = &sclks[idx];
without any checks.
I just do not see why runtime check is necessary. This is purely build
time relation and either we do not care, because the code should be
synced between one and other place, or (if you care) then it must be
build time check.
Best regards,
Krzysztof
On 9/8/25 8:45 AM, Krzysztof Kozlowski wrote:
> On 08/09/2025 09:39, Tudor Ambarus wrote:
>>>> +
>>>> + aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
>>>> + if (!aclks)
>>>> + return -ENOMEM;
>>>> +
>>>> + for (i = 0; i < count; i++) {
>>>> + const struct acpm_clk_variant *variant = &drv_data->clks[i];
>>>> + unsigned int id = variant->id;
>>>> + struct acpm_clk *aclk;
>>>> +
>>>> + if (id >= count)
>>>
>>> This is not possible. You control the IDs build time, so this must be
>>> either build time check or no check. I vote for no check, because I
>>
>> using BUILD_BUG_ON_MSG? that would work, see below the why.
>>
>>> don't think the ID is anyhow related to number of clocks. What if (not
>>> recommended but what if) the IDs have a gap and next ID is 1000. I see
>>> your code using ID:
>>>
>>>
>>>> + return dev_err_probe(dev, -EINVAL,
>>>> + "Invalid ACPM clock ID.\n");
>>>> +
>>>> + aclk = &aclks[id];
>>>> + aclk->id = id;
>>>> + aclk->handle = acpm_handle;
>>>> + aclk->mbox_chan_id = mbox_chan_id;
>>>> +
>>>> + hws[id] = &aclk->hw;
>>>
>>> ^^^ here, but why do you need it? Why it cannot be hws[i]?
>>
>> so that it works correctly with of_clk_hw_onecell_get() in case the clocks
>
> Ah true, hws[] has to be indexed by ID.
>
>> IDs are not starting from 0 or are reordered when defined. For example let's
>> consider clock ID 1 is wrongly defined at index 0 in the array. When someone
>> references clock ID 1 in the device tree, and we use of_clk_hw_onecell_get,
>> it would get the clock defined at index 1.
>>
>> In my case the clocks start from index 0 and they are defined in ascending
>> order with no gaps, so the check is gratuitously made. I wanted to have some
>> sanity check. Do you still think I shall remove the check and use hws[i]?
>
>
> Look at some users of of_clk_hw_onecell_get() - they all don't care
> about this and do:
>
> 441 for (idx = 0; idx < count; idx++) {
> 442 struct scmi_clk *sclk = &sclks[idx];
>
> without any checks.
I saw, it felt a bit rugged at first when reading it, but not so more now,
see below why.
>
> I just do not see why runtime check is necessary. This is purely build
> time relation and either we do not care, because the code should be
> synced between one and other place, or (if you care) then it must be
> build time check.
>
I tried the following:
+/*
+ * Use a static assertion to check that the clock IDs are sequential
+ * and do not have gaps. This check is performed at compile-time.
+ */
+static void acpm_clk_build_check(void)
+{
+ BUILD_BUG_ON_MSG(gs101_acpm_clks[ARRAY_SIZE(gs101_acpm_clks) - 1].id !=
+ (ARRAY_SIZE(gs101_acpm_clks) - 1),
+ "ACPM clock IDs are not sequential or have gaps.");
+}
and then in probe() called it. It works in a few cases, but it leaves the
possibility open for the intermediate clocks to have unrestricted values,
even if last-clk-id == nr-clks - 1;
So to be comprehensive I'd have to combine a build time check with a run-time
check. Which feels like over engineering. The assumptions scmi and other do
don't look that bad now :).
I'll drop the sanity checks and use hws[i] instead of hws[id] so that at
least there's no out of array accesses in case the writer really mangles
the clock definitions.
Thanks,
ta
On 9/8/25 11:55 AM, Tudor Ambarus wrote: > I'll drop the sanity checks and use hws[i] instead of hws[id] so that at > least there's no out of array accesses in case the writer really mangles > the clock definitions. Having the assumption that the clock IDs start form zero, are sequential and do not have gaps makes the inclusion of dt-bindings/clock/google,gs101-acpm.h unnecessary, so I removed that as well in v4. The clocks are now defined solely by name in the driver. Wanted to emphasize why the acpm-clk driver will no longer depend on the bindings in v4 :)
© 2016 - 2025 Red Hat, Inc.