| Message ID | 20241203134137.2114847-2-m.wilczynski@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,v1,01/14] clk: thead: Refactor TH1520 clock driver to share common code | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
Quoting Michal Wilczynski (2024-12-03 05:41:24) > diff --git a/drivers/clk/thead/Makefile b/drivers/clk/thead/Makefile > index 7ee0bec1f251..d7cf88390b69 100644 > --- a/drivers/clk/thead/Makefile > +++ b/drivers/clk/thead/Makefile > @@ -1,2 +1,2 @@ > # SPDX-License-Identifier: GPL-2.0 > -obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520-ap.o > +obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520.o clk-th1520-ap.o Can the -ap driver be extended instead? Or are the clks in a different IO region? > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c > index 17e32ae08720..a6015805b859 100644 > --- a/drivers/clk/thead/clk-th1520-ap.c > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -5,297 +5,9 @@ > * Authors: Yangtao Li <frank.li@vivo.com> > */ > > -#include <dt-bindings/clock/thead,th1520-clk-ap.h> Presumably this should stay here. > -#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> These should all stay here as well. > - > -#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 > - > -struct ccu_internal { > - u8 shift; > - u8 width; > -}; > - > -struct ccu_div_internal { > - u8 shift; > - u8 width; > - u32 flags; > -}; > - > -struct ccu_common { > - int clkid; > - struct regmap *map; > - u16 cfg0; > - u16 cfg1; > - struct clk_hw hw; > -}; > - > -struct ccu_mux { > - struct ccu_internal mux; > - struct ccu_common common; > -}; > - > -struct ccu_gate { > - u32 enable; > - struct ccu_common common; > -}; > - > -struct ccu_div { > - u32 enable; > - struct ccu_div_internal div; > - struct ccu_internal mux; > - struct ccu_common common; > -}; > - > -struct ccu_pll { > - struct ccu_common common; > -}; > - > -#define TH_CCU_ARG(_shift, _width) \ > - { \ > - .shift = _shift, \ > - .width = _width, \ > - } > - > -#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ > - { \ > - .shift = _shift, \ > - .width = _width, \ > - .flags = _flags, \ > - } > - > -#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ > - struct ccu_gate _struct = { \ > - .enable = _gate, \ > - .common = { \ > - .clkid = _clkid, \ > - .cfg0 = _reg, \ > - .hw.init = CLK_HW_INIT_PARENTS_DATA( \ > - _name, \ > - _parent, \ > - &clk_gate_ops, \ > - _flags), \ > - } \ > - } > - > -static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) > -{ > - return container_of(hw, struct ccu_common, hw); > -} > - > -static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) > -{ > - struct ccu_common *common = hw_to_ccu_common(hw); > - > - return container_of(common, struct ccu_mux, common); > -} > - > -static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) > -{ > - struct ccu_common *common = hw_to_ccu_common(hw); > +#include "clk-th1520.h" > > - return container_of(common, struct ccu_pll, common); > -} > - > -static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) > -{ > - struct ccu_common *common = hw_to_ccu_common(hw); > - > - return container_of(common, struct ccu_div, common); > -} > - > -static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) > -{ > - struct ccu_common *common = hw_to_ccu_common(hw); > - > - return container_of(common, struct ccu_gate, common); > -} > - > -static u8 ccu_get_parent_helper(struct ccu_common *common, > - struct ccu_internal *mux) > -{ > - unsigned int val; > - u8 parent; > - > - regmap_read(common->map, common->cfg0, &val); > - parent = val >> mux->shift; > - parent &= GENMASK(mux->width - 1, 0); > - > - return parent; > -} > - > -static int ccu_set_parent_helper(struct ccu_common *common, > - struct ccu_internal *mux, > - u8 index) > -{ > - return regmap_update_bits(common->map, common->cfg0, > - GENMASK(mux->width - 1, 0) << mux->shift, > - index << mux->shift); > -} > - > -static void ccu_disable_helper(struct ccu_common *common, u32 gate) > -{ > - if (!gate) > - return; > - regmap_update_bits(common->map, common->cfg0, > - gate, ~gate); > -} > - > -static int ccu_enable_helper(struct ccu_common *common, u32 gate) > -{ > - unsigned int val; > - int ret; > - > - if (!gate) > - return 0; > - > - ret = regmap_update_bits(common->map, common->cfg0, gate, gate); > - regmap_read(common->map, common->cfg0, &val); > - return ret; > -} > - > -static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) > -{ > - unsigned int val; > - > - if (!gate) > - return true; > - > - regmap_read(common->map, common->cfg0, &val); > - return val & gate; > -} > - > -static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, > - unsigned long parent_rate) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - unsigned long rate; > - unsigned int val; > - > - regmap_read(cd->common.map, cd->common.cfg0, &val); > - val = val >> cd->div.shift; > - val &= GENMASK(cd->div.width - 1, 0); > - rate = divider_recalc_rate(hw, parent_rate, val, NULL, > - cd->div.flags, cd->div.width); > - > - return rate; > -} > - > -static u8 ccu_div_get_parent(struct clk_hw *hw) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - > - return ccu_get_parent_helper(&cd->common, &cd->mux); > -} > - > -static int ccu_div_set_parent(struct clk_hw *hw, u8 index) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - > - return ccu_set_parent_helper(&cd->common, &cd->mux, index); > -} > - > -static void ccu_div_disable(struct clk_hw *hw) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - > - ccu_disable_helper(&cd->common, cd->enable); > -} > - > -static int ccu_div_enable(struct clk_hw *hw) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - > - return ccu_enable_helper(&cd->common, cd->enable); > -} > - > -static int ccu_div_is_enabled(struct clk_hw *hw) > -{ > - struct ccu_div *cd = hw_to_ccu_div(hw); > - > - return ccu_is_enabled_helper(&cd->common, cd->enable); > -} > - > -static const struct clk_ops ccu_div_ops = { > - .disable = ccu_div_disable, > - .enable = ccu_div_enable, > - .is_enabled = ccu_div_is_enabled, > - .get_parent = ccu_div_get_parent, > - .set_parent = ccu_div_set_parent, > - .recalc_rate = ccu_div_recalc_rate, > - .determine_rate = clk_hw_determine_rate_no_reparent, > -}; > - > -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; > - unsigned int cfg0, cfg1; > - u64 rate = parent_rate; > - > - 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; > - rate = rate / div; > - 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 div, 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; > - > - div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * > - FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); > - > - rate = rate / div; > - > - 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; > -} > - > -static const struct clk_ops clk_pll_ops = { > - .recalc_rate = ccu_pll_recalc_rate, > -}; > +#define NR_CLKS (CLK_UART_SCLK + 1) > > static const struct clk_parent_data osc_24m_clk[] = { > { .index = 0 } > @@ -956,15 +668,6 @@ static struct ccu_common *th1520_gate_clks[] = { > &sram3_clk.common, > }; > > -#define NR_CLKS (CLK_UART_SCLK + 1) Why did this move? > - > -static const struct regmap_config th1520_clk_regmap_config = { > - .reg_bits = 32, > - .val_bits = 32, > - .reg_stride = 4, > - .fast_io = true, > -}; > - > static int th1520_clk_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > diff --git a/drivers/clk/thead/clk-th1520.c b/drivers/clk/thead/clk-th1520.c > new file mode 100644 > index 000000000000..e2bfe56de9af > --- /dev/null > +++ b/drivers/clk/thead/clk-th1520.c > @@ -0,0 +1,188 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. > + * Authors: Yangtao Li <frank.li@vivo.com> > + */ > + > +#include "clk-th1520.h" Explicitly include linux headers here, don't just put them all into a header file. It helps us easily see what C files are using different parts of the kernel infrastructure. > + > +static u8 ccu_get_parent_helper(struct ccu_common *common, > + struct ccu_internal *mux) > +{ > + unsigned int val; > + u8 parent; > + > + regmap_read(common->map, common->cfg0, &val); > + parent = val >> mux->shift; > + parent &= GENMASK(mux->width - 1, 0); > + > + return parent; > +} > + > +static int ccu_set_parent_helper(struct ccu_common *common, > + struct ccu_internal *mux, u8 index) > +{ > + return regmap_update_bits(common->map, common->cfg0, > + GENMASK(mux->width - 1, 0) << mux->shift, > + index << mux->shift); > +} > + > +static void ccu_disable_helper(struct ccu_common *common, u32 gate) > +{ > + if (!gate) > + return; > + regmap_update_bits(common->map, common->cfg0, gate, ~gate); > +} > + > +static int ccu_enable_helper(struct ccu_common *common, u32 gate) > +{ > + unsigned int val; > + int ret; > + > + if (!gate) > + return 0; > + > + ret = regmap_update_bits(common->map, common->cfg0, gate, gate); > + regmap_read(common->map, common->cfg0, &val); > + return ret; > +} > + > +static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) > +{ > + unsigned int val; > + > + if (!gate) > + return true; > + > + regmap_read(common->map, common->cfg0, &val); > + return val & gate; > +} > + > +static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + unsigned long rate; > + unsigned int val; > + > + regmap_read(cd->common.map, cd->common.cfg0, &val); > + val = val >> cd->div.shift; > + val &= GENMASK(cd->div.width - 1, 0); > + rate = divider_recalc_rate(hw, parent_rate, val, NULL, cd->div.flags, > + cd->div.width); > + > + return rate; > +} > + > +static u8 ccu_div_get_parent(struct clk_hw *hw) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + > + return ccu_get_parent_helper(&cd->common, &cd->mux); > +} > + > +static int ccu_div_set_parent(struct clk_hw *hw, u8 index) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + > + return ccu_set_parent_helper(&cd->common, &cd->mux, index); > +} > + > +static void ccu_div_disable(struct clk_hw *hw) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + > + ccu_disable_helper(&cd->common, cd->enable); > +} > + > +static int ccu_div_enable(struct clk_hw *hw) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + > + return ccu_enable_helper(&cd->common, cd->enable); > +} > + > +static int ccu_div_is_enabled(struct clk_hw *hw) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + > + return ccu_is_enabled_helper(&cd->common, cd->enable); > +} > + > +const struct clk_ops ccu_div_ops = { > + .disable = ccu_div_disable, > + .enable = ccu_div_enable, > + .is_enabled = ccu_div_is_enabled, > + .get_parent = ccu_div_get_parent, > + .set_parent = ccu_div_set_parent, > + .recalc_rate = ccu_div_recalc_rate, > + .determine_rate = clk_hw_determine_rate_no_reparent, > +}; > + > +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; > + unsigned int cfg0, cfg1; > + u64 rate = parent_rate; > + > + 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; > + rate = rate / div; > + 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 div, 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; > + > + div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * > + FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); > + > + rate = rate / div; > + > + 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; > +} > + > +const struct clk_ops clk_pll_ops = { > + .recalc_rate = ccu_pll_recalc_rate, > +}; > + > +const struct regmap_config th1520_clk_regmap_config = { I don't get why this is moved. > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, > +}; > diff --git a/drivers/clk/thead/clk-th1520.h b/drivers/clk/thead/clk-th1520.h > new file mode 100644 > index 000000000000..285d41e65008 > --- /dev/null > +++ b/drivers/clk/thead/clk-th1520.h > @@ -0,0 +1,134 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. > + * Authors: Yangtao Li <frank.li@vivo.com> > + * > + * clk-th1520.h - Common definitions for T-HEAD TH1520 Clock Drivers > + */ > + > +#ifndef CLK_TH1520_H > +#define CLK_TH1520_H > + > +#include <dt-bindings/clock/thead,th1520-clk-ap.h> dt-bindings comes after linux includes, but this shouldn't be here anyway. > +#include <linux/bitfield.h> > +#include <linux/clk-provider.h> Seems we have to have this one for clk_hw. > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> I don't see these includes used here, so remove them. > +#include <linux/regmap.h> Forward declare regmap and drop the include struct regmap; > + > +#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 Are these going to be used in multiple drivers? > + > +struct ccu_internal { > + u8 shift; > + u8 width; > +}; > + > +struct ccu_div_internal { > + u8 shift; > + u8 width; > + u32 flags; > +}; > + > +struct ccu_common { > + int clkid; > + struct regmap *map; > + u16 cfg0; > + u16 cfg1; > + struct clk_hw hw; > +}; > + > +struct ccu_mux { > + struct ccu_internal mux; > + struct ccu_common common; > +}; > + > +struct ccu_gate { > + u32 enable; > + struct ccu_common common; > +}; > + > +struct ccu_div { > + u32 enable; > + struct ccu_div_internal div; > + struct ccu_internal mux; > + struct ccu_common common; > +}; > + > +struct ccu_pll { > + struct ccu_common common; > +}; > + > +#define TH_CCU_ARG(_shift, _width) \ > + { \ > + .shift = _shift, \ > + .width = _width, \ > + } > + > +#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ > + { \ > + .shift = _shift, \ > + .width = _width, \ > + .flags = _flags, \ > + } > + > +#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ > + struct ccu_gate _struct = { \ > + .enable = _gate, \ > + .common = { \ > + .clkid = _clkid, \ > + .cfg0 = _reg, \ > + .hw.init = CLK_HW_INIT_PARENTS_DATA( \ > + _name, \ > + _parent, \ > + &clk_gate_ops, \ > + _flags), \ > + } \ > + } > + > +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) > +{ > + return container_of(hw, struct ccu_common, hw); > +} > + > +static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) > +{ > + struct ccu_common *common = hw_to_ccu_common(hw); > + > + return container_of(common, struct ccu_mux, common); > +} > + > +static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) > +{ > + struct ccu_common *common = hw_to_ccu_common(hw); > + > + return container_of(common, struct ccu_pll, common); > +} > + > +static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) > +{ > + struct ccu_common *common = hw_to_ccu_common(hw); > + > + return container_of(common, struct ccu_div, common); > +} > + > +static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) > +{ > + struct ccu_common *common = hw_to_ccu_common(hw); > + > + return container_of(common, struct ccu_gate, common); > +} > + > +extern const struct clk_ops ccu_div_ops; > +extern const struct clk_ops clk_pll_ops; > +extern const struct regmap_config th1520_clk_regmap_config; Why is the regmap config exported?
On 12/3/24 20:56, Stephen Boyd wrote: > Quoting Michal Wilczynski (2024-12-03 05:41:24) >> diff --git a/drivers/clk/thead/Makefile b/drivers/clk/thead/Makefile >> index 7ee0bec1f251..d7cf88390b69 100644 >> --- a/drivers/clk/thead/Makefile >> +++ b/drivers/clk/thead/Makefile >> @@ -1,2 +1,2 @@ >> # SPDX-License-Identifier: GPL-2.0 >> -obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520-ap.o >> +obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520.o clk-th1520-ap.o > > Can the -ap driver be extended instead? Or are the clks in a different > IO region? The Video Output (VO) clocks reside in a different address space as defined in the T-HEAD manual 4.4.1 [1]. Therefore, creating a separate driver made sense to maintain clarity and adhere to the existing convention of having one driver per subsystem, similar to the AP-specific driver. [1] - https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf > >> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c >> index 17e32ae08720..a6015805b859 100644 >> --- a/drivers/clk/thead/clk-th1520-ap.c >> +++ b/drivers/clk/thead/clk-th1520-ap.c >> @@ -5,297 +5,9 @@ >> * Authors: Yangtao Li <frank.li@vivo.com> >> */ >> >> -#include <dt-bindings/clock/thead,th1520-clk-ap.h> > > Presumably this should stay here. > >> -#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> > > These should all stay here as well. > >> - >> -#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 >> - >> -struct ccu_internal { >> - u8 shift; >> - u8 width; >> -}; >> - >> -struct ccu_div_internal { >> - u8 shift; >> - u8 width; >> - u32 flags; >> -}; >> - >> -struct ccu_common { >> - int clkid; >> - struct regmap *map; >> - u16 cfg0; >> - u16 cfg1; >> - struct clk_hw hw; >> -}; >> - >> -struct ccu_mux { >> - struct ccu_internal mux; >> - struct ccu_common common; >> -}; >> - >> -struct ccu_gate { >> - u32 enable; >> - struct ccu_common common; >> -}; >> - >> -struct ccu_div { >> - u32 enable; >> - struct ccu_div_internal div; >> - struct ccu_internal mux; >> - struct ccu_common common; >> -}; >> - >> -struct ccu_pll { >> - struct ccu_common common; >> -}; >> - >> -#define TH_CCU_ARG(_shift, _width) \ >> - { \ >> - .shift = _shift, \ >> - .width = _width, \ >> - } >> - >> -#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ >> - { \ >> - .shift = _shift, \ >> - .width = _width, \ >> - .flags = _flags, \ >> - } >> - >> -#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ >> - struct ccu_gate _struct = { \ >> - .enable = _gate, \ >> - .common = { \ >> - .clkid = _clkid, \ >> - .cfg0 = _reg, \ >> - .hw.init = CLK_HW_INIT_PARENTS_DATA( \ >> - _name, \ >> - _parent, \ >> - &clk_gate_ops, \ >> - _flags), \ >> - } \ >> - } >> - >> -static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) >> -{ >> - return container_of(hw, struct ccu_common, hw); >> -} >> - >> -static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) >> -{ >> - struct ccu_common *common = hw_to_ccu_common(hw); >> - >> - return container_of(common, struct ccu_mux, common); >> -} >> - >> -static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) >> -{ >> - struct ccu_common *common = hw_to_ccu_common(hw); >> +#include "clk-th1520.h" >> >> - return container_of(common, struct ccu_pll, common); >> -} >> - >> -static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) >> -{ >> - struct ccu_common *common = hw_to_ccu_common(hw); >> - >> - return container_of(common, struct ccu_div, common); >> -} >> - >> -static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) >> -{ >> - struct ccu_common *common = hw_to_ccu_common(hw); >> - >> - return container_of(common, struct ccu_gate, common); >> -} >> - >> -static u8 ccu_get_parent_helper(struct ccu_common *common, >> - struct ccu_internal *mux) >> -{ >> - unsigned int val; >> - u8 parent; >> - >> - regmap_read(common->map, common->cfg0, &val); >> - parent = val >> mux->shift; >> - parent &= GENMASK(mux->width - 1, 0); >> - >> - return parent; >> -} >> - >> -static int ccu_set_parent_helper(struct ccu_common *common, >> - struct ccu_internal *mux, >> - u8 index) >> -{ >> - return regmap_update_bits(common->map, common->cfg0, >> - GENMASK(mux->width - 1, 0) << mux->shift, >> - index << mux->shift); >> -} >> - >> -static void ccu_disable_helper(struct ccu_common *common, u32 gate) >> -{ >> - if (!gate) >> - return; >> - regmap_update_bits(common->map, common->cfg0, >> - gate, ~gate); >> -} >> - >> -static int ccu_enable_helper(struct ccu_common *common, u32 gate) >> -{ >> - unsigned int val; >> - int ret; >> - >> - if (!gate) >> - return 0; >> - >> - ret = regmap_update_bits(common->map, common->cfg0, gate, gate); >> - regmap_read(common->map, common->cfg0, &val); >> - return ret; >> -} >> - >> -static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) >> -{ >> - unsigned int val; >> - >> - if (!gate) >> - return true; >> - >> - regmap_read(common->map, common->cfg0, &val); >> - return val & gate; >> -} >> - >> -static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, >> - unsigned long parent_rate) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - unsigned long rate; >> - unsigned int val; >> - >> - regmap_read(cd->common.map, cd->common.cfg0, &val); >> - val = val >> cd->div.shift; >> - val &= GENMASK(cd->div.width - 1, 0); >> - rate = divider_recalc_rate(hw, parent_rate, val, NULL, >> - cd->div.flags, cd->div.width); >> - >> - return rate; >> -} >> - >> -static u8 ccu_div_get_parent(struct clk_hw *hw) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - >> - return ccu_get_parent_helper(&cd->common, &cd->mux); >> -} >> - >> -static int ccu_div_set_parent(struct clk_hw *hw, u8 index) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - >> - return ccu_set_parent_helper(&cd->common, &cd->mux, index); >> -} >> - >> -static void ccu_div_disable(struct clk_hw *hw) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - >> - ccu_disable_helper(&cd->common, cd->enable); >> -} >> - >> -static int ccu_div_enable(struct clk_hw *hw) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - >> - return ccu_enable_helper(&cd->common, cd->enable); >> -} >> - >> -static int ccu_div_is_enabled(struct clk_hw *hw) >> -{ >> - struct ccu_div *cd = hw_to_ccu_div(hw); >> - >> - return ccu_is_enabled_helper(&cd->common, cd->enable); >> -} >> - >> -static const struct clk_ops ccu_div_ops = { >> - .disable = ccu_div_disable, >> - .enable = ccu_div_enable, >> - .is_enabled = ccu_div_is_enabled, >> - .get_parent = ccu_div_get_parent, >> - .set_parent = ccu_div_set_parent, >> - .recalc_rate = ccu_div_recalc_rate, >> - .determine_rate = clk_hw_determine_rate_no_reparent, >> -}; >> - >> -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; >> - unsigned int cfg0, cfg1; >> - u64 rate = parent_rate; >> - >> - 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; >> - rate = rate / div; >> - 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 div, 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; >> - >> - div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * >> - FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); >> - >> - rate = rate / div; >> - >> - 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; >> -} >> - >> -static const struct clk_ops clk_pll_ops = { >> - .recalc_rate = ccu_pll_recalc_rate, >> -}; >> +#define NR_CLKS (CLK_UART_SCLK + 1) >> >> static const struct clk_parent_data osc_24m_clk[] = { >> { .index = 0 } >> @@ -956,15 +668,6 @@ static struct ccu_common *th1520_gate_clks[] = { >> &sram3_clk.common, >> }; >> >> -#define NR_CLKS (CLK_UART_SCLK + 1) > > Why did this move? > >> - >> -static const struct regmap_config th1520_clk_regmap_config = { >> - .reg_bits = 32, >> - .val_bits = 32, >> - .reg_stride = 4, >> - .fast_io = true, >> -}; >> - >> static int th1520_clk_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> diff --git a/drivers/clk/thead/clk-th1520.c b/drivers/clk/thead/clk-th1520.c >> new file mode 100644 >> index 000000000000..e2bfe56de9af >> --- /dev/null >> +++ b/drivers/clk/thead/clk-th1520.c >> @@ -0,0 +1,188 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> >> + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. >> + * Authors: Yangtao Li <frank.li@vivo.com> >> + */ >> + >> +#include "clk-th1520.h" > > Explicitly include linux headers here, don't just put them all into a > header file. It helps us easily see what C files are using different > parts of the kernel infrastructure. > >> + >> +static u8 ccu_get_parent_helper(struct ccu_common *common, >> + struct ccu_internal *mux) >> +{ >> + unsigned int val; >> + u8 parent; >> + >> + regmap_read(common->map, common->cfg0, &val); >> + parent = val >> mux->shift; >> + parent &= GENMASK(mux->width - 1, 0); >> + >> + return parent; >> +} >> + >> +static int ccu_set_parent_helper(struct ccu_common *common, >> + struct ccu_internal *mux, u8 index) >> +{ >> + return regmap_update_bits(common->map, common->cfg0, >> + GENMASK(mux->width - 1, 0) << mux->shift, >> + index << mux->shift); >> +} >> + >> +static void ccu_disable_helper(struct ccu_common *common, u32 gate) >> +{ >> + if (!gate) >> + return; >> + regmap_update_bits(common->map, common->cfg0, gate, ~gate); >> +} >> + >> +static int ccu_enable_helper(struct ccu_common *common, u32 gate) >> +{ >> + unsigned int val; >> + int ret; >> + >> + if (!gate) >> + return 0; >> + >> + ret = regmap_update_bits(common->map, common->cfg0, gate, gate); >> + regmap_read(common->map, common->cfg0, &val); >> + return ret; >> +} >> + >> +static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) >> +{ >> + unsigned int val; >> + >> + if (!gate) >> + return true; >> + >> + regmap_read(common->map, common->cfg0, &val); >> + return val & gate; >> +} >> + >> +static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, >> + unsigned long parent_rate) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + unsigned long rate; >> + unsigned int val; >> + >> + regmap_read(cd->common.map, cd->common.cfg0, &val); >> + val = val >> cd->div.shift; >> + val &= GENMASK(cd->div.width - 1, 0); >> + rate = divider_recalc_rate(hw, parent_rate, val, NULL, cd->div.flags, >> + cd->div.width); >> + >> + return rate; >> +} >> + >> +static u8 ccu_div_get_parent(struct clk_hw *hw) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + >> + return ccu_get_parent_helper(&cd->common, &cd->mux); >> +} >> + >> +static int ccu_div_set_parent(struct clk_hw *hw, u8 index) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + >> + return ccu_set_parent_helper(&cd->common, &cd->mux, index); >> +} >> + >> +static void ccu_div_disable(struct clk_hw *hw) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + >> + ccu_disable_helper(&cd->common, cd->enable); >> +} >> + >> +static int ccu_div_enable(struct clk_hw *hw) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + >> + return ccu_enable_helper(&cd->common, cd->enable); >> +} >> + >> +static int ccu_div_is_enabled(struct clk_hw *hw) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + >> + return ccu_is_enabled_helper(&cd->common, cd->enable); >> +} >> + >> +const struct clk_ops ccu_div_ops = { >> + .disable = ccu_div_disable, >> + .enable = ccu_div_enable, >> + .is_enabled = ccu_div_is_enabled, >> + .get_parent = ccu_div_get_parent, >> + .set_parent = ccu_div_set_parent, >> + .recalc_rate = ccu_div_recalc_rate, >> + .determine_rate = clk_hw_determine_rate_no_reparent, >> +}; >> + >> +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; >> + unsigned int cfg0, cfg1; >> + u64 rate = parent_rate; >> + >> + 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; >> + rate = rate / div; >> + 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 div, 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; >> + >> + div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * >> + FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); >> + >> + rate = rate / div; >> + >> + 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; >> +} >> + >> +const struct clk_ops clk_pll_ops = { >> + .recalc_rate = ccu_pll_recalc_rate, >> +}; >> + >> +const struct regmap_config th1520_clk_regmap_config = { > > I don't get why this is moved. > >> + .reg_bits = 32, >> + .val_bits = 32, >> + .reg_stride = 4, >> + .fast_io = true, >> +}; >> diff --git a/drivers/clk/thead/clk-th1520.h b/drivers/clk/thead/clk-th1520.h >> new file mode 100644 >> index 000000000000..285d41e65008 >> --- /dev/null >> +++ b/drivers/clk/thead/clk-th1520.h >> @@ -0,0 +1,134 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> >> + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. >> + * Authors: Yangtao Li <frank.li@vivo.com> >> + * >> + * clk-th1520.h - Common definitions for T-HEAD TH1520 Clock Drivers >> + */ >> + >> +#ifndef CLK_TH1520_H >> +#define CLK_TH1520_H >> + >> +#include <dt-bindings/clock/thead,th1520-clk-ap.h> > > dt-bindings comes after linux includes, but this shouldn't be here > anyway. > >> +#include <linux/bitfield.h> >> +#include <linux/clk-provider.h> > > Seems we have to have this one for clk_hw. > >> +#include <linux/device.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> > > I don't see these includes used here, so remove them. > >> +#include <linux/regmap.h> > > Forward declare regmap and drop the include > > struct regmap; > >> + >> +#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 > > Are these going to be used in multiple drivers? > >> + >> +struct ccu_internal { >> + u8 shift; >> + u8 width; >> +}; >> + >> +struct ccu_div_internal { >> + u8 shift; >> + u8 width; >> + u32 flags; >> +}; >> + >> +struct ccu_common { >> + int clkid; >> + struct regmap *map; >> + u16 cfg0; >> + u16 cfg1; >> + struct clk_hw hw; >> +}; >> + >> +struct ccu_mux { >> + struct ccu_internal mux; >> + struct ccu_common common; >> +}; >> + >> +struct ccu_gate { >> + u32 enable; >> + struct ccu_common common; >> +}; >> + >> +struct ccu_div { >> + u32 enable; >> + struct ccu_div_internal div; >> + struct ccu_internal mux; >> + struct ccu_common common; >> +}; >> + >> +struct ccu_pll { >> + struct ccu_common common; >> +}; >> + >> +#define TH_CCU_ARG(_shift, _width) \ >> + { \ >> + .shift = _shift, \ >> + .width = _width, \ >> + } >> + >> +#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ >> + { \ >> + .shift = _shift, \ >> + .width = _width, \ >> + .flags = _flags, \ >> + } >> + >> +#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ >> + struct ccu_gate _struct = { \ >> + .enable = _gate, \ >> + .common = { \ >> + .clkid = _clkid, \ >> + .cfg0 = _reg, \ >> + .hw.init = CLK_HW_INIT_PARENTS_DATA( \ >> + _name, \ >> + _parent, \ >> + &clk_gate_ops, \ >> + _flags), \ >> + } \ >> + } >> + >> +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) >> +{ >> + return container_of(hw, struct ccu_common, hw); >> +} >> + >> +static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) >> +{ >> + struct ccu_common *common = hw_to_ccu_common(hw); >> + >> + return container_of(common, struct ccu_mux, common); >> +} >> + >> +static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) >> +{ >> + struct ccu_common *common = hw_to_ccu_common(hw); >> + >> + return container_of(common, struct ccu_pll, common); >> +} >> + >> +static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) >> +{ >> + struct ccu_common *common = hw_to_ccu_common(hw); >> + >> + return container_of(common, struct ccu_div, common); >> +} >> + >> +static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) >> +{ >> + struct ccu_common *common = hw_to_ccu_common(hw); >> + >> + return container_of(common, struct ccu_gate, common); >> +} >> + >> +extern const struct clk_ops ccu_div_ops; >> +extern const struct clk_ops clk_pll_ops; >> +extern const struct regmap_config th1520_clk_regmap_config; > > Why is the regmap config exported? The regmap_config is exported to allow reuse across multiple drivers. Initially, I passed the clock VOSYS address space using the reg property and created the regmap from it, enabling other drivers to utilize the same configuration. Later, I switched to a regmap-based syscon approach but haven’t moved the regmap_config back to the AP driver. Based on Krzysztof's feedback, using the reg property to pass memory-mapped registers is preferred. If needed, I can create a separate regmap_config for the VO subsystem instead of reusing the existing one. I will also address the other points you mentioned in your review. Thank you for your feedback. Michał >
diff --git a/MAINTAINERS b/MAINTAINERS index 1e930c7a58b1..7c85abf1dd1e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20188,7 +20188,7 @@ F: Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml F: Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml F: Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml F: arch/riscv/boot/dts/thead/ -F: drivers/clk/thead/clk-th1520-ap.c +F: drivers/clk/thead/ F: drivers/mailbox/mailbox-th1520.c F: drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c F: drivers/pinctrl/pinctrl-th1520.c diff --git a/drivers/clk/thead/Makefile b/drivers/clk/thead/Makefile index 7ee0bec1f251..d7cf88390b69 100644 --- a/drivers/clk/thead/Makefile +++ b/drivers/clk/thead/Makefile @@ -1,2 +1,2 @@ # SPDX-License-Identifier: GPL-2.0 -obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520-ap.o +obj-$(CONFIG_CLK_THEAD_TH1520_AP) += clk-th1520.o clk-th1520-ap.o diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 17e32ae08720..a6015805b859 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -5,297 +5,9 @@ * Authors: Yangtao Li <frank.li@vivo.com> */ -#include <dt-bindings/clock/thead,th1520-clk-ap.h> -#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 - -struct ccu_internal { - u8 shift; - u8 width; -}; - -struct ccu_div_internal { - u8 shift; - u8 width; - u32 flags; -}; - -struct ccu_common { - int clkid; - struct regmap *map; - u16 cfg0; - u16 cfg1; - struct clk_hw hw; -}; - -struct ccu_mux { - struct ccu_internal mux; - struct ccu_common common; -}; - -struct ccu_gate { - u32 enable; - struct ccu_common common; -}; - -struct ccu_div { - u32 enable; - struct ccu_div_internal div; - struct ccu_internal mux; - struct ccu_common common; -}; - -struct ccu_pll { - struct ccu_common common; -}; - -#define TH_CCU_ARG(_shift, _width) \ - { \ - .shift = _shift, \ - .width = _width, \ - } - -#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ - { \ - .shift = _shift, \ - .width = _width, \ - .flags = _flags, \ - } - -#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ - struct ccu_gate _struct = { \ - .enable = _gate, \ - .common = { \ - .clkid = _clkid, \ - .cfg0 = _reg, \ - .hw.init = CLK_HW_INIT_PARENTS_DATA( \ - _name, \ - _parent, \ - &clk_gate_ops, \ - _flags), \ - } \ - } - -static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) -{ - return container_of(hw, struct ccu_common, hw); -} - -static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) -{ - struct ccu_common *common = hw_to_ccu_common(hw); - - return container_of(common, struct ccu_mux, common); -} - -static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) -{ - struct ccu_common *common = hw_to_ccu_common(hw); +#include "clk-th1520.h" - return container_of(common, struct ccu_pll, common); -} - -static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) -{ - struct ccu_common *common = hw_to_ccu_common(hw); - - return container_of(common, struct ccu_div, common); -} - -static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) -{ - struct ccu_common *common = hw_to_ccu_common(hw); - - return container_of(common, struct ccu_gate, common); -} - -static u8 ccu_get_parent_helper(struct ccu_common *common, - struct ccu_internal *mux) -{ - unsigned int val; - u8 parent; - - regmap_read(common->map, common->cfg0, &val); - parent = val >> mux->shift; - parent &= GENMASK(mux->width - 1, 0); - - return parent; -} - -static int ccu_set_parent_helper(struct ccu_common *common, - struct ccu_internal *mux, - u8 index) -{ - return regmap_update_bits(common->map, common->cfg0, - GENMASK(mux->width - 1, 0) << mux->shift, - index << mux->shift); -} - -static void ccu_disable_helper(struct ccu_common *common, u32 gate) -{ - if (!gate) - return; - regmap_update_bits(common->map, common->cfg0, - gate, ~gate); -} - -static int ccu_enable_helper(struct ccu_common *common, u32 gate) -{ - unsigned int val; - int ret; - - if (!gate) - return 0; - - ret = regmap_update_bits(common->map, common->cfg0, gate, gate); - regmap_read(common->map, common->cfg0, &val); - return ret; -} - -static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) -{ - unsigned int val; - - if (!gate) - return true; - - regmap_read(common->map, common->cfg0, &val); - return val & gate; -} - -static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, - unsigned long parent_rate) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - unsigned long rate; - unsigned int val; - - regmap_read(cd->common.map, cd->common.cfg0, &val); - val = val >> cd->div.shift; - val &= GENMASK(cd->div.width - 1, 0); - rate = divider_recalc_rate(hw, parent_rate, val, NULL, - cd->div.flags, cd->div.width); - - return rate; -} - -static u8 ccu_div_get_parent(struct clk_hw *hw) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - - return ccu_get_parent_helper(&cd->common, &cd->mux); -} - -static int ccu_div_set_parent(struct clk_hw *hw, u8 index) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - - return ccu_set_parent_helper(&cd->common, &cd->mux, index); -} - -static void ccu_div_disable(struct clk_hw *hw) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - - ccu_disable_helper(&cd->common, cd->enable); -} - -static int ccu_div_enable(struct clk_hw *hw) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - - return ccu_enable_helper(&cd->common, cd->enable); -} - -static int ccu_div_is_enabled(struct clk_hw *hw) -{ - struct ccu_div *cd = hw_to_ccu_div(hw); - - return ccu_is_enabled_helper(&cd->common, cd->enable); -} - -static const struct clk_ops ccu_div_ops = { - .disable = ccu_div_disable, - .enable = ccu_div_enable, - .is_enabled = ccu_div_is_enabled, - .get_parent = ccu_div_get_parent, - .set_parent = ccu_div_set_parent, - .recalc_rate = ccu_div_recalc_rate, - .determine_rate = clk_hw_determine_rate_no_reparent, -}; - -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; - unsigned int cfg0, cfg1; - u64 rate = parent_rate; - - 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; - rate = rate / div; - 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 div, 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; - - div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * - FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); - - rate = rate / div; - - 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; -} - -static const struct clk_ops clk_pll_ops = { - .recalc_rate = ccu_pll_recalc_rate, -}; +#define NR_CLKS (CLK_UART_SCLK + 1) static const struct clk_parent_data osc_24m_clk[] = { { .index = 0 } @@ -956,15 +668,6 @@ static struct ccu_common *th1520_gate_clks[] = { &sram3_clk.common, }; -#define NR_CLKS (CLK_UART_SCLK + 1) - -static const struct regmap_config th1520_clk_regmap_config = { - .reg_bits = 32, - .val_bits = 32, - .reg_stride = 4, - .fast_io = true, -}; - static int th1520_clk_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; diff --git a/drivers/clk/thead/clk-th1520.c b/drivers/clk/thead/clk-th1520.c new file mode 100644 index 000000000000..e2bfe56de9af --- /dev/null +++ b/drivers/clk/thead/clk-th1520.c @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. + * Authors: Yangtao Li <frank.li@vivo.com> + */ + +#include "clk-th1520.h" + +static u8 ccu_get_parent_helper(struct ccu_common *common, + struct ccu_internal *mux) +{ + unsigned int val; + u8 parent; + + regmap_read(common->map, common->cfg0, &val); + parent = val >> mux->shift; + parent &= GENMASK(mux->width - 1, 0); + + return parent; +} + +static int ccu_set_parent_helper(struct ccu_common *common, + struct ccu_internal *mux, u8 index) +{ + return regmap_update_bits(common->map, common->cfg0, + GENMASK(mux->width - 1, 0) << mux->shift, + index << mux->shift); +} + +static void ccu_disable_helper(struct ccu_common *common, u32 gate) +{ + if (!gate) + return; + regmap_update_bits(common->map, common->cfg0, gate, ~gate); +} + +static int ccu_enable_helper(struct ccu_common *common, u32 gate) +{ + unsigned int val; + int ret; + + if (!gate) + return 0; + + ret = regmap_update_bits(common->map, common->cfg0, gate, gate); + regmap_read(common->map, common->cfg0, &val); + return ret; +} + +static int ccu_is_enabled_helper(struct ccu_common *common, u32 gate) +{ + unsigned int val; + + if (!gate) + return true; + + regmap_read(common->map, common->cfg0, &val); + return val & gate; +} + +static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + unsigned long rate; + unsigned int val; + + regmap_read(cd->common.map, cd->common.cfg0, &val); + val = val >> cd->div.shift; + val &= GENMASK(cd->div.width - 1, 0); + rate = divider_recalc_rate(hw, parent_rate, val, NULL, cd->div.flags, + cd->div.width); + + return rate; +} + +static u8 ccu_div_get_parent(struct clk_hw *hw) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + + return ccu_get_parent_helper(&cd->common, &cd->mux); +} + +static int ccu_div_set_parent(struct clk_hw *hw, u8 index) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + + return ccu_set_parent_helper(&cd->common, &cd->mux, index); +} + +static void ccu_div_disable(struct clk_hw *hw) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + + ccu_disable_helper(&cd->common, cd->enable); +} + +static int ccu_div_enable(struct clk_hw *hw) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + + return ccu_enable_helper(&cd->common, cd->enable); +} + +static int ccu_div_is_enabled(struct clk_hw *hw) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + + return ccu_is_enabled_helper(&cd->common, cd->enable); +} + +const struct clk_ops ccu_div_ops = { + .disable = ccu_div_disable, + .enable = ccu_div_enable, + .is_enabled = ccu_div_is_enabled, + .get_parent = ccu_div_get_parent, + .set_parent = ccu_div_set_parent, + .recalc_rate = ccu_div_recalc_rate, + .determine_rate = clk_hw_determine_rate_no_reparent, +}; + +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; + unsigned int cfg0, cfg1; + u64 rate = parent_rate; + + 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; + rate = rate / div; + 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 div, 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; + + div = FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) * + FIELD_GET(TH1520_PLL_POSTDIV2, cfg0); + + rate = rate / div; + + 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; +} + +const struct clk_ops clk_pll_ops = { + .recalc_rate = ccu_pll_recalc_rate, +}; + +const struct regmap_config th1520_clk_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, + .fast_io = true, +}; diff --git a/drivers/clk/thead/clk-th1520.h b/drivers/clk/thead/clk-th1520.h new file mode 100644 index 000000000000..285d41e65008 --- /dev/null +++ b/drivers/clk/thead/clk-th1520.h @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd. + * Authors: Yangtao Li <frank.li@vivo.com> + * + * clk-th1520.h - Common definitions for T-HEAD TH1520 Clock Drivers + */ + +#ifndef CLK_TH1520_H +#define CLK_TH1520_H + +#include <dt-bindings/clock/thead,th1520-clk-ap.h> +#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 + +struct ccu_internal { + u8 shift; + u8 width; +}; + +struct ccu_div_internal { + u8 shift; + u8 width; + u32 flags; +}; + +struct ccu_common { + int clkid; + struct regmap *map; + u16 cfg0; + u16 cfg1; + struct clk_hw hw; +}; + +struct ccu_mux { + struct ccu_internal mux; + struct ccu_common common; +}; + +struct ccu_gate { + u32 enable; + struct ccu_common common; +}; + +struct ccu_div { + u32 enable; + struct ccu_div_internal div; + struct ccu_internal mux; + struct ccu_common common; +}; + +struct ccu_pll { + struct ccu_common common; +}; + +#define TH_CCU_ARG(_shift, _width) \ + { \ + .shift = _shift, \ + .width = _width, \ + } + +#define TH_CCU_DIV_FLAGS(_shift, _width, _flags) \ + { \ + .shift = _shift, \ + .width = _width, \ + .flags = _flags, \ + } + +#define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _gate, _flags) \ + struct ccu_gate _struct = { \ + .enable = _gate, \ + .common = { \ + .clkid = _clkid, \ + .cfg0 = _reg, \ + .hw.init = CLK_HW_INIT_PARENTS_DATA( \ + _name, \ + _parent, \ + &clk_gate_ops, \ + _flags), \ + } \ + } + +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw) +{ + return container_of(hw, struct ccu_common, hw); +} + +static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw) +{ + struct ccu_common *common = hw_to_ccu_common(hw); + + return container_of(common, struct ccu_mux, common); +} + +static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw) +{ + struct ccu_common *common = hw_to_ccu_common(hw); + + return container_of(common, struct ccu_pll, common); +} + +static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw) +{ + struct ccu_common *common = hw_to_ccu_common(hw); + + return container_of(common, struct ccu_div, common); +} + +static inline struct ccu_gate *hw_to_ccu_gate(struct clk_hw *hw) +{ + struct ccu_common *common = hw_to_ccu_common(hw); + + return container_of(common, struct ccu_gate, common); +} + +extern const struct clk_ops ccu_div_ops; +extern const struct clk_ops clk_pll_ops; +extern const struct regmap_config th1520_clk_regmap_config; + +#endif /* CLK_TH1520_H */
The T-Head TH1520 SoC includes various clocks for different subsystems like Application Processor (AP) and Video Output (VO) [1]. Currently, the clock driver only implements AP clocks. Since the programming interface for these clocks is identical across subsystems, refactor the code to move common functions into clk-th1520.c. This prepares the driver to support VO clocks by reducing code duplication and improving maintainability. No functional changes are introduced with this refactoring. Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1] Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- MAINTAINERS | 2 +- drivers/clk/thead/Makefile | 2 +- drivers/clk/thead/clk-th1520-ap.c | 301 +----------------------------- drivers/clk/thead/clk-th1520.c | 188 +++++++++++++++++++ drivers/clk/thead/clk-th1520.h | 134 +++++++++++++ 5 files changed, 326 insertions(+), 301 deletions(-) create mode 100644 drivers/clk/thead/clk-th1520.c create mode 100644 drivers/clk/thead/clk-th1520.h