[PATCH RFC 0/4] ZTE zx297520v3 clock bindings and driver

[PATCH RFC 0/4] ZTE zx297520v3 clock bindings and driver

[Stefan Dösinger]

Hi,

I am sending my clk driver for zx297520v3 boards for comment. The patch
series sits on top of my previously sent patches for very basic support
for this board [0]. The first caveat is that b4 dependencies don't seem
to do what I expected, so I have backed out the MAINTAINERS and DTSI
changes from this RFC submission. It should still be good enough to
review the actual clock driver and bindings.

I have pretty good understanding of the clock hardware, but a number of 
questions about the prefered way of implementing such a driver. Either 
the clock framework and its documentation are constantly evolving or I 
am just bad at discovering things:

1) The clock controls on this board consist of 3 controllers that I 
called - following ZTE's nomenclature - top, matrix and LSP. The top 
controller is fed by two oscillators and generates other frequencies 
with PLLs. I have expressed this in the device tree: The clk driver 
binding requires these two clocks.

The matrix controller feeds 10 clocks to the LSP controller and can turn 
them on and off on a per-clock bases. The DT bindings express this as 
well - the LSP binding expects those 10 clock handles.

There are no explicit HW controls for the top->matrix link, but the 
muxes and gates in the matrix controller use the PLL outputs as parents. 
I have not expressed that in the binding as it would require 25 or so 
clock handles. Instead, the matrix driver quietly expects the right 
parent clock names to be registered by the top driver. (OTOH having 60 
or so clock parameters didn't stop st,stm32mp21-rcc)

The entire LSP part (I guess the name stands for low speed peripherals) 
looks like it might be reused on different baords. The top and matrix 
part are very interdependent.

2) The clk-reset interaction: Both clocks and resets are in the same IO 
space, sometimes in the same registers. I see a number of clk drivers 
that register a reset control. I noticed Yu-Chun Lin's RTD1625 clock 
submission added an aux device and placed the reset code in 
drivers/reset instead. Is there a preference for either way or any 
guideline of which way to use in which circumstances?

3) Unused clocks: I looked at recently introduced clk drivers 
(mediatek,mt8196-clock.h, sun55i-a523-mcu-ccu.h) and they do add all a 
lot of clocks that do not have an active consumer - which in a way means 
unused ABI. Please let me know if you prefer to add clocks one by one as 
their consumers are added.

That said, there are a lot of clocks that I want to define for the sole 
purpose of shutting them off. The boot loader leaves pretty much every 
device enabled, including proprietary timers that I don't even plan to 
write a driver for. Registering their clocks in the kernel will allow 
the kernel to shut them off, so they aren't entirely unused.

4) I took some naming from the old zx2967 code. In particular, each 
device has two clocks: "WCLK" for the device operation and "PCLK" for 
register access. Are there more standard names for them? Likewise I took 
some device names from ZTE's downstream sources and I am open to better 
suggestions.

5) I took care to test unbinding and rebinding my clock driver to the 
hardware. It works at least as no clock consumers are defined. It seems 
mutually exclusive though with declaring static initialization data as 
__initdata, as it will be gone after the first time the driver is bound. 
I also don't see how unbinding and rebinding will be tested later on 
when core peripherals are clock consumers.

6) Clock name string matching vs passing pointers: A presentation by
Chen-Yu Tsai from 2024 [1] gave me the impression that the kernel is
trying to move away from building the clock tree with string matching. I 
see APIs for passing a struct clk_hw * as parent instead of strings, but 
that makes it more difficult to build static initialization tables. I 
think the static tables make the code a lot easier to read and I prefer 
that over boot time performance.

I think the list of clocks in my driver is fairly complete; It is 
certainly a lot better than what the downstream ZTE drivers have. I 
deduced a lot of it by trial and error. I am sure there are some clocks 
missing that will need to be added to the binding later. Afaiu adding 
clocks is not an issue, but removing or reordering them is an ABI break.

0: https://lore.kernel.org/linux-arm-kernel/20260506-send-v8-0-f1bdf3243b34@gmail.com/
1: https://www.youtube.com/watch?v=d1VIAnVb3hI

Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
---
Stefan Dösinger (4):
      dt-bindings: clk: zte: Add zx297520v3 clock and reset bindings.
      clk: zte: Introduce a driver for zx297520v3 top clocks and resets.
      clk: zte: Introduce a driver for zx297520v3 matrix clocks and resets.
      clk: zte: Introduce a driver for zx297520v3 LSP clocks and resets.

 .../bindings/clock/zte,zx297520v3-clk.yaml         | 173 ++++
 drivers/clk/Kconfig                                |   1 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/zte/Kconfig                            |  18 +
 drivers/clk/zte/Makefile                           |   2 +
 drivers/clk/zte/clk-zx297520v3.c                   | 980 +++++++++++++++++++++
 include/dt-bindings/clock/zte,zx297520v3-clk.h     | 179 ++++
 7 files changed, 1354 insertions(+)
---
base-commit: 028ef9c96e96197026887c0f092424679298aae8
change-id: 20260510-zx29clk-2e4d39e3128c
prerequisite-change-id: 20260416-send-5c08e095e5c9:v8
prerequisite-patch-id: 94a9b9f889829e5c1899cb1be89b7ee9899b2626
prerequisite-patch-id: 8a849ffe79ba35ef560250fb38587487cc5009fb
prerequisite-patch-id: ef282d0a261dc1097f05057cd43e9e75ae52d92b
prerequisite-patch-id: 5d615c9f855fca6da461168f45a5670a3c3cde81
prerequisite-patch-id: 980a7e66a1031cdcc244a5e461220d68c72309a5
prerequisite-patch-id: 45b6fc60cee81a793cd69e704bf098f1a68769a9

Best regards,
-- 
Stefan Dösinger <stefandoesinger@gmail.com>

[PATCH RFC 1/4] dt-bindings: clk: zte: Add zx297520v3 clock and reset bindings.

[Stefan Dösinger]

These SoCs have 3 clock and reset controllers. The "top" controller -
all names follow ZTE's naming - controls core devices like the AHB bus,
most timers and the Cortex M0 that brings up the board. The register
layout is fairly chaotic. Some patterns can be found, but nothing that
holds true for all devices it controls.

Generally every device has two clocks (one work clock, and one that
connects it to the bus, I call it PCLK), two reset bits (I don't know
what the difference is - sometimes asserting one is enough to reset the
device, sometimes both need to be asserted) and one mux. Some devices,
like the GPIO controller, only have reset bits and no clocks.

The top clock controller is fed by a 26mhz external oscillator and has 4
PLLs to generate other clock rates. ZTE's kernel does not manipulate the
PLLs at all and relies on BROM and the boot loader to set them up. The
bitfields in the control registers are somewhat documented in a
Lauterback TRACE32 debug file in the kernel sources though. At the
moment, my driver extracts clock rates from the PLLs, but cannot change
them. A proper PLL clk is on my TODO list before I remove the [RFC] tag
from the submission. It will be necessary for the LTE hardware with
replacement boot loaders because BROM does not set up the LTE-related
PLL.

The "matrix" controller controls the main Cortex A53 CPU, the LTE ZSP,
SDIO and a few others. It is even more chaotic than the "top"
controller.

The "LSP" controller - I suspect it stands for "low speed peripherals" -
is very regular. One 32 bit register for 2 clock gates, two resets, one
mux (1-3 bit) and in some cases a 4 bit divider.

