| 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: <aWmc73irBAM8DZwF@redhat.com>
Date: Thu, 15 Jan 2026 21:05:35 -0500
From: Brian Masney <bmasney@...hat.com>
To: Anirudh Srinivasan <asrinivasan@....tenstorrent.com>
Cc: Drew Fustini <dfustini@....tenstorrent.com>,
Joel Stanley <jms@....tenstorrent.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
joel@....id.au, fustini@...nel.org, mpe@...nel.org,
mpe@....tenstorrent.com, npiggin@....tenstorrent.com,
agross@...nel.org, agross@....tenstorrent.com
Subject: Re: [PATCH 3/8] clk: tenstorrent: Add Atlantis clock controller
driver
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@....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
Powered by blists - more mailing lists