Re: [PATCH 3/8] clk: tenstorrent: Add Atlantis clock controller driver

[Brian Masney <bmasney@redhat.com>]

Hi Anirudh,

Thanks for the patch!

On Thu, Jan 15, 2026 at 05:42:02PM -0600, Anirudh Srinivasan wrote:
> Add driver for syscon block in Tenstorrent Atlantis SoC. This version of
> the driver coves clocks from RCPU syscon.
> 
> 5 types of clocks generated by this controller: PLLs (PLLs
> with bypass functionality and an additional Gate clk at output), Shared
> Gates (Multiple Gate clks that share an enable bit), standard Muxes,
> Dividers and Gates. All clocks are derived from a 24 Mhz oscillator.
> 
> Signed-off-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
> ---
> diff --git a/drivers/clk/tenstorrent/atlantis-ccu.c b/drivers/clk/tenstorrent/atlantis-ccu.c
> new file mode 100644
> index 000000000000..f3a2ea49a82e
> --- /dev/null
> +++ b/drivers/clk/tenstorrent/atlantis-ccu.c
> @@ -0,0 +1,932 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2026 Tenstorrent
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/idr.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <soc/tenstorrent/atlantis-syscon.h>
> +
> +#include <dt-bindings/clock/tenstorrent,atlantis-syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/clk-provider.h>
> +#include <linux/math.h>
> +#include <linux/bitfield.h>

Please sort the headers. clk-provider.h is listed twice. Remove the
unnecessary newlines.

> +static void atlantis_ccu_lock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_lock(lock);
> +}
> +
> +static void atlantis_ccu_unlock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_unlock(lock);
> +}

Are these abstractions really needed? Why not just call spin_lock/unlock
directly?

> +static int atlantis_ccu_clocks_register(struct device *dev,
> +					struct atlantis_ccu *ccu,
> +					const struct atlantis_ccu_data *data)
> +{
> +	struct regmap *regmap = ccu->regmap;
> +	struct clk_hw_onecell_data *clk_data;
> +	int i, ret;
> +	size_t num_clks = data->num;
> +
> +	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, data->num),
> +				GFP_KERNEL);
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	ccu->clk_data = clk_data;
> +
> +	for (i = 0; i < data->num; i++) {
> +		struct clk_hw *hw = data->hws[i];
> +		const char *name = hw->init->name;
> +		struct atlantis_clk_common *common =
> +			hw_to_atlantis_clk_common(hw);
> +		common->regmap = regmap;
> +
> +		/* Fixup missing handle to parent for gates/muxes/dividers */
> +		if (hw->init->parent_hws && hw->init->num_parents == 1) {
> +			const struct atlantis_clk_common *parent =
> +				hw_to_atlantis_clk_common(
> +					hw->init->parent_hws[0]);
> +			hw->init->parent_hws[0] = clk_data->hws[parent->clkid];
> +		}
> +
> +		switch (common->clk_type) {
> +		case ATLANTIS_CLK_MUX:
> +			struct atlantis_clk_mux *mux =
> +				hw_to_atlantis_clk_mux(hw);
> +
> +			hw = devm_clk_hw_register_mux_parent_data_table(
> +				ccu->dev, name, hw->init->parent_data,
> +				hw->init->num_parents, hw->init->flags,
> +				ccu->base + mux->config.reg_offset,
> +				mux->config.shift, mux->config.width, 0, NULL,
> +				&lock);
> +
> +			if (IS_ERR(hw)) {
> +				dev_err(dev, "Cannot register clock %d - %s\n",
> +					i, name);
> +				return ret;

return PTR_ERR(hw);

> +			}
> +
> +			if (data == &atlantis_ccu_rcpu_data) {
> +				switch (common->clkid) {
> +				case CLK_RCPU_ROOT:
> +					ret = clk_hw_set_parent(
> +						hw,
> +						clk_data->hws[CLK_RCPU_PLL]);

Should the parent be defined in device tree instead of statically in the
driver? devm_of_clk_add_hw_provider() is called below, and it calls
of_clk_set_defaults(), which will allow the use of the
assigned-clock-parents and assigned-clocks properties.

> +					if (ret)
> +						dev_err(ccu->dev,
> +							"Failed to set RCPU ROOT MUX parent: %d\n",
> +							ret);
> +					break;
> +				case CLK_NOCC_CLK:
> +					ret = clk_hw_set_parent(
> +						hw, clk_data->hws[CLK_NOC_PLL]);
> +					if (ret)
> +						dev_err(ccu->dev,
> +							"Failed to set NOCC Mux parent: %d\n",
> +							ret);
> +					break;
> +				}
> +			}
> +			break;
> +		case ATLANTIS_CLK_DIVIDER:
> +			struct atlantis_clk_divider *div =
> +				hw_to_atlantis_clk_divider(hw);
> +
> +			hw = devm_clk_hw_register_divider_parent_hw(
> +				ccu->dev, name, common->hw.init->parent_hws[0],
> +				div->common.hw.init->flags,
> +				ccu->base + div->config.reg_offset,
> +				div->config.shift, div->config.width,
> +				div->config.flags, &lock);
> +
> +			if (IS_ERR(hw)) {
> +				dev_err(dev, "Cannot register clock %d - %s\n",
> +					i, name);
> +				return ret;
> +			}
> +
> +			break;
> +		case ATLANTIS_CLK_GATE:
> +			struct atlantis_clk_gate *gate =
> +				hw_to_atlantis_clk_gate(hw);
> +
> +			hw = devm_clk_hw_register_gate_parent_hw(
> +				ccu->dev, name, common->hw.init->parent_hws[0],
> +				hw->init->flags,
> +				ccu->base + gate->config.reg_offset,
> +				ffs(gate->config.enable) - 1, 0, &lock);
> +
> +			if (IS_ERR(hw)) {
> +				dev_err(dev, "Cannot register clock %d - %s\n",
> +					i, name);
> +				return ret;

return PTR_ERR(hw);

> +			}
> +
> +			break;
> +		case ATLANTIS_CLK_FIXED_FACTOR:
> +			struct atlantis_clk_fixed_factor *factor =
> +				hw_to_atlantis_clk_fixed_factor(hw);
> +
> +			if (hw->init->parent_data) {
> +				hw = devm_clk_hw_register_fixed_factor_index(
> +					dev, name,
> +					hw->init->parent_data[0].index,
> +					hw->init->flags, factor->config.mult,
> +					factor->config.div);
> +			} else {
> +				hw = devm_clk_hw_register_fixed_factor_parent_hw(
> +					dev, name, hw->init->parent_hws[0],
> +					hw->init->flags, factor->config.mult,
> +					factor->config.div);
> +			}
> +			if (IS_ERR(hw)) {
> +				dev_err(dev, "Cannot register clock %d - %s\n",
> +					i, name);
> +				return ret;

return PTR_ERR(hw);

> +			}
> +			break;
> +		case ATLANTIS_CLK_GATE_SHARED:
> +			struct atlantis_clk_gate_shared *gate_shared =
> +				hw_to_atlantis_clk_gate_shared(hw);
> +			gate_shared->config.refcount_lock = &refcount_lock;
> +
> +			ret = devm_clk_hw_register(dev, hw);
> +

Unnecessary newline

> +			if (ret) {
> +				dev_err(dev, "Cannot register clock %d - %s\n",
> +					i, name);
> +				return ret;
> +			}
> +
> +			break;
> +		default:
> +
> +			ret = devm_clk_hw_register(dev, hw);
> +

Unnecessary newline

Brian