Re-implement the clock driver for Loongson-1 to
add devicetree support and fit into the clock framework.
Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
V2 -> V3: Add MODULE_AUTHOR and MODULE_DESCRIPTION info
V1 -> V2: Implement one clock controller instead of single clocks
(suggested by Krzysztof Kozlowski)
---
drivers/clk/Makefile | 1 +
drivers/clk/clk-loongson1.c | 301 ++++++++++++++++++++++++++++++++++++
2 files changed, 302 insertions(+)
create mode 100644 drivers/clk/clk-loongson1.c
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b7b2c6d64636..417bc27ab6e8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_COMMON_CLK_K210) += clk-k210.o
obj-$(CONFIG_LMK04832) += clk-lmk04832.o
obj-$(CONFIG_COMMON_CLK_LAN966X) += clk-lan966x.o
obj-$(CONFIG_COMMON_CLK_LOCHNAGAR) += clk-lochnagar.o
+obj-$(CONFIG_MACH_LOONGSON32) += clk-loongson1.o
obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
obj-$(CONFIG_COMMON_CLK_MAX9485) += clk-max9485.o
obj-$(CONFIG_ARCH_MILBEAUT_M10V) += clk-milbeaut.o
diff --git a/drivers/clk/clk-loongson1.c b/drivers/clk/clk-loongson1.c
new file mode 100644
index 000000000000..4fda55c67d8d
--- /dev/null
+++ b/drivers/clk/clk-loongson1.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Clock driver for Loongson-1 SoC
+ *
+ * Copyright (C) 2012-2023 Keguang Zhang <keguang.zhang@gmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/loongson,ls1x-clk.h>
+
+/* Loongson 1 Clock Register Definitions */
+#define CLK_PLL_FREQ 0x0
+#define CLK_PLL_DIV 0x4
+
+static DEFINE_SPINLOCK(ls1x_clk_div_lock);
+
+struct ls1x_clk_pll_data {
+ u32 fixed;
+ u8 shift;
+ u8 int_shift;
+ u8 int_width;
+ u8 frac_shift;
+ u8 frac_width;
+};
+
+struct ls1x_clk_div_data {
+ u8 shift;
+ u8 width;
+ unsigned long flags;
+ const struct clk_div_table *table;
+ u8 bypass_shift;
+ u8 bypass_inv;
+ spinlock_t *lock; /* protect access to DIV registers */
+};
+
+struct ls1x_clk {
+ void __iomem *reg;
+ unsigned int offset;
+ struct clk_hw hw;
+ void *data;
+};
+
+#define to_ls1x_clk(_hw) container_of(_hw, struct ls1x_clk, hw)
+
+static inline unsigned long ls1x_pll_rate_part(unsigned int val,
+ unsigned int shift,
+ unsigned int width)
+{
+ return (val & GENMASK(shift + width, shift)) >> shift;
+}
+
+static unsigned long ls1x_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
+ const struct ls1x_clk_pll_data *d = ls1x_clk->data;
+ u32 val, rate;
+
+ val = readl(ls1x_clk->reg);
+ rate = d->fixed;
+ rate += ls1x_pll_rate_part(val, d->int_shift, d->int_width);
+ if (d->frac_width)
+ rate += ls1x_pll_rate_part(val, d->frac_shift, d->frac_width);
+ rate *= parent_rate;
+ rate >>= d->shift;
+
+ return rate;
+}
+
+static const struct clk_ops ls1x_pll_clk_ops = {
+ .recalc_rate = ls1x_pll_recalc_rate,
+};
+
+static unsigned long ls1x_divider_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
+ const struct ls1x_clk_div_data *d = ls1x_clk->data;
+ unsigned int val;
+
+ val = readl(ls1x_clk->reg) >> d->shift;
+ val &= clk_div_mask(d->width);
+
+ return divider_recalc_rate(hw, parent_rate, val, d->table,
+ d->flags, d->width);
+}
+
+static long ls1x_divider_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
+ const struct ls1x_clk_div_data *d = ls1x_clk->data;
+
+ return divider_round_rate(hw, rate, prate, d->table,
+ d->width, d->flags);
+}
+
+static int ls1x_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
+ const struct ls1x_clk_div_data *d = ls1x_clk->data;
+ int val, div_val;
+ unsigned long flags = 0;
+
+ div_val = divider_get_val(rate, parent_rate, d->table,
+ d->width, d->flags);
+ if (div_val < 0)
+ return div_val;
+
+ if (d->lock)
+ spin_lock_irqsave(d->lock, flags);
+ else
+ __acquire(d->lock);
+
+ /* Bypass the clock */
+ val = readl(ls1x_clk->reg);
+ if (d->bypass_inv)
+ val &= ~BIT(d->bypass_shift);
+ else
+ val |= BIT(d->bypass_shift);
+ writel(val, ls1x_clk->reg);
+
+ val = readl(ls1x_clk->reg);
+ val &= ~(clk_div_mask(d->width) << d->shift);
+ val |= (u32)div_val << d->shift;
+ writel(val, ls1x_clk->reg);
+
+ /* Restore the clock */
+ val = readl(ls1x_clk->reg);
+ if (d->bypass_inv)
+ val |= BIT(d->bypass_shift);
+ else
+ val &= ~BIT(d->bypass_shift);
+ writel(val, ls1x_clk->reg);
+
+ if (d->lock)
+ spin_unlock_irqrestore(d->lock, flags);
+ else
+ __release(d->lock);
+
+ return 0;
+}
+
+static const struct clk_ops ls1x_clk_divider_ops = {
+ .recalc_rate = ls1x_divider_recalc_rate,
+ .round_rate = ls1x_divider_round_rate,
+ .set_rate = ls1x_divider_set_rate,
+};
+
+#define LS1X_CLK_PLL(_name, _offset, _fixed, _shift, \
+ f_shift, f_width, i_shift, i_width) \
+struct ls1x_clk _name = { \
+ .offset = (_offset), \
+ .data = &(struct ls1x_clk_pll_data) { \
+ .fixed = (_fixed), \
+ .shift = (_shift), \
+ .int_shift = (i_shift), \
+ .int_width = (i_width), \
+ .frac_shift = (f_shift), \
+ .frac_width = (f_width), \
+ }, \
+ .hw.init = &(struct clk_init_data) { \
+ .name = #_name, \
+ .ops = &ls1x_pll_clk_ops, \
+ .parent_data = &(const struct clk_parent_data) { \
+ .fw_name = "xtal", \
+ .name = "xtal", \
+ .index = -1, \
+ }, \
+ .num_parents = 1, \
+ }, \
+}
+
+#define LS1X_CLK_DIV(_name, _pname, _offset, _shift, _width, \
+ _table, _bypass_shift, _bypass_inv, _flags) \
+struct ls1x_clk _name = { \
+ .offset = (_offset), \
+ .data = &(struct ls1x_clk_div_data){ \
+ .shift = (_shift), \
+ .width = (_width), \
+ .table = (_table), \
+ .flags = (_flags), \
+ .bypass_shift = (_bypass_shift), \
+ .bypass_inv = (_bypass_inv), \
+ .lock = &ls1x_clk_div_lock, \
+ }, \
+ .hw.init = &(struct clk_init_data) { \
+ .name = #_name, \
+ .ops = &ls1x_clk_divider_ops, \
+ .parent_hws = (const struct clk_hw *[]) { _pname }, \
+ .num_parents = 1, \
+ .flags = CLK_GET_RATE_NOCACHE, \
+ }, \
+}
+
+static LS1X_CLK_PLL(ls1b_clk_pll, CLK_PLL_FREQ, 12, 1, 0, 5, 0, 0);
+static LS1X_CLK_DIV(ls1b_clk_cpu, &ls1b_clk_pll.hw, CLK_PLL_DIV,
+ 20, 4, NULL, 8, 0,
+ CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
+static LS1X_CLK_DIV(ls1b_clk_dc, &ls1b_clk_pll.hw, CLK_PLL_DIV,
+ 26, 4, NULL, 12, 0, CLK_DIVIDER_ONE_BASED);
+static LS1X_CLK_DIV(ls1b_clk_ahb, &ls1b_clk_pll.hw, CLK_PLL_DIV,
+ 14, 4, NULL, 10, 0, CLK_DIVIDER_ONE_BASED);
+static CLK_FIXED_FACTOR(ls1b_clk_apb, "ls1b_clk_apb", "ls1b_clk_ahb", 2, 1,
+ CLK_SET_RATE_PARENT);
+
+static struct clk_hw_onecell_data ls1b_clk_hw_data = {
+ .hws = {
+ [LS1X_CLKID_PLL] = &ls1b_clk_pll.hw,
+ [LS1X_CLKID_CPU] = &ls1b_clk_cpu.hw,
+ [LS1X_CLKID_DC] = &ls1b_clk_dc.hw,
+ [LS1X_CLKID_AHB] = &ls1b_clk_ahb.hw,
+ [LS1X_CLKID_APB] = &ls1b_clk_apb.hw,
+ [CLK_NR_CLKS] = NULL,
+ },
+ .num = CLK_NR_CLKS,
+};
+
+static const struct clk_div_table ls1c_ahb_div_table[] = {
+ [0] = { .val = 0, .div = 2 },
+ [1] = { .val = 1, .div = 4 },
+ [2] = { .val = 2, .div = 3 },
+ [3] = { .val = 3, .div = 3 },
+ [4] = { /* sentinel */ }
+};
+
+static LS1X_CLK_PLL(ls1c_clk_pll, CLK_PLL_FREQ, 0, 2, 8, 8, 16, 8);
+static LS1X_CLK_DIV(ls1c_clk_cpu, &ls1c_clk_pll.hw, CLK_PLL_DIV,
+ 8, 7, NULL, 0, 1,
+ CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
+static LS1X_CLK_DIV(ls1c_clk_dc, &ls1c_clk_pll.hw, CLK_PLL_DIV,
+ 24, 7, NULL, 4, 1, CLK_DIVIDER_ONE_BASED);
+static LS1X_CLK_DIV(ls1c_clk_ahb, &ls1c_clk_cpu.hw, CLK_PLL_FREQ,
+ 0, 2, ls1c_ahb_div_table, 0, 0, CLK_DIVIDER_ALLOW_ZERO);
+static CLK_FIXED_FACTOR(ls1c_clk_apb, "ls1c_clk_apb", "ls1c_clk_ahb", 1, 1,
+ CLK_SET_RATE_PARENT);
+
+static struct clk_hw_onecell_data ls1c_clk_hw_data = {
+ .hws = {
+ [LS1X_CLKID_PLL] = &ls1c_clk_pll.hw,
+ [LS1X_CLKID_CPU] = &ls1c_clk_cpu.hw,
+ [LS1X_CLKID_DC] = &ls1c_clk_dc.hw,
+ [LS1X_CLKID_AHB] = &ls1c_clk_ahb.hw,
+ [LS1X_CLKID_APB] = &ls1c_clk_apb.hw,
+ [CLK_NR_CLKS] = NULL,
+ },
+ .num = CLK_NR_CLKS,
+};
+
+static void __init ls1x_clk_init(struct device_node *np,
+ struct clk_hw_onecell_data *hw_data)
+{
+ struct ls1x_clk *ls1x_clk;
+ void __iomem *reg;
+ int i, ret;
+
+ reg = of_iomap(np, 0);
+ if (!reg) {
+ pr_err("Unable to map base for %pOF\n", np);
+ return;
+ }
+
+ for (i = 0; i < CLK_NR_CLKS; i++) {
+ /* array might be sparse */
+ if (!hw_data->hws[i])
+ continue;
+
+ if (i != LS1X_CLKID_APB) {
+ ls1x_clk = to_ls1x_clk(hw_data->hws[i]);
+ ls1x_clk->reg = reg + ls1x_clk->offset;
+ }
+
+ ret = of_clk_hw_register(np, hw_data->hws[i]);
+ if (ret)
+ return;
+ }
+
+ ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, hw_data);
+ if (ret)
+ pr_err("Failed to register %pOF\n", np);
+}
+
+static void __init ls1b_clk_init(struct device_node *np)
+{
+ return ls1x_clk_init(np, &ls1b_clk_hw_data);
+}
+
+static void __init ls1c_clk_init(struct device_node *np)
+{
+ return ls1x_clk_init(np, &ls1c_clk_hw_data);
+}
+
+CLK_OF_DECLARE(ls1b_clk, "loongson,ls1b-clk", ls1b_clk_init);
+CLK_OF_DECLARE(ls1c_clk, "loongson,ls1c-clk", ls1c_clk_init);
+
+MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
+MODULE_DESCRIPTION("Loongson1 clock driver");
--
2.34.1Quoting Keguang Zhang (2023-03-16 03:47:06)
> diff --git a/drivers/clk/clk-loongson1.c b/drivers/clk/clk-loongson1.c
> new file mode 100644
> index 000000000000..4fda55c67d8d
> --- /dev/null
> +++ b/drivers/clk/clk-loongson1.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Clock driver for Loongson-1 SoC
> + *
> + * Copyright (C) 2012-2023 Keguang Zhang <keguang.zhang@gmail.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
Need some more includes here, for container_of() and GENMASK(), readl(),
etc.
> +
> +#include <dt-bindings/clock/loongson,ls1x-clk.h>
> +
> +/* Loongson 1 Clock Register Definitions */
> +#define CLK_PLL_FREQ 0x0
> +#define CLK_PLL_DIV 0x4
> +
> +static DEFINE_SPINLOCK(ls1x_clk_div_lock);
> +
Needs include.
> +struct ls1x_clk_pll_data {
> + u32 fixed;
> + u8 shift;
> + u8 int_shift;
> + u8 int_width;
> + u8 frac_shift;
> + u8 frac_width;
> +};
> +
> +struct ls1x_clk_div_data {
> + u8 shift;
> + u8 width;
> + unsigned long flags;
> + const struct clk_div_table *table;
> + u8 bypass_shift;
> + u8 bypass_inv;
> + spinlock_t *lock; /* protect access to DIV registers */
> +};
> +
> +struct ls1x_clk {
> + void __iomem *reg;
> + unsigned int offset;
> + struct clk_hw hw;
> + void *data;
> +};
> +
> +#define to_ls1x_clk(_hw) container_of(_hw, struct ls1x_clk, hw)
> +
> +static inline unsigned long ls1x_pll_rate_part(unsigned int val,
return a u32?
> + unsigned int shift,
> + unsigned int width)
> +{
> + return (val & GENMASK(shift + width, shift)) >> shift;
> +}
> +
> +static unsigned long ls1x_pll_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> + const struct ls1x_clk_pll_data *d = ls1x_clk->data;
> + u32 val, rate;
> +
> + val = readl(ls1x_clk->reg);
> + rate = d->fixed;
> + rate += ls1x_pll_rate_part(val, d->int_shift, d->int_width);
> + if (d->frac_width)
> + rate += ls1x_pll_rate_part(val, d->frac_shift, d->frac_width);
> + rate *= parent_rate;
> + rate >>= d->shift;
> +
> + return rate;
> +}
> +
> +static const struct clk_ops ls1x_pll_clk_ops = {
> + .recalc_rate = ls1x_pll_recalc_rate,
> +};
> +
> +static unsigned long ls1x_divider_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> + unsigned int val;
> +
> + val = readl(ls1x_clk->reg) >> d->shift;
> + val &= clk_div_mask(d->width);
> +
> + return divider_recalc_rate(hw, parent_rate, val, d->table,
> + d->flags, d->width);
> +}
> +
> +static long ls1x_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> +
> + return divider_round_rate(hw, rate, prate, d->table,
> + d->width, d->flags);
> +}
> +
> +static int ls1x_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> + int val, div_val;
> + unsigned long flags = 0;
> +
> + div_val = divider_get_val(rate, parent_rate, d->table,
> + d->width, d->flags);
> + if (div_val < 0)
> + return div_val;
> +
> + if (d->lock)
> + spin_lock_irqsave(d->lock, flags);
> + else
> + __acquire(d->lock);
> +
> + /* Bypass the clock */
> + val = readl(ls1x_clk->reg);
> + if (d->bypass_inv)
> + val &= ~BIT(d->bypass_shift);
> + else
> + val |= BIT(d->bypass_shift);
> + writel(val, ls1x_clk->reg);
> +
> + val = readl(ls1x_clk->reg);
> + val &= ~(clk_div_mask(d->width) << d->shift);
> + val |= (u32)div_val << d->shift;
> + writel(val, ls1x_clk->reg);
> +
> + /* Restore the clock */
> + val = readl(ls1x_clk->reg);
> + if (d->bypass_inv)
> + val |= BIT(d->bypass_shift);
> + else
> + val &= ~BIT(d->bypass_shift);
> + writel(val, ls1x_clk->reg);
> +
> + if (d->lock)
> + spin_unlock_irqrestore(d->lock, flags);
> + else
> + __release(d->lock);
Is there a case where there isn't a lock? It would be easier to read if
this always had a lock and it wasn't optional.
> +
> + return 0;
> +}
> +
> +static const struct clk_ops ls1x_clk_divider_ops = {
> + .recalc_rate = ls1x_divider_recalc_rate,
> + .round_rate = ls1x_divider_round_rate,
> + .set_rate = ls1x_divider_set_rate,
> +};
> +
> +#define LS1X_CLK_PLL(_name, _offset, _fixed, _shift, \
> + f_shift, f_width, i_shift, i_width) \
> +struct ls1x_clk _name = { \
> + .offset = (_offset), \
> + .data = &(struct ls1x_clk_pll_data) { \
> + .fixed = (_fixed), \
> + .shift = (_shift), \
> + .int_shift = (i_shift), \
> + .int_width = (i_width), \
> + .frac_shift = (f_shift), \
> + .frac_width = (f_width), \
> + }, \
> + .hw.init = &(struct clk_init_data) { \
> + .name = #_name, \
> + .ops = &ls1x_pll_clk_ops, \
> + .parent_data = &(const struct clk_parent_data) { \
> + .fw_name = "xtal", \
> + .name = "xtal", \
> + .index = -1, \
> + }, \
> + .num_parents = 1, \
> + }, \
> +}
> +
> +#define LS1X_CLK_DIV(_name, _pname, _offset, _shift, _width, \
> + _table, _bypass_shift, _bypass_inv, _flags) \
> +struct ls1x_clk _name = { \
> + .offset = (_offset), \
> + .data = &(struct ls1x_clk_div_data){ \
> + .shift = (_shift), \
> + .width = (_width), \
> + .table = (_table), \
> + .flags = (_flags), \
> + .bypass_shift = (_bypass_shift), \
> + .bypass_inv = (_bypass_inv), \
> + .lock = &ls1x_clk_div_lock, \
> + }, \
> + .hw.init = &(struct clk_init_data) { \
Can be const.
> + .name = #_name, \
> + .ops = &ls1x_clk_divider_ops, \
> + .parent_hws = (const struct clk_hw *[]) { _pname }, \
> + .num_parents = 1, \
> + .flags = CLK_GET_RATE_NOCACHE, \
> + }, \
> +}
> +
> +static LS1X_CLK_PLL(ls1b_clk_pll, CLK_PLL_FREQ, 12, 1, 0, 5, 0, 0);
> +static LS1X_CLK_DIV(ls1b_clk_cpu, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> + 20, 4, NULL, 8, 0,
> + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
> +static LS1X_CLK_DIV(ls1b_clk_dc, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> + 26, 4, NULL, 12, 0, CLK_DIVIDER_ONE_BASED);
> +static LS1X_CLK_DIV(ls1b_clk_ahb, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> + 14, 4, NULL, 10, 0, CLK_DIVIDER_ONE_BASED);
> +static CLK_FIXED_FACTOR(ls1b_clk_apb, "ls1b_clk_apb", "ls1b_clk_ahb", 2, 1,
> + CLK_SET_RATE_PARENT);
> +
> +static struct clk_hw_onecell_data ls1b_clk_hw_data = {
> + .hws = {
> + [LS1X_CLKID_PLL] = &ls1b_clk_pll.hw,
> + [LS1X_CLKID_CPU] = &ls1b_clk_cpu.hw,
> + [LS1X_CLKID_DC] = &ls1b_clk_dc.hw,
> + [LS1X_CLKID_AHB] = &ls1b_clk_ahb.hw,
> + [LS1X_CLKID_APB] = &ls1b_clk_apb.hw,
> + [CLK_NR_CLKS] = NULL,
Do you need a CLK_NR_CLKS sentinel entry?
> + },
> + .num = CLK_NR_CLKS,
> +};
> +
> +static const struct clk_div_table ls1c_ahb_div_table[] = {
> + [0] = { .val = 0, .div = 2 },
> + [1] = { .val = 1, .div = 4 },
> + [2] = { .val = 2, .div = 3 },
> + [3] = { .val = 3, .div = 3 },
> + [4] = { /* sentinel */ }
> +};
> +
> +static LS1X_CLK_PLL(ls1c_clk_pll, CLK_PLL_FREQ, 0, 2, 8, 8, 16, 8);
> +static LS1X_CLK_DIV(ls1c_clk_cpu, &ls1c_clk_pll.hw, CLK_PLL_DIV,
> + 8, 7, NULL, 0, 1,
> + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
> +static LS1X_CLK_DIV(ls1c_clk_dc, &ls1c_clk_pll.hw, CLK_PLL_DIV,
> + 24, 7, NULL, 4, 1, CLK_DIVIDER_ONE_BASED);
> +static LS1X_CLK_DIV(ls1c_clk_ahb, &ls1c_clk_cpu.hw, CLK_PLL_FREQ,
> + 0, 2, ls1c_ahb_div_table, 0, 0, CLK_DIVIDER_ALLOW_ZERO);
> +static CLK_FIXED_FACTOR(ls1c_clk_apb, "ls1c_clk_apb", "ls1c_clk_ahb", 1, 1,
> + CLK_SET_RATE_PARENT);
> +
> +static struct clk_hw_onecell_data ls1c_clk_hw_data = {
> + .hws = {
> + [LS1X_CLKID_PLL] = &ls1c_clk_pll.hw,
> + [LS1X_CLKID_CPU] = &ls1c_clk_cpu.hw,
> + [LS1X_CLKID_DC] = &ls1c_clk_dc.hw,
> + [LS1X_CLKID_AHB] = &ls1c_clk_ahb.hw,
> + [LS1X_CLKID_APB] = &ls1c_clk_apb.hw,
> + [CLK_NR_CLKS] = NULL,
> + },
> + .num = CLK_NR_CLKS,
> +};
> +
> +static void __init ls1x_clk_init(struct device_node *np,
> + struct clk_hw_onecell_data *hw_data)
> +{
> + struct ls1x_clk *ls1x_clk;
> + void __iomem *reg;
> + int i, ret;
> +
> + reg = of_iomap(np, 0);
> + if (!reg) {
> + pr_err("Unable to map base for %pOF\n", np);
Needs include.
> + return;
> + }
> +
> + for (i = 0; i < CLK_NR_CLKS; i++) {
> + /* array might be sparse */
> + if (!hw_data->hws[i])
> + continue;
> +
> + if (i != LS1X_CLKID_APB) {
> + ls1x_clk = to_ls1x_clk(hw_data->hws[i]);
> + ls1x_clk->reg = reg + ls1x_clk->offset;
> + }
> +
> + ret = of_clk_hw_register(np, hw_data->hws[i]);
> + if (ret)
> + return;
unmap memory on failure? and unregister clks?
> + }
> +
> + ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, hw_data);
> + if (ret)
> + pr_err("Failed to register %pOF\n", np);
unmap memory on failure? And unregister clks?
> +}
> +
> +static void __init ls1b_clk_init(struct device_node *np)
> +{
> + return ls1x_clk_init(np, &ls1b_clk_hw_data);
> +}
> +
> +static void __init ls1c_clk_init(struct device_node *np)
> +{
> + return ls1x_clk_init(np, &ls1c_clk_hw_data);
> +}
> +
> +CLK_OF_DECLARE(ls1b_clk, "loongson,ls1b-clk", ls1b_clk_init);
> +CLK_OF_DECLARE(ls1c_clk, "loongson,ls1c-clk", ls1c_clk_init);
Any reason these can't be platform device drivers?
> +
> +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
> +MODULE_DESCRIPTION("Loongson1 clock driver");
It's not a module. So these are useless macros. Drop them?
On Tue, Mar 21, 2023 at 4:06 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Keguang Zhang (2023-03-16 03:47:06)
> > diff --git a/drivers/clk/clk-loongson1.c b/drivers/clk/clk-loongson1.c
> > new file mode 100644
> > index 000000000000..4fda55c67d8d
> > --- /dev/null
> > +++ b/drivers/clk/clk-loongson1.c
> > @@ -0,0 +1,301 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Clock driver for Loongson-1 SoC
> > + *
> > + * Copyright (C) 2012-2023 Keguang Zhang <keguang.zhang@gmail.com>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
>
> Need some more includes here, for container_of() and GENMASK(), readl(),
> etc.
>
Will do.
> > +
> > +#include <dt-bindings/clock/loongson,ls1x-clk.h>
> > +
> > +/* Loongson 1 Clock Register Definitions */
> > +#define CLK_PLL_FREQ 0x0
> > +#define CLK_PLL_DIV 0x4
> > +
> > +static DEFINE_SPINLOCK(ls1x_clk_div_lock);
> > +
>
> Needs include.
>
Will do.
> > +struct ls1x_clk_pll_data {
> > + u32 fixed;
> > + u8 shift;
> > + u8 int_shift;
> > + u8 int_width;
> > + u8 frac_shift;
> > + u8 frac_width;
> > +};
> > +
> > +struct ls1x_clk_div_data {
> > + u8 shift;
> > + u8 width;
> > + unsigned long flags;
> > + const struct clk_div_table *table;
> > + u8 bypass_shift;
> > + u8 bypass_inv;
> > + spinlock_t *lock; /* protect access to DIV registers */
> > +};
> > +
> > +struct ls1x_clk {
> > + void __iomem *reg;
> > + unsigned int offset;
> > + struct clk_hw hw;
> > + void *data;
> > +};
> > +
> > +#define to_ls1x_clk(_hw) container_of(_hw, struct ls1x_clk, hw)
> > +
> > +static inline unsigned long ls1x_pll_rate_part(unsigned int val,
>
> return a u32?
>
Yes.
> > + unsigned int shift,
> > + unsigned int width)
> > +{
> > + return (val & GENMASK(shift + width, shift)) >> shift;
> > +}
> > +
> > +static unsigned long ls1x_pll_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> > + const struct ls1x_clk_pll_data *d = ls1x_clk->data;
> > + u32 val, rate;
> > +
> > + val = readl(ls1x_clk->reg);
> > + rate = d->fixed;
> > + rate += ls1x_pll_rate_part(val, d->int_shift, d->int_width);
> > + if (d->frac_width)
> > + rate += ls1x_pll_rate_part(val, d->frac_shift, d->frac_width);
> > + rate *= parent_rate;
> > + rate >>= d->shift;
> > +
> > + return rate;
> > +}
> > +
> > +static const struct clk_ops ls1x_pll_clk_ops = {
> > + .recalc_rate = ls1x_pll_recalc_rate,
> > +};
> > +
> > +static unsigned long ls1x_divider_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> > + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> > + unsigned int val;
> > +
> > + val = readl(ls1x_clk->reg) >> d->shift;
> > + val &= clk_div_mask(d->width);
> > +
> > + return divider_recalc_rate(hw, parent_rate, val, d->table,
> > + d->flags, d->width);
> > +}
> > +
> > +static long ls1x_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long *prate)
> > +{
> > + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> > + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> > +
> > + return divider_round_rate(hw, rate, prate, d->table,
> > + d->width, d->flags);
> > +}
> > +
> > +static int ls1x_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct ls1x_clk *ls1x_clk = to_ls1x_clk(hw);
> > + const struct ls1x_clk_div_data *d = ls1x_clk->data;
> > + int val, div_val;
> > + unsigned long flags = 0;
> > +
> > + div_val = divider_get_val(rate, parent_rate, d->table,
> > + d->width, d->flags);
> > + if (div_val < 0)
> > + return div_val;
> > +
> > + if (d->lock)
> > + spin_lock_irqsave(d->lock, flags);
> > + else
> > + __acquire(d->lock);
> > +
> > + /* Bypass the clock */
> > + val = readl(ls1x_clk->reg);
> > + if (d->bypass_inv)
> > + val &= ~BIT(d->bypass_shift);
> > + else
> > + val |= BIT(d->bypass_shift);
> > + writel(val, ls1x_clk->reg);
> > +
> > + val = readl(ls1x_clk->reg);
> > + val &= ~(clk_div_mask(d->width) << d->shift);
> > + val |= (u32)div_val << d->shift;
> > + writel(val, ls1x_clk->reg);
> > +
> > + /* Restore the clock */
> > + val = readl(ls1x_clk->reg);
> > + if (d->bypass_inv)
> > + val |= BIT(d->bypass_shift);
> > + else
> > + val &= ~BIT(d->bypass_shift);
> > + writel(val, ls1x_clk->reg);
> > +
> > + if (d->lock)
> > + spin_unlock_irqrestore(d->lock, flags);
> > + else
> > + __release(d->lock);
>
> Is there a case where there isn't a lock? It would be easier to read if
> this always had a lock and it wasn't optional.
>
Indeed. The lock always exists in this driver.
Will remove the unnecessary __acquire()/__release().
> > +
> > + return 0;
> > +}
> > +
> > +static const struct clk_ops ls1x_clk_divider_ops = {
> > + .recalc_rate = ls1x_divider_recalc_rate,
> > + .round_rate = ls1x_divider_round_rate,
> > + .set_rate = ls1x_divider_set_rate,
> > +};
> > +
> > +#define LS1X_CLK_PLL(_name, _offset, _fixed, _shift, \
> > + f_shift, f_width, i_shift, i_width) \
> > +struct ls1x_clk _name = { \
> > + .offset = (_offset), \
> > + .data = &(struct ls1x_clk_pll_data) { \
> > + .fixed = (_fixed), \
> > + .shift = (_shift), \
> > + .int_shift = (i_shift), \
> > + .int_width = (i_width), \
> > + .frac_shift = (f_shift), \
> > + .frac_width = (f_width), \
> > + }, \
> > + .hw.init = &(struct clk_init_data) { \
> > + .name = #_name, \
> > + .ops = &ls1x_pll_clk_ops, \
> > + .parent_data = &(const struct clk_parent_data) { \
> > + .fw_name = "xtal", \
> > + .name = "xtal", \
> > + .index = -1, \
> > + }, \
> > + .num_parents = 1, \
> > + }, \
> > +}
> > +
> > +#define LS1X_CLK_DIV(_name, _pname, _offset, _shift, _width, \
> > + _table, _bypass_shift, _bypass_inv, _flags) \
> > +struct ls1x_clk _name = { \
> > + .offset = (_offset), \
> > + .data = &(struct ls1x_clk_div_data){ \
> > + .shift = (_shift), \
> > + .width = (_width), \
> > + .table = (_table), \
> > + .flags = (_flags), \
> > + .bypass_shift = (_bypass_shift), \
> > + .bypass_inv = (_bypass_inv), \
> > + .lock = &ls1x_clk_div_lock, \
> > + }, \
> > + .hw.init = &(struct clk_init_data) { \
>
> Can be const.
>
Will do.
> > + .name = #_name, \
> > + .ops = &ls1x_clk_divider_ops, \
> > + .parent_hws = (const struct clk_hw *[]) { _pname }, \
> > + .num_parents = 1, \
> > + .flags = CLK_GET_RATE_NOCACHE, \
> > + }, \
> > +}
> > +
> > +static LS1X_CLK_PLL(ls1b_clk_pll, CLK_PLL_FREQ, 12, 1, 0, 5, 0, 0);
> > +static LS1X_CLK_DIV(ls1b_clk_cpu, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> > + 20, 4, NULL, 8, 0,
> > + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
> > +static LS1X_CLK_DIV(ls1b_clk_dc, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> > + 26, 4, NULL, 12, 0, CLK_DIVIDER_ONE_BASED);
> > +static LS1X_CLK_DIV(ls1b_clk_ahb, &ls1b_clk_pll.hw, CLK_PLL_DIV,
> > + 14, 4, NULL, 10, 0, CLK_DIVIDER_ONE_BASED);
> > +static CLK_FIXED_FACTOR(ls1b_clk_apb, "ls1b_clk_apb", "ls1b_clk_ahb", 2, 1,
> > + CLK_SET_RATE_PARENT);
> > +
> > +static struct clk_hw_onecell_data ls1b_clk_hw_data = {
> > + .hws = {
> > + [LS1X_CLKID_PLL] = &ls1b_clk_pll.hw,
> > + [LS1X_CLKID_CPU] = &ls1b_clk_cpu.hw,
> > + [LS1X_CLKID_DC] = &ls1b_clk_dc.hw,
> > + [LS1X_CLKID_AHB] = &ls1b_clk_ahb.hw,
> > + [LS1X_CLKID_APB] = &ls1b_clk_apb.hw,
> > + [CLK_NR_CLKS] = NULL,
>
> Do you need a CLK_NR_CLKS sentinel entry?
>
Not necessary.
Will remove.
> > + },
> > + .num = CLK_NR_CLKS,
> > +};
> > +
> > +static const struct clk_div_table ls1c_ahb_div_table[] = {
> > + [0] = { .val = 0, .div = 2 },
> > + [1] = { .val = 1, .div = 4 },
> > + [2] = { .val = 2, .div = 3 },
> > + [3] = { .val = 3, .div = 3 },
> > + [4] = { /* sentinel */ }
> > +};
> > +
> > +static LS1X_CLK_PLL(ls1c_clk_pll, CLK_PLL_FREQ, 0, 2, 8, 8, 16, 8);
> > +static LS1X_CLK_DIV(ls1c_clk_cpu, &ls1c_clk_pll.hw, CLK_PLL_DIV,
> > + 8, 7, NULL, 0, 1,
> > + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ROUND_CLOSEST);
> > +static LS1X_CLK_DIV(ls1c_clk_dc, &ls1c_clk_pll.hw, CLK_PLL_DIV,
> > + 24, 7, NULL, 4, 1, CLK_DIVIDER_ONE_BASED);
> > +static LS1X_CLK_DIV(ls1c_clk_ahb, &ls1c_clk_cpu.hw, CLK_PLL_FREQ,
> > + 0, 2, ls1c_ahb_div_table, 0, 0, CLK_DIVIDER_ALLOW_ZERO);
> > +static CLK_FIXED_FACTOR(ls1c_clk_apb, "ls1c_clk_apb", "ls1c_clk_ahb", 1, 1,
> > + CLK_SET_RATE_PARENT);
> > +
> > +static struct clk_hw_onecell_data ls1c_clk_hw_data = {
> > + .hws = {
> > + [LS1X_CLKID_PLL] = &ls1c_clk_pll.hw,
> > + [LS1X_CLKID_CPU] = &ls1c_clk_cpu.hw,
> > + [LS1X_CLKID_DC] = &ls1c_clk_dc.hw,
> > + [LS1X_CLKID_AHB] = &ls1c_clk_ahb.hw,
> > + [LS1X_CLKID_APB] = &ls1c_clk_apb.hw,
> > + [CLK_NR_CLKS] = NULL,
> > + },
> > + .num = CLK_NR_CLKS,
> > +};
> > +
> > +static void __init ls1x_clk_init(struct device_node *np,
> > + struct clk_hw_onecell_data *hw_data)
> > +{
> > + struct ls1x_clk *ls1x_clk;
> > + void __iomem *reg;
> > + int i, ret;
> > +
> > + reg = of_iomap(np, 0);
> > + if (!reg) {
> > + pr_err("Unable to map base for %pOF\n", np);
>
> Needs include.
>
Will do.
> > + return;
> > + }
> > +
> > + for (i = 0; i < CLK_NR_CLKS; i++) {
> > + /* array might be sparse */
> > + if (!hw_data->hws[i])
> > + continue;
> > +
> > + if (i != LS1X_CLKID_APB) {
> > + ls1x_clk = to_ls1x_clk(hw_data->hws[i]);
> > + ls1x_clk->reg = reg + ls1x_clk->offset;
> > + }
> > +
> > + ret = of_clk_hw_register(np, hw_data->hws[i]);
> > + if (ret)
> > + return;
>
> unmap memory on failure? and unregister clks?
>
Will add the error handling.
> > + }
> > +
> > + ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, hw_data);
> > + if (ret)
> > + pr_err("Failed to register %pOF\n", np);
>
> unmap memory on failure? And unregister clks?
>
Will add the error handling.
> > +}
> > +
> > +static void __init ls1b_clk_init(struct device_node *np)
> > +{
> > + return ls1x_clk_init(np, &ls1b_clk_hw_data);
> > +}
> > +
> > +static void __init ls1c_clk_init(struct device_node *np)
> > +{
> > + return ls1x_clk_init(np, &ls1c_clk_hw_data);
> > +}
> > +
> > +CLK_OF_DECLARE(ls1b_clk, "loongson,ls1b-clk", ls1b_clk_init);
> > +CLK_OF_DECLARE(ls1c_clk, "loongson,ls1c-clk", ls1c_clk_init);
>
> Any reason these can't be platform device drivers?
>
The Loongson1 system uses performance counter as the default clocksource.
Then, mips_hpt_frequency should be figured out when doing plat_time_init(),
just before calling mips_clockevent_init() and init_mips_clocksource().
And mips_hpt_frequency depends on CPU clock.
So these clocks should be initialized as early as possible.
It's too late for being a platform device driver.
> > +
> > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
> > +MODULE_DESCRIPTION("Loongson1 clock driver");
>
> It's not a module. So these are useless macros. Drop them?
Yes. I realized these macros will never be called.
Will drop.
--
Best regards,
Keguang Zhang
Hi Keguang,
I love your patch! Yet something to improve:
[auto build test ERROR on 6f173737e1b5670c200329677e821cce1d3d755e]
url: https://github.com/intel-lab-lkp/linux/commits/Keguang-Zhang/dt-bindings-clock-Add-Loongson-1-clock/20230316-185026
base: 6f173737e1b5670c200329677e821cce1d3d755e
patch link: https://lore.kernel.org/r/20230316104707.236034-4-keguang.zhang%40gmail.com
patch subject: [PATCH v3 3/4] clk: loongson1: Re-implement the clock driver
config: mips-loongson1c_defconfig (https://download.01.org/0day-ci/archive/20230318/202303181358.BXLJVMkh-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mipsel-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/de00eab744ddc82edb1853048dd5d50aa8220115
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Keguang-Zhang/dt-bindings-clock-Add-Loongson-1-clock/20230316-185026
git checkout de00eab744ddc82edb1853048dd5d50aa8220115
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303181358.BXLJVMkh-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/clk/clk-loongson1.c:300:15: error: expected parameter declarator
MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
^
>> drivers/clk/clk-loongson1.c:300:15: error: expected ')'
drivers/clk/clk-loongson1.c:300:14: note: to match this '('
MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
^
>> drivers/clk/clk-loongson1.c:300:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
^
int
>> drivers/clk/clk-loongson1.c:300:14: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
^
void
drivers/clk/clk-loongson1.c:301:20: error: expected parameter declarator
MODULE_DESCRIPTION("Loongson1 clock driver");
^
drivers/clk/clk-loongson1.c:301:20: error: expected ')'
drivers/clk/clk-loongson1.c:301:19: note: to match this '('
MODULE_DESCRIPTION("Loongson1 clock driver");
^
drivers/clk/clk-loongson1.c:301:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
MODULE_DESCRIPTION("Loongson1 clock driver");
^
int
drivers/clk/clk-loongson1.c:301:19: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
MODULE_DESCRIPTION("Loongson1 clock driver");
^
void
8 errors generated.
vim +300 drivers/clk/clk-loongson1.c
299
> 300 MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
© 2016 - 2026 Red Hat, Inc.