Quoting Jacky Huang (2023-04-11 22:38:21) > diff --git a/drivers/clk/nuvoton/clk-ma35d1-divider.c b/drivers/clk/nuvoton/clk-ma35d1-divider.c > new file mode 100644 > index 000000000000..8d573ba3dfd3 > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1-divider.c > @@ -0,0 +1,134 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 Nuvoton Technology Corp. [...] > +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; > + 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; Can you use parent_data instead? > + 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) > + 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..6de67c964a2d > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c > @@ -0,0 +1,315 @@ > +// 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/container_of.h> > +#include <linux/device.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/slab.h> > + > +#include "clk-ma35d1.h" > + > +struct ma35d1_clk_pll { > + struct clk_hw hw; > + u8 type; > + u8 mode; > + void __iomem *ctl0_base; > + void __iomem *ctl1_base; > + void __iomem *ctl2_base; > +}; > + [..] > +struct clk_hw *ma35d1_reg_clk_pll(enum ma35d1_pll_type type, > + struct device *dev, > + u8 u8mode, const char *name, > + const char *parent, > + void __iomem *base) > +{ > + struct ma35d1_clk_pll *pll; > + struct clk_hw *hw; > + struct clk_init_data init = {}; > + int ret; > + > + pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL); > + if (!pll) > + return ERR_PTR(-ENOMEM); > + > + pll->type = type; > + pll->mode = u8mode; > + pll->ctl0_base = base + REG_PLL_CTL0_OFFSET; > + pll->ctl1_base = base + REG_PLL_CTL1_OFFSET; > + pll->ctl2_base = base + REG_PLL_CTL2_OFFSET; > + > + init.name = name; > + init.flags = 0; > + init.parent_names = &parent; Can you use parent_data instead? > + init.num_parents = 1; > + > + if (type == MA35D1_CAPLL || type == MA35D1_DDRPLL) > + init.ops = &ma35d1_clk_fixed_pll_ops; > + else > + init.ops = &ma35d1_clk_pll_ops; > + > + pll->hw.init = &init; > + hw = &pll->hw; > + > + ret = devm_clk_hw_register(dev, hw); > + if (ret) > + 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..066e1c6f2d35 > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1.c > @@ -0,0 +1,897 @@ > +// 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 struct clk_parent_data ca35clk_sel_clks[] = { > + { .fw_name = "hxt", .name = "hxt" }, > + { .fw_name = "capll", .name = "capll" }, > + { .fw_name = "ddrpll", .name = "ddrpll" }, > + { .fw_name = "dummy", .name = "dummy" } What is 'dummy'? Is that in the binding? Note, don't put both .fw_name and .name in the binding. For new drivers, prefer to use .index or .hw and never use .name to describe parents. > +}; > + > +static const char *const sysclk0_sel_clks[] = { > + "epll_div2", "syspll" [...] > diff --git a/drivers/clk/nuvoton/clk-ma35d1.h b/drivers/clk/nuvoton/clk-ma35d1.h > new file mode 100644 > index 000000000000..28c60f081788 > --- /dev/null > +++ b/drivers/clk/nuvoton/clk-ma35d1.h > @@ -0,0 +1,123 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2023 Nuvoton Technology Corp. > + * Author: Chi-Fang Li <cfli0@xxxxxxxxxxx> > + */ > + > +#ifndef __DRV_CLK_NUVOTON_MA35D1_H > +#define __DRV_CLK_NUVOTON_MA35D1_H Is this header included in one C file? If so, remove the header file and put the contents in the C file.