Quoting Drew Fustini (2024-06-23 19:12:32) > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c > new file mode 100644 > index 000000000000..982d4d40f783 > --- /dev/null > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -0,0 +1,1086 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2023 Jisheng Zhang <jszhang@xxxxxxxxxx> > + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. > + * Authors: Yangtao Li <frank.li@xxxxxxxx> > + */ > + > +#include <dt-bindings/clock/thead,th1520-clk-ap.h> Preferably include dt-bindings after linux includes. > +#include <linux/bitfield.h> > +#include <linux/clk-provider.h> > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#define TH1520_PLL_POSTDIV2 GENMASK(26, 24) > +#define TH1520_PLL_POSTDIV1 GENMASK(22, 20) > +#define TH1520_PLL_FBDIV GENMASK(19, 8) > +#define TH1520_PLL_REFDIV GENMASK(5, 0) > +#define TH1520_PLL_BYPASS BIT(30) > +#define TH1520_PLL_DSMPD BIT(24) > +#define TH1520_PLL_FRAC GENMASK(23, 0) > +#define TH1520_PLL_FRAC_BITS 24 [...] > + > +static unsigned long th1520_pll_vco_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct ccu_pll *pll = hw_to_ccu_pll(hw); > + unsigned long div, mul, frac, rate = parent_rate; > + unsigned int cfg0, cfg1; > + > + regmap_read(pll->common.map, pll->common.cfg0, &cfg0); > + regmap_read(pll->common.map, pll->common.cfg1, &cfg1); > + > + mul = FIELD_GET(TH1520_PLL_FBDIV, cfg0); > + div = FIELD_GET(TH1520_PLL_REFDIV, cfg0); > + if (!(cfg1 & TH1520_PLL_DSMPD)) { > + mul <<= TH1520_PLL_FRAC_BITS; > + frac = FIELD_GET(TH1520_PLL_FRAC, cfg1); > + mul += frac; > + div <<= TH1520_PLL_FRAC_BITS; > + } > + rate = parent_rate * mul; > + do_div(rate, div); 'rate' is only unsigned long, so do_div() isn't needed here. Perhaps if 'parent_rate * mul' can overflow 32-bits then 'rate' should be u64. > + return rate; > +} > + > +static unsigned long th1520_pll_postdiv_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct ccu_pll *pll = hw_to_ccu_pll(hw); > + unsigned long rate = parent_rate; > + unsigned int cfg0, cfg1; > + > + regmap_read(pll->common.map, pll->common.cfg0, &cfg0); > + regmap_read(pll->common.map, pll->common.cfg1, &cfg1); > + > + if (cfg1 & TH1520_PLL_BYPASS) > + return rate; > + > + do_div(rate, FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * Same, 'rate' is unsigned long. Did you get some compilation error without this? How big is the divisor going to be? The fields are only 3-bits wide, so the multiplication would fit into a u32 just fine. Given that 'rate' is unsigned long though I think you can just put the multiplication result into a local variable that's also unsigned long and then just write the divide with unsigned longs div = FIELD_GET(...) * FIELD_GET(...); return rate / div; > + FIELD_GET(TH1520_PLL_POSTDIV2, cfg0)); > + > + return rate; > +} > + > +static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + unsigned long rate = parent_rate; > + > + rate = th1520_pll_vco_recalc_rate(hw, rate); > + rate = th1520_pll_postdiv_recalc_rate(hw, rate); > + > + return rate; > +} Please fold this in ----8<--- diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 982d4d40f783..0c3821d50765 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -411,7 +411,7 @@ static const struct clk_parent_data c910_i0_parents[] = { { .index = 0 } }; -struct ccu_mux c910_i0_clk = { +static struct ccu_mux c910_i0_clk = { .mux = TH_CCU_ARG(1, 1), .common = { .clkid = CLK_C910_I0, @@ -428,7 +428,7 @@ static const struct clk_parent_data c910_parents[] = { { .hw = &cpu_pll1_clk.common.hw } }; -struct ccu_mux c910_clk = { +static struct ccu_mux c910_clk = { .mux = TH_CCU_ARG(0, 1), .common = { .clkid = CLK_C910, @@ -845,7 +845,7 @@ static const struct clk_parent_data uart_sclk_parents[] = { { .index = 0 }, }; -struct ccu_mux uart_sclk = { +static struct ccu_mux uart_sclk = { .mux = TH_CCU_ARG(0, 1), .common = { .clkid = CLK_UART_SCLK,