| 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: <4e0a5db18ed7d37038e67be0f1ddcb08.sboyd@kernel.org>
Date: Thu, 20 Jul 2023 17:02:15 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Jonathan Neuschäfer <j.neuschaefer@....net>,
linux-clk@...r.kernel.org, openbmc@...ts.ozlabs.org
Cc: linux-kernel@...r.kernel.org, linux-watchdog@...r.kernel.org,
devicetree@...r.kernel.org,
Jonathan Neuschäfer <j.neuschaefer@....net>,
Michael Turquette <mturquette@...libre.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Avi Fishman <avifishman70@...il.com>,
Tomer Maimon <tmaimon77@...il.com>,
Tali Perry <tali.perry1@...il.com>,
Patrick Venture <venture@...gle.com>,
Nancy Yuen <yuenn@...gle.com>,
Benjamin Fair <benjaminfair@...gle.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Philipp Zabel <p.zabel@...gutronix.de>,
Wim Van Sebroeck <wim@...ux-watchdog.org>,
Guenter Roeck <linux@...ck-us.net>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
Joel Stanley <joel@....id.au>
Subject: Re: [PATCH v8 2/2] clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver
Quoting Jonathan Neuschäfer (2023-04-28 12:02:26)
> diff --git a/drivers/clk/clk-wpcm450.c b/drivers/clk/clk-wpcm450.c
> new file mode 100644
> index 0000000000000..358be5befa887
> --- /dev/null
> +++ b/drivers/clk/clk-wpcm450.c
> @@ -0,0 +1,374 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton WPCM450 clock and reset controller driver.
> + *
> + * Copyright (C) 2022 Jonathan Neuschäfer <j.neuschaefer@....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__);
This can happen more often than you think, so probably best to remove
it.
> + 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 },
This is new code, please don't use .name. Instead use .fw_name or .index with preference to
.index first and .hw if the pointer is available in this driver.
> + { "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);
> +
> +/*
> + * NOTE: Error handling is very rudimentary here. If the clock driver initial-
> + * ization fails, the system is probably in bigger trouble than what is caused
> + * by a few leaked resources.
> + */
> +
> +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)
> + return;
> +
> + clk_data->num = WPCM450_NUM_CLKS;
> + hws = clk_data->hws;
> +
> + for (i = 0; i < WPCM450_NUM_CLKS; i++)
> + hws[i] = ERR_PTR(-ENOENT);
> +
> + // PLLs
Please use /* comments like this */
> + 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);
Missing newline?
> + return;
> + }
> + }
> +
> + // 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);
> + return;
> + }
> + }
> +
> + // 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);
> + return;
> + }
> + 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);
> + return;
> + }
> + }
> +
> + // 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);
> + return;
> + }
> + 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: %pe\n", ERR_PTR(ret));
> +
> + // Reset controller
> + reset = kzalloc(sizeof(*reset), GFP_KERNEL);
> + if (!reset)
> + return;
> + 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: %pe\n", ERR_PTR(ret));
> +
> + of_node_put(clk_np);
> +}
> +
> +CLK_OF_DECLARE(wpcm450_clk_init, "nuvoton,wpcm450-clk", wpcm450_clk_init);
Is something preventing this from being a platform driver?
Powered by blists - more mailing lists