Not all clocks will have an explicit user in the end. I am defining a
lot of them simply to shut them off. The boot loader sets up a few of
the proprietary timers, which will send regular IRQs (although the
kernel of course doesn't need to listen to them). I don't plan to add a
driver for the proprietary timer as I see no use for them - the ARM arch
timer works just fine. I will add a driver for the very similar
proprietary watchdog though.

The top and matrix list is not exhaustive. There are other bits
that are enabled, but I couldn't deduce what they are controlling by
trial and error. Some of them seem to do nothing. Others cause an
instant hang of the board when disabled. I isolated a few (SRAM PCLK,
arm arch timer clock) where I don't see a reason to manipulate them. It
is quite likely that a handful more clocks will be added in the future,
but not a large number.

Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
---
 .../bindings/clock/zte,zx297520v3-clk.yaml         | 173 ++++++++++++++++++++
 include/dt-bindings/clock/zte,zx297520v3-clk.h     | 179 +++++++++++++++++++++
 2 files changed, 352 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml b/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml
new file mode 100644
index 000000000000..3b7084a18a97
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml
@@ -0,0 +1,173 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/zte,zx297520v3-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ZTE zx297520v3 SoC clock and reset controller
+
+maintainers:
+  - Stefan Dösinger <stefandoesinger@gmail.com>
+
+description: |
+  The zx297520v3's clock controller consists of 3 controllers, which generate
+  clocks for internal SoC devices. In addition to clocks it also has reset
+  controls for most, but not all, devices.
+
+  While there is a certain hierarchy among the controllers ("top" controlls core
+  parts like the boot-up Cortex M0, "matrix" controls the main CPU and LTE DSP,
+  "lsp" controls peripherals"), in practise all 3 are required to reasonably
+  operate the SoC.
+
+  The top controller has two inputs: a 26 MHz and a 32 KHz external oscillator.
+  They need to be provided as input clocks. The matrix controller controlls 10
+  clock lines that get fed into the LSP controller. The LSP device node needs
+  to list these input clocks.
+
+  The matrix controller consumes clocks generated by PLLs in the top
+  controller, but there are no controls in the top controller to sever this
+  link. The interface between these controllers is not expressed in the device
+  tree, but the matrix controller cannot work without the clock handles
+  registered by the top controller.
+
+  All available clocks are defined as preprocessor macros in
+  'dt-bindings/clock/zte,zx297520v3-clk.h' header.
+
+properties:
+  compatible:
+    enum:
+      - zte,zx297520v3-topclk
+      - zte,zx297520v3-matrixclk
+      - zte,zx297520v3-lspclk
+
+  clocks:
+    minItems: 2
+    maxItems: 10
+
+  clock-names:
+    minItems: 2
+    maxItems: 10
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+  reg:
+    maxItems: 1
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: zte,zx297520v3-topclk
+    then:
+      properties:
+        clocks:
+          items:
+            - description: External reference clock (26 MHz)
+            - description: External reference clock (32 KHz)
+        clock-names:
+          items:
+            - const: osc26m
+            - const: osc32k
+      required:
+        - clocks
+        - clock-names
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: zte,zx297520v3-lspclk
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Main PLL divided by 5 output from matrixclk (124.8 MHz)
+            - description: Main PLL divided by 4 output from matrixclk (156 MHz)
+            - description: Main PLL divided by 6 output from matrixclk (104 MHz)
+            - description: Main PLL divided by 8 output from matrixclk (78 MHz)
+            - description: Main PLL divided by 12 output from matrixclk (52 MHz)
+            - description: Main oscillator output from matrixclk (26 MHz)
+            - description: Timer oscillator output from matrixclk (32 KHz)
+            - description: LSP pclk output from matrixclk (26 MHz)
+            - description: TDM wclk mux output from matrixclk
+            - description: DPLL divided by 4 output from matrixclk (122.88 MHz)
+        clock-names:
+          items:
+            - const: mpll_d5
+            - const: mpll_d4
+            - const: mpll_d6
+            - const: mpll_d8
+            - const: mpll_d12
+            - const: osc26m
+            - const: osc32k
+            - const: pclk
+            - const: tdm_wclk
+            - const: dpll_d4
+      required:
+        - clocks
+        - clock-names
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#clock-cells'
+  - reg
+  - '#reset-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/zte,zx297520v3-clk.h>
+
+    osc26m: osc26m {
+      compatible = "fixed-clock";
+      clock-output-names = "osc26m";
+      #clock-cells = <0>;
+    };
+
+    osc32k: osc32k {
+      compatible = "fixed-clock";
+      clock-output-names = "osc32k";
+      #clock-cells = <0>;
+    };
+
+    topclk: topclk@13b000 {
+        compatible = "zte,zx297520v3-topclk";
+        reg = <0x0013b000 0x400>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+        clocks = <&osc26m>, <&osc32k>;
+        clock-names = "osc26m", "osc32k";
+    };
+
+    matrixclk: matrixclk@1306000 {
+        compatible = "zte,zx297520v3-matrixclk";
+        reg = <0x01306000 0x400>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
+
+    lspclk: lspclk@1400000 {
+        compatible = "zte,zx297520v3-lspclk";
+        reg = <0x01400000 0x100>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+
+        clocks = <&matrixclk ZX297520V3_LSP_MPLL_D5_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_MPLL_D4_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_MPLL_D6_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_MPLL_D8_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_MPLL_D12_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_OSC26M_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_OSC32K_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_PCLK>,
+                 <&matrixclk ZX297520V3_LSP_TDM_WCLK>,
+                 <&matrixclk ZX297520V3_LSP_DPLL_D4_WCLK>;
+        clock-names = "mpll_d5", "mpll_d4", "mpll_d6", "mpll_d8", "mpll_d12",
+                      "osc26m", "osc32k", "pclk", "tdm_wclk", "dpll_d4";
+    };
diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
new file mode 100644
index 000000000000..322b53be8b12
--- /dev/null
+++ b/include/dt-bindings/clock/zte,zx297520v3-clk.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) Stefan Dösinger.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_ZX297520V3_H
+#define __DT_BINDINGS_CLOCK_ZX297520V3_H
+
+#define ZX297520V3_AHB_WCLK			0
+#define ZX297520V3_AHB_PCLK			1
+#define ZX297520V3_PMM_WCLK			2
+#define ZX297520V3_PMM_PCLK			3
+#define ZX297520V3_USB_24M			4
+#define ZX297520V3_USB_AHB			5
+#define ZX297520V3_TIMER_T08_WCLK		6
+#define ZX297520V3_TIMER_T08_PCLK		7
+#define ZX297520V3_TIMER_T09_WCLK		8
+#define ZX297520V3_TIMER_T09_PCLK		9
+#define ZX297520V3_TIMER_T12_WCLK		10
+#define ZX297520V3_TIMER_T12_PCLK		11
+#define ZX297520V3_TIMER_T13_WCLK		12
+#define ZX297520V3_TIMER_T13_PCLK		13
+#define ZX297520V3_TIMER_T14_WCLK		14
+#define ZX297520V3_TIMER_T14_PCLK		15
+#define ZX297520V3_TIMER_T15_WCLK		16
+#define ZX297520V3_TIMER_T15_PCLK		17
+#define ZX297520V3_TIMER_T16_WCLK		18
+#define ZX297520V3_TIMER_T16_PCLK		19
+#define ZX297520V3_TIMER_T17_WCLK		20
+#define ZX297520V3_TIMER_T17_PCLK		21
+#define ZX297520V3_WDT_T18_WCLK			22
+#define ZX297520V3_WDT_T18_PCLK			23
+#define ZX297520V3_UART0_WCLK			24
+#define ZX297520V3_UART0_PCLK			25
+#define ZX297520V3_I2C0_WCLK			26
+#define ZX297520V3_I2C0_PCLK			27
+#define ZX297520V3_RTC_WCLK			28
+#define ZX297520V3_RTC_PCLK			29
+#define ZX297520V3_LPM_GSM_WCLK			30
+#define ZX297520V3_LPM_GSM_PCLK			31
+#define ZX297520V3_LPM_LTE_WCLK			32
+#define ZX297520V3_LPM_LTE_PCLK			33
+#define ZX297520V3_LPM_TD_WCLK			34
+#define ZX297520V3_LPM_TD_PCLK			35
+#define ZX297520V3_LPM_W_WCLK			36
+#define ZX297520V3_LPM_W_PCLK			37
+#define ZX297520V3_USIM1_WCLK			38
+#define ZX297520V3_USIM1_PCLK			39
+#define ZX297520V3_M0_WCLK			40
+#define ZX297520V3_TOPCLK_END			41
+
+#define ZX297520V3_AHB_RESET			0
+#define ZX297520V3_TIMER_T08_RESET		1
+#define ZX297520V3_TIMER_T09_RESET		2
+#define ZX297520V3_TIMER_T12_RESET		3
+#define ZX297520V3_TIMER_T13_RESET		4
+#define ZX297520V3_TIMER_T14_RESET		5
+#define ZX297520V3_TIMER_T15_RESET		6
+#define ZX297520V3_TIMER_T16_RESET		7
+#define ZX297520V3_TIMER_T17_RESET		8
+#define ZX297520V3_WDT_T18_RESET		9
+#define ZX297520V3_UART0_RESET			10
+#define ZX297520V3_I2C0_RESET			11
+#define ZX297520V3_RTC_RESET			12
+#define ZX297520V3_USIM1_RESET			13
+#define ZX297520V3_PMM_RESET			14
+#define ZX297520V3_GPIO8_RESET			15
+#define ZX297520V3_GPIO_RESET			16
+#define ZX297520V3_ZSP_RESET			17
+#define ZX297520V3_USB_RESET			18
+#define ZX297520V3_TOPRST_END			19
+
+#define ZX297520V3_CPU_WCLK			0
+#define ZX297520V3_CPU_PCLK			1
+#define ZX297520V3_SD0_WCLK			2
+#define ZX297520V3_SD0_PCLK			3
+#define ZX297520V3_SD1_WCLK			4
+#define ZX297520V3_SD1_PCLK			5
+#define ZX297520V3_SD1_CDET			6
+#define ZX297520V3_NAND_WCLK			7
+#define ZX297520V3_NAND_PCLK			8
+#define ZX297520V3_SSC_WCLK			9
+#define ZX297520V3_SSC_PCLK			10
+#define ZX297520V3_EDCP_WCLK			11
+#define ZX297520V3_EDCP_PCLK			12
+#define ZX297520V3_EDCP_SYNCAXI			13
+#define ZX297520V3_VOU_WCLK			14
+#define ZX297520V3_VOU_PCLK			15
+#define ZX297520V3_PDCFG_WCLK			16
+#define ZX297520V3_PDCFG_PCLK			17
+#define ZX297520V3_GMAC_WCLK			18
+#define ZX297520V3_GMAC_RMII			19
+#define ZX297520V3_GMAC_PCLK			20
+#define ZX297520V3_ZSP_WCLK			21
+#define ZX297520V3_MBOX_PCLK			22
+#define ZX297520V3_DMA_PCLK			23
+#define ZX297520V3_LSP_MPLL_D5_WCLK		24
+#define ZX297520V3_LSP_MPLL_D4_WCLK		25
+#define ZX297520V3_LSP_MPLL_D6_WCLK		26
+#define ZX297520V3_LSP_MPLL_D8_WCLK		27
+#define ZX297520V3_LSP_MPLL_D12_WCLK		28
+#define ZX297520V3_LSP_OSC26M_WCLK		29
+#define ZX297520V3_LSP_OSC32K_WCLK		30
+#define ZX297520V3_LSP_PCLK			31
+#define ZX297520V3_LSP_TDM_WCLK			32
+#define ZX297520V3_LSP_DPLL_D4_WCLK		33
+#define ZX297520V3_MATRIXCLK_END		34
+
+#define ZX297520V3_CPU_RESET			0
+#define ZX297520V3_SD0_RESET			1
+#define ZX297520V3_SD1_RESET			2
+#define ZX297520V3_NAND_RESET			3
+#define ZX297520V3_SSC_RESET			4
+#define ZX297520V3_EDCP_RESET			5
+#define ZX297520V3_VOU_RESET			6
+#define ZX297520V3_PDCFG_RESET			7
+#define ZX297520V3_GMAC_RESET			8
+#define ZX297520V3_DMA_RESET			9
+#define ZX297520V3_MATRIXRST_END		10
+
+#define ZX297520V3_TIMER_L1_WCLK		0
+#define ZX297520V3_TIMER_L1_PCLK		1
+#define ZX297520V3_WDT_L2_WCLK			2
+#define ZX297520V3_WDT_L2_PCLK			3
+#define ZX297520V3_WDT_L3_WCLK			4
+#define ZX297520V3_WDT_L3_PCLK			5
+#define ZX297520V3_I2C1_WCLK			6
+#define ZX297520V3_I2C1_PCLK			7
+#define ZX297520V3_I2S0_WCLK			8
+#define ZX297520V3_I2S0_PCLK			9
+#define ZX297520V3_I2S1_WCLK			10
+#define ZX297520V3_I2S1_PCLK			11
+#define ZX297520V3_QSPI_WCLK			12
+#define ZX297520V3_QSPI_PCLK			13
+#define ZX297520V3_UART1_WCLK			14
+#define ZX297520V3_UART1_PCLK			15
+#define ZX297520V3_I2C2_WCLK			16
+#define ZX297520V3_I2C2_PCLK			17
+#define ZX297520V3_SPI0_WCLK			18
+#define ZX297520V3_SPI0_PCLK			19
+#define ZX297520V3_TIMER_LB_WCLK		20
+#define ZX297520V3_TIMER_LB_PCLK		21
+#define ZX297520V3_TIMER_LC_WCLK		22
+#define ZX297520V3_TIMER_LC_PCLK		23
+#define ZX297520V3_UART2_WCLK			24
+#define ZX297520V3_UART2_PCLK			25
+#define ZX297520V3_WDT_LE_WCLK			26
+#define ZX297520V3_WDT_LE_PCLK			27
+#define ZX297520V3_TIMER_LF_WCLK		28
+#define ZX297520V3_TIMER_LF_PCLK		29
+#define ZX297520V3_SPI1_WCLK			30
+#define ZX297520V3_SPI1_PCLK			31
+#define ZX297520V3_TIMER_L11_WCLK		32
+#define ZX297520V3_TIMER_L11_PCLK		33
+#define ZX297520V3_TDM_WCLK			34
+#define ZX297520V3_TDM_PCLK			35
+#define ZX297520V3_LSPCLK_END			36
+
+#define ZX297520V3_TIMER_L1_RESET		0
+#define ZX297520V3_WDT_L2_RESET			1
+#define ZX297520V3_WDT_L3_RESET			2
+#define ZX297520V3_I2C1_RESET			3
+#define ZX297520V3_I2S0_RESET			4
+#define ZX297520V3_I2S1_RESET			5
+#define ZX297520V3_QSPI_RESET			6
+#define ZX297520V3_UART1_RESET			7
+#define ZX297520V3_I2C2_RESET			8
+#define ZX297520V3_SPI0_RESET			9
+#define ZX297520V3_TIMER_LB_RESET		10
+#define ZX297520V3_TIMER_LC_RESET		11
+#define ZX297520V3_UART2_RESET			12
+#define ZX297520V3_WDT_LE_RESET			13
+#define ZX297520V3_TIMER_LF_RESET		14
+#define ZX297520V3_SPI1_RESET			15
+#define ZX297520V3_TIMER_L11_RESET		16
+#define ZX297520V3_TDM_RESET			17
+#define ZX297520V3_LSPRST_END			18
+
+#endif /* __DT_BINDINGS_CLOCK_ZX297520V3_H */

-- 
2.53.0

[PATCH RFC 2/4] clk: zte: Introduce a driver for zx297520v3 top clocks and resets.

[Stefan Dösinger]

This register space controls core devices: PLLs, the AHB bus, a lot of
timers, the USB controller, the Cortex M0 processor that boots the board
and a few other devices. For some reason the LTE coprocessor is also
partially controlled by it. The main application processor and DDR
memory are not found here though.

The register to reboot the board is also found here.

Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
---
 drivers/clk/Kconfig              |   1 +
 drivers/clk/Makefile             |   1 +
 drivers/clk/zte/Kconfig          |  18 ++
 drivers/clk/zte/Makefile         |   2 +
 drivers/clk/zte/clk-zx297520v3.c | 584 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 606 insertions(+)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 3d803b4cf5c1..971ea6daa2b6 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -539,6 +539,7 @@ source "drivers/clk/visconti/Kconfig"
 source "drivers/clk/x86/Kconfig"
 source "drivers/clk/xilinx/Kconfig"
 source "drivers/clk/zynqmp/Kconfig"
+source "drivers/clk/zte/Kconfig"
 
 # Kunit test cases
 config CLK_KUNIT_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f7bce3951a30..c164a3de2b14 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -165,5 +165,6 @@ ifeq ($(CONFIG_COMMON_CLK), y)
 obj-$(CONFIG_X86)			+= x86/
 endif
 obj-y					+= xilinx/
+obj-$(CONFIG_ARCH_ZTE)			+= zte/
 obj-$(CONFIG_ARCH_ZYNQ)			+= zynq/
 obj-$(CONFIG_COMMON_CLK_ZYNQMP)         += zynqmp/
diff --git a/drivers/clk/zte/Kconfig b/drivers/clk/zte/Kconfig
new file mode 100644
index 000000000000..e7acd28832cd
--- /dev/null
+++ b/drivers/clk/zte/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# ZTE Clock Drivers
+#
+menu "Clock driver for ZTE SoC"
+	depends on ARCH_ZTE || COMPILE_TEST
+
+config COMMON_CLK_ZX297520V3
+	tristate "Clock driver for ZTE zx297520v3"
+	default SOC_ZX297520V3
+	help
+	  This driver supports ZTE zx297520v3 basic clocks.
+
+	  Enable this if you want to build a kernel that is able to run on
+	  boards based on this SoC. You can safely enable multiple clock
+	  drivers. The one(s) matching the device tree will be used.
+
+endmenu
diff --git a/drivers/clk/zte/Makefile b/drivers/clk/zte/Makefile
new file mode 100644
index 000000000000..3751ebcba0b0
--- /dev/null
+++ b/drivers/clk/zte/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_COMMON_CLK_ZX297520V3) += clk-zx297520v3.o
diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
new file mode 100644
index 000000000000..f73f5c006641
--- /dev/null
+++ b/drivers/clk/zte/clk-zx297520v3.c
@@ -0,0 +1,584 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Stefan Dösinger
+ */
+#include <dt-bindings/clock/zte,zx297520v3-clk.h>
+#include <linux/reset-controller.h>
+#include <linux/platform_device.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/reboot.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+
+static DEFINE_SPINLOCK(reg_lock);
+
+struct zx29_reset_reg {
+	void __iomem *reg;
+	u32 mask;
+};
+
+struct zx29_clk_controller {
+	void __iomem *base;
+	struct clk_hw_onecell_data *clocks;
+	struct reset_controller_dev rcdev;
+	struct zx29_reset_reg resets[];
+};
+
+static int zx297520v3_rst_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct zx29_clk_controller *data = container_of(rcdev, struct zx29_clk_controller, rcdev);
+	u32 val;
+
+	val = readl(data->resets[id].reg);
+	val &= ~data->resets[id].mask;
+	writel(val, data->resets[id].reg);
+
+	return 0;
+}
+
+static int zx297520v3_rst_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct zx29_clk_controller *data = container_of(rcdev, struct zx29_clk_controller, rcdev);
+	u32 val;
+
+	val = readl(data->resets[id].reg);
+	val |= data->resets[id].mask;
+	writel(val, data->resets[id].reg);
+
+	return 0;
+}
+
+static int zx297520v3_rst_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	int ret;
+
+	ret = zx297520v3_rst_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	usleep_range(100, 100 * 2);
+
+	return zx297520v3_rst_deassert(rcdev, id);
+}
+
+static int zx297520v3_rst_status(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct zx29_clk_controller *data = container_of(rcdev, struct zx29_clk_controller, rcdev);
+	u32 val;
+
+	val = readl(data->resets[id].reg);
+
+	return (val & data->resets[id].mask) == data->resets[id].mask;
+}
+
+const struct reset_control_ops zx297520v3_rst_ops = {
+	.assert		= zx297520v3_rst_assert,
+	.deassert	= zx297520v3_rst_deassert,
+	.reset		= zx297520v3_rst_reset,
+	.status		= zx297520v3_rst_status,
+};
+
+/* Used for gates where we don't know the parent input(s). Assume general bus clock. */
+static const char * const clk_unknown[] = {
+	"osc26m",
+};
+
+/* Used for gates where we know it is using the 26 mhz main clock. */
+static const char * const clk_main[] = {
+	"osc26m",
+};
+
+/* Top and matrix clocks are chaotic - I haven't found a consistent pattern behind their register
+ * and bit locations. Generally there are two gates (pclk, wclk), one mux, one reset and sometimes
+ * one divider, but exceptions apply. For some devices there is only a reset and some general
+ * (parent) clocks need setup. This structure plus macro handles the somewhat regular parts.
+ *
+ * There are some patterns that can be observed.
+ * mux 0x3c, div 0x48, gate 0x54
+ * mux 0x40, div 0x4c, gate 0x5c
+ * mux 0x44, div 0x50, gate 0x60
+ *
+ * For a 0 - 0xc - 0x18 pattern. Muxes from 0x3c to 0x44, dividers from 0x48 to 0x50, gates 0x54 to
+ * 0x60. The pattern is broken for timer t17 though.
+ *
+ * Gates have 4 bits per clock - bit 0 for wclk, bit 1 for pclk, bit 2 for something the ZTE kernel
+ * calls "gate" (the bits we use here are called "en"), which I don't know what it does, and bit 3
+ * seems unused. E.g. offset 0x54 accepts all bits in 0xF77F7F7F - suggesting RTC, I2C0 have an
+ * extra gate bit.
+ */
+struct zx297520v3_composite {
+	u32 reset_id, wclk_id, pclk_id;
+	const char *name;
+	u32 reset_reg, reset_shift;
+	u32 gate_reg, wclk_gate_shift, pclk_gate_shift;
+	const char *pclk_parent;
+	u32 mux_reg, mux_shift, mux_size;
+	const char * const *mux_sel;
+	u32 mux_sel_count;
+	u32 div_reg, div_shift, div_size;
+	u32 flags;
+};
+
+struct zx297520v3_gate {
+	u32 id;
+	const char *name, *parent;
+	u32 reg, shift;
+};
+
+#define _ZX_CLK(name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift, pclk_parent,\
+		mux_reg, mux_shift, mux_size, mux_sel,\
+		div_reg, div_shift, div_size, flags) \
+		{ZX297520V3_##name##_RESET, ZX297520V3_##name##_WCLK, ZX297520V3_##name##_PCLK,\
+		 #name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift, pclk_parent,\
+		 mux_reg, mux_shift, mux_size, mux_sel, ARRAY_SIZE(mux_sel),\
+		 div_reg, div_shift, div_size, flags}
+
+#define ZX_CLK(name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift,\
+	       mux_reg, mux_shift, mux_size, mux_sel,\
+	       div_reg, div_shift, div_size) \
+	       _ZX_CLK(name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift, "osc26m",\
+		mux_reg, mux_shift, mux_size, mux_sel,\
+		div_reg, div_shift, div_size, 0)
+
+#define ZX_CLK_CRIT(name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift,\
+	       mux_reg, mux_shift, mux_size, mux_sel,\
+	       div_reg, div_shift, div_size) \
+	       _ZX_CLK(name, reset_reg, reset_shift, gate_reg, wclk_shift, pclk_shift, "osc26m",\
+		mux_reg, mux_shift, mux_size, mux_sel,\
+		div_reg, div_shift, div_size, CLK_IS_CRITICAL)
+
+/* The default mpll settings multiply the 26 MHz reference clock times 24. A mux selection of 26 MHz
+ * could mean using the 26 MHz oscillator directly, or passing it through the PLL and divide by 24.
+ *
+ * If a UART is set to mpl_d6 (default 104 MHz), changing the mpll multipliers does affect UART
+ * timing as it should. This does not happen when the UART is set to 26 MHz input or timers that
+ * read 26 MHz input. This suggests 26 MHz clocks use the reference clock directly.
+ */
+static const char * const ahb_sel[] = {
+	"osc26m",
+	"mpll_d6",	/* 104 mhz */
+	"mpll_d8",	/* 78 mhz */
+	"mpll_d8",	/* 78 mhz */
+};
+
+static const char * const timer_top_sel[] = {
+	"osc32k",
+	"osc26m",
+};
+
+static const char * const uart_top_sel[] = {
+	"osc26m",
+	"mpll_d6",	/* 104 mhz */
+};
+
+static const char * const m0_sel[] = {
+	"osc26m",
+	"mpll_d6",	/* 104 mhz */
+	"mpll_d8",	/* 78 mhz */
+	"osc32k",	/* Yes, tested. It is SLLLLOOOOOWWW. */
+};
+
+static const struct zx297520v3_composite top_clocks[] = {
+	/*    (NAME,       RESET,     GATE,         MUX,                        DIV        ), */
+
+	/* AHB: Don't turn this one off. The clock mux works and impact can be tested e.g. with
+	 * iperf speed testing of the USB network connection. Values 2 and 3 give the same speed.
+	 */
+	ZX_CLK_CRIT(AHB,   0x70,  0,  0x54, 12, 13, 0x3c,  4, 2, ahb_sel,       0,     0, 0),
+
+	/* Pinmux (AON, TOP, IOCFG but not PDCFG). Critical as well until we have a driver that
+	 * consumes it. I don't think we'll realistically shut this off ever.
+	 *
+	 * Setting either bit 0 or 1 in register 0x58 makes the device work.
+	 */
+	ZX_CLK_CRIT(PMM,   0x74,  0,  0x58,  0,  1, 0x00,  0, 0, clk_unknown,   0,     0, 0),
+
+	/* Timers. We don't use any of them, just shut them off. The timers are named and sorted
+	 * by the IO address of the main timer controls. Some of the controls are documented in
+	 * ZTE's kernel. Some I found by trial and error.
+	 *
+	 * Timer T17 is used by the ZSP firmware. The rproc driver will enable them as needed.
+	 */
+	ZX_CLK(TIMER_T08,  0x78,   4, 0x5c,  8,  9, 0x40,  1, 1, timer_top_sel, 0x4c,   8, 4),
+	ZX_CLK(TIMER_T09,  0x78,   2, 0x5c,  4,  5, 0x40,  0, 1, timer_top_sel, 0x4c,   0, 4),
+	ZX_CLK(TIMER_T12,  0x74,   6, 0x54,  4,  5, 0x3c,  0, 1, timer_top_sel, 0x48,   0, 4),
+	ZX_CLK(TIMER_T13,  0x7c,   0, 0x60,  0,  1, 0x44,  0, 1, timer_top_sel, 0x50,   0, 4),
+	ZX_CLK(TIMER_T14,  0x7c,   2, 0x60,  4,  5, 0x44,  1, 1, timer_top_sel, 0x50,   4, 4),
+	ZX_CLK(TIMER_T15,  0x74,  10, 0x54, 20, 21, 0x3c,  3, 1, timer_top_sel, 0x48,   4, 4),
+	ZX_CLK(TIMER_T16,  0x7c,   4, 0x60,  8,  9, 0x44,  2, 1, timer_top_sel, 0x50,   8, 4),
+	ZX_CLK(TIMER_T17,  0x12c,  0, 0x128, 0,  1, 0x120, 0, 1, timer_top_sel, 0x124,  0, 4),
+
+	/* This watchdog is set up by the bootloader and in normal operation the m0 firmware will
+	 * feed the dog. The m0 firmware in turn wants to be fed in its own way. Since we normally
+	 * don't run any m0 firmware we shut it off by default and expose it to userspace via the
+	 * watchdog driver.
+	 */
+	ZX_CLK(WDT_T18,    0x74,  12, 0x54, 24, 25, 0x3c,  6, 1, timer_top_sel, 0x48,   8, 4),
+
+	ZX_CLK(I2C0,       0x74,   8, 0x54,  8,  9, 0x3c,  1, 1, uart_top_sel,  0,      0, 0),
+	ZX_CLK(UART0,      0x78,   6, 0x5c, 12, 13, 0x40,  2, 1, uart_top_sel,  0,      0, 0),
+
+	/* How does this RTC work? I don't know, the ZTE kernel does not talk to it. It has an
+	 * external RTC connected to I2C0.
+	 */
+	ZX_CLK(RTC,        0x74,   4, 0x54,  0,  1, 0x00,  0, 0, timer_top_sel, 0,      0, 0),
+
+	/* This doesn't see to be talking to the physical SIM card. I can turn it off on the ZTE
+	 * firmware without breaking LTE, and the "uicc" IRQ count keeps climbing. I think this is
+	 * a eSim-like chip that can be provisioned with data at runtime, but I have no idea how to
+	 * do it.
+	 */
+	ZX_CLK(USIM1,      0x74,  14, 0x54, 28, 29, 0x00,  0, 0, clk_main,      0x48,  12, 1),
+
+	/*    (NAME,       RESET,     GATE,         MUX,                        DIV         ), */
+};
+
+/* Stand-alone topclk gates. */
+static const struct zx297520v3_gate top_gates[] = {
+	{ZX297520V3_USB_24M,		"usb_24m",	"mpll_d26",	0x6c,	  3},
+	{ZX297520V3_USB_AHB,		"usb_ahb",	"AHB_wclk",	0x6c,	  4},
+	/* LTE: gate only as far as I can see. I looked for resets and did not find any. There may
+	 * be mux/div, but without understanding the behavior of this hardware it is impossible to
+	 * tell. They are sorted by physical MMIO address of the devices, which happens to be the
+	 * inverse order of the bits.
+	 *
+	 * I don't know what "LPM", "TD" and "W" mean. I copied them from ZTE's names.
+	 */
+	{ZX297520V3_LPM_GSM_WCLK,	"LPM_GSM_wclk",	clk_unknown[0],	0x58,	10},
+	{ZX297520V3_LPM_GSM_PCLK,	"LPM_GSM_pclk",	clk_unknown[0],	0x58,	11},
+	{ZX297520V3_LPM_LTE_WCLK,	"LPM_LTE_wclk",	clk_unknown[0],	0x58,	 8},
+	{ZX297520V3_LPM_LTE_PCLK,	"LPM_LTE_pclk",	clk_unknown[0],	0x58,	 9},
+	{ZX297520V3_LPM_TD_WCLK,	"LPM_TD_wclk",	clk_unknown[0],	0x58,	 6},
+	{ZX297520V3_LPM_TD_PCLK,	"LPM_TD_pclk",	clk_unknown[0],	0x58,	 7},
+	{ZX297520V3_LPM_W_WCLK,		"LPM_W_wclk",	clk_unknown[0],	0x58,	 4},
+	{ZX297520V3_LPM_W_PCLK,		"LPM_W_pclk",	clk_unknown[0],	0x58,	 5},
+	/* There are PCLKs for BROM/SRAM2 in 0x54, bit 16 and SRAM1 in 0x54, bit 18. Turning them
+	 * off locks up the Cortex M0 coproc. Not added to the kernel until a way is found to
+	 * recover the Cortex M0 or evidence of power savings.
+	 */
+};
+
+static int zx297520v3_pll(struct device *dev, void __iomem *base, const char *name,
+			  struct clk *parent)
+{
+	/* These are the fractionals of the PLLs I have seen. There should be a better way to
+	 * generate them than hardcode the list.
+	 */
+	static const unsigned int pll_fract[] = {2, 3, 4, 5, 6, 8, 12, 26};
+
+	unsigned long ref, refdiv, fbdiv, vco, postdiv1, postdiv2, freq;
+	struct clk_hw *hw;
+	char plldiv[16];
+	unsigned int i;
+	u32 val;
+
+	/* PLLs are configured by the boot rom, we only read their settings to know how the rate
+	 * of the derived clocks. ZTE's sources explain the PLL register contents only in a .cmm
+	 * file (A Lauterback TRACE32 script). When calculating the frequencies with the default
+	 * PLL configuration the results match the fixed rate clocks from their clock driver.
+	 *
+	 * The 26mhz and 32khz clocks can be easily observed with the timers. The 104mhz output
+	 * can be observed through the UART. All others can only be indirectly observed by e.g.
+	 * comparing the CPU speed at 26mhz and 624mhz.
+	 *
+	 * The contents of the PLL registers is as follows:
+	 *
+	 * Bit 31: PLL Locked
+	 * Bit 30: PLL disable bit (0 = PLL enabled, 1 = PLL disabled)
+	 * Bits 29:25: Unknown. Could be a parent mux
+	 * Bits 24:18: refdiv
+	 * Bits 17:6: fbdiv
+	 * Bits 5:3: post vco divider 1
+	 * Bits 2:0: post vco divider 2
+	 *
+	 * There is a second register following immediately afterwards that is supposed to have
+	 * another value that gets added to fbdiv, but it doesn't seem to make a difference in my
+	 * testing and it is always 0 in the preconfigured values.
+	 */
+	ref = clk_get_rate(parent);
+	val = readl(base);
+
+	refdiv = (val & GENMASK(24, 18)) >> 18;
+	fbdiv = (val & GENMASK(17, 6)) >> 6;
+	postdiv1 = (val & GENMASK(5, 3)) >> 3;
+	postdiv2 = (val & GENMASK(2, 0));
+	dev_dbg(dev, "%s: reference clock %lu HZ, PLL setting 0x%08x\n", name, ref, val);
+
+	if (!refdiv || !postdiv1 || !postdiv2)
+		return -EINVAL;
+
+	vco = (ref / refdiv) * fbdiv;
+	freq = vco / postdiv1 / postdiv2;
+	dev_dbg(dev, "%s: %lu MHZ\n", name, freq / 1000000);
+
+	hw = devm_clk_hw_register_fixed_factor(dev, name, __clk_get_name(parent), 0, fbdiv,
+					       refdiv * postdiv1 * postdiv2);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	for (i = 0; i < ARRAY_SIZE(pll_fract); ++i) {
+		sprintf(plldiv, "%s_d%u", name, pll_fract[i]);
+		hw = devm_clk_hw_register_fixed_factor(dev, plldiv, name, 0, 1, pll_fract[i]);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		dev_dbg(dev, "%s: %lu hz\n", clk_hw_get_name(hw), clk_hw_get_rate(hw));
+	}
+
+	return 0;
+}
+
+static int zx297520v3_composite(struct device *dev, void __iomem *base,
+				struct clk_hw_onecell_data *clocks, struct zx29_reset_reg *resets,
+				const struct zx297520v3_composite *input, size_t size)
+{
+	char pclk_name[32], wclk_name[32], mux_name[32], div_name[32];
+	const char *wclk_parent, *div_parent;
+	struct clk_hw *hw;
+	size_t copy_res;
+	unsigned int i;
+
+	for (i = 0; i < size; ++i) {
+		strscpy(wclk_name, input[i].name, ARRAY_SIZE(wclk_name));
+		strcat(wclk_name, "_wclk");
+		strscpy(pclk_name, input[i].name, ARRAY_SIZE(pclk_name));
+		strcat(pclk_name, "_pclk");
+		strscpy(mux_name, input[i].name, ARRAY_SIZE(mux_name));
+		strcat(mux_name, "_mux");
+		strscpy(div_name, input[i].name, ARRAY_SIZE(div_name));
+		strcat(div_name, "_div");
+
+		resets[input[i].reset_id].reg = base + input[i].reset_reg;
+		resets[input[i].reset_id].mask = BIT(input[i].reset_shift) +
+						 BIT(input[i].reset_shift + 1);
+
+		if (input[i].div_size)
+			wclk_parent = div_name;
+		else if (input[i].mux_size)
+			wclk_parent = mux_name;
+		else
+			wclk_parent = input[i].mux_sel[0];
+
+		hw = devm_clk_hw_register_gate(dev, pclk_name, input[i].pclk_parent, input[i].flags,
+					       base + input[i].gate_reg, input[i].pclk_gate_shift,
+					       0, &reg_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		clocks->hws[input[i].pclk_id] = hw;
+
+		if (input[i].mux_size) {
+			hw = devm_clk_hw_register_mux(dev, mux_name, input[i].mux_sel,
+						      input[i].mux_sel_count, 0,
+						      base + input[i].mux_reg,
+						      input[i].mux_shift, input[i].mux_size, 0,
+						      &reg_lock);
+			if (IS_ERR(hw))
+				return PTR_ERR(hw);
+			div_parent = mux_name;
+		} else {
+			div_parent = input[i].mux_sel[0];
+		}
+
+		hw = devm_clk_hw_register_gate(dev, wclk_name, wclk_parent,
+					       input[i].flags | CLK_SET_RATE_PARENT,
+					       base + input[i].gate_reg, input[i].wclk_gate_shift,
+					       0, &reg_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		clocks->hws[input[i].wclk_id] = hw;
+
+		if (!input[i].div_size)
+			continue;
+
+		hw = devm_clk_hw_register_divider(dev, div_name, div_parent, CLK_SET_RATE_PARENT,
+						  base + input[i].div_reg, input[i].div_shift,
+						  input[i].div_size, 0, &reg_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+	}
+
+	return 0;
+}
+
+static int zx297520v3_gate(struct device *dev, void __iomem *base,
+			   struct clk_hw_onecell_data *clocks,
+			   const struct zx297520v3_gate *input, size_t size)
+{
+	struct clk_hw *hw;
+	unsigned int i;
+
+	for (i = 0; i < size; ++i) {
+		hw = devm_clk_hw_register_gate(dev, input[i].name, input[i].parent,
+					       CLK_SET_RATE_PARENT, base + input[i].reg,
+					       input[i].shift, 0, &reg_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		clocks->hws[input[i].id] = hw;
+	}
+
+	return 0;
+}
+
+static int zx_restart_handle(struct sys_off_data *data)
+{
+	struct zx29_clk_controller *top = data->cb_data;
+
+	writel(1, top->base);
+	mdelay(1000);
+
+	pr_emerg("Unable to restart system\n");
+	return NOTIFY_DONE;
+}
+
+static int zx297520_topclk_probe(struct platform_device *pdev)
+{
+	struct zx29_clk_controller *top;
+	struct device *dev = &pdev->dev;
+	struct clk_hw *hw;
+	struct clk *clk;
+	int res;
+
+	dev_info(dev, "Registering zx297520v3 top clocks and resets\n");
+	top = devm_kzalloc(dev, offsetof(struct zx29_clk_controller,
+					 resets[ZX297520V3_TOPRST_END]), GFP_KERNEL);
+	if (!top)
+		return -ENOMEM;
+
+	top->clocks = devm_kzalloc(dev, struct_size(top->clocks, hws,
+				   ZX297520V3_TOPCLK_END), GFP_KERNEL);
+	if (!top->clocks)
+		return -ENOMEM;
+	top->clocks->num = ZX297520V3_TOPCLK_END;
+
+	top->base = devm_platform_ioremap_resource(pdev, 0);
+	WARN_ON(!top->base);
+
+	/* Offset 0x0 is the global board reset
+	 * Offset 0x4 gives some static boot information - raw NAND or SPI NAND
+	 */
+
+	clk = devm_clk_get_prepared(dev, "osc32k");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "32 KHz input clock not found 1\n");
+		return PTR_ERR(clk);
+	}
+
+	clk = devm_clk_get_prepared(dev, "osc26m");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "26 MHz input clock not found\n");
+		return PTR_ERR(clk);
+	}
+
+	/* Default setting: 0x48040c11. 624/312/156... */
+	res = zx297520v3_pll(dev, top->base + 0x8, "mpll", clk);
+	if (res)
+		return res;
+
+	/* There is a PLL at 0x10 called "upll" in ZTE's code, but I don't see any documented
+	 * consumers. Default setting 0x48347811. 480/240/160 MHz.
+	 */
+
+	/* Default value 0x4834902d. Feeds dpll. 46.08 MHz */
+	res = zx297520v3_pll(dev, top->base + 0x100, "unknownpll", clk);
+	if (res)
+		return res;
+
+	/* The documentation says 491.52 MHz and measurement with the LSP TDM device supports this.
+	 * The default value is 0x480C2011. To get to 491.52 with these settings it needs a 23.04
+	 * MHz reference clock, which matches unknownpll_d2. If unknownpll is disabled, dpll loses
+	 * its lock.
+	 *
+	 * The proprietary LTE driver or coproc enables and disables it. TDM and I2S can use it.
+	 *
+	 * FIXME: Isn't there a nicer way to get the struct clk for unknownpll_d2? I don't want to
+	 * return all generated clocks from zx297520v3_pll or store them in the controller because
+	 * I need one of them here. I could always pass the parent by name though.
+	 */
+	res = zx297520v3_pll(dev, top->base + 0x18, "dpll", __clk_lookup("unknownpll_d2"));
+	if (res)
+		return res;
+	res = zx297520v3_pll(dev, top->base + 0x110, "gpll", clk);
+	if (res)
+		return res;
+
+	res = zx297520v3_composite(dev, top->base, top->clocks, top->resets,
+				 top_clocks, ARRAY_SIZE(top_clocks));
+	if (res)
+		return res;
+
+	res = zx297520v3_gate(dev, top->base, top->clocks, top_gates, ARRAY_SIZE(top_gates));
+	if (res)
+		return res;
+
+	/* The Cortex M0 coprocessor. It is responsible for booting the board and runs some power
+	 * management helper code on the stock firmware, but isn't critical. We can run custom code
+	 * on it but currently do not.  These bits control the speed and the values are mentioned in
+	 * ZTE's uboot. It isn't clear to me if this is directly responsible for the m0 clock, or
+	 * if it is the input to another clock. I also haven't found a gate that shuts the m0 off
+	 * and allows restarting. There don't seem to be resets either.
+	 *
+	 * Also note the comment about SRAM1 and SRAM2 PCLKs. They can be turned off to crash the
+	 * M0 by feeding it garbage instructions.
+	 */
+	hw = devm_clk_hw_register_mux(dev, "m0_wclk", m0_sel, ARRAY_SIZE(m0_sel),
+				      0, top->base + 0x38, 0, 2, 0, &reg_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	top->clocks->hws[ZX297520V3_M0_WCLK] = hw;
+
+	of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, top->clocks);
+
+	res = devm_register_restart_handler(dev, zx_restart_handle, top);
+	if (res)
+		dev_err(dev, "can't register restart handler (res=%d)\n", res);
+
+	/* Stray reset bits follow.
+	 *
+	 * I haven't found any clocks for GPIO. It probably wouldn't make much
+	 * sense anyway. Only one bit per controller.
+	 */
+	top->resets[ZX297520V3_GPIO8_RESET].reg = top->base + 0x74;
+	top->resets[ZX297520V3_GPIO8_RESET].mask = BIT(2);
+	top->resets[ZX297520V3_GPIO_RESET].reg = top->base + 0x74;
+	top->resets[ZX297520V3_GPIO_RESET].mask = BIT(3);
+
+	/* USB reset. This is slightly special because it needs to wait for a ready bit after
+	 * deasserting.
+	 *
+	 * FIXME: Actually implement this waiting.
+	 */
+	top->resets[ZX297520V3_USB_RESET].reg = top->base + 0x80;
+	top->resets[ZX297520V3_USB_RESET].mask = BIT(3) | BIT(4) | BIT(5);
+
+	/* This bit is set by ZTE's cpko.ko blob, it looks like a reset bit for the LTE DSP
+	 * coprocessor. Clocks for it are in matrixclk.
+	 */
+	top->resets[ZX297520V3_ZSP_RESET].reg = top->base + 0x13c;
+	top->resets[ZX297520V3_ZSP_RESET].mask = BIT(0);
+
+	top->rcdev.owner = THIS_MODULE;
+	top->rcdev.nr_resets = ZX297520V3_TOPRST_END;
+	top->rcdev.ops = &zx297520v3_rst_ops;
+	top->rcdev.of_node = dev->of_node;
+	return devm_reset_controller_register(dev, &top->rcdev);
+}
+
+static const struct of_device_id of_match_zx297520v3_topclk[] = {
+	{ .compatible = "zte,zx297520v3-topclk"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, of_match_zx297520v3_topclk);
+
+static struct platform_driver clk_zx297520v3_topclk = {
+	.probe = zx297520_topclk_probe,
+	.driver = {
+		.name = "clk-zx297520v3-topclk",
+		.of_match_table = of_match_zx297520v3_topclk,
+	},
+};
+module_platform_driver(clk_zx297520v3_topclk);
+
+MODULE_AUTHOR("Stefan Dösinger <stefandoesinger@gmail.com>");
+MODULE_DESCRIPTION("ZTE zx297520v3 clock driver");
+MODULE_LICENSE("GPL");

-- 
2.53.0

[PATCH RFC 3/4] clk: zte: Introduce a driver for zx297520v3 matrix clocks and resets.

[Stefan Dösinger]

This controls the CPU, DSP, DDR RAM, ethernet, SDIO controllers and a
few more devices. It also contains a number of clock gates to pass
clock signals down to the next controller.

Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
---
 drivers/clk/zte/clk-zx297520v3.c | 215 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 214 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
index f73f5c006641..0c06add433ee 100644
--- a/drivers/clk/zte/clk-zx297520v3.c
+++ b/drivers/clk/zte/clk-zx297520v3.c
@@ -266,7 +266,7 @@ static int zx297520v3_pll(struct device *dev, void __iomem *base, const char *na
 	/* These are the fractionals of the PLLs I have seen. There should be a better way to
 	 * generate them than hardcode the list.
 	 */
-	static const unsigned int pll_fract[] = {2, 3, 4, 5, 6, 8, 12, 26};
+	static const unsigned int pll_fract[] = {2, 3, 4, 5, 6, 8, 12, 16, 26};
 
 	unsigned long ref, refdiv, fbdiv, vco, postdiv1, postdiv2, freq;
 	struct clk_hw *hw;
@@ -579,6 +579,219 @@ static struct platform_driver clk_zx297520v3_topclk = {
 };
 module_platform_driver(clk_zx297520v3_topclk);
 
+static const char * const cpu_sel[] = {
+	"osc26m",
+	"mpll",		/* 624 MHz */
+	"mpll_d2",	/* 312 MHz */
+	"mpll_d4",	/* 156 MHz */
+};
+
+static const char * const sd0_sel[] = {
+	"osc26m",
+	"mpll_d4",	/* 156 MHz */
+	"gpll_d2",	/* 100 MHz */
+	"mpll_d8",	/* 78 MHz */
+	"gpll_d4",	/* 50 MHz */
+	"gpll_d8",	/* 25 MHz */
+};
+
+static const char * const sd1_sel[] = {
+	"osc26m",
+	"gpll_d2",	/* 100 MHz */
+	"mpll_d8",	/* 78 MHz */
+	"gpll_d4",	/* 50 MHz */
+	"mpll_d16",	/* 39 MHz */
+	"gpll_d8",	/* 25 MHz */
+};
+
+static const char * const nand_sel[] = {
+	"mpll_d4",	/* 156 MHz */
+	"osc26m",
+};
+
+static const char * const edcp_sel[] = {
+	"osc26m",
+	"mpll_d4",	/* 156 MHz */
+	"mpll_d5",	/* 124.8 MHz */
+	"mpll_d6",	/* 104 MHz */
+};
+
+static const char * const tdm_sel[] = {
+	"osc26m",
+	"dpll_d4",	/* 122.88 MHz */
+	"mpll_d6",	/* 104 MHz */
+};
+
+static const struct zx297520v3_composite matrix_clocks[] = {
+	/* Both 0x24 and 0x28 bits 1 and 2 stop the CPU. There is also a bit in topclk+0x138, which
+	 * ZTE's uboot calls "A53 reset", which also stops the CPU. I can't really tell the
+	 * difference between matrix+28 and top+138. The clock can be disabled and enabled from the
+	 * Cortex M0 and it will nicely stop and restart the A53, retaining all state.
+	 *
+	 * 0x50, bits 0-3 have the DDR clock. A lot of DDR gates and resets are in 0x100.
+	 */
+	ZX_CLK_CRIT(CPU,    0x28,  1,  0x24,  1,  2, 0x20,  0, 2, cpu_sel,     0,     0, 0),
+	/* TODO: 0x54 bit 14 and 0x54 bit 6 are supposed to be card detection clocks. */
+	ZX_CLK(SD0,         0x58,  1,  0x54, 12, 13, 0x50,  4, 3, sd0_sel,     0,     0, 0),
+	ZX_CLK(SD1,         0x58,  0,  0x54,  4,  5, 0x50,  8, 3, sd1_sel,     0,     0, 0),
+	/* This is some "denali" NAND, not the qspi connected one. */
+	ZX_CLK(NAND,        0x58,  4,  0x54, 20, 21, 0x50, 12, 2, nand_sel,    0,     0, 0),
+	ZX_CLK(SSC,         0x94, 24,  0x84,  1,  2, 0,     0, 0, clk_unknown, 0,     0, 0),
+	ZX_CLK(EDCP,        0x68,  0,  0x64,  2,  1, 0x50, 16, 2, edcp_sel,    0,     0, 0),
+	/* PDCFG. Like PMM, either clock bit will allow the device to function. */
+	ZX_CLK_CRIT(PDCFG,  0x94, 20,  0x88,  0,  1, 0x50, 16, 2, clk_unknown, 0,     0, 0),
+	/* There are a lot more VOU related controls in these registers, but turning off the main
+	 * clock seems to shut off the entire VOU MMIO range.
+	 */
+	ZX_CLK(VOU,        0x16c,  0, 0x168,  0,  1, 0,     0, 0, clk_main,       0,     0, 0),
+};
+
+static const struct zx297520v3_gate matrix_gates[] = {
+	/* ZTE's driver has a statemt to the effect of *(matrix->base+0x11C) = 5, with a comment
+	 * suggesting that this sets a 50 mhz clock. The clock code itself lists the parents of
+	 * these clock as 50mhz pll output, but the GMAC driver never enables the clocks.
+	 *
+	 * The clocks below are enabled by the boot loader though, so they are on. And it turns
+	 * out that they are necessary for proper operation of the ethernet hardware. As far as
+	 * I can see trough experimentation, bit 1 affects the PHY whereas 0 and 2 affect the
+	 * MAC chip itself.
+	 *
+	 * Chain the wclk and rmii clk together for now. I haven't found a way to make either
+	 * the mdio node or the phy node enable a clock. According to ethernet-phy.yaml it is
+	 * supposed to be possible, but I can't find code to that effect in of_mdio.c.
+	 */
+	{ZX297520V3_GMAC_PCLK,	"gmac_pclk",		"gpll_d4",	0x110,	  0},
+	{ZX297520V3_GMAC_RMII,	"gmac_rmii",		"gpll_d4",	0x110,	  1},
+	{ZX297520V3_GMAC_RMII,	"gmac_wclk",		"gmac_rmii",	0x110,	  2},
+
+	/* ZSP aka LTE DSP clock. I think there is a mux at matrix+0x30, but I have no idea
+	 * about the frequencies it selects. Gate is at matrix+0x3c.
+	 */
+	{ZX297520V3_ZSP_WCLK,	"zsp_wclk",		"osc26m",	0x3c,	  0},
+
+	/* Mailbox. I haven't found a reset for this. It seems to have a PCLK only - turning it off
+	 * makes the MMIO area read 0x0. It looks like it does not need a WCLK. It generates IRQs
+	 * fine with just bit 2 set. Bits 1 and 3 are 0 by default in this register.
+	 */
+	{ZX297520V3_MBOX_PCLK,	"mbox_pclk",		"osc26m",	0x88,	  2},
+
+	/* DMA Controller. It has a reset and PCLK, but no WCLK. */
+	{ZX297520V3_DMA_PCLK,	"dma_pclk",		"osc26m",	0x94,	  3},
+
+	/* There is another clock controlling some "GSM" IP at 0xF3000000 in 0x88, bit 8. It appears
+	 * to be a PCLK, but I have not found a matching WCLK or reset yet.
+	 */
+
+	/* LSP uplink clocks. The PCLK is fairly obvious (disabling it shuts off the entire LSP
+	 * register area). The WCLK speeds were deduced by setting timers and qspi muxes to a
+	 * specific speed and seeing which bit in matrix+0x7c needs to be enabled for the device
+	 * to work.
+	 *
+	 * Due to the timers I am certain about the 26mhz and 32khz clocks. I cannot directly
+	 * observe the qspi mux frequency, so the clock rates depend on ZTE's qspi mux selection
+	 * being correct.
+	 *
+	 * Two additional bits are specific to sound components - the mux for the LSP's TDM IP is
+	 * in matrixclk and gets passed down. I2S has a mux in LSP, which can select the dpll_d4
+	 * clock.
+	 *
+	 * This code is commented out until the next patch because disabling unused clocks without
+	 * an LSP consumer breaks the UART.
+	 */
+#if 0
+	{ZX297520V3_LSP_MPLL_D5_WCLK,	"lsp_mpll_d5",	"mpll_d5",	0x7c,	  0},
+	{ZX297520V3_LSP_MPLL_D4_WCLK,	"lsp_mpll_d4",	"mpll_d4",	0x7c,	  1},
+	{ZX297520V3_LSP_MPLL_D6_WCLK,	"lsp_mpll_d6",	"mpll_d6",	0x7c,	  2},
+	{ZX297520V3_LSP_MPLL_D8_WCLK,	"lsp_mpll_d8",	"mpll_d8",	0x7c,	  3},
+	{ZX297520V3_LSP_MPLL_D12_WCLK,	"lsp_mpll_d12",	"mpll_d12",	0x7c,	  4},
+	{ZX297520V3_LSP_OSC26M_WCLK,	"lsp_osc26m",	"osc26m",	0x7c,	  5},
+	{ZX297520V3_LSP_OSC32K_WCLK,	"lsp_osc32k",	"osc32k",	0x7c,	  6},
+	{ZX297520V3_LSP_PCLK,		"lsp_pclk",	"osc26m",	0x7c,	  7},
+	{ZX297520V3_LSP_TDM_WCLK,	"lsp_tdm_wclk",	"tdm_mux",	0x7c,	  8},
+	{ZX297520V3_LSP_DPLL_D4_WCLK,	"lsp_dpll_d4",	"dpll_d4",	0x7c,	  9},
+#endif
+};
+
+static int zx297520_matrixclk_probe(struct platform_device *pdev)
+{
+	struct zx29_clk_controller *matrix;
+	struct device *dev = &pdev->dev;
+	struct clk_hw *hw;
+	unsigned int i;
+	int res;
+
+	dev_info(dev, "Registering zx297520v3 matrix clocks\n");
+
+	matrix = devm_kzalloc(dev, offsetof(struct zx29_clk_controller,
+					    resets[ZX297520V3_MATRIXRST_END]), GFP_KERNEL);
+	if (!matrix)
+		return -ENOMEM;
+
+	matrix->clocks = devm_kzalloc(dev, struct_size(matrix->clocks, hws,
+				   ZX297520V3_MATRIXCLK_END), GFP_KERNEL);
+	if (!matrix->clocks)
+		return -ENOMEM;
+	matrix->clocks->num = ZX297520V3_MATRIXCLK_END;
+
+	matrix->base = devm_platform_ioremap_resource(pdev, 0);
+	WARN_ON(!matrix->base);
+
+	/* One stray mux: The TDM mux is in matrixclk and it is passed to the LSP controller. In a
+	 * way the link gate (LSP_TDM_WCLK) could be considered a matching gate, but there is no
+	 * reset and no pclk.
+	 */
+	hw = devm_clk_hw_register_mux(dev, "tdm_mux", tdm_sel, ARRAY_SIZE(tdm_sel), 0,
+				      matrix->base + 0x50, 24, 2, 0, &reg_lock);
+
+	res = zx297520v3_composite(dev, matrix->base, matrix->clocks, matrix->resets,
+				   matrix_clocks, ARRAY_SIZE(matrix_clocks));
+	if (res)
+		return res;
+
+	res = zx297520v3_gate(dev, matrix->base, matrix->clocks,
+			      matrix_gates, ARRAY_SIZE(matrix_gates));
+	if (res)
+		return res;
+
+	for (i = 0; i < ZX297520V3_MATRIXCLK_END; i++) {
+		if (IS_ERR(matrix->clocks->hws[i])) {
+			pr_err("zx297520 clk %d: register failed with %ld\n",
+				i, PTR_ERR(matrix->clocks->hws[i]));
+			return -ENODEV;
+		}
+	}
+
+	res = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, matrix->clocks);
+	if (res)
+		return res;
+
+	matrix->resets[ZX297520V3_DMA_RESET].reg = matrix->base + 0x70;
+	matrix->resets[ZX297520V3_DMA_RESET].mask = BIT(0) | BIT(1);
+	matrix->resets[ZX297520V3_GMAC_RESET].reg = matrix->base + 0x114;
+	matrix->resets[ZX297520V3_GMAC_RESET].mask = BIT(0) | BIT(1);
+
+	matrix->rcdev.owner = THIS_MODULE;
+	matrix->rcdev.nr_resets = ZX297520V3_MATRIXRST_END;
+	matrix->rcdev.ops = &zx297520v3_rst_ops;
+	matrix->rcdev.of_node = dev->of_node;
+	return devm_reset_controller_register(dev, &matrix->rcdev);
+}
+
+static const struct of_device_id of_match_zx297520v3_matrixclk[] = {
+	{ .compatible = "zte,zx297520v3-matrixclk"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, of_match_zx297520v3_matrixclk);
+
+static struct platform_driver clk_zx297520v3_matrixclk = {
+	.probe = zx297520_matrixclk_probe,
+	.driver = {
+		.name = "clk-zx297520v3-matrixclk",
+		.of_match_table = of_match_zx297520v3_matrixclk,
+	},
+};
+module_platform_driver(clk_zx297520v3_matrixclk);
+
 MODULE_AUTHOR("Stefan Dösinger <stefandoesinger@gmail.com>");
 MODULE_DESCRIPTION("ZTE zx297520v3 clock driver");
 MODULE_LICENSE("GPL");

-- 
2.53.0

[PATCH RFC 4/4] clk: zte: Introduce a driver for zx297520v3 LSP clocks and resets.

[Stefan Dösinger]

"LSP" is ZTE's term for this part of the SoC, I suspect it stands for
"low speed peripherals". The main UART is here, together with the flash
controller and more surplus proprietary timers.

It also has two more I2C controllers that supposedly connect to a
battery charger, SPI for displays and I2S for analog telephones. The
boards I have don't have any of these components though.

Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
---
 drivers/clk/zte/clk-zx297520v3.c | 183 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 183 insertions(+)

diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
index 0c06add433ee..e69be686c1b1 100644
--- a/drivers/clk/zte/clk-zx297520v3.c
+++ b/drivers/clk/zte/clk-zx297520v3.c
@@ -792,6 +792,189 @@ static struct platform_driver clk_zx297520v3_matrixclk = {
 };
 module_platform_driver(clk_zx297520v3_matrixclk);
 
+/* LSP clock entries have a common pattern: Bit 0 for PCLK, Bit 1 for WCLK. Bit 4 (and sometimes
+ * more) for WCLK mux.
+ *
+ * Bit 8 and 9 are reset bits. I don't know the difference between the two, but they both
+ * need to be set to deassert the reset.
+ *
+ * Bits 12-16 can be a divisor, but not all clocks have it. Some clocks have a divisor in 16-20.
+ *
+ * The ID given in this table is the first register in the device's MMIO space. ZTE's drivers
+ * usually call this a version register, but it looks more like a device identifier.
+ *
+ * It looks like the registers map to devices like this:
+ *
+ * Timer reg	function	div	dev offset(lsp + xxxx)	ID
+ * 0x0: Read-only, probably device identifier			0x00752100
+ * 0x4:		timer_l1	Y	0x1000			0x02020000
+ * 0x8:		watchdog_l2	Y	0x2000			0x02020000
+ * 0xc:		watchdog_l3	Y	0x3000			0x02020000
+ * 0x10:	i2c1		N	0x4000			0x01020000
+ * 0x14:	i2s0		Yh	0x5000			0x01030000
+ * 0x18:	always 0	N	-
+ * 0x1c:	i2s1		Yh	0x6000			0x01030000
+ * 0x20:	always 0	N	-
+ * 0x24:	qspi		N	0x7000			0x01040000
+ * 0x28:	uart1		N	0x8000			0x01060000
+ * 0x2c:	i2c2		N	0x9000			0x01020000
+ * 0x30:	spi0		Y	0xa000			0x01040000
+ * 0x34:	timer_lb	Y	0xb000			0x02020000
+ * 0x38:	timer_lc	Y	0xc000			0x02020000
+ * 0x3c:	uart2		N	0xd000			0x01060000
+ * 0x40:	watchdog_le	Y	0xe000			0x02020000
+ * 0x44:	timer_lf	Y	0xf000			0x02020000
+ * 0x48:	spi1		Y	0x10000			0x01040000
+ * 0x4c:	timer_l11	Y	0x11000			0x02020000
+ * 0x50:	tdm		Y	0x12000			0x01040000
+ *
+ * Registers 0x58, 0x5c, 0x60, 0x64, 0x68 seem to contain more controls for i2s and tdm.
+ */
+
+static const char * const timer_lsp_sel[] = {
+	"lsp_osc32k",
+	"lsp_osc26m",
+};
+
+static const char * const uart_lsp_sel[] = {
+	"lsp_osc26m",
+	"lsp_mpll_d6",
+};
+
+static const char * const i2s_lsp_sel[] = {
+	"lsp_osc26m",
+	"lsp_dpll_d4",
+	"lsp_mpll_d6",
+	/* Unknown */
+};
+
+static const char * const tdm_lsp_sel[] = {
+	"lsp_tdm_wclk",
+};
+
+static const char * const spi_lsp_sel[] = {
+	"lsp_osc26m",
+	"lsp_mpll_d4",
+	"lsp_mpll_d6",
+	/* Unknown */
+};
+
+static const char * const qspi_lsp_sel[] = {
+	"lsp_osc26m",
+	"lsp_mpll_d4",
+	"lsp_mpll_d5",
+	"lsp_mpll_d6",
+	"lsp_mpll_d8",
+	"lsp_mpll_d12",
+	"lsp_osc26m",
+	"lsp_osc26m",
+};
+
+#define LSP_CLOCK(offset, name, mux, div_shift, div_size) {\
+		ZX297520V3_##name##_RESET, ZX297520V3_##name##_WCLK, ZX297520V3_##name##_PCLK,\
+		#name, offset, 8, offset, 0, 1, "lsp_pclk", offset, 4, 4, mux, ARRAY_SIZE(mux),\
+		offset, div_shift, div_size, 0}
+
+static const struct zx297520v3_composite lsp_clocks[] =  {
+	LSP_CLOCK(0x4,	TIMER_L1,	timer_lsp_sel,	0,	0),
+	LSP_CLOCK(0x8,	WDT_L2,		timer_lsp_sel,	0,	0),
+	LSP_CLOCK(0xc,	WDT_L3,		timer_lsp_sel,	0,	0),
+	LSP_CLOCK(0x10,	I2C1,		uart_lsp_sel,	0,	0),
+	LSP_CLOCK(0x14,	I2S0,		i2s_lsp_sel,	16,	4),
+	LSP_CLOCK(0x1c,	I2S1,		i2s_lsp_sel,	16,	4),
+	LSP_CLOCK(0x24,	QSPI,		qspi_lsp_sel,	0,	0),
+	LSP_CLOCK(0x28,	UART1,		uart_lsp_sel,	0,	0),
+	LSP_CLOCK(0x2C,	I2C2,		uart_lsp_sel,	0,	0),
+	LSP_CLOCK(0x30,	SPI0,		spi_lsp_sel,	12,	4),
+	LSP_CLOCK(0x34,	TIMER_LB,	timer_lsp_sel,	12,	4),
+	LSP_CLOCK(0x38,	TIMER_LC,	timer_lsp_sel,	12,	4),
+	LSP_CLOCK(0x3c,	UART2,		uart_lsp_sel,	0,	0),
+	LSP_CLOCK(0x40,	WDT_LE,		timer_lsp_sel,	12,	4),
+	LSP_CLOCK(0x44,	TIMER_LF,	timer_lsp_sel,	12,	4),
+	LSP_CLOCK(0x48,	SPI1,		spi_lsp_sel,	12,	4),
+	LSP_CLOCK(0x4c,	TIMER_L11,	timer_lsp_sel,	12,	4),
+	LSP_CLOCK(0x50,	TDM,		tdm_lsp_sel,	16,	4),
+};
+
+#undef LSP_CLOCK
+
+static int zx297520_lspclk_probe(struct platform_device *pdev)
+{
+	static const char * const *parent_names[] = { "mpll_d5", "mpll_d4", "mpll_d6", "mpll_d8",
+						      "mpll_d12", "osc26m", "osc32k", "pclk" };
+
+	struct zx29_clk_controller *lsp;
+	struct device *dev = &pdev->dev;
+	struct clk *parent;
+	unsigned int i;
+	int res;
+
+	dev_info(dev, "Registering zx297520v3 LSP clocks and resets\n");
+
+	lsp = devm_kzalloc(dev, offsetof(struct zx29_clk_controller,
+					 resets[ZX297520V3_LSPRST_END]), GFP_KERNEL);
+	if (!lsp)
+		return -ENOMEM;
+
+	lsp->clocks = devm_kzalloc(dev, struct_size(lsp->clocks, hws,
+				   ZX297520V3_LSPCLK_END), GFP_KERNEL);
+	if (!lsp->clocks)
+		return -ENOMEM;
+	lsp->clocks->num = ZX297520V3_LSPCLK_END;
+
+	lsp->base = devm_platform_ioremap_resource(pdev, 0);
+	WARN_ON(!lsp->base);
+
+	/* TODO: Technically we can disable the pclk if all LSP devices are shut down, but that
+	 * needs custom clk ops to tiptoe around a disabled LSP pclk before attempting to access
+	 * the actual clock. In normal operation it is unlikely that all LSP devices are shut down
+	 * simultaneously though as UART and NAND are located here.
+	 */
+	parent = devm_clk_get_enabled(dev, "pclk");
+	if (IS_ERR(parent)) {
+		dev_err(dev, "failed to find lsp pclk\n");
+		return PTR_ERR(parent);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(parent_names); ++i) {
+		parent = devm_clk_get(dev, parent_names[i]);
+		if (IS_ERR(parent)) {
+			dev_err(dev, "failed to find lsp %s clock\n", parent_names[i]);
+			return PTR_ERR(parent);
+		}
+	}
+
+	res = zx297520v3_composite(dev, lsp->base, lsp->clocks, lsp->resets,
+				 lsp_clocks, ARRAY_SIZE(lsp_clocks));
+	if (res)
+		return res;
+
+	res = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, lsp->clocks);
+	if (res)
+		return res;
+
+	lsp->rcdev.owner = THIS_MODULE;
+	lsp->rcdev.nr_resets = ZX297520V3_LSPRST_END;
+	lsp->rcdev.ops = &zx297520v3_rst_ops;
+	lsp->rcdev.of_node = dev->of_node;
+	return devm_reset_controller_register(dev, &lsp->rcdev);
+}
+
+static const struct of_device_id of_match_zx297520v3_lspclk[] = {
+	{ .compatible = "zte,zx297520v3-lspclk"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, of_match_zx297520v3_lspclk);
+
+static struct platform_driver zx297520v3_lspclk = {
+	.probe = zx297520_lspclk_probe,
+	.driver = {
+		.name = "clk-zx297520v3-lspclk",
+		.of_match_table = of_match_zx297520v3_lspclk,
+	},
+};
+module_platform_driver(zx297520v3_lspclk);
+
 MODULE_AUTHOR("Stefan Dösinger <stefandoesinger@gmail.com>");
 MODULE_DESCRIPTION("ZTE zx297520v3 clock driver");
 MODULE_LICENSE("GPL");

-- 
2.53.0

Re: [PATCH RFC 4/4] clk: zte: Introduce a driver for zx297520v3 LSP clocks and resets.

[Stefan Dösinger]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

Am Sonntag, 10. Mai 2026, 22:01:03 Ostafrikanische Zeit schrieben Sie:
> +       static const char * const *parent_names[] = { "mpll_d5", "mpll_d4",
> "mpll_d6", "mpll_d8", +                                                    
> "mpll_d12", "osc26m", "osc32k", "pclk" };

So this obviously doesn't build, and I didn't spot it because the git branch I 
sent this from didn't have the Kconfig options introduced by my other patch 
series. I'll resend, my apologies for the quick v2 spam.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH RFC 1/4] dt-bindings: clk: zte: Add zx297520v3 clock and reset bindings.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Incomplete Device Tree modeling and schema validation for the matrix clock controller.
- [Low] The 'osc26m' and 'osc32k' fixed-clock example nodes are missing the mandatory 'clock-frequency' property.
- [Low] Non-standard generic node names in schema examples.
--

commit 241b89034c405c46e2b45636517463ab2381b279
Author: Stefan Dösinger <stefandoesinger@gmail.com>

dt-bindings: clk: zte: Add zx297520v3 clock and reset bindings.

This commit introduces device tree bindings for the ZTE zx297520v3 clock
and reset controllers. It defines compatible strings for the top, matrix,
and lsp controllers, along with required properties and standard macros.

> diff --git a/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml b/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml
> new file mode 100644
> index 0000000000000..3b7084a18a975
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx297520v3-clk.yaml

[ ... ]

> +  The matrix controller consumes clocks generated by PLLs in the top
> +  controller, but there are no controls in the top controller to sever this
> +  link. The interface between these controllers is not expressed in the device
> +  tree, but the matrix controller cannot work without the clock handles
> +  registered by the top controller.

Should this hardware dependency be explicitly modeled in the device tree?
Device tree design principles generally rely on explicitly modeled
connections to handle probe ordering correctly.

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - zte,zx297520v3-topclk
> +      - zte,zx297520v3-matrixclk
> +      - zte,zx297520v3-lspclk
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 10
> +
> +  clock-names:
> +    minItems: 2
> +    maxItems: 10

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: zte,zx297520v3-topclk

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: zte,zx297520v3-lspclk

Since clocks and clock-names are defined globally in the schema, but there
is no conditional block explicitly forbidding them for the matrix controller
using zte,zx297520v3-matrixclk, will the schema silently permit invalid clock
assignments for the matrix node?

Could an if/then block be added to set clocks to false for the matrix
controller?

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +
> +    osc26m: osc26m {
> +      compatible = "fixed-clock";
> +      clock-output-names = "osc26m";
> +      #clock-cells = <0>;
> +    };
> +
> +    osc32k: osc32k {
> +      compatible = "fixed-clock";
> +      clock-output-names = "osc32k";
> +      #clock-cells = <0>;
> +    };

Are these fixed-clock example nodes missing the mandatory clock-frequency
property? Omitting it might cause dt_binding_check to report schema
validation errors.

> +
> +    topclk: topclk@13b000 {
> +        compatible = "zte,zx297520v3-topclk";

[ ... ]

> +    matrixclk: matrixclk@1306000 {
> +        compatible = "zte,zx297520v3-matrixclk";

[ ... ]

> +    lspclk: lspclk@1400000 {
> +        compatible = "zte,zx297520v3-lspclk";

This isn't a bug, but should these node names reflect the generic class
of the device, like clock-controller@13b000, instead of hardware block
designations like topclk, matrixclk, and lspclk? Standard conventions
require generic node names, and dt_binding_check might warn about this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-zx29clk-v1-0-e1bacfffe967@gmail.com?part=1

Re: [PATCH RFC 2/4] clk: zte: Introduce a driver for zx297520v3 top clocks and resets.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] The driver uses the internal `__clk_lookup` function, which is not exported to modules, causing a linker error.
- [High] The reset controller operations lack locking, leading to read-modify-write race conditions on shared registers.
- [High] The clock provider is registered using `of_clk_add_hw_provider()` but never deregistered, leading to a Use-After-Free vulnerability.
- [High] Reset controller callbacks dereference unpopulated reset IDs without validation, leading to a NULL pointer dereference.
- [High] Incorrect error checking for `devm_platform_ioremap_resource()` leads to an unhandled `ERR_PTR` dereference.
- [High] The reset controller's `.status` callback returns inverted logic compared to the reset API requirements.
--

commit b59c44c93de9913db3a45af0e58b45b578c88685
Author: Stefan Dösinger <stefandoesinger@gmail.com>

clk: zte: Introduce a driver for zx297520v3 top clocks and resets.

This commit introduces a driver for the ZTE zx297520v3 top clocks and
resets, which controls core devices like PLLs, the AHB bus, and timers.

> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> new file mode 100644
> index 0000000000000..f73f5c006641f
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx297520v3.c

[ ... ]

> +static int zx297520v3_rst_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct zx29_clk_controller *data = container_of(rcdev, struct zx29_clk_controller, rcdev);
> +	u32 val;
> +
> +	val = readl(data->resets[id].reg);

Does this code safely handle unpopulated reset IDs?

The resets array is zero-initialized via devm_kzalloc(), and only specific IDs
have their .reg address populated in the probe function. If a consumer requests
an unpopulated ID, could data->resets[id].reg be NULL, leading to a NULL
pointer dereference here?

Additionally, does this read-modify-write sequence need locking?

Several reset IDs share the same physical register (e.g., offset 0x74 is used
by PMM, TIMER_T12, and others). If concurrent reset operations occur for
different devices on the same register, could the read-modify-write cycles
overlap and corrupt the register state?

> +	val &= ~data->resets[id].mask;
> +	writel(val, data->resets[id].reg);
> +
> +	return 0;
> +}

[ ... ]

> +static int zx297520v3_rst_status(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct zx29_clk_controller *data = container_of(rcdev, struct zx29_clk_controller, rcdev);
> +	u32 val;
> +
> +	val = readl(data->resets[id].reg);
> +
> +	return (val & data->resets[id].mask) == data->resets[id].mask;
> +}

Is the return value of this status callback inverted?

In zx297520v3_rst_assert(), the reset is asserted by clearing the mask bits.
However, this status function returns 1 when the bits are set (deasserted) and
0 when the bits are cleared (asserted). The reset API expects the status
callback to return a positive value when the reset line is asserted.

[ ... ]

> +static int zx297520_topclk_probe(struct platform_device *pdev)
> +{
> +	struct zx29_clk_controller *top;
> +	struct device *dev = &pdev->dev;
> +	struct clk_hw *hw;
> +	struct clk *clk;
> +	int res;

[ ... ]

> +	top->base = devm_platform_ioremap_resource(pdev, 0);
> +	WARN_ON(!top->base);

Does devm_platform_ioremap_resource() return NULL on failure?

This function returns an ERR_PTR (such as ERR_PTR(-EINVAL)) on failure, not
NULL. If mapping fails, the WARN_ON check will pass and the error pointer will
be used as a base address in subsequent calls like zx297520v3_pll(), causing
an invalid memory access. Should this be checked with IS_ERR() instead?

[ ... ]

> +	/* The documentation says 491.52 MHz and measurement with the LSP TDM device supports this.
> +	 * The default value is 0x480C2011. To get to 491.52 with these settings it needs a 23.04
> +	 * MHz reference clock, which matches unknownpll_d2. If unknownpll is disabled, dpll loses
> +	 * its lock.
> +	 *
> +	 * The proprietary LTE driver or coproc enables and disables it. TDM and I2S can use it.
> +	 *
> +	 * FIXME: Isn't there a nicer way to get the struct clk for unknownpll_d2? I don't want to
> +	 * return all generated clocks from zx297520v3_pll or store them in the controller because
> +	 * I need one of them here. I could always pass the parent by name though.
> +	 */
> +	res = zx297520v3_pll(dev, top->base + 0x18, "dpll", __clk_lookup("unknownpll_d2"));

Will this cause a linker error if the driver is built as a module?

The Kconfig defines COMMON_CLK_ZX297520V3 as a tristate option, but
__clk_lookup() is an internal clock framework function that is not exported.
If the driver is compiled as a module, this might cause an undefined symbol
linker error for __clk_lookup.

[ ... ]

> +	of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, top->clocks);

Is there a missing cleanup for this clock provider?

The driver registers the clock provider with of_clk_add_hw_provider(), but
doesn't seem to use the managed devm_of_clk_add_hw_provider() or provide a
.remove callback to deregister it.

If a later probe step (like devm_reset_controller_register()) fails, or if the
driver is unbound, top->clocks will be freed by devres. Could this leave a
dangling pointer in the global of_clk_providers list and cause a
use-after-free?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-zx29clk-v1-0-e1bacfffe967@gmail.com?part=2

Re: [PATCH RFC 3/4] clk: zte: Introduce a driver for zx297520v3 matrix clocks and resets.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Incorrect error checking for `devm_platform_ioremap_resource()` return value leads to an invalid pointer dereference.
- [High] The critical `PDCFG` clock shares mux register bits with `EDCP` and defines an invalid mux parent size.
- [High] Missing cleanup for OF clock provider causes a use-after-free on probe failure or module unbind.
- [High] Error codes from clock registration are masked by a hardcoded `-ENODEV`, breaking deferred probing.
- [High] Reset assert and deassert operations perform read-modify-write on shared hardware registers without holding locks.
- [Medium] Duplicate DT index mapping for `ZX297520V3_GMAC_RMII` overwrites clock provider array data.
- [Medium] Unchecked return value from `devm_clk_hw_register_mux()` for `tdm_mux`.
- [Low] Multiple `module_platform_driver()` macros in a single file cause a build error.
- [Low] Unacceptable use of `#if 0` blocks to store dead code.
--

commit 2a00ceb112a3ae2f6a880753c5234263bae84bcf
Author: Stefan Dösinger <stefandoesinger@gmail.com>
Date:   Sun May 10 19:01:02 2026 +0000

    clk: zte: Introduce a driver for zx297520v3 matrix clocks and resets.
    
    This controls the CPU, DSP, DDR RAM, ethernet, SDIO controllers and a
    few more devices. It also contains a number of clock gates to pass
    clock signals down to the next controller.

> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> index f73f5c006641f..0c06add433ee9 100644
> --- a/drivers/clk/zte/clk-zx297520v3.c
> +++ b/drivers/clk/zte/clk-zx297520v3.c
[ ... ]
> @@ -579,6 +579,219 @@ static struct platform_driver clk_zx297520v3_topclk = {
>  };
>  module_platform_driver(clk_zx297520v3_topclk);
>  
[ ... ]
> +static const struct zx297520v3_composite matrix_clocks[] = {
[ ... ]
> +	ZX_CLK(EDCP,        0x68,  0,  0x64,  2,  1, 0x50, 16, 2, edcp_sel,    0,     0, 0),
> +	/* PDCFG. Like PMM, either clock bit will allow the device to function. */
> +	ZX_CLK_CRIT(PDCFG,  0x94, 20,  0x88,  0,  1, 0x50, 16, 2, clk_unknown, 0,     0, 0),

Can changing the rate or parent of the EDCP clock inadvertently modify the PDCFG
parent? Both clocks appear to share the same mux register and shift
(0x50, 16, 2).

Since PDCFG is a critical clock with only a single parent (clk_unknown), would
it be safer to use 0, 0, 0 for its mux registers to prevent it from being
accidentally disabled?

[ ... ]
> +static const struct zx297520v3_gate matrix_gates[] = {
[ ... ]
> +	{ZX297520V3_GMAC_PCLK,	"gmac_pclk",		"gpll_d4",	0x110,	  0},
> +	{ZX297520V3_GMAC_RMII,	"gmac_rmii",		"gpll_d4",	0x110,	  1},
> +	{ZX297520V3_GMAC_RMII,	"gmac_wclk",		"gmac_rmii",	0x110,	  2},

Does this unintentionally assign the identical ID ZX297520V3_GMAC_RMII to both
gmac_rmii and gmac_wclk?

During registration, gmac_wclk might silently overwrite gmac_rmii in the
clocks->hws array, making the first clock inaccessible via direct DT provider
lookup.

[ ... ]
> +	/* This code is commented out until the next patch because disabling unused clocks without
> +	 * an LSP consumer breaks the UART.
> +	 */
> +#if 0
> +	{ZX297520V3_LSP_MPLL_D5_WCLK,	"lsp_mpll_d5",	"mpll_d5",	0x7c,	  0},
> +	{ZX297520V3_LSP_MPLL_D4_WCLK,	"lsp_mpll_d4",	"mpll_d4",	0x7c,	  1},
[ ... ]
> +	{ZX297520V3_LSP_DPLL_D4_WCLK,	"lsp_dpll_d4",	"dpll_d4",	0x7c,	  9},
> +#endif
> +};

Is it strictly necessary to include this block of code inside an #if 0 block?
Usually, code is removed entirely until it is fully functional to adhere to
mainline coding standards.

> +
> +static int zx297520_matrixclk_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	matrix->base = devm_platform_ioremap_resource(pdev, 0);
> +	WARN_ON(!matrix->base);

Does this correctly handle failure? devm_platform_ioremap_resource() returns an
error pointer like -EINVAL on failure, not NULL.

A WARN_ON(!matrix->base) would evaluate to false, allowing the code to continue
and subsequently dereference an invalid pointer. Should this check
IS_ERR(matrix->base) instead?

> +
> +	/* One stray mux: The TDM mux is in matrixclk and it is passed to the LSP controller. In a
> +	 * way the link gate (LSP_TDM_WCLK) could be considered a matching gate, but there is no
> +	 * reset and no pclk.
> +	 */
> +	hw = devm_clk_hw_register_mux(dev, "tdm_mux", tdm_sel, ARRAY_SIZE(tdm_sel), 0,
> +				      matrix->base + 0x50, 24, 2, 0, &reg_lock);

Is it intentional to ignore the return value of devm_clk_hw_register_mux()?

If this registration fails and returns an error pointer, the error is never
checked, which might silently break clocks depending on tdm_mux as a parent.

[ ... ]
> +	for (i = 0; i < ZX297520V3_MATRIXCLK_END; i++) {
> +		if (IS_ERR(matrix->clocks->hws[i])) {
> +			pr_err("zx297520 clk %d: register failed with %ld\n",
> +				i, PTR_ERR(matrix->clocks->hws[i]));
> +			return -ENODEV;
> +		}
> +	}

Will returning a hardcoded -ENODEV break deferred probing?

If a required parent clock is not yet available, masking the actual error code
from PTR_ERR() prevents -EPROBE_DEFER from propagating to the driver core,
causing the probe to fail permanently.

> +
> +	res = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, matrix->clocks);
> +	if (res)
> +		return res;

Could this leak the provider registration or cause a use-after-free on probe
failure?

If the subsequent call to devm_reset_controller_register() fails, the
devm-managed matrix->clocks will be freed, but the global OF provider remains
registered since of_clk_add_hw_provider() doesn't tie cleanup to the driver's
lifecycle.

Would devm_of_clk_add_hw_provider() be better suited here?

[ ... ]
> +static struct platform_driver clk_zx297520v3_matrixclk = {
> +	.probe = zx297520_matrixclk_probe,
> +	.driver = {
> +		.name = "clk-zx297520v3-matrixclk",
> +		.of_match_table = of_match_zx297520v3_matrixclk,
> +	},
> +};
> +module_platform_driver(clk_zx297520v3_matrixclk);

Since module_platform_driver(clk_zx297520v3_topclk) already exists earlier in
this file, does adding another module_platform_driver() here cause a build
error?

The macro expands to module_init() and module_exit(), and using it more than
once typically causes a redefinition failure when compiled as a loadable module.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-zx29clk-v1-0-e1bacfffe967@gmail.com?part=3

Re: [PATCH RFC 4/4] clk: zte: Introduce a driver for zx297520v3 LSP clocks and resets.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Incorrect error checking for devm_platform_ioremap_resource() leading to kernel panic.
- [High] The clock provider is registered without devres, leading to a Use-After-Free.
- [High] PCLK and WCLK gate shifts are swapped in the LSP_CLOCK macro.
- [High] Unprotected read-modify-write data race on shared LSP hardware registers.
- [Low] Incompatible pointer type for parent_names array.
--

commit a23190a3a7795f784d87109b7d775267f0845b7b
Author: Stefan Dösinger <stefandoesinger@gmail.com>

clk: zte: Introduce a driver for zx297520v3 LSP clocks and resets.

"LSP" is ZTE's term for this part of the SoC, I suspect it stands for
"low speed peripherals". The main UART is here, together with the flash
controller and more surplus proprietary timers.

> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> index 0c06add433ee9..e69be686c1b1d 100644
> --- a/drivers/clk/zte/clk-zx297520v3.c
> +++ b/drivers/clk/zte/clk-zx297520v3.c
[ ... ]
> +/* LSP clock entries have a common pattern: Bit 0 for PCLK, Bit 1 for WCLK. Bit 4 (and sometimes
> + * more) for WCLK mux.
[ ... ]
> +#define LSP_CLOCK(offset, name, mux, div_shift, div_size) {\
> +		ZX297520V3_##name##_RESET, ZX297520V3_##name##_WCLK, ZX297520V3_##name##_PCLK,\
> +		#name, offset, 8, offset, 0, 1, "lsp_pclk", offset, 4, 4, mux, ARRAY_SIZE(mux),\
> +		offset, div_shift, div_size, 0}

The comment above states that Bit 0 is for PCLK and Bit 1 is for WCLK.
However, passing 0 and 1 to this macro seems to map to wclk_gate_shift and
pclk_gate_shift in struct zx297520v3_composite.

Does this accidentally assign wclk_gate_shift to 0 and pclk_gate_shift to 1,
swapping the gate controls?

Also, the clock gate, mux, and reset controls are assigned to the exact same
memory offset (a shared 32-bit register).

The clock framework uses reg_lock for read-modify-write operations. Do the
reset controller operations (like zx297520v3_rst_assert) use the same lock?
If they read and write these registers locklessly, could concurrent clock
and reset operations cause a data race resulting in lost writes?

> +static const struct zx297520v3_composite lsp_clocks[] =  {
[ ... ]
> +};
> +
> +#undef LSP_CLOCK
> +
> +static int zx297520_lspclk_probe(struct platform_device *pdev)
> +{
> +	static const char * const *parent_names[] = { "mpll_d5", "mpll_d4", "mpll_d6", "mpll_d8",
> +						      "mpll_d12", "osc26m", "osc32k", "pclk" };

Does this array declaration use an incompatible pointer type?

The array is declared as an array of pointers to const char *, but is
initialized directly with string literals. When parent_names[i] is passed to
devm_clk_get() below, it passes a pointer-to-pointer instead of a string.
This can trigger build errors on compilers like GCC 14.

Should this be declared as static const char * const parent_names[] instead?

> +
> +	struct zx29_clk_controller *lsp;
[ ... ]
> +	lsp->clocks = devm_kzalloc(dev, struct_size(lsp->clocks, hws,
> +				   ZX297520V3_LSPCLK_END), GFP_KERNEL);
> +	if (!lsp->clocks)
> +		return -ENOMEM;
> +	lsp->clocks->num = ZX297520V3_LSPCLK_END;
> +
> +	lsp->base = devm_platform_ioremap_resource(pdev, 0);
> +	WARN_ON(!lsp->base);

Since devm_platform_ioremap_resource() returns an ERR_PTR on failure rather
than NULL, this WARN_ON() will not catch failures because an ERR_PTR
evaluates to true.

If the invalid error pointer is then passed into zx297520v3_composite()
as the base address, will this lead to a kernel panic when read/write
operations occur?

Should this use IS_ERR() and return PTR_ERR() instead?

> +
> +	/* TODO: Technically we can disable the pclk if all LSP devices are shut down, but that
[ ... ]
> +	res = zx297520v3_composite(dev, lsp->base, lsp->clocks, lsp->resets,
> +				 lsp_clocks, ARRAY_SIZE(lsp_clocks));
> +	if (res)
> +		return res;
> +
> +	res = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, lsp->clocks);

Since lsp->clocks is allocated using devm_kzalloc(), it will be
automatically freed if devm_reset_controller_register() fails later or if
the driver is unbound.

Because the clock provider is registered with of_clk_add_hw_provider()
instead of the managed devm_of_clk_add_hw_provider(), could the provider
remain registered to the device node after lsp->clocks is freed?

Will this leave a dangling pointer that triggers a use-after-free upon the
next clock lookup?

> +	if (res)
> +		return res;
> +
> +	lsp->rcdev.owner = THIS_MODULE;
> +	lsp->rcdev.nr_resets = ZX297520V3_LSPRST_END;
> +	lsp->rcdev.ops = &zx297520v3_rst_ops;
> +	lsp->rcdev.of_node = dev->of_node;
> +	return devm_reset_controller_register(dev, &lsp->rcdev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-zx29clk-v1-0-e1bacfffe967@gmail.com?part=4