linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU
@ 2025-08-30 17:08 Chen-Yu Tsai
  2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

Hi folks,

This series adds support for the MCU PRCM and the NPU.

The MCU PRCM is a Power, Reset & Clock Management block that has some
clock and reset controls for the MCU, DSP and some peripherals that the
MCU could use.

The NPU is a Vivante IP block. It is clocked from the NPU PLL from the
main clock unit, but the bus clock and reset controls lie in the MCU
PRCM.

Patch 1 adds the missing NPU module clock to the main clock control
unit's binding.

Patch 2 adds the binding for the MCU PRCM clock control unit

Patch 3 fixes clock rate readback for the new dual-divider type added
with the A523 family.

Patch 4 adds the missing NPU module clock.

Patch 5 adds support for power-of-two dividers to the sunxi-ng clk
library.

Patch 6 adds a new driver for the A523 MCU PRCM CCU.

Patch 7 adds a device node for the MCU PRCM CCU.

Patch 8 adds a device node for the NPU.

The NPU was only lightly tested: the driver correctly probes and detects
a model GC9000, revision 9003.

Please have a look. All patches will be merged through the sunxi tree.


Thanks
ChenYu


Chen-Yu Tsai (8):
  dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock
  dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller
  clk: sunxi-ng: mp: Fix dual-divider clock rate readback
  clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock
  clk: sunxi-ng: div: support power-of-two dividers
  clk: sunxi-ng: add support for the A523/T527 MCU CCU
  arm64: dts: allwinner: a523: Add MCU PRCM CCU node
  arm64: dts: allwinner: a523: Add NPU device node

 .../clock/allwinner,sun55i-a523-ccu.yaml      |  35 +-
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi |  37 ++
 drivers/clk/sunxi-ng/Kconfig                  |   5 +
 drivers/clk/sunxi-ng/Makefile                 |   2 +
 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c    | 447 ++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c        |  21 +-
 drivers/clk/sunxi-ng/ccu-sun55i-a523.h        |  14 -
 drivers/clk/sunxi-ng/ccu_div.h                |  18 +
 drivers/clk/sunxi-ng/ccu_mp.c                 |   2 +-
 include/dt-bindings/clock/sun55i-a523-ccu.h   |   1 +
 .../dt-bindings/clock/sun55i-a523-mcu-ccu.h   |  54 +++
 .../dt-bindings/reset/sun55i-a523-mcu-ccu.h   |  30 ++
 12 files changed, 646 insertions(+), 20 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
 delete mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h
 create mode 100644 include/dt-bindings/clock/sun55i-a523-mcu-ccu.h
 create mode 100644 include/dt-bindings/reset/sun55i-a523-mcu-ccu.h

