Quoting Jacky Huang (2023-03-27 19:19:09) > diff --git a/drivers/clk/nuvoton/Kconfig b/drivers/clk/nuvoton/Kconfig > new file mode 100644 > index 000000000000..c1324efedcb9 > --- /dev/null > +++ b/drivers/clk/nuvoton/Kconfig > @@ -0,0 +1,19 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# common clock support for Nuvoton SoC family. > + > +config COMMON_CLK_NUVOTON > + bool "Nuvoton clock controller common support" > + depends on ARCH_NUVOTON Add || COMPILE_TEST > + default ARCH_NUVOTON > + help > + Say y here to enable common clock controller for Nuvoton platforms. > + > +if COMMON_CLK_NUVOTON Add a newline here. > +config CLK_MA35D1 > + bool "Nuvoton MA35D1 clock controller support" > + depends on ARM64 || COMPILE_TEST Drop this > + default y And this. > + help > + Build the driver for MA35D1 Clock Driver. This says driver twice. "Build the clk driver for MA35D1 SoCs"? > + > +endif > diff --git a/drivers/clk/nuvoton/clk-ma35d1-divider.c b/drivers/clk/nuvoton/clk-ma35d1-divider.c > new file mode 100644 > index 000000000000..340a889a1417 > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1-divider.c > @@ -0,0 +1,140 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 Nuvoton Technology Corp. > + * Author: Chi-Fang Li <cfli0@xxxxxxxxxxx> > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/device.h> > +#include <linux/regmap.h> > +#include <linux/spinlock.h> > + > +#include "clk-ma35d1.h" > + > +#define clk_div_mask(width) ((1 << (width)) - 1) clk-provider.h defines this already, use that? > + > +struct ma35d1_adc_clk_div { > + struct clk_hw hw; > + void __iomem *reg; > + u8 shift; > + u8 width; > + u32 mask; > + const struct clk_div_table *table; > + /* protects concurrent access to clock divider registers */ > + spinlock_t *lock; > +}; > + > +static inline struct ma35d1_adc_clk_div *to_ma35d1_adc_clk_div(struct clk_hw *_hw) > +{ > + return container_of(_hw, struct ma35d1_adc_clk_div, hw); > +} > + > +static inline unsigned long ma35d1_clkdiv_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + unsigned int val; > + struct ma35d1_adc_clk_div *dclk = to_ma35d1_adc_clk_div(hw); > + > + val = readl_relaxed(dclk->reg) >> dclk->shift; > + val &= clk_div_mask(dclk->width); > + val += 1; > + return divider_recalc_rate(hw, parent_rate, val, dclk->table, > + CLK_DIVIDER_ROUND_CLOSEST, dclk->width); > +} > + > +static inline long ma35d1_clkdiv_round_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long *prate) > +{ > + struct ma35d1_adc_clk_div *dclk = to_ma35d1_adc_clk_div(hw); > + > + return divider_round_rate(hw, rate, prate, dclk->table, > + dclk->width, CLK_DIVIDER_ROUND_CLOSEST); > +} > + > +static inline int ma35d1_clkdiv_set_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long parent_rate) > +{ > + int value; > + unsigned long flags = 0; > + u32 data; > + struct ma35d1_adc_clk_div *dclk = to_ma35d1_adc_clk_div(hw); > + > + value = divider_get_val(rate, parent_rate, dclk->table, > + dclk->width, CLK_DIVIDER_ROUND_CLOSEST); > + > + if (dclk->lock) > + spin_lock_irqsave(dclk->lock, flags); Can you just always lock? That makes the conditional locking go away. > + > + data = readl_relaxed(dclk->reg); > + data &= ~(clk_div_mask(dclk->width) << dclk->shift); > + data |= (value - 1) << dclk->shift; > + data |= dclk->mask; > + > + writel_relaxed(data, dclk->reg); > + > + if (dclk->lock) > + spin_unlock_irqrestore(dclk->lock, flags); > + > + return 0; > +} > + > +static const struct clk_ops ma35d1_adc_clkdiv_ops = { > + .recalc_rate = ma35d1_clkdiv_recalc_rate, > + .round_rate = ma35d1_clkdiv_round_rate, > + .set_rate = ma35d1_clkdiv_set_rate, > +}; > + > +struct clk_hw *ma35d1_reg_adc_clkdiv(struct device *dev, const char *name, > + const char *parent_name, > + spinlock_t *lock, > + unsigned long flags, void __iomem *reg, > + u8 shift, u8 width, u32 mask_bit) > +{ > + struct ma35d1_adc_clk_div *div; > + struct clk_init_data init; > + struct clk_div_table *table; > + u32 max_div, min_div; > + struct clk_hw *hw; > + int ret; > + int i; > + > + div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL); > + if (!div) > + return ERR_PTR(-ENOMEM); > + > + max_div = clk_div_mask(width) + 1; > + min_div = 1; > + > + table = devm_kcalloc(dev, max_div + 1, sizeof(*table), GFP_KERNEL); > + if (!table) > + return ERR_PTR(-ENOMEM); > + > + for (i = 0; i < max_div; i++) { > + table[i].val = (min_div + i); Drop useless parenthesis. > + table[i].div = 2 * table[i].val; > + } > + table[max_div].val = 0; > + table[max_div].div = 0; > + > + init.name = name; > + init.ops = &ma35d1_adc_clkdiv_ops; > + init.flags |= flags; > + init.parent_names = parent_name ? &parent_name : NULL; > + init.num_parents = parent_name ? 1 : 0; > + > + div->reg = reg; > + div->shift = shift; > + div->width = width; > + div->mask = mask_bit ? BIT(mask_bit) : 0; > + div->lock = lock; > + div->hw.init = &init; > + div->table = table; > + > + hw = &div->hw; > + ret = devm_clk_hw_register(dev, hw); > + if (ret < 0) Use if (ret) instead. > + return ERR_PTR(ret); > + return hw; > +} > diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c > new file mode 100644 > index 000000000000..b36fbda4fa0a > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c > @@ -0,0 +1,350 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 Nuvoton Technology Corp. > + * Author: Chi-Fang Li <cfli0@xxxxxxxxxxx> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk-provider.h> > +#include <linux/slab.h> > +#include <linux/regmap.h> > + > +#include "clk-ma35d1.h" > + > +#define REG_SYS_RLKTZNS 0x1a4 /* Register Lock Control */ > + > +struct ma35d1_clk_pll { > + struct clk_hw hw; > + u8 type; > + u8 mode; > + unsigned long rate; > + void __iomem *ctl0_base; This needs an include for __iomem define. Just include kernel.h for now. > + void __iomem *ctl1_base; > + void __iomem *ctl2_base; > + struct regmap *regmap; > +}; > + > +static inline struct ma35d1_clk_pll *to_ma35d1_clk_pll(struct clk_hw *_hw) > +{ > + return container_of(_hw, struct ma35d1_clk_pll, hw); Missing include as well. > +} > + > +static void ma35d1_unlock_regs(struct ma35d1_clk_pll *pll) > +{ > + int ret; > + > + do { > + regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x59); > + regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x16); > + regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x88); > + regmap_read(pll->regmap, REG_SYS_RLKTZNS, &ret); > + } while (ret == 0); Any sort of timeout? > +} > + > +static void ma35d1_lock_regs(struct ma35d1_clk_pll *pll) > +{ > + regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x0); > +} > + > +static unsigned long ma35d1_calc_smic_pll_freq(u32 pll0_ctl0, > + unsigned long parent_rate) > +{ > + u32 m, n, p, outdiv; > + u64 pll_freq; > + u32 clk_div_table[] = { 1, 2, 4, 8 }; const > + > + if (pll0_ctl0 & SPLL0_CTL0_BP) > + return parent_rate; > + > + n = FIELD_GET(SPLL0_CTL0_FBDIV, pll0_ctl0); > + m = FIELD_GET(SPLL0_CTL0_INDIV, pll0_ctl0); > + p = FIELD_GET(SPLL0_CTL0_OUTDIV, pll0_ctl0); > + outdiv = clk_div_table[p]; > + pll_freq = (u64)parent_rate * n; > + do_div(pll_freq, m * outdiv); > + return (unsigned long)pll_freq; Remove useless cast. > +} > + > +static unsigned long ma35d1_calc_pll_freq(u8 mode, u32 *reg_ctl, > + unsigned long parent_rate) > +{ > + u32 m, n, p; > + u64 pll_freq, x; > + > + if (reg_ctl[1] & PLL_CTL1_BP) > + return parent_rate; > + > + if (mode == PLL_MODE_INT) { > + n = FIELD_GET(PLL_CTL0_FBDIV, reg_ctl[0]); > + m = FIELD_GET(PLL_CTL0_INDIV, reg_ctl[0]); > + p = FIELD_GET(PLL_CTL1_OUTDIV, reg_ctl[1]); > + pll_freq = (u64)parent_rate * n; > + do_div(pll_freq, m * p); > + } else { > + n = FIELD_GET(PLL_CTL0_FBDIV, reg_ctl[0]); > + m = FIELD_GET(PLL_CTL0_INDIV, reg_ctl[0]); > + p = FIELD_GET(PLL_CTL1_OUTDIV, reg_ctl[1]); The n, m, p are the same, so pull them out of the if-else to deduplicate. > + x = FIELD_GET(PLL_CTL1_FRAC, reg_ctl[1]); > + /* 2 decimal places floating to integer (ex. 1.23 to 123) */ > + n = n * 100 + ((x * 100) / FIELD_MAX(PLL_CTL1_FRAC)); Is this mult_frac()? > + pll_freq = ((u64)parent_rate * n) / 100 / m / p; > + } Add a newline > + return (unsigned long)pll_freq; Remove useless cast. > +} > + > +static int ma35d1_pll_find_closest(struct ma35d1_clk_pll *pll, > + unsigned long rate, > + unsigned long parent_rate, > + u32 *reg_ctl, unsigned long *freq) > +{ > + int p, m, n; > + int fbdiv_min, fbdiv_max; > + unsigned long diff = 0xffffffff; > + > + *freq = 0; > + if ((rate < PLL_FCLKO_MIN_FREQ) || (rate > PLL_FCLKO_MAX_FREQ)) > + return -EINVAL; > + > + if (pll->mode == PLL_MODE_INT) { > + fbdiv_min = FBDIV_MIN; > + fbdiv_max = FBDIV_MAX; > + } else { > + fbdiv_min = FBDIV_FRAC_MIN; > + fbdiv_max = FBDIV_FRAC_MAX; > + } > + > + for (m = INDIV_MIN; m <= INDIV_MAX; m++) { > + for (n = fbdiv_min; n <= fbdiv_max; n++) { > + for (p = OUTDIV_MIN; p <= OUTDIV_MAX; p++) { > + unsigned long tmp, fout; > + u64 fclk; > + > + tmp = parent_rate / m; > + if (tmp < PLL_FREF_M_MIN_FREQ || > + tmp > PLL_FREF_M_MAX_FREQ) > + continue; /* constrain */ > + > + fclk = (u64)parent_rate * n / m; > + /* for 2 decimal places */ > + if (pll->mode != PLL_MODE_INT) > + fclk /= 100; > + > + if (fclk < PLL_FCLK_MIN_FREQ || > + fclk > PLL_FCLK_MAX_FREQ) > + continue; /* constrain */ > + > + fout = (unsigned long)(fclk / p); > + if (fout < PLL_FCLKO_MIN_FREQ || > + fout > PLL_FCLKO_MAX_FREQ) > + continue; /* constrain */ > + > + if (abs(rate - fout) < diff) { > + reg_ctl[0] = FIELD_PREP(PLL_CTL0_INDIV, m) | > + FIELD_PREP(PLL_CTL0_FBDIV, n); > + reg_ctl[1] = FIELD_PREP(PLL_CTL1_OUTDIV, p); > + *freq = fout; > + diff = abs(rate - fout); > + if (diff == 0) > + break; > + } > + } > + } > + } > + if (*freq == 0) > + return -EINVAL; /* cannot find even one valid setting */ > + return 0; > +} > + > +static int ma35d1_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 reg_ctl[3] = { 0 }; > + unsigned long pll_freq; > + int ret; > + > + if ((parent_rate < PLL_FREF_MIN_FREQ) || > + (parent_rate > PLL_FREF_MAX_FREQ)) Remove useless parenthesis. > + return -EINVAL; > + > + if ((pll->type == MA35D1_CAPLL) || (pll->type == MA35D1_DDRPLL)) { Remove useless parenthesis. > + pr_warn("Nuvoton MA35D1 CAPLL/DDRPLL is read only.\n"); > + return -EACCES; > + } > + > + ret = ma35d1_pll_find_closest(pll, rate, parent_rate, reg_ctl, &pll_freq); > + if (ret != 0) > + return ret; > + pll->rate = pll_freq; > + > + switch (pll->mode) { > + case PLL_MODE_INT: > + reg_ctl[0] |= FIELD_PREP(PLL_CTL0_MODE, PLL_MODE_INT); > + break; > + case PLL_MODE_FRAC: > + reg_ctl[0] |= FIELD_PREP(PLL_CTL0_MODE, PLL_MODE_FRAC); > + break; > + case PLL_MODE_SS: > + reg_ctl[0] |= FIELD_PREP(PLL_CTL0_MODE, PLL_MODE_SS) | > + FIELD_PREP(PLL_CTL0_SSRATE, PLL_SS_RATE); > + reg_ctl[2] = FIELD_PREP(PLL_CTL2_SLOPE, PLL_SLOPE); > + break; > + } > + reg_ctl[1] |= PLL_CTL1_PD; > + > + ma35d1_unlock_regs(pll); > + writel_relaxed(reg_ctl[0], pll->ctl0_base); > + writel_relaxed(reg_ctl[1], pll->ctl1_base); > + writel_relaxed(reg_ctl[2], pll->ctl2_base); > + ma35d1_lock_regs(pll); > + return 0; > +} > + > +static unsigned long ma35d1_clk_pll_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 reg_ctl[3]; > + unsigned long pll_freq; > + > + if ((parent_rate < PLL_FREF_MIN_FREQ) || (parent_rate > PLL_FREF_MAX_FREQ)) Remove useless parenthesis. > + return 0; > + > + switch (pll->type) { > + case MA35D1_CAPLL: > + reg_ctl[0] = readl_relaxed(pll->ctl0_base); > + pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], parent_rate); return? > + break; > + case MA35D1_DDRPLL: > + case MA35D1_APLL: > + case MA35D1_EPLL: > + case MA35D1_VPLL: > + reg_ctl[0] = readl_relaxed(pll->ctl0_base); > + reg_ctl[1] = readl_relaxed(pll->ctl1_base); > + pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, parent_rate); return? > + break; > + } > + return pll_freq; return 0? > +} > + > +static long ma35d1_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 reg_ctl[3] = { 0 }; > + unsigned long pll_freq; > + long ret; > + > + if (*parent_rate < PLL_FREF_MIN_FREQ || *parent_rate > PLL_FREF_MAX_FREQ) > + return -EINVAL; > + > + ret = ma35d1_pll_find_closest(pll, rate, *parent_rate, reg_ctl, &pll_freq); > + if (ret != 0) > + return ret; > + > + switch (pll->type) { > + case MA35D1_CAPLL: > + reg_ctl[0] = readl_relaxed(pll->ctl0_base); > + pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], *parent_rate); return? > + break; > + case MA35D1_DDRPLL: > + case MA35D1_APLL: > + case MA35D1_EPLL: > + case MA35D1_VPLL: > + reg_ctl[0] = readl_relaxed(pll->ctl0_base); > + reg_ctl[1] = readl_relaxed(pll->ctl1_base); > + pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, *parent_rate); return? > + break; > + } > + return pll_freq; return 0? > +} > + > +static int ma35d1_clk_pll_is_prepared(struct clk_hw *hw) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 val = readl_relaxed(pll->ctl1_base); > + > + return val & PLL_CTL1_PD ? 0 : 1; > +} > + > +static int ma35d1_clk_pll_prepare(struct clk_hw *hw) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 val; > + > + if ((pll->type == MA35D1_CAPLL) || (pll->type == MA35D1_DDRPLL)) { Drop useless parenthesis. > + pr_warn("Nuvoton MA35D1 CAPLL/DDRPLL is read only.\n"); > + return -EACCES; > + } > + > + ma35d1_unlock_regs(pll); > + val = readl_relaxed(pll->ctl1_base); > + val &= ~PLL_CTL1_PD; > + writel_relaxed(val, pll->ctl1_base); > + ma35d1_lock_regs(pll); > + return 0; > +} > + > +static void ma35d1_clk_pll_unprepare(struct clk_hw *hw) > +{ > + struct ma35d1_clk_pll *pll = to_ma35d1_clk_pll(hw); > + u32 val; > + > + if ((pll->type == MA35D1_CAPLL) || (pll->type == MA35D1_DDRPLL)) { Drop useless parenthesis. > + pr_warn("Nuvoton MA35D1 CAPLL/DDRPLL is read only.\n"); These read-only checks should be removed and different clk_ops should be assigned for the read-only type of clks. > + } else { > + val = readl_relaxed(pll->ctl1_base); > + val |= PLL_CTL1_PD; > + writel_relaxed(val, pll->ctl1_base); > + } > +} > + > +static const struct clk_ops ma35d1_clk_pll_ops = { > + .is_prepared = ma35d1_clk_pll_is_prepared, > + .prepare = ma35d1_clk_pll_prepare, > + .unprepare = ma35d1_clk_pll_unprepare, > + .set_rate = ma35d1_clk_pll_set_rate, > + .recalc_rate = ma35d1_clk_pll_recalc_rate, > + .round_rate = ma35d1_clk_pll_round_rate, > +}; > + > +struct clk_hw *ma35d1_reg_clk_pll(enum ma35d1_pll_type type, Pass a device here. > + u8 u8mode, const char *name, > + const char *parent, > + unsigned long targetFreq, > + void __iomem *base, > + struct regmap *regmap) > +{ > + struct ma35d1_clk_pll *pll; > + struct clk_hw *hw; > + struct clk_init_data init; init = {}; > + int ret; > + > + pll = kmalloc(sizeof(*pll), GFP_KERNEL); Just use kzalloc(). This isn't a hot path. > + if (!pll) > + return ERR_PTR(-ENOMEM); > + > + pll->type = type; > + pll->mode = u8mode; > + pll->rate = targetFreq; > + pll->ctl0_base = base + REG_PLL_CTL0_OFFSET; > + pll->ctl1_base = base + REG_PLL_CTL1_OFFSET; > + pll->ctl2_base = base + REG_PLL_CTL2_OFFSET; > + pll->regmap = regmap; > + > + init.name = name; > + init.flags = 0; > + init.parent_names = &parent; > + init.num_parents = 1; > + init.ops = &ma35d1_clk_pll_ops; > + pll->hw.init = &init; > + hw = &pll->hw; > + > + ret = clk_hw_register(NULL, hw); Use devm_ > + if (ret) { > + pr_err("failed to register vsi-pll clock!!!\n"); I'm going to put a dev_err_probe() into clk registration code. Remove this printk. > + kfree(pll); > + return ERR_PTR(ret); > + } > + return hw; > +} > diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c > new file mode 100644 > index 000000000000..e4d3ced396a3 > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1.c > @@ -0,0 +1,963 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 Nuvoton Technology Corp. > + * Author: Chi-Fang Li <cfli0@xxxxxxxxxxx> > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/spinlock.h> > +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h> > + > +#include "clk-ma35d1.h" > + > +static DEFINE_SPINLOCK(ma35d1_lock); > + > +static const char *const ca35clk_sel_clks[] = { Use clk_parent_data instead. > + "hxt", "capll", "ddrpll", "dummy" > +}; > + [...] > + > +static inline struct clk_hw *ma35d1_clk_fixed(const char *name, int rate) > +{ > + return clk_hw_register_fixed_rate(NULL, name, NULL, 0, rate); > +} > + > +static inline struct clk_hw *ma35d1_clk_mux(const char *name, > + void __iomem *reg, > + u8 shift, u8 width, > + const char *const *parents, > + int num_parents) > +{ > + return clk_hw_register_mux(NULL, name, parents, num_parents, > + CLK_SET_RATE_NO_REPARENT, reg, shift, > + width, 0, &ma35d1_lock); > +} > + > +static inline struct clk_hw *ma35d1_clk_divider(const char *name, > + const char *parent, > + void __iomem *reg, u8 shift, > + u8 width) > +{ > + return clk_hw_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT, > + reg, shift, width, 0, &ma35d1_lock); > +} > + > +static inline struct clk_hw *ma35d1_clk_divider_pow2(const char *name, > + const char *parent, > + void __iomem *reg, > + u8 shift, u8 width) > +{ > + return clk_hw_register_divider(NULL, name, parent, > + CLK_DIVIDER_POWER_OF_TWO, reg, shift, > + width, 0, &ma35d1_lock); > +} > + > +static inline struct clk_hw *ma35d1_clk_divider_table(const char *name, > + const char *parent, > + void __iomem *reg, > + u8 shift, u8 width, > + const struct clk_div_table *table) > +{ > + return clk_hw_register_divider_table(NULL, name, parent, 0, > + reg, shift, width, 0, table, > + &ma35d1_lock); > +} > + > +static inline struct clk_hw *ma35d1_clk_fixed_factor(const char *name, > + const char *parent, > + unsigned int mult, > + unsigned int div) > +{ > + return clk_hw_register_fixed_factor(NULL, name, parent, > + CLK_SET_RATE_PARENT, mult, div); > +} > + > +static inline struct clk_hw *ma35d1_clk_gate(const char *name, > + const char *parent, > + void __iomem *reg, u8 shift) > +{ > + return clk_hw_register_gate(NULL, name, parent, CLK_SET_RATE_PARENT, > + reg, shift, 0, &ma35d1_lock); > +} These need to use devm_ to unregister on failure. > + > +static int ma35d1_get_pll_setting(struct device_node *clk_node, > + u32 *pllmode, u32 *pllfreq) > +{ > + const char *of_str; > + int i; > + > + for (i = 0; i < PLL_MAX_NUM; i++) { > + if (of_property_read_string_index(clk_node, "nuvoton,pll-mode", i, &of_str)) > + return -EINVAL; > + if (!strcmp(of_str, "integer")) > + pllmode[i] = PLL_MODE_INT; > + else if (!strcmp(of_str, "fractional")) > + pllmode[i] = PLL_MODE_FRAC; > + else if (!strcmp(of_str, "spread-spectrum")) > + pllmode[i] = PLL_MODE_SS; > + else > + return -EINVAL; > + } > + return of_property_read_u32_array(clk_node, "assigned-clock-rates", The clk framework looks at this property. Why is the driver looking at it too? > + pllfreq, PLL_MAX_NUM); > +} > +