Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> The documentation for this SOC only talks about two
> registers regarding to the clocks:
> * SYSC_REG_CPLL_CLKCFG0 - provides some information about
> boostrapped refclock. PLL and dividers used for CPU and some
> sort of BUS.
> * SYSC_REG_CPLL_CLKCFG1 - a banch of gates to enable/disable
> clocks for all or some ip cores.
>
> Looking into driver code, and some openWRT patched there are
> another frequences which are used in some drivers (uart, sd...).
s/frequences/frequencies/
> According to all of this information the clock plan for this
> SoC is set as follows:
> - Main top clock "xtal" from where all the rest of the world is
> derived.
> - CPU clock "cpu" derived from "xtal" frequencies and a bunch of
> register reads and predividers.
> - BUS clock "bus" derived from "cpu" and with (cpu / 4) MHz.
> - Fixed clocks from "xtal":
> * "50m": 50 MHz.
> * "125m": 125 MHz.
> * "150m": 150 MHz.
> * "250m": 250 MHz.
> * "270m": 270 MHz.
>
> We also have a buch of gate clocks with their parents:
> * "hsdma": "150m"
> * "fe": "250m"
> * "sp_divtx": "270m"
> * "timer": "50m"
> * "pcm": "270m"
> * "pio": "50m"
> * "gdma": "bus"
> * "nand": "125m"
> * "i2c": "50m"
> * "i2s": "270m"
> * "spi": "bus"
> * "uart1": "50m"
> * "uart2": "50m"
> * "uart3": "50m"
> * "eth": "50m"
> * "pcie0": "125m"
> * "pcie1": "125m"
> * "pcie2": "125m"
> * "crypto": "250m"
> * "shxc": "50m"
>
> With this information the clk driver will provide clock and gates
> functionality from a a set of hardcoded clocks allowing to define
> a nice device tree without fixed clocks.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 1 +
> drivers/clk/ralink/Kconfig | 14 +
> drivers/clk/ralink/Makefile | 2 +
> drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
> 5 files changed, 453 insertions(+)
> create mode 100644 drivers/clk/ralink/Kconfig
> create mode 100644 drivers/clk/ralink/Makefile
> create mode 100644 drivers/clk/ralink/clk-mt7621.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index c715d4681a0b..5f94c4329033 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -372,6 +372,7 @@ source "drivers/clk/mediatek/Kconfig"
> source "drivers/clk/meson/Kconfig"
> source "drivers/clk/mvebu/Kconfig"
> source "drivers/clk/qcom/Kconfig"
> +source "drivers/clk/ralink/Kconfig"
> source "drivers/clk/renesas/Kconfig"
> source "drivers/clk/rockchip/Kconfig"
> source "drivers/clk/samsung/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index da8fcf147eb1..6578e167b047 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_COMMON_CLK_NXP) += nxp/
> obj-$(CONFIG_MACH_PISTACHIO) += pistachio/
> obj-$(CONFIG_COMMON_CLK_PXA) += pxa/
> obj-$(CONFIG_COMMON_CLK_QCOM) += qcom/
> +obj-y += ralink/
> obj-y += renesas/
> obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
> obj-$(CONFIG_COMMON_CLK_SAMSUNG) += samsung/
Thanks for keeping it sorted!
> diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> new file mode 100644
> index 000000000000..7e8697327e0c
> --- /dev/null
> +++ b/drivers/clk/ralink/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# MediaTek Mt7621 Clock Driver
> +#
> +menu "Clock driver for mediatek mt7621 SoC"
Capitalize MediaTek?
> + depends on SOC_MT7621 || COMPILE_TEST
> +
> +config CLK_MT7621
> + bool "Clock driver for MediaTek MT7621"
Like it is done here.
> + depends on SOC_MT7621 || COMPILE_TEST
> + default SOC_MT7621
> + help
> + This driver supports MediaTek MT7621 basic clocks.
> +endmenu
> diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
> new file mode 100644
> index 000000000000..cf6f9216379d
> --- /dev/null
> +++ b/drivers/clk/ralink/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
> diff --git a/drivers/clk/ralink/clk-mt7621.c b/drivers/clk/ralink/clk-mt7621.c
> new file mode 100644
> index 000000000000..4e929f13fe7c
> --- /dev/null
> +++ b/drivers/clk/ralink/clk-mt7621.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek MT7621 Clock Driver
> + * Author: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <asm/mach-ralink/ralink_regs.h>
Is it possible to drop this include? Doing so would make this portable
and compilable on more architectures so us cross compilers can check
build stuff and make changes easily.
> +#include <dt-bindings/clock/mt7621-clk.h>
> +
> +/* Configuration registers */
> +#define SYSC_REG_SYSTEM_CONFIG0 0x10
> +#define SYSC_REG_SYSTEM_CONFIG1 0x14
> +#define SYSC_REG_CLKCFG0 0x2c
> +#define SYSC_REG_CLKCFG1 0x30
> +#define SYSC_REG_CUR_CLK_STS 0x44
> +
> +#define MEMC_REG_CPU_PLL 0x648
> +#define XTAL_MODE_SEL_MASK 0x7
> +#define XTAL_MODE_SEL_SHIFT 6
> +
> +#define CPU_CLK_SEL_MASK 0x3
> +#define CPU_CLK_SEL_SHIFT 30
> +
> +#define CUR_CPU_FDIV_MASK 0x1f
> +#define CUR_CPU_FDIV_SHIFT 8
> +#define CUR_CPU_FFRAC_MASK 0x1f
> +#define CUR_CPU_FFRAC_SHIFT 0
> +
> +#define CPU_PLL_PREDIV_MASK 0x3
> +#define CPU_PLL_PREDIV_SHIFT 12
> +#define CPU_PLL_FBDIV_MASK 0x7f
> +#define CPU_PLL_FBDIV_SHIFT 4
> +
> +#define MHZ(x) ((x) * 1000 * 1000)
I'd rather we just wrote it out in Hz and dropped this macro.
> +
> +struct mt7621_clk_provider {
> + struct device_node *node;
> + struct regmap *syscon_regmap;
> + struct clk_hw_onecell_data *clk_data;
> +};
> +
> +struct mt7621_clk {
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw hw;
> +};
> +
> +struct mt7621_fixed_clk {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + unsigned long rate;
> + struct clk_hw *hw;
> +};
> +
> +struct mt7621_gate {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + u32 bit_idx;
> + struct clk_hw hw;
> +};
> +
> +#define GATE(_id, _name, _pname, _shift) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
> + .bit_idx = _shift \
> + }
> +
> +static struct mt7621_gate mt7621_gates[] = {
> + GATE(MT7621_CLK_HSDMA, "hsdma", "150m", BIT(5)),
> + GATE(MT7621_CLK_FE, "fe", "250m", BIT(6)),
> + GATE(MT7621_CLK_SP_DIVTX, "sp_divtx", "270m", BIT(7)),
> + GATE(MT7621_CLK_TIMER, "timer", "50m", BIT(8)),
> + GATE(MT7621_CLK_PCM, "pcm", "270m", BIT(11)),
> + GATE(MT7621_CLK_PIO, "pio", "50m", BIT(13)),
> + GATE(MT7621_CLK_GDMA, "gdma", "bus", BIT(14)),
> + GATE(MT7621_CLK_NAND, "nand", "125m", BIT(15)),
> + GATE(MT7621_CLK_I2C, "i2c", "50m", BIT(16)),
> + GATE(MT7621_CLK_I2S, "i2s", "270m", BIT(17)),
> + GATE(MT7621_CLK_SPI, "spi", "bus", BIT(18)),
> + GATE(MT7621_CLK_UART1, "uart1", "50m", BIT(19)),
> + GATE(MT7621_CLK_UART2, "uart2", "50m", BIT(20)),
> + GATE(MT7621_CLK_UART3, "uart3", "50m", BIT(21)),
> + GATE(MT7621_CLK_ETH, "eth", "50m", BIT(23)),
> + GATE(MT7621_CLK_PCIE0, "pcie0", "125m", BIT(24)),
> + GATE(MT7621_CLK_PCIE1, "pcie1", "125m", BIT(25)),
> + GATE(MT7621_CLK_PCIE2, "pcie2", "125m", BIT(26)),
> + GATE(MT7621_CLK_CRYPTO, "crypto", "250m", BIT(29)),
> + GATE(MT7621_CLK_SHXC, "shxc", "50m", BIT(30))
> +};
> +
> +static inline struct mt7621_gate *to_mt7621_gate(struct clk_hw *hw)
> +{
> + return container_of(hw, struct mt7621_gate, hw);
> +}
> +
> +static int mt7621_gate_enable(struct clk_hw *hw)
> +{
> + struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> + struct regmap *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + return regmap_update_bits(scon, SYSC_REG_CLKCFG1,
> + clk_gate->bit_idx, clk_gate->bit_idx);
> +}
> +
> +static void mt7621_gate_disable(struct clk_hw *hw)
> +{
> + struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> + struct regmap *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + regmap_update_bits(scon, SYSC_REG_CLKCFG1, clk_gate->bit_idx, 0);
> +}
> +
> +static int mt7621_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> + struct regmap *scon = clk_gate->clk_prov->syscon_regmap;
> + unsigned int val;
> +
> + if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
> + return 0;
> +
> + return val & clk_gate->bit_idx;
> +}
> +
> +static const struct clk_ops mt7621_gate_ops = {
> + .enable = mt7621_gate_enable,
> + .disable = mt7621_gate_disable,
> + .is_enabled = mt7621_gate_is_enabled,
> +};
> +
> +static int mt7621_gate_ops_init(struct device_node *np,
> + struct mt7621_gate *sclk)
> +{
> + struct clk_init_data init = {
> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
driver probe instead of here? Or left out of the kernel entirely if they
shouldn't be turned off?
> + .num_parents = 1,
> + .parent_names = &sclk->parent_name,
> + .ops = &mt7621_gate_ops,
> + .name = sclk->name,
> + };
> +
> + sclk->hw.init = &init;
> + return of_clk_hw_register(np, &sclk->hw);
> +}
> +
> +static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
Why do we take a pointer of a pointer.
> + struct clk_hw **hws = (*clk_data)->hws;
And then deref that pointer?
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> + struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = mt7621_gate_ops_init(clk_prov->node, sclk);
> + if (ret) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = &sclk->hw;
> + (*clk_data)->num++;
Just use the pointer?
> + }
> +
> + return 0;
> +
> +err_clk_unreg:
> + while (--i >= 0) {
> + struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> + return ret;
> +}
> +
> +#define FIXED(_id, _name, _pname, _rate) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
This can be dropped as NULL is the default.
> + .rate = _rate \
> + }
> +
> +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> + FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
> + FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
> + FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
> + FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
> + FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
parent is always "xtal" so maybe just hardcode that?
> +};
> +
> +static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> + struct clk_hw **hws = (*clk_data)->hws;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> + struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> + sclk->clk_prov = clk_prov;
> + sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
> + sclk->parent_name, 0,
> + sclk->rate);
> + if (IS_ERR(sclk->hw)) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + ret = PTR_ERR(sclk->hw);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + return 0;
> +
> +err_clk_unreg:
> + while (--i >= 0) {
> + struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> + clk_hw_unregister_fixed_rate(sclk->hw);
> + }
> + return ret;
> +}
> +
> +static inline struct mt7621_clk *to_mt7621_clk(struct clk_hw *hw)
> +{
> + return container_of(hw, struct mt7621_clk, hw);
> +}
> +
> +static unsigned long mt7621_xtal_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct mt7621_clk *clk = to_mt7621_clk(hw);
> + struct regmap *scon = clk->clk_prov->syscon_regmap;
> + u32 val;
> +
> + regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
> + val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> +
> + if (val <= 2)
> + return MHZ(20);
> + else if (val <= 5)
Replace with 'if' because it returns above.
> + return MHZ(40);
> +
> + return MHZ(25);
> +}
> +
> +static unsigned long mt7621_cpu_recalc_rate(struct clk_hw *hw,
> + unsigned long xtal_clk)
> +{
> + static const u32 prediv_tbl[] = { 0, 1, 2, 2 };
> + struct mt7621_clk *clk = to_mt7621_clk(hw);
> + struct regmap *scon = clk->clk_prov->syscon_regmap;
> + u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> + u32 pll, prediv, fbdiv;
> + unsigned long cpu_clk;
> +
> + regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
> + clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
> +
> + regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
> + ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
> + ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
> +
> + switch (clk_sel) {
> + case 0:
> + cpu_clk = MHZ(500);
> + break;
> + case 1:
> + pll = rt_memc_r32(MEMC_REG_CPU_PLL);
> + fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
> + prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
> + cpu_clk = ((fbdiv + 1) * xtal_clk) >> prediv_tbl[prediv];
> + break;
> + default:
> + cpu_clk = xtal_clk;
> + }
> +
> + return cpu_clk / ffiv * ffrac;
> +}
> +
> +static unsigned long mt7621_bus_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + return parent_rate / 4;
> +}
> +
> +#define CLK_BASE(_name, _parent, _recalc) { \
> + .init = &(struct clk_init_data) { \
> + .name = _name, \
> + .ops = &(const struct clk_ops) { \
> + .recalc_rate = _recalc, \
> + }, \
> + .parent_names = (const char *const[]) { _parent }, \
Please use clk_parent_data instead
> + .num_parents = _parent ? 1 : 0 \
> + }, \
> +}
> +
> +static struct mt7621_clk mt7621_clks_base[] = {
> + { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> + { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> + { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> +};
> +
> +static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> + struct clk_hw **hws = (*clk_data)->hws;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
> + if (ret) {
> + pr_err("Couldn't register top clock %i\n", i);
> + goto err_clk_unreg;
> + }
> +
> + hws[i] = &sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + return 0;
> +
> +err_clk_unreg:
> + while (--i >= 0) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> + return ret;
> +}
> +
> +static void __init mt7621_clk_init(struct device_node *node)
> +{
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw_onecell_data **clk_data;
> + int i, ret, count;
> +
> + clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
> + if (!clk_prov)
> + return;
> +
> + clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
> + if (IS_ERR(clk_prov->syscon_regmap)) {
> + pr_err("Could not get syscon regmap\n");
> + goto free_clk_prov;
> + }
> +
> + clk_prov->node = node;
> +
> + clk_data = &clk_prov->clk_data;
> + count = ARRAY_SIZE(mt7621_clks_base) +
> + ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
> + *clk_data = kzalloc(struct_size(*clk_data, hws, count), GFP_KERNEL);
> + if (!*clk_data)
> + goto free_clk_prov;
> +
> + ret = mt7621_register_top_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register top clocks\n");
> + goto free_clk_data;
> + }
> +
> + ret = mt7621_register_fixed_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clocks\n");
> + goto unreg_clk_top;
> + }
> +
> + ret = mt7621_register_gates(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clock gates\n");
> + goto unreg_clk_fixed;
> + }
> +
> + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> + clk_prov->clk_data);
> + if (ret) {
> + pr_err("Couldn't add clk hw provider\n");
> + goto unreg_clk_gates;
> + }
> +
> + return;
> +
> +unreg_clk_gates:
> + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> + struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> +
> +unreg_clk_fixed:
> + for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> + struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> + clk_hw_unregister_fixed_rate(sclk->hw);
> + }
> +
> +unreg_clk_top:
> + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> +
> +free_clk_data:
> + kfree(clk_prov->clk_data);
> +
> +free_clk_prov:
> + kfree(clk_prov);
> +}
> +
> +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
Any reason to use this vs. a platform driver?
> +
> +MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>");
> +MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
> +MODULE_LICENSE("GPL v2");
It isn't a module, but it would be nice if it was one. If the Kconfig is
bool these shouldn't be here.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