-- 
2.39.5


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-09-01 21:29   ` Rob Herring (Arm)
  2025-09-05 15:12   ` Andre Przywara
  2025-08-30 17:08 ` [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller Chen-Yu Tsai
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The main clock controller on the A523/T527 has the NPU's module clock.
It was missing from the original submission, likely because that was
based on the A523 user manual; the A523 is marketed without the NPU.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 include/dt-bindings/clock/sun55i-a523-ccu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/sun55i-a523-ccu.h b/include/dt-bindings/clock/sun55i-a523-ccu.h
index c8259ac5ada7..54808fcfd556 100644
--- a/include/dt-bindings/clock/sun55i-a523-ccu.h
+++ b/include/dt-bindings/clock/sun55i-a523-ccu.h
@@ -185,5 +185,6 @@
 #define CLK_FANOUT0		176
 #define CLK_FANOUT1		177
 #define CLK_FANOUT2		178
+#define CLK_NPU			179
 
 #endif /* _DT_BINDINGS_CLK_SUN55I_A523_CCU_H_ */
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
  2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-09-01 21:31   ` Rob Herring (Arm)
  2025-08-30 17:08 ` [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback Chen-Yu Tsai
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

There are four clock controllers in the A523 SoC. The existing binding
already covers two of them that are critical for basic operation. The
remaining ones are the MCU clock controller and CPU PLL clock
controller.

Add a description for the MCU CCU. This unit controls and provides
clocks to the MCU (RISC-V) subsystem and peripherals meant to operate
under low power conditions.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../clock/allwinner,sun55i-a523-ccu.yaml      | 35 +++++++++++-
 .../dt-bindings/clock/sun55i-a523-mcu-ccu.h   | 54 +++++++++++++++++++
 .../dt-bindings/reset/sun55i-a523-mcu-ccu.h   | 30 +++++++++++
 3 files changed, 117 insertions(+), 2 deletions(-)
 create mode 100644 include/dt-bindings/clock/sun55i-a523-mcu-ccu.h
 create mode 100644 include/dt-bindings/reset/sun55i-a523-mcu-ccu.h

diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
index f5f62e9a10a1..1dbd92febc47 100644
--- a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
+++ b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
@@ -19,6 +19,7 @@ properties:
   compatible:
     enum:
       - allwinner,sun55i-a523-ccu
+      - allwinner,sun55i-a523-mcu-ccu
       - allwinner,sun55i-a523-r-ccu
 
   reg:
@@ -26,11 +27,11 @@ properties:
 
   clocks:
     minItems: 4
-    maxItems: 5
+    maxItems: 8
 
   clock-names:
     minItems: 4
-    maxItems: 5
+    maxItems: 8
 
 required:
   - "#clock-cells"
@@ -63,6 +64,36 @@ allOf:
             - const: iosc
             - const: losc-fanout
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun55i-a523-mcu-ccu
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: High Frequency Oscillator (usually at 24MHz)
+            - description: Low Frequency Oscillator (usually at 32kHz)
+            - description: Internal Oscillator
+            - description: Audio PLL (4x)
+            - description: Peripherals PLL 0 (300 MHz output)
+            - description: DSP module clock
+            - description: PRCM AHB clock
+            - description: MBUS clock
+
+        clock-names:
+          items:
+            - const: hosc
+            - const: losc
+            - const: iosc
+            - const: pll-audio0-4x
+            - const: pll-periph0-300m
+            - const: dsp
+            - const: r-ahb
+            - const: mbus
+
   - if:
       properties:
         compatible:
diff --git a/include/dt-bindings/clock/sun55i-a523-mcu-ccu.h b/include/dt-bindings/clock/sun55i-a523-mcu-ccu.h
new file mode 100644
index 000000000000..6efc6bc7e11a
--- /dev/null
+++ b/include/dt-bindings/clock/sun55i-a523-mcu-ccu.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
+ */
+
+#ifndef _DT_BINDINGS_CLK_SUN55I_A523_MCU_CCU_H_
+#define _DT_BINDINGS_CLK_SUN55I_A523_MCU_CCU_H_
+
+#define CLK_MCU_PLL_AUDIO1	0
+#define CLK_MCU_PLL_AUDIO1_DIV2	1
+#define CLK_MCU_PLL_AUDIO1_DIV5	2
+#define CLK_MCU_AUDIO_OUT	3
+#define CLK_MCU_DSP		4
+#define CLK_MCU_I2S0		5
+#define CLK_MCU_I2S1		6
+#define CLK_MCU_I2S2		7
+#define CLK_MCU_I2S3		8
+#define CLK_MCU_I2S3_ASRC	9
+#define CLK_BUS_MCU_I2S0	10
+#define CLK_BUS_MCU_I2S1	11
+#define CLK_BUS_MCU_I2S2	12
+#define CLK_BUS_MCU_I2S3	13
+#define CLK_MCU_SPDIF_TX	14
+#define CLK_MCU_SPDIF_RX	15
+#define CLK_BUS_MCU_SPDIF	16
+#define CLK_MCU_DMIC		17
+#define CLK_BUS_MCU_DMIC	18
+#define CLK_MCU_AUDIO_CODEC_DAC	19
+#define CLK_MCU_AUDIO_CODEC_ADC	20
+#define CLK_BUS_MCU_AUDIO_CODEC	21
+#define CLK_BUS_MCU_DSP_MSGBOX	22
+#define CLK_BUS_MCU_DSP_CFG	23
+#define CLK_BUS_MCU_NPU_HCLK	24
+#define CLK_BUS_MCU_NPU_ACLK	25
+#define CLK_MCU_TIMER0		26
+#define CLK_MCU_TIMER1		27
+#define CLK_MCU_TIMER2		28
+#define CLK_MCU_TIMER3		29
+#define CLK_MCU_TIMER4		30
+#define CLK_MCU_TIMER5		31
+#define CLK_BUS_MCU_TIMER	32
+#define CLK_BUS_MCU_DMA		33
+#define CLK_MCU_TZMA0		34
+#define CLK_MCU_TZMA1		35
+#define CLK_BUS_MCU_PUBSRAM	36
+#define CLK_MCU_MBUS_DMA	37
+#define CLK_MCU_MBUS		38
+#define CLK_MCU_RISCV		39
+#define CLK_BUS_MCU_RISCV_CFG	40
+#define CLK_BUS_MCU_RISCV_MSGBOX	41
+#define CLK_MCU_PWM0		42
+#define CLK_BUS_MCU_PWM0	43
+
+#endif /* _DT_BINDINGS_CLK_SUN55I_A523_MCU_CCU_H_ */
diff --git a/include/dt-bindings/reset/sun55i-a523-mcu-ccu.h b/include/dt-bindings/reset/sun55i-a523-mcu-ccu.h
new file mode 100644
index 000000000000..a89a0b44f08b
--- /dev/null
+++ b/include/dt-bindings/reset/sun55i-a523-mcu-ccu.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
+ */
+
+#ifndef _DT_BINDINGS_RST_SUN55I_A523_MCU_CCU_H_
+#define _DT_BINDINGS_RST_SUN55I_A523_MCU_CCU_H_
+
+#define RST_BUS_MCU_I2S0		0
+#define RST_BUS_MCU_I2S1		1
+#define RST_BUS_MCU_I2S2		2
+#define RST_BUS_MCU_I2S3		3
+#define RST_BUS_MCU_SPDIF		4
+#define RST_BUS_MCU_DMIC		5
+#define RST_BUS_MCU_AUDIO_CODEC		6
+#define RST_BUS_MCU_DSP_MSGBOX		7
+#define RST_BUS_MCU_DSP_CFG		8
+#define RST_BUS_MCU_NPU			9
+#define RST_BUS_MCU_TIMER		10
+#define RST_BUS_MCU_DSP_DEBUG		11
+#define RST_BUS_MCU_DSP			12
+#define RST_BUS_MCU_DMA			13
+#define RST_BUS_MCU_PUBSRAM		14
+#define RST_BUS_MCU_RISCV_CFG		15
+#define RST_BUS_MCU_RISCV_DEBUG		16
+#define RST_BUS_MCU_RISCV_CORE		17
+#define RST_BUS_MCU_RISCV_MSGBOX	18
+#define RST_BUS_MCU_PWM0		19
+
+#endif /* _DT_BINDINGS_RST_SUN55I_A523_MCU_CCU_H_ */
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
  2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
  2025-08-30 17:08 ` [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-09-05 15:12   ` Andre Przywara
  2025-08-30 17:08 ` [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

When dual-divider clock support was introduced, the P divider offset was
left out of the .recalc_rate readback function. This causes the clock
rate to become bogus or even zero (possibly due to the P divider being
1, leading to a divide-by-zero).

Fix this by incorporating the P divider offset into the calculation.

Fixes: 45717804b75e ("clk: sunxi-ng: mp: introduce dual-divider clock")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
index 354c981943b6..4221b1888b38 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.c
+++ b/drivers/clk/sunxi-ng/ccu_mp.c
@@ -185,7 +185,7 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw,
 	p &= (1 << cmp->p.width) - 1;
 
 	if (cmp->common.features & CCU_FEATURE_DUAL_DIV)
-		rate = (parent_rate / p) / m;
+		rate = (parent_rate / (p + cmp->p.offset)) / m;
 	else
 		rate = (parent_rate >> p) / m;
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2025-08-30 17:08 ` [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-09-05 15:14   ` Andre Przywara
  2025-08-30 17:08 ` [PATCH 5/8] clk: sunxi-ng: div: support power-of-two dividers Chen-Yu Tsai
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The main clock controller on the A523/T527 has the NPU's module clock.
It was missing from the original submission, likely because that was
based on the A523 user manual; the A523 is marketed without the NPU.

Also, merge the private header back into the driver code itself. The
header only contains a macro containing the total number of clocks.
This has to be updated every time a missing clock gets added. Having
it in a separate file doesn't help the process. Instead just drop the
macro, and thus the header no longer has any reason to exist.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 21 ++++++++++++++++++---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.h | 14 --------------
 2 files changed, 18 insertions(+), 17 deletions(-)
 delete mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h

diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
index 1a9a1cb869e2..88405b624dc5 100644
--- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
+++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
@@ -11,6 +11,9 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
+#include <dt-bindings/clock/sun55i-a523-ccu.h>
+#include <dt-bindings/reset/sun55i-a523-ccu.h>
+
 #include "../clk.h"
 
 #include "ccu_common.h"
@@ -25,8 +28,6 @@
 #include "ccu_nkmp.h"
 #include "ccu_nm.h"
 
-#include "ccu-sun55i-a523.h"
-
 /*
  * The 24 MHz oscillator, the root of most of the clock tree.
  * .fw_name is the string used in the DT "clock-names" property, used to
@@ -486,6 +487,18 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
 
 static SUNXI_CCU_GATE_HWS(bus_ve_clk, "bus-ve", ahb_hws, 0x69c, BIT(0), 0);
 
+static const struct clk_hw *npu_parents[] = {
+	&pll_periph0_480M_clk.common.hw,
+	&pll_periph0_600M_clk.hw,
+	&pll_periph0_800M_clk.common.hw,
+	&pll_npu_2x_clk.hw,
+};
+static SUNXI_CCU_M_HW_WITH_MUX_GATE(npu_clk, "npu", npu_parents, 0x6e0,
+				    0, 5,	/* M */
+				    24, 3,	/* mux */
+				    BIT(31),	/* gate */
+				    CLK_SET_RATE_PARENT);
+
 static SUNXI_CCU_GATE_HWS(bus_dma_clk, "bus-dma", ahb_hws, 0x70c, BIT(0), 0);
 
 static SUNXI_CCU_GATE_HWS(bus_msgbox_clk, "bus-msgbox", ahb_hws, 0x71c,
@@ -1217,6 +1230,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
 	&bus_ce_sys_clk.common,
 	&ve_clk.common,
 	&bus_ve_clk.common,
+	&npu_clk.common,
 	&bus_dma_clk.common,
 	&bus_msgbox_clk.common,
 	&bus_spinlock_clk.common,
@@ -1343,7 +1357,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
 };
 
 static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
-	.num	= CLK_NUMBER,
+	.num	= CLK_NPU + 1,
 	.hws	= {
 		[CLK_PLL_DDR0]		= &pll_ddr_clk.common.hw,
 		[CLK_PLL_PERIPH0_4X]	= &pll_periph0_4x_clk.common.hw,
@@ -1524,6 +1538,7 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
 		[CLK_FANOUT0]		= &fanout0_clk.common.hw,
 		[CLK_FANOUT1]		= &fanout1_clk.common.hw,
 		[CLK_FANOUT2]		= &fanout2_clk.common.hw,
+		[CLK_NPU]		= &npu_clk.common.hw,
 	},
 };
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h b/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
deleted file mode 100644
index fc8dd42f1b47..000000000000
--- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright 2024 Arm Ltd.
- */
-
-#ifndef _CCU_SUN55I_A523_H
-#define _CCU_SUN55I_A523_H
-
-#include <dt-bindings/clock/sun55i-a523-ccu.h>
-#include <dt-bindings/reset/sun55i-a523-ccu.h>
-
-#define CLK_NUMBER	(CLK_FANOUT2 + 1)
-
-#endif /* _CCU_SUN55I_A523_H */
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/8] clk: sunxi-ng: div: support power-of-two dividers
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
                   ` (3 preceding siblings ...)
  2025-08-30 17:08 ` [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-08-30 17:08 ` [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU Chen-Yu Tsai
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

Some clocks (for timers) on the A523 are mux-divider-gate types
with the divider being values of power-of-two.

Add a macro for these types of clocks so that we can use the divider
types instead of the M-P types without an M divider.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu_div.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu_div.h b/drivers/clk/sunxi-ng/ccu_div.h
index 90d49ee8e0cc..be00b3277e97 100644
--- a/drivers/clk/sunxi-ng/ccu_div.h
+++ b/drivers/clk/sunxi-ng/ccu_div.h
@@ -274,6 +274,24 @@ struct ccu_div {
 	SUNXI_CCU_M_HWS_WITH_GATE(_struct, _name, _parent, _reg,	\
 				  _mshift, _mwidth, 0, _flags)
 
+#define SUNXI_CCU_P_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
+				       _mshift, _mwidth,		\
+				       _muxshift, _muxwidth,		\
+				       _gate, _flags)			\
+	struct ccu_div _struct = {					\
+		.enable	= _gate,					\
+		.div	= _SUNXI_CCU_DIV_FLAGS(_mshift, _mwidth,	\
+					       CLK_DIVIDER_POWER_OF_TWO), \
+		.mux	= _SUNXI_CCU_MUX(_muxshift, _muxwidth),		\
+		.common	= {						\
+			.reg		= _reg,				\
+			.hw.init	= CLK_HW_INIT_PARENTS_DATA(_name, \
+								   _parents, \
+								   &ccu_div_ops, \
+								   _flags), \
+		},							\
+	}
+
 static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw)
 {
 	struct ccu_common *common = hw_to_ccu_common(hw);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
                   ` (4 preceding siblings ...)
  2025-08-30 17:08 ` [PATCH 5/8] clk: sunxi-ng: div: support power-of-two dividers Chen-Yu Tsai
@ 2025-08-30 17:08 ` Chen-Yu Tsai
  2025-09-05 15:14   ` Andre Przywara
  2025-08-30 17:09 ` [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node Chen-Yu Tsai
  2025-08-30 17:09 ` [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node Chen-Yu Tsai
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:08 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The A523/T527 SoCs have a new MCU PRCM, which has more clocks and reset
controls for the RISC-V MCU and other peripherals. There is no visible
bus in this part, but there is a second audio PLL. The BSP driver uses
the 24MHz main oscillator as the parent for all the bus clocks.

Add a driver to support this part. Unlike the BSP driver, the SoC's main
MBUS clock is chosen as the parent for the MCU MBUS clock, and the
latter then serves as the parent of the MCU DMA controller's MBUS clock.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/Kconfig               |   5 +
 drivers/clk/sunxi-ng/Makefile              |   2 +
 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c | 447 +++++++++++++++++++++
 3 files changed, 454 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c

diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
index 8896fd052ef1..6af2d020e03e 100644
--- a/drivers/clk/sunxi-ng/Kconfig
+++ b/drivers/clk/sunxi-ng/Kconfig
@@ -57,6 +57,11 @@ config SUN55I_A523_CCU
 	default ARCH_SUNXI
 	depends on ARM64 || COMPILE_TEST
 
+config SUN55I_A523_MCU_CCU
+	tristate "Support for the Allwinner A523/T527 MCU CCU"
+	default ARCH_SUNXI
+	depends on ARM64 || COMPILE_TEST
+
 config SUN55I_A523_R_CCU
 	tristate "Support for the Allwinner A523/T527 PRCM CCU"
 	default ARCH_SUNXI
diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
index 82e471036de6..a1c4087d7241 100644
--- a/drivers/clk/sunxi-ng/Makefile
+++ b/drivers/clk/sunxi-ng/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_SUN50I_H6_CCU)	+= sun50i-h6-ccu.o
 obj-$(CONFIG_SUN50I_H6_R_CCU)	+= sun50i-h6-r-ccu.o
 obj-$(CONFIG_SUN50I_H616_CCU)	+= sun50i-h616-ccu.o
 obj-$(CONFIG_SUN55I_A523_CCU)	+= sun55i-a523-ccu.o
+obj-$(CONFIG_SUN55I_A523_MCU_CCU)	+= sun55i-a523-mcu-ccu.o
 obj-$(CONFIG_SUN55I_A523_R_CCU)	+= sun55i-a523-r-ccu.o
 obj-$(CONFIG_SUN4I_A10_CCU)	+= sun4i-a10-ccu.o
 obj-$(CONFIG_SUN5I_CCU)		+= sun5i-ccu.o
@@ -61,6 +62,7 @@ sun50i-h6-ccu-y			+= ccu-sun50i-h6.o
 sun50i-h6-r-ccu-y		+= ccu-sun50i-h6-r.o
 sun50i-h616-ccu-y		+= ccu-sun50i-h616.o
 sun55i-a523-ccu-y		+= ccu-sun55i-a523.o
+sun55i-a523-mcu-ccu-y		+= ccu-sun55i-a523-mcu.o
 sun55i-a523-r-ccu-y		+= ccu-sun55i-a523-r.o
 sun4i-a10-ccu-y			+= ccu-sun4i-a10.o
 sun5i-ccu-y			+= ccu-sun5i.o
diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
new file mode 100644
index 000000000000..6105485837c9
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
@@ -0,0 +1,447 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
+ *
+ * Based on the A523 CCU driver:
+ *   Copyright (C) 2023-2024 Arm Ltd.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
+#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
+
+#include "ccu_common.h"
+#include "ccu_reset.h"
+
+#include "ccu_div.h"
+#include "ccu_gate.h"
+#include "ccu_mp.h"
+#include "ccu_mult.h"
+#include "ccu_nm.h"
+
+static const struct clk_parent_data osc24M[] = {
+	{ .fw_name = "hosc" }
+};
+
+#define SUN55I_A523_PLL_AUDIO1_REG	0x00c
+static struct ccu_sdm_setting pll_audio1_sdm_table[] = {
+	{ .rate = 2167603200, .pattern = 0xa000a234, .m = 1, .n = 90 }, /* div2->22.5792 */
+	{ .rate = 2359296000, .pattern = 0xa0009ba6, .m = 1, .n = 98 }, /* div2->24.576 */
+	{ .rate = 1806336000, .pattern = 0xa000872b, .m = 1, .n = 75 }, /* div5->22.576 */
+};
+
+static struct ccu_nm pll_audio1_clk = {
+	.enable		= BIT(27),
+	.lock		= BIT(28),
+	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 11),
+	.m		= _SUNXI_CCU_DIV(1, 1),
+	.sdm		= _SUNXI_CCU_SDM(pll_audio1_sdm_table, BIT(24),
+					 0x010, BIT(31)),
+	.min_rate	= 180000000U,
+	.max_rate	= 3500000000U,
+	.common		= {
+		.reg		= 0x00c,
+		.features	= CCU_FEATURE_SIGMA_DELTA_MOD,
+		.hw.init	= CLK_HW_INIT_PARENTS_DATA("pll-audio1",
+							   osc24M, &ccu_nm_ops,
+							   CLK_SET_RATE_GATE),
+	},
+};
+
+static const struct clk_hw *pll_audio1_div_parents[] = { &pll_audio1_clk.common.hw };
+static CLK_FIXED_FACTOR_HWS(pll_periph1_div2_clk, "pll-audio1-div2",
+			    pll_audio1_div_parents, 2, 1,
+			    CLK_SET_RATE_PARENT);
+static CLK_FIXED_FACTOR_HWS(pll_periph1_div5_clk, "pll-audio1-div5",
+			    pll_audio1_div_parents, 5, 1,
+			    CLK_SET_RATE_PARENT);
+
+static SUNXI_CCU_M_WITH_GATE(audio_out_clk, "audio-out",
+			     "pll-audio1-div2", 0x01c,
+			     0, 5, BIT(31), CLK_SET_RATE_PARENT);
+
+static const struct clk_parent_data dsp_parents[] = {
+	{ .fw_name = "hosc" },
+	{ .fw_name = "losc" },
+	{ .fw_name = "iosc" },
+	{ .hw = &pll_periph1_div5_clk.hw },
+	{ .hw = &pll_periph1_div2_clk.hw },
+	{ .fw_name = "dsp" },
+};
+static SUNXI_CCU_M_DATA_WITH_MUX_GATE(dsp_clk, "mcu-dsp", dsp_parents, 0x0020,
+				      0, 5,	/* M */
+				      24, 3,	/* mux */
+				      BIT(31),	/* gate */
+				      0);
+
+static const struct clk_parent_data i2s_parents[] = {
+	{ .fw_name = "pll-audio0-4x" },
+	{ .hw = &pll_periph1_div2_clk.hw },
+	{ .hw = &pll_periph1_div5_clk.hw },
+};
+
+static SUNXI_CCU_DUALDIV_MUX_GATE(i2s0_clk, "i2s0", i2s_parents, 0x02c,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+static SUNXI_CCU_DUALDIV_MUX_GATE(i2s1_clk, "i2s1", i2s_parents, 0x030,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+static SUNXI_CCU_DUALDIV_MUX_GATE(i2s2_clk, "i2s2", i2s_parents, 0x034,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_clk, "i2s3", i2s_parents, 0x038,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+
+static const struct clk_parent_data i2s3_asrc_parents[] = {
+	{ .fw_name = "pll-periph0-300m" },
+	{ .hw = &pll_periph1_div2_clk.hw },
+	{ .hw = &pll_periph1_div5_clk.hw },
+};
+static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_asrc_clk, "i2s3-asrc",
+				  i2s3_asrc_parents, 0x038,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+
+static SUNXI_CCU_GATE_DATA(bus_i2s0_clk, "bus-i2s0", osc24M, 0x040, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(bus_i2s1_clk, "bus-i2s1", osc24M, 0x040, BIT(1), 0);
+static SUNXI_CCU_GATE_DATA(bus_i2s2_clk, "bus-i2s2", osc24M, 0x040, BIT(2), 0);
+static SUNXI_CCU_GATE_DATA(bus_i2s3_clk, "bus-i2s3", osc24M, 0x040, BIT(3), 0);
+
+static const struct clk_parent_data audio_parents[] = {
+	{ .fw_name = "pll-audio0-4x" },
+	{ .hw = &pll_periph1_div2_clk.hw },
+	{ .hw = &pll_periph1_div5_clk.hw },
+};
+static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_tx_clk, "spdif-tx",
+				  audio_parents, 0x044,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_rx_clk, "spdif-rx",
+				  i2s3_asrc_parents, 0x048,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+
+static SUNXI_CCU_GATE_DATA(bus_spdif_clk, "bus-spdif", osc24M, 0x04c, BIT(0), 0);
+
+static SUNXI_CCU_DUALDIV_MUX_GATE(dmic_clk, "dmic", audio_parents, 0x050,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+
+static SUNXI_CCU_GATE_DATA(bus_dmic_clk, "bus-dmic", osc24M, 0x054, BIT(0), 0);
+
+static SUNXI_CCU_DUALDIV_MUX_GATE(audio_dac_clk, "audio-dac",
+				  audio_parents, 0x058,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+static SUNXI_CCU_DUALDIV_MUX_GATE(audio_adc_clk, "audio-adc",
+				  audio_parents, 0x05c,
+				  0, 5,		/* M */
+				  5, 5,		/* P */
+				  24, 3,	/* mux */
+				  BIT(31),	/* gate */
+				  CLK_SET_RATE_PARENT);
+
+static SUNXI_CCU_GATE_DATA(bus_audio_codec_clk, "bus-audio-codec",
+			   osc24M, 0x060, BIT(0), 0);
+
+static SUNXI_CCU_GATE_DATA(bus_dsp_msgbox_clk, "bus-dsp-msgbox",
+			   osc24M, 0x068, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(bus_dsp_cfg_clk, "bus-dsp-cfg",
+			   osc24M, 0x06c, BIT(0), 0);
+
+static SUNXI_CCU_GATE_DATA(bus_npu_hclk, "bus-npu-hclk", osc24M, 0x070, BIT(1), 0);
+static SUNXI_CCU_GATE_DATA(bus_npu_aclk, "bus-npu-aclk", osc24M, 0x070, BIT(2), 0);
+
+static const struct clk_parent_data timer_parents[] = {
+	{ .fw_name = "hosc" },
+	{ .fw_name = "losc" },
+	{ .fw_name = "iosc" },
+	{ .fw_name = "r-ahb" }
+};
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer0_clk, "mcu-timer0", timer_parents,
+				      0x074,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer1_clk, "mcu-timer1", timer_parents,
+				      0x078,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer2_clk, "mcu-timer2", timer_parents,
+				      0x07c,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer3_clk, "mcu-timer3", timer_parents,
+				      0x080,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer4_clk, "mcu-timer4", timer_parents,
+				      0x084,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer5_clk, "mcu-timer5", timer_parents,
+				      0x088,
+				      1, 3,	/* P */
+				      4, 2,	/* mux */
+				      BIT(0),	/* gate */
+				      0);
+static SUNXI_CCU_GATE_DATA(bus_mcu_timer_clk, "bus-mcu-timer", osc24M, 0x08c, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(bus_mcu_dma_clk, "bus-mcu-dma", osc24M, 0x104, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(tzma0_clk, "tzma0", osc24M, 0x108, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(tzma1_clk, "tzma1", osc24M, 0x10c, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(bus_pubsram_clk, "bus-pubsram", osc24M, 0x114, BIT(0), 0);
+
+/*
+ * user manual has "mbus" clock as parent of both clocks below,
+ * but this makes more sense, since BSP MCU DMA controller has
+ * reference to both of them, likely needing both enabled.
+ */
+static SUNXI_CCU_GATE_FW(mbus_mcu_clk, "mbus-mcu", "mbus", 0x11c, BIT(1), 0);
+static SUNXI_CCU_GATE_HW(mbus_mcu_dma_clk, "mbus-mcu-dma",
+			 &mbus_mcu_clk.common.hw, 0x11c, BIT(0), 0);
+
+static const struct clk_parent_data riscv_pwm_parents[] = {
+	{ .fw_name = "hosc" },
+	{ .fw_name = "losc" },
+	{ .fw_name = "iosc" },
+};
+
+static SUNXI_CCU_MUX_DATA_WITH_GATE(riscv_clk, "riscv",
+				    riscv_pwm_parents, 0x120,
+				    27, 3, BIT(31), 0);
+static SUNXI_CCU_GATE_DATA(bus_riscv_cfg_clk, "bus-riscv-cfg", osc24M,
+			   0x124, BIT(0), 0);
+static SUNXI_CCU_GATE_DATA(bus_riscv_msgbox_clk, "bus-riscv-msgbox", osc24M,
+			   0x128, BIT(0), 0);
+
+static SUNXI_CCU_MUX_DATA_WITH_GATE(mcu_pwm0_clk, "mcu-pwm0",
+				    riscv_pwm_parents, 0x130,
+				    27, 3, BIT(31), 0);
+static SUNXI_CCU_GATE_DATA(bus_mcu_pwm0_clk, "bus-mcu-pwm0", osc24M,
+			   0x128, BIT(0), 0);
+
+/*
+ * Contains all clocks that are controlled by a hardware register. They
+ * have a (sunxi) .common member, which needs to be initialised by the common
+ * sunxi CCU code, to be filled with the MMIO base address and the shared lock.
+ */
+static struct ccu_common *sun55i_a523_mcu_ccu_clks[] = {
+	&pll_audio1_clk.common,
+	&audio_out_clk.common,
+	&dsp_clk.common,
+	&i2s0_clk.common,
+	&i2s1_clk.common,
+	&i2s2_clk.common,
+	&i2s3_clk.common,
+	&i2s3_asrc_clk.common,
+	&bus_i2s0_clk.common,
+	&bus_i2s1_clk.common,
+	&bus_i2s2_clk.common,
+	&bus_i2s3_clk.common,
+	&spdif_tx_clk.common,
+	&spdif_rx_clk.common,
+	&bus_spdif_clk.common,
+	&dmic_clk.common,
+	&bus_dmic_clk.common,
+	&audio_dac_clk.common,
+	&audio_adc_clk.common,
+	&bus_audio_codec_clk.common,
+	&bus_dsp_msgbox_clk.common,
+	&bus_dsp_cfg_clk.common,
+	&bus_npu_aclk.common,
+	&bus_npu_hclk.common,
+	&mcu_timer0_clk.common,
+	&mcu_timer1_clk.common,
+	&mcu_timer2_clk.common,
+	&mcu_timer3_clk.common,
+	&mcu_timer4_clk.common,
+	&mcu_timer5_clk.common,
+	&bus_mcu_timer_clk.common,
+	&bus_mcu_dma_clk.common,
+	&tzma0_clk.common,
+	&tzma1_clk.common,
+	&bus_pubsram_clk.common,
+	&mbus_mcu_dma_clk.common,
+	&mbus_mcu_clk.common,
+	&riscv_clk.common,
+	&bus_riscv_cfg_clk.common,
+	&bus_riscv_msgbox_clk.common,
+	&mcu_pwm0_clk.common,
+	&bus_mcu_pwm0_clk.common,
+};
+
+static struct clk_hw_onecell_data sun55i_a523_mcu_hw_clks = {
+	.num	= CLK_BUS_MCU_PWM0 + 1,
+	.hws	= {
+		[CLK_MCU_PLL_AUDIO1]		= &pll_audio1_clk.common.hw,
+		[CLK_MCU_PLL_AUDIO1_DIV2]	= &pll_periph1_div2_clk.hw,
+		[CLK_MCU_PLL_AUDIO1_DIV5]	= &pll_periph1_div5_clk.hw,
+		[CLK_MCU_AUDIO_OUT]		= &audio_out_clk.common.hw,
+		[CLK_MCU_DSP]			= &dsp_clk.common.hw,
+		[CLK_MCU_I2S0]			= &i2s0_clk.common.hw,
+		[CLK_MCU_I2S1]			= &i2s1_clk.common.hw,
+		[CLK_MCU_I2S2]			= &i2s2_clk.common.hw,
+		[CLK_MCU_I2S3]			= &i2s3_clk.common.hw,
+		[CLK_MCU_I2S3_ASRC]		= &i2s3_asrc_clk.common.hw,
+		[CLK_BUS_MCU_I2S0]		= &bus_i2s0_clk.common.hw,
+		[CLK_BUS_MCU_I2S1]		= &bus_i2s1_clk.common.hw,
+		[CLK_BUS_MCU_I2S2]		= &bus_i2s2_clk.common.hw,
+		[CLK_BUS_MCU_I2S3]		= &bus_i2s3_clk.common.hw,
+		[CLK_MCU_SPDIF_TX]		= &spdif_tx_clk.common.hw,
+		[CLK_MCU_SPDIF_RX]		= &spdif_rx_clk.common.hw,
+		[CLK_BUS_MCU_SPDIF]		= &bus_spdif_clk.common.hw,
+		[CLK_MCU_DMIC]			= &dmic_clk.common.hw,
+		[CLK_BUS_MCU_DMIC]		= &bus_dmic_clk.common.hw,
+		[CLK_MCU_AUDIO_CODEC_DAC]	= &audio_dac_clk.common.hw,
+		[CLK_MCU_AUDIO_CODEC_ADC]	= &audio_adc_clk.common.hw,
+		[CLK_BUS_MCU_AUDIO_CODEC]	= &bus_audio_codec_clk.common.hw,
+		[CLK_BUS_MCU_DSP_MSGBOX]	= &bus_dsp_msgbox_clk.common.hw,
+		[CLK_BUS_MCU_DSP_CFG]		= &bus_dsp_cfg_clk.common.hw,
+		[CLK_BUS_MCU_NPU_HCLK]		= &bus_npu_hclk.common.hw,
+		[CLK_BUS_MCU_NPU_ACLK]		= &bus_npu_aclk.common.hw,
+		[CLK_MCU_TIMER0]		= &mcu_timer0_clk.common.hw,
+		[CLK_MCU_TIMER1]		= &mcu_timer1_clk.common.hw,
+		[CLK_MCU_TIMER2]		= &mcu_timer2_clk.common.hw,
+		[CLK_MCU_TIMER3]		= &mcu_timer3_clk.common.hw,
+		[CLK_MCU_TIMER4]		= &mcu_timer4_clk.common.hw,
+		[CLK_MCU_TIMER5]		= &mcu_timer5_clk.common.hw,
+		[CLK_BUS_MCU_TIMER]		= &bus_mcu_timer_clk.common.hw,
+		[CLK_BUS_MCU_DMA]		= &bus_mcu_dma_clk.common.hw,
+		[CLK_MCU_TZMA0]			= &tzma0_clk.common.hw,
+		[CLK_MCU_TZMA1]			= &tzma1_clk.common.hw,
+		[CLK_BUS_MCU_PUBSRAM]		= &bus_pubsram_clk.common.hw,
+		[CLK_MCU_MBUS_DMA]		= &mbus_mcu_dma_clk.common.hw,
+		[CLK_MCU_MBUS]			= &mbus_mcu_clk.common.hw,
+		[CLK_MCU_RISCV]			= &riscv_clk.common.hw,
+		[CLK_BUS_MCU_RISCV_CFG]		= &bus_riscv_cfg_clk.common.hw,
+		[CLK_BUS_MCU_RISCV_MSGBOX]	= &bus_riscv_msgbox_clk.common.hw,
+		[CLK_MCU_PWM0]			= &mcu_pwm0_clk.common.hw,
+		[CLK_BUS_MCU_PWM0]		= &bus_mcu_pwm0_clk.common.hw,
+	},
+};
+
+static struct ccu_reset_map sun55i_a523_mcu_ccu_resets[] = {
+	[RST_BUS_MCU_I2S0]		= { 0x0040, BIT(16) },
+	[RST_BUS_MCU_I2S1]		= { 0x0040, BIT(17) },
+	[RST_BUS_MCU_I2S2]		= { 0x0040, BIT(18) },
+	[RST_BUS_MCU_I2S3]		= { 0x0040, BIT(19) },
+	[RST_BUS_MCU_SPDIF]		= { 0x004c, BIT(16) },
+	[RST_BUS_MCU_DMIC]		= { 0x0054, BIT(16) },
+	[RST_BUS_MCU_AUDIO_CODEC]	= { 0x0060, BIT(16) },
+	[RST_BUS_MCU_DSP_MSGBOX]	= { 0x0068, BIT(16) },
+	[RST_BUS_MCU_DSP_CFG]		= { 0x006c, BIT(16) },
+	[RST_BUS_MCU_NPU]		= { 0x0070, BIT(16) },
+	[RST_BUS_MCU_TIMER]		= { 0x008c, BIT(16) },
+	[RST_BUS_MCU_DSP_DEBUG]		= { 0x0100, BIT(16) },
+	[RST_BUS_MCU_DSP]		= { 0x0100, BIT(17) },
+	[RST_BUS_MCU_DMA]		= { 0x0104, BIT(16) },
+	[RST_BUS_MCU_PUBSRAM]		= { 0x0114, BIT(16) },
+	[RST_BUS_MCU_RISCV_CFG]		= { 0x0124, BIT(16) },
+	[RST_BUS_MCU_RISCV_DEBUG]	= { 0x0124, BIT(17) },
+	[RST_BUS_MCU_RISCV_CORE]	= { 0x0124, BIT(18) },
+	[RST_BUS_MCU_RISCV_MSGBOX]	= { 0x0128, BIT(16) },
+	[RST_BUS_MCU_PWM0]		= { 0x0134, BIT(16) },
+};
+
+static const struct sunxi_ccu_desc sun55i_a523_mcu_ccu_desc = {
+	.ccu_clks	= sun55i_a523_mcu_ccu_clks,
+	.num_ccu_clks	= ARRAY_SIZE(sun55i_a523_mcu_ccu_clks),
+
+	.hw_clks	= &sun55i_a523_mcu_hw_clks,
+
+	.resets		= sun55i_a523_mcu_ccu_resets,
+	.num_resets	= ARRAY_SIZE(sun55i_a523_mcu_ccu_resets),
+};
+
+static int sun55i_a523_mcu_ccu_probe(struct platform_device *pdev)
+{
+	void __iomem *reg;
+	u32 val;
+	int ret;
+
+	reg = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	val = readl(reg + SUN55I_A523_PLL_AUDIO1_REG);
+
+	/*
+	 * The PLL clock code does not model all bits, for instance it does
+	 * not support a separate enable and gate bit. We present the
+	 * gate bit(27) as the enable bit, but then have to set the
+	 * PLL Enable, LDO Enable, and Lock Enable bits on all PLLs here.
+	 */
+	val |= BIT(31) | BIT(30) | BIT(29);
+
+	/* Enforce p1 = 5, p0 = 2 (the default) for PLL_AUDIO1 */
+	val &= ~(GENMASK(22, 20) | GENMASK(18, 16));
+	val |= (4 << 20) | (1 << 16);
+
+	writel(val, reg + SUN55I_A523_PLL_AUDIO1_REG);
+
+	ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun55i_a523_mcu_ccu_desc);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id sun55i_a523_mcu_ccu_ids[] = {
+	{ .compatible = "allwinner,sun55i-a523-mcu-ccu" },
+	{ }
+};
+
+static struct platform_driver sun55i_a523_mcu_ccu_driver = {
+	.probe	= sun55i_a523_mcu_ccu_probe,
+	.driver	= {
+		.name			= "sun55i-a523-mcu-ccu",
+		.suppress_bind_attrs	= true,
+		.of_match_table		= sun55i_a523_mcu_ccu_ids,
+	},
+};
+module_platform_driver(sun55i_a523_mcu_ccu_driver);
+
+MODULE_IMPORT_NS("SUNXI_CCU");
+MODULE_DESCRIPTION("Support for the Allwinner A523 MCU CCU");
+MODULE_LICENSE("GPL");
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
                   ` (5 preceding siblings ...)
  2025-08-30 17:08 ` [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU Chen-Yu Tsai
@ 2025-08-30 17:09 ` Chen-Yu Tsai
  2025-09-05 15:14   ` Andre Przywara
  2025-08-30 17:09 ` [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node Chen-Yu Tsai
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

Add a device node for the third supported clock controller found in the
A523 / T527 SoCs. This controller has clocks and resets for the RISC-V
MCU, and others peripherals possibly meant to operate in low power mode
driven by the MCU, such as audio interfaces, an audio DSP, and the NPU.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index 79bd9ce08c7c..b6e82d53af54 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -4,8 +4,10 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/sun6i-rtc.h>
 #include <dt-bindings/clock/sun55i-a523-ccu.h>
+#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
 #include <dt-bindings/clock/sun55i-a523-r-ccu.h>
 #include <dt-bindings/reset/sun55i-a523-ccu.h>
+#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
 #include <dt-bindings/reset/sun55i-a523-r-ccu.h>
 #include <dt-bindings/power/allwinner,sun55i-a523-ppu.h>
 #include <dt-bindings/power/allwinner,sun55i-a523-pck-600.h>
@@ -825,6 +827,29 @@ rtc: rtc@7090000 {
 			clock-names = "bus", "hosc", "ahb";
 			#clock-cells = <1>;
 		};
+
+		mcu_ccu: clock-controller@7102000 {
+			compatible = "allwinner,sun55i-a523-mcu-ccu";
+			reg = <0x7102000 0x164>;
+			clocks = <&osc24M>,
+				 <&rtc CLK_OSC32K>,
+				 <&rtc CLK_IOSC>,
+				 <&ccu CLK_PLL_AUDIO0_4X>,
+				 <&ccu CLK_PLL_PERIPH0_300M>,
+				 <&ccu CLK_DSP>,
+				 <&r_ccu CLK_R_AHB>,
+				 <&ccu CLK_MBUS>;
+			clock-names = "hosc",
+				      "losc",
+				      "iosc",
+				      "pll-audio0-4x",
+				      "pll-periph0-300m",
+				      "dsp",
+				      "r-ahb",
+				      "mbus";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
 	};
 
 	thermal-zones {
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node
  2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
                   ` (6 preceding siblings ...)
  2025-08-30 17:09 ` [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node Chen-Yu Tsai
@ 2025-08-30 17:09 ` Chen-Yu Tsai
  2025-09-05 15:14   ` Andre Przywara
  7 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-08-30 17:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Andre Przywara, linux-sunxi, linux-clk, linux-arm-kernel,
	devicetree, linux-kernel

From: Chen-Yu Tsai <wens@csie.org>

The Allwinner T527 SoC has an NPU built in. Based on identifiers found
in the BSP, it is a Vivante IP block. After enabling it, the etnaviv
driver reports it as a GC9000 revision 9003.

The standard bindings are used as everything matches directly. There is
no option for DVFS at the moment. That might require some more work,
perhaps on the efuse side to map speed bins.

It is unclear whether the NPU block is fused out at the hardware level
or the BSP limits use of the NPU through software, as the author only
has boards with the T527.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index b6e82d53af54..1ab5b87ec78e 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -850,6 +850,18 @@ mcu_ccu: clock-controller@7102000 {
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
+
+		npu: npu@7122000 {
+			compatible = "vivante,gc";
+			reg = <0x07122000 0x1000>;
+			interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&mcu_ccu CLK_BUS_MCU_NPU_ACLK>,
+				 <&ccu CLK_NPU>,
+				 <&mcu_ccu CLK_BUS_MCU_NPU_HCLK>;
+			clock-names = "bus", "core", "reg";
+			resets = <&mcu_ccu RST_BUS_MCU_NPU>;
+			power-domains = <&ppu PD_NPU>;
+		};
 	};
 
 	thermal-zones {
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock
  2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
@ 2025-09-01 21:29   ` Rob Herring (Arm)
  2025-09-05 15:12   ` Andre Przywara
  1 sibling, 0 replies; 21+ messages in thread
From: Rob Herring (Arm) @ 2025-09-01 21:29 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: linux-clk, Stephen Boyd, linux-kernel, linux-sunxi,
	Jernej Skrabec, devicetree, Samuel Holland, Chen-Yu Tsai,
	linux-arm-kernel, Conor Dooley, Andre Przywara,
	Krzysztof Kozlowski


On Sun, 31 Aug 2025 01:08:54 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The main clock controller on the A523/T527 has the NPU's module clock.
> It was missing from the original submission, likely because that was
> based on the A523 user manual; the A523 is marketed without the NPU.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  include/dt-bindings/clock/sun55i-a523-ccu.h | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller
  2025-08-30 17:08 ` [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller Chen-Yu Tsai
@ 2025-09-01 21:31   ` Rob Herring (Arm)
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring (Arm) @ 2025-09-01 21:31 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Stephen Boyd, Samuel Holland, linux-arm-kernel, linux-kernel,
	Jernej Skrabec, Conor Dooley, linux-sunxi, linux-clk, devicetree,
	Krzysztof Kozlowski, Andre Przywara, Chen-Yu Tsai


On Sun, 31 Aug 2025 01:08:55 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> There are four clock controllers in the A523 SoC. The existing binding
> already covers two of them that are critical for basic operation. The
> remaining ones are the MCU clock controller and CPU PLL clock
> controller.
> 
> Add a description for the MCU CCU. This unit controls and provides
> clocks to the MCU (RISC-V) subsystem and peripherals meant to operate
> under low power conditions.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../clock/allwinner,sun55i-a523-ccu.yaml      | 35 +++++++++++-
>  .../dt-bindings/clock/sun55i-a523-mcu-ccu.h   | 54 +++++++++++++++++++
>  .../dt-bindings/reset/sun55i-a523-mcu-ccu.h   | 30 +++++++++++
>  3 files changed, 117 insertions(+), 2 deletions(-)
>  create mode 100644 include/dt-bindings/clock/sun55i-a523-mcu-ccu.h
>  create mode 100644 include/dt-bindings/reset/sun55i-a523-mcu-ccu.h
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock
  2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
  2025-09-01 21:29   ` Rob Herring (Arm)
@ 2025-09-05 15:12   ` Andre Przywara
  1 sibling, 0 replies; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:12 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:08:54 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

> From: Chen-Yu Tsai <wens@csie.org>
> 
> The main clock controller on the A523/T527 has the NPU's module clock.
> It was missing from the original submission, likely because that was
> based on the A523 user manual; the A523 is marketed without the NPU.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre

> ---
>  include/dt-bindings/clock/sun55i-a523-ccu.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/clock/sun55i-a523-ccu.h b/include/dt-bindings/clock/sun55i-a523-ccu.h
> index c8259ac5ada7..54808fcfd556 100644
> --- a/include/dt-bindings/clock/sun55i-a523-ccu.h
> +++ b/include/dt-bindings/clock/sun55i-a523-ccu.h
> @@ -185,5 +185,6 @@
>  #define CLK_FANOUT0		176
>  #define CLK_FANOUT1		177
>  #define CLK_FANOUT2		178
> +#define CLK_NPU			179
>  
>  #endif /* _DT_BINDINGS_CLK_SUN55I_A523_CCU_H_ */


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback
  2025-08-30 17:08 ` [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback Chen-Yu Tsai
@ 2025-09-05 15:12   ` Andre Przywara
  2025-09-05 15:17     ` Chen-Yu Tsai
  0 siblings, 1 reply; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:12 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:08:56 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

> From: Chen-Yu Tsai <wens@csie.org>
> 
> When dual-divider clock support was introduced, the P divider offset was
> left out of the .recalc_rate readback function. This causes the clock
> rate to become bogus or even zero (possibly due to the P divider being
> 1, leading to a divide-by-zero).

Ah, a nice catch, thanks for that! Just curious, how did you find this?
The MMC clocks use the dual divider type as well, but I didn't observe
them being wrong?

Regardless:

> Fix this by incorporating the P divider offset into the calculation.
> 
> Fixes: 45717804b75e ("clk: sunxi-ng: mp: introduce dual-divider clock")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre

> ---
>  drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> index 354c981943b6..4221b1888b38 100644
> --- a/drivers/clk/sunxi-ng/ccu_mp.c
> +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> @@ -185,7 +185,7 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw,
>  	p &= (1 << cmp->p.width) - 1;
>  
>  	if (cmp->common.features & CCU_FEATURE_DUAL_DIV)
> -		rate = (parent_rate / p) / m;
> +		rate = (parent_rate / (p + cmp->p.offset)) / m;
>  	else
>  		rate = (parent_rate >> p) / m;
>  


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock
  2025-08-30 17:08 ` [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
@ 2025-09-05 15:14   ` Andre Przywara
  2025-09-05 15:19     ` Chen-Yu Tsai
  0 siblings, 1 reply; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:08:57 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

Hi,

> From: Chen-Yu Tsai <wens@csie.org>
> 
> The main clock controller on the A523/T527 has the NPU's module clock.
> It was missing from the original submission, likely because that was
> based on the A523 user manual; the A523 is marketed without the NPU.

Ah, sorry, I missed that one. I think I spotted writable bits in that
register, but didn't find a clue what this clock was about. Anyway, checked
the bits against the T527 manual, they match up.

> Also, merge the private header back into the driver code itself. The
> header only contains a macro containing the total number of clocks.
> This has to be updated every time a missing clock gets added. Having
> it in a separate file doesn't help the process. Instead just drop the
> macro, and thus the header no longer has any reason to exist.

Interesting, looks nice, and solves Krzysztof's complaint the other
day about the binding header inclusion missing from the driver as well.
Just one thought:

> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 21 ++++++++++++++++++---
>  drivers/clk/sunxi-ng/ccu-sun55i-a523.h | 14 --------------
>  2 files changed, 18 insertions(+), 17 deletions(-)
>  delete mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> index 1a9a1cb869e2..88405b624dc5 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> @@ -11,6 +11,9 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  
> +#include <dt-bindings/clock/sun55i-a523-ccu.h>
> +#include <dt-bindings/reset/sun55i-a523-ccu.h>
> +

Should we have the number #define here, at a more central location? Seems a
bit buried down in there. And then use a plural name while at it:

#define NUM_CLOCKS	CLK_NPU + 1

Alternatively, put .num behind .hws below, so that the last clock and the
number definition are close together?

Cheers,
Andre

>  #include "../clk.h"
>  
>  #include "ccu_common.h"
> @@ -25,8 +28,6 @@
>  #include "ccu_nkmp.h"
>  #include "ccu_nm.h"
>  
> -#include "ccu-sun55i-a523.h"
> -
>  /*
>   * The 24 MHz oscillator, the root of most of the clock tree.
>   * .fw_name is the string used in the DT "clock-names" property, used to
> @@ -486,6 +487,18 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
>  
>  static SUNXI_CCU_GATE_HWS(bus_ve_clk, "bus-ve", ahb_hws, 0x69c, BIT(0), 0);
>  
> +static const struct clk_hw *npu_parents[] = {
> +	&pll_periph0_480M_clk.common.hw,
> +	&pll_periph0_600M_clk.hw,
> +	&pll_periph0_800M_clk.common.hw,
> +	&pll_npu_2x_clk.hw,
> +};
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(npu_clk, "npu", npu_parents, 0x6e0,
> +				    0, 5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
>  static SUNXI_CCU_GATE_HWS(bus_dma_clk, "bus-dma", ahb_hws, 0x70c, BIT(0), 0);
>  
>  static SUNXI_CCU_GATE_HWS(bus_msgbox_clk, "bus-msgbox", ahb_hws, 0x71c,
> @@ -1217,6 +1230,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
>  	&bus_ce_sys_clk.common,
>  	&ve_clk.common,
>  	&bus_ve_clk.common,
> +	&npu_clk.common,
>  	&bus_dma_clk.common,
>  	&bus_msgbox_clk.common,
>  	&bus_spinlock_clk.common,
> @@ -1343,7 +1357,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
>  };
>  
>  static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> -	.num	= CLK_NUMBER,
> +	.num	= CLK_NPU + 1,
>  	.hws	= {
>  		[CLK_PLL_DDR0]		= &pll_ddr_clk.common.hw,
>  		[CLK_PLL_PERIPH0_4X]	= &pll_periph0_4x_clk.common.hw,
> @@ -1524,6 +1538,7 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
>  		[CLK_FANOUT0]		= &fanout0_clk.common.hw,
>  		[CLK_FANOUT1]		= &fanout1_clk.common.hw,
>  		[CLK_FANOUT2]		= &fanout2_clk.common.hw,
> +		[CLK_NPU]		= &npu_clk.common.hw,
>  	},
>  };
>  
> diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h b/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> deleted file mode 100644
> index fc8dd42f1b47..000000000000
> --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * Copyright 2024 Arm Ltd.
> - */
> -
> -#ifndef _CCU_SUN55I_A523_H
> -#define _CCU_SUN55I_A523_H
> -
> -#include <dt-bindings/clock/sun55i-a523-ccu.h>
> -#include <dt-bindings/reset/sun55i-a523-ccu.h>
> -
> -#define CLK_NUMBER	(CLK_FANOUT2 + 1)
> -
> -#endif /* _CCU_SUN55I_A523_H */


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU
  2025-08-30 17:08 ` [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU Chen-Yu Tsai
@ 2025-09-05 15:14   ` Andre Przywara
  2025-09-05 16:13     ` Chen-Yu Tsai
  0 siblings, 1 reply; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:08:59 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

Hi Chen-Yu,

many thanks for this patch, I feel with you when it comes to model
Allwinner CCUs in the kernel ;-)

> From: Chen-Yu Tsai <wens@csie.org>
> 
> The A523/T527 SoCs have a new MCU PRCM, which has more clocks and reset
> controls for the RISC-V MCU and other peripherals. There is no visible
> bus in this part, but there is a second audio PLL. The BSP driver uses
> the 24MHz main oscillator as the parent for all the bus clocks.

So my copy of the T527 manual (v0.92) shows the system but tree of the
MCU_PRCM in figure 2-24, and there some peripherals are on AHB_DEC0, while
others are on APBS0. Shall we model this correctly, then?

> Add a driver to support this part. Unlike the BSP driver, the SoC's main
> MBUS clock is chosen as the parent for the MCU MBUS clock, and the
> latter then serves as the parent of the MCU DMA controller's MBUS clock.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/clk/sunxi-ng/Kconfig               |   5 +
>  drivers/clk/sunxi-ng/Makefile              |   2 +
>  drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c | 447 +++++++++++++++++++++
>  3 files changed, 454 insertions(+)
>  create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> 
> diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> index 8896fd052ef1..6af2d020e03e 100644
> --- a/drivers/clk/sunxi-ng/Kconfig
> +++ b/drivers/clk/sunxi-ng/Kconfig
> @@ -57,6 +57,11 @@ config SUN55I_A523_CCU
>  	default ARCH_SUNXI
>  	depends on ARM64 || COMPILE_TEST
>  
> +config SUN55I_A523_MCU_CCU
> +	tristate "Support for the Allwinner A523/T527 MCU CCU"
> +	default ARCH_SUNXI
> +	depends on ARM64 || COMPILE_TEST
> +
>  config SUN55I_A523_R_CCU
>  	tristate "Support for the Allwinner A523/T527 PRCM CCU"
>  	default ARCH_SUNXI
> diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> index 82e471036de6..a1c4087d7241 100644
> --- a/drivers/clk/sunxi-ng/Makefile
> +++ b/drivers/clk/sunxi-ng/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_SUN50I_H6_CCU)	+= sun50i-h6-ccu.o
>  obj-$(CONFIG_SUN50I_H6_R_CCU)	+= sun50i-h6-r-ccu.o
>  obj-$(CONFIG_SUN50I_H616_CCU)	+= sun50i-h616-ccu.o
>  obj-$(CONFIG_SUN55I_A523_CCU)	+= sun55i-a523-ccu.o
> +obj-$(CONFIG_SUN55I_A523_MCU_CCU)	+= sun55i-a523-mcu-ccu.o
>  obj-$(CONFIG_SUN55I_A523_R_CCU)	+= sun55i-a523-r-ccu.o
>  obj-$(CONFIG_SUN4I_A10_CCU)	+= sun4i-a10-ccu.o
>  obj-$(CONFIG_SUN5I_CCU)		+= sun5i-ccu.o
> @@ -61,6 +62,7 @@ sun50i-h6-ccu-y			+= ccu-sun50i-h6.o
>  sun50i-h6-r-ccu-y		+= ccu-sun50i-h6-r.o
>  sun50i-h616-ccu-y		+= ccu-sun50i-h616.o
>  sun55i-a523-ccu-y		+= ccu-sun55i-a523.o
> +sun55i-a523-mcu-ccu-y		+= ccu-sun55i-a523-mcu.o
>  sun55i-a523-r-ccu-y		+= ccu-sun55i-a523-r.o
>  sun4i-a10-ccu-y			+= ccu-sun4i-a10.o
>  sun5i-ccu-y			+= ccu-sun5i.o
> diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> new file mode 100644
> index 000000000000..6105485837c9
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> @@ -0,0 +1,447 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
> + *
> + * Based on the A523 CCU driver:
> + *   Copyright (C) 2023-2024 Arm Ltd.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
> +#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
> +
> +#include "ccu_common.h"
> +#include "ccu_reset.h"
> +
> +#include "ccu_div.h"
> +#include "ccu_gate.h"
> +#include "ccu_mp.h"
> +#include "ccu_mult.h"
> +#include "ccu_nm.h"
> +
> +static const struct clk_parent_data osc24M[] = {
> +	{ .fw_name = "hosc" }
> +};
> +
> +#define SUN55I_A523_PLL_AUDIO1_REG	0x00c
> +static struct ccu_sdm_setting pll_audio1_sdm_table[] = {
> +	{ .rate = 2167603200, .pattern = 0xa000a234, .m = 1, .n = 90 }, /* div2->22.5792 */
> +	{ .rate = 2359296000, .pattern = 0xa0009ba6, .m = 1, .n = 98 }, /* div2->24.576 */
> +	{ .rate = 1806336000, .pattern = 0xa000872b, .m = 1, .n = 75 }, /* div5->22.576 */
> +};
> +
> +static struct ccu_nm pll_audio1_clk = {
> +	.enable		= BIT(27),
> +	.lock		= BIT(28),
> +	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 11),
> +	.m		= _SUNXI_CCU_DIV(1, 1),
> +	.sdm		= _SUNXI_CCU_SDM(pll_audio1_sdm_table, BIT(24),
> +					 0x010, BIT(31)),
> +	.min_rate	= 180000000U,
> +	.max_rate	= 3500000000U,
> +	.common		= {
> +		.reg		= 0x00c,
> +		.features	= CCU_FEATURE_SIGMA_DELTA_MOD,
> +		.hw.init	= CLK_HW_INIT_PARENTS_DATA("pll-audio1",
> +							   osc24M, &ccu_nm_ops,
> +							   CLK_SET_RATE_GATE),
> +	},
> +};
> +
> +static const struct clk_hw *pll_audio1_div_parents[] = { &pll_audio1_clk.common.hw };
> +static CLK_FIXED_FACTOR_HWS(pll_periph1_div2_clk, "pll-audio1-div2",
> +			    pll_audio1_div_parents, 2, 1,
> +			    CLK_SET_RATE_PARENT);

I admit that those "fixed programmable" dividers are odd, but there are
fields in the PLL control register that we should use, so it's not a
fixed divider clock, but a programmable divider, using
SUNXI_CCU_M_HWS().

And I think you want the struct name to contain audio1, not periph1?

> +static CLK_FIXED_FACTOR_HWS(pll_periph1_div5_clk, "pll-audio1-div5",
> +			    pll_audio1_div_parents, 5, 1,
> +			    CLK_SET_RATE_PARENT);

Same here.

> +
> +static SUNXI_CCU_M_WITH_GATE(audio_out_clk, "audio-out",
> +			     "pll-audio1-div2", 0x01c,
> +			     0, 5, BIT(31), CLK_SET_RATE_PARENT);

I wonder if CLK_SET_RATE_PARENT is a good idea here. IIUC, then the
idea would be that the PLL is running at a fixed high rate (3072 MHz),
and gets divided down here to something more audio-y, like 48 or 96 MHz?

Do you have an idea what this clock is supposed to be used for? I
don't see it used anywhere, neither in this series, nor in the manual's
other clock descriptions or even pins.

> +
> +static const struct clk_parent_data dsp_parents[] = {
> +	{ .fw_name = "hosc" },
> +	{ .fw_name = "losc" },
> +	{ .fw_name = "iosc" },
> +	{ .hw = &pll_periph1_div5_clk.hw },
> +	{ .hw = &pll_periph1_div2_clk.hw },

The manual says that parent 0b011 is the DIV2 clock, and 0b100 is the
DIV5 clock, so those lines should be swapped.

> +	{ .fw_name = "dsp" },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(dsp_clk, "mcu-dsp", dsp_parents, 0x0020,
> +				      0, 5,	/* M */
> +				      24, 3,	/* mux */
> +				      BIT(31),	/* gate */
> +				      0);
> +
> +static const struct clk_parent_data i2s_parents[] = {
> +	{ .fw_name = "pll-audio0-4x" },
> +	{ .hw = &pll_periph1_div2_clk.hw },
> +	{ .hw = &pll_periph1_div5_clk.hw },
> +};
> +
> +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s0_clk, "i2s0", i2s_parents, 0x02c,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);

Same question about CLK_SET_RATE_PARENT here. Does the flag mean that any
rate request is only to be fulfilled by the parent? Couldn't find a good
explanation for that.

> +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s1_clk, "i2s1", i2s_parents, 0x030,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s2_clk, "i2s2", i2s_parents, 0x034,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_clk, "i2s3", i2s_parents, 0x038,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +
> +static const struct clk_parent_data i2s3_asrc_parents[] = {
> +	{ .fw_name = "pll-periph0-300m" },
> +	{ .hw = &pll_periph1_div2_clk.hw },
> +	{ .hw = &pll_periph1_div5_clk.hw },
> +};
> +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_asrc_clk, "i2s3-asrc",
> +				  i2s3_asrc_parents, 0x038,

that address should be 0x03c

> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_GATE_DATA(bus_i2s0_clk, "bus-i2s0", osc24M, 0x040, BIT(0), 0);

My manual says that APBS0 is the bus clock for the I2S peripherals. I
guess another one for the list of "clocks" in the DT binding :-(
This applies to the other bus clocks as well, they should be either APBS0
or AHB(_DEC0?).

> +static SUNXI_CCU_GATE_DATA(bus_i2s1_clk, "bus-i2s1", osc24M, 0x040, BIT(1), 0);
> +static SUNXI_CCU_GATE_DATA(bus_i2s2_clk, "bus-i2s2", osc24M, 0x040, BIT(2), 0);
> +static SUNXI_CCU_GATE_DATA(bus_i2s3_clk, "bus-i2s3", osc24M, 0x040, BIT(3), 0);
> +
> +static const struct clk_parent_data audio_parents[] = {
> +	{ .fw_name = "pll-audio0-4x" },
> +	{ .hw = &pll_periph1_div2_clk.hw },
> +	{ .hw = &pll_periph1_div5_clk.hw },
> +};
> +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_tx_clk, "spdif-tx",
> +				  audio_parents, 0x044,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_rx_clk, "spdif-rx",
> +				  i2s3_asrc_parents, 0x048,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_GATE_DATA(bus_spdif_clk, "bus-spdif", osc24M, 0x04c, BIT(0), 0);
> +
> +static SUNXI_CCU_DUALDIV_MUX_GATE(dmic_clk, "dmic", audio_parents, 0x050,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_GATE_DATA(bus_dmic_clk, "bus-dmic", osc24M, 0x054, BIT(0), 0);
> +
> +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_dac_clk, "audio-dac",
> +				  audio_parents, 0x058,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_adc_clk, "audio-adc",
> +				  audio_parents, 0x05c,
> +				  0, 5,		/* M */
> +				  5, 5,		/* P */
> +				  24, 3,	/* mux */
> +				  BIT(31),	/* gate */
> +				  CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_GATE_DATA(bus_audio_codec_clk, "bus-audio-codec",
> +			   osc24M, 0x060, BIT(0), 0);
> +
> +static SUNXI_CCU_GATE_DATA(bus_dsp_msgbox_clk, "bus-dsp-msgbox",
> +			   osc24M, 0x068, BIT(0), 0);
> +static SUNXI_CCU_GATE_DATA(bus_dsp_cfg_clk, "bus-dsp-cfg",
> +			   osc24M, 0x06c, BIT(0), 0);
> +
> +static SUNXI_CCU_GATE_DATA(bus_npu_hclk, "bus-npu-hclk", osc24M, 0x070, BIT(1), 0);
> +static SUNXI_CCU_GATE_DATA(bus_npu_aclk, "bus-npu-aclk", osc24M, 0x070, BIT(2), 0);
> +
> +static const struct clk_parent_data timer_parents[] = {
> +	{ .fw_name = "hosc" },
> +	{ .fw_name = "losc" },
> +	{ .fw_name = "iosc" },
> +	{ .fw_name = "r-ahb" }
> +};
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer0_clk, "mcu-timer0", timer_parents,
> +				      0x074,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer1_clk, "mcu-timer1", timer_parents,
> +				      0x078,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer2_clk, "mcu-timer2", timer_parents,
> +				      0x07c,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer3_clk, "mcu-timer3", timer_parents,
> +				      0x080,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer4_clk, "mcu-timer4", timer_parents,
> +				      0x084,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer5_clk, "mcu-timer5", timer_parents,
> +				      0x088,
> +				      1, 3,	/* P */
> +				      4, 2,	/* mux */
> +				      BIT(0),	/* gate */
> +				      0);
> +static SUNXI_CCU_GATE_DATA(bus_mcu_timer_clk, "bus-mcu-timer", osc24M, 0x08c, BIT(0), 0);
> +static SUNXI_CCU_GATE_DATA(bus_mcu_dma_clk, "bus-mcu-dma", osc24M, 0x104, BIT(0), 0);
> +static SUNXI_CCU_GATE_DATA(tzma0_clk, "tzma0", osc24M, 0x108, BIT(0), 0);
> +static SUNXI_CCU_GATE_DATA(tzma1_clk, "tzma1", osc24M, 0x10c, BIT(0), 0);

Where did you find those two? I guess in the BSP code? Can you maybe add a
comment about it then?

> +static SUNXI_CCU_GATE_DATA(bus_pubsram_clk, "bus-pubsram", osc24M, 0x114, BIT(0), 0);
> +
> +/*
> + * user manual has "mbus" clock as parent of both clocks below,
> + * but this makes more sense, since BSP MCU DMA controller has
> + * reference to both of them, likely needing both enabled.
> + */
> +static SUNXI_CCU_GATE_FW(mbus_mcu_clk, "mbus-mcu", "mbus", 0x11c, BIT(1), 0);
> +static SUNXI_CCU_GATE_HW(mbus_mcu_dma_clk, "mbus-mcu-dma",
> +			 &mbus_mcu_clk.common.hw, 0x11c, BIT(0), 0);
> +
> +static const struct clk_parent_data riscv_pwm_parents[] = {

Where does the pwm part come from? Is the clock below actually the RISC-V
PWM clock? Which would make more sense, since I don't see a PLL or any
other fast clock in there.

> +	{ .fw_name = "hosc" },
> +	{ .fw_name = "losc" },
> +	{ .fw_name = "iosc" },
> +};
> +
> +static SUNXI_CCU_MUX_DATA_WITH_GATE(riscv_clk, "riscv",

Related to above: what RISC-V clock is this exactly? Is that some PWM
clock source, as the parents name suggests, or the main clock, which looks
rather slow then? Or is that to select the root of the RISC-V clock tree?

> +				    riscv_pwm_parents, 0x120,
> +				    27, 3, BIT(31), 0);
> +static SUNXI_CCU_GATE_DATA(bus_riscv_cfg_clk, "bus-riscv-cfg", osc24M,
> +			   0x124, BIT(0), 0);
> +static SUNXI_CCU_GATE_DATA(bus_riscv_msgbox_clk, "bus-riscv-msgbox", osc24M,
> +			   0x128, BIT(0), 0);
> +
> +static SUNXI_CCU_MUX_DATA_WITH_GATE(mcu_pwmmcu0_clk, "mcu-pwm0",
> +				    riscv_pwm_parents, 0x130,
> +				    27, 3, BIT(31), 0);

The mux fields for this clock start at bit 24.

> +static SUNXI_CCU_GATE_DATA(bus_mcu_pwm0_clk, "bus-mcu-pwm0", osc24M,
> +			   0x128, BIT(0), 0);

The register offset is 0x134.

> +
> +/*
> + * Contains all clocks that are controlled by a hardware register. They
> + * have a (sunxi) .common member, which needs to be initialised by the common
> + * sunxi CCU code, to be filled with the MMIO base address and the shared lock.
> + */
> +static struct ccu_common *sun55i_a523_mcu_ccu_clks[] = {
> +	&pll_audio1_clk.common,
> +	&audio_out_clk.common,
> +	&dsp_clk.common,
> +	&i2s0_clk.common,
> +	&i2s1_clk.common,
> +	&i2s2_clk.common,
> +	&i2s3_clk.common,
> +	&i2s3_asrc_clk.common,
> +	&bus_i2s0_clk.common,
> +	&bus_i2s1_clk.common,
> +	&bus_i2s2_clk.common,
> +	&bus_i2s3_clk.common,
> +	&spdif_tx_clk.common,
> +	&spdif_rx_clk.common,
> +	&bus_spdif_clk.common,
> +	&dmic_clk.common,
> +	&bus_dmic_clk.common,
> +	&audio_dac_clk.common,
> +	&audio_adc_clk.common,
> +	&bus_audio_codec_clk.common,
> +	&bus_dsp_msgbox_clk.common,
> +	&bus_dsp_cfg_clk.common,
> +	&bus_npu_aclk.common,
> +	&bus_npu_hclk.common,
> +	&mcu_timer0_clk.common,
> +	&mcu_timer1_clk.common,
> +	&mcu_timer2_clk.common,
> +	&mcu_timer3_clk.common,
> +	&mcu_timer4_clk.common,
> +	&mcu_timer5_clk.common,
> +	&bus_mcu_timer_clk.common,
> +	&bus_mcu_dma_clk.common,
> +	&tzma0_clk.common,
> +	&tzma1_clk.common,
> +	&bus_pubsram_clk.common,
> +	&mbus_mcu_dma_clk.common,
> +	&mbus_mcu_clk.common,
> +	&riscv_clk.common,
> +	&bus_riscv_cfg_clk.common,
> +	&bus_riscv_msgbox_clk.common,
> +	&mcu_pwm0_clk.common,
> +	&bus_mcu_pwm0_clk.common,
> +};
> +
> +static struct clk_hw_onecell_data sun55i_a523_mcu_hw_clks = {
> +	.num	= CLK_BUS_MCU_PWM0 + 1,

like of the NPU patch, can we use ".num = NUM_CLOCKS," here, and
#define NUM_CLOCKS at the beginning of the file, right after including the
binding headers?
Or alternatively, put .num after the .hws definitions, so that last clock
and number are closer together?

> +	.hws	= {
> +		[CLK_MCU_PLL_AUDIO1]		= &pll_audio1_clk.common.hw,
> +		[CLK_MCU_PLL_AUDIO1_DIV2]	= &pll_periph1_div2_clk.hw,
> +		[CLK_MCU_PLL_AUDIO1_DIV5]	= &pll_periph1_div5_clk.hw,
> +		[CLK_MCU_AUDIO_OUT]		= &audio_out_clk.common.hw,
> +		[CLK_MCU_DSP]			= &dsp_clk.common.hw,
> +		[CLK_MCU_I2S0]			= &i2s0_clk.common.hw,
> +		[CLK_MCU_I2S1]			= &i2s1_clk.common.hw,
> +		[CLK_MCU_I2S2]			= &i2s2_clk.common.hw,
> +		[CLK_MCU_I2S3]			= &i2s3_clk.common.hw,
> +		[CLK_MCU_I2S3_ASRC]		= &i2s3_asrc_clk.common.hw,
> +		[CLK_BUS_MCU_I2S0]		= &bus_i2s0_clk.common.hw,
> +		[CLK_BUS_MCU_I2S1]		= &bus_i2s1_clk.common.hw,
> +		[CLK_BUS_MCU_I2S2]		= &bus_i2s2_clk.common.hw,
> +		[CLK_BUS_MCU_I2S3]		= &bus_i2s3_clk.common.hw,
> +		[CLK_MCU_SPDIF_TX]		= &spdif_tx_clk.common.hw,
> +		[CLK_MCU_SPDIF_RX]		= &spdif_rx_clk.common.hw,
> +		[CLK_BUS_MCU_SPDIF]		= &bus_spdif_clk.common.hw,
> +		[CLK_MCU_DMIC]			= &dmic_clk.common.hw,
> +		[CLK_BUS_MCU_DMIC]		= &bus_dmic_clk.common.hw,
> +		[CLK_MCU_AUDIO_CODEC_DAC]	= &audio_dac_clk.common.hw,
> +		[CLK_MCU_AUDIO_CODEC_ADC]	= &audio_adc_clk.common.hw,
> +		[CLK_BUS_MCU_AUDIO_CODEC]	= &bus_audio_codec_clk.common.hw,
> +		[CLK_BUS_MCU_DSP_MSGBOX]	= &bus_dsp_msgbox_clk.common.hw,
> +		[CLK_BUS_MCU_DSP_CFG]		= &bus_dsp_cfg_clk.common.hw,
> +		[CLK_BUS_MCU_NPU_HCLK]		= &bus_npu_hclk.common.hw,
> +		[CLK_BUS_MCU_NPU_ACLK]		= &bus_npu_aclk.common.hw,
> +		[CLK_MCU_TIMER0]		= &mcu_timer0_clk.common.hw,
> +		[CLK_MCU_TIMER1]		= &mcu_timer1_clk.common.hw,
> +		[CLK_MCU_TIMER2]		= &mcu_timer2_clk.common.hw,
> +		[CLK_MCU_TIMER3]		= &mcu_timer3_clk.common.hw,
> +		[CLK_MCU_TIMER4]		= &mcu_timer4_clk.common.hw,
> +		[CLK_MCU_TIMER5]		= &mcu_timer5_clk.common.hw,
> +		[CLK_BUS_MCU_TIMER]		= &bus_mcu_timer_clk.common.hw,
> +		[CLK_BUS_MCU_DMA]		= &bus_mcu_dma_clk.common.hw,
> +		[CLK_MCU_TZMA0]			= &tzma0_clk.common.hw,
> +		[CLK_MCU_TZMA1]			= &tzma1_clk.common.hw,
> +		[CLK_BUS_MCU_PUBSRAM]		= &bus_pubsram_clk.common.hw,
> +		[CLK_MCU_MBUS_DMA]		= &mbus_mcu_dma_clk.common.hw,
> +		[CLK_MCU_MBUS]			= &mbus_mcu_clk.common.hw,
> +		[CLK_MCU_RISCV]			= &riscv_clk.common.hw,
> +		[CLK_BUS_MCU_RISCV_CFG]		= &bus_riscv_cfg_clk.common.hw,
> +		[CLK_BUS_MCU_RISCV_MSGBOX]	= &bus_riscv_msgbox_clk.common.hw,
> +		[CLK_MCU_PWM0]			= &mcu_pwm0_clk.common.hw,
> +		[CLK_BUS_MCU_PWM0]		= &bus_mcu_pwm0_clk.common.hw,
> +	},
> +};
> +
> +static struct ccu_reset_map sun55i_a523_mcu_ccu_resets[] = {
> +	[RST_BUS_MCU_I2S0]		= { 0x0040, BIT(16) },
> +	[RST_BUS_MCU_I2S1]		= { 0x0040, BIT(17) },
> +	[RST_BUS_MCU_I2S2]		= { 0x0040, BIT(18) },
> +	[RST_BUS_MCU_I2S3]		= { 0x0040, BIT(19) },
> +	[RST_BUS_MCU_SPDIF]		= { 0x004c, BIT(16) },
> +	[RST_BUS_MCU_DMIC]		= { 0x0054, BIT(16) },
> +	[RST_BUS_MCU_AUDIO_CODEC]	= { 0x0060, BIT(16) },
> +	[RST_BUS_MCU_DSP_MSGBOX]	= { 0x0068, BIT(16) },
> +	[RST_BUS_MCU_DSP_CFG]		= { 0x006c, BIT(16) },
> +	[RST_BUS_MCU_NPU]		= { 0x0070, BIT(16) },
> +	[RST_BUS_MCU_TIMER]		= { 0x008c, BIT(16) },
> +	[RST_BUS_MCU_DSP_DEBUG]		= { 0x0100, BIT(16) },
> +	[RST_BUS_MCU_DSP]		= { 0x0100, BIT(17) },

I don't see those two in the manual.

> +	[RST_BUS_MCU_DMA]		= { 0x0104, BIT(16) },
> +	[RST_BUS_MCU_PUBSRAM]		= { 0x0114, BIT(16) },

The manual shows a reset bit in register 0x120, in the same register as
this ominous riscv_clk above.

> +	[RST_BUS_MCU_RISCV_CFG]		= { 0x0124, BIT(16) },
> +	[RST_BUS_MCU_RISCV_DEBUG]	= { 0x0124, BIT(17) },
> +	[RST_BUS_MCU_RISCV_CORE]	= { 0x0124, BIT(18) },
> +	[RST_BUS_MCU_RISCV_MSGBOX]	= { 0x0128, BIT(16) },

There is a reset bit for the PWM0 clock in the manual, register 0x130,
same as the mcu_pwmmcu0_clk above.


> +	[RST_BUS_MCU_PWM0]		= { 0x0134, BIT(16) },

Verified that all of the names defined in the binding headers appear here,
and all definitions here are mentioned in the binding headers.

> +};
> +
> +static const struct sunxi_ccu_desc sun55i_a523_mcu_ccu_desc = {
> +	.ccu_clks	= sun55i_a523_mcu_ccu_clks,
> +	.num_ccu_clks	= ARRAY_SIZE(sun55i_a523_mcu_ccu_clks),
> +
> +	.hw_clks	= &sun55i_a523_mcu_hw_clks,
> +
> +	.resets		= sun55i_a523_mcu_ccu_resets,
> +	.num_resets	= ARRAY_SIZE(sun55i_a523_mcu_ccu_resets),
> +};
> +
> +static int sun55i_a523_mcu_ccu_probe(struct platform_device *pdev)
> +{
> +	void __iomem *reg;
> +	u32 val;
> +	int ret;
> +
> +	reg = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	val = readl(reg + SUN55I_A523_PLL_AUDIO1_REG);
> +
> +	/*
> +	 * The PLL clock code does not model all bits, for instance it does
> +	 * not support a separate enable and gate bit. We present the
> +	 * gate bit(27) as the enable bit, but then have to set the
> +	 * PLL Enable, LDO Enable, and Lock Enable bits on all PLLs here.
> +	 */
> +	val |= BIT(31) | BIT(30) | BIT(29);
> +
> +	/* Enforce p1 = 5, p0 = 2 (the default) for PLL_AUDIO1 */
> +	val &= ~(GENMASK(22, 20) | GENMASK(18, 16));
> +	val |= (4 << 20) | (1 << 16);

Ah, I see, here you set those fixed dividers. I still think we should
model them properly, as suggested above.

Cheers,
Andre

> +
> +	writel(val, reg + SUN55I_A523_PLL_AUDIO1_REG);
> +
> +	ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun55i_a523_mcu_ccu_desc);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun55i_a523_mcu_ccu_ids[] = {
> +	{ .compatible = "allwinner,sun55i-a523-mcu-ccu" },
> +	{ }
> +};
> +
> +static struct platform_driver sun55i_a523_mcu_ccu_driver = {
> +	.probe	= sun55i_a523_mcu_ccu_probe,
> +	.driver	= {
> +		.name			= "sun55i-a523-mcu-ccu",
> +		.suppress_bind_attrs	= true,
> +		.of_match_table		= sun55i_a523_mcu_ccu_ids,
> +	},
> +};
> +module_platform_driver(sun55i_a523_mcu_ccu_driver);
> +
> +MODULE_IMPORT_NS("SUNXI_CCU");
> +MODULE_DESCRIPTION("Support for the Allwinner A523 MCU CCU");
> +MODULE_LICENSE("GPL");


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node
  2025-08-30 17:09 ` [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node Chen-Yu Tsai
@ 2025-09-05 15:14   ` Andre Przywara
  0 siblings, 0 replies; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:09:00 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

> From: Chen-Yu Tsai <wens@csie.org>
> 
> Add a device node for the third supported clock controller found in the
> A523 / T527 SoCs. This controller has clocks and resets for the RISC-V
> MCU, and others peripherals possibly meant to operate in low power mode
> driven by the MCU, such as audio interfaces, an audio DSP, and the NPU.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> index 79bd9ce08c7c..b6e82d53af54 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> @@ -4,8 +4,10 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/sun6i-rtc.h>
>  #include <dt-bindings/clock/sun55i-a523-ccu.h>
> +#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
>  #include <dt-bindings/clock/sun55i-a523-r-ccu.h>
>  #include <dt-bindings/reset/sun55i-a523-ccu.h>
> +#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
>  #include <dt-bindings/reset/sun55i-a523-r-ccu.h>
>  #include <dt-bindings/power/allwinner,sun55i-a523-ppu.h>
>  #include <dt-bindings/power/allwinner,sun55i-a523-pck-600.h>
> @@ -825,6 +827,29 @@ rtc: rtc@7090000 {
>  			clock-names = "bus", "hosc", "ahb";
>  			#clock-cells = <1>;
>  		};
> +
> +		mcu_ccu: clock-controller@7102000 {
> +			compatible = "allwinner,sun55i-a523-mcu-ccu";
> +			reg = <0x7102000 0x164>;

There is a register at 0x164, and even though the Linux driver doesn't use
it, we should let the region cover it. Maybe even going to 0x200?

The clock names match up with those .fw_name used in the driver, so with
the region size fixed:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> +			clocks = <&osc24M>,
> +				 <&rtc CLK_OSC32K>,
> +				 <&rtc CLK_IOSC>,
> +				 <&ccu CLK_PLL_AUDIO0_4X>,
> +				 <&ccu CLK_PLL_PERIPH0_300M>,
> +				 <&ccu CLK_DSP>,
> +				 <&r_ccu CLK_R_AHB>,
> +				 <&ccu CLK_MBUS>;
> +			clock-names = "hosc",
> +				      "losc",
> +				      "iosc",
> +				      "pll-audio0-4x",
> +				      "pll-periph0-300m",
> +				      "dsp",
> +				      "r-ahb",
> +				      "mbus";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
>  	};
>  
>  	thermal-zones {


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node
  2025-08-30 17:09 ` [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node Chen-Yu Tsai
@ 2025-09-05 15:14   ` Andre Przywara
  0 siblings, 0 replies; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 15:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-sunxi,
	linux-clk, linux-arm-kernel, devicetree, linux-kernel

On Sun, 31 Aug 2025 01:09:01 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

Hi,

> From: Chen-Yu Tsai <wens@csie.org>
> 
> The Allwinner T527 SoC has an NPU built in. Based on identifiers found
> in the BSP, it is a Vivante IP block. After enabling it, the etnaviv
> driver reports it as a GC9000 revision 9003.
> 
> The standard bindings are used as everything matches directly. There is
> no option for DVFS at the moment. That might require some more work,
> perhaps on the efuse side to map speed bins.
> 
> It is unclear whether the NPU block is fused out at the hardware level
> or the BSP limits use of the NPU through software, as the author only
> has boards with the T527.

I happen to only have boards without the NPU, one A523, two A527s, one
T527, but the SKU without the NPU, and a H728.
So I can confirm that the clock gates and resets exist, but the whole NPU
MMIO frame behaves as read-as-zero/write ignore. At least it doesn't
crash, and the Linux driver just skips this NPU as it cannot identify it
(with all the ID registers being 0).

So I think it's fine to have this node in all the DTBs. We *could* have
something in U-Boot that probes for this RAZ/WI behaviour and slaps a
status = "disabled"; on it. In which case it might be beneficial to have a
status node in already. But I'd rather avoid the churn and reliance on
firmware, instead try to auto detect as much as possible.

> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Matches the binding and the manual:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> index b6e82d53af54..1ab5b87ec78e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> @@ -850,6 +850,18 @@ mcu_ccu: clock-controller@7102000 {
>  			#clock-cells = <1>;
>  			#reset-cells = <1>;
>  		};
> +
> +		npu: npu@7122000 {
> +			compatible = "vivante,gc";
> +			reg = <0x07122000 0x1000>;
> +			interrupts = <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&mcu_ccu CLK_BUS_MCU_NPU_ACLK>,
> +				 <&ccu CLK_NPU>,
> +				 <&mcu_ccu CLK_BUS_MCU_NPU_HCLK>;
> +			clock-names = "bus", "core", "reg";
> +			resets = <&mcu_ccu RST_BUS_MCU_NPU>;
> +			power-domains = <&ppu PD_NPU>;
> +		};
>  	};
>  
>  	thermal-zones {


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback
  2025-09-05 15:12   ` Andre Przywara
@ 2025-09-05 15:17     ` Chen-Yu Tsai
  0 siblings, 0 replies; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-09-05 15:17 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Jernej Skrabec, Samuel Holland, linux-sunxi, linux-clk,
	linux-arm-kernel, devicetree, linux-kernel

On Fri, Sep 5, 2025 at 11:12 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 31 Aug 2025 01:08:56 +0800
> Chen-Yu Tsai <wens@kernel.org> wrote:
>
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > When dual-divider clock support was introduced, the P divider offset was
> > left out of the .recalc_rate readback function. This causes the clock
> > rate to become bogus or even zero (possibly due to the P divider being
> > 1, leading to a divide-by-zero).
>
> Ah, a nice catch, thanks for that! Just curious, how did you find this?
> The MMC clocks use the dual divider type as well, but I didn't observe
> them being wrong?

I was looking at clk_summary in debugfs and kept seeing some of the audio
clocks returning 0 and dumped the register to get the actual value. I then
did a printk trace of the .determine_rate callback and saw that one of the
dividers was always 1. I guessed that the .recalc_rate callback was to blame.

> Regardless:
>
> > Fix this by incorporating the P divider offset into the calculation.
> >
> > Fixes: 45717804b75e ("clk: sunxi-ng: mp: introduce dual-divider clock")
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks!


> Thanks,
> Andre
>
> > ---
> >  drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> > index 354c981943b6..4221b1888b38 100644
> > --- a/drivers/clk/sunxi-ng/ccu_mp.c
> > +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> > @@ -185,7 +185,7 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw,
> >       p &= (1 << cmp->p.width) - 1;
> >
> >       if (cmp->common.features & CCU_FEATURE_DUAL_DIV)
> > -             rate = (parent_rate / p) / m;
> > +             rate = (parent_rate / (p + cmp->p.offset)) / m;
> >       else
> >               rate = (parent_rate >> p) / m;
> >
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock
  2025-09-05 15:14   ` Andre Przywara
@ 2025-09-05 15:19     ` Chen-Yu Tsai
  0 siblings, 0 replies; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-09-05 15:19 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Jernej Skrabec, Samuel Holland, linux-sunxi, linux-clk,
	linux-arm-kernel, devicetree, linux-kernel

On Fri, Sep 5, 2025 at 11:14 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 31 Aug 2025 01:08:57 +0800
> Chen-Yu Tsai <wens@kernel.org> wrote:
>
> Hi,
>
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The main clock controller on the A523/T527 has the NPU's module clock.
> > It was missing from the original submission, likely because that was
> > based on the A523 user manual; the A523 is marketed without the NPU.
>
> Ah, sorry, I missed that one. I think I spotted writable bits in that
> register, but didn't find a clue what this clock was about. Anyway, checked
> the bits against the T527 manual, they match up.
>
> > Also, merge the private header back into the driver code itself. The
> > header only contains a macro containing the total number of clocks.
> > This has to be updated every time a missing clock gets added. Having
> > it in a separate file doesn't help the process. Instead just drop the
> > macro, and thus the header no longer has any reason to exist.
>
> Interesting, looks nice, and solves Krzysztof's complaint the other
> day about the binding header inclusion missing from the driver as well.
> Just one thought:
>
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >  drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 21 ++++++++++++++++++---
> >  drivers/clk/sunxi-ng/ccu-sun55i-a523.h | 14 --------------
> >  2 files changed, 18 insertions(+), 17 deletions(-)
> >  delete mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > index 1a9a1cb869e2..88405b624dc5 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > @@ -11,6 +11,9 @@
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >
> > +#include <dt-bindings/clock/sun55i-a523-ccu.h>
> > +#include <dt-bindings/reset/sun55i-a523-ccu.h>
> > +
>
> Should we have the number #define here, at a more central location? Seems a
> bit buried down in there. And then use a plural name while at it:
>
> #define NUM_CLOCKS      CLK_NPU + 1
>
> Alternatively, put .num behind .hws below, so that the last clock and the
> number definition are close together?

I think this works better. One less place to look at.

ChenYu

> Cheers,
> Andre
>
> >  #include "../clk.h"
> >
> >  #include "ccu_common.h"
> > @@ -25,8 +28,6 @@
> >  #include "ccu_nkmp.h"
> >  #include "ccu_nm.h"
> >
> > -#include "ccu-sun55i-a523.h"
> > -
> >  /*
> >   * The 24 MHz oscillator, the root of most of the clock tree.
> >   * .fw_name is the string used in the DT "clock-names" property, used to
> > @@ -486,6 +487,18 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
> >
> >  static SUNXI_CCU_GATE_HWS(bus_ve_clk, "bus-ve", ahb_hws, 0x69c, BIT(0), 0);
> >
> > +static const struct clk_hw *npu_parents[] = {
> > +     &pll_periph0_480M_clk.common.hw,
> > +     &pll_periph0_600M_clk.hw,
> > +     &pll_periph0_800M_clk.common.hw,
> > +     &pll_npu_2x_clk.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(npu_clk, "npu", npu_parents, 0x6e0,
> > +                                 0, 5,       /* M */
> > +                                 24, 3,      /* mux */
> > +                                 BIT(31),    /* gate */
> > +                                 CLK_SET_RATE_PARENT);
> > +
> >  static SUNXI_CCU_GATE_HWS(bus_dma_clk, "bus-dma", ahb_hws, 0x70c, BIT(0), 0);
> >
> >  static SUNXI_CCU_GATE_HWS(bus_msgbox_clk, "bus-msgbox", ahb_hws, 0x71c,
> > @@ -1217,6 +1230,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> >       &bus_ce_sys_clk.common,
> >       &ve_clk.common,
> >       &bus_ve_clk.common,
> > +     &npu_clk.common,
> >       &bus_dma_clk.common,
> >       &bus_msgbox_clk.common,
> >       &bus_spinlock_clk.common,
> > @@ -1343,7 +1357,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> >  };
> >
> >  static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> > -     .num    = CLK_NUMBER,
> > +     .num    = CLK_NPU + 1,
> >       .hws    = {
> >               [CLK_PLL_DDR0]          = &pll_ddr_clk.common.hw,
> >               [CLK_PLL_PERIPH0_4X]    = &pll_periph0_4x_clk.common.hw,
> > @@ -1524,6 +1538,7 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> >               [CLK_FANOUT0]           = &fanout0_clk.common.hw,
> >               [CLK_FANOUT1]           = &fanout1_clk.common.hw,
> >               [CLK_FANOUT2]           = &fanout2_clk.common.hw,
> > +             [CLK_NPU]               = &npu_clk.common.hw,
> >       },
> >  };
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h b/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> > deleted file mode 100644
> > index fc8dd42f1b47..000000000000
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> > +++ /dev/null
> > @@ -1,14 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -/*
> > - * Copyright 2024 Arm Ltd.
> > - */
> > -
> > -#ifndef _CCU_SUN55I_A523_H
> > -#define _CCU_SUN55I_A523_H
> > -
> > -#include <dt-bindings/clock/sun55i-a523-ccu.h>
> > -#include <dt-bindings/reset/sun55i-a523-ccu.h>
> > -
> > -#define CLK_NUMBER   (CLK_FANOUT2 + 1)
> > -
> > -#endif /* _CCU_SUN55I_A523_H */
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU
  2025-09-05 15:14   ` Andre Przywara
@ 2025-09-05 16:13     ` Chen-Yu Tsai
  2025-09-05 17:21       ` Andre Przywara
  0 siblings, 1 reply; 21+ messages in thread
From: Chen-Yu Tsai @ 2025-09-05 16:13 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Jernej Skrabec, Samuel Holland, linux-sunxi, linux-clk,
	linux-arm-kernel, devicetree, linux-kernel

On Fri, Sep 5, 2025 at 11:14 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 31 Aug 2025 01:08:59 +0800
> Chen-Yu Tsai <wens@kernel.org> wrote:
>
> Hi Chen-Yu,
>
> many thanks for this patch, I feel with you when it comes to model
> Allwinner CCUs in the kernel ;-)
>
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The A523/T527 SoCs have a new MCU PRCM, which has more clocks and reset
> > controls for the RISC-V MCU and other peripherals. There is no visible
> > bus in this part, but there is a second audio PLL. The BSP driver uses
> > the 24MHz main oscillator as the parent for all the bus clocks.
>
> So my copy of the T527 manual (v0.92) shows the system but tree of the
> MCU_PRCM in figure 2-24, and there some peripherals are on AHB_DEC0, while
> others are on APBS0. Shall we model this correctly, then?

The figure was a bit misleading as it had at its root "CPU" and "MCU_AHB"
instead of "CPUX". I guess given that we can actually access these devices,
they should be the same. It was weird because there were no bus clock
dividers in this block.

> > Add a driver to support this part. Unlike the BSP driver, the SoC's main
> > MBUS clock is chosen as the parent for the MCU MBUS clock, and the
> > latter then serves as the parent of the MCU DMA controller's MBUS clock.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >  drivers/clk/sunxi-ng/Kconfig               |   5 +
> >  drivers/clk/sunxi-ng/Makefile              |   2 +
> >  drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c | 447 +++++++++++++++++++++
> >  3 files changed, 454 insertions(+)
> >  create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> >
> > diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> > index 8896fd052ef1..6af2d020e03e 100644
> > --- a/drivers/clk/sunxi-ng/Kconfig
> > +++ b/drivers/clk/sunxi-ng/Kconfig
> > @@ -57,6 +57,11 @@ config SUN55I_A523_CCU
> >       default ARCH_SUNXI
> >       depends on ARM64 || COMPILE_TEST
> >
> > +config SUN55I_A523_MCU_CCU
> > +     tristate "Support for the Allwinner A523/T527 MCU CCU"
> > +     default ARCH_SUNXI
> > +     depends on ARM64 || COMPILE_TEST
> > +
> >  config SUN55I_A523_R_CCU
> >       tristate "Support for the Allwinner A523/T527 PRCM CCU"
> >       default ARCH_SUNXI
> > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> > index 82e471036de6..a1c4087d7241 100644
> > --- a/drivers/clk/sunxi-ng/Makefile
> > +++ b/drivers/clk/sunxi-ng/Makefile
> > @@ -34,6 +34,7 @@ obj-$(CONFIG_SUN50I_H6_CCU) += sun50i-h6-ccu.o
> >  obj-$(CONFIG_SUN50I_H6_R_CCU)        += sun50i-h6-r-ccu.o
> >  obj-$(CONFIG_SUN50I_H616_CCU)        += sun50i-h616-ccu.o
> >  obj-$(CONFIG_SUN55I_A523_CCU)        += sun55i-a523-ccu.o
> > +obj-$(CONFIG_SUN55I_A523_MCU_CCU)    += sun55i-a523-mcu-ccu.o
> >  obj-$(CONFIG_SUN55I_A523_R_CCU)      += sun55i-a523-r-ccu.o
> >  obj-$(CONFIG_SUN4I_A10_CCU)  += sun4i-a10-ccu.o
> >  obj-$(CONFIG_SUN5I_CCU)              += sun5i-ccu.o
> > @@ -61,6 +62,7 @@ sun50i-h6-ccu-y                     += ccu-sun50i-h6.o
> >  sun50i-h6-r-ccu-y            += ccu-sun50i-h6-r.o
> >  sun50i-h616-ccu-y            += ccu-sun50i-h616.o
> >  sun55i-a523-ccu-y            += ccu-sun55i-a523.o
> > +sun55i-a523-mcu-ccu-y                += ccu-sun55i-a523-mcu.o
> >  sun55i-a523-r-ccu-y          += ccu-sun55i-a523-r.o
> >  sun4i-a10-ccu-y                      += ccu-sun4i-a10.o
> >  sun5i-ccu-y                  += ccu-sun5i.o
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> > new file mode 100644
> > index 000000000000..6105485837c9
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> > @@ -0,0 +1,447 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
> > + *
> > + * Based on the A523 CCU driver:
> > + *   Copyright (C) 2023-2024 Arm Ltd.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
> > +#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
> > +
> > +#include "ccu_common.h"
> > +#include "ccu_reset.h"
> > +
> > +#include "ccu_div.h"
> > +#include "ccu_gate.h"
> > +#include "ccu_mp.h"
> > +#include "ccu_mult.h"
> > +#include "ccu_nm.h"
> > +
> > +static const struct clk_parent_data osc24M[] = {
> > +     { .fw_name = "hosc" }
> > +};
> > +
> > +#define SUN55I_A523_PLL_AUDIO1_REG   0x00c
> > +static struct ccu_sdm_setting pll_audio1_sdm_table[] = {
> > +     { .rate = 2167603200, .pattern = 0xa000a234, .m = 1, .n = 90 }, /* div2->22.5792 */
> > +     { .rate = 2359296000, .pattern = 0xa0009ba6, .m = 1, .n = 98 }, /* div2->24.576 */
> > +     { .rate = 1806336000, .pattern = 0xa000872b, .m = 1, .n = 75 }, /* div5->22.576 */
> > +};
> > +
> > +static struct ccu_nm pll_audio1_clk = {
> > +     .enable         = BIT(27),
> > +     .lock           = BIT(28),
> > +     .n              = _SUNXI_CCU_MULT_MIN(8, 8, 11),
> > +     .m              = _SUNXI_CCU_DIV(1, 1),
> > +     .sdm            = _SUNXI_CCU_SDM(pll_audio1_sdm_table, BIT(24),
> > +                                      0x010, BIT(31)),
> > +     .min_rate       = 180000000U,
> > +     .max_rate       = 3500000000U,
> > +     .common         = {
> > +             .reg            = 0x00c,
> > +             .features       = CCU_FEATURE_SIGMA_DELTA_MOD,
> > +             .hw.init        = CLK_HW_INIT_PARENTS_DATA("pll-audio1",
> > +                                                        osc24M, &ccu_nm_ops,
> > +                                                        CLK_SET_RATE_GATE),
> > +     },
> > +};
> > +
> > +static const struct clk_hw *pll_audio1_div_parents[] = { &pll_audio1_clk.common.hw };
> > +static CLK_FIXED_FACTOR_HWS(pll_periph1_div2_clk, "pll-audio1-div2",
> > +                         pll_audio1_div_parents, 2, 1,
> > +                         CLK_SET_RATE_PARENT);
>
> I admit that those "fixed programmable" dividers are odd, but there are
> fields in the PLL control register that we should use, so it's not a
> fixed divider clock, but a programmable divider, using
> SUNXI_CCU_M_HWS().

As you found out below, this is programmed to be fixed.

> And I think you want the struct name to contain audio1, not periph1?

Correct. Copy-paste thing.

> > +static CLK_FIXED_FACTOR_HWS(pll_periph1_div5_clk, "pll-audio1-div5",
> > +                         pll_audio1_div_parents, 5, 1,
> > +                         CLK_SET_RATE_PARENT);
>
> Same here.
>
> > +
> > +static SUNXI_CCU_M_WITH_GATE(audio_out_clk, "audio-out",
> > +                          "pll-audio1-div2", 0x01c,
> > +                          0, 5, BIT(31), CLK_SET_RATE_PARENT);
>
> I wonder if CLK_SET_RATE_PARENT is a good idea here. IIUC, then the
> idea would be that the PLL is running at a fixed high rate (3072 MHz),
> and gets divided down here to something more audio-y, like 48 or 96 MHz?

No. For audio there are only two classes of clock rates that matter:

 - multiple of 24.576 MHz for 48 KHz family of sample rates
 - multiple of 22.5792 MHz for 44.1 KHz family of sample rates

The PLL has to be able to switch between these. The SDM table below
contains predefined values from the BSP.

> Do you have an idea what this clock is supposed to be used for? I
> don't see it used anywhere, neither in this series, nor in the manual's
> other clock descriptions or even pins.

I've no idea, but didn't want to leave it out and then later on someone
has to add it.

> > +
> > +static const struct clk_parent_data dsp_parents[] = {
> > +     { .fw_name = "hosc" },
> > +     { .fw_name = "losc" },
> > +     { .fw_name = "iosc" },
> > +     { .hw = &pll_periph1_div5_clk.hw },
> > +     { .hw = &pll_periph1_div2_clk.hw },
>
> The manual says that parent 0b011 is the DIV2 clock, and 0b100 is the
> DIV5 clock, so those lines should be swapped.

BSP has this order, so I'm confused as you are. I can just leave a note
here saying the order is from the BSP, which is different from the manual.

Until someone actually plays with the DSP, we won't know which one is correct.

> > +     { .fw_name = "dsp" },
> > +};
> > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(dsp_clk, "mcu-dsp", dsp_parents, 0x0020,
> > +                                   0, 5,     /* M */
> > +                                   24, 3,    /* mux */
> > +                                   BIT(31),  /* gate */
> > +                                   0);
> > +
> > +static const struct clk_parent_data i2s_parents[] = {
> > +     { .fw_name = "pll-audio0-4x" },
> > +     { .hw = &pll_periph1_div2_clk.hw },
> > +     { .hw = &pll_periph1_div5_clk.hw },
> > +};
> > +
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s0_clk, "i2s0", i2s_parents, 0x02c,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
>
> Same question about CLK_SET_RATE_PARENT here. Does the flag mean that any
> rate request is only to be fulfilled by the parent? Couldn't find a good
> explanation for that.

AFAIK the flag is _supposed_ to mean that during a rate change, the clk
provider is free to request a different rate from the parent, one that
is suitable for its own use. This actually depends on clk drivers correctly
implementing it in their .determine_rate callbacks though.

> > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s1_clk, "i2s1", i2s_parents, 0x030,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s2_clk, "i2s2", i2s_parents, 0x034,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_clk, "i2s3", i2s_parents, 0x038,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +
> > +static const struct clk_parent_data i2s3_asrc_parents[] = {
> > +     { .fw_name = "pll-periph0-300m" },
> > +     { .hw = &pll_periph1_div2_clk.hw },
> > +     { .hw = &pll_periph1_div5_clk.hw },
> > +};
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_asrc_clk, "i2s3-asrc",
> > +                               i2s3_asrc_parents, 0x038,
>
> that address should be 0x03c

It might have been a "copy-paste and then forget to update it" situation.
It was around midnight when I wrote this driver.

> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_i2s0_clk, "bus-i2s0", osc24M, 0x040, BIT(0), 0);
>
> My manual says that APBS0 is the bus clock for the I2S peripherals. I
> guess another one for the list of "clocks" in the DT binding :-(
> This applies to the other bus clocks as well, they should be either APBS0
> or AHB(_DEC0?).

OK.

> > +static SUNXI_CCU_GATE_DATA(bus_i2s1_clk, "bus-i2s1", osc24M, 0x040, BIT(1), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_i2s2_clk, "bus-i2s2", osc24M, 0x040, BIT(2), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_i2s3_clk, "bus-i2s3", osc24M, 0x040, BIT(3), 0);
> > +
> > +static const struct clk_parent_data audio_parents[] = {
> > +     { .fw_name = "pll-audio0-4x" },
> > +     { .hw = &pll_periph1_div2_clk.hw },
> > +     { .hw = &pll_periph1_div5_clk.hw },
> > +};
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_tx_clk, "spdif-tx",
> > +                               audio_parents, 0x044,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_rx_clk, "spdif-rx",
> > +                               i2s3_asrc_parents, 0x048,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_spdif_clk, "bus-spdif", osc24M, 0x04c, BIT(0), 0);
> > +
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(dmic_clk, "dmic", audio_parents, 0x050,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_dmic_clk, "bus-dmic", osc24M, 0x054, BIT(0), 0);
> > +
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_dac_clk, "audio-dac",
> > +                               audio_parents, 0x058,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_adc_clk, "audio-adc",
> > +                               audio_parents, 0x05c,
> > +                               0, 5,         /* M */
> > +                               5, 5,         /* P */
> > +                               24, 3,        /* mux */
> > +                               BIT(31),      /* gate */
> > +                               CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_audio_codec_clk, "bus-audio-codec",
> > +                        osc24M, 0x060, BIT(0), 0);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_dsp_msgbox_clk, "bus-dsp-msgbox",
> > +                        osc24M, 0x068, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_dsp_cfg_clk, "bus-dsp-cfg",
> > +                        osc24M, 0x06c, BIT(0), 0);
> > +
> > +static SUNXI_CCU_GATE_DATA(bus_npu_hclk, "bus-npu-hclk", osc24M, 0x070, BIT(1), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_npu_aclk, "bus-npu-aclk", osc24M, 0x070, BIT(2), 0);
> > +
> > +static const struct clk_parent_data timer_parents[] = {
> > +     { .fw_name = "hosc" },
> > +     { .fw_name = "losc" },
> > +     { .fw_name = "iosc" },
> > +     { .fw_name = "r-ahb" }
> > +};
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer0_clk, "mcu-timer0", timer_parents,
> > +                                   0x074,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer1_clk, "mcu-timer1", timer_parents,
> > +                                   0x078,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer2_clk, "mcu-timer2", timer_parents,
> > +                                   0x07c,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer3_clk, "mcu-timer3", timer_parents,
> > +                                   0x080,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer4_clk, "mcu-timer4", timer_parents,
> > +                                   0x084,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer5_clk, "mcu-timer5", timer_parents,
> > +                                   0x088,
> > +                                   1, 3,     /* P */
> > +                                   4, 2,     /* mux */
> > +                                   BIT(0),   /* gate */
> > +                                   0);
> > +static SUNXI_CCU_GATE_DATA(bus_mcu_timer_clk, "bus-mcu-timer", osc24M, 0x08c, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_mcu_dma_clk, "bus-mcu-dma", osc24M, 0x104, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA(tzma0_clk, "tzma0", osc24M, 0x108, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA(tzma1_clk, "tzma1", osc24M, 0x10c, BIT(0), 0);
>
> Where did you find those two? I guess in the BSP code? Can you maybe add a
> comment about it then?

OK.

> > +static SUNXI_CCU_GATE_DATA(bus_pubsram_clk, "bus-pubsram", osc24M, 0x114, BIT(0), 0);
> > +
> > +/*
> > + * user manual has "mbus" clock as parent of both clocks below,
> > + * but this makes more sense, since BSP MCU DMA controller has
> > + * reference to both of them, likely needing both enabled.
> > + */
> > +static SUNXI_CCU_GATE_FW(mbus_mcu_clk, "mbus-mcu", "mbus", 0x11c, BIT(1), 0);
> > +static SUNXI_CCU_GATE_HW(mbus_mcu_dma_clk, "mbus-mcu-dma",
> > +                      &mbus_mcu_clk.common.hw, 0x11c, BIT(0), 0);
> > +
> > +static const struct clk_parent_data riscv_pwm_parents[] = {
>
> Where does the pwm part come from? Is the clock below actually the RISC-V
> PWM clock? Which would make more sense, since I don't see a PLL or any
> other fast clock in there.

I'm reusing the parent list here, since the list is the same for the riscv
mod clock and the pwm0 mod clock, hence the name has riscv + pwm.

> > +     { .fw_name = "hosc" },
> > +     { .fw_name = "losc" },
> > +     { .fw_name = "iosc" },
> > +};
> > +
> > +static SUNXI_CCU_MUX_DATA_WITH_GATE(riscv_clk, "riscv",
>
> Related to above: what RISC-V clock is this exactly? Is that some PWM
> clock source, as the parents name suggests, or the main clock, which looks
> rather slow then? Or is that to select the root of the RISC-V clock tree?

From https://github.com/radxa/allwinner-bsp/blob/product-t527-linux/configs/linux-5.15/sun55iw3p1.dtsi#L2981
it looks like it is the actual clock for the risc-v core. Given that it
is supposed to be a low power standby thing, I guess Allwinner didn't
think it was necessary to have anything faster.

> > +                                 riscv_pwm_parents, 0x120,
> > +                                 27, 3, BIT(31), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_riscv_cfg_clk, "bus-riscv-cfg", osc24M,
> > +                        0x124, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA(bus_riscv_msgbox_clk, "bus-riscv-msgbox", osc24M,
> > +                        0x128, BIT(0), 0);
> > +
> > +static SUNXI_CCU_MUX_DATA_WITH_GATE(mcu_pwmmcu0_clk, "mcu-pwm0",
> > +                                 riscv_pwm_parents, 0x130,
> > +                                 27, 3, BIT(31), 0);
>
> The mux fields for this clock start at bit 24.

Oops.

> > +static SUNXI_CCU_GATE_DATA(bus_mcu_pwm0_clk, "bus-mcu-pwm0", osc24M,
> > +                        0x128, BIT(0), 0);
>
> The register offset is 0x134.

(facepalm)

> > +
> > +/*
> > + * Contains all clocks that are controlled by a hardware register. They
> > + * have a (sunxi) .common member, which needs to be initialised by the common
> > + * sunxi CCU code, to be filled with the MMIO base address and the shared lock.
> > + */
> > +static struct ccu_common *sun55i_a523_mcu_ccu_clks[] = {
> > +     &pll_audio1_clk.common,
> > +     &audio_out_clk.common,
> > +     &dsp_clk.common,
> > +     &i2s0_clk.common,
> > +     &i2s1_clk.common,
> > +     &i2s2_clk.common,
> > +     &i2s3_clk.common,
> > +     &i2s3_asrc_clk.common,
> > +     &bus_i2s0_clk.common,
> > +     &bus_i2s1_clk.common,
> > +     &bus_i2s2_clk.common,
> > +     &bus_i2s3_clk.common,
> > +     &spdif_tx_clk.common,
> > +     &spdif_rx_clk.common,
> > +     &bus_spdif_clk.common,
> > +     &dmic_clk.common,
> > +     &bus_dmic_clk.common,
> > +     &audio_dac_clk.common,
> > +     &audio_adc_clk.common,
> > +     &bus_audio_codec_clk.common,
> > +     &bus_dsp_msgbox_clk.common,
> > +     &bus_dsp_cfg_clk.common,
> > +     &bus_npu_aclk.common,
> > +     &bus_npu_hclk.common,
> > +     &mcu_timer0_clk.common,
> > +     &mcu_timer1_clk.common,
> > +     &mcu_timer2_clk.common,
> > +     &mcu_timer3_clk.common,
> > +     &mcu_timer4_clk.common,
> > +     &mcu_timer5_clk.common,
> > +     &bus_mcu_timer_clk.common,
> > +     &bus_mcu_dma_clk.common,
> > +     &tzma0_clk.common,
> > +     &tzma1_clk.common,
> > +     &bus_pubsram_clk.common,
> > +     &mbus_mcu_dma_clk.common,
> > +     &mbus_mcu_clk.common,
> > +     &riscv_clk.common,
> > +     &bus_riscv_cfg_clk.common,
> > +     &bus_riscv_msgbox_clk.common,
> > +     &mcu_pwm0_clk.common,
> > +     &bus_mcu_pwm0_clk.common,
> > +};
> > +
> > +static struct clk_hw_onecell_data sun55i_a523_mcu_hw_clks = {
> > +     .num    = CLK_BUS_MCU_PWM0 + 1,
>
> like of the NPU patch, can we use ".num = NUM_CLOCKS," here, and
> #define NUM_CLOCKS at the beginning of the file, right after including the
> binding headers?
> Or alternatively, put .num after the .hws definitions, so that last clock
> and number are closer together?

As mentioned in that patch, I prefer the latter.

> > +     .hws    = {
> > +             [CLK_MCU_PLL_AUDIO1]            = &pll_audio1_clk.common.hw,
> > +             [CLK_MCU_PLL_AUDIO1_DIV2]       = &pll_periph1_div2_clk.hw,
> > +             [CLK_MCU_PLL_AUDIO1_DIV5]       = &pll_periph1_div5_clk.hw,
> > +             [CLK_MCU_AUDIO_OUT]             = &audio_out_clk.common.hw,
> > +             [CLK_MCU_DSP]                   = &dsp_clk.common.hw,
> > +             [CLK_MCU_I2S0]                  = &i2s0_clk.common.hw,
> > +             [CLK_MCU_I2S1]                  = &i2s1_clk.common.hw,
> > +             [CLK_MCU_I2S2]                  = &i2s2_clk.common.hw,
> > +             [CLK_MCU_I2S3]                  = &i2s3_clk.common.hw,
> > +             [CLK_MCU_I2S3_ASRC]             = &i2s3_asrc_clk.common.hw,
> > +             [CLK_BUS_MCU_I2S0]              = &bus_i2s0_clk.common.hw,
> > +             [CLK_BUS_MCU_I2S1]              = &bus_i2s1_clk.common.hw,
> > +             [CLK_BUS_MCU_I2S2]              = &bus_i2s2_clk.common.hw,
> > +             [CLK_BUS_MCU_I2S3]              = &bus_i2s3_clk.common.hw,
> > +             [CLK_MCU_SPDIF_TX]              = &spdif_tx_clk.common.hw,
> > +             [CLK_MCU_SPDIF_RX]              = &spdif_rx_clk.common.hw,
> > +             [CLK_BUS_MCU_SPDIF]             = &bus_spdif_clk.common.hw,
> > +             [CLK_MCU_DMIC]                  = &dmic_clk.common.hw,
> > +             [CLK_BUS_MCU_DMIC]              = &bus_dmic_clk.common.hw,
> > +             [CLK_MCU_AUDIO_CODEC_DAC]       = &audio_dac_clk.common.hw,
> > +             [CLK_MCU_AUDIO_CODEC_ADC]       = &audio_adc_clk.common.hw,
> > +             [CLK_BUS_MCU_AUDIO_CODEC]       = &bus_audio_codec_clk.common.hw,
> > +             [CLK_BUS_MCU_DSP_MSGBOX]        = &bus_dsp_msgbox_clk.common.hw,
> > +             [CLK_BUS_MCU_DSP_CFG]           = &bus_dsp_cfg_clk.common.hw,
> > +             [CLK_BUS_MCU_NPU_HCLK]          = &bus_npu_hclk.common.hw,
> > +             [CLK_BUS_MCU_NPU_ACLK]          = &bus_npu_aclk.common.hw,
> > +             [CLK_MCU_TIMER0]                = &mcu_timer0_clk.common.hw,
> > +             [CLK_MCU_TIMER1]                = &mcu_timer1_clk.common.hw,
> > +             [CLK_MCU_TIMER2]                = &mcu_timer2_clk.common.hw,
> > +             [CLK_MCU_TIMER3]                = &mcu_timer3_clk.common.hw,
> > +             [CLK_MCU_TIMER4]                = &mcu_timer4_clk.common.hw,
> > +             [CLK_MCU_TIMER5]                = &mcu_timer5_clk.common.hw,
> > +             [CLK_BUS_MCU_TIMER]             = &bus_mcu_timer_clk.common.hw,
> > +             [CLK_BUS_MCU_DMA]               = &bus_mcu_dma_clk.common.hw,
> > +             [CLK_MCU_TZMA0]                 = &tzma0_clk.common.hw,
> > +             [CLK_MCU_TZMA1]                 = &tzma1_clk.common.hw,
> > +             [CLK_BUS_MCU_PUBSRAM]           = &bus_pubsram_clk.common.hw,
> > +             [CLK_MCU_MBUS_DMA]              = &mbus_mcu_dma_clk.common.hw,
> > +             [CLK_MCU_MBUS]                  = &mbus_mcu_clk.common.hw,
> > +             [CLK_MCU_RISCV]                 = &riscv_clk.common.hw,
> > +             [CLK_BUS_MCU_RISCV_CFG]         = &bus_riscv_cfg_clk.common.hw,
> > +             [CLK_BUS_MCU_RISCV_MSGBOX]      = &bus_riscv_msgbox_clk.common.hw,
> > +             [CLK_MCU_PWM0]                  = &mcu_pwm0_clk.common.hw,
> > +             [CLK_BUS_MCU_PWM0]              = &bus_mcu_pwm0_clk.common.hw,
> > +     },
> > +};
> > +
> > +static struct ccu_reset_map sun55i_a523_mcu_ccu_resets[] = {
> > +     [RST_BUS_MCU_I2S0]              = { 0x0040, BIT(16) },
> > +     [RST_BUS_MCU_I2S1]              = { 0x0040, BIT(17) },
> > +     [RST_BUS_MCU_I2S2]              = { 0x0040, BIT(18) },
> > +     [RST_BUS_MCU_I2S3]              = { 0x0040, BIT(19) },
> > +     [RST_BUS_MCU_SPDIF]             = { 0x004c, BIT(16) },
> > +     [RST_BUS_MCU_DMIC]              = { 0x0054, BIT(16) },
> > +     [RST_BUS_MCU_AUDIO_CODEC]       = { 0x0060, BIT(16) },
> > +     [RST_BUS_MCU_DSP_MSGBOX]        = { 0x0068, BIT(16) },
> > +     [RST_BUS_MCU_DSP_CFG]           = { 0x006c, BIT(16) },
> > +     [RST_BUS_MCU_NPU]               = { 0x0070, BIT(16) },
> > +     [RST_BUS_MCU_TIMER]             = { 0x008c, BIT(16) },
> > +     [RST_BUS_MCU_DSP_DEBUG]         = { 0x0100, BIT(16) },
> > +     [RST_BUS_MCU_DSP]               = { 0x0100, BIT(17) },
>
> I don't see those two in the manual.
>
> > +     [RST_BUS_MCU_DMA]               = { 0x0104, BIT(16) },
> > +     [RST_BUS_MCU_PUBSRAM]           = { 0x0114, BIT(16) },
>
> The manual shows a reset bit in register 0x120, in the same register as
> this ominous riscv_clk above.

Hmm. I don't see this in the T527 manual (0.92), nor the A523 manuals
(both 1.1 and 1.4), nor the BSP code.

> > +     [RST_BUS_MCU_RISCV_CFG]         = { 0x0124, BIT(16) },
> > +     [RST_BUS_MCU_RISCV_DEBUG]       = { 0x0124, BIT(17) },
> > +     [RST_BUS_MCU_RISCV_CORE]        = { 0x0124, BIT(18) },
> > +     [RST_BUS_MCU_RISCV_MSGBOX]      = { 0x0128, BIT(16) },
>
> There is a reset bit for the PWM0 clock in the manual, register 0x130,
> same as the mcu_pwmmcu0_clk above.

I don't see this one either.

> > +     [RST_BUS_MCU_PWM0]              = { 0x0134, BIT(16) },
>
> Verified that all of the names defined in the binding headers appear here,
> and all definitions here are mentioned in the binding headers.
>
> > +};
> > +
> > +static const struct sunxi_ccu_desc sun55i_a523_mcu_ccu_desc = {
> > +     .ccu_clks       = sun55i_a523_mcu_ccu_clks,
> > +     .num_ccu_clks   = ARRAY_SIZE(sun55i_a523_mcu_ccu_clks),
> > +
> > +     .hw_clks        = &sun55i_a523_mcu_hw_clks,
> > +
> > +     .resets         = sun55i_a523_mcu_ccu_resets,
> > +     .num_resets     = ARRAY_SIZE(sun55i_a523_mcu_ccu_resets),
> > +};
> > +
> > +static int sun55i_a523_mcu_ccu_probe(struct platform_device *pdev)
> > +{
> > +     void __iomem *reg;
> > +     u32 val;
> > +     int ret;
> > +
> > +     reg = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(reg))
> > +             return PTR_ERR(reg);
> > +
> > +     val = readl(reg + SUN55I_A523_PLL_AUDIO1_REG);
> > +
> > +     /*
> > +      * The PLL clock code does not model all bits, for instance it does
> > +      * not support a separate enable and gate bit. We present the
> > +      * gate bit(27) as the enable bit, but then have to set the
> > +      * PLL Enable, LDO Enable, and Lock Enable bits on all PLLs here.
> > +      */
> > +     val |= BIT(31) | BIT(30) | BIT(29);
> > +
> > +     /* Enforce p1 = 5, p0 = 2 (the default) for PLL_AUDIO1 */
> > +     val &= ~(GENMASK(22, 20) | GENMASK(18, 16));
> > +     val |= (4 << 20) | (1 << 16);
>
> Ah, I see, here you set those fixed dividers. I still think we should
> model them properly, as suggested above.

I'm not sure why it's designed this way, especially having a value that
is only close but not exactly 22.5792 MHz on the 5-divider.

Nevertheless, since the outputs are named this way, I think we should
just follow it. It causes less confusion. I can add a comment above
the PLL part.


Thanks
ChenYu

> Cheers,
> Andre
>
> > +
> > +     writel(val, reg + SUN55I_A523_PLL_AUDIO1_REG);
> > +
> > +     ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun55i_a523_mcu_ccu_desc);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id sun55i_a523_mcu_ccu_ids[] = {
> > +     { .compatible = "allwinner,sun55i-a523-mcu-ccu" },
> > +     { }
> > +};
> > +
> > +static struct platform_driver sun55i_a523_mcu_ccu_driver = {
> > +     .probe  = sun55i_a523_mcu_ccu_probe,
> > +     .driver = {
> > +             .name                   = "sun55i-a523-mcu-ccu",
> > +             .suppress_bind_attrs    = true,
> > +             .of_match_table         = sun55i_a523_mcu_ccu_ids,
> > +     },
> > +};
> > +module_platform_driver(sun55i_a523_mcu_ccu_driver);
> > +
> > +MODULE_IMPORT_NS("SUNXI_CCU");
> > +MODULE_DESCRIPTION("Support for the Allwinner A523 MCU CCU");
> > +MODULE_LICENSE("GPL");
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU
  2025-09-05 16:13     ` Chen-Yu Tsai
@ 2025-09-05 17:21       ` Andre Przywara
  0 siblings, 0 replies; 21+ messages in thread
From: Andre Przywara @ 2025-09-05 17:21 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Stephen Boyd,
	Jernej Skrabec, Samuel Holland, linux-sunxi, linux-clk,
	linux-arm-kernel, devicetree, linux-kernel

On Sat, 6 Sep 2025 00:13:33 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

Hi,

> On Fri, Sep 5, 2025 at 11:14 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Sun, 31 Aug 2025 01:08:59 +0800
> > Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > Hi Chen-Yu,
> >
> > many thanks for this patch, I feel with you when it comes to model
> > Allwinner CCUs in the kernel ;-)
> >  
> > > From: Chen-Yu Tsai <wens@csie.org>
> > >
> > > The A523/T527 SoCs have a new MCU PRCM, which has more clocks and reset
> > > controls for the RISC-V MCU and other peripherals. There is no visible
> > > bus in this part, but there is a second audio PLL. The BSP driver uses
> > > the 24MHz main oscillator as the parent for all the bus clocks.  
> >
> > So my copy of the T527 manual (v0.92) shows the system but tree of the
> > MCU_PRCM in figure 2-24, and there some peripherals are on AHB_DEC0, while
> > others are on APBS0. Shall we model this correctly, then?  
> 
> The figure was a bit misleading as it had at its root "CPU" and "MCU_AHB"
> instead of "CPUX". I guess given that we can actually access these devices,
> they should be the same. It was weird because there were no bus clock
> dividers in this block.

Yeah, I agree that this figure is as misleading as it's useful. For
instance what is AHB_DEC0? Is that the same as AHB or AHB_DEC? And
indeed does CPU refer to the Arm cores or the RISC-V core? And is that
APBS0 the same as the APBS0 from the PRCM CCU? Though they seem to have
different parents ...

> 
> > > Add a driver to support this part. Unlike the BSP driver, the SoC's main
> > > MBUS clock is chosen as the parent for the MCU MBUS clock, and the
> > > latter then serves as the parent of the MCU DMA controller's MBUS clock.
> > >
> > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > ---
> > >  drivers/clk/sunxi-ng/Kconfig               |   5 +
> > >  drivers/clk/sunxi-ng/Makefile              |   2 +
> > >  drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c | 447 +++++++++++++++++++++
> > >  3 files changed, 454 insertions(+)
> > >  create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> > >
> > > diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> > > index 8896fd052ef1..6af2d020e03e 100644
> > > --- a/drivers/clk/sunxi-ng/Kconfig
> > > +++ b/drivers/clk/sunxi-ng/Kconfig
> > > @@ -57,6 +57,11 @@ config SUN55I_A523_CCU
> > >       default ARCH_SUNXI
> > >       depends on ARM64 || COMPILE_TEST
> > >
> > > +config SUN55I_A523_MCU_CCU
> > > +     tristate "Support for the Allwinner A523/T527 MCU CCU"
> > > +     default ARCH_SUNXI
> > > +     depends on ARM64 || COMPILE_TEST
> > > +
> > >  config SUN55I_A523_R_CCU
> > >       tristate "Support for the Allwinner A523/T527 PRCM CCU"
> > >       default ARCH_SUNXI
> > > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> > > index 82e471036de6..a1c4087d7241 100644
> > > --- a/drivers/clk/sunxi-ng/Makefile
> > > +++ b/drivers/clk/sunxi-ng/Makefile
> > > @@ -34,6 +34,7 @@ obj-$(CONFIG_SUN50I_H6_CCU) += sun50i-h6-ccu.o
> > >  obj-$(CONFIG_SUN50I_H6_R_CCU)        += sun50i-h6-r-ccu.o
> > >  obj-$(CONFIG_SUN50I_H616_CCU)        += sun50i-h616-ccu.o
> > >  obj-$(CONFIG_SUN55I_A523_CCU)        += sun55i-a523-ccu.o
> > > +obj-$(CONFIG_SUN55I_A523_MCU_CCU)    += sun55i-a523-mcu-ccu.o
> > >  obj-$(CONFIG_SUN55I_A523_R_CCU)      += sun55i-a523-r-ccu.o
> > >  obj-$(CONFIG_SUN4I_A10_CCU)  += sun4i-a10-ccu.o
> > >  obj-$(CONFIG_SUN5I_CCU)              += sun5i-ccu.o
> > > @@ -61,6 +62,7 @@ sun50i-h6-ccu-y                     += ccu-sun50i-h6.o
> > >  sun50i-h6-r-ccu-y            += ccu-sun50i-h6-r.o
> > >  sun50i-h616-ccu-y            += ccu-sun50i-h616.o
> > >  sun55i-a523-ccu-y            += ccu-sun55i-a523.o
> > > +sun55i-a523-mcu-ccu-y                += ccu-sun55i-a523-mcu.o
> > >  sun55i-a523-r-ccu-y          += ccu-sun55i-a523-r.o
> > >  sun4i-a10-ccu-y                      += ccu-sun4i-a10.o
> > >  sun5i-ccu-y                  += ccu-sun5i.o
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> > > new file mode 100644
> > > index 000000000000..6105485837c9
> > > --- /dev/null
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523-mcu.c
> > > @@ -0,0 +1,447 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
> > > + *
> > > + * Based on the A523 CCU driver:
> > > + *   Copyright (C) 2023-2024 Arm Ltd.
> > > + */
> > > +
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include <dt-bindings/clock/sun55i-a523-mcu-ccu.h>
> > > +#include <dt-bindings/reset/sun55i-a523-mcu-ccu.h>
> > > +
> > > +#include "ccu_common.h"
> > > +#include "ccu_reset.h"
> > > +
> > > +#include "ccu_div.h"
> > > +#include "ccu_gate.h"
> > > +#include "ccu_mp.h"
> > > +#include "ccu_mult.h"
> > > +#include "ccu_nm.h"
> > > +
> > > +static const struct clk_parent_data osc24M[] = {
> > > +     { .fw_name = "hosc" }
> > > +};
> > > +
> > > +#define SUN55I_A523_PLL_AUDIO1_REG   0x00c
> > > +static struct ccu_sdm_setting pll_audio1_sdm_table[] = {
> > > +     { .rate = 2167603200, .pattern = 0xa000a234, .m = 1, .n = 90 }, /* div2->22.5792 */
> > > +     { .rate = 2359296000, .pattern = 0xa0009ba6, .m = 1, .n = 98 }, /* div2->24.576 */
> > > +     { .rate = 1806336000, .pattern = 0xa000872b, .m = 1, .n = 75 }, /* div5->22.576 */
> > > +};
> > > +
> > > +static struct ccu_nm pll_audio1_clk = {
> > > +     .enable         = BIT(27),
> > > +     .lock           = BIT(28),
> > > +     .n              = _SUNXI_CCU_MULT_MIN(8, 8, 11),
> > > +     .m              = _SUNXI_CCU_DIV(1, 1),
> > > +     .sdm            = _SUNXI_CCU_SDM(pll_audio1_sdm_table, BIT(24),
> > > +                                      0x010, BIT(31)),
> > > +     .min_rate       = 180000000U,
> > > +     .max_rate       = 3500000000U,
> > > +     .common         = {
> > > +             .reg            = 0x00c,
> > > +             .features       = CCU_FEATURE_SIGMA_DELTA_MOD,
> > > +             .hw.init        = CLK_HW_INIT_PARENTS_DATA("pll-audio1",
> > > +                                                        osc24M, &ccu_nm_ops,
> > > +                                                        CLK_SET_RATE_GATE),
> > > +     },
> > > +};
> > > +
> > > +static const struct clk_hw *pll_audio1_div_parents[] = { &pll_audio1_clk.common.hw };
> > > +static CLK_FIXED_FACTOR_HWS(pll_periph1_div2_clk, "pll-audio1-div2",
> > > +                         pll_audio1_div_parents, 2, 1,
> > > +                         CLK_SET_RATE_PARENT);  
> >
> > I admit that those "fixed programmable" dividers are odd, but there are
> > fields in the PLL control register that we should use, so it's not a
> > fixed divider clock, but a programmable divider, using
> > SUNXI_CCU_M_HWS().  
> 
> As you found out below, this is programmed to be fixed.
> 
> > And I think you want the struct name to contain audio1, not periph1?  
> 
> Correct. Copy-paste thing.
> 
> > > +static CLK_FIXED_FACTOR_HWS(pll_periph1_div5_clk, "pll-audio1-div5",
> > > +                         pll_audio1_div_parents, 5, 1,
> > > +                         CLK_SET_RATE_PARENT);  
> >
> > Same here.
> >  
> > > +
> > > +static SUNXI_CCU_M_WITH_GATE(audio_out_clk, "audio-out",
> > > +                          "pll-audio1-div2", 0x01c,
> > > +                          0, 5, BIT(31), CLK_SET_RATE_PARENT);  
> >
> > I wonder if CLK_SET_RATE_PARENT is a good idea here. IIUC, then the
> > idea would be that the PLL is running at a fixed high rate (3072 MHz),
> > and gets divided down here to something more audio-y, like 48 or 96 MHz?  
> 
> No. For audio there are only two classes of clock rates that matter:
> 
>  - multiple of 24.576 MHz for 48 KHz family of sample rates
>  - multiple of 22.5792 MHz for 44.1 KHz family of sample rates
> 
> The PLL has to be able to switch between these. The SDM table below
> contains predefined values from the BSP.
> 
> > Do you have an idea what this clock is supposed to be used for? I
> > don't see it used anywhere, neither in this series, nor in the manual's
> > other clock descriptions or even pins.  
> 
> I've no idea, but didn't want to leave it out and then later on someone
> has to add it.

Fair enough, I guess we can fix that if needed, when we get actual users.

> > > +
> > > +static const struct clk_parent_data dsp_parents[] = {
> > > +     { .fw_name = "hosc" },
> > > +     { .fw_name = "losc" },
> > > +     { .fw_name = "iosc" },
> > > +     { .hw = &pll_periph1_div5_clk.hw },
> > > +     { .hw = &pll_periph1_div2_clk.hw },  
> >
> > The manual says that parent 0b011 is the DIV2 clock, and 0b100 is the
> > DIV5 clock, so those lines should be swapped.  
> 
> BSP has this order, so I'm confused as you are. I can just leave a note
> here saying the order is from the BSP, which is different from the manual.
> 
> Until someone actually plays with the DSP, we won't know which one is correct.

The rest of the parent lists always use DIV2 first, than DIV5. So I am
really inclined to believe in a BSP bug. I guess in real life people will
use the "dsp" clock anyway, so nobody noticed.

> 
> > > +     { .fw_name = "dsp" },
> > > +};
> > > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(dsp_clk, "mcu-dsp", dsp_parents, 0x0020,
> > > +                                   0, 5,     /* M */
> > > +                                   24, 3,    /* mux */
> > > +                                   BIT(31),  /* gate */
> > > +                                   0);
> > > +
> > > +static const struct clk_parent_data i2s_parents[] = {
> > > +     { .fw_name = "pll-audio0-4x" },
> > > +     { .hw = &pll_periph1_div2_clk.hw },
> > > +     { .hw = &pll_periph1_div5_clk.hw },
> > > +};
> > > +
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s0_clk, "i2s0", i2s_parents, 0x02c,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);  
> >
> > Same question about CLK_SET_RATE_PARENT here. Does the flag mean that any
> > rate request is only to be fulfilled by the parent? Couldn't find a good
> > explanation for that.  
> 
> AFAIK the flag is _supposed_ to mean that during a rate change, the clk
> provider is free to request a different rate from the parent, one that
> is suitable for its own use. This actually depends on clk drivers correctly
> implementing it in their .determine_rate callbacks though.

Ah, thanks, that's a good point: it's somewhat IMPDEF.

> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s1_clk, "i2s1", i2s_parents, 0x030,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s2_clk, "i2s2", i2s_parents, 0x034,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_clk, "i2s3", i2s_parents, 0x038,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +
> > > +static const struct clk_parent_data i2s3_asrc_parents[] = {
> > > +     { .fw_name = "pll-periph0-300m" },
> > > +     { .hw = &pll_periph1_div2_clk.hw },
> > > +     { .hw = &pll_periph1_div5_clk.hw },
> > > +};
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(i2s3_asrc_clk, "i2s3-asrc",
> > > +                               i2s3_asrc_parents, 0x038,  
> >
> > that address should be 0x03c  
> 
> It might have been a "copy-paste and then forget to update it" situation.
> It was around midnight when I wrote this driver.

Don't worry, I know the drill. Especially those large clock drivers are a
nightmare in this respect, and missing some update after copy&paste
happened to me so many times before.

> 
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_i2s0_clk, "bus-i2s0", osc24M, 0x040, BIT(0), 0);  
> >
> > My manual says that APBS0 is the bus clock for the I2S peripherals. I
> > guess another one for the list of "clocks" in the DT binding :-(
> > This applies to the other bus clocks as well, they should be either APBS0
> > or AHB(_DEC0?).  
> 
> OK.

I am wondering what the consequences are in terms of compatibility?
Maybe we should at least make those clocks part of the DT clocks property,
so that old DTs have them, should we need them in the future?

> > > +static SUNXI_CCU_GATE_DATA(bus_i2s1_clk, "bus-i2s1", osc24M, 0x040, BIT(1), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_i2s2_clk, "bus-i2s2", osc24M, 0x040, BIT(2), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_i2s3_clk, "bus-i2s3", osc24M, 0x040, BIT(3), 0);
> > > +
> > > +static const struct clk_parent_data audio_parents[] = {
> > > +     { .fw_name = "pll-audio0-4x" },
> > > +     { .hw = &pll_periph1_div2_clk.hw },
> > > +     { .hw = &pll_periph1_div5_clk.hw },
> > > +};
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_tx_clk, "spdif-tx",
> > > +                               audio_parents, 0x044,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(spdif_rx_clk, "spdif-rx",
> > > +                               i2s3_asrc_parents, 0x048,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_spdif_clk, "bus-spdif", osc24M, 0x04c, BIT(0), 0);
> > > +
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(dmic_clk, "dmic", audio_parents, 0x050,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_dmic_clk, "bus-dmic", osc24M, 0x054, BIT(0), 0);
> > > +
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_dac_clk, "audio-dac",
> > > +                               audio_parents, 0x058,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +static SUNXI_CCU_DUALDIV_MUX_GATE(audio_adc_clk, "audio-adc",
> > > +                               audio_parents, 0x05c,
> > > +                               0, 5,         /* M */
> > > +                               5, 5,         /* P */
> > > +                               24, 3,        /* mux */
> > > +                               BIT(31),      /* gate */
> > > +                               CLK_SET_RATE_PARENT);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_audio_codec_clk, "bus-audio-codec",
> > > +                        osc24M, 0x060, BIT(0), 0);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_dsp_msgbox_clk, "bus-dsp-msgbox",
> > > +                        osc24M, 0x068, BIT(0), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_dsp_cfg_clk, "bus-dsp-cfg",
> > > +                        osc24M, 0x06c, BIT(0), 0);
> > > +
> > > +static SUNXI_CCU_GATE_DATA(bus_npu_hclk, "bus-npu-hclk", osc24M, 0x070, BIT(1), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_npu_aclk, "bus-npu-aclk", osc24M, 0x070, BIT(2), 0);
> > > +
> > > +static const struct clk_parent_data timer_parents[] = {
> > > +     { .fw_name = "hosc" },
> > > +     { .fw_name = "losc" },
> > > +     { .fw_name = "iosc" },
> > > +     { .fw_name = "r-ahb" }
> > > +};
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer0_clk, "mcu-timer0", timer_parents,
> > > +                                   0x074,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer1_clk, "mcu-timer1", timer_parents,
> > > +                                   0x078,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer2_clk, "mcu-timer2", timer_parents,
> > > +                                   0x07c,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer3_clk, "mcu-timer3", timer_parents,
> > > +                                   0x080,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer4_clk, "mcu-timer4", timer_parents,
> > > +                                   0x084,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(mcu_timer5_clk, "mcu-timer5", timer_parents,
> > > +                                   0x088,
> > > +                                   1, 3,     /* P */
> > > +                                   4, 2,     /* mux */
> > > +                                   BIT(0),   /* gate */
> > > +                                   0);
> > > +static SUNXI_CCU_GATE_DATA(bus_mcu_timer_clk, "bus-mcu-timer", osc24M, 0x08c, BIT(0), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_mcu_dma_clk, "bus-mcu-dma", osc24M, 0x104, BIT(0), 0);
> > > +static SUNXI_CCU_GATE_DATA(tzma0_clk, "tzma0", osc24M, 0x108, BIT(0), 0);
> > > +static SUNXI_CCU_GATE_DATA(tzma1_clk, "tzma1", osc24M, 0x10c, BIT(0), 0);  
> >
> > Where did you find those two? I guess in the BSP code? Can you maybe add a
> > comment about it then?  
> 
> OK.
> 
> > > +static SUNXI_CCU_GATE_DATA(bus_pubsram_clk, "bus-pubsram", osc24M, 0x114, BIT(0), 0);
> > > +
> > > +/*
> > > + * user manual has "mbus" clock as parent of both clocks below,
> > > + * but this makes more sense, since BSP MCU DMA controller has
> > > + * reference to both of them, likely needing both enabled.
> > > + */
> > > +static SUNXI_CCU_GATE_FW(mbus_mcu_clk, "mbus-mcu", "mbus", 0x11c, BIT(1), 0);
> > > +static SUNXI_CCU_GATE_HW(mbus_mcu_dma_clk, "mbus-mcu-dma",
> > > +                      &mbus_mcu_clk.common.hw, 0x11c, BIT(0), 0);
> > > +
> > > +static const struct clk_parent_data riscv_pwm_parents[] = {  
> >
> > Where does the pwm part come from? Is the clock below actually the RISC-V
> > PWM clock? Which would make more sense, since I don't see a PLL or any
> > other fast clock in there.  
> 
> I'm reusing the parent list here, since the list is the same for the riscv
> mod clock and the pwm0 mod clock, hence the name has riscv + pwm.

Ah, I see, makes sense, and indeed we used that naming pattern before.

> 
> > > +     { .fw_name = "hosc" },
> > > +     { .fw_name = "losc" },
> > > +     { .fw_name = "iosc" },
> > > +};
> > > +
> > > +static SUNXI_CCU_MUX_DATA_WITH_GATE(riscv_clk, "riscv",  
> >
> > Related to above: what RISC-V clock is this exactly? Is that some PWM
> > clock source, as the parents name suggests, or the main clock, which looks
> > rather slow then? Or is that to select the root of the RISC-V clock tree?  
> 
> From https://github.com/radxa/allwinner-bsp/blob/product-t527-linux/configs/linux-5.15/sun55iw3p1.dtsi#L2981
> it looks like it is the actual clock for the risc-v core. Given that it
> is supposed to be a low power standby thing, I guess Allwinner didn't
> think it was necessary to have anything faster.

But is it meant to be the SCP? Since we found the ARISC still being in the
SoC, plus ARISC code in the BSP, and I think Jernej mentioned that a E906
is quite some overkill for just power management. The manual mentions 200
MHz for the RISC-V core, so I wonder if there is something missing.
Also since they call it MCU, I wonder if that's designed as some kind of
realtime core? Running some RTOS, for instance?

> > > +                                 riscv_pwm_parents, 0x120,
> > > +                                 27, 3, BIT(31), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_riscv_cfg_clk, "bus-riscv-cfg", osc24M,
> > > +                        0x124, BIT(0), 0);
> > > +static SUNXI_CCU_GATE_DATA(bus_riscv_msgbox_clk, "bus-riscv-msgbox", osc24M,
> > > +                        0x128, BIT(0), 0);
> > > +
> > > +static SUNXI_CCU_MUX_DATA_WITH_GATE(mcu_pwmmcu0_clk, "mcu-pwm0",
> > > +                                 riscv_pwm_parents, 0x130,
> > > +                                 27, 3, BIT(31), 0);  
> >
> > The mux fields for this clock start at bit 24.  
> 
> Oops.
> 
> > > +static SUNXI_CCU_GATE_DATA(bus_mcu_pwm0_clk, "bus-mcu-pwm0", osc24M,
> > > +                        0x128, BIT(0), 0);  
> >
> > The register offset is 0x134.  
> 
> (facepalm)

Hey, don't worry, I prefer finding those bugs over not finding anything!

> 
> > > +
> > > +/*
> > > + * Contains all clocks that are controlled by a hardware register. They
> > > + * have a (sunxi) .common member, which needs to be initialised by the common
> > > + * sunxi CCU code, to be filled with the MMIO base address and the shared lock.
> > > + */
> > > +static struct ccu_common *sun55i_a523_mcu_ccu_clks[] = {
> > > +     &pll_audio1_clk.common,
> > > +     &audio_out_clk.common,
> > > +     &dsp_clk.common,
> > > +     &i2s0_clk.common,
> > > +     &i2s1_clk.common,
> > > +     &i2s2_clk.common,
> > > +     &i2s3_clk.common,
> > > +     &i2s3_asrc_clk.common,
> > > +     &bus_i2s0_clk.common,
> > > +     &bus_i2s1_clk.common,
> > > +     &bus_i2s2_clk.common,
> > > +     &bus_i2s3_clk.common,
> > > +     &spdif_tx_clk.common,
> > > +     &spdif_rx_clk.common,
> > > +     &bus_spdif_clk.common,
> > > +     &dmic_clk.common,
> > > +     &bus_dmic_clk.common,
> > > +     &audio_dac_clk.common,
> > > +     &audio_adc_clk.common,
> > > +     &bus_audio_codec_clk.common,
> > > +     &bus_dsp_msgbox_clk.common,
> > > +     &bus_dsp_cfg_clk.common,
> > > +     &bus_npu_aclk.common,
> > > +     &bus_npu_hclk.common,
> > > +     &mcu_timer0_clk.common,
> > > +     &mcu_timer1_clk.common,
> > > +     &mcu_timer2_clk.common,
> > > +     &mcu_timer3_clk.common,
> > > +     &mcu_timer4_clk.common,
> > > +     &mcu_timer5_clk.common,
> > > +     &bus_mcu_timer_clk.common,
> > > +     &bus_mcu_dma_clk.common,
> > > +     &tzma0_clk.common,
> > > +     &tzma1_clk.common,
> > > +     &bus_pubsram_clk.common,
> > > +     &mbus_mcu_dma_clk.common,
> > > +     &mbus_mcu_clk.common,
> > > +     &riscv_clk.common,
> > > +     &bus_riscv_cfg_clk.common,
> > > +     &bus_riscv_msgbox_clk.common,
> > > +     &mcu_pwm0_clk.common,
> > > +     &bus_mcu_pwm0_clk.common,
> > > +};
> > > +
> > > +static struct clk_hw_onecell_data sun55i_a523_mcu_hw_clks = {
> > > +     .num    = CLK_BUS_MCU_PWM0 + 1,  
> >
> > like of the NPU patch, can we use ".num = NUM_CLOCKS," here, and
> > #define NUM_CLOCKS at the beginning of the file, right after including the
> > binding headers?
> > Or alternatively, put .num after the .hws definitions, so that last clock
> > and number are closer together?  
> 
> As mentioned in that patch, I prefer the latter.

Thanks, works for me!

> 
> > > +     .hws    = {
> > > +             [CLK_MCU_PLL_AUDIO1]            = &pll_audio1_clk.common.hw,
> > > +             [CLK_MCU_PLL_AUDIO1_DIV2]       = &pll_periph1_div2_clk.hw,
> > > +             [CLK_MCU_PLL_AUDIO1_DIV5]       = &pll_periph1_div5_clk.hw,
> > > +             [CLK_MCU_AUDIO_OUT]             = &audio_out_clk.common.hw,
> > > +             [CLK_MCU_DSP]                   = &dsp_clk.common.hw,
> > > +             [CLK_MCU_I2S0]                  = &i2s0_clk.common.hw,
> > > +             [CLK_MCU_I2S1]                  = &i2s1_clk.common.hw,
> > > +             [CLK_MCU_I2S2]                  = &i2s2_clk.common.hw,
> > > +             [CLK_MCU_I2S3]                  = &i2s3_clk.common.hw,
> > > +             [CLK_MCU_I2S3_ASRC]             = &i2s3_asrc_clk.common.hw,
> > > +             [CLK_BUS_MCU_I2S0]              = &bus_i2s0_clk.common.hw,
> > > +             [CLK_BUS_MCU_I2S1]              = &bus_i2s1_clk.common.hw,
> > > +             [CLK_BUS_MCU_I2S2]              = &bus_i2s2_clk.common.hw,
> > > +             [CLK_BUS_MCU_I2S3]              = &bus_i2s3_clk.common.hw,
> > > +             [CLK_MCU_SPDIF_TX]              = &spdif_tx_clk.common.hw,
> > > +             [CLK_MCU_SPDIF_RX]              = &spdif_rx_clk.common.hw,
> > > +             [CLK_BUS_MCU_SPDIF]             = &bus_spdif_clk.common.hw,
> > > +             [CLK_MCU_DMIC]                  = &dmic_clk.common.hw,
> > > +             [CLK_BUS_MCU_DMIC]              = &bus_dmic_clk.common.hw,
> > > +             [CLK_MCU_AUDIO_CODEC_DAC]       = &audio_dac_clk.common.hw,
> > > +             [CLK_MCU_AUDIO_CODEC_ADC]       = &audio_adc_clk.common.hw,
> > > +             [CLK_BUS_MCU_AUDIO_CODEC]       = &bus_audio_codec_clk.common.hw,
> > > +             [CLK_BUS_MCU_DSP_MSGBOX]        = &bus_dsp_msgbox_clk.common.hw,
> > > +             [CLK_BUS_MCU_DSP_CFG]           = &bus_dsp_cfg_clk.common.hw,
> > > +             [CLK_BUS_MCU_NPU_HCLK]          = &bus_npu_hclk.common.hw,
> > > +             [CLK_BUS_MCU_NPU_ACLK]          = &bus_npu_aclk.common.hw,
> > > +             [CLK_MCU_TIMER0]                = &mcu_timer0_clk.common.hw,
> > > +             [CLK_MCU_TIMER1]                = &mcu_timer1_clk.common.hw,
> > > +             [CLK_MCU_TIMER2]                = &mcu_timer2_clk.common.hw,
> > > +             [CLK_MCU_TIMER3]                = &mcu_timer3_clk.common.hw,
> > > +             [CLK_MCU_TIMER4]                = &mcu_timer4_clk.common.hw,
> > > +             [CLK_MCU_TIMER5]                = &mcu_timer5_clk.common.hw,
> > > +             [CLK_BUS_MCU_TIMER]             = &bus_mcu_timer_clk.common.hw,
> > > +             [CLK_BUS_MCU_DMA]               = &bus_mcu_dma_clk.common.hw,
> > > +             [CLK_MCU_TZMA0]                 = &tzma0_clk.common.hw,
> > > +             [CLK_MCU_TZMA1]                 = &tzma1_clk.common.hw,
> > > +             [CLK_BUS_MCU_PUBSRAM]           = &bus_pubsram_clk.common.hw,
> > > +             [CLK_MCU_MBUS_DMA]              = &mbus_mcu_dma_clk.common.hw,
> > > +             [CLK_MCU_MBUS]                  = &mbus_mcu_clk.common.hw,
> > > +             [CLK_MCU_RISCV]                 = &riscv_clk.common.hw,
> > > +             [CLK_BUS_MCU_RISCV_CFG]         = &bus_riscv_cfg_clk.common.hw,
> > > +             [CLK_BUS_MCU_RISCV_MSGBOX]      = &bus_riscv_msgbox_clk.common.hw,
> > > +             [CLK_MCU_PWM0]                  = &mcu_pwm0_clk.common.hw,
> > > +             [CLK_BUS_MCU_PWM0]              = &bus_mcu_pwm0_clk.common.hw,
> > > +     },
> > > +};
> > > +
> > > +static struct ccu_reset_map sun55i_a523_mcu_ccu_resets[] = {
> > > +     [RST_BUS_MCU_I2S0]              = { 0x0040, BIT(16) },
> > > +     [RST_BUS_MCU_I2S1]              = { 0x0040, BIT(17) },
> > > +     [RST_BUS_MCU_I2S2]              = { 0x0040, BIT(18) },
> > > +     [RST_BUS_MCU_I2S3]              = { 0x0040, BIT(19) },
> > > +     [RST_BUS_MCU_SPDIF]             = { 0x004c, BIT(16) },
> > > +     [RST_BUS_MCU_DMIC]              = { 0x0054, BIT(16) },
> > > +     [RST_BUS_MCU_AUDIO_CODEC]       = { 0x0060, BIT(16) },
> > > +     [RST_BUS_MCU_DSP_MSGBOX]        = { 0x0068, BIT(16) },
> > > +     [RST_BUS_MCU_DSP_CFG]           = { 0x006c, BIT(16) },
> > > +     [RST_BUS_MCU_NPU]               = { 0x0070, BIT(16) },
> > > +     [RST_BUS_MCU_TIMER]             = { 0x008c, BIT(16) },
> > > +     [RST_BUS_MCU_DSP_DEBUG]         = { 0x0100, BIT(16) },
> > > +     [RST_BUS_MCU_DSP]               = { 0x0100, BIT(17) },  
> >
> > I don't see those two in the manual.
> >  
> > > +     [RST_BUS_MCU_DMA]               = { 0x0104, BIT(16) },
> > > +     [RST_BUS_MCU_PUBSRAM]           = { 0x0114, BIT(16) },  
> >
> > The manual shows a reset bit in register 0x120, in the same register as
> > this ominous riscv_clk above.  
> 
> Hmm. I don't see this in the T527 manual (0.92), nor the A523 manuals
> (both 1.1 and 1.4), nor the BSP code.

Scratch that, starring at code and manual for too long. Indeed there is no
reset bit, just a clock enable at bit 31.

> > > +     [RST_BUS_MCU_RISCV_CFG]         = { 0x0124, BIT(16) },
> > > +     [RST_BUS_MCU_RISCV_DEBUG]       = { 0x0124, BIT(17) },
> > > +     [RST_BUS_MCU_RISCV_CORE]        = { 0x0124, BIT(18) },
> > > +     [RST_BUS_MCU_RISCV_MSGBOX]      = { 0x0128, BIT(16) },  
> >
> > There is a reset bit for the PWM0 clock in the manual, register 0x130,
> > same as the mcu_pwmmcu0_clk above.  
> 
> I don't see this one either.

And now it's gone for me as well! Must have been pre-lunch starvation,
dulling my senses ;-) Sorry for that!

Cheers,
Andre

> > > +     [RST_BUS_MCU_PWM0]              = { 0x0134, BIT(16) },  
> >
> > Verified that all of the names defined in the binding headers appear here,
> > and all definitions here are mentioned in the binding headers.
> >  
> > > +};
> > > +
> > > +static const struct sunxi_ccu_desc sun55i_a523_mcu_ccu_desc = {
> > > +     .ccu_clks       = sun55i_a523_mcu_ccu_clks,
> > > +     .num_ccu_clks   = ARRAY_SIZE(sun55i_a523_mcu_ccu_clks),
> > > +
> > > +     .hw_clks        = &sun55i_a523_mcu_hw_clks,
> > > +
> > > +     .resets         = sun55i_a523_mcu_ccu_resets,
> > > +     .num_resets     = ARRAY_SIZE(sun55i_a523_mcu_ccu_resets),
> > > +};
> > > +
> > > +static int sun55i_a523_mcu_ccu_probe(struct platform_device *pdev)
> > > +{
> > > +     void __iomem *reg;
> > > +     u32 val;
> > > +     int ret;
> > > +
> > > +     reg = devm_platform_ioremap_resource(pdev, 0);
> > > +     if (IS_ERR(reg))
> > > +             return PTR_ERR(reg);
> > > +
> > > +     val = readl(reg + SUN55I_A523_PLL_AUDIO1_REG);
> > > +
> > > +     /*
> > > +      * The PLL clock code does not model all bits, for instance it does
> > > +      * not support a separate enable and gate bit. We present the
> > > +      * gate bit(27) as the enable bit, but then have to set the
> > > +      * PLL Enable, LDO Enable, and Lock Enable bits on all PLLs here.
> > > +      */
> > > +     val |= BIT(31) | BIT(30) | BIT(29);
> > > +
> > > +     /* Enforce p1 = 5, p0 = 2 (the default) for PLL_AUDIO1 */
> > > +     val &= ~(GENMASK(22, 20) | GENMASK(18, 16));
> > > +     val |= (4 << 20) | (1 << 16);  
> >
> > Ah, I see, here you set those fixed dividers. I still think we should
> > model them properly, as suggested above.  
> 
> I'm not sure why it's designed this way, especially having a value that
> is only close but not exactly 22.5792 MHz on the 5-divider.
> 
> Nevertheless, since the outputs are named this way, I think we should
> just follow it. It causes less confusion. I can add a comment above
> the PLL part.
> 
> 
> Thanks
> ChenYu
> 
> > Cheers,
> > Andre
> >  
> > > +
> > > +     writel(val, reg + SUN55I_A523_PLL_AUDIO1_REG);
> > > +
> > > +     ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun55i_a523_mcu_ccu_desc);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static const struct of_device_id sun55i_a523_mcu_ccu_ids[] = {
> > > +     { .compatible = "allwinner,sun55i-a523-mcu-ccu" },
> > > +     { }
> > > +};
> > > +
> > > +static struct platform_driver sun55i_a523_mcu_ccu_driver = {
> > > +     .probe  = sun55i_a523_mcu_ccu_probe,
> > > +     .driver = {
> > > +             .name                   = "sun55i-a523-mcu-ccu",
> > > +             .suppress_bind_attrs    = true,
> > > +             .of_match_table         = sun55i_a523_mcu_ccu_ids,
> > > +     },
> > > +};
> > > +module_platform_driver(sun55i_a523_mcu_ccu_driver);
> > > +
> > > +MODULE_IMPORT_NS("SUNXI_CCU");
> > > +MODULE_DESCRIPTION("Support for the Allwinner A523 MCU CCU");
> > > +MODULE_LICENSE("GPL");  
> >  


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-09-05 17:21 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-30 17:08 [PATCH 0/8] arm64: allwinner: a523: Enable MCU PRCM and NPU Chen-Yu Tsai
2025-08-30 17:08 ` [PATCH 1/8] dt-bindings: clock: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
2025-09-01 21:29   ` Rob Herring (Arm)
2025-09-05 15:12   ` Andre Przywara
2025-08-30 17:08 ` [PATCH 2/8] dt-bindings: clock: sun55i-a523-ccu: Add A523 MCU CCU clock controller Chen-Yu Tsai
2025-09-01 21:31   ` Rob Herring (Arm)
2025-08-30 17:08 ` [PATCH 3/8] clk: sunxi-ng: mp: Fix dual-divider clock rate readback Chen-Yu Tsai
2025-09-05 15:12   ` Andre Przywara
2025-09-05 15:17     ` Chen-Yu Tsai
2025-08-30 17:08 ` [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU module clock Chen-Yu Tsai
2025-09-05 15:14   ` Andre Przywara
2025-09-05 15:19     ` Chen-Yu Tsai
2025-08-30 17:08 ` [PATCH 5/8] clk: sunxi-ng: div: support power-of-two dividers Chen-Yu Tsai
2025-08-30 17:08 ` [PATCH 6/8] clk: sunxi-ng: add support for the A523/T527 MCU CCU Chen-Yu Tsai
2025-09-05 15:14   ` Andre Przywara
2025-09-05 16:13     ` Chen-Yu Tsai
2025-09-05 17:21       ` Andre Przywara
2025-08-30 17:09 ` [PATCH 7/8] arm64: dts: allwinner: a523: Add MCU PRCM CCU node Chen-Yu Tsai
2025-09-05 15:14   ` Andre Przywara
2025-08-30 17:09 ` [PATCH 8/8] arm64: dts: allwinner: a523: Add NPU device node Chen-Yu Tsai
2025-09-05 15:14   ` Andre Przywara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).