| 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: <20211203080345.2B9DBC53FAD@smtp.kernel.org>
Date: Fri, 03 Dec 2021 00:03:43 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Felix Fietkau <nbd@....name>,
Michael Turquette <mturquette@...libre.com>,
Rob Herring <robh+dt@...nel.org>,
linux-arm-kernel@...ts.infradead.org
Cc: john@...ozen.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v5 07/13] clk: en7523: Add clock driver for Airoha EN7523 SoC
Quoting Felix Fietkau (2021-11-29 07:33:23)
> This driver only registers fixed rate clocks, since the clocks are fully
> initialized by the boot loader and should not be changed later, according
> to Airoha.
>
> Signed-off-by: Felix Fietkau <nbd@....name>
> ---
> arch/arm/boot/dts/en7523.dtsi | 8 +
> drivers/clk/Kconfig | 9 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-en7523.c | 356 ++++++++++++++++++++++++++++++++++
> 4 files changed, 374 insertions(+)
> create mode 100644 drivers/clk/clk-en7523.c
Pleas don't mix clk driver changes and dts file updates together.
Instead, introduce the clk driver in one patch and add the dts node in
another patch so that the different maintainers can pick up the patch
for the area they maintain.
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index c5b3dc97396a..b542f58c58d2 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -192,6 +192,15 @@ config COMMON_CLK_CS2000_CP
> help
> If you say yes here you get support for the CS2000 clock multiplier.
>
> +config COMMON_CLK_EN7523
> + bool "Clock driver for Airoha EN7523 SoC system clocks"
> + depends on OF
> + depends on ARCH_AIROHA || ARM || COMPILE_TEST
Is this supposed to have parenthesis somewhere? Why is depending on ARM
useful?
> + default ARCH_AIROHA
> + help
> + This driver provides the fixed clocks and gates present on Airoha
> + ARM silicon.
> +
> config COMMON_CLK_FSL_FLEXSPI
> tristate "Clock driver for FlexSPI on Layerscape SoCs"
> depends on ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index e42312121e51..be11d88c1603 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
> obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
> obj-$(CONFIG_COMMON_CLK_CS2000_CP) += clk-cs2000-cp.o
> obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o
> +obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o
> obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o
> obj-$(CONFIG_COMMON_CLK_FSL_FLEXSPI) += clk-fsl-flexspi.o
> obj-$(CONFIG_COMMON_CLK_FSL_SAI) += clk-fsl-sai.o
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> new file mode 100644
> index 000000000000..f1774a5bf537
> --- /dev/null
> +++ b/drivers/clk/clk-en7523.c
> @@ -0,0 +1,356 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/regmap.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/en7523-clk.h>
> +#include <linux/clk.h>
Is this include used?
> +
> +#define REG_PCI_CONTROL 0x88
> +#define REG_PCI_CONTROL_PERSTOUT BIT(29)
> +#define REG_PCI_CONTROL_PERSTOUT1 BIT(26)
> +#define REG_PCI_CONTROL_REFCLK_EN1 BIT(22)
> +#define REG_GSW_CLK_DIV_SEL 0x1b4
[...]
> +
> +static struct clk *en7523_register_pcie_clk(struct device *dev,
> + void __iomem *np_base)
> +{
> + static const struct clk_ops pcie_gate_ops = {
> + .is_enabled = en7523_pci_is_enabled,
> + .enable = en7523_pci_enable,
> + .disable = en7523_pci_disable,
> + };
> + struct clk_init_data init = {
> + .name = "pcie",
> + .ops = &pcie_gate_ops,
> + };
> + struct en_clk_gate *cg;
> + struct clk *clk;
> +
> + cg = devm_kzalloc(dev, sizeof(*cg), GFP_KERNEL);
> + if (!cg)
> + return NULL;
> +
> + cg->base = np_base;
> + cg->hw.init = &init;
> + en7523_pci_disable(&cg->hw);
> +
> + clk = clk_register(NULL, &cg->hw);
Please use clk_hw_register
> + if (IS_ERR(clk))
> + clk = NULL;
> +
> + return clk;
> +}
> +
> +static void en7523_register_clocks(struct device *dev, struct clk_onecell_data *clk_data,
> + void __iomem *base, void __iomem *np_base)
> +{
> + struct clk *clk;
> + u32 rate;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(en7523_base_clks); i++) {
> + const struct en_clk_desc *desc = &en7523_base_clks[i];
> +
> + rate = en7523_get_base_rate(base, i);
> + rate /= en7523_get_div(base, i);
> +
> + clk = clk_register_fixed_rate(NULL, desc->name, NULL, 0, rate);
> + if (IS_ERR(clk)) {
> + pr_err("Failed to register clk %s: %ld\n",
> + desc->name, PTR_ERR(clk));
> + continue;
> + }
> +
> + clk_data->clks[desc->id] = clk;
> + }
> +
> + clk = en7523_register_pcie_clk(dev, np_base);
> + clk_data->clks[EN7523_CLK_PCIE] = clk;
> +
> + clk_data->clk_num = EN7523_NUM_CLOCKS;
> +}
> +
> +static int en7523_clk_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct clk_onecell_data *clk_data;
> + void __iomem *base, *np_base;
> + int r;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + np_base = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(base))
> + return PTR_ERR(np_base);
> +
> + clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + clk_data->clks = devm_kcalloc(&pdev->dev, EN7523_NUM_CLOCKS,
> + sizeof(*clk_data->clks), GFP_KERNEL);
> + if (!clk_data->clks)
> + return -ENOMEM;
> +
> + en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
Please add a clk_hw provider instead of a clk provider.
> + if (r)
Powered by blists - more mailing lists