[PATCH v2 0/4] Mediatek MT6735 main clock and reset drivers

[PATCH v2 0/4] Mediatek MT6735 main clock and reset drivers

[Yassine Oudjana]

This series adds support for the main clock and reset controllers on the
Mediatek MT6735 SoC:
- apmixedsys (global PLLs)
- topckgen (global divisors and muxes)
- infracfg (gates and resets for internal components)
- pericfg (gates and resets for peripherals)

MT6735 has other more specialized clock controllers, support for which is
not included in this series:
- imgsys (camera)
- mmsys (display)
- vdecsys (video decoder)
- audsys (audio)

Tested on a Samsung Galaxy Grand Prime+ "grandpplte" with MT6737T, a slight
variant of MT6735 with no known differences in the clock controllers. 

Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
- clk: mediatek: Improvements to simple probe/remove and reset controller unregistration
  https://patchwork.kernel.org/project/linux-clk/cover/20220519134728.456643-1-y.oudjana@protonmail.com/

Above are dependencies for patch 4/4 only; DT bindings don't need them.

Changes since v1:
- Rebase on some pending patches (listed as dependencies above).
- Move common clock improvemenets to a separate series (last dependency
  listed above).
- Use mtk_clk_simple_probe/remove after making them support several clock types
  in said series.
- Combine all 4 drivers into one patch, and use one Kconfig symbol for all
  following a conversation seen on a different series[1].
- Correct APLL2 registers in apmixedsys driver (were offset backwards by 0x4).
- Make irtx clock name lower case to match the other clocks.

[1] https://lore.kernel.org/linux-mediatek/CAGXv+5H4gF5GXzfk8mjkG4Kry8uCs1CQbKoViBuc9LC+XdHH=A@mail.gmail.com/

Yassine Oudjana (4):
  dt-bindings: clock: Add Mediatek MT6735 clock bindings
  dt-bindings: reset: Add MT6735 reset bindings
  dt-bindings: arm: mediatek: Add MT6735 clock controller compatibles
  clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

 .../arm/mediatek/mediatek,infracfg.yaml       |    8 +-
 .../arm/mediatek/mediatek,pericfg.yaml        |    1 +
 .../bindings/clock/mediatek,apmixedsys.yaml   |    4 +-
 .../bindings/clock/mediatek,topckgen.yaml     |    4 +-
 MAINTAINERS                                   |   16 +
 drivers/clk/mediatek/Kconfig                  |    9 +
 drivers/clk/mediatek/Makefile                 |    1 +
 drivers/clk/mediatek/clk-mt6735-apmixedsys.c  |  235 ++++
 drivers/clk/mediatek/clk-mt6735-infracfg.c    |  205 ++++
 drivers/clk/mediatek/clk-mt6735-pericfg.c     |  301 +++++
 drivers/clk/mediatek/clk-mt6735-topckgen.c    | 1087 +++++++++++++++++
 .../clock/mediatek,mt6735-apmixedsys.h        |   16 +
 .../clock/mediatek,mt6735-infracfg.h          |   25 +
 .../clock/mediatek,mt6735-pericfg.h           |   37 +
 .../clock/mediatek,mt6735-topckgen.h          |   79 ++
 .../reset/mediatek,mt6735-infracfg.h          |   31 +
 .../reset/mediatek,mt6735-pericfg.h           |   31 +
 17 files changed, 2085 insertions(+), 5 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-mt6735-apmixedsys.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-infracfg.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-pericfg.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-topckgen.c
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-infracfg.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-pericfg.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-topckgen.h
 create mode 100644 include/dt-bindings/reset/mediatek,mt6735-infracfg.h
 create mode 100644 include/dt-bindings/reset/mediatek,mt6735-pericfg.h

-- 
2.36.1

[PATCH v2 1/4] dt-bindings: clock: Add Mediatek MT6735 clock bindings

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add clock definitions for Mediatek MT6735 clocks provided by
apmixedsys, topckgen, infracfg and pericfg.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 MAINTAINERS                                   | 10 +++
 .../clock/mediatek,mt6735-apmixedsys.h        | 16 ++++
 .../clock/mediatek,mt6735-infracfg.h          | 25 ++++++
 .../clock/mediatek,mt6735-pericfg.h           | 37 +++++++++
 .../clock/mediatek,mt6735-topckgen.h          | 79 +++++++++++++++++++
 5 files changed, 167 insertions(+)
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-infracfg.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-pericfg.h
 create mode 100644 include/dt-bindings/clock/mediatek,mt6735-topckgen.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6516f9c6d28e..a59069263cfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12494,6 +12494,16 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/mmc/mtk-sd.yaml
 F:	drivers/mmc/host/mtk-sd.c
 
+MEDIATEK MT6735 CLOCK DRIVERS
+M:	Yassine Oudjana <y.oudjana@protonmail.com>
+L:	linux-clk@vger.kernel.org
+L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
+F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
+F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
+F:	include/dt-bindings/clock/mediatek,mt6735-topckgen.h
+
 MEDIATEK MT76 WIRELESS LAN DRIVER
 M:	Felix Fietkau <nbd@nbd.name>
 M:	Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h b/include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
new file mode 100644
index 000000000000..3dda719fd5d5
--- /dev/null
+++ b/include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLK_MT6735_APMIXEDSYS_H
+#define _DT_BINDINGS_CLK_MT6735_APMIXEDSYS_H
+
+#define ARMPLL				0
+#define MAINPLL				1
+#define UNIVPLL				2
+#define MMPLL				3
+#define MSDCPLL				4
+#define VENCPLL				5
+#define TVDPLL				6
+#define APLL1				7
+#define APLL2				8
+
+#endif
diff --git a/include/dt-bindings/clock/mediatek,mt6735-infracfg.h b/include/dt-bindings/clock/mediatek,mt6735-infracfg.h
new file mode 100644
index 000000000000..979a174ff8b6
--- /dev/null
+++ b/include/dt-bindings/clock/mediatek,mt6735-infracfg.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLK_MT6735_INFRACFG_H
+#define _DT_BINDINGS_CLK_MT6735_INFRACFG_H
+
+#define DBGCLK				0
+#define GCE				1
+#define TRBG				2
+#define CPUM				3
+#define DEVAPC				4
+#define AUDIO				5
+#define GCPU				6
+#define L2C_SRAM			7
+#define M4U				8
+#define CLDMA				9
+#define CONNMCU_BUS			10
+#define KP				11
+#define APXGPT				12
+#define SEJ				13
+#define CCIF0_AP			14
+#define CCIF1_AP			15
+#define PMIC_SPI			16
+#define PMIC_WRAP			17
+
+#endif
diff --git a/include/dt-bindings/clock/mediatek,mt6735-pericfg.h b/include/dt-bindings/clock/mediatek,mt6735-pericfg.h
new file mode 100644
index 000000000000..16f3c6a9a772
--- /dev/null
+++ b/include/dt-bindings/clock/mediatek,mt6735-pericfg.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLK_MT6735_PERICFG_H
+#define _DT_BINDINGS_CLK_MT6735_PERICFG_H
+
+#define DISP_PWM			0
+#define THERM				1
+#define PWM1				2
+#define PWM2				3
+#define PWM3				4
+#define PWM4				5
+#define PWM5				6
+#define PWM6				7
+#define PWM7				8
+#define PWM				9
+#define USB0				10
+#define IRDA				11
+#define APDMA				12
+#define MSDC30_0			13
+#define MSDC30_1			14
+#define MSDC30_2			15
+#define MSDC30_3			16
+#define UART0				17
+#define UART1				18
+#define UART2				19
+#define UART3				20
+#define UART4				21
+#define BTIF				22
+#define I2C0				23
+#define I2C1				24
+#define I2C2				25
+#define I2C3				26
+#define AUXADC				27
+#define SPI0				28
+#define IRTX				29
+
+#endif
diff --git a/include/dt-bindings/clock/mediatek,mt6735-topckgen.h b/include/dt-bindings/clock/mediatek,mt6735-topckgen.h
new file mode 100644
index 000000000000..a771910a4b8a
--- /dev/null
+++ b/include/dt-bindings/clock/mediatek,mt6735-topckgen.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLK_MT6735_TOPCKGEN_H
+#define _DT_BINDINGS_CLK_MT6735_TOPCKGEN_H
+
+#define AD_SYS_26M_CK			0
+#define CLKPH_MCK_O			1
+#define DMPLL				2
+#define DPI_CK				3
+#define WHPLL_AUDIO_CK			4
+
+#define SYSPLL_D2			5
+#define SYSPLL_D3			6
+#define SYSPLL_D5			7
+#define SYSPLL1_D2			8
+#define SYSPLL1_D4			9
+#define SYSPLL1_D8			10
+#define SYSPLL1_D16			11
+#define SYSPLL2_D2			12
+#define SYSPLL2_D4			13
+#define SYSPLL3_D2			14
+#define SYSPLL3_D4			15
+#define SYSPLL4_D2			16
+#define SYSPLL4_D4			17
+#define UNIVPLL_D2			18
+#define UNIVPLL_D3			19
+#define UNIVPLL_D5			20
+#define UNIVPLL_D26			21
+#define UNIVPLL1_D2			22
+#define UNIVPLL1_D4			23
+#define UNIVPLL1_D8			24
+#define UNIVPLL2_D2			25
+#define UNIVPLL2_D4			26
+#define UNIVPLL2_D8			27
+#define UNIVPLL3_D2			28
+#define UNIVPLL3_D4			29
+#define MSDCPLL_D2			30
+#define MSDCPLL_D4			31
+#define MSDCPLL_D8			32
+#define MSDCPLL_D16			33
+#define VENCPLL_D3			34
+#define TVDPLL_D2			35
+#define TVDPLL_D4			36
+#define DMPLL_D2			37
+#define DMPLL_D4			38
+#define DMPLL_D8			39
+#define AD_SYS_26M_D2			40
+
+#define AXI_SEL				41
+#define MEM_SEL				42
+#define DDRPHY_SEL			43
+#define MM_SEL				44
+#define PWM_SEL				45
+#define VDEC_SEL			46
+#define MFG_SEL				47
+#define CAMTG_SEL			48
+#define UART_SEL			49
+#define SPI_SEL				50
+#define USB20_SEL			51
+#define MSDC50_0_SEL			52
+#define MSDC30_0_SEL			53
+#define MSDC30_1_SEL			54
+#define MSDC30_2_SEL			55
+#define MSDC30_3_SEL			56
+#define AUDIO_SEL			57
+#define AUDINTBUS_SEL			58
+#define PMICSPI_SEL			59
+#define SCP_SEL				60
+#define ATB_SEL				61
+#define DPI0_SEL			62
+#define SCAM_SEL			63
+#define MFG13M_SEL			64
+#define AUD1_SEL			65
+#define AUD2_SEL			66
+#define IRDA_SEL			67
+#define IRTX_SEL			68
+#define DISPPWM_SEL			69
+
+#endif
-- 
2.36.1

[PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add reset definitions for Mediatek MT6735 resets provided by
infracfg and pericfg.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 MAINTAINERS                                   |  2 ++
 .../reset/mediatek,mt6735-infracfg.h          | 31 +++++++++++++++++++
 .../reset/mediatek,mt6735-pericfg.h           | 31 +++++++++++++++++++
 3 files changed, 64 insertions(+)
 create mode 100644 include/dt-bindings/reset/mediatek,mt6735-infracfg.h
 create mode 100644 include/dt-bindings/reset/mediatek,mt6735-pericfg.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a59069263cfb..1c0af554a7b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12503,6 +12503,8 @@ F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
 F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
 F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
 F:	include/dt-bindings/clock/mediatek,mt6735-topckgen.h
+F:	include/dt-bindings/reset/mediatek,mt6735-infracfg.h
+F:	include/dt-bindings/reset/mediatek,mt6735-pericfg.h
 
 MEDIATEK MT76 WIRELESS LAN DRIVER
 M:	Felix Fietkau <nbd@nbd.name>
diff --git a/include/dt-bindings/reset/mediatek,mt6735-infracfg.h b/include/dt-bindings/reset/mediatek,mt6735-infracfg.h
new file mode 100644
index 000000000000..86448f946568
--- /dev/null
+++ b/include/dt-bindings/reset/mediatek,mt6735-infracfg.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_RST_MT6735_INFRACFG_H
+#define _DT_BINDINGS_RST_MT6735_INFRACFG_H
+
+#define EMI_REG_RST			0
+#define DRAMC0_AO_RST			1
+#define AP_CIRQ_EINT_RST		3
+#define APXGPT_RST			4
+#define SCPSYS_RST			5
+#define KP_RST				6
+#define PMIC_WRAP_RST			7
+#define CLDMA_AO_TOP_RST		8
+#define EMI_RST				16
+#define CCIF_RST			17
+#define DRAMC0_RST			18
+#define EMI_AO_REG_RST			19
+#define CCIF_AO_RST			20
+#define TRNG_RST			21
+#define SYS_CIRQ_RST			22
+#define GCE_RST				23
+#define MM_IOMMU_RST			24
+#define CCIF1_RST			25
+#define CLDMA_TOP_PD_RST		26
+#define CBIP_P2P_MFG			27
+#define CBIP_P2P_APMIXED		28
+#define CBIP_P2P_CKSYS			29
+#define CBIP_P2P_MIPI			30
+#define CBIP_P2P_DDRPHY			31
+
+#endif
diff --git a/include/dt-bindings/reset/mediatek,mt6735-pericfg.h b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
new file mode 100644
index 000000000000..6cdfaa7ddadf
--- /dev/null
+++ b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_RST_MT6735_PERICFG_H
+#define _DT_BINDINGS_RST_MT6735_PERICFG_H
+
+#define UART0_SW_RST			0
+#define UART1_SW_RST			1
+#define UART2_SW_RST			2
+#define UART3_SW_RST			3
+#define UART4_SW_RST			4
+#define BTIF_SW_RST			6
+#define DISP_PWM_SW_RST			7
+#define PWM_SW_RST			8
+#define AUXADC_SW_RST			10
+#define DMA_SW_RST			11
+#define IRDA_SW_RST			12
+#define IRTX_SW_RST			13
+#define THERM_SW_RST			16
+#define MSDC2_SW_RST			17
+#define MSDC3_SW_RST			17
+#define MSDC0_SW_RST			19
+#define MSDC1_SW_RST			20
+#define I2C0_SW_RST			22
+#define I2C1_SW_RST			23
+#define I2C2_SW_RST			24
+#define I2C3_SW_RST			25
+#define USB_SW_RST			28
+
+#define SPI0_SW_RST			33
+
+#endif
-- 
2.36.1

Re: [PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings

[AngeloGioacchino Del Regno]

Il 19/05/22 16:22, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Add reset definitions for Mediatek MT6735 resets provided by
> infracfg and pericfg.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>   MAINTAINERS                                   |  2 ++
>   .../reset/mediatek,mt6735-infracfg.h          | 31 +++++++++++++++++++
>   .../reset/mediatek,mt6735-pericfg.h           | 31 +++++++++++++++++++
>   3 files changed, 64 insertions(+)
>   create mode 100644 include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>   create mode 100644 include/dt-bindings/reset/mediatek,mt6735-pericfg.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a59069263cfb..1c0af554a7b6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12503,6 +12503,8 @@ F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
>   F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
>   F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
>   F:	include/dt-bindings/clock/mediatek,mt6735-topckgen.h
> +F:	include/dt-bindings/reset/mediatek,mt6735-infracfg.h
> +F:	include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>   
>   MEDIATEK MT76 WIRELESS LAN DRIVER
>   M:	Felix Fietkau <nbd@nbd.name>

..snip..

> diff --git a/include/dt-bindings/reset/mediatek,mt6735-pericfg.h b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
> new file mode 100644
> index 000000000000..6cdfaa7ddadf
> --- /dev/null
> +++ b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_RST_MT6735_PERICFG_H
> +#define _DT_BINDINGS_RST_MT6735_PERICFG_H
> +
> +#define UART0_SW_RST			0
> +#define UART1_SW_RST			1
> +#define UART2_SW_RST			2
> +#define UART3_SW_RST			3
> +#define UART4_SW_RST			4

where's number 5?

> +#define BTIF_SW_RST			6
> +#define DISP_PWM_SW_RST			7
> +#define PWM_SW_RST			8

...and where's 9?

> +#define AUXADC_SW_RST			10
> +#define DMA_SW_RST			11
> +#define IRDA_SW_RST			12
> +#define IRTX_SW_RST			13

and 14, 15?

> +#define THERM_SW_RST			16
> +#define MSDC2_SW_RST			17
> +#define MSDC3_SW_RST			17

MSDC 2 and 3 are both 17?! :-)

> +#define MSDC0_SW_RST			19
> +#define MSDC1_SW_RST			20

21?

> +#define I2C0_SW_RST			22
> +#define I2C1_SW_RST			23
> +#define I2C2_SW_RST			24
> +#define I2C3_SW_RST			25
> +#define USB_SW_RST			28
> +

and 29-32?

> +#define SPI0_SW_RST			33
> +
> +#endif

I have a hunch that you've misunderstood the changes in the resets...

What Rex-BC has done in his reset cleanup is exactly to stop directly
mapping these to the actual bits that we're using... so the definitions
in there will simply be sequential, and the actual mapping is done in
your clk-mt6735-pericfg.c driver.

Is it clear? Does it make sense now? :-)

Cheers,
Angelo

Re: [PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings

[Yassine Oudjana]

On Fri, May 20 2022 at 10:55:24 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 16:22, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Add reset definitions for Mediatek MT6735 resets provided by
>> infracfg and pericfg.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   MAINTAINERS                                   |  2 ++
>>   .../reset/mediatek,mt6735-infracfg.h          | 31 
>> +++++++++++++++++++
>>   .../reset/mediatek,mt6735-pericfg.h           | 31 
>> +++++++++++++++++++
>>   3 files changed, 64 insertions(+)
>>   create mode 100644 
>> include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>>   create mode 100644 
>> include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a59069263cfb..1c0af554a7b6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -12503,6 +12503,8 @@ 
>> F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-topckgen.h
>> +F:	include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>> +F:	include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>   \x7f  MEDIATEK MT76 WIRELESS LAN DRIVER
>>   M:	Felix Fietkau <nbd@nbd.name>
> 
> ..snip..
> 
>> diff --git a/include/dt-bindings/reset/mediatek,mt6735-pericfg.h 
>> b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> new file mode 100644
>> index 000000000000..6cdfaa7ddadf
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> @@ -0,0 +1,31 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +
>> +#ifndef _DT_BINDINGS_RST_MT6735_PERICFG_H
>> +#define _DT_BINDINGS_RST_MT6735_PERICFG_H
>> +
>> +#define UART0_SW_RST			0
>> +#define UART1_SW_RST			1
>> +#define UART2_SW_RST			2
>> +#define UART3_SW_RST			3
>> +#define UART4_SW_RST			4
> 
> where's number 5?
> 
>> +#define BTIF_SW_RST			6
>> +#define DISP_PWM_SW_RST			7
>> +#define PWM_SW_RST			8
> 
> ...and where's 9?
> 
>> +#define AUXADC_SW_RST			10
>> +#define DMA_SW_RST			11
>> +#define IRDA_SW_RST			12
>> +#define IRTX_SW_RST			13
> 
> and 14, 15?
> 
>> +#define THERM_SW_RST			16
>> +#define MSDC2_SW_RST			17
>> +#define MSDC3_SW_RST			17
> 
> MSDC 2 and 3 are both 17?! :-)
> 
>> +#define MSDC0_SW_RST			19
>> +#define MSDC1_SW_RST			20
> 
> 21?
> 
>> +#define I2C0_SW_RST			22
>> +#define I2C1_SW_RST			23
>> +#define I2C2_SW_RST			24
>> +#define I2C3_SW_RST			25
>> +#define USB_SW_RST			28
>> +
> 
> and 29-32?
> 
>> +#define SPI0_SW_RST			33
>> +
>> +#endif
> 
> I have a hunch that you've misunderstood the changes in the resets...
> 
> What Rex-BC has done in his reset cleanup is exactly to stop directly
> mapping these to the actual bits that we're using... so the 
> definitions
> in there will simply be sequential, and the actual mapping is done in
> your clk-mt6735-pericfg.c driver.

I did notice that, but reading the documentation in reset.h:

 * @rst_idx_map:Pointer to an array containing ids if input argument is 
index.
 *		This array is not necessary if our input argument does not mean 
index.

I thought that it wasn't necessary to use it. Thinking
about it now however, I guess that was to maintain compatibility
with old device trees. I'll change it next time.
Maybe a note should be put there to avoid confusion
in the future.

Thanks,
Yassine

Re: [PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings

[AngeloGioacchino Del Regno]

Il 20/05/22 11:13, Yassine Oudjana ha scritto:
> 
> On Fri, May 20 2022 at 10:55:24 +0200, AngeloGioacchino Del Regno 
> <angelogioacchino.delregno@collabora.com> wrote:
>> Il 19/05/22 16:22, Yassine Oudjana ha scritto:
>>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>>
>>> Add reset definitions for Mediatek MT6735 resets provided by
>>> infracfg and pericfg.
>>>
>>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>>   MAINTAINERS                                   |  2 ++
>>>   .../reset/mediatek,mt6735-infracfg.h          | 31 +++++++++++++++++++
>>>   .../reset/mediatek,mt6735-pericfg.h           | 31 +++++++++++++++++++
>>>   3 files changed, 64 insertions(+)
>>>   create mode 100644 include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>>>   create mode 100644 include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index a59069263cfb..1c0af554a7b6 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -12503,6 +12503,8 @@ F:    
>>> include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
>>>   F:    include/dt-bindings/clock/mediatek,mt6735-infracfg.h
>>>   F:    include/dt-bindings/clock/mediatek,mt6735-pericfg.h
>>>   F:    include/dt-bindings/clock/mediatek,mt6735-topckgen.h
>>> +F:    include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>>> +F:    include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>>   \x7f  MEDIATEK MT76 WIRELESS LAN DRIVER
>>>   M:    Felix Fietkau <nbd@nbd.name>
>>
>> ..snip..
>>
>>> diff --git a/include/dt-bindings/reset/mediatek,mt6735-pericfg.h 
>>> b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>> new file mode 100644
>>> index 000000000000..6cdfaa7ddadf
>>> --- /dev/null
>>> +++ b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>> @@ -0,0 +1,31 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +
>>> +#ifndef _DT_BINDINGS_RST_MT6735_PERICFG_H
>>> +#define _DT_BINDINGS_RST_MT6735_PERICFG_H
>>> +
>>> +#define UART0_SW_RST            0
>>> +#define UART1_SW_RST            1
>>> +#define UART2_SW_RST            2
>>> +#define UART3_SW_RST            3
>>> +#define UART4_SW_RST            4
>>
>> where's number 5?
>>
>>> +#define BTIF_SW_RST            6
>>> +#define DISP_PWM_SW_RST            7
>>> +#define PWM_SW_RST            8
>>
>> ...and where's 9?
>>
>>> +#define AUXADC_SW_RST            10
>>> +#define DMA_SW_RST            11
>>> +#define IRDA_SW_RST            12
>>> +#define IRTX_SW_RST            13
>>
>> and 14, 15?
>>
>>> +#define THERM_SW_RST            16
>>> +#define MSDC2_SW_RST            17
>>> +#define MSDC3_SW_RST            17
>>
>> MSDC 2 and 3 are both 17?! :-)
>>
>>> +#define MSDC0_SW_RST            19
>>> +#define MSDC1_SW_RST            20
>>
>> 21?
>>
>>> +#define I2C0_SW_RST            22
>>> +#define I2C1_SW_RST            23
>>> +#define I2C2_SW_RST            24
>>> +#define I2C3_SW_RST            25
>>> +#define USB_SW_RST            28
>>> +
>>
>> and 29-32?
>>
>>> +#define SPI0_SW_RST            33
>>> +
>>> +#endif
>>
>> I have a hunch that you've misunderstood the changes in the resets...
>>
>> What Rex-BC has done in his reset cleanup is exactly to stop directly
>> mapping these to the actual bits that we're using... so the definitions
>> in there will simply be sequential, and the actual mapping is done in
>> your clk-mt6735-pericfg.c driver.
> 
> I did notice that, but reading the documentation in reset.h:
> 
> * @rst_idx_map:Pointer to an array containing ids if input argument is index.
> *        This array is not necessary if our input argument does not mean index.
> 
> I thought that it wasn't necessary to use it. Thinking
> about it now however, I guess that was to maintain compatibility
> with old device trees. I'll change it next time.
> Maybe a note should be put there to avoid confusion
> in the future.

I agree this comment should be clarified.

Thanks,
Angelo

[PATCH v2 3/4] dt-bindings: arm: mediatek: Add MT6735 clock controller compatibles

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add compatible strings for MT6735 apmixedsys, topckgen, infracfg
and pericfg.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/arm/mediatek/mediatek,infracfg.yaml          | 8 +++++---
 .../bindings/arm/mediatek/mediatek,pericfg.yaml           | 1 +
 .../devicetree/bindings/clock/mediatek,apmixedsys.yaml    | 4 +++-
 .../devicetree/bindings/clock/mediatek,topckgen.yaml      | 4 +++-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,infracfg.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,infracfg.yaml
index 8681b785ed6d..aa1bb13e0d67 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,infracfg.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,infracfg.yaml
@@ -11,9 +11,10 @@ maintainers:
 
 description:
   The Mediatek infracfg controller provides various clocks and reset outputs
