diff mbox series

[v6,2/2] clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver

Message ID 20230415111355.696738-3-j.neuschaefer@gmx.net
State Superseded
Headers show
Series Nuvoton WPCM450 clock and reset driver | expand

Commit Message

J. Neuschäfer April 15, 2023, 11:13 a.m. UTC 
This driver implements the following features w.r.t. the clock and reset
controller in the WPCM450 SoC:

- It calculates the rates for all clocks managed by the clock controller
- It leaves the clock tree mostly unchanged, except that it enables/
  disables clock gates based on usage.
- It exposes the reset lines managed by the controller using the
  Generic Reset Controller subsystem

NOTE: If the driver and the corresponding devicetree node are present,
      the driver will disable "unused" clocks. This is problem until
      the clock relations are properly declared in the devicetree (in a
      later patch). Until then, the clk_ignore_unused kernel parameter
      can be used as a workaround.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---

v6:
- Enable RESET_SIMPLE based on ARCH_WPCM450, not ARCH_NPCM, as suggested by Tomer Maimon

v5:
- https://lore.kernel.org/lkml/20221104161850.2889894-6-j.neuschaefer@gmx.net/
- Switch to using clk_parent_data

v4:
- https://lore.kernel.org/lkml/20220610072141.347795-6-j.neuschaefer@gmx.net/
- Fix reset controller initialization

v3:
- https://lore.kernel.org/lkml/20220508194333.2170161-7-j.neuschaefer@gmx.net/
- Change reference clock name from "refclk" to "ref"
- Remove unused variable in return path of wpcm450_clk_register_pll
- Remove unused divisor tables

v2:
- https://lore.kernel.org/lkml/20220429172030.398011-7-j.neuschaefer@gmx.net/
- no changes
---
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-wpcm450.c | 375 ++++++++++++++++++++++++++++++++++++++
 drivers/reset/Kconfig     |   2 +-
 3 files changed, 377 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/clk-wpcm450.c

--
2.39.2

Comments

J. Neuschäfer April 19, 2023, 9:52 p.m. UTC | #1 
Hello Christophe,

On Sat, Apr 15, 2023 at 02:16:09PM +0200, Christophe JAILLET wrote:
> Le 15/04/2023 à 13:13, Jonathan Neuschäfer a écrit :
> > This driver implements the following features w.r.t. the clock and reset
> > controller in the WPCM450 SoC:
> > 
> > - It calculates the rates for all clocks managed by the clock controller
> > - It leaves the clock tree mostly unchanged, except that it enables/
> >    disables clock gates based on usage.
> > - It exposes the reset lines managed by the controller using the
> >    Generic Reset Controller subsystem
> > 
> > NOTE: If the driver and the corresponding devicetree node are present,
> >        the driver will disable "unused" clocks. This is problem until
> >        the clock relations are properly declared in the devicetree (in a
> >        later patch). Until then, the clk_ignore_unused kernel parameter
> >        can be used as a workaround.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
> > ---
[...]
> > +	// Enables/gates
> > +	for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
> > +		const struct wpcm450_clken_data *data = &clken_data[i];
> > +
> > +		hw = clk_hw_register_gate_parent_data(NULL, data->name, &data->parent, data->flags,
> > +						      clk_base + REG_CLKEN, data->bitnum,
> > +						      data->flags, &wpcm450_clk_lock);
> 
> If an error occures in the 'for' loop or after it, should this be
> clk_hw_unregister_gate()'ed somewhere?

Ideally yes —

in this case, if the clock driver fails, the system is arguably in such
a bad state that there isn't much point in bothering.


> 
> CJ
> 
> > +		if (IS_ERR(hw)) {
> > +			pr_err("Failed to register gate: %pe\n", hw);
> > +			goto err_free;
> > +		}
> > +		clk_data->hws[data->bitnum] = hw;
> > +	}



Best regards,
Jonathan
J. Neuschäfer April 22, 2023, 3:56 p.m. UTC | #2 
On Thu, Apr 20, 2023 at 07:32:07AM +0200, Christophe JAILLET wrote:
> Le 19/04/2023 à 23:52, Jonathan Neuschäfer a écrit :
> > On Sat, Apr 15, 2023 at 02:16:09PM +0200, Christophe JAILLET wrote:
> > > Le 15/04/2023 à 13:13, Jonathan Neuschäfer a écrit :
[...]
> > > > +	// Enables/gates
> > > > +	for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
> > > > +		const struct wpcm450_clken_data *data = &clken_data[i];
> > > > +
> > > > +		hw = clk_hw_register_gate_parent_data(NULL, data->name, &data->parent, data->flags,
> > > > +						      clk_base + REG_CLKEN, data->bitnum,
> > > > +						      data->flags, &wpcm450_clk_lock);
> > > 
> > > If an error occures in the 'for' loop or after it, should this be
> > > clk_hw_unregister_gate()'ed somewhere?
> > 
> > Ideally yes —
> > 
> > in this case, if the clock driver fails, the system is arguably in such
> > a bad state that there isn't much point in bothering.
> > 
> 
> Ok, but below we care about freeing clk_data->hws in the error handling
> path.
> 
> Why do we handle just half of the resources?
> Shouldn't it be all (to be clean, if it makes sense) or nothing (to reduce
> the LoC and have a smaller driver)?

I thought about it for a bit, and I think I'm ok with reducing the
deallocation in this driver to nothing. I'll spin a new version.

Conversely, if I were to implement proper error handling here, I'd
convert it into a platform driver and use devm_* functions, because
dealing with all the little clock objects is just too painful and
fragile for my taste.


Thanks,
Jonathan
diff mbox series

Patch

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e3ca0d058a256..b58352d4d615d 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -75,6 +75,7 @@  obj-$(CONFIG_COMMON_CLK_RS9_PCIE)	+= clk-renesas-pcie.o
 obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
 obj-$(CONFIG_COMMON_CLK_VC7)		+= clk-versaclock7.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
+obj-$(CONFIG_ARCH_WPCM450)		+= clk-wpcm450.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o

 # please keep this section sorted lexicographically by directory path name
diff --git a/drivers/clk/clk-wpcm450.c b/drivers/clk/clk-wpcm450.c
new file mode 100644
index 0000000000000..b5e81b3b6b982
--- /dev/null
+++ b/drivers/clk/clk-wpcm450.c
@@ -0,0 +1,375 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton WPCM450 clock and reset controller driver.
+ *
+ * Copyright (C) 2022 Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/reset-controller.h>
+#include <linux/reset/reset-simple.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
+
+struct wpcm450_clk_pll {
+	struct clk_hw hw;
+	void __iomem *pllcon;
+	u8 flags;
+};
+
+#define to_wpcm450_clk_pll(_hw) container_of(_hw, struct wpcm450_clk_pll, hw)
+
+#define PLLCON_FBDV	GENMASK(24, 16)
+#define PLLCON_PRST	BIT(13)
+#define PLLCON_PWDEN	BIT(12)
+#define PLLCON_OTDV	GENMASK(10, 8)
+#define PLLCON_INDV	GENMASK(5, 0)
+
+static unsigned long wpcm450_clk_pll_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	unsigned long fbdv, indv, otdv;
+	u64 rate;
+	u32 pllcon;
+
+	if (parent_rate == 0) {
+		pr_err("%s: parent rate is zero", __func__);
+		return 0;
+	}
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	indv = FIELD_GET(PLLCON_INDV, pllcon) + 1;
+	fbdv = FIELD_GET(PLLCON_FBDV, pllcon) + 1;
+	otdv = FIELD_GET(PLLCON_OTDV, pllcon) + 1;
+
+	rate = (u64)parent_rate * fbdv;
+	do_div(rate, indv * otdv);
+
+	return rate;
+}
+
+static int wpcm450_clk_pll_is_enabled(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	return !(pllcon & PLLCON_PRST);
+}
+
+static void wpcm450_clk_pll_disable(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+	pllcon |= PLLCON_PRST | PLLCON_PWDEN;
+	writel(pllcon, pll->pllcon);
+}
+
+static const struct clk_ops wpcm450_clk_pll_ops = {
+	.recalc_rate = wpcm450_clk_pll_recalc_rate,
+	.is_enabled = wpcm450_clk_pll_is_enabled,
+	.disable = wpcm450_clk_pll_disable
+};
+
+static struct clk_hw *
+wpcm450_clk_register_pll(void __iomem *pllcon, const char *name,
+			 const struct clk_parent_data *parent, unsigned long flags)
+{
+	struct wpcm450_clk_pll *pll;
+	struct clk_init_data init = {};
+	int ret;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &wpcm450_clk_pll_ops;
+	init.parent_data = parent;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll->pllcon = pllcon;
+	pll->hw.init = &init;
+
+	ret = clk_hw_register(NULL, &pll->hw);
+	if (ret) {
+		kfree(pll);
+		return ERR_PTR(ret);
+	}
+
+	return &pll->hw;
+}
+
+#define REG_CLKEN	0x00
+#define REG_CLKSEL	0x04
+#define REG_CLKDIV	0x08
+#define REG_PLLCON0	0x0c
+#define REG_PLLCON1	0x10
+#define REG_PMCON	0x14
+#define REG_IRQWAKECON	0x18
+#define REG_IRQWAKEFLAG	0x1c
+#define REG_IPSRST	0x20
+
+struct wpcm450_pll_data {
+	const char *name;
+	struct clk_parent_data parent;
+	unsigned int reg;
+	unsigned long flags;
+};
+
+static const struct wpcm450_pll_data pll_data[] = {
+	{ "pll0", { .name = "ref" }, REG_PLLCON0, 0 },
+	{ "pll1", { .name = "ref" }, REG_PLLCON1, 0 },
+};
+
+struct wpcm450_clksel_data {
+	const char *name;
+	const struct clk_parent_data *parents;
+	unsigned int num_parents;
+	const u32 *table;
+	int shift;
+	int width;
+	int index;
+	unsigned long flags;
+};
+
+static const u32 parent_table[] = { 0, 1, 2 };
+
+static const struct clk_parent_data default_parents[] = {
+	{ .name = "pll0" },
+	{ .name = "pll1" },
+	{ .name = "ref" },
+};
+
+static const struct clk_parent_data huart_parents[] = {
+	{ .name = "ref" },
+	{ .name = "refdiv2" },
+};
+
+static const struct wpcm450_clksel_data clksel_data[] = {
+	{ "cpusel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 0, 2, -1, CLK_IS_CRITICAL },
+	{ "clkout", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 2, 2, -1, 0 },
+	{ "usbphy", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 6, 2, -1, 0 },
+	{ "uartsel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 8, 2, WPCM450_CLK_USBPHY, 0 },
+	{ "huartsel", huart_parents, ARRAY_SIZE(huart_parents),
+		parent_table, 10, 1, -1, 0 },
+};
+
+static const struct clk_div_table div_fixed2[] = {
+	{ .val = 0, .div = 2 },
+	{ }
+};
+
+struct wpcm450_clkdiv_data {
+	const char *name;
+	struct clk_parent_data parent;
+	int div_flags;
+	const struct clk_div_table *table;
+	int shift;
+	int width;
+	unsigned long flags;
+};
+
+static struct wpcm450_clkdiv_data clkdiv_data_early[] = {
+	{ "refdiv2", { .name = "ref" }, 0, div_fixed2, 0, 0 },
+};
+
+static const struct wpcm450_clkdiv_data clkdiv_data[] = {
+	{ "cpu", { .name = "cpusel" }, 0, div_fixed2, 0, 0, CLK_IS_CRITICAL },
+	{ "adcdiv", { .name = "ref" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 28, 2, 0 },
+	{ "apb", { .name = "ahb" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 26, 2, 0 },
+	{ "ahb", { .name = "cpu" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 24, 2, 0 },
+	{ "uart", { .name = "uartsel" }, 0, NULL, 16, 4, 0 },
+	{ "ahb3", { .name = "ahb" }, CLK_DIVIDER_POWER_OF_TWO, NULL, 8, 2, 0 },
+};
+
+struct wpcm450_clken_data {
+	const char *name;
+	struct clk_parent_data parent;
+	int bitnum;
+	unsigned long flags;
+};
+
+static const struct wpcm450_clken_data clken_data[] = {
+	{ "fiu", { .name = "ahb3" }, WPCM450_CLK_FIU, 0 },
+	{ "xbus", { .name = "ahb3" }, WPCM450_CLK_XBUS, 0 },
+	{ "kcs", { .name = "apb" }, WPCM450_CLK_KCS, 0 },
+	{ "shm", { .name = "ahb3" }, WPCM450_CLK_SHM, 0 },
+	{ "usb1", { .name = "ahb" }, WPCM450_CLK_USB1, 0 },
+	{ "emc0", { .name = "ahb" }, WPCM450_CLK_EMC0, 0 },
+	{ "emc1", { .name = "ahb" }, WPCM450_CLK_EMC1, 0 },
+	{ "usb0", { .name = "ahb" }, WPCM450_CLK_USB0, 0 },
+	{ "peci", { .name = "apb" }, WPCM450_CLK_PECI, 0 },
+	{ "aes", { .name = "apb" }, WPCM450_CLK_AES, 0 },
+	{ "uart0", { .name = "uart" }, WPCM450_CLK_UART0, 0 },
+	{ "uart1", { .name = "uart" }, WPCM450_CLK_UART1, 0 },
+	{ "smb2", { .name = "apb" }, WPCM450_CLK_SMB2, 0 },
+	{ "smb3", { .name = "apb" }, WPCM450_CLK_SMB3, 0 },
+	{ "smb4", { .name = "apb" }, WPCM450_CLK_SMB4, 0 },
+	{ "smb5", { .name = "apb" }, WPCM450_CLK_SMB5, 0 },
+	{ "huart", { .name = "huartsel" }, WPCM450_CLK_HUART, 0 },
+	{ "pwm", { .name = "apb" }, WPCM450_CLK_PWM, 0 },
+	{ "timer0", { .name = "refdiv2" }, WPCM450_CLK_TIMER0, 0 },
+	{ "timer1", { .name = "refdiv2" }, WPCM450_CLK_TIMER1, 0 },
+	{ "timer2", { .name = "refdiv2" }, WPCM450_CLK_TIMER2, 0 },
+	{ "timer3", { .name = "refdiv2" }, WPCM450_CLK_TIMER3, 0 },
+	{ "timer4", { .name = "refdiv2" }, WPCM450_CLK_TIMER4, 0 },
+	{ "mft0", { .name = "apb" }, WPCM450_CLK_MFT0, 0 },
+	{ "mft1", { .name = "apb" }, WPCM450_CLK_MFT1, 0 },
+	{ "wdt", { .name = "refdiv2" }, WPCM450_CLK_WDT, 0 },
+	{ "adc", { .name = "adcdiv" }, WPCM450_CLK_ADC, 0 },
+	{ "sdio", { .name = "ahb" }, WPCM450_CLK_SDIO, 0 },
+	{ "sspi", { .name = "apb" }, WPCM450_CLK_SSPI, 0 },
+	{ "smb0", { .name = "apb" }, WPCM450_CLK_SMB0, 0 },
+	{ "smb1", { .name = "apb" }, WPCM450_CLK_SMB1, 0 },
+};
+
+static DEFINE_SPINLOCK(wpcm450_clk_lock);
+
+static void __init wpcm450_clk_init(struct device_node *clk_np)
+{
+	struct clk_hw_onecell_data *clk_data;
+	static struct clk_hw **hws;
+	static struct clk_hw *hw;
+	void __iomem *clk_base;
+	int i, ret;
+	struct reset_simple_data *reset;
+
+	clk_base = of_iomap(clk_np, 0);
+	if (!clk_base) {
+		pr_err("%pOFP: failed to map registers\n", clk_np);
+		of_node_put(clk_np);
+		return;
+	}
+	of_node_put(clk_np);
+
+	clk_data = kzalloc(struct_size(clk_data, hws, WPCM450_NUM_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clk_data->num = WPCM450_NUM_CLKS;
+	hws = clk_data->hws;
+
+	for (i = 0; i < WPCM450_NUM_CLKS; i++)
+		hws[i] = ERR_PTR(-ENOENT);
+
+	// PLLs
+	for (i = 0; i < ARRAY_SIZE(pll_data); i++) {
+		const struct wpcm450_pll_data *data = &pll_data[i];
+
+		hw = wpcm450_clk_register_pll(clk_base + data->reg, data->name,
+					      &data->parent, data->flags);
+		if (IS_ERR(hw)) {
+			pr_info("Failed to register PLL: %pe", hw);
+			goto err_free;
+		}
+	}
+
+	// Early divisors (REF/2)
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data_early); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data_early[i];
+
+		hw = clk_hw_register_divider_table_parent_data(NULL, data->name, &data->parent,
+							       data->flags, clk_base + REG_CLKDIV,
+							       data->shift, data->width,
+							       data->div_flags, data->table,
+							       &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register div table: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Selects/muxes
+	for (i = 0; i < ARRAY_SIZE(clksel_data); i++) {
+		const struct wpcm450_clksel_data *data = &clksel_data[i];
+
+		hw = clk_hw_register_mux_parent_data(NULL, data->name, data->parents,
+						     data->num_parents, data->flags,
+						     clk_base + REG_CLKSEL, data->shift,
+						     data->width, 0,
+						     &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register mux: %pe\n", hw);
+			goto err_free;
+		}
+		if (data->index >= 0)
+			clk_data->hws[data->index] = hw;
+	}
+
+	// Divisors
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data[i];
+
+		hw = clk_hw_register_divider_table_parent_data(NULL, data->name, &data->parent,
+							       data->flags, clk_base + REG_CLKDIV,
+							       data->shift, data->width,
+							       data->div_flags, data->table,
+							       &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register divider: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Enables/gates
+	for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
+		const struct wpcm450_clken_data *data = &clken_data[i];
+
+		hw = clk_hw_register_gate_parent_data(NULL, data->name, &data->parent, data->flags,
+						      clk_base + REG_CLKEN, data->bitnum,
+						      data->flags, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register gate: %pe\n", hw);
+			goto err_free;
+		}
+		clk_data->hws[data->bitnum] = hw;
+	}
+
+	ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		pr_err("Failed to add DT provider: %d\n", ret);
+
+	// Reset controller
+	reset = kzalloc(sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		goto err_free;
+	reset->rcdev.owner = THIS_MODULE;
+	reset->rcdev.nr_resets = WPCM450_NUM_RESETS;
+	reset->rcdev.ops = &reset_simple_ops;
+	reset->rcdev.of_node = clk_np;
+	reset->membase = clk_base + REG_IPSRST;
+	ret = reset_controller_register(&reset->rcdev);
+	if (ret)
+		pr_err("Failed to register reset controller: %d\n", ret);
+
+	of_node_put(clk_np);
+	return;
+
+err_free:
+	kfree(clk_data->hws);
+err_unmap:
+	iounmap(clk_base);
+	of_node_put(clk_np);
+}
+
+CLK_OF_DECLARE(wpcm450_clk_init, "nuvoton,wpcm450-clk", wpcm450_clk_init);
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 2a52c990d4fec..16e111d213560 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -208,7 +208,7 @@  config RESET_SCMI

 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST || EXPERT
-	default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC
+	default ARCH_ASPEED || ARCH_BCMBCA || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC || ARCH_WPCM450
 	depends on HAS_IOMEM
 	help
 	  This enables a simple reset controller driver for reset lines that