Re: [PATCH v4 4/4] clk: mediatek: add drivers for MT7988 SoC

From: AngeloGioacchino Del Regno
Date: Mon Dec 11 2023 - 04:57:09 EST


Il 09/12/23 22:56, Daniel Golle ha scritto:
From: Sam Shih <sam.shih@xxxxxxxxxxxx>

Add APMIXED, ETH, INFRACFG and TOPCKGEN clock drivers which are
typical MediaTek designs.

Also add driver for XFIPLL clock generating the 156.25MHz clock for
the XFI SerDes. It needs an undocumented software workaround and has
an unknown internal design.

Signed-off-by: Sam Shih <sam.shih@xxxxxxxxxxxx>
Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
---
v4:
* make use of existing GATE_MTK_FLAGS macro
* reformat to max. 100 columns
* cosmetics

v3: use git --from ...
v2: no changes


drivers/clk/mediatek/Kconfig | 9 +
drivers/clk/mediatek/Makefile | 5 +
drivers/clk/mediatek/clk-mt7988-apmixed.c | 102 +++++++
drivers/clk/mediatek/clk-mt7988-eth.c | 133 +++++++++
drivers/clk/mediatek/clk-mt7988-infracfg.c | 274 +++++++++++++++++
drivers/clk/mediatek/clk-mt7988-topckgen.c | 325 +++++++++++++++++++++
drivers/clk/mediatek/clk-mt7988-xfipll.c | 78 +++++
7 files changed, 926 insertions(+)
create mode 100644 drivers/clk/mediatek/clk-mt7988-apmixed.c
create mode 100644 drivers/clk/mediatek/clk-mt7988-eth.c
create mode 100644 drivers/clk/mediatek/clk-mt7988-infracfg.c
create mode 100644 drivers/clk/mediatek/clk-mt7988-topckgen.c
create mode 100644 drivers/clk/mediatek/clk-mt7988-xfipll.c

diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index 48b42d11111cd..70a005e7e1b18 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -423,6 +423,15 @@ config COMMON_CLK_MT7986_ETHSYS
This driver adds support for clocks for Ethernet and SGMII
required on MediaTek MT7986 SoC.
+config COMMON_CLK_MT7988
+ tristate "Clock driver for MediaTek MT7988"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select COMMON_CLK_MEDIATEK
+ default ARCH_MEDIATEK
+ help
+ This driver supports MediaTek MT7988 basic clocks and clocks
+ required for various periperals found on this SoC.
+
config COMMON_CLK_MT8135
tristate "Clock driver for MediaTek MT8135"
depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST
diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index dbeaa5b41177d..eeccfa039896f 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -62,6 +62,11 @@ obj-$(CONFIG_COMMON_CLK_MT7986) += clk-mt7986-apmixed.o
obj-$(CONFIG_COMMON_CLK_MT7986) += clk-mt7986-topckgen.o
obj-$(CONFIG_COMMON_CLK_MT7986) += clk-mt7986-infracfg.o
obj-$(CONFIG_COMMON_CLK_MT7986_ETHSYS) += clk-mt7986-eth.o
+obj-$(CONFIG_COMMON_CLK_MT7988) += clk-mt7988-apmixed.o
+obj-$(CONFIG_COMMON_CLK_MT7988) += clk-mt7988-topckgen.o
+obj-$(CONFIG_COMMON_CLK_MT7988) += clk-mt7988-infracfg.o
+obj-$(CONFIG_COMMON_CLK_MT7988) += clk-mt7988-eth.o
+obj-$(CONFIG_COMMON_CLK_MT7988) += clk-mt7988-xfipll.o
obj-$(CONFIG_COMMON_CLK_MT8135) += clk-mt8135-apmixedsys.o clk-mt8135.o
obj-$(CONFIG_COMMON_CLK_MT8167) += clk-mt8167-apmixedsys.o clk-mt8167.o
obj-$(CONFIG_COMMON_CLK_MT8167_AUDSYS) += clk-mt8167-aud.o
diff --git a/drivers/clk/mediatek/clk-mt7988-apmixed.c b/drivers/clk/mediatek/clk-mt7988-apmixed.c
new file mode 100644
index 0000000000000..02eb6354b01a8
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt7988-apmixed.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Sam Shih <sam.shih@xxxxxxxxxxxx>
+ * Author: Xiufeng Li <Xiufeng.Li@xxxxxxxxxxxx>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include "clk-mtk.h"
+#include "clk-gate.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
+#include <dt-bindings/clock/mediatek,mt7988-clk.h>
+
+#define MT7988_PLL_FMAX (2500UL * MHZ)
+#define MT7988_PCW_CHG_SHIFT 2
+
+#define PLL(_id, _name, _reg, _pwr_reg, _en_mask, _flags, _rst_bar_mask, _pcwbits, _pd_reg, \
+ _pd_shift, _tuner_reg, _tuner_en_reg, _tuner_en_bit, _pcw_reg, _pcw_shift, \
+ _pcw_chg_reg) \
+ { \
+ .id = _id, .name = _name, .reg = _reg, .pwr_reg = _pwr_reg, .en_mask = _en_mask, \
+ .flags = _flags, .rst_bar_mask = BIT(_rst_bar_mask), .fmax = MT7988_PLL_FMAX, \
+ .pcwbits = _pcwbits, .pd_reg = _pd_reg, .pd_shift = _pd_shift, \
+ .tuner_reg = _tuner_reg, .tuner_en_reg = _tuner_en_reg, \
+ .tuner_en_bit = _tuner_en_bit, .pcw_reg = _pcw_reg, .pcw_shift = _pcw_shift, \
+ .pcw_chg_reg = _pcw_chg_reg, .pcw_chg_shift = MT7988_PCW_CHG_SHIFT, \
+ .parent_name = "clkxtal", \
+ }

I think that there was a bit of misunderstanding here: I said 100cols, and that's
fine, but I wanted you to do that with everything but the macros, following what
was done in all the other MediaTek clock drivers.

Can you please change the macros again?

Also, there's some discrepancy in the usage of tabulations vs spaces, please fix.

#define PLL(....)*TAB*\

....

{ \
.id = _id, \
.name = _name, \
.reg = _reg, \
.pwr_reg = _pwr_reg, \
.en_mask = _en_mask, \
...etc etc etc... \
}

Thanks,
Angelo