[PATCH v4 08/25] clk: mtmips: add clock driver for MediaTek MT7621 SoC
Weijie Gao
weijie.gao at mediatek.com
Thu May 12 04:38:00 CEST 2022
On Wed, 2022-05-11 at 12:29 -0400, Sean Anderson wrote:
> Hi Weijie,
>
> I found a copy of the MT7260 programming guide, which I am using
> as a reference. I assume that they have not diverged too much,
> but presumably most discrepencies will be accounted for in the
> difference in part numbers.
>
> On 5/10/22 8:18 AM, Weijie Gao wrote:
> > This patch adds a clock driver for MediaTek MT7621 SoC.
> > This driver provides clock gate control as well as getting clock
> > frequency
> > for CPU/SYS/XTAL and some peripherals.
> >
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > ---
> > v4 changes: none
> > v3 changes: update clock definitions to match upstream kernel
> > v2 changes: none
> > ---
> > drivers/clk/mtmips/Makefile | 1 +
> > drivers/clk/mtmips/clk-mt7621.c | 315
> > +++++++++++++++++++++++++
> > include/dt-bindings/clock/mt7621-clk.h | 46 ++++
> > 3 files changed, 362 insertions(+)
> > create mode 100644 drivers/clk/mtmips/clk-mt7621.c
> > create mode 100644 include/dt-bindings/clock/mt7621-clk.h
> >
> > diff --git a/drivers/clk/mtmips/Makefile
> > b/drivers/clk/mtmips/Makefile
> > index 732e7f2545..ee8b5afe87 100644
> > --- a/drivers/clk/mtmips/Makefile
> > +++ b/drivers/clk/mtmips/Makefile
> > @@ -1,4 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > obj-$(CONFIG_SOC_MT7620) += clk-mt7620.o
> > +obj-$(CONFIG_SOC_MT7621) += clk-mt7621.o
> > obj-$(CONFIG_SOC_MT7628) += clk-mt7628.o
> > diff --git a/drivers/clk/mtmips/clk-mt7621.c
> > b/drivers/clk/mtmips/clk-mt7621.c
> > new file mode 100644
> > index 0000000000..e8203016a5
> > --- /dev/null
> > +++ b/drivers/clk/mtmips/clk-mt7621.c
> > @@ -0,0 +1,315 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2022 MediaTek Inc. All rights reserved.
> > + *
> > + * Author: Weijie Gao <weijie.gao at mediatek.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <clk-uclass.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <regmap.h>
> > +#include <syscon.h>
> > +#include <dt-bindings/clock/mt7621-clk.h>
> > +#include <linux/bitops.h>
> > +#include <linux/io.h>
> > +
> > +#define SYSC_MAP_SIZE 0x100
> > +#define MEMC_MAP_SIZE 0x1000
> > +
> > +/* SYSC */
> > +#define SYSCFG0_REG 0x10
> > +#define XTAL_MODE_SEL_S 6
> > +#define XTAL_MODE_SEL_M 0x1c0
>
> Please use GENMASK and BIT for these defines.
OK
>
> > +
> > +#define CLKCFG0_REG 0x2c
> > +#define CPU_CLK_SEL_S 30
> > +#define CPU_CLK_SEL_M 0xc0000000
> > +#define PERI_CLK_SEL 0x10
> > +
> > +#define CLKCFG1_REG 0x30
> > +
> > +#define CUR_CLK_STS_REG 0x44
> > +#define CUR_CPU_FDIV_S 8
> > +#define CUR_CPU_FDIV_M 0x1f00
> > +#define CUR_CPU_FFRAC_S 0
> > +#define CUR_CPU_FFRAC_M 0x1f
> > +
> > +/* MEMC */
> > +#define MEMPLL1_REG 0x0604
> > +#define RG_MEPL_DIV2_SEL_S 1
> > +#define RG_MEPL_DIV2_SEL_M 0x06
> > +
> > +#define MEMPLL6_REG 0x0618
> > +#define MEMPLL18_REG 0x0648
> > +#define RG_MEPL_PREDIV_S 12
> > +#define RG_MEPL_PREDIV_M 0x3000
> > +#define RG_MEPL_FBDIV_S 4
> > +#define RG_MEPL_FBDIV_M 0x7f0
> > +
> > +/* EPLL clock */
> > +#define EPLL_CLK 50000000
> > +
> > +struct mt7621_clk_priv {
> > + void __iomem *sysc_base;
> > + void __iomem *memc_base;
> > + int cpu_clk;
> > + int ddr_clk;
> > + int sys_clk;
> > + int xtal_clk;
> > +};
> > +
> > +enum mt7621_clk_src {
> > + CLK_SRC_CPU,
> > + CLK_SRC_DDR,
> > + CLK_SRC_SYS,
> > + CLK_SRC_XTAL,
> > + CLK_SRC_PERI,
> > + CLK_SRC_125M,
> > + CLK_SRC_150M,
> > + CLK_SRC_250M,
> > + CLK_SRC_270M,
> > +
> > + __CLK_SRC_MAX
> > +};
> > +
> > +struct mt7621_clk_map {
> > + u32 cgbit;
> > + enum mt7621_clk_src clksrc;
> > +};
> > +
> > +#define CLK_MAP(_id, _cg, _src) \
> > + [_id] = { .cgbit = (_cg), .clksrc = (_src) }
> > +
> > +#define CLK_MAP_SRC(_id, _src) \
> > + [_id] = { .cgbit = UINT32_MAX, .clksrc = (_src) }
> > +
> > +static const struct mt7621_clk_map mt7621_clk_mappings[] = {
> > + CLK_MAP_SRC(MT7621_CLK_XTAL, CLK_SRC_XTAL),
> > + CLK_MAP_SRC(MT7621_CLK_CPU, CLK_SRC_CPU),
> > + CLK_MAP_SRC(MT7621_CLK_BUS, CLK_SRC_SYS),
> > + CLK_MAP_SRC(MT7621_CLK_50M, CLK_SRC_PERI),
> > + CLK_MAP_SRC(MT7621_CLK_125M, CLK_SRC_125M),
> > + CLK_MAP_SRC(MT7621_CLK_150M, CLK_SRC_150M),
> > + CLK_MAP_SRC(MT7621_CLK_250M, CLK_SRC_250M),
> > + CLK_MAP_SRC(MT7621_CLK_270M, CLK_SRC_270M),
> > +
> > + CLK_MAP(MT7621_CLK_HSDMA, 5, CLK_SRC_150M),
> > + CLK_MAP(MT7621_CLK_FE, 6, CLK_SRC_250M),
> > + CLK_MAP(MT7621_CLK_SP_DIVTX, 7, CLK_SRC_270M),
> > + CLK_MAP(MT7621_CLK_TIMER, 8, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_PCM, 11, CLK_SRC_270M),
> > + CLK_MAP(MT7621_CLK_PIO, 13, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_GDMA, 14, CLK_SRC_SYS),
> > + CLK_MAP(MT7621_CLK_NAND, 15, CLK_SRC_125M),
> > + CLK_MAP(MT7621_CLK_I2C, 16, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_I2S, 17, CLK_SRC_270M),
>
> Should this be PERI?
According to clock plan it's 270M
>
> > + CLK_MAP(MT7621_CLK_SPI, 18, CLK_SRC_SYS),
> > + CLK_MAP(MT7621_CLK_UART1, 19, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_UART2, 20, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_UART3, 21, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_ETH, 23, CLK_SRC_PERI),
> > + CLK_MAP(MT7621_CLK_PCIE0, 24, CLK_SRC_125M),
> > + CLK_MAP(MT7621_CLK_PCIE1, 25, CLK_SRC_125M),
> > + CLK_MAP(MT7621_CLK_PCIE2, 26, CLK_SRC_125M),
> > + CLK_MAP(MT7621_CLK_CRYPTO, 29, CLK_SRC_250M),
> > + CLK_MAP(MT7621_CLK_SHXC, 30, CLK_SRC_PERI),
> > +
> > + CLK_MAP_SRC(MT7621_CLK_MAX, __CLK_SRC_MAX),
> > +
> > + CLK_MAP_SRC(MT7621_CLK_DDR, CLK_SRC_DDR),
> > +};
> > +
> > +static ulong mt7621_clk_get_rate(struct clk *clk)
> > +{
> > + struct mt7621_clk_priv *priv = dev_get_priv(clk->dev);
> > + u32 val;
> > +
> > + if (clk->id >= ARRAY_SIZE(mt7621_clk_mappings))
> > + return 0;
>
> Do this check in request().
OK
>
> > +
> > + switch (mt7621_clk_mappings[clk->id].clksrc) {
> > + case CLK_SRC_CPU:
> > + return priv->cpu_clk;
> > + case CLK_SRC_DDR:
> > + return priv->ddr_clk;
> > + case CLK_SRC_SYS:
> > + return priv->sys_clk;
> > + case CLK_SRC_XTAL:
> > + return priv->xtal_clk;
> > + case CLK_SRC_PERI:
> > + val = readl(priv->sysc_base + CLKCFG0_REG);
> > + if (val & PERI_CLK_SEL)
> > + return priv->xtal_clk;
> > + else
> > + return EPLL_CLK;
>
> Shouldn't this be a proper clock like the CLK_SRC_XXXMs? In the
> 7620 this is (BBP PLL)/12 (aka the parent of PCM_480). I assume
> that this setup is different on this SoC?
The clock tree of mt7621 is completely different from that of mt7620.
The parent of EPLL is a fixed 500MHz source which is also a source
for
CPU clock when CPU_CLK_SEL == 0
>
> > + case CLK_SRC_125M:
> > + return 125000000;
> > + case CLK_SRC_150M:
> > + return 150000000;
> > + case CLK_SRC_250M:
> > + return 250000000;
> > + case CLK_SRC_270M:
> > + return 270000000;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int mt7621_clk_enable(struct clk *clk)
> > +{
> > + struct mt7621_clk_priv *priv = dev_get_priv(clk->dev);
> > + u32 cgbit;
> > +
> > + if (clk->id >= ARRAY_SIZE(mt7621_clk_mappings))
> > + return -1;
>
> Please use a real error code (e.g. ENOENT)
OK
>
> > + cgbit = mt7621_clk_mappings[clk->id].cgbit;
> > + if (cgbit == UINT32_MAX)
> > + return -1;
>
> return -ENOSYS
>
> > +
> > + setbits_32(priv->sysc_base + CLKCFG1_REG, BIT(cgbit));
> > +
> > + return 0;
> > +}
> > +
> > +static int mt7621_clk_disable(struct clk *clk)
> > +{
> > + struct mt7621_clk_priv *priv = dev_get_priv(clk->dev);
> > + u32 cgbit;
> > +
> > + if (clk->id >= ARRAY_SIZE(mt7621_clk_mappings))
> > + return -1;
> > +
> > + cgbit = mt7621_clk_mappings[clk->id].cgbit;
> > + if (cgbit == UINT32_MAX)
> > + return -1;
> > +
> > + clrbits_32(priv->sysc_base + CLKCFG1_REG, BIT(cgbit));
> > +
> > + return 0;
> > +}
>
> ditto
>
> > +
> > +const struct clk_ops mt7621_clk_ops = {
> > + .enable = mt7621_clk_enable,
> > + .disable = mt7621_clk_disable,
> > + .get_rate = mt7621_clk_get_rate,
> > +};
> > +
> > +static void mt7621_get_clocks(struct mt7621_clk_priv *priv)
> > +{
> > + u32 bs, xtal_sel, clkcfg0, cur_clk, mempll, dividx, fb;
> > + u32 xtal_clk, xtal_div, ffiv, ffrac, cpu_clk, ddr_clk;
> > + static const u32 xtal_div_tbl[] = {0, 1, 2, 2};
> > +
> > + bs = readl(priv->sysc_base + SYSCFG0_REG);
> > + clkcfg0 = readl(priv->sysc_base + CLKCFG0_REG);
> > + cur_clk = readl(priv->sysc_base + CUR_CLK_STS_REG);
> > +
> > + xtal_sel = (bs & XTAL_MODE_SEL_M) >> XTAL_MODE_SEL_S;
> > +
> > + if (xtal_sel <= 2)
> > + xtal_clk = 20 * 1000 * 1000;
> > + else if (xtal_sel <= 5)
> > + xtal_clk = 40 * 1000 * 1000;
> > + else
> > + xtal_clk = 25 * 1000 * 1000;
> > +
> > + switch ((clkcfg0 & CPU_CLK_SEL_M) >> CPU_CLK_SEL_S) {
> > + case 0:
> > + cpu_clk = 500 * 1000 * 1000;
> > + break;
> > + case 1:
> > + mempll = readl(priv->memc_base + MEMPLL18_REG);
> > + dividx = (mempll & RG_MEPL_PREDIV_M) >>
> > RG_MEPL_PREDIV_S;
> > + fb = (mempll & RG_MEPL_FBDIV_M) >>
> > RG_MEPL_FBDIV_S;
> > + xtal_div = 1 << xtal_div_tbl[dividx];
> > + cpu_clk = (fb + 1) * xtal_clk / xtal_div;
> > + break;
> > + default:
> > + cpu_clk = xtal_clk;
> > + }
> > +
> > + ffiv = (cur_clk & CUR_CPU_FDIV_M) >> CUR_CPU_FDIV_S;
> > + ffrac = (cur_clk & CUR_CPU_FFRAC_M) >> CUR_CPU_FFRAC_S;
> > + cpu_clk = cpu_clk / ffiv * ffrac;
> > +
> > + mempll = readl(priv->memc_base + MEMPLL6_REG);
> > + dividx = (mempll & RG_MEPL_PREDIV_M) >> RG_MEPL_PREDIV_S;
> > + fb = (mempll & RG_MEPL_FBDIV_M) >> RG_MEPL_FBDIV_S;
> > + xtal_div = 1 << xtal_div_tbl[dividx];
> > + ddr_clk = fb * xtal_clk / xtal_div;
> > +
> > + bs = readl(priv->memc_base + MEMPLL1_REG);
> > + if (((bs & RG_MEPL_DIV2_SEL_M) >> RG_MEPL_DIV2_SEL_S) ==
> > 0)
> > + ddr_clk *= 2;
> > +
> > + priv->cpu_clk = cpu_clk;
> > + priv->sys_clk = cpu_clk / 4;
> > + priv->ddr_clk = ddr_clk;
> > + priv->xtal_clk = xtal_clk;
> > +}
>
> I would generally like to see these calculations done in get_rate
> (to allow for set_rate), but this style is fine .
All clocks except for CLK_SRC_PERI are fixed in mt7621 after
initialization and there is no way to change them. Therefore the
calculation can be done in probe stage.
As for the CLK_SRC_PERI, although its parent can be changed, the xtal
source is never used and nearly all mt7621 devices use hardcoded 50M.
It should not be change otherwise it will break the compatibility with
all existed devices.
>
> > +static int mt7621_clk_probe(struct udevice *dev)
> > +{
> > + struct mt7621_clk_priv *priv = dev_get_priv(dev);
> > + struct ofnode_phandle_args args;
> > + struct regmap *regmap;
> > + void __iomem *base;
> > + int ret;
> > +
> > + /* get corresponding sysc phandle */
> > + ret = dev_read_phandle_with_args(dev, "mediatek,sysc",
> > NULL, 0, 0,
> > + &args);
> > + if (ret)
> > + return ret;
>
> I think it might be better to have this node be a child of the sysc.
> Like
>
> sysc: sysc at 1e000000 {
> compatible = "mediatek,mt7621-sysc", "syscon";
> reg = <0x1e000000 0x100>;
> u-boot,dm-pre-reloc;
>
> clkctrl: clock-controller {
> compatible = "mediatek,mt7621-clk";
> mediatek,memc = <&memc>;
> #clock-cells = <1>;
> };
>
> rstctrl: reset-controller {
> compatible = "mediatek,mtmips-reset";
> #reset-cells = <1>;
> };
>
> reboot {
> compatible = "resetctl-reboot";
>
> resets = <&rstctrl RST_SYS>;
> reset-names = "sysreset";
> };
> };
>
> And then do something like
>
> base = dev_read_addr_ptr(dev_get_parent(dev));
> if (!priv->base)
> return -EINVAL;
This seems good. I'll try it.
>
> > + regmap = syscon_node_to_regmap(args.node);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + base = regmap_get_range(regmap, 0);
> > + if (!base) {
> > + dev_err(dev, "Unable to find sysc\n");
> > + return -ENODEV;
> > + }
> > +
> > + priv->sysc_base = ioremap_nocache((phys_addr_t)base,
> > SYSC_MAP_SIZE);
> > +
> > + /* get corresponding memc phandle */
> > + ret = dev_read_phandle_with_args(dev, "mediatek,memc",
> > NULL, 0, 0,
> > + &args);
> > + if (ret)
> > + return ret;
> > +
> > + regmap = syscon_node_to_regmap(args.node);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + base = regmap_get_range(regmap, 0);
> > + if (!base) {
> > + dev_err(dev, "Unable to find memc\n");
> > + return -ENODEV;
> > + }
> > +
> > + priv->memc_base = ioremap_nocache((phys_addr_t)base,
> > MEMC_MAP_SIZE);
>
> Why not just use the regmap api?
Oh, I just forgot that.
>
> > +
> > + mt7621_get_clocks(priv);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id mt7621_clk_ids[] = {
> > + { .compatible = "mediatek,mt7621-clk" },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(mt7621_clk) = {
> > + .name = "mt7621-clk",
> > + .id = UCLASS_CLK,
> > + .of_match = mt7621_clk_ids,
> > + .probe = mt7621_clk_probe,
> > + .priv_auto = sizeof(struct mt7621_clk_priv),
> > + .ops = &mt7621_clk_ops,
> > +};
> > diff --git a/include/dt-bindings/clock/mt7621-clk.h b/include/dt-
> > bindings/clock/mt7621-clk.h
> > new file mode 100644
> > index 0000000000..978c67951b
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/mt7621-clk.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2022 MediaTek Inc. All rights reserved.
> > + *
> > + * Author: Weijie Gao <weijie.gao at mediatek.com>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_MT7621_CLK_H_
> > +#define _DT_BINDINGS_MT7621_CLK_H_
> > +
> > +#define MT7621_CLK_XTAL 0
> > +#define MT7621_CLK_CPU 1
> > +#define MT7621_CLK_BUS 2
> > +#define MT7621_CLK_50M 3
> > +#define MT7621_CLK_125M 4
> > +#define MT7621_CLK_150M 5
> > +#define MT7621_CLK_250M 6
> > +#define MT7621_CLK_270M 7
> > +
> > +#define MT7621_CLK_HSDMA 8
> > +#define MT7621_CLK_FE 9
> > +#define MT7621_CLK_SP_DIVTX 10
> > +#define MT7621_CLK_TIMER 11
> > +#define MT7621_CLK_PCM 12
> > +#define MT7621_CLK_PIO 13
> > +#define MT7621_CLK_GDMA 14
> > +#define MT7621_CLK_NAND 15
> > +#define MT7621_CLK_I2C 16
> > +#define MT7621_CLK_I2S 17
> > +#define MT7621_CLK_SPI 18
> > +#define MT7621_CLK_UART1 19
> > +#define MT7621_CLK_UART2 20
> > +#define MT7621_CLK_UART3 21
> > +#define MT7621_CLK_ETH 22
> > +#define MT7621_CLK_PCIE0 23
> > +#define MT7621_CLK_PCIE1 24
> > +#define MT7621_CLK_PCIE2 25
> > +#define MT7621_CLK_CRYPTO 26
> > +#define MT7621_CLK_SHXC 27
> > +
> > +#define MT7621_CLK_MAX 28
> > +
> > +/* for u-boot only */
> > +#define MT7621_CLK_DDR 29
> > +
> > +#endif /* _DT_BINDINGS_MT7621_CLK_H_ */
> >
>
> --Sean
More information about the U-Boot
mailing list