[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aTo8sCPpVM1o9PKX@pie>
Date: Thu, 11 Dec 2025 03:38:24 +0000
From: Yao Zi <me@...ao.cc>
To: Yixun Lan <dlan@...too.org>, Stephen Boyd <sboyd@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Haylen Chu <heylenay@....org>, Inochi Amaoto <inochiama@...il.com>,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 4/4] clk: spacemit: k3: add the clock tree
On Thu, Dec 11, 2025 at 09:19:44AM +0800, Yixun Lan wrote:
> Add clock support to SpacemiT K3 SoC, the clock tree consist of several
> blocks which are APBC, APBS, APMU, DCIU, MPUM.
>
> Signed-off-by: Yixun Lan <dlan@...too.org>
> ---
> drivers/clk/spacemit/Kconfig | 6 +
> drivers/clk/spacemit/Makefile | 11 +-
> drivers/clk/spacemit/ccu-k3.c | 1641 ++++++++++++++++++++++++++++++++++++++
> include/soc/spacemit/ccu.h | 18 +
> include/soc/spacemit/k1-syscon.h | 12 +-
> include/soc/spacemit/k3-syscon.h | 273 +++++++
> 6 files changed, 1947 insertions(+), 14 deletions(-)
...
> diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
> new file mode 100644
> index 000000000000..948889e8ca8c
> --- /dev/null
> +++ b/drivers/clk/spacemit/ccu-k3.c
> @@ -0,0 +1,1641 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2025 SpacemiT Technology Co. Ltd
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <soc/spacemit/k3-syscon.h>
> +
> +#include "ccu_common.h"
> +#include "ccu_pll.h"
> +#include "ccu_mix.h"
> +#include "ccu_ddn.h"
> +
> +#include <dt-bindings/clock/spacemit,k3-clocks.h>
> +
> +struct spacemit_ccu_data {
> + const char *reset_name;
> + struct clk_hw **hws;
> + size_t num;
> +};
...
> +static const struct spacemit_ccu_data k3_ccu_dciu_data = {
> + .reset_name = "dciu-reset",
> + .hws = k3_ccu_dciu_hws,
> + .num = ARRAY_SIZE(k3_ccu_dciu_hws),
> +};
> +
> +static int spacemit_ccu_register(struct device *dev,
> + struct regmap *regmap,
> + struct regmap *lock_regmap,
> + const struct spacemit_ccu_data *data)
> +{
> + struct clk_hw_onecell_data *clk_data;
> + int i, ret;
> +
> + /* Nothing to do if the CCU does not implement any clocks */
> + if (!data->hws)
> + return 0;
> +
> + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, data->num),
> + GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num; i++) {
> + struct clk_hw *hw = data->hws[i];
> + struct ccu_common *common;
> + const char *name;
> +
> + if (!hw) {
> + clk_data->hws[i] = ERR_PTR(-ENOENT);
> + continue;
> + }
> +
> + name = hw->init->name;
> +
> + common = hw_to_ccu_common(hw);
> + common->regmap = regmap;
> + common->lock_regmap = lock_regmap;
> +
> + ret = devm_clk_hw_register(dev, hw);
> + if (ret) {
> + dev_err(dev, "Cannot register clock %d - %s\n",
> + i, name);
> + return ret;
> + }
> +
> + clk_data->hws[i] = hw;
> + }
> +
> + clk_data->num = data->num;
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> + if (ret)
> + dev_err(dev, "failed to add clock hardware provider (%d)\n", ret);
> +
> + return ret;
> +}
> +
> +static void spacemit_cadev_release(struct device *dev)
> +{
> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +
> + kfree(to_spacemit_ccu_adev(adev));
> +}
> +
> +static void spacemit_adev_unregister(void *data)
> +{
> + struct auxiliary_device *adev = data;
> +
> + auxiliary_device_delete(adev);
> + auxiliary_device_uninit(adev);
> +}
> +
> +static int spacemit_ccu_reset_register(struct device *dev,
> + struct regmap *regmap,
> + const char *reset_name)
> +{
> + struct spacemit_ccu_adev *cadev;
> + struct auxiliary_device *adev;
> + static u32 next_id;
> + int ret;
> +
> + /* Nothing to do if the CCU does not implement a reset controller */
> + if (!reset_name)
> + return 0;
> +
> + cadev = devm_kzalloc(dev, sizeof(*cadev), GFP_KERNEL);
> + if (!cadev)
> + return -ENOMEM;
> + cadev->regmap = regmap;
> +
> + adev = &cadev->adev;
> + adev->name = reset_name;
> + adev->dev.parent = dev;
> + adev->dev.release = spacemit_cadev_release;
> + adev->dev.of_node = dev->of_node;
> + adev->id = next_id++;
> +
> + ret = auxiliary_device_init(adev);
> + if (ret)
> + return ret;
> +
> + ret = auxiliary_device_add(adev);
> + if (ret) {
> + auxiliary_device_uninit(adev);
> + return ret;
> + }
> +
> + return devm_add_action_or_reset(dev, spacemit_adev_unregister, adev);
> +}
This piece of code looks quiet similar to types/functions with the same
names in ccu-k1.c. If I'm correct, could we separate the logic into a
new file and avoid duplication?
> +static int k3_ccu_probe(struct platform_device *pdev)
> +{
> + struct regmap *base_regmap, *lock_regmap = NULL;
> + const struct spacemit_ccu_data *data;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + base_regmap = device_node_to_regmap(dev->of_node);
> + if (IS_ERR(base_regmap))
> + return dev_err_probe(dev, PTR_ERR(base_regmap),
> + "failed to get regmap\n");
> + /*
> + * The lock status of PLLs locate in MPMU region, while PLLs themselves
> + * are in APBS region. Reference to MPMU syscon is required to check PLL
> + * status.
> + */
> + if (of_device_is_compatible(dev->of_node, "spacemit,k3-pll")) {
> + struct device_node *mpmu = of_parse_phandle(dev->of_node, "spacemit,mpmu", 0);
> +
> + if (!mpmu)
> + return dev_err_probe(dev, -ENODEV,
> + "Cannot parse MPMU region\n");
> +
> + lock_regmap = device_node_to_regmap(mpmu);
> + of_node_put(mpmu);
> +
> + if (IS_ERR(lock_regmap))
> + return dev_err_probe(dev, PTR_ERR(lock_regmap),
> + "failed to get lock regmap\n");
> + }
> +
> + data = of_device_get_match_data(dev);
> +
> + ret = spacemit_ccu_register(dev, base_regmap, lock_regmap, data);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register clocks\n");
> +
> + ret = spacemit_ccu_reset_register(dev, base_regmap, data->reset_name);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register resets\n");
> +
> + return 0;
> +}
k3_ccu_probe looks quote similar to k1_ccu_probe, too. The only
difference is that k3_ccu_probe checks for spacemit,k3-pll instead of
spacemit,k1-pll.
We could share most of the probe code by writing a SoC-independent probe
function,
int spacemit_ccu_probe(struct platform_dev *pdev,
const char *pll_compatible);
and calling it in ccu-k1.c and ccu-k3.c with different pll_compatible.
Regards,
Yao Zi
Powered by blists - more mailing lists