[PATCH v4 08/25] clk: mtmips: add clock driver for MediaTek MT7621 SoC

Sean Anderson seanga2 at gmail.com
Wed May 11 18:29:24 CEST 2022


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.

> +
> +#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?

> +	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().

> +
> +	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?

> +	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)

> +	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 .

> +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;

> +	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?

> +
> +	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