Re: [PATCH v5 07/13] clk: en7523: Add clock driver for Airoha EN7523 SoC
From: Stephen Boyd <sboyd@kernel.org>
Date: 2021-12-03 08:03:49
Also in:
linux-clk, linux-devicetree, lkml
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@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.
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
+ 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_TESTdiff --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.odiff --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)
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel