Re: [PATCH v5 5/6] clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver

From: Tomer Maimon
Date: Mon Dec 12 2022 - 02:31:04 EST


Hi Jonathan,

Thanks for your patch, and sorry for the late reply.

On Fri, 4 Nov 2022 at 18:19, Jonathan Neuschäfer <j.neuschaefer@xxxxxxx> wrote:
>
> 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@xxxxxxx>
> ---
>
> v5:
> - Switch to using clk_parent_data
>
> v4:
> - https://lore.kernel.org/lkml/20220610072141.347795-6-j.neuschaefer@xxxxxxx/
> - Fix reset controller initialization
>
> v3:
> - https://lore.kernel.org/lkml/20220508194333.2170161-7-j.neuschaefer@xxxxxxx/
> - 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@xxxxxxx/
> - 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
>
> 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@xxxxxxx>
> + */
> +
> +#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 de176c2fbad96..de5662830fce8 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_NPCM
Please modify the default ARCH_NPCM to the specific Hermon use,
ARCH_WPCM450, since NPCM7XX and NPCM8XX use a specific NPCM reset
driver.
https://elixir.bootlin.com/linux/v6.1-rc8/source/drivers/reset/reset-npcm.c
we prefer not to have two reset drivers when compiling ARCH_NPCM.
> depends on HAS_IOMEM
> help
> This enables a simple reset controller driver for reset lines that
> --
> 2.35.1
>

Best regards,

Tomer