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>
Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
Tested-by: Peter Griffin <peter.griffin@linaro.org> # on gs101-oriole
---
drivers/clk/samsung/Kconfig | 10 +++
drivers/clk/samsung/Makefile | 1 +
drivers/clk/samsung/clk-acpm.c | 185 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 196 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 ef464f434740f96623f9df62f94e2903e14e2226..f3657f2e1b98c6f431ab1f04c2d2a44fe317261b 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -28,6 +28,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..b90809ce3f882c489114c9d7299417d7fe373749
--- /dev/null
+++ b/drivers/clk/samsung/clk-acpm.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Samsung Exynos ACPM protocol based clock driver.
+ *
+ * Copyright 2025 Linaro Ltd.
+ */
+
+#include <linux/array_size.h>
+#include <linux/clk-provider.h>
+#include <linux/container_of.h>
+#include <linux/device/devres.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/firmware/samsung/exynos-acpm-protocol.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+struct acpm_clk {
+ u32 id;
+ struct clk_hw hw;
+ unsigned int mbox_chan_id;
+ const struct acpm_handle *handle;
+};
+
+struct acpm_clk_variant {
+ 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(cname) \
+ { \
+ .name = cname, \
+ }
+
+static const struct acpm_clk_variant gs101_acpm_clks[] = {
+ ACPM_CLK("mif"),
+ ACPM_CLK("int"),
+ ACPM_CLK("cpucl0"),
+ ACPM_CLK("cpucl1"),
+ ACPM_CLK("cpucl2"),
+ ACPM_CLK("g3d"),
+ ACPM_CLK("g3dl2"),
+ ACPM_CLK("tpu"),
+ ACPM_CLK("intcam"),
+ ACPM_CLK("tnr"),
+ ACPM_CLK("cam"),
+ ACPM_CLK("mfc"),
+ ACPM_CLK("disp"),
+ ACPM_CLK("bo"),
+};
+
+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);
+}
+
+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_register(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)
+{
+ 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;
+
+ 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 = acpm_clk_gs101.nr_clks;
+ mbox_chan_id = acpm_clk_gs101.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++) {
+ struct acpm_clk *aclk = &aclks[i];
+
+ /*
+ * The code assumes the clock IDs start from zero,
+ * are sequential and do not have gaps.
+ */
+ aclk->id = i;
+ aclk->handle = acpm_handle;
+ aclk->mbox_chan_id = mbox_chan_id;
+
+ hws[i] = &aclk->hw;
+
+ err = acpm_clk_register(dev, aclk,
+ acpm_clk_gs101.clks[i].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" },
+ {}
+};
+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.740.g6adb054d12-googOn 10/10/2025 14:46, Tudor Ambarus wrote: > 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> > Reviewed-by: Peter Griffin <peter.griffin@linaro.org> > Tested-by: Peter Griffin <peter.griffin@linaro.org> # on gs101-oriole > --- > drivers/clk/samsung/Kconfig | 10 +++ > drivers/clk/samsung/Makefile | 1 + > drivers/clk/samsung/clk-acpm.c | 185 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 196 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) I merged the patches but I don't get why we are not enabling it by default, just like every other clock driver. What is so special here? > + 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 Best regards, Krzysztof
On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >> 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) > > I merged the patches but I don't get why we are not enabling it by > default, just like every other clock driver. What is so special here? Thanks! Are you referring to the depends on line? I needed it otherwise on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: ERROR: modpost: "devm_acpm_get_by_node" [drivers/clk/samsung/clk-acpm.ko] undefined! Cheers, ta
Quoting Tudor Ambarus (2025-10-20 00:45:58) > > > On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: > >> 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) > > > > I merged the patches but I don't get why we are not enabling it by > > default, just like every other clock driver. What is so special here? > > Thanks! Are you referring to the depends on line? I needed it otherwise > on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: > > ERROR: modpost: "devm_acpm_get_by_node" [drivers/clk/samsung/clk-acpm.ko] undefined! > I don't understand that part. The depends on statement "COMPILE_TEST && !EXYNOS_ACPM_PROTOCOL" is equivalent to COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n, so are you trying to avoid EXYNOS_ACPM_PROTOCOL=y when COMPILE_TEST=y?
On 11/11/2025 02:39, Stephen Boyd wrote: > Quoting Tudor Ambarus (2025-10-20 00:45:58) >> >> >> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>>> 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) >>> >>> I merged the patches but I don't get why we are not enabling it by >>> default, just like every other clock driver. What is so special here? >> >> Thanks! Are you referring to the depends on line? I needed it otherwise >> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: >> >> ERROR: modpost: "devm_acpm_get_by_node" [drivers/clk/samsung/clk-acpm.ko] undefined! >> > > I don't understand that part. The depends on statement "COMPILE_TEST && > !EXYNOS_ACPM_PROTOCOL" is equivalent to COMPILE_TEST=y and > EXYNOS_ACPM_PROTOCOL=n, so are you trying to avoid > EXYNOS_ACPM_PROTOCOL=y when COMPILE_TEST=y? This is the standard kconfig module dependency to avoid having built-in when other is module: EXYNOS_ACPM_PROTOCOL || !EXYNOS_ACPM_PROTOCOL It is used everywhere and already documented. Now, the only difference here is that !EXYNOS_ACPM_PROTOCOL is actually not meaningful for the driver, because it is not an optional dependency, thus we allow this only for compile testing, therefore: || COMPILE_TEST && !EXYNOS_ACPM_PROTOCOL. Best regards, Krzysztof
On 11/11/25 3:39 AM, Stephen Boyd wrote: Hi, Stephen! > Quoting Tudor Ambarus (2025-10-20 00:45:58) >> >> >> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>>> 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) >>> >>> I merged the patches but I don't get why we are not enabling it by >>> default, just like every other clock driver. What is so special here? >> >> Thanks! Are you referring to the depends on line? I needed it otherwise >> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: >> >> ERROR: modpost: "devm_acpm_get_by_node" [drivers/clk/samsung/clk-acpm.ko] undefined! >> > > I don't understand that part. The depends on statement "COMPILE_TEST && > !EXYNOS_ACPM_PROTOCOL" is equivalent to COMPILE_TEST=y and > EXYNOS_ACPM_PROTOCOL=n, so are you trying to avoid > EXYNOS_ACPM_PROTOCOL=y when COMPILE_TEST=y? My previous comment was misleading. The depends on line allows CONFIG_EXYNOS_ACPM_CLK to be selected in two main scenarios: 1/ if EXYNOS_ACPM_PROTOCOL is enabled the clock driver that uses it can be enabled (the normal case). 2/ COMPILE_TEST is enabled AND EXYNOS_ACPM_PROTOCOL is NOT enabled. This is the special scenario for build testing. I want to build test the clock driver even if EXYNOS_ACPM_PROTOCOL is NOT enabled. For that I also needed the following patch: https://lore.kernel.org/linux-samsung-soc/20251021-fix-acpm-clk-build-test-v1-1-236a3d6db7f5@linaro.org/ Cheers, ta
On 11/11/25 8:24 AM, Tudor Ambarus wrote: > > > On 11/11/25 3:39 AM, Stephen Boyd wrote: > > Hi, Stephen! > >> Quoting Tudor Ambarus (2025-10-20 00:45:58) >>> >>> >>> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>>>> 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) >>>> >>>> I merged the patches but I don't get why we are not enabling it by >>>> default, just like every other clock driver. What is so special here? >>> >>> Thanks! Are you referring to the depends on line? I needed it otherwise >>> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: >>> >>> ERROR: modpost: "devm_acpm_get_by_node" [drivers/clk/samsung/clk-acpm.ko] undefined! >>> >> >> I don't understand that part. The depends on statement "COMPILE_TEST && >> !EXYNOS_ACPM_PROTOCOL" is equivalent to COMPILE_TEST=y and >> EXYNOS_ACPM_PROTOCOL=n, so are you trying to avoid >> EXYNOS_ACPM_PROTOCOL=y when COMPILE_TEST=y? > > My previous comment was misleading. > The depends on line allows CONFIG_EXYNOS_ACPM_CLK to be selected in two > main scenarios: > 1/ if EXYNOS_ACPM_PROTOCOL is enabled the clock driver that uses it can > be enabled (the normal case). > 2/ COMPILE_TEST is enabled AND EXYNOS_ACPM_PROTOCOL is NOT enabled. This > is the special scenario for build testing. I want to build test the > clock driver even if EXYNOS_ACPM_PROTOCOL is NOT enabled. For that I > also needed the following patch: > > https://lore.kernel.org/linux-samsung-soc/20251021-fix-acpm-clk-build-test-v1-1-236a3d6db7f5@linaro.org/ > What I described in 2/ EXYNOS_ACPM_PROTOCOL [=n] && EXYNOS_ACPM_CLK [=y] can be achieved with a more relaxed: depends on EXYNOS_ACPM_PROTOCOL || COMPILE_TEST because of the stub (dummy method) that I referenced in the link above. It's really what Krzysztof explained in his reply, I wanted to avoid the link failure for COMPILE_TEST [=y] when EXYNOS_ACPM_PROTOCOL [=m] && EXYNOS_ACPM_CLK [=y]. We have the following possibilities with: depends on EXYNOS_ACPM_PROTOCOL || (COMPILE_TEST && !EXYNOS_ACPM_PROTOCOL) 1/ CONMPILE_TEST=n EXYNOS_ACPM_PROTOCOL=n EXYNOS_ACPM_CLK=n EXYNOS_ACPM_PROTOCOL=m EXYNOS_ACPM_CLK=n,m EXYNOS_ACPM_PROTOCOL=y EXYNOS_ACPM_CLK=n,m,y 2/COMPILE_TEST=y EXYNOS_ACPM_PROTOCOL=n EXYNOS_ACPM_CLK=n,m,y EXYNOS_ACPM_PROTOCOL=m EXYNOS_ACPM_CLK=n,m EXYNOS_ACPM_PROTOCOL=y EXYNOS_ACPM_CLK=n,m,y We have the following possibilities with: depends on EXYNOS_ACPM_PROTOCOL || COMPILE_TEST 1/ CONMPILE_TEST=n EXYNOS_ACPM_PROTOCOL=n EXYNOS_ACPM_CLK=n EXYNOS_ACPM_PROTOCOL=m EXYNOS_ACPM_CLK=n,m EXYNOS_ACPM_PROTOCOL=y EXYNOS_ACPM_CLK=n,m,y 2/COMPILE_TEST=y EXYNOS_ACPM_PROTOCOL=n EXYNOS_ACPM_CLK=n,m,y EXYNOS_ACPM_PROTOCOL=m EXYNOS_ACPM_CLK=n,m,y <- link failure when y EXYNOS_ACPM_PROTOCOL=y EXYNOS_ACPM_CLK=n,m,y Thanks, ta
On 20/10/2025 09:45, Tudor Ambarus wrote: > > > On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>> 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) >> >> I merged the patches but I don't get why we are not enabling it by >> default, just like every other clock driver. What is so special here? > > Thanks! Are you referring to the depends on line? I needed it otherwise > on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: No. I am referring to missing default and defconfig patch. Best regards, Krzysztof
On 10/20/25 9:22 AM, Krzysztof Kozlowski wrote: > On 20/10/2025 09:45, Tudor Ambarus wrote: >> >> >> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>>> 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) >>> >>> I merged the patches but I don't get why we are not enabling it by >>> default, just like every other clock driver. What is so special here? >> >> Thanks! Are you referring to the depends on line? I needed it otherwise >> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: > > > No. I am referring to missing default and defconfig patch. > default m or y would force compilation of EXYNOS_ACPM_CLK and EXYNOS_ACPM_PROTOCOL for all ARCH_EXYNOS, even on Exynos platforms that don't use ACPM. Since ACPM is not universally required by the Exynos architecture, I thought to make it opt-in (default n). Setting it as a module in arm64 defconfig makes it available on compatible platforms. Similar clock drivers do the same: CLK_RASPBERRYPI. COMMON_CLK_SCPI, COMMON_CLK_SCMI - no defaults and set them as builtin in the arm64 defconfig. Cheers, ta
On 20/10/2025 11:19, Tudor Ambarus wrote: > > > On 10/20/25 9:22 AM, Krzysztof Kozlowski wrote: >> On 20/10/2025 09:45, Tudor Ambarus wrote: >>> >>> >>> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote: >>>>> 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) >>>> >>>> I merged the patches but I don't get why we are not enabling it by >>>> default, just like every other clock driver. What is so special here? >>> >>> Thanks! Are you referring to the depends on line? I needed it otherwise >>> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get: >> >> >> No. I am referring to missing default and defconfig patch. >> > > default m or y would force compilation of EXYNOS_ACPM_CLK and > EXYNOS_ACPM_PROTOCOL for all ARCH_EXYNOS, even on Exynos platforms that > don't use ACPM. Since ACPM is not universally required by the Exynos > architecture, I thought to make it opt-in (default n). Just like every clock driver. So again - how is it different? > > Setting it as a module in arm64 defconfig makes it available on > compatible platforms. > > Similar clock drivers do the same: CLK_RASPBERRYPI. > COMMON_CLK_SCPI, COMMON_CLK_SCMI - no defaults and set them as builtin > in the arm64 defconfig. I am speaking about Samsung drivers. Best regards, Krzysztof
On 10/20/25 10:58 AM, Krzysztof Kozlowski wrote:
> On 20/10/2025 11:19, Tudor Ambarus wrote:
>>
>>
>> On 10/20/25 9:22 AM, Krzysztof Kozlowski wrote:
>>> On 20/10/2025 09:45, Tudor Ambarus wrote:
>>>>
>>>>
>>>> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote:
>>>>>> 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)
>>>>>
>>>>> I merged the patches but I don't get why we are not enabling it by
>>>>> default, just like every other clock driver. What is so special here?
>>>>
>>>> Thanks! Are you referring to the depends on line? I needed it otherwise
>>>> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get:
>>>
>>>
>>> No. I am referring to missing default and defconfig patch.
>>>
>>
>> default m or y would force compilation of EXYNOS_ACPM_CLK and
>> EXYNOS_ACPM_PROTOCOL for all ARCH_EXYNOS, even on Exynos platforms that
>> don't use ACPM. Since ACPM is not universally required by the Exynos
>> architecture, I thought to make it opt-in (default n).
>
>
> Just like every clock driver. So again - how is it different?
The key difference lies in the universality of the hardware interface
across the ARCH_EXYNOS family. If EXYNOS_AUDSS_CLK_CON, EXYNOS_CLKOUT
are considered core, integral, or nearly universal features across
Exynos SoCs, then it's alright to have them by default. I can't tell how
common is ACPM across the Samsung Exynos SoCs. I know it's present on
gs{1,2)01 and e850. Heard it mentioned around some other phone but I
can't remember which. Maybe we shall set to a default m when more users
reveal themselves? I'm of course open to drop the defconfig patch and
follow up with a patch setting EXYNOS_ACPM_CLK to
"default m if ARCH_EXYNOS" if you think it's common enough.
Cheers,
ta
On 20/10/2025 13:02, Tudor Ambarus wrote:
>
>
> On 10/20/25 10:58 AM, Krzysztof Kozlowski wrote:
>> On 20/10/2025 11:19, Tudor Ambarus wrote:
>>>
>>>
>>> On 10/20/25 9:22 AM, Krzysztof Kozlowski wrote:
>>>> On 20/10/2025 09:45, Tudor Ambarus wrote:
>>>>>
>>>>>
>>>>> On 10/20/25 7:54 AM, Krzysztof Kozlowski wrote:
>>>>>>> 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)
>>>>>>
>>>>>> I merged the patches but I don't get why we are not enabling it by
>>>>>> default, just like every other clock driver. What is so special here?
>>>>>
>>>>> Thanks! Are you referring to the depends on line? I needed it otherwise
>>>>> on randconfigs where COMPILE_TEST=y and EXYNOS_ACPM_PROTOCOL=n I get:
>>>>
>>>>
>>>> No. I am referring to missing default and defconfig patch.
>>>>
>>>
>>> default m or y would force compilation of EXYNOS_ACPM_CLK and
>>> EXYNOS_ACPM_PROTOCOL for all ARCH_EXYNOS, even on Exynos platforms that
>>> don't use ACPM. Since ACPM is not universally required by the Exynos
>>> architecture, I thought to make it opt-in (default n).
>>
>>
>> Just like every clock driver. So again - how is it different?
>
> The key difference lies in the universality of the hardware interface
> across the ARCH_EXYNOS family. If EXYNOS_AUDSS_CLK_CON, EXYNOS_CLKOUT
> are considered core, integral, or nearly universal features across
> Exynos SoCs, then it's alright to have them by default. I can't tell how
> common is ACPM across the Samsung Exynos SoCs. I know it's present on
> gs{1,2)01 and e850. Heard it mentioned around some other phone but I
> can't remember which. Maybe we shall set to a default m when more users
> reveal themselves? I'm of course open to drop the defconfig patch and
> follow up with a patch setting EXYNOS_ACPM_CLK to
> "default m if ARCH_EXYNOS" if you think it's common enough.
OK, we can wait till one more user appears. Although the choice is less
about universality, but rather if this is a core SoC element. If this is
a core SoC element, then SoC ARCH should select it (not allow to disable
it), even if this is only for one ARCH_EXYNOS SoC.
Best regards,
Krzysztof
On Fri, 10 Oct 2025 12:46:34 +0000, Tudor Ambarus wrote:
> Add the Exynos ACPM clock driver. It provides support for clocks that
> are controlled by firmware that implements the ACPM interface.
>
>
Applied, thanks!
[4/6] clk: samsung: add Exynos ACPM clock driver
https://git.kernel.org/krzk/linux/c/40498a74205371400a5b0088acb886ac47a523aa
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
© 2016 - 2025 Red Hat, Inc.