Quoting Yinbo Zhu (2023-03-18 00:53:40) > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1eef05bb1f99..c0f32d9c1cc4 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -325,6 +325,15 @@ config COMMON_CLK_LOCHNAGAR > This driver supports the clocking features of the Cirrus Logic > Lochnagar audio development board. > > +config COMMON_CLK_LOONGSON2 > + bool "Clock driver for Loongson-2 SoC" > + depends on COMMON_CLK && OF It doesn't depend on OF to build, right? If so, remove it. Also, this is within the 'if COMMON_CLK' section of this file, so the 'depends on COMMON_CLK' is redundant and should be removed. > + help > + This driver provides support for clock controller on Loongson-2 SoC. > + The clock controller can generates and supplies clock to various > + peripherals within the SoC. > + Say Y here to support Loongson-2 SoC clock driver. > + > config COMMON_CLK_NXP > def_bool COMMON_CLK && (ARCH_LPC18XX || ARCH_LPC32XX) > select REGMAP_MMIO if ARCH_LPC32XX > diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c > new file mode 100644 > index 000000000000..c423932b626d > --- /dev/null > +++ b/drivers/clk/clk-loongson2.c > @@ -0,0 +1,356 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Author: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> > + * Copyright (C) 2022-2023 Loongson Technology Corporation Limited > + */ > + > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/of.h> Don't think this include will be needed. > +#include <linux/of_address.h> Don't include this. > +#include <linux/clk-provider.h> > +#include <linux/slab.h> > +#include <linux/clk.h> Drop this include. This isn't a clk consumer. > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/io-64-nonatomic-lo-hi.h> > +#include <dt-bindings/clock/loongson,ls2k-clk.h> > + > +#define LOONGSON2_PLL_MULT_SHIFT 32 > +#define LOONGSON2_PLL_MULT_WIDTH 10 > +#define LOONGSON2_PLL_DIV_SHIFT 26 > +#define LOONGSON2_PLL_DIV_WIDTH 6 > +#define LOONGSON2_APB_FREQSCALE_SHIFT 20 > +#define LOONGSON2_APB_FREQSCALE_WIDTH 3 > +#define LOONGSON2_USB_FREQSCALE_SHIFT 16 > +#define LOONGSON2_USB_FREQSCALE_WIDTH 3 > +#define LOONGSON2_SATA_FREQSCALE_SHIFT 12 > +#define LOONGSON2_SATA_FREQSCALE_WIDTH 3 > +#define LOONGSON2_BOOT_FREQSCALE_SHIFT 8 > +#define LOONGSON2_BOOT_FREQSCALE_WIDTH 3 > + > +static void __iomem *loongson2_pll_base; > + > +static const struct clk_parent_data pdata[] = { > + { .fw_name = "ref_100m", .name = "ref_clk", }, Are you mainintain backwards compatibility? If not, which I believe is the case, drop .name assignment. > +}; > + > +static struct clk_hw *loongson2_clk_register(struct device_node *np, Take a struct device instead. > + const char *name, > + const char *parent_name, > + const struct clk_ops *ops, > + unsigned long flags) > +{ > + int ret; > + struct clk_hw *hw; > + struct clk_init_data init; > + > + /* allocate the divider */ > + hw = kzalloc(sizeof(*hw), GFP_KERNEL); > + if (!hw) > + return ERR_PTR(-ENOMEM); > + > + init.name = name; > + init.ops = ops; > + init.flags = flags; > + init.num_parents = 1; > + > + if (!parent_name) > + init.parent_data = pdata; > + else > + init.parent_names = &parent_name; > + > + hw->init = &init; > + > + /* register the clock */ > + ret = of_clk_hw_register(np, hw); Use devm_clk_hw_register() > + if (ret) { > + kfree(hw); > + hw = ERR_PTR(ret); > + } > + > + return hw; > +} > + > +static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate) > +{ > + u64 val; > + u32 mult = 1, div = 1; Why are these initialized? > + > + val = readq(loongson2_pll_base + offset); > + > + mult = (val >> LOONGSON2_PLL_MULT_SHIFT) & > + clk_div_mask(LOONGSON2_PLL_MULT_WIDTH); > + div = (val >> LOONGSON2_PLL_DIV_SHIFT) & > + clk_div_mask(LOONGSON2_PLL_DIV_WIDTH); They're overwritten here. > + > + return div_u64((u64)rate * mult, div); > +} > + > +static unsigned long loongson2_node_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return loongson2_calc_pll_rate(0x0, parent_rate); > +} [...] > + > +static inline void loongson2_check_clk_hws(struct clk_hw *clks[], unsigned int count) > +{ > + unsigned int i; > + > + for (i = 0; i < count; i++) > + if (IS_ERR(clks[i])) > + pr_err("Loongson2 clk %u: register failed with %ld\n", > + i, PTR_ERR(clks[i])); > +} > + > +static void loongson2_clocks_init(struct device_node *np) Inline this function at the caller. > +{ > + struct clk_hw **hws; > + struct clk_hw_onecell_data *clk_hw_data; > + spinlock_t loongson2_clk_lock; > + > + loongson2_pll_base = of_iomap(np, 0); Use platform device APIs. > + > + if (!loongson2_pll_base) { > + pr_err("clk: unable to map loongson2 clk registers\n"); Drop error messages when mapping. > + return; > + } > + > + clk_hw_data = kzalloc(struct_size(clk_hw_data, hws, LOONGSON2_CLK_END), Use devm_kzalloc() > + GFP_KERNEL); > + if (WARN_ON(!clk_hw_data)) > + goto err; > + [...] > + > +static int loongson2_clk_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + > + loongson2_clocks_init(np); > + > + return 0; > +} > +