| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <175848703636.4354.2936744718103927060@lazor>
Date: Sun, 21 Sep 2025 13:37:16 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Alim Akhtar <alim.akhtar@...sung.com>, André Draszik <andre.draszik@...aro.org>, Catalin Marinas <catalin.marinas@....com>, Chanwoo Choi <cw00.choi@...sung.com>, Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>, Michael Turquette <mturquette@...libre.com>, Peter Griffin <peter.griffin@...aro.org>, Rob Herring <robh@...nel.org>, Sylwester Nawrocki <s.nawrocki@...sung.com>, Tudor Ambarus <tudor.ambarus@...aro.org>, Will Deacon <will@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org, willmcvicker@...gle.com, kernel-team@...roid.com, Tudor Ambarus <tudor.ambarus@...aro.org>
Subject: Re: [PATCH v4 4/5] clk: samsung: add Exynos ACPM clock driver
Quoting Tudor Ambarus (2025-09-08 06:12:45)
> 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)
Why is COMPILE_TEST limited to !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/clk-acpm.c b/drivers/clk/samsung/clk-acpm.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..8566495265ee3e06dbf370f9e424d5540f5c7457
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-acpm.c
> @@ -0,0 +1,184 @@
> +// 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/device.h>
> +#include <linux/err.h>
> +#include <linux/firmware/samsung/exynos-acpm-protocol.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
Is this include used?
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
Are you avoiding kernel.h? If so, please include container_of.h and
device/devres.h to avoid implicit includes.
> +
> +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("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,
Maybe acpm_clk_register() is a more appropriate name.
> + 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");
Remove the period please. Most error messages don't have proper
punctuation.
> +
> + 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_ops_init(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" },
> + {},
Please drop comma here so that nothing can come after.
Powered by blists - more mailing lists