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

Reply via email to