-  to the system. The clock values can be found in <dt-bindings/clock/mt*-clk.h>,
-  and reset values in <dt-bindings/reset/mt*-reset.h> and
-  <dt-bindings/reset/mt*-resets.h>.
+  to the system. The clock values can be found in <dt-bindings/clock/mt*-clk.h>
+  and <dt-bindings/clock/mediatek,mt6735-infracfg.h>, and reset values in
+  <dt-bindings/reset/mt*-reset.h>, <dt-bindings/reset/mt*-resets.h> and
+  <dt-bindings/reset/mediatek,mt6735-infracfg.h>.
 
 properties:
   compatible:
@@ -22,6 +23,7 @@ properties:
           - enum:
               - mediatek,mt2701-infracfg
               - mediatek,mt2712-infracfg
+              - mediatek,mt6735-infracfg
               - mediatek,mt6765-infracfg
               - mediatek,mt6779-infracfg_ao
               - mediatek,mt6797-infracfg
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,pericfg.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,pericfg.yaml
index 611f666f359d..94e5e003e60e 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,pericfg.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,pericfg.yaml
@@ -20,6 +20,7 @@ properties:
           - enum:
               - mediatek,mt2701-pericfg
               - mediatek,mt2712-pericfg
+              - mediatek,mt6735-pericfg
               - mediatek,mt6765-pericfg
               - mediatek,mt7622-pericfg
               - mediatek,mt7629-pericfg
diff --git a/Documentation/devicetree/bindings/clock/mediatek,apmixedsys.yaml b/Documentation/devicetree/bindings/clock/mediatek,apmixedsys.yaml
index 770546195fb5..3a186621e7a9 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,apmixedsys.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,apmixedsys.yaml
@@ -12,7 +12,8 @@ maintainers:
 
 description:
   The Mediatek apmixedsys controller provides PLLs to the system.
-  The clock values can be found in <dt-bindings/clock/mt*-clk.h>.
+  The clock values can be found in <dt-bindings/clock/mt*-clk.h>
+  and <dt-bindings/clock/mediatek,mt6735-apmixedsys.h>.
 
 properties:
   compatible:
@@ -32,6 +33,7 @@ properties:
           - enum:
               - mediatek,mt2701-apmixedsys
               - mediatek,mt2712-apmixedsys
+              - mediatek,mt6735-apmixedsys
               - mediatek,mt6765-apmixedsys
               - mediatek,mt6779-apmixedsys
               - mediatek,mt7629-apmixedsys
diff --git a/Documentation/devicetree/bindings/clock/mediatek,topckgen.yaml b/Documentation/devicetree/bindings/clock/mediatek,topckgen.yaml
index 5b8b37a2e594..920bf0828d58 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,topckgen.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,topckgen.yaml
@@ -12,7 +12,8 @@ maintainers:
 
 description:
   The Mediatek topckgen controller provides various clocks to the system.
-  The clock values can be found in <dt-bindings/clock/mt*-clk.h>.
+  The clock values can be found in <dt-bindings/clock/mt*-clk.h> and
+  <dt-bindings/clock/mediatek,mt6735-topckgen.h>.
 
 properties:
   compatible:
@@ -31,6 +32,7 @@ properties:
           - enum:
               - mediatek,mt2701-topckgen
               - mediatek,mt2712-topckgen
+              - mediatek,mt6735-topckgen
               - mediatek,mt6765-topckgen
               - mediatek,mt6779-topckgen
               - mediatek,mt7629-topckgen
-- 
2.36.1

[PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
clock and reset controllers. These provide the base clocks on the
platform, and should be enough to bring up all essential blocks
including PWRAP, MSDC and peripherals (UART, I2C, SPI).

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
- clk: mediatek: Improvements to simple probe/remove and reset controller unregistration
  https://patchwork.kernel.org/project/linux-clk/cover/20220519134728.456643-1-y.oudjana@protonmail.com/

 MAINTAINERS                                  |    4 +
 drivers/clk/mediatek/Kconfig                 |    9 +
 drivers/clk/mediatek/Makefile                |    1 +
 drivers/clk/mediatek/clk-mt6735-apmixedsys.c |  235 ++++
 drivers/clk/mediatek/clk-mt6735-infracfg.c   |  205 ++++
 drivers/clk/mediatek/clk-mt6735-pericfg.c    |  301 +++++
 drivers/clk/mediatek/clk-mt6735-topckgen.c   | 1087 ++++++++++++++++++
 7 files changed, 1842 insertions(+)
 create mode 100644 drivers/clk/mediatek/clk-mt6735-apmixedsys.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-infracfg.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-pericfg.c
 create mode 100644 drivers/clk/mediatek/clk-mt6735-topckgen.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c0af554a7b6..65f7c95bba9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12499,6 +12499,10 @@ M:	Yassine Oudjana <y.oudjana@protonmail.com>
 L:	linux-clk@vger.kernel.org
 L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
+F:	drivers/clk/mediatek/clk-mt6735-apmixedsys.c
+F:	drivers/clk/mediatek/clk-mt6735-infracfg.c
+F:	drivers/clk/mediatek/clk-mt6735-pericfg.c
+F:	drivers/clk/mediatek/clk-mt6735-topckgen.c
 F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
 F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
 F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index d5936cfb3bee..2d2d51c9829e 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -117,6 +117,15 @@ config COMMON_CLK_MT2712_VENCSYS
 	help
 	  This driver supports MediaTek MT2712 vencsys clocks.
 
+config COMMON_CLK_MT6735
+	tristate "Main clock drivers for MediaTek MT6735"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	select COMMON_CLK_MEDIATEK
+	help
+	  This enables drivers for clocks and resets provided
+	  by apmixedsys, topckgen, infracfg and pericfg on the
+	  MediaTek MT6735 SoC.
+
 config COMMON_CLK_MT6765
        bool "Clock driver for MediaTek MT6765"
        depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index caf2ce93d666..45530dae64a2 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o
 
+obj-$(CONFIG_COMMON_CLK_MT6735) += clk-mt6735-apmixedsys.o clk-mt6735-infracfg.o clk-mt6735-pericfg.o clk-mt6735-topckgen.o
 obj-$(CONFIG_COMMON_CLK_MT6765) += clk-mt6765.o
 obj-$(CONFIG_COMMON_CLK_MT6765_AUDIOSYS) += clk-mt6765-audio.o
 obj-$(CONFIG_COMMON_CLK_MT6765_CAMSYS) += clk-mt6765-cam.o
diff --git a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
new file mode 100644
index 000000000000..65afbe8d38fa
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "clk-mtk.h"
+#include "clk-pll.h"
+
+#include <dt-bindings/clock/mediatek,mt6735-apmixedsys.h>
+
+#define AP_PLL_CON_5		0x014
+#define ARMPLL_CON0		0x200
+#define ARMPLL_CON1		0x204
+#define ARMPLL_PWR_CON0		0x20c
+#define MAINPLL_CON0		0x210
+#define MAINPLL_CON1		0x214
+#define MAINPLL_PWR_CON0	0x21c
+#define UNIVPLL_CON0		0x220
+#define UNIVPLL_CON1		0x224
+#define UNIVPLL_PWR_CON0	0x22c
+#define MMPLL_CON0		0x230
+#define MMPLL_CON1		0x234
+#define MMPLL_PWR_CON0		0x23c
+#define MSDCPLL_CON0		0x240
+#define MSDCPLL_CON1		0x244
+#define MSDCPLL_PWR_CON0	0x24c
+#define VENCPLL_CON0		0x250
+#define VENCPLL_CON1		0x254
+#define VENCPLL_PWR_CON0	0x25c
+#define TVDPLL_CON0		0x260
+#define TVDPLL_CON1		0x264
+#define TVDPLL_PWR_CON0		0x26c
+#define APLL1_CON0		0x270
+#define APLL1_CON1		0x274
+#define APLL1_CON2		0x278
+#define APLL1_PWR_CON0		0x280
+#define APLL2_CON0		0x284
+#define APLL2_CON1		0x288
+#define APLL2_CON2		0x28c
+#define APLL2_PWR_CON0		0x294
+
+#define CON0_RST_BAR		BIT(24)
+
+static const struct mtk_pll_data apmixedsys_plls[] = {
+	{
+		.id = ARMPLL,
+		.name = "armpll",
+		.parent_name = "clk26m",
+
+		.reg = ARMPLL_CON0,
+		.pwr_reg = ARMPLL_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = ARMPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = ARMPLL_CON1,
+		.pcw_chg_reg = ARMPLL_CON1,
+		.pcwbits = 21,
+
+		.flags = PLL_AO
+	},
+	{
+		.id = MAINPLL,
+		.name = "mainpll",
+		.parent_name = "clk26m",
+
+		.reg = MAINPLL_CON0,
+		.pwr_reg = MAINPLL_PWR_CON0,
+		.en_mask = 0xf0000101,
+
+		.pd_reg = MAINPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = MAINPLL_CON1,
+		.pcw_chg_reg = MAINPLL_CON1,
+		.pcwbits = 21,
+
+		.flags = HAVE_RST_BAR,
+		.rst_bar_mask = CON0_RST_BAR
+	},
+	{
+		.id = UNIVPLL,
+		.name = "univpll",
+		.parent_name = "clk26m",
+
+		.reg = UNIVPLL_CON0,
+		.pwr_reg = UNIVPLL_PWR_CON0,
+		.en_mask = 0xfc000001,
+
+		.pd_reg = UNIVPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = UNIVPLL_CON1,
+		.pcw_chg_reg = UNIVPLL_CON1,
+		.pcwbits = 21,
+
+		.flags = HAVE_RST_BAR,
+		.rst_bar_mask = CON0_RST_BAR
+	},
+	{
+		.id = MMPLL,
+		.name = "mmpll",
+		.parent_name = "clk26m",
+
+		.reg = MMPLL_CON0,
+		.pwr_reg = MMPLL_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = MMPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = MMPLL_CON1,
+		.pcw_chg_reg = MMPLL_CON1,
+		.pcwbits = 21
+	},
+	{
+		.id = MSDCPLL,
+		.name = "msdcpll",
+		.parent_name = "clk26m",
+
+		.reg = MSDCPLL_CON0,
+		.pwr_reg = MSDCPLL_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = MSDCPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = MSDCPLL_CON1,
+		.pcw_chg_reg = MSDCPLL_CON1,
+		.pcwbits = 21,
+	},
+	{
+		.id = VENCPLL,
+		.name = "vencpll",
+		.parent_name = "clk26m",
+
+		.reg = VENCPLL_CON0,
+		.pwr_reg = VENCPLL_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = VENCPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = VENCPLL_CON1,
+		.pcw_chg_reg = VENCPLL_CON1,
+		.pcwbits = 21,
+
+		.flags = HAVE_RST_BAR,
+		.rst_bar_mask = CON0_RST_BAR
+	},
+	{
+		.id = TVDPLL,
+		.name = "tvdpll",
+		.parent_name = "clk26m",
+
+		.reg = TVDPLL_CON0,
+		.pwr_reg = TVDPLL_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = TVDPLL_CON1,
+		.pd_shift = 24,
+
+		.pcw_reg = TVDPLL_CON1,
+		.pcw_chg_reg = TVDPLL_CON1,
+		.pcwbits = 21
+	},
+	{
+		.id = APLL1,
+		.name = "apll1",
+		.parent_name = "clk26m",
+
+		.reg = APLL1_CON0,
+		.pwr_reg = APLL1_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = APLL1_CON0,
+		.pd_shift = 4,
+
+		.pcw_reg = APLL1_CON1,
+		.pcw_chg_reg = APLL1_CON1,
+		.pcwbits = 31,
+
+		.tuner_reg = APLL1_CON2,
+		.tuner_en_reg = AP_PLL_CON_5,
+		.tuner_en_bit = 0
+	},
+	{
+		.id = APLL2,
+		.name = "apll2",
+		.parent_name = "clk26m",
+
+		.reg = APLL2_CON0,
+		.pwr_reg = APLL2_PWR_CON0,
+		.en_mask = 0x00000001,
+
+		.pd_reg = APLL2_CON0,
+		.pd_shift = 4,
+
+		.pcw_reg = APLL2_CON1,
+		.pcw_chg_reg = APLL2_CON1,
+		.pcwbits = 31,
+
+		.tuner_reg = APLL1_CON2,
+		.tuner_en_reg = AP_PLL_CON_5,
+		.tuner_en_bit = 1
+	}
+};
+
+static const struct mtk_clk_desc apmixedsys_clks = {
+	.plls = apmixedsys_plls,
+	.num_plls = ARRAY_SIZE(apmixedsys_plls)
+};
+
+static const struct of_device_id of_match_mt6735_apmixedsys[] = {
+	{ .compatible = "mediatek,mt6735-apmixedsys", .data = &apmixedsys_clks },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver clk_mt6735_apmixedsys = {
+	.probe = mtk_clk_simple_probe,
+	.remove = mtk_clk_simple_remove,
+	.driver = {
+		.name = "clk-mt6735-apmixedsys",
+		.of_match_table = of_match_mt6735_apmixedsys,
+	},
+};
+module_platform_driver(clk_mt6735_apmixedsys);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Mediatek MT6735 apmixedsys clock driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-mt6735-infracfg.c b/drivers/clk/mediatek/clk-mt6735-infracfg.c
new file mode 100644
index 000000000000..37cf64a192ab
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt6735-infracfg.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "clk-gate.h"
+#include "clk-mtk.h"
+
+#include <dt-bindings/clock/mediatek,mt6735-infracfg.h>
+
+#define INFRA_RST0			0x30
+#define INFRA_GLOBALCON_PDN0		0x40
+#define INFRA_PDN1			0x44
+#define	INFRA_PDN_STA			0x48
+
+static struct mtk_gate_regs infra_cg_regs = {
+	.set_ofs = INFRA_GLOBALCON_PDN0,
+	.clr_ofs = INFRA_PDN1,
+	.sta_ofs = INFRA_PDN_STA,
+};
+
+static const struct mtk_gate infracfg_gates[] = {
+	{
+		.id = DBGCLK,
+		.name = "dbgclk",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 0,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = GCE,
+		.name = "gce",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 1,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = TRBG,
+		.name = "trbg",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 2,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = CPUM,
+		.name = "cpum",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 3,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = DEVAPC,
+		.name = "devapc",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 4,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = AUDIO,
+		.name = "audio",
+		.parent_name = "aud_intbus_sel",
+		.regs = &infra_cg_regs,
+		.shift = 5,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = GCPU,
+		.name = "gcpu",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 6,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = L2C_SRAM,
+		.name = "l2csram",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 7,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = M4U,
+		.name = "m4u",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 8,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = CLDMA,
+		.name = "cldma",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 12,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = CONNMCU_BUS,
+		.name = "connmcu_bus",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 15,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = KP,
+		.name = "kp",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 16,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = APXGPT,
+		.name = "apxgpt",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 18,
+		.ops = &mtk_clk_gate_ops_setclr,
+		.flags = CLK_IS_CRITICAL
+	},
+	{
+		.id = SEJ,
+		.name = "sej",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 19,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = CCIF0_AP,
+		.name = "ccif0ap",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 20,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = CCIF1_AP,
+		.name = "ccif1ap",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 21,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PMIC_SPI,
+		.name = "pmicspi",
+		.parent_name = "pmicspi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 22,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PMIC_WRAP,
+		.name = "pmicwrap",
+		.parent_name = "axi_sel",
+		.regs = &infra_cg_regs,
+		.shift = 23,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+};
+
+static u16 infracfg_rst_ofs[] = { INFRA_RST0 };
+
+static const struct mtk_clk_rst_desc infracfg_resets = {
+	.version = MTK_RST_SET_CLR,
+	.rst_bank_ofs = infracfg_rst_ofs,
+	.rst_bank_nr = ARRAY_SIZE(infracfg_rst_ofs)
+};
+
+static const struct mtk_clk_desc infracfg_clks = {
+	.gates = infracfg_gates,
+	.num_gates = ARRAY_SIZE(infracfg_gates),
+
+	.rst_desc = &infracfg_resets
+};
+
+static const struct of_device_id of_match_mt6735_infracfg[] = {
+	{ .compatible = "mediatek,mt6735-infracfg", .data = &infracfg_clks },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver clk_mt6735_infracfg = {
+	.probe = mtk_clk_simple_probe,
+	.remove = mtk_clk_simple_remove,
+	.driver = {
+		.name = "clk-mt6735-infracfg",
+		.of_match_table = of_match_mt6735_infracfg,
+	},
+};
+module_platform_driver(clk_mt6735_infracfg);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Mediatek MT6735 infracfg clock and reset driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-mt6735-pericfg.c b/drivers/clk/mediatek/clk-mt6735-pericfg.c
new file mode 100644
index 000000000000..6ec987197a22
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt6735-pericfg.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "clk-gate.h"
+#include "clk-mtk.h"
+
+#include <dt-bindings/clock/mediatek,mt6735-pericfg.h>
+
+#define PERI_GLOBALCON_RST0		0x00
+#define PERI_GLOBALCON_RST1		0x04
+#define PERI_GLOBALCON_PDN0_SET		0x08
+#define PERI_GLOBALCON_PDN0_CLR		0x10
+#define	PERI_GLOBALCON_PDN0_STA		0x18
+
+static struct mtk_gate_regs peri_cg_regs = {
+	.set_ofs = PERI_GLOBALCON_PDN0_SET,
+	.clr_ofs = PERI_GLOBALCON_PDN0_CLR,
+	.sta_ofs = PERI_GLOBALCON_PDN0_STA,
+};
+
+static const struct mtk_gate pericfg_gates[] = {
+	{
+		.id = DISP_PWM,
+		.name = "disp_pwm",
+		.parent_name = "disppwm_sel",
+		.regs = &peri_cg_regs,
+		.shift = 0,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = THERM,
+		.name = "therm",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 1,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM1,
+		.name = "pwm1",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 2,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM2,
+		.name = "pwm2",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 3,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM3,
+		.name = "pwm3",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 4,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM4,
+		.name = "pwm4",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 5,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM5,
+		.name = "pwm5",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 6,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM6,
+		.name = "pwm6",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 7,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM7,
+		.name = "pwm7",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 8,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = PWM,
+		.name = "pwm",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 9,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = USB0,
+		.name = "usb0",
+		.parent_name = "usb20_sel",
+		.regs = &peri_cg_regs,
+		.shift = 10,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = IRDA,
+		.name = "irda",
+		.parent_name = "irda_sel",
+		.regs = &peri_cg_regs,
+		.shift = 11,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = APDMA,
+		.name = "apdma",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 12,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = MSDC30_0,
+		.name = "msdc30_0",
+		.parent_name = "msdc30_0_sel",
+		.regs = &peri_cg_regs,
+		.shift = 13,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = MSDC30_1,
+		.name = "msdc30_1",
+		.parent_name = "msdc30_1_sel",
+		.regs = &peri_cg_regs,
+		.shift = 14,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = MSDC30_2,
+		.name = "msdc30_2",
+		.parent_name = "msdc30_2_sel",
+		.regs = &peri_cg_regs,
+		.shift = 15,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = MSDC30_3,
+		.name = "msdc30_3",
+		.parent_name = "msdc30_3_sel",
+		.regs = &peri_cg_regs,
+		.shift = 16,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = UART0,
+		.name = "uart0",
+		.parent_name = "uart_sel",
+		.regs = &peri_cg_regs,
+		.shift = 17,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = UART1,
+		.name = "uart1",
+		.parent_name = "uart_sel",
+		.regs = &peri_cg_regs,
+		.shift = 18,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = UART2,
+		.name = "uart2",
+		.parent_name = "uart_sel",
+		.regs = &peri_cg_regs,
+		.shift = 19,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = UART3,
+		.name = "uart3",
+		.parent_name = "uart_sel",
+		.regs = &peri_cg_regs,
+		.shift = 20,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = UART4,
+		.name = "uart4",
+		.parent_name = "uart_sel",
+		.regs = &peri_cg_regs,
+		.shift = 21,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = BTIF,
+		.name = "btif",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 22,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = I2C0,
+		.name = "i2c0",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 23,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = I2C1,
+		.name = "i2c1",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 24,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = I2C2,
+		.name = "i2c2",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 25,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = I2C3,
+		.name = "i2c3",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 26,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = AUXADC,
+		.name = "auxadc",
+		.parent_name = "axi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 27,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = SPI0,
+		.name = "spi0",
+		.parent_name = "spi_sel",
+		.regs = &peri_cg_regs,
+		.shift = 28,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+	{
+		.id = IRTX,
+		.name = "irtx",
+		.parent_name = "irtx_sel",
+		.regs = &peri_cg_regs,
+		.shift = 29,
+		.ops = &mtk_clk_gate_ops_setclr
+	},
+};
+
+static u16 pericfg_rst_ofs[] = { PERI_GLOBALCON_RST0, PERI_GLOBALCON_RST1 };
+
+static const struct mtk_clk_rst_desc pericfg_resets = {
+	.version = MTK_RST_SIMPLE,
+	.rst_bank_ofs = pericfg_rst_ofs,
+	.rst_bank_nr = ARRAY_SIZE(pericfg_rst_ofs)
+};
+
+static const struct mtk_clk_desc pericfg_clks = {
+	.gates = pericfg_gates,
+	.num_gates = ARRAY_SIZE(pericfg_gates),
+
+	.rst_desc = &pericfg_resets
+};
+
+static const struct of_device_id of_match_mt6735_pericfg[] = {
+	{ .compatible = "mediatek,mt6735-pericfg", .data = &pericfg_clks },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver clk_mt6735_pericfg = {
+	.probe = mtk_clk_simple_probe,
+	.remove = mtk_clk_simple_remove,
+	.driver = {
+		.name = "clk-mt6735-pericfg",
+		.of_match_table = of_match_mt6735_pericfg,
+	},
+};
+module_platform_driver(clk_mt6735_pericfg);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Mediatek MT6735 pericfg clock driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-mt6735-topckgen.c b/drivers/clk/mediatek/clk-mt6735-topckgen.c
new file mode 100644
index 000000000000..4540bbd2cfcd
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt6735-topckgen.c
@@ -0,0 +1,1087 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "clk-mtk.h"
+#include "clk-mux.h"
+
+#include <dt-bindings/clock/mediatek,mt6735-topckgen.h>
+
+#define CLK_CFG_0		0x40
+#define CLK_CFG_0_SET		0x44
+#define CLK_CFG_0_CLR		0x48
+#define CLK_CFG_1		0x50
+#define CLK_CFG_1_SET		0x54
+#define CLK_CFG_1_CLR		0x58
+#define CLK_CFG_2		0x60
+#define CLK_CFG_2_SET		0x64
+#define CLK_CFG_2_CLR		0x68
+#define CLK_CFG_3		0x70
+#define CLK_CFG_3_SET		0x74
+#define CLK_CFG_3_CLR		0x78
+#define CLK_CFG_4		0x80
+#define CLK_CFG_4_SET		0x84
+#define CLK_CFG_4_CLR		0x88
+#define CLK_CFG_5		0x90
+#define CLK_CFG_5_SET		0x94
+#define CLK_CFG_5_CLR		0x98
+#define CLK_CFG_6		0xa0
+#define CLK_CFG_6_SET		0xa4
+#define CLK_CFG_6_CLR		0xa8
+#define CLK_CFG_7		0xb0
+#define CLK_CFG_7_SET		0xb4
+#define CLK_CFG_7_CLR		0xb8
+
+/* Some clocks with unknown details are modeled as fixed clocks */
+static const struct mtk_fixed_clk topckgen_fixed_clks[] = {
+	{
+		/*
+		 * This clock is available as a parent option for multiple
+		 * muxes and seems like an alternative name for clk26m at first,
+		 * but it appears alongside it in several muxes which should
+		 * mean it is a separate clock.
+		 */
+		.id = AD_SYS_26M_CK,
+		.name = "ad_sys_26m_ck",
+		.parent = "clk26m",
+		.rate = 26 * MHZ,
+	},
+	{
+		/*
+		 * This clock is the parent of DMPLL divisors. It might be MEMPLL
+		 * or its parent, as DMPLL appears to be an alternative name for
+		 * MEMPLL.
+		 */
+		.id = CLKPH_MCK_O,
+		.name = "clkph_mck_o",
+		.parent = NULL
+	},
+	{
+		/*
+		 * DMPLL clock (dmpll_ck), controlled by DDRPHY.
+		 */
+		.id = DMPLL,
+		.name = "dmpll",
+		.parent = "clkph_mck_o"
+	},
+	{
+		/*
+		 * MIPI DPI clock. Parent option for dpi0_sel. Unknown parent.
+		 */
+		.id = DPI_CK,
+		.name = "dpi_ck",
+		.parent = NULL
+	},
+	{
+		/*
+		 * This clock is a child of WHPLL which is controlled by
+		 * the modem.
+		 */
+		.id = WHPLL_AUDIO_CK,
+		.name = "whpll_audio_ck",
+		.parent = NULL
+	},
+};
+
+static const struct mtk_fixed_factor topckgen_factors[] = {
+	{
+		.id = SYSPLL_D2,
+		.name = "syspll_d2",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = SYSPLL_D3,
+		.name = "syspll_d3",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 3
+	},
+	{
+		.id = SYSPLL_D5,
+		.name = "syspll_d5",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 5
+	},
+	{
+		.id = SYSPLL1_D2,
+		.name = "syspll1_d2",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = SYSPLL1_D4,
+		.name = "syspll1_d4",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = SYSPLL1_D8,
+		.name = "syspll1_d8",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 8
+	},
+	{
+		.id = SYSPLL1_D16,
+		.name = "syspll1_d16",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 16
+	},
+	{
+		.id = SYSPLL2_D2,
+		.name = "syspll2_d2",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = SYSPLL2_D4,
+		.name = "syspll2_d4",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = SYSPLL3_D2,
+		.name = "syspll3_d2",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = SYSPLL3_D4,
+		.name = "syspll3_d4",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = SYSPLL4_D2,
+		.name = "syspll4_d2",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = SYSPLL4_D4,
+		.name = "syspll4_d4",
+		.parent_name = "mainpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = UNIVPLL_D2,
+		.name = "univpll_d2",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = UNIVPLL_D3,
+		.name = "univpll_d3",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 3
+	},
+	{
+		.id = UNIVPLL_D5,
+		.name = "univpll_d5",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 5
+	},
+	{
+		.id = UNIVPLL_D26,
+		.name = "univpll_d26",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 26
+	},
+	{
+		.id = UNIVPLL1_D2,
+		.name = "univpll1_d2",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = UNIVPLL1_D4,
+		.name = "univpll1_d4",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = UNIVPLL1_D8,
+		.name = "univpll1_d8",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 8
+	},
+	{
+		.id = UNIVPLL2_D2,
+		.name = "univpll2_d2",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = UNIVPLL2_D4,
+		.name = "univpll2_d4",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = UNIVPLL2_D8,
+		.name = "univpll2_d8",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 8
+	},
+	{
+		.id = UNIVPLL3_D2,
+		.name = "univpll3_d2",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = UNIVPLL3_D4,
+		.name = "univpll3_d4",
+		.parent_name = "univpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = MSDCPLL_D2,
+		.name = "msdcpll_d2",
+		.parent_name = "msdcpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = MSDCPLL_D4,
+		.name = "msdcpll_d4",
+		.parent_name = "msdcpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = MSDCPLL_D8,
+		.name = "msdcpll_d8",
+		.parent_name = "msdcpll",
+		.mult = 1,
+		.div = 8
+	},
+	{
+		.id = MSDCPLL_D16,
+		.name = "msdcpll_d16",
+		.parent_name = "msdcpll",
+		.mult = 1,
+		.div = 16
+	},
+	{
+		.id = VENCPLL_D3,
+		.name = "vencpll_d3",
+		.parent_name = "vencpll",
+		.mult = 1,
+		.div = 3
+	},
+	{
+		.id = TVDPLL_D2,
+		.name = "tvdpll_d2",
+		.parent_name = "tvdpll",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = TVDPLL_D4,
+		.name = "tvdpll_d4",
+		.parent_name = "tvdpll",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = DMPLL_D2,
+		.name = "dmpll_d2",
+		.parent_name = "clkph_mck_o",
+		.mult = 1,
+		.div = 2
+	},
+	{
+		.id = DMPLL_D4,
+		.name = "dmpll_d4",
+		.parent_name = "clkph_mck_o",
+		.mult = 1,
+		.div = 4
+	},
+	{
+		.id = DMPLL_D8,
+		.name = "dmpll_d8",
+		.parent_name = "clkph_mck_o",
+		.mult = 1,
+		.div = 8
+	},
+	{
+		.id = AD_SYS_26M_D2,
+		.name = "ad_sys_26m_d2",
+		.parent_name = "clk26m",
+		.mult = 1,
+		.div = 2
+	},
+};
+
+static const char * const axi_sel_parents[] = {
+	"clk26m",
+	"syspll1_d2",
+	"syspll_d5",
+	"syspll1_d4",
+	"univpll_d5",
+	"univpll2_d2",
+	"dmpll",
+	"dmpll_d2"
+};
+
+static const char * const mem_sel_parents[] = {
+	"clk26m",
+	"dmpll"
+};
+
+static const char * const ddrphycfg_parents[] = {
+	"clk26m",
+	"syspll1_d8"
+};
+
+static const char * const mm_sel_parents[] = {
+	"clk26m",
+	"vencpll",
+	"syspll1_d2",
+	"syspll_d5",
+	"syspll1_d4",
+	"univpll_d5",
+	"univpll2_d2",
+	"dmpll"
+};
+
+static const char * const pwm_sel_parents[] = {
+	"clk26m",
+	"univpll2_d4",
+	"univpll3_d2",
+	"univpll1_d4"
+};
+
+static const char * const vdec_sel_parents[] = {
+	"clk26m",
+	"syspll1_d2",
+	"syspll_d5",
+	"syspll1_d4",
+	"univpll_d5",
+	"syspll_d2",
+	"syspll2_d2",
+	"msdcpll_d2"
+};
+
+static const char * const mfg_sel_parents[] = {
+	"clk26m",
+	"mmpll",
+	"clk26m",
+	"clk26m",
+	"clk26m",
+	"clk26m",
+	"clk26m",
+	"clk26m",
+	"clk26m",
+	"syspll_d3",
+	"syspll1_d2",
+	"syspll_d5",
+	"univpll_d3",
+	"univpll1_d2"
+};
+
+static const char * const camtg_sel_parents[] = {
+	"clk26m",
+	"univpll_d26",
+	"univpll2_d2",
+	"syspll3_d2",
+	"syspll3_d4",
+	"msdcpll_d4"
+};
+
+static const char * const uart_sel_parents[] = {
+	"clk26m",
+	"univpll2_d8"
+};
+
+static const char * const spi_sel_parents[] = {
+	"clk26m",
+	"syspll3_d2",
+	"msdcpll_d8",
+	"syspll2_d4",
+	"syspll4_d2",
+	"univpll2_d4",
+	"univpll1_d8"
+};
+
+static const char * const usb20_sel_parents[] = {
+	"clk26m",
+	"univpll1_d8",
+	"univpll3_d4"
+};
+
+static const char * const msdc50_0_sel_parents[] = {
+	"clk26m",
+	"syspll1_d2",
+	"syspll2_d2",
+	"syspll4_d2",
+	"univpll_d5",
+	"univpll1_d4"
+};
+
+static const char * const msdc30_0_sel_parents[] = {
+	"clk26m",
+	"msdcpll",
+	"msdcpll_d2",
+	"msdcpll_d4",
+	"syspll2_d2",
+	"syspll1_d4",
+	"univpll1_d4",
+	"univpll_d3",
+	"univpll_d26",
+	"syspll2_d4",
+	"univpll_d2"
+};
+
+static const char * const msdc30_1_2_sel_parents[] = {
+	"clk26m",
+	"univpll2_d2",
+	"msdcpll_d4",
+	"syspll2_d2",
+	"syspll1_d4",
+	"univpll1_d4",
+	"univpll_d26",
+	"syspll2_d4"
+};
+
+static const char * const msdc30_3_sel_parents[] = {
+	"clk26m",
+	"univpll2_d2",
+	"msdcpll_d4",
+	"syspll2_d2",
+	"syspll1_d4",
+	"univpll1_d4",
+	"univpll_d26",
+	"msdcpll_d16",
+	"syspll2_d4"
+};
+
+static const char * const audio_sel_parents[] = {
+	"clk26m",
+	"syspll3_d4",
+	"syspll4_d4",
+	"syspll1_d16"
+};
+
+static const char * const aud_intbus_sel_parents[] = {
+	"clk26m",
+	"syspll1_d4",
+	"syspll4_d2",
+	"dmpll_d4"
+};
+
+static const char * const pmicspi_sel_parents[] = {
+	"clk26m",
+	"syspll1_d8",
+	"syspll3_d4",
+	"syspll1_d16",
+	"univpll3_d4",
+	"univpll_d26",
+	"dmpll_d4",
+	"dmpll_d8"
+};
+
+static const char * const scp_sel_parents[] = {
+	"clk26m",
+	"syspll1_d8",
+	"dmpll_d2",
+	"dmpll_d4"
+};
+
+static const char * const atb_sel_parents[] = {
+	"clk26m",
+	"syspll1_d2",
+	"syspll_d5",
+	"dmpll"
+};
+
+static const char * const dpi0_sel_parents[] = {
+	"clk26m",
+	"tvdpll",
+	"tvdpll_d2",
+	"tvdpll_d4",
+	"dpi_ck"
+};
+
+static const char * const scam_sel_parents[] = {
+	"clk26m",
+	"syspll3_d2",
+	"univpll2_d4",
+	"vencpll_d3"
+};
+
+static const char * const mfg13m_sel_parents[] = {
+	"clk26m",
+	"ad_sys_26m_d2"
+};
+
+static const char * const aud_1_2_sel_parents[] = {
+	"clk26m",
+	"apll1"
+};
+
+static const char * const irda_sel_parents[] = {
+	"clk26m",
+	"univpll2_d4"
+};
+
+static const char * const irtx_sel_parents[] = {
+	"clk26m",
+	"ad_sys_26m_ck"
+};
+
+static const char * const disppwm_sel_parents[] = {
+	"clk26m",
+	"univpll2_d4",
+	"syspll4_d2_d8",
+	"ad_sys_26m_ck"
+};
+
+static const struct mtk_mux topckgen_muxes[] = {
+	{
+		.id = AXI_SEL,
+		.name = "axi_sel",
+		.parent_names = axi_sel_parents,
+		.num_parents = ARRAY_SIZE(axi_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_0,
+		.set_ofs = CLK_CFG_0_SET,
+		.clr_ofs = CLK_CFG_0_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 3,
+
+		.ops = &mtk_mux_clr_set_upd_ops,
+	},
+	{
+		.id = MEM_SEL,
+		.name = "mem_sel",
+		.parent_names = mem_sel_parents,
+		.num_parents = ARRAY_SIZE(mem_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_0,
+		.set_ofs = CLK_CFG_0_SET,
+		.clr_ofs = CLK_CFG_0_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 1,
+
+		.ops = &mtk_mux_clr_set_upd_ops,
+	},
+	{
+		.id = DDRPHY_SEL,
+		.name = "ddrphycfg_sel",
+		.parent_names = ddrphycfg_parents,
+		.num_parents = ARRAY_SIZE(ddrphycfg_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_0,
+		.set_ofs = CLK_CFG_0_SET,
+		.clr_ofs = CLK_CFG_0_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 1,
+
+		.ops = &mtk_mux_clr_set_upd_ops,
+	},
+	{
+		.id = MM_SEL,
+		.name = "mm_sel",
+		.parent_names = mm_sel_parents,
+		.num_parents = ARRAY_SIZE(mm_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_0,
+		.set_ofs = CLK_CFG_0_SET,
+		.clr_ofs = CLK_CFG_0_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 3,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = PWM_SEL,
+		.name = "pwm_sel",
+		.parent_names = pwm_sel_parents,
+		.num_parents = ARRAY_SIZE(pwm_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_1,
+		.set_ofs = CLK_CFG_1_SET,
+		.clr_ofs = CLK_CFG_1_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 2,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = VDEC_SEL,
+		.name = "vdec_sel",
+		.parent_names = vdec_sel_parents,
+		.num_parents = ARRAY_SIZE(vdec_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_1,
+		.set_ofs = CLK_CFG_1_SET,
+		.clr_ofs = CLK_CFG_1_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 3,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MFG_SEL,
+		.name = "mfg_sel",
+		.parent_names = mfg_sel_parents,
+		.num_parents = ARRAY_SIZE(mfg_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_1,
+		.set_ofs = CLK_CFG_1_SET,
+		.clr_ofs = CLK_CFG_1_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 4,
+		.gate_shift = 23,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = CAMTG_SEL,
+		.name = "camtg_sel",
+		.parent_names = camtg_sel_parents,
+		.num_parents = ARRAY_SIZE(camtg_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_1,
+		.set_ofs = CLK_CFG_1_SET,
+		.clr_ofs = CLK_CFG_1_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 3,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = UART_SEL,
+		.name = "uart_sel",
+		.parent_names = uart_sel_parents,
+		.num_parents = ARRAY_SIZE(uart_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_2,
+		.set_ofs = CLK_CFG_2_SET,
+		.clr_ofs = CLK_CFG_2_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 1,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = SPI_SEL,
+		.name = "spi_sel",
+		.parent_names = spi_sel_parents,
+		.num_parents = ARRAY_SIZE(spi_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_2,
+		.set_ofs = CLK_CFG_2_SET,
+		.clr_ofs = CLK_CFG_2_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 3,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = USB20_SEL,
+		.name = "usb20_sel",
+		.parent_names = usb20_sel_parents,
+		.num_parents = ARRAY_SIZE(usb20_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_2,
+		.set_ofs = CLK_CFG_2_SET,
+		.clr_ofs = CLK_CFG_2_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 2,
+		.gate_shift = 23,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MSDC50_0_SEL,
+		.name = "msdc50_0_sel",
+		.parent_names = msdc50_0_sel_parents,
+		.num_parents = ARRAY_SIZE(msdc50_0_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_2,
+		.set_ofs = CLK_CFG_2_SET,
+		.clr_ofs = CLK_CFG_2_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 3,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MSDC30_0_SEL,
+		.name = "msdc30_0_sel",
+		.parent_names = msdc30_0_sel_parents,
+		.num_parents = ARRAY_SIZE(msdc30_0_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_3,
+		.set_ofs = CLK_CFG_3_SET,
+		.clr_ofs = CLK_CFG_3_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 4,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MSDC30_1_SEL,
+		.name = "msdc30_1_sel",
+		.parent_names = msdc30_1_2_sel_parents,
+		.num_parents = ARRAY_SIZE(msdc30_1_2_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_3,
+		.set_ofs = CLK_CFG_3_SET,
+		.clr_ofs = CLK_CFG_3_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 3,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MSDC30_2_SEL,
+		.name = "msdc30_2_sel",
+		.parent_names = msdc30_1_2_sel_parents,
+		.num_parents = ARRAY_SIZE(msdc30_1_2_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_3,
+		.set_ofs = CLK_CFG_3_SET,
+		.clr_ofs = CLK_CFG_3_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 3,
+		.gate_shift = 23,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MSDC30_3_SEL,
+		.name = "msdc30_3_sel",
+		.parent_names = msdc30_3_sel_parents,
+		.num_parents = ARRAY_SIZE(msdc30_3_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_3,
+		.set_ofs = CLK_CFG_3_SET,
+		.clr_ofs = CLK_CFG_3_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 4,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = AUDIO_SEL,
+		.name = "audio_sel",
+		.parent_names = audio_sel_parents,
+		.num_parents = ARRAY_SIZE(audio_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_4,
+		.set_ofs = CLK_CFG_4_SET,
+		.clr_ofs = CLK_CFG_4_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 2,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = AUDINTBUS_SEL,
+		.name = "aud_intbus_sel",
+		.parent_names = aud_intbus_sel_parents,
+		.num_parents = ARRAY_SIZE(aud_intbus_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_4,
+		.set_ofs = CLK_CFG_4_SET,
+		.clr_ofs = CLK_CFG_4_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 2,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = PMICSPI_SEL,
+		.name = "pmicspi_sel",
+		.parent_names = pmicspi_sel_parents,
+		.num_parents = ARRAY_SIZE(pmicspi_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_4,
+		.set_ofs = CLK_CFG_4_SET,
+		.clr_ofs = CLK_CFG_4_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 3,
+
+		.ops = &mtk_mux_clr_set_upd_ops,
+	},
+	{
+		.id = SCP_SEL,
+		.name = "scp_sel",
+		.parent_names = scp_sel_parents,
+		.num_parents = ARRAY_SIZE(scp_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_4,
+		.set_ofs = CLK_CFG_4_SET,
+		.clr_ofs = CLK_CFG_4_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 2,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = ATB_SEL,
+		.name = "atb_sel",
+		.parent_names = atb_sel_parents,
+		.num_parents = ARRAY_SIZE(atb_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_5,
+		.set_ofs = CLK_CFG_5_SET,
+		.clr_ofs = CLK_CFG_5_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 2,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = DPI0_SEL,
+		.name = "dpi0_sel",
+		.parent_names = dpi0_sel_parents,
+		.num_parents = ARRAY_SIZE(dpi0_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_5,
+		.set_ofs = CLK_CFG_5_SET,
+		.clr_ofs = CLK_CFG_5_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 3,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = SCAM_SEL,
+		.name = "scam_sel",
+		.parent_names = scam_sel_parents,
+		.num_parents = ARRAY_SIZE(scam_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_5,
+		.set_ofs = CLK_CFG_5_SET,
+		.clr_ofs = CLK_CFG_5_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 2,
+		.gate_shift = 23,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = MFG13M_SEL,
+		.name = "mfg13m_sel",
+		.parent_names = mfg13m_sel_parents,
+		.num_parents = ARRAY_SIZE(mfg13m_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_5,
+		.set_ofs = CLK_CFG_5_SET,
+		.clr_ofs = CLK_CFG_5_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 1,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = AUD1_SEL,
+		.name = "aud_1_sel",
+		.parent_names = aud_1_2_sel_parents,
+		.num_parents = ARRAY_SIZE(aud_1_2_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_6,
+		.set_ofs = CLK_CFG_6_SET,
+		.clr_ofs = CLK_CFG_6_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 1,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = AUD2_SEL,
+		.name = "aud_2_sel",
+		.parent_names = aud_1_2_sel_parents,
+		.num_parents = ARRAY_SIZE(aud_1_2_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_6,
+		.set_ofs = CLK_CFG_6_SET,
+		.clr_ofs = CLK_CFG_6_CLR,
+
+		.mux_shift = 8,
+		.mux_width = 1,
+		.gate_shift = 15,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = IRDA_SEL,
+		.name = "irda_sel",
+		.parent_names = irda_sel_parents,
+		.num_parents = ARRAY_SIZE(irda_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_6,
+		.set_ofs = CLK_CFG_6_SET,
+		.clr_ofs = CLK_CFG_6_CLR,
+
+		.mux_shift = 16,
+		.mux_width = 1,
+		.gate_shift = 23,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = IRTX_SEL,
+		.name = "irtx_sel",
+		.parent_names = irtx_sel_parents,
+		.num_parents = ARRAY_SIZE(irtx_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_6,
+		.set_ofs = CLK_CFG_6_SET,
+		.clr_ofs = CLK_CFG_6_CLR,
+
+		.mux_shift = 24,
+		.mux_width = 1,
+		.gate_shift = 31,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+	{
+		.id = DISPPWM_SEL,
+		.name = "disppwm_sel",
+		.parent_names = disppwm_sel_parents,
+		.num_parents = ARRAY_SIZE(disppwm_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+
+		.mux_ofs = CLK_CFG_7,
+		.set_ofs = CLK_CFG_7_SET,
+		.clr_ofs = CLK_CFG_7_CLR,
+
+		.mux_shift = 0,
+		.mux_width = 2,
+		.gate_shift = 7,
+
+		.ops = &mtk_mux_gate_clr_set_upd_ops,
+	},
+};
+
+static const struct mtk_clk_desc topckgen_clks = {
+	.fixed_clks = topckgen_fixed_clks,
+	.num_fixed_clks = ARRAY_SIZE(topckgen_fixed_clks),
+	.factors = topckgen_factors,
+	.num_factors = ARRAY_SIZE(topckgen_factors),
+	.muxes = topckgen_muxes,
+	.num_muxes = ARRAY_SIZE(topckgen_muxes)
+};
+
+static const struct of_device_id of_match_mt6735_topckgen[] = {
+	{ .compatible = "mediatek,mt6735-topckgen", .data = &topckgen_clks },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver clk_mt6735_topckgen = {
+	.probe = mtk_clk_simple_probe,
+	.remove = mtk_clk_simple_remove,
+	.driver = {
+		.name = "clk-mt6735-topckgen",
+		.of_match_table = of_match_mt6735_topckgen,
+	},
+};
+module_platform_driver(clk_mt6735_topckgen);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Mediatek MT6735 topckgen clock driver");
+MODULE_LICENSE("GPL");
-- 
2.36.1

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Miles Chen]

hi Yassine,

> Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
> clock and reset controllers. These provide the base clocks on the
> platform, and should be enough to bring up all essential blocks
> including PWRAP, MSDC and peripherals (UART, I2C, SPI).
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single patch)
>   https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
> - clk: mediatek: Improvements to simple probe/remove and reset controller unregistration
>   https://patchwork.kernel.org/project/linux-clk/cover/20220519134728.456643-1-y.oudjana@protonmail.com/
> 
>  MAINTAINERS                                  |    4 +
>  drivers/clk/mediatek/Kconfig                 |    9 +
>  drivers/clk/mediatek/Makefile                |    1 +
>  drivers/clk/mediatek/clk-mt6735-apmixedsys.c |  235 ++++

...snip...

> +#define APLL2_CON0		0x284
> +#define APLL2_CON1		0x288
> +#define APLL2_CON2		0x28c
> +#define APLL2_PWR_CON0		0x294
> +
> +#define CON0_RST_BAR		BIT(24)
> +
> +static const struct mtk_pll_data apmixedsys_plls[] = {
> +	{
> +		.id = ARMPLL,
> +		.name = "armpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = ARMPLL_CON0,
> +		.pwr_reg = ARMPLL_PWR_CON0,
> +		.en_mask = 0x00000001,
> +
> +		.pd_reg = ARMPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = ARMPLL_CON1,
> +		.pcw_chg_reg = ARMPLL_CON1,
> +		.pcwbits = 21,
> +
> +		.flags = PLL_AO

Thanks for submitting this patch.

I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
and other clk files are using macros to make the mtk_pll_data array
more readable.

Would you mind following the same style for all c files, please?

e.g.,
	static const struct mtk_pll_data plls[] = {
		PLL(CLK_APMIXED_ARMPLL, "armpll", 0x0200, 0x020C, 0x00000001, 0, 32,
				0x0200, 4, 0, 0x0204, 0),
		PLL(CLK_APMIXED_NET2PLL, "net2pll", 0x0210, 0x021C, 0x00000001, 0, 32,
				0x0210, 4, 0, 0x0214, 0),                                           
		...
	};

> +	},
> +	{
> +		.id = MAINPLL,
> +		.name = "mainpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = MAINPLL_CON0,
> +		.pwr_reg = MAINPLL_PWR_CON0,
> +		.en_mask = 0xf0000101,
> +
> +		.pd_reg = MAINPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = MAINPLL_CON1,
> +		.pcw_chg_reg = MAINPLL_CON1,
> +		.pcwbits = 21,
> +
> +		.flags = HAVE_RST_BAR,
> +		.rst_bar_mask = CON0_RST_BAR
> +	},
> +	{
> +		.id = UNIVPLL,
> +		.name = "univpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = UNIVPLL_CON0,
> +		.pwr_reg = UNIVPLL_PWR_CON0,
> +		.en_mask = 0xfc000001,
> +
> +		.pd_reg = UNIVPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = UNIVPLL_CON1,
> +		.pcw_chg_reg = UNIVPLL_CON1,
> +		.pcwbits = 21,
> +
> +		.flags = HAVE_RST_BAR,
> +		.rst_bar_mask = CON0_RST_BAR
> +	},
> +	{
> +		.id = MMPLL,
> +		.name = "mmpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = MMPLL_CON0,
> +		.pwr_reg = MMPLL_PWR_CON0,
> +		.en_mask = 0x00000001,
> +
> +		.pd_reg = MMPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = MMPLL_CON1,
> +		.pcw_chg_reg = MMPLL_CON1,
> +		.pcwbits = 21
> +	},
> +	{
> +		.id = MSDCPLL,
> +		.name = "msdcpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = MSDCPLL_CON0,
> +		.pwr_reg = MSDCPLL_PWR_CON0,
> +		.en_mask = 0x00000001,
> +
> +		.pd_reg = MSDCPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = MSDCPLL_CON1,
> +		.pcw_chg_reg = MSDCPLL_CON1,
> +		.pcwbits = 21,
> +	},
> +	{
> +		.id = VENCPLL,
> +		.name = "vencpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = VENCPLL_CON0,
> +		.pwr_reg = VENCPLL_PWR_CON0,
> +		.en_mask = 0x00000001,
> +
> +		.pd_reg = VENCPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = VENCPLL_CON1,
> +		.pcw_chg_reg = VENCPLL_CON1,
> +		.pcwbits = 21,
> +
> +		.flags = HAVE_RST_BAR,
> +		.rst_bar_mask = CON0_RST_BAR
> +	},
> +	{
> +		.id = TVDPLL,
> +		.name = "tvdpll",
> +		.parent_name = "clk26m",
> +
> +		.reg = TVDPLL_CON0,
> +		.pwr_reg = TVDPLL_PWR_CON0,
> +		.en_mask = 0x00000001,
> +
> +		.pd_reg = TVDPLL_CON1,
> +		.pd_shift = 24,
> +
> +		.pcw_reg = TVDPLL_CON1,
> +		.pcw_chg_reg = TVDPLL_CON1,
> +		.pcwbits = 21
> +	},
> +	{
> +		.id = APLL1,
> +		.name = "apll1",
> +		.parent_name = "clk26m",
> +
> +		.reg = APLL1_CON0,
> +		.pwr_reg = APLL1_PWR_CON0,
> +module_platform_driver(clk_mt6735_apmixedsys);
> +
> +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
> +MODULE_DESCRIPTION("Mediatek MT6735 apmixedsys clock driver");

Would you mind changing all Mediatek to MediaTek?
i.e.,

s/Mediatek/MediaTek/


thanks,
Miles
> +MODULE_LICENSE("GPL");

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Yassine Oudjana]

On Fri, May 20 2022 at 16:35:14 +0800, Miles Chen 
<miles.chen@mediatek.com> wrote:
> hi Yassine,
> 
>>  Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
>>  clock and reset controllers. These provide the base clocks on the
>>  platform, and should be enough to bring up all essential blocks
>>  including PWRAP, MSDC and peripherals (UART, I2C, SPI).
>> 
>>  Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>  ---
>>  Dependencies:
>>  - clk: mediatek: Move to struct clk_hw provider APIs (series)
>>    
>> https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
>>  - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 
>> (series)
>>    
>> https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
>>  - Export required symbols to compile clk drivers as module (single 
>> patch)
>>    
>> https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
>>  - clk: mediatek: Improvements to simple probe/remove and reset 
>> controller unregistration
>>    
>> https://patchwork.kernel.org/project/linux-clk/cover/20220519134728.456643-1-y.oudjana@protonmail.com/
>> 
>>   MAINTAINERS                                  |    4 +
>>   drivers/clk/mediatek/Kconfig                 |    9 +
>>   drivers/clk/mediatek/Makefile                |    1 +
>>   drivers/clk/mediatek/clk-mt6735-apmixedsys.c |  235 ++++
> 
> ...snip...
> 
>>  +#define APLL2_CON0		0x284
>>  +#define APLL2_CON1		0x288
>>  +#define APLL2_CON2		0x28c
>>  +#define APLL2_PWR_CON0		0x294
>>  +
>>  +#define CON0_RST_BAR		BIT(24)
>>  +
>>  +static const struct mtk_pll_data apmixedsys_plls[] = {
>>  +	{
>>  +		.id = ARMPLL,
>>  +		.name = "armpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = ARMPLL_CON0,
>>  +		.pwr_reg = ARMPLL_PWR_CON0,
>>  +		.en_mask = 0x00000001,
>>  +
>>  +		.pd_reg = ARMPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = ARMPLL_CON1,
>>  +		.pcw_chg_reg = ARMPLL_CON1,
>>  +		.pcwbits = 21,
>>  +
>>  +		.flags = PLL_AO
> 
> Thanks for submitting this patch.
> 
> I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
> and other clk files are using macros to make the mtk_pll_data array
> more readable.

I'd actually argue that macros make it less readable. While reading
other drivers I had a lot of trouble figuring out which argument
is which field of the struct, and had to constantly go back to the
macro definitions and count arguments to find it. Having it this
way, each value is labeled clearly with the field it's in. I think
the tradeoff between line count and readability here is worth it.

> 
> Would you mind following the same style for all c files, please?
> 
> e.g.,
> 	static const struct mtk_pll_data plls[] = {
> 		PLL(CLK_APMIXED_ARMPLL, "armpll", 0x0200, 0x020C, 0x00000001, 0, 32,
> 				0x0200, 4, 0, 0x0204, 0),
> 		PLL(CLK_APMIXED_NET2PLL, "net2pll", 0x0210, 0x021C, 0x00000001, 0, 
> 32,
> 				0x0210, 4, 0, 0x0214, 0),
> 		...
> 	};
> 
>>  +	},
>>  +	{
>>  +		.id = MAINPLL,
>>  +		.name = "mainpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = MAINPLL_CON0,
>>  +		.pwr_reg = MAINPLL_PWR_CON0,
>>  +		.en_mask = 0xf0000101,
>>  +
>>  +		.pd_reg = MAINPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = MAINPLL_CON1,
>>  +		.pcw_chg_reg = MAINPLL_CON1,
>>  +		.pcwbits = 21,
>>  +
>>  +		.flags = HAVE_RST_BAR,
>>  +		.rst_bar_mask = CON0_RST_BAR
>>  +	},
>>  +	{
>>  +		.id = UNIVPLL,
>>  +		.name = "univpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = UNIVPLL_CON0,
>>  +		.pwr_reg = UNIVPLL_PWR_CON0,
>>  +		.en_mask = 0xfc000001,
>>  +
>>  +		.pd_reg = UNIVPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = UNIVPLL_CON1,
>>  +		.pcw_chg_reg = UNIVPLL_CON1,
>>  +		.pcwbits = 21,
>>  +
>>  +		.flags = HAVE_RST_BAR,
>>  +		.rst_bar_mask = CON0_RST_BAR
>>  +	},
>>  +	{
>>  +		.id = MMPLL,
>>  +		.name = "mmpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = MMPLL_CON0,
>>  +		.pwr_reg = MMPLL_PWR_CON0,
>>  +		.en_mask = 0x00000001,
>>  +
>>  +		.pd_reg = MMPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = MMPLL_CON1,
>>  +		.pcw_chg_reg = MMPLL_CON1,
>>  +		.pcwbits = 21
>>  +	},
>>  +	{
>>  +		.id = MSDCPLL,
>>  +		.name = "msdcpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = MSDCPLL_CON0,
>>  +		.pwr_reg = MSDCPLL_PWR_CON0,
>>  +		.en_mask = 0x00000001,
>>  +
>>  +		.pd_reg = MSDCPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = MSDCPLL_CON1,
>>  +		.pcw_chg_reg = MSDCPLL_CON1,
>>  +		.pcwbits = 21,
>>  +	},
>>  +	{
>>  +		.id = VENCPLL,
>>  +		.name = "vencpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = VENCPLL_CON0,
>>  +		.pwr_reg = VENCPLL_PWR_CON0,
>>  +		.en_mask = 0x00000001,
>>  +
>>  +		.pd_reg = VENCPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = VENCPLL_CON1,
>>  +		.pcw_chg_reg = VENCPLL_CON1,
>>  +		.pcwbits = 21,
>>  +
>>  +		.flags = HAVE_RST_BAR,
>>  +		.rst_bar_mask = CON0_RST_BAR
>>  +	},
>>  +	{
>>  +		.id = TVDPLL,
>>  +		.name = "tvdpll",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = TVDPLL_CON0,
>>  +		.pwr_reg = TVDPLL_PWR_CON0,
>>  +		.en_mask = 0x00000001,
>>  +
>>  +		.pd_reg = TVDPLL_CON1,
>>  +		.pd_shift = 24,
>>  +
>>  +		.pcw_reg = TVDPLL_CON1,
>>  +		.pcw_chg_reg = TVDPLL_CON1,
>>  +		.pcwbits = 21
>>  +	},
>>  +	{
>>  +		.id = APLL1,
>>  +		.name = "apll1",
>>  +		.parent_name = "clk26m",
>>  +
>>  +		.reg = APLL1_CON0,
>>  +		.pwr_reg = APLL1_PWR_CON0,
>>  +module_platform_driver(clk_mt6735_apmixedsys);
>>  +
>>  +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
>>  +MODULE_DESCRIPTION("Mediatek MT6735 apmixedsys clock driver");
> 
> Would you mind changing all Mediatek to MediaTek?
> i.e.,
> 
> s/Mediatek/MediaTek/
> 

Sure. Will fix it.

> 
> thanks,
> Miles
>  +MODULE_LICENSE("GPL");
> 

Thanks,
Yassine

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Miles Chen]

>> 
>> Thanks for submitting this patch.
>> 
>> I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
>> and other clk files are using macros to make the mtk_pll_data array
>> more readable.
>
>I'd actually argue that macros make it less readable. While reading
>other drivers I had a lot of trouble figuring out which argument
>is which field of the struct, and had to constantly go back to the
>macro definitions and count arguments to find it. Having it this
>way, each value is labeled clearly with the field it's in. I think
>the tradeoff between line count and readability here is worth it.

It is easier for multiple developers to work together if we have a common style.

How do you think?

Thanks,
Miles

>
>> 
>> Would you mind following the same style for all c files, please?
>> 
>> e.g.,
>> 	static const struct mtk_pll_data plls[] = {
>> 		PLL(CLK_APMIXED_ARMPLL, "armpll", 0x0200, 0x020C, 0x00000001, 0, 32,
>> 				0x0200, 4, 0, 0x0204, 0),
>> 		PLL(CLK_APMIXED_NET2PLL, "net2pll", 0x0210, 0x021C, 0x00000001, 0, 
>> 32,
>> 				0x0210, 4, 0, 0x0214, 0),
>> 		...
>> 	};
>> 
>>>  +	},
>>>  +		.reg = APLL1_CON0,
>>>  +		.pwr_reg = APLL1_PWR_CON0,
>>>  +module_platform_driver(clk_mt6735_apmixedsys);
>>>  +
>>>  +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
>>>  +MODULE_DESCRIPTION("Mediatek MT6735 apmixedsys clock driver");
>> 
>> Would you mind changing all Mediatek to MediaTek?
>> i.e.,
>> 
>> s/Mediatek/MediaTek/
>> 
>
>Sure. Will fix it.

thanks
>
>> 
>> thanks,
>> Miles
>>  +MODULE_LICENSE("GPL");
>> 
>
>Thanks,
>Yassine
>
>
>
>

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[AngeloGioacchino Del Regno]

Il 20/05/22 11:35, Miles Chen ha scritto:
> 
>>>
>>> Thanks for submitting this patch.
>>>
>>> I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
>>> and other clk files are using macros to make the mtk_pll_data array
>>> more readable.
>>
>> I'd actually argue that macros make it less readable. While reading
>> other drivers I had a lot of trouble figuring out which argument
>> is which field of the struct, and had to constantly go back to the
>> macro definitions and count arguments to find it. Having it this
>> way, each value is labeled clearly with the field it's in. I think
>> the tradeoff between line count and readability here is worth it.
> 
> It is easier for multiple developers to work together if we have a common style.
> 
> How do you think?
> 

In my opinion, Yassine is definitely right about this one: unrolling these macros
will make the code more readable, even though this has the side effect of making
it bigger in the source code form (obviously, when compiled, it's going to be the
exact same size).

I wouldn't mind getting this clock driver in without the usage of macros, as much
as I wouldn't mind converting all of the existing drivers to open-code everything
instead of using macros that you have to find in various headers... this practice
was done in multiple drivers (clock or elsewhere), so I don't think that it would
actually be a bad idea to do it here on MediaTek too, even though I'm not aware of
any *rule* that may want us to do that: if you check across drivers/clk/*, there's
a big split in how drivers are made, where some are using macros (davinci, renesas,
samsung, sprd, etc), and some are not (bcm, sunxi-ng, qcom, tegra, versatile, etc),
so it's really "do it as you wish"...

... *but:*

Apart from that, I also don't think that it is a good idea to convert the other
MTK clock drivers right now, as this would make the upstreaming of MediaTek clock
drivers harder for some of the community in this moment... especially when we look
at how many MTK SoCs are out there in the wild, and how many we have upstream:
something like 10% of them, or less.

I see the huge benefit of having a bigger community around MediaTek platforms as
that's beneficial to get a way better support and solidity for all SoCs as they
are sharing the same drivers and same framework, and expanding the support to more
of them will only make it better with highly valuable community contributions.


That said, Yassine, you should've understood that you have my full support on
unrolling these macros - but it's not time to do that yet: you definitely know
that MediaTek clock drivers are going through a big cleanup phase which is, at
this point, unavoidable... if we are able to get the aid of scripts (cocci and
others), that will make our life easier in this cleanup, and will also make us
able to perform the entire cleanup with less effort and in less overall time.

With that, I'm sad but I have to support Miles' decision on this one, and I also
have to ask you to use macros in this driver.


I am sure - and it is my wish - to see MediaTek clock drivers open-coding stuff
instead of using macros, but that's something for the future - which will happen
after the more important cleanups.

After all, it will be just about running "gcc -E xxxx.c" and copy-pasting the
unrolled macros to the clock drivers, which will be pretty fast and straightforward.

Sorry for the wall of text, by the way.

Cheers,
Angelo

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Yassine Oudjana]

On Fri, May 20 2022 at 12:26:25 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 20/05/22 11:35, Miles Chen ha scritto:
>> 
>>>> 
>>>> Thanks for submitting this patch.
>>>> 
>>>> I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
>>>> and other clk files are using macros to make the mtk_pll_data array
>>>> more readable.
>>> 
>>> I'd actually argue that macros make it less readable. While reading
>>> other drivers I had a lot of trouble figuring out which argument
>>> is which field of the struct, and had to constantly go back to the
>>> macro definitions and count arguments to find it. Having it this
>>> way, each value is labeled clearly with the field it's in. I think
>>> the tradeoff between line count and readability here is worth it.
>> 
>> It is easier for multiple developers to work together if we have a 
>> common style.
>> 
>> How do you think?
>> 
> 
> In my opinion, Yassine is definitely right about this one: unrolling 
> these macros
> will make the code more readable, even though this has the side 
> effect of making
> it bigger in the source code form (obviously, when compiled, it's 
> going to be the
> exact same size).
> 
> I wouldn't mind getting this clock driver in without the usage of 
> macros, as much
> as I wouldn't mind converting all of the existing drivers to 
> open-code everything
> instead of using macros that you have to find in various headers... 
> this practice
> was done in multiple drivers (clock or elsewhere), so I don't think 
> that it would
> actually be a bad idea to do it here on MediaTek too, even though I'm 
> not aware of
> any *rule* that may want us to do that: if you check across 
> drivers/clk/*, there's
> a big split in how drivers are made, where some are using macros 
> (davinci, renesas,
> samsung, sprd, etc), and some are not (bcm, sunxi-ng, qcom, tegra, 
> versatile, etc),
> so it's really "do it as you wish"...
> 
> ... *but:*
> 
> Apart from that, I also don't think that it is a good idea to convert 
> the other
> MTK clock drivers right now, as this would make the upstreaming of 
> MediaTek clock
> drivers harder for some of the community in this moment... especially 
> when we look
> at how many MTK SoCs are out there in the wild, and how many we have 
> upstream:
> something like 10% of them, or less.
> 
> I see the huge benefit of having a bigger community around MediaTek 
> platforms as
> that's beneficial to get a way better support and solidity for all 
> SoCs as they
> are sharing the same drivers and same framework, and expanding the 
> support to more
> of them will only make it better with highly valuable community 
> contributions.
> 
> 
> That said, Yassine, you should've understood that you have my full 
> support on
> unrolling these macros - but it's not time to do that yet: you 
> definitely know
> that MediaTek clock drivers are going through a big cleanup phase 
> which is, at
> this point, unavoidable... if we are able to get the aid of scripts 
> (cocci and
> others), that will make our life easier in this cleanup, and will 
> also make us
> able to perform the entire cleanup with less effort and in less 
> overall time.
> 
> With that, I'm sad but I have to support Miles' decision on this one, 
> and I also
> have to ask you to use macros in this driver.
> 
> 
> I am sure - and it is my wish - to see MediaTek clock drivers 
> open-coding stuff
> instead of using macros, but that's something for the future - which 
> will happen
> after the more important cleanups.
> 
> After all, it will be just about running "gcc -E xxxx.c" and 
> copy-pasting the
> unrolled macros to the clock drivers, which will be pretty fast and 
> straightforward.
> 
> Sorry for the wall of text, by the way.
> 
> Cheers,
> Angelo

Fair enough. I'll switch to macros in the next version.

Thanks,
Yassine

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Miles Chen]

Hi Angelo, Yassine,

>>>
>>> I'd actually argue that macros make it less readable. While reading
>>> other drivers I had a lot of trouble figuring out which argument
>>> is which field of the struct, and had to constantly go back to the
>>> macro definitions and count arguments to find it. Having it this
>>> way, each value is labeled clearly with the field it's in. I think
>>> the tradeoff between line count and readability here is worth it.
>> 
>> It is easier for multiple developers to work together if we have a common style.
>> 
>> How do you think?
>> 
>
>In my opinion, Yassine is definitely right about this one: unrolling these macros
>will make the code more readable, even though this has the side effect of making
>it bigger in the source code form (obviously, when compiled, it's going to be the
>exact same size).
>
>I wouldn't mind getting this clock driver in without the usage of macros, as much
>as I wouldn't mind converting all of the existing drivers to open-code everything
>instead of using macros that you have to find in various headers... this practice
>was done in multiple drivers (clock or elsewhere), so I don't think that it would
>actually be a bad idea to do it here on MediaTek too, even though I'm not aware of
>any *rule* that may want us to do that: if you check across drivers/clk/*, there's
>a big split in how drivers are made, where some are using macros (davinci, renesas,
>samsung, sprd, etc), and some are not (bcm, sunxi-ng, qcom, tegra, versatile, etc),
>so it's really "do it as you wish"...
>

Thanks for the explanation and guide. I think we can do that for all MediaTek
clock driver in the future, not having two styles in MediaTek clock driver now.

>
>... *but:*
>
>Apart from that, I also don't think that it is a good idea to convert the other
>MTK clock drivers right now, as this would make the upstreaming of MediaTek clock
>drivers harder for some of the community in this moment... especially when we look
>at how many MTK SoCs are out there in the wild, and how many we have upstream:
>something like 10% of them, or less.

and thanks for considering this too.

>
>I see the huge benefit of having a bigger community around MediaTek platforms as
>that's beneficial to get a way better support and solidity for all SoCs as they
>are sharing the same drivers and same framework, and expanding the support to more
>of them will only make it better with highly valuable community contributions.
>
>
>That said, Yassine, you should've understood that you have my full support on
>unrolling these macros - but it's not time to do that yet: you definitely know
>that MediaTek clock drivers are going through a big cleanup phase which is, at
>this point, unavoidable... if we are able to get the aid of scripts (cocci and
>others), that will make our life easier in this cleanup, and will also make us
>able to perform the entire cleanup with less effort and in less overall time.
>
>With that, I'm sad but I have to support Miles' decision on this one, and I also
>have to ask you to use macros in this driver.
>
>
>I am sure - and it is my wish - to see MediaTek clock drivers open-coding stuff
>instead of using macros, but that's something for the future - which will happen
>after the more important cleanups.
>
>After all, it will be just about running "gcc -E xxxx.c" and copy-pasting the
>unrolled macros to the clock drivers, which will be pretty fast and straightforward.
>
>
>Sorry for the wall of text, by the way.

Sounds good and I want to say thank you again, I learned a lot from your post
and patches you submitted.

and I also want to say thank you to Yassine for the patch.

thanks,
Miles
>
>Cheers,
>Angelo
>

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[Yassine Oudjana]

On Friday, May 20th, 2022 at 11:26 AM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:

> Il 20/05/22 11:35, Miles Chen ha scritto:
>
> > > > Thanks for submitting this patch.
> > > >
> > > > I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
> > > > and other clk files are using macros to make the mtk_pll_data array
> > > > more readable.
> > >
> > > I'd actually argue that macros make it less readable. While reading
> > > other drivers I had a lot of trouble figuring out which argument
> > > is which field of the struct, and had to constantly go back to the
> > > macro definitions and count arguments to find it. Having it this
> > > way, each value is labeled clearly with the field it's in. I think
> > > the tradeoff between line count and readability here is worth it.
> >
> > It is easier for multiple developers to work together if we have a common style.
> >
> > How do you think?
>
>
> In my opinion, Yassine is definitely right about this one: unrolling these macros
> will make the code more readable, even though this has the side effect of making
> it bigger in the source code form (obviously, when compiled, it's going to be the
> exact same size).
>
> I wouldn't mind getting this clock driver in without the usage of macros, as much
> as I wouldn't mind converting all of the existing drivers to open-code everything
> instead of using macros that you have to find in various headers... this practice
> was done in multiple drivers (clock or elsewhere), so I don't think that it would
> actually be a bad idea to do it here on MediaTek too, even though I'm not aware of
> any rule that may want us to do that: if you check across drivers/clk/*, there's
> a big split in how drivers are made, where some are using macros (davinci, renesas,
> samsung, sprd, etc), and some are not (bcm, sunxi-ng, qcom, tegra, versatile, etc),
> so it's really "do it as you wish"...
>
> ... but:
>
> Apart from that, I also don't think that it is a good idea to convert the other
> MTK clock drivers right now, as this would make the upstreaming of MediaTek clock
> drivers harder for some of the community in this moment... especially when we look
> at how many MTK SoCs are out there in the wild, and how many we have upstream:
> something like 10% of them, or less.
>
> I see the huge benefit of having a bigger community around MediaTek platforms as
> that's beneficial to get a way better support and solidity for all SoCs as they
> are sharing the same drivers and same framework, and expanding the support to more
> of them will only make it better with highly valuable community contributions.
>
>
> That said, Yassine, you should've understood that you have my full support on
> unrolling these macros - but it's not time to do that yet: you definitely know
> that MediaTek clock drivers are going through a big cleanup phase which is, at
> this point, unavoidable... if we are able to get the aid of scripts (cocci and
> others), that will make our life easier in this cleanup, and will also make us
> able to perform the entire cleanup with less effort and in less overall time.
>
> With that, I'm sad but I have to support Miles' decision on this one, and I also
> have to ask you to use macros in this driver.

I'm picking up this series again now after taking a long break to allow for
ongoing cleanup and refactoring work to settle down. I was going to make this
change but then I couldn't find the PLL macro defined in any common header.
It seems that it is defined in every driver that uses it, with slight variations
in some of them. Should I just do the same, or would it be better to define it
in clk-pll.h? Also, would now be a good time to unroll the macros in all drivers,
or is it still too soon?

Another thing: Since I've been out of touch with the cleanup work for a while,
it would be great if someone makes me aware of any pending cleanup patches that
I should know of so that I base my patches on them and avoid duplicating work.

> ...

Re: [PATCH v2 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock drivers

[AngeloGioacchino Del Regno]

Il 13/08/22 12:44, Yassine Oudjana ha scritto:
> On Friday, May 20th, 2022 at 11:26 AM, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:
> 
>> Il 20/05/22 11:35, Miles Chen ha scritto:
>>
>>>>> Thanks for submitting this patch.
>>>>>
>>>>> I compare this with drivers/clk/mediatek/clk-mt7986-apmixed.c,
>>>>> and other clk files are using macros to make the mtk_pll_data array
>>>>> more readable.
>>>>
>>>> I'd actually argue that macros make it less readable. While reading
>>>> other drivers I had a lot of trouble figuring out which argument
>>>> is which field of the struct, and had to constantly go back to the
>>>> macro definitions and count arguments to find it. Having it this
>>>> way, each value is labeled clearly with the field it's in. I think
>>>> the tradeoff between line count and readability here is worth it.
>>>
>>> It is easier for multiple developers to work together if we have a common style.
>>>
>>> How do you think?
>>
>>
>> In my opinion, Yassine is definitely right about this one: unrolling these macros
>> will make the code more readable, even though this has the side effect of making
>> it bigger in the source code form (obviously, when compiled, it's going to be the
>> exact same size).
>>
>> I wouldn't mind getting this clock driver in without the usage of macros, as much
>> as I wouldn't mind converting all of the existing drivers to open-code everything
>> instead of using macros that you have to find in various headers... this practice
>> was done in multiple drivers (clock or elsewhere), so I don't think that it would
>> actually be a bad idea to do it here on MediaTek too, even though I'm not aware of
>> any rule that may want us to do that: if you check across drivers/clk/*, there's
>> a big split in how drivers are made, where some are using macros (davinci, renesas,
>> samsung, sprd, etc), and some are not (bcm, sunxi-ng, qcom, tegra, versatile, etc),
>> so it's really "do it as you wish"...
>>
>> ... but:
>>
>> Apart from that, I also don't think that it is a good idea to convert the other
>> MTK clock drivers right now, as this would make the upstreaming of MediaTek clock
>> drivers harder for some of the community in this moment... especially when we look
>> at how many MTK SoCs are out there in the wild, and how many we have upstream:
>> something like 10% of them, or less.
>>
>> I see the huge benefit of having a bigger community around MediaTek platforms as
>> that's beneficial to get a way better support and solidity for all SoCs as they
>> are sharing the same drivers and same framework, and expanding the support to more
>> of them will only make it better with highly valuable community contributions.
>>
>>
>> That said, Yassine, you should've understood that you have my full support on
>> unrolling these macros - but it's not time to do that yet: you definitely know
>> that MediaTek clock drivers are going through a big cleanup phase which is, at
>> this point, unavoidable... if we are able to get the aid of scripts (cocci and
>> others), that will make our life easier in this cleanup, and will also make us
>> able to perform the entire cleanup with less effort and in less overall time.
>>
>> With that, I'm sad but I have to support Miles' decision on this one, and I also
>> have to ask you to use macros in this driver.
> 
> I'm picking up this series again now after taking a long break to allow for
> ongoing cleanup and refactoring work to settle down. I was going to make this
> change but then I couldn't find the PLL macro defined in any common header.
> It seems that it is defined in every driver that uses it, with slight variations
> in some of them. Should I just do the same, or would it be better to define it
> in clk-pll.h? Also, would now be a good time to unroll the macros in all drivers,
> or is it still too soon?

Hello Yassine,
I'm sorry for the very late reply to this topic, but I just got back from vacation.

Please follow the current way of defining the PLL macro into the SoC-specific
driver: MediaTek folks are in the process of implementing Frequency Hopping (FHCTL)
on supported PLLs so "some things may change"... maybe in your driver too... I'm
not sure, though, whether MT6735 does support FHCTL and anyway, even if it does,
I would recommend to just go without it as a first step, as adding FHCTL capability
will be done on all(?) of the supported SoC clock drivers as soon as it lands.

> 
> Another thing: Since I've been out of touch with the cleanup work for a while,
> it would be great if someone makes me aware of any pending cleanup patches that
> I should know of so that I base my patches on them and avoid duplicating work.
> 

Simply check linux-mediatek, but I don't think that there's anything in the
pipeline that would be blocking your MT6735 drivers.

Cheers,
Angelo