[PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[Navid Ghahremani]

Add Device Tree binding schemas for the ZTE ZX279128S clock controllers:
topcrm, lspcrpm, and crm2. Define fixed clock lists and simple-mfd
for child nodes inside topcrm.

Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
---
 .../bindings/clock/zte,zx279128s-crm2.yaml    |  48 +++++++++
 .../bindings/clock/zte,zx279128s-lspcrpm.yaml | 101 ++++++++++++++++++
 .../bindings/clock/zte,zx279128s-topcrm.yaml  |  86 +++++++++++++++
 include/dt-bindings/clock/zte,zx279128s-crm.h |  51 +++++++++
 4 files changed, 286 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
 create mode 100644 include/dt-bindings/clock/zte,zx279128s-crm.h

diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
new file mode 100644
index 0000000000..aa90ad15d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/zte,zx279128s-crm2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ZTE ZX279128S CRM2 clock and reset controller
+
+maintainers:
+  - Navid Ghahremani <ghahramani.navid@gmail.com>
+
+description:
+  Top CRM 2 (clock and reset module 2) provides clocks and resets for
+  PCIe controllers on the ZTE ZX279128S SoC.
+
+properties:
+  compatible:
+    oneOf:
+      - const: zte,zx279128s-crm2
+      - items:
+          - const: zte,zx279128s-crm2
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@94100000 {
+        compatible = "zte,zx279128s-crm2";
+        reg = <0x94100000 0x1000>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml
new file mode 100644
index 0000000000..3448829a69
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/zte,zx279128s-lspcrpm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ZTE ZX279128S Low Speed Peripheral Clock Modules
+
+maintainers:
+  - Stefan Dösinger <stefandoesinger@gmail.com>
+  - Navid Ghahremani <ghahramani.navid@gmail.com>
+
+description: |
+  Each of the two groups of low speed peripherals (LSP) has a peripheral clock
+  module (LSP CRPM) of its own, fed by the top CRM.
+
+properties:
+  compatible:
+    enum:
+      - zte,zx279128s-lsp0crpm
+      - zte,zx279128s-lsp1crpm
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 4
+    maxItems: 5
+
+  clock-names:
+    minItems: 4
+    maxItems: 5
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: zte,zx279128s-lsp0crpm
+    then:
+      properties:
+        clocks:
+          items:
+            - description: register (APB) clock of the group
+            - description: 25 MHz work clock
+            - description: 32.768 kHz work clock
+            - description: 100 MHz work clock
+        clock-names:
+          items:
+            - const: pclk
+            - const: wclk25m
+            - const: wclk32k
+            - const: wclk100m
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: zte,zx279128s-lsp1crpm
+    then:
+      properties:
+        clocks:
+          items:
+            - description: register (APB) clock of the group
+            - description: AXI clock of the group
+            - description: 25 MHz work clock
+            - description: 49.152 MHz work clock
+            - description: 100 MHz work clock
+        clock-names:
+          items:
+            - const: pclk
+            - const: aclk
+            - const: wclk25m
+            - const: wclk49m
+            - const: wclk100m
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/zte,zx279128s-crm.h>
+
+    clock-controller@94400000 {
+        compatible = "zte,zx279128s-lsp0crpm";
+        reg = <0x94400000 0x1000>;
+        clocks = <&topcrm ZX279128S_TOP_LSP0_PCLK>,
+                 <&topcrm ZX279128S_TOP_LSP0_25M>,
+                 <&topcrm ZX279128S_TOP_LSP0_32K>,
+                 <&topcrm ZX279128S_TOP_LSP0_100M>;
+        clock-names = "pclk", "wclk25m", "wclk32k", "wclk100m";
+        #clock-cells = <1>;
+    };
diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
new file mode 100644
index 0000000000..9466661929
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/zte,zx279128s-topcrm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ZTE ZX279128S Top Clock and Reset Module
+
+maintainers:
+  - Stefan Dösinger <stefandoesinger@gmail.com>
+  - Navid Ghahremani <ghahramani.navid@gmail.com>
+
+description: |
+  The top clock and reset module (CRM) of the ZTE ZX279128S holds the PLLs,
+  bus clocks, peripheral clocks and the reset controller.
+
+properties:
+  compatible:
+    items:
+      - const: zte,zx279128s-topcrm
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: 25 MHz crystal oscillator
+
+  clock-names:
+    items:
+      - const: osc
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  reboot:
+    type: object
+    $ref: /schemas/power/reset/syscon-reboot.yaml#
+    description: System reboot controller child node
+
+patternProperties:
+  "^phy@[01]$":
+    type: object
+    description: PCIe PHY child nodes
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/zte,zx279128s-crm.h>
+    #include <dt-bindings/reset/zte,zx279128s-reset.h>
+
+    topcrm: clock-controller@94000000 {
+        compatible = "zte,zx279128s-topcrm", "syscon", "simple-mfd";
+        reg = <0x94000000 0x1000>;
+        clocks = <&osc>;
+        clock-names = "osc";
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+
+        reboot {
+            compatible = "syscon-reboot";
+            offset = <0x44>;
+            mask = <0x1>;
+            value = <0x1>;
+        };
+    };
diff --git a/include/dt-bindings/clock/zte,zx279128s-crm.h b/include/dt-bindings/clock/zte,zx279128s-crm.h
new file mode 100644
index 0000000000..dadb2cd007
--- /dev/null
+++ b/include/dt-bindings/clock/zte,zx279128s-crm.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2026 Navid Ghahremani <ghahramani.navid@gmail.com>
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_ZTE_ZX279128S_CRM_H
+#define _DT_BINDINGS_CLOCK_ZTE_ZX279128S_CRM_H
+
+/* Top CRM */
+#define ZX279128S_TOP_PLL_A9		0
+#define ZX279128S_TOP_PLL_LSP		1
+#define ZX279128S_TOP_CPU		2
+#define ZX279128S_TOP_A9_PERIPH		3
+#define ZX279128S_TOP_MATRIX_ACLK	4
+#define ZX279128S_TOP_MATRIX_HCLK	5
+#define ZX279128S_TOP_MATRIX_PCLK	6
+#define ZX279128S_TOP_LSP0_PCLK		7
+#define ZX279128S_TOP_LSP0_25M		8
+#define ZX279128S_TOP_LSP0_32K		9
+#define ZX279128S_TOP_LSP0_100M		10
+#define ZX279128S_TOP_LSP1_PCLK		11
+#define ZX279128S_TOP_LSP1_ACLK		12
+#define ZX279128S_TOP_LSP1_25M		13
+#define ZX279128S_TOP_LSP1_49M		14
+#define ZX279128S_TOP_LSP1_100M		15
+#define ZX279128S_TOP_IRAM_ACLK		16
+#define ZX279128S_TOP_IROM_ACLK		17
+#define ZX279128S_TOP_SYS_CTRL_PCLK	18
+#define ZX279128S_TOP_USB_ACLK		19
+#define ZX279128S_TOP_USB_SUSPEND	20
+#define ZX279128S_TOP_USB_REF		21
+
+
+/* Low speed peripherals 0 */
+#define ZX279128S_LSP0_UART0_WCLK	0
+#define ZX279128S_LSP0_UART0_PCLK	1
+#define ZX279128S_LSP0_UART1_WCLK	2
+#define ZX279128S_LSP0_UART1_PCLK	3
+#define ZX279128S_LSP0_SPI_WCLK		4
+#define ZX279128S_LSP0_SPI_PCLK		5
+#define ZX279128S_LSP0_GPIO_PCLK	6
+
+/* Low speed peripherals 1 */
+#define ZX279128S_LSP1_MDIO_WCLK	0
+#define ZX279128S_LSP1_MDIO_PCLK	1
+
+/* Top CRM 2 */
+#define ZX279128S_CRM2_PCIE0_CLK	0
+#define ZX279128S_CRM2_PCIE1_CLK	1
+
+#endif
-- 
2.55.0

Re: [PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[Stefan Dösinger]

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

Am Mittwoch, 14. Oktober 2026, 09:20:44 Mitteleuropäische Sommerzeit schrieb 
Navid Ghahremani:
> Add Device Tree binding schemas for the ZTE ZX279128S clock controllers:
> topcrm, lspcrpm, and crm2. Define fixed clock lists and simple-mfd
> for child nodes inside topcrm.

This would be easier to review if it were split in 3 patches, one yaml per 
patch. I'll try to comment on the details anyway.

> +properties:
> +  compatible:
> +    oneOf:
> +      - const: zte,zx279128s-crm2
> +      - items:
> +          - const: zte,zx279128s-crm2
> +          - const: syscon

Afaiu this makes "syscon" optional. Why do you need syscon? Why might you not 
need syscon?

The goal should be to avoid "syscon" as much as possible. The meaning of the 
syscon compatible is that a different driver will access these registers 
directly instead of going through a properly exported interface like #clock-
cells. Is there anything in crm2 that is not a clock or reset?

> a/Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml
> b/Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml new

> +maintainers:
> +  - Stefan Dösinger <stefandoesinger@gmail.com>
> +  - Navid Ghahremani <ghahramani.navid@gmail.com>

I think you should be the maintainer alone here, I don't have the hardware to 
do proper testing of patches.

> +description: |
> +  Each of the two groups of low speed peripherals (LSP) has a peripheral
> clock +  module (LSP CRPM) of its own, fed by the top CRM.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - zte,zx279128s-lsp0crpm
> +      - zte,zx279128s-lsp1crpm

I think splitting those into different bindings is still a viable choice. Yeah, 
ZTE gave them very similar names, but from what I can see the two controllers 
have nothing in common.

> +patternProperties:
> +  "^phy@[01]$":
> +    type: object
> +    description: PCIe PHY child nodes

You can drop the phy child node and just add "#phy-cells=1". See https://
docs.kernel.org/devicetree/bindings/writing-bindings.html : "DON’T create 
nodes just for the sake of instantiating drivers. Multi-function devices only 
need child nodes when the child nodes have their own DT resources. A single 
node can be multiple providers (e.g. clocks and resets)."

In practical terms, putting a child node into the DT makes this a public ABI 
contract. The alternative is to split the device with an MFD driver or using 
the aux bus. In the latter case, what components a device consists of is a 
driver implementation detail that can easily be changed later.

The PHY subsystem has a quirk that devm_of_phy_provider_register wants the 
device node to have a DT node, which is not the case if the device is an MFD 
child. device_set_of_node_from_dev(dev, dev->parent); helps.

The other purpose of having a DT child node is if a sub-component is likely to 
be used in a different piece of hardware. syscon-reboot is a pretty generic 
example of that.

> --- /dev/null
> +++ b/include/dt-bindings/clock/zte,zx279128s-crm.h

I'd suggest to rename clock/zte,zx279128s-crm.h to clock/zte,zx279128s-clk.h. 
"CRM" implies clock-reset-manager, but the -reset.h header has the reset 
handles, so the "crm.h" header won't have them.

> +#define ZX279128S_TOP_PLL_A9		0
> +#define ZX279128S_TOP_PLL_LSP		1
> +#define ZX279128S_TOP_CPU		2

Do they all need to be exported? You only need those that will be used for a 
clock = <&topcrm ZX279128S_TOP_SOMETHING> handle. That could e.g. be the CPU 
wclk and pclk, but the parent PLL itself can remain a driver detail. Most 
likely all your device clocks have a gate somewhere. In my zx29 case I only 
export gates from the clock driver, with the exception of the Cortex M0 mux - 
the Cortex M0 can't be turned off, so it has no gate.

I realize that a lot of devices may be missing from the DTSI in patch 23.

> +/* Low speed peripherals 0 */
> +#define ZX279128S_LSP0_UART0_WCLK	0
> +#define ZX279128S_LSP0_UART0_PCLK	1
> +#define ZX279128S_LSP0_UART1_WCLK	2
> +#define ZX279128S_LSP0_UART1_PCLK	3
> +#define ZX279128S_LSP0_SPI_WCLK		4
> +#define ZX279128S_LSP0_SPI_PCLK		5
> +#define ZX279128S_LSP0_GPIO_PCLK	6

E.g. exporting those intuitively makes sense.

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

Re: [PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: missing reset header include breaks dt_binding_check
- [Low] dt-bindings: topcrm PHY child node lacks $ref or compatible constraint
--

commit 2a48341d23f9ed02596c916c05af28f44dc6b0bb
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

Add Device Tree binding schemas for the ZTE ZX279128S clock controllers:
topcrm, lspcrpm, and crm2. Define fixed clock lists and simple-mfd
for child nodes inside topcrm.

> diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
> new file mode 100644
> index 0000000000000..9466661929f03
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
[ ... ]
> +patternProperties:
> +  "^phy@[01]$":
> +    type: object
> +    description: PCIe PHY child nodes

[Severity: Low]
Does this node definition allow malformed PHY child nodes to silently pass
validation?

Because type: object imposes no constraints by default and there is no
$ref: /schemas/phy/phy-provider.yaml# (nor a requirement for a compatible
string), dt-schema will not apply the necessary structural checks to the
child node.

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/clock/zte,zx279128s-crm.h>
> +    #include <dt-bindings/reset/zte,zx279128s-reset.h>

[Severity: Low]
Does this missing header break dt_binding_check?

The missing header <dt-bindings/reset/zte,zx279128s-reset.h> is introduced in
the very next patch of this series.

Any developer running schema checks on this exact commit will hit a 'No such
file or directory' error, which breaks git bisect for anyone validating
schemas.

Each commit in a series must build independently without introducing regressions
to preserve bisectability.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014072109.1759502-1-ghahramani.navid@gmail.com?part=4

Re: [PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[Krzysztof Kozlowski]

On Wed, Oct 14, 2026 at 08:20:44AM +0100, Navid Ghahremani wrote:
> Add Device Tree binding schemas for the ZTE ZX279128S clock controllers:
> topcrm, lspcrpm, and crm2. Define fixed clock lists and simple-mfd
> for child nodes inside topcrm.
> 
> Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
> ---
>  .../bindings/clock/zte,zx279128s-crm2.yaml    |  48 +++++++++
>  .../bindings/clock/zte,zx279128s-lspcrpm.yaml | 101 ++++++++++++++++++
>  .../bindings/clock/zte,zx279128s-topcrm.yaml  |  86 +++++++++++++++
>  include/dt-bindings/clock/zte,zx279128s-crm.h |  51 +++++++++
>  4 files changed, 286 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-lspcrpm.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
>  create mode 100644 include/dt-bindings/clock/zte,zx279128s-crm.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
> new file mode 100644
> index 0000000000..aa90ad15d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm2.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/zte,zx279128s-crm2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ZTE ZX279128S CRM2 clock and reset controller
> +
> +maintainers:
> +  - Navid Ghahremani <ghahramani.navid@gmail.com>
> +
> +description:
> +  Top CRM 2 (clock and reset module 2) provides clocks and resets for
> +  PCIe controllers on the ZTE ZX279128S SoC.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: zte,zx279128s-crm2
> +      - items:
> +          - const: zte,zx279128s-crm2
> +          - const: syscon

This is odd. Device is specific, fixed. It either is or is not
compatible, not both.

> +
> +  reg:
> +    maxItems: 1
> +
> +  "#clock-cells":
> +    const: 1

...

> diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
> new file mode 100644
> index 0000000000..9466661929
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-topcrm.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/zte,zx279128s-topcrm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ZTE ZX279128S Top Clock and Reset Module
> +
> +maintainers:
> +  - Stefan Dösinger <stefandoesinger@gmail.com>
> +  - Navid Ghahremani <ghahramani.navid@gmail.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The top clock and reset module (CRM) of the ZTE ZX279128S holds the PLLs,
> +  bus clocks, peripheral clocks and the reset controller.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: zte,zx279128s-topcrm
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: 25 MHz crystal oscillator
> +
> +  clock-names:
> +    items:
> +      - const: osc
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  reboot:
> +    type: object
> +    $ref: /schemas/power/reset/syscon-reboot.yaml#
> +    description: System reboot controller child node
> +
> +patternProperties:
> +  "^phy@[01]$":

phy-0
phy-1

because your reboot node does not have unit address.

> +    type: object
> +    description: PCIe PHY child nodes

Missing description, compatibles with additionalProperties: true or
full schema with 'false'

So the PHY does not need any resources from the parent? It gets its own
clocks? See explanation of simple-mfd.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/zte,zx279128s-crm.h>
> +    #include <dt-bindings/reset/zte,zx279128s-reset.h>
> +
> +    topcrm: clock-controller@94000000 {
> +        compatible = "zte,zx279128s-topcrm", "syscon", "simple-mfd";
> +        reg = <0x94000000 0x1000>;
> +        clocks = <&osc>;
> +        clock-names = "osc";
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        reboot {
> +            compatible = "syscon-reboot";
> +            offset = <0x44>;
> +            mask = <0x1>;
> +            value = <0x1>;

Incomplete example.


Best regards,
Krzysztof

Re: [PATCH v2 04/24] dt-bindings: clock: Add ZTE ZX279128S CRM clock bindings

[Krzysztof Kozlowski]

On Wed, Oct 14, 2026 at 08:20:44AM +0100, Navid Ghahremani wrote:
> Add Device Tree binding schemas for the ZTE ZX279128S clock controllers:
> topcrm, lspcrpm, and crm2. Define fixed clock lists and simple-mfd
> for child nodes inside topcrm.
> 
> Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
> ---

Also:

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23

Best regards,
Krzysztof

[PATCH v2 06/24] clk: zte: Add ZX279128S CRM clock and reset driver

[Navid Ghahremani]

Support the top CRM and low-speed peripheral clock controllers of the
ZTE ZX279128S SoC. Implement custom reset controller ops mapping
logical reset IDs to internal hardware register offsets, and provide
CRM2 clock gating and reset controller support for PCIe.

Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
---
 drivers/clk/Kconfig             |   1 +
 drivers/clk/Makefile            |   1 +
 drivers/clk/zte/Kconfig         |  14 +
 drivers/clk/zte/Makefile        |   2 +
 drivers/clk/zte/clk-zx279128s.c | 758 ++++++++++++++++++++++++++++++++
 5 files changed, 776 insertions(+)
 create mode 100644 drivers/clk/zte/Kconfig
 create mode 100644 drivers/clk/zte/Makefile
 create mode 100644 drivers/clk/zte/clk-zx279128s.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index f9592fd9ec..5200de26c0 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -547,6 +547,7 @@ source "drivers/clk/uniphier/Kconfig"
 source "drivers/clk/visconti/Kconfig"
 source "drivers/clk/x86/Kconfig"
 source "drivers/clk/xilinx/Kconfig"
+source "drivers/clk/zte/Kconfig"
 source "drivers/clk/zynqmp/Kconfig"
 
 # Kunit test cases
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b18af485d7..07393019c3 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -181,5 +181,6 @@ ifeq ($(CONFIG_COMMON_CLK), y)
 obj-$(CONFIG_X86)			+= x86/
 endif
 obj-y					+= xilinx/
+obj-y					+= 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 0000000000..92fd27ea42
--- /dev/null
+++ b/drivers/clk/zte/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config CLK_ZX279128S
+	bool "ZTE zx279128s clock driver"
+	depends on SOC_ZX279128S || COMPILE_TEST
+	depends on OF
+	default SOC_ZX279128S
+	select RESET_CONTROLLER
+	select RESET_SIMPLE
+	help
+	  This driver supports the clocks of the ZTE zx279128s SoC: the
+	  top clock and reset module (CRM) with the PLLs, the CPU, bus and
+	  peripheral group clocks, and the CRPMs of the two groups of low
+	  speed peripherals (UARTs, SPI flash controller, GPIO, MDIO).
+	  Say Y if you build a kernel for a board with this SoC.
diff --git a/drivers/clk/zte/Makefile b/drivers/clk/zte/Makefile
new file mode 100644
index 0000000000..a55ffbfaf1
--- /dev/null
+++ b/drivers/clk/zte/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_CLK_ZX279128S)	+= clk-zx279128s.o
diff --git a/drivers/clk/zte/clk-zx279128s.c b/drivers/clk/zte/clk-zx279128s.c
new file mode 100644
index 0000000000..892e0c22e5
--- /dev/null
+++ b/drivers/clk/zte/clk-zx279128s.c
@@ -0,0 +1,758 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ZTE zx279128s clocks: the top CRM and the CRPMs of the two groups of low
+ * speed peripherals
+ *
+ * Copyright (C) 2026 Navid Ghahremani <ghahramani.navid@gmail.com>
+ *
+ * The boot loader sets up the PLLs and the CPU and bus clocks, and the driver
+ * leaves them as they are: it reads them once and registers their rates. The
+ * gates, and the muxes and dividers of the peripherals, are under its
+ * control.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/reset-controller.h>
+
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/zte,zx279128s-crm.h>
+#include <dt-bindings/reset/zte,zx279128s-reset.h>
+
+/* Top CRM */
+#define TOP_CLK_SEL		0x0c
+#define  TOP_MATRIX_SEL_SHIFT	15
+#define  TOP_CPU_SEL_SHIFT	13
+#define TOP_CLK_EN		0x14
+#define TOP_PLL_A9		0x18
+#define TOP_PLL_AUDIO		0x28
+#define TOP_USB_EN		0x48
+
+#define TOP_NUM_CLKS		(ZX279128S_TOP_USB_REF + 1)
+
+struct zx_reset_line {
+	u16 reg;
+	u8 bit;
+};
+
+static const struct zx_reset_line zx279128s_top_resets[] = {
+	[ZX279128S_RESET_USB_B9]  = { 0x4c, 9 },
+	[ZX279128S_RESET_USB_B10] = { 0x4c, 10 },
+	[ZX279128S_RESET_USB_B11] = { 0x4c, 11 },
+	[ZX279128S_RESET_USB_B14] = { 0x4c, 14 },
+};
+
+static DEFINE_SPINLOCK(zx_top_lock);
+
+struct zx_reset_data {
+	struct reset_controller_dev rcdev;
+	void __iomem *base;
+	spinlock_t *lock;
+};
+
+static inline struct zx_reset_data *to_zx_reset_data(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct zx_reset_data, rcdev);
+}
+
+static int zx_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct zx_reset_data *data = to_zx_reset_data(rcdev);
+	const struct zx_reset_line *line;
+	unsigned long flags;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx279128s_top_resets))
+		return -EINVAL;
+
+	line = &zx279128s_top_resets[id];
+	spin_lock_irqsave(data->lock, flags);
+	val = readl(data->base + line->reg);
+	val &= ~BIT(line->bit);
+	writel(val, data->base + line->reg);
+	spin_unlock_irqrestore(data->lock, flags);
+	return 0;
+}
+
+static int zx_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct zx_reset_data *data = to_zx_reset_data(rcdev);
+	const struct zx_reset_line *line;
+	unsigned long flags;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx279128s_top_resets))
+		return -EINVAL;
+
+	line = &zx279128s_top_resets[id];
+	spin_lock_irqsave(data->lock, flags);
+	val = readl(data->base + line->reg);
+	val |= BIT(line->bit);
+	writel(val, data->base + line->reg);
+	spin_unlock_irqrestore(data->lock, flags);
+	return 0;
+}
+
+static int zx_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct zx_reset_data *data = to_zx_reset_data(rcdev);
+	const struct zx_reset_line *line;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx279128s_top_resets))
+		return -EINVAL;
+
+	line = &zx279128s_top_resets[id];
+	val = readl(data->base + line->reg);
+	return !(val & BIT(line->bit));
+}
+
+static const struct reset_control_ops zx_reset_ops = {
+	.assert = zx_reset_assert,
+	.deassert = zx_reset_deassert,
+	.status = zx_reset_status,
+};
+
+/*
+ * Reset bits are cleared to hold the reset. The registers also hold other
+ * controls, but no other driver writes the reset registers.
+ */
+static int __init zx_top_reset_init(struct device_node *np,
+				    void __iomem *base)
+{
+	struct zx_reset_data *rst;
+	int ret;
+
+	rst = kzalloc_obj(*rst, GFP_KERNEL);
+	if (!rst)
+		return -ENOMEM;
+
+	rst->lock = &zx_top_lock;
+	rst->base = base;
+	rst->rcdev.ops = &zx_reset_ops;
+	rst->rcdev.owner = THIS_MODULE;
+	rst->rcdev.nr_resets = ARRAY_SIZE(zx279128s_top_resets);
+	rst->rcdev.of_node = np;
+
+	ret = reset_controller_register(&rst->rcdev);
+	if (ret)
+		kfree(rst);
+
+	return ret;
+}
+
+/*
+ * A PLL has two registers. The output is
+ * osc * (fbdiv + frac / 2^24) / pd1 / pd2.
+ */
+#define PLL_FBDIV		GENMASK(17, 6)
+#define PLL_PD1			GENMASK(5, 3)
+#define PLL_PD2			GENMASK(2, 0)
+#define PLL_FRAC		GENMASK(23, 0)	/* second register */
+#define PLL_FRAC_BITS		24
+
+/* The LSP CRPMs: one register per peripheral */
+#define LSP_PCLK_EN		0	/* bit of the register clock gate */
+#define LSP_WCLK_EN		1	/* bit of the work clock gate */
+#define LSP_WCLK_SEL_SHIFT	9
+#define LSP_WCLK_DIV_SHIFT	11
+
+/*
+ * Register a PLL as a fixed factor of the crystal, from the setting the boot
+ * loader left in it.
+ */
+static struct clk_hw * __init zx_top_pll(struct device_node *np,
+					 const char *name, void __iomem *reg)
+{
+	u32 cfg0 = readl(reg), cfg1 = readl(reg + 4);
+	unsigned int pd = FIELD_GET(PLL_PD1, cfg0) * FIELD_GET(PLL_PD2, cfg0);
+	u64 mult = ((u64)FIELD_GET(PLL_FBDIV, cfg0) << PLL_FRAC_BITS) |
+		   FIELD_GET(PLL_FRAC, cfg1);
+	u64 div = (u64)pd << PLL_FRAC_BITS;
+	unsigned int shift;
+
+	if (!mult || !pd)
+		return ERR_PTR(-EINVAL);
+
+	/* Fit both into 32 bits: drop trailing zeroes, then fraction bits */
+	shift = min(__ffs64(mult), PLL_FRAC_BITS);
+	mult >>= shift;
+	div >>= shift;
+	while (mult > UINT_MAX) {
+		mult >>= 1;
+		div >>= 1;
+	}
+
+	return clk_hw_register_fixed_factor_fwname(NULL, np, name, "osc", 0,
+						   mult, div);
+}
+
+static struct clk_hw * __init zx_top_gate(void __iomem *reg,
+					  const char *name,
+					  const struct clk_hw *parent, u8 bit,
+					  unsigned long flags)
+{
+	return clk_hw_register_gate_parent_hw(NULL, name, parent, flags, reg,
+					      bit, 0, &zx_top_lock);
+}
+
+
+
+static void __init zx279128s_topcrm_init(struct device_node *np)
+{
+	const struct clk_hw *cpu_parents[4], *matrix_parents[4];
+	struct clk_hw *osc, *lsp, *lsp_500m, *lsp_250m, *lsp_125m, *lsp_100m;
+	struct clk_hw *audio, *audio_32k, *matrix, *hclk, *pclk, *m200, *m20;
+	struct clk_hw_onecell_data *data;
+	struct clk_hw **hws;
+	void __iomem *base, *en;
+	int i;
+
+	base = of_iomap(np, 0);
+	if (!base) {
+		pr_err("%pOF: no registers\n", np);
+		return;
+	}
+
+	data = kzalloc(struct_size(data, hws, TOP_NUM_CLKS), GFP_KERNEL);
+	if (!data)
+		return;
+	data->num = TOP_NUM_CLKS;
+	hws = data->hws;
+
+	hws[ZX279128S_TOP_PLL_A9] = zx_top_pll(np, "pll_a9", base + TOP_PLL_A9);
+	if (IS_ERR(hws[ZX279128S_TOP_PLL_A9]))
+		goto err;
+	osc = clk_hw_get_parent(hws[ZX279128S_TOP_PLL_A9]);
+	if (!osc)
+		goto err;
+
+	/*
+	 * The formula above gives 200 MHz for the LSP PLL, but the vendor
+	 * kernel has it at 1 GHz, with fixed taps for the bus (250 MHz) and
+	 * the peripherals (100 MHz). Until that is measured, it is registered
+	 * with the vendor's rate.
+	 */
+	lsp = clk_hw_register_fixed_factor_parent_hw(NULL, "pll_lsp", osc, 0,
+						     40, 1);
+	if (IS_ERR(lsp))
+		goto err;
+	hws[ZX279128S_TOP_PLL_LSP] = lsp;
+	lsp_500m = clk_hw_register_fixed_factor_parent_hw(NULL, "lsp_500m", lsp,
+							  0, 1, 2);
+	lsp_250m = clk_hw_register_fixed_factor_parent_hw(NULL, "lsp_250m", lsp,
+							  0, 1, 4);
+	lsp_125m = clk_hw_register_fixed_factor_parent_hw(NULL, "lsp_125m", lsp,
+							  0, 1, 8);
+	lsp_100m = clk_hw_register_fixed_factor_parent_hw(NULL, "lsp_100m", lsp,
+							  0, 1, 10);
+	if (IS_ERR(lsp_500m) || IS_ERR(lsp_250m) || IS_ERR(lsp_125m) ||
+	    IS_ERR(lsp_100m))
+		goto err;
+
+	/* 49.152 MHz, and the 32.768 kHz derived from it */
+	audio = zx_top_pll(np, "pll_audio", base + TOP_PLL_AUDIO);
+	if (IS_ERR(audio))
+		goto err;
+	audio_32k = clk_hw_register_fixed_factor_parent_hw(NULL, "audio_32k",
+							   audio, 0, 1, 1500);
+	if (IS_ERR(audio_32k))
+		goto err;
+
+	cpu_parents[0] = osc;
+	cpu_parents[1] = lsp_250m;
+	cpu_parents[2] = lsp_500m;
+	cpu_parents[3] = hws[ZX279128S_TOP_PLL_A9];
+	hws[ZX279128S_TOP_CPU] =
+		clk_hw_register_mux_hws(NULL, "cpu", cpu_parents, 4, 0,
+					base + TOP_CLK_SEL, TOP_CPU_SEL_SHIFT,
+					2, CLK_MUX_READ_ONLY, &zx_top_lock);
+	if (IS_ERR(hws[ZX279128S_TOP_CPU]))
+		goto err;
+	hws[ZX279128S_TOP_A9_PERIPH] =
+		clk_hw_register_fixed_factor_parent_hw(NULL, "a9_periph",
+						       hws[ZX279128S_TOP_CPU],
+						       0, 1, 2);
+
+	/*
+	 * A 200 MHz clock of unknown source; the vendor kernel declares it at
+	 * this rate. It feeds the bus select and, divided, the USB reference.
+	 */
+	m200 = clk_hw_register_fixed_rate(NULL, "clk_200m", NULL, 0,
+					  200000000);
+	if (IS_ERR(m200))
+		goto err;
+	m20 = clk_hw_register_fixed_factor_parent_hw(NULL, "clk_20m", m200, 0,
+						     1, 10);
+	if (IS_ERR(m20))
+		goto err;
+
+	matrix_parents[0] = osc;
+	matrix_parents[1] = m200;
+	matrix_parents[2] = lsp_250m;
+	matrix_parents[3] = lsp_125m;
+	matrix = clk_hw_register_mux_hws(NULL, "matrix_aclk", matrix_parents,
+					 4, 0, base + TOP_CLK_SEL,
+					 TOP_MATRIX_SEL_SHIFT, 2,
+					 CLK_MUX_READ_ONLY, &zx_top_lock);
+	if (IS_ERR(matrix))
+		goto err;
+	hws[ZX279128S_TOP_MATRIX_ACLK] = matrix;
+	hclk = clk_hw_register_fixed_factor_parent_hw(NULL, "matrix_hclk",
+						      matrix, 0, 1, 2);
+	hws[ZX279128S_TOP_MATRIX_HCLK] = hclk;
+	pclk = clk_hw_register_fixed_factor_parent_hw(NULL, "matrix_pclk",
+						      matrix, 0, 1, 2);
+	if (IS_ERR(pclk))
+		goto err;
+	hws[ZX279128S_TOP_MATRIX_PCLK] = pclk;
+
+	en = base + TOP_CLK_EN;
+	hws[ZX279128S_TOP_LSP0_100M] = zx_top_gate(en, "lsp0_100m", lsp_100m,
+						   13, 0);
+	hws[ZX279128S_TOP_LSP0_32K] = zx_top_gate(en, "lsp0_32k", audio_32k,
+						  12, 0);
+	hws[ZX279128S_TOP_LSP0_PCLK] = zx_top_gate(en, "lsp0_pclk", pclk, 11, 0);
+	hws[ZX279128S_TOP_LSP0_25M] = zx_top_gate(en, "lsp0_25m", osc, 9, 0);
+	hws[ZX279128S_TOP_LSP1_ACLK] = zx_top_gate(en, "lsp1_aclk", matrix, 8, 0);
+	hws[ZX279128S_TOP_LSP1_100M] = zx_top_gate(en, "lsp1_100m", lsp_100m,
+						   7, 0);
+	hws[ZX279128S_TOP_LSP1_49M] = zx_top_gate(en, "lsp1_49m", audio, 6, 0);
+	hws[ZX279128S_TOP_LSP1_PCLK] = zx_top_gate(en, "lsp1_pclk", pclk, 5, 0);
+	hws[ZX279128S_TOP_LSP1_25M] = zx_top_gate(en, "lsp1_25m", osc, 4, 0);
+	/* the parked second CPU waits in the IRAM */
+	hws[ZX279128S_TOP_IRAM_ACLK] = zx_top_gate(en, "iram_aclk", matrix, 2,
+						   CLK_IS_CRITICAL);
+	hws[ZX279128S_TOP_IROM_ACLK] = zx_top_gate(en, "irom_aclk", matrix, 1,
+						   CLK_IS_CRITICAL);
+	hws[ZX279128S_TOP_SYS_CTRL_PCLK] = zx_top_gate(en, "sys_ctrl_pclk", pclk,
+						       0, CLK_IS_CRITICAL);
+
+	/* The USB 3.0 controller's bus, suspend and reference clocks */
+	en = base + TOP_USB_EN;
+	hws[ZX279128S_TOP_USB_ACLK] = zx_top_gate(en, "usb_aclk", matrix, 21, 0);
+	hws[ZX279128S_TOP_USB_SUSPEND] = zx_top_gate(en, "usb_suspend",
+						     audio_32k, 20, 0);
+	hws[ZX279128S_TOP_USB_REF] = zx_top_gate(en, "usb_ref", m20, 19, 0);
+
+	for (i = 0; i < TOP_NUM_CLKS; i++)
+		if (IS_ERR_OR_NULL(hws[i]))
+			goto err;
+
+	if (of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data))
+		goto err;
+
+	if (zx_top_reset_init(np, base))
+		pr_err("%pOF: failed to register the resets\n", np);
+	return;
+
+err:
+	/* Without its timer and bus clocks the system won't boot anyway */
+	pr_err("%pOF: failed to register the clocks\n", np);
+}
+
+CLK_OF_DECLARE_DRIVER(zx279128s_topcrm, "zte,zx279128s-topcrm",
+	       zx279128s_topcrm_init);
+
+/*
+ * A peripheral of an LSP group: its work clock, optionally behind a mux of
+ * two inputs or a divider, and its register clock.
+ */
+struct zx_lsp_periph {
+	const char *name;
+	unsigned int reg;
+	int wclk;			/* index of the work clock, or -1 */
+	int pclk;			/* index of the register clock */
+	const char *parents[2];		/* the mux inputs, or one parent */
+	u8 div_width;			/* 0: no divider */
+};
+
+struct zx_lsp_data {
+	const struct zx_lsp_periph *periphs;
+	unsigned int num_periphs;
+	unsigned int num_clks;
+	bool aclk;			/* the group has an AXI clock */
+};
+
+/* The work clock: an optional mux or divider, then the gate */
+static struct clk_hw *zx_lsp_wclk(struct device *dev, void __iomem *reg,
+				  spinlock_t *lock,
+				  const struct zx_lsp_periph *p)
+{
+	struct clk_parent_data pd[2] = {
+		{ .fw_name = p->parents[0] },
+		{ .fw_name = p->parents[1] },
+	};
+	struct clk_hw *src = NULL;
+	const char *name;
+
+	if (p->parents[1]) {
+		name = devm_kasprintf(dev, GFP_KERNEL, "%s_wsel", p->name);
+		if (!name)
+			return ERR_PTR(-ENOMEM);
+		src = devm_clk_hw_register_mux_parent_data_table(dev, name, pd,
+								 2, 0, reg,
+								 LSP_WCLK_SEL_SHIFT,
+								 1, 0, NULL,
+								 lock);
+	} else if (p->div_width) {
+		name = devm_kasprintf(dev, GFP_KERNEL, "%s_wdiv", p->name);
+		if (!name)
+			return ERR_PTR(-ENOMEM);
+		src = __devm_clk_hw_register_divider(dev, NULL, name, NULL,
+						     NULL, pd, 0, reg,
+						     LSP_WCLK_DIV_SHIFT,
+						     p->div_width, 0, NULL,
+						     lock);
+	}
+	if (IS_ERR(src))
+		return src;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s_wclk", p->name);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+	if (src)
+		return devm_clk_hw_register_gate_parent_hw(dev, name, src,
+							   CLK_SET_RATE_PARENT,
+							   reg, LSP_WCLK_EN, 0,
+							   lock);
+	return devm_clk_hw_register_gate_parent_data(dev, name, pd, 0, reg,
+						     LSP_WCLK_EN, 0, lock);
+}
+
+/* The register clock */
+static struct clk_hw *zx_lsp_pclk(struct device *dev, void __iomem *reg,
+				  spinlock_t *lock,
+				  const struct zx_lsp_periph *p)
+{
+	struct clk_parent_data pclk = { .fw_name = "pclk" };
+	const char *name;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s_pclk", p->name);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+	return devm_clk_hw_register_gate_parent_data(dev, name, &pclk, 0, reg,
+						     LSP_PCLK_EN, 0, lock);
+}
+
+static int zx279128s_lsp_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct zx_lsp_data *match = device_get_match_data(dev);
+	struct clk_hw_onecell_data *data;
+	void __iomem *base;
+	spinlock_t *lock;	/* serializes the group's clock registers */
+	struct clk *clk;
+	unsigned int i;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* The group's own registers need these */
+	clk = devm_clk_get_enabled(dev, "pclk");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "no register clock\n");
+	if (match->aclk) {
+		clk = devm_clk_get_enabled(dev, "aclk");
+		if (IS_ERR(clk))
+			return dev_err_probe(dev, PTR_ERR(clk),
+					     "no AXI clock\n");
+	}
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	data = devm_kzalloc(dev, struct_size(data, hws, match->num_clks),
+			    GFP_KERNEL);
+	if (!lock || !data)
+		return -ENOMEM;
+	spin_lock_init(lock);
+	data->num = match->num_clks;
+
+	for (i = 0; i < match->num_periphs; i++) {
+		const struct zx_lsp_periph *p = &match->periphs[i];
+		void __iomem *reg = base + p->reg;
+		struct clk_hw *hw;
+
+		if (p->wclk >= 0) {
+			hw = zx_lsp_wclk(dev, reg, lock, p);
+			if (IS_ERR(hw))
+				return dev_err_probe(dev, PTR_ERR(hw),
+						     "failed to register %s\n",
+						     p->name);
+			data->hws[p->wclk] = hw;
+		}
+
+		hw = zx_lsp_pclk(dev, reg, lock, p);
+		if (IS_ERR(hw))
+			return dev_err_probe(dev, PTR_ERR(hw),
+					     "failed to register %s\n", p->name);
+		data->hws[p->pclk] = hw;
+	}
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, data);
+}
+
+static const struct zx_lsp_periph zx_lsp0_periphs[] = {
+	{ "uart0", 0x10, ZX279128S_LSP0_UART0_WCLK, ZX279128S_LSP0_UART0_PCLK,
+	  { "wclk25m", "wclk100m" } },
+	{ "uart1", 0x14, ZX279128S_LSP0_UART1_WCLK, ZX279128S_LSP0_UART1_PCLK,
+	  { "wclk25m", "wclk100m" } },
+	{ "spi", 0x18, ZX279128S_LSP0_SPI_WCLK, ZX279128S_LSP0_SPI_PCLK,
+	  { "wclk100m" }, 6 },
+	{ "gpio", 0x1c, -1, ZX279128S_LSP0_GPIO_PCLK },
+};
+
+static const struct zx_lsp_data zx_lsp0_data = {
+	.periphs = zx_lsp0_periphs,
+	.num_periphs = ARRAY_SIZE(zx_lsp0_periphs),
+	.num_clks = ZX279128S_LSP0_GPIO_PCLK + 1,
+};
+
+static const struct zx_lsp_periph zx_lsp1_periphs[] = {
+	{ "mdio", 0x04, ZX279128S_LSP1_MDIO_WCLK, ZX279128S_LSP1_MDIO_PCLK,
+	  { "wclk100m" }, 7 },
+};
+
+static const struct zx_lsp_data zx_lsp1_data = {
+	.periphs = zx_lsp1_periphs,
+	.num_periphs = ARRAY_SIZE(zx_lsp1_periphs),
+	.num_clks = ZX279128S_LSP1_MDIO_PCLK + 1,
+	.aclk = true,
+};
+
+static const struct of_device_id zx279128s_lsp_of_match[] = {
+	{ .compatible = "zte,zx279128s-lsp0crpm", .data = &zx_lsp0_data },
+	{ .compatible = "zte,zx279128s-lsp1crpm", .data = &zx_lsp1_data },
+	{ }
+};
+
+static struct platform_driver zx279128s_lsp_driver = {
+	.probe = zx279128s_lsp_probe,
+	.driver = {
+		.name = "zx279128s-lspcrpm",
+		.of_match_table = zx279128s_lsp_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(zx279128s_lsp_driver);
+
+/* Top CRM 2 */
+#define CRM2_CLK		0x070
+#define CRM2_RST		0x074
+
+struct zx_crm2_gate {
+	struct clk_hw hw;
+	void __iomem *reg;
+	u32 clk_on;
+	u32 clk_off;
+	spinlock_t *lock;
+};
+
+static inline struct zx_crm2_gate *to_zx_crm2_gate(struct clk_hw *hw)
+{
+	return container_of(hw, struct zx_crm2_gate, hw);
+}
+
+static int zx_crm2_clk_enable(struct clk_hw *hw)
+{
+	struct zx_crm2_gate *gate = to_zx_crm2_gate(hw);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(gate->lock, flags);
+	val = readl(gate->reg);
+	val |= gate->clk_on;
+	val &= ~gate->clk_off;
+	writel(val, gate->reg);
+	spin_unlock_irqrestore(gate->lock, flags);
+	return 0;
+}
+
+static void zx_crm2_clk_disable(struct clk_hw *hw)
+{
+	struct zx_crm2_gate *gate = to_zx_crm2_gate(hw);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(gate->lock, flags);
+	val = readl(gate->reg);
+	val &= ~gate->clk_on;
+	writel(val, gate->reg);
+	spin_unlock_irqrestore(gate->lock, flags);
+}
+
+static int zx_crm2_clk_is_enabled(struct clk_hw *hw)
+{
+	struct zx_crm2_gate *gate = to_zx_crm2_gate(hw);
+	u32 val = readl(gate->reg);
+
+	return (val & gate->clk_on) == gate->clk_on;
+}
+
+static const struct clk_ops zx_crm2_clk_ops = {
+	.enable = zx_crm2_clk_enable,
+	.disable = zx_crm2_clk_disable,
+	.is_enabled = zx_crm2_clk_is_enabled,
+};
+
+struct zx_crm2_reset_data {
+	struct reset_controller_dev rcdev;
+	void __iomem *base;
+	spinlock_t *lock;
+};
+
+static const u8 zx_crm2_reset_bits[] = {
+	[ZX279128S_CRM2_RESET_PCIE0] = 0,
+	[ZX279128S_CRM2_RESET_PCIE1] = 3,
+};
+
+static int zx_crm2_reset_assert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct zx_crm2_reset_data *data =
+		container_of(rcdev, struct zx_crm2_reset_data, rcdev);
+	unsigned long flags;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx_crm2_reset_bits))
+		return -EINVAL;
+
+	spin_lock_irqsave(data->lock, flags);
+	val = readl(data->base + CRM2_RST);
+	val &= ~BIT(zx_crm2_reset_bits[id]);
+	writel(val, data->base + CRM2_RST);
+	spin_unlock_irqrestore(data->lock, flags);
+	return 0;
+}
+
+static int zx_crm2_reset_deassert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct zx_crm2_reset_data *data =
+		container_of(rcdev, struct zx_crm2_reset_data, rcdev);
+	unsigned long flags;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx_crm2_reset_bits))
+		return -EINVAL;
+
+	spin_lock_irqsave(data->lock, flags);
+	val = readl(data->base + CRM2_RST);
+	val |= BIT(zx_crm2_reset_bits[id]);
+	writel(val, data->base + CRM2_RST);
+	spin_unlock_irqrestore(data->lock, flags);
+	return 0;
+}
+
+static int zx_crm2_reset_status(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct zx_crm2_reset_data *data =
+		container_of(rcdev, struct zx_crm2_reset_data, rcdev);
+	u32 val;
+
+	if (id >= ARRAY_SIZE(zx_crm2_reset_bits))
+		return -EINVAL;
+
+	val = readl(data->base + CRM2_RST);
+	return !(val & BIT(zx_crm2_reset_bits[id]));
+}
+
+static const struct reset_control_ops zx_crm2_reset_ops = {
+	.assert = zx_crm2_reset_assert,
+	.deassert = zx_crm2_reset_deassert,
+	.status = zx_crm2_reset_status,
+};
+
+static int zx279128s_crm2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct clk_hw_onecell_data *clk_data;
+	struct zx_crm2_reset_data *rst_data;
+	struct zx_crm2_gate *gates;
+	struct clk_init_data init0 = { };
+	struct clk_init_data init1 = { };
+	void __iomem *base;
+	spinlock_t *lock;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+	spin_lock_init(lock);
+
+	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, 2), GFP_KERNEL);
+	gates = devm_kcalloc(dev, 2, sizeof(*gates), GFP_KERNEL);
+	rst_data = devm_kzalloc(dev, sizeof(*rst_data), GFP_KERNEL);
+	if (!clk_data || !gates || !rst_data)
+		return -ENOMEM;
+
+	clk_data->num = 2;
+
+	/* PCIe0 clock */
+	gates[0].reg = base + CRM2_CLK;
+	gates[0].clk_on = 0xb5;
+	gates[0].clk_off = BIT(8);
+	gates[0].lock = lock;
+	gates[0].hw.init = &init0;
+	init0.name = "crm2_pcie0";
+	init0.ops = &zx_crm2_clk_ops;
+	ret = devm_clk_hw_register(dev, &gates[0].hw);
+	if (ret)
+		return ret;
+	clk_data->hws[ZX279128S_CRM2_PCIE0_CLK] = &gates[0].hw;
+
+	/* PCIe1 clock */
+	gates[1].reg = base + CRM2_CLK;
+	gates[1].clk_on = 0x1aa00;
+	gates[1].clk_off = BIT(17);
+	gates[1].lock = lock;
+	gates[1].hw.init = &init1;
+	init1.name = "crm2_pcie1";
+	init1.ops = &zx_crm2_clk_ops;
+	ret = devm_clk_hw_register(dev, &gates[1].hw);
+	if (ret)
+		return ret;
+	clk_data->hws[ZX279128S_CRM2_PCIE1_CLK] = &gates[1].hw;
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		return ret;
+
+	/* CRM2 Resets */
+	rst_data->base = base;
+	rst_data->lock = lock;
+	rst_data->rcdev.owner = THIS_MODULE;
+	rst_data->rcdev.nr_resets = ARRAY_SIZE(zx_crm2_reset_bits);
+	rst_data->rcdev.ops = &zx_crm2_reset_ops;
+	rst_data->rcdev.of_node = dev->of_node;
+
+	return devm_reset_controller_register(dev, &rst_data->rcdev);
+}
+
+static const struct of_device_id zx279128s_crm2_of_match[] = {
+	{ .compatible = "zte,zx279128s-crm2" },
+	{ }
+};
+
+static struct platform_driver zx279128s_crm2_driver = {
+	.probe = zx279128s_crm2_probe,
+	.driver = {
+		.name = "zx279128s-crm2",
+		.of_match_table = zx279128s_crm2_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(zx279128s_crm2_driver);
-- 
2.55.0

Re: [PATCH v2 06/24] clk: zte: Add ZX279128S CRM clock and reset driver

[Philipp Zabel]

On Mi, 2026-10-14 at 08:20 +0100, Navid Ghahremani wrote:
> Support the top CRM and low-speed peripheral clock controllers of the
> ZTE ZX279128S SoC. Implement custom reset controller ops mapping
> logical reset IDs to internal hardware register offsets, and provide
> CRM2 clock gating and reset controller support for PCIe.
>
> Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>
> ---
>  drivers/clk/Kconfig             |   1 +
>  drivers/clk/Makefile            |   1 +
>  drivers/clk/zte/Kconfig         |  14 +
>  drivers/clk/zte/Makefile        |   2 +
>  drivers/clk/zte/clk-zx279128s.c | 758 ++++++++++++++++++++++++++++++++
>  5 files changed, 776 insertions(+)
>  create mode 100644 drivers/clk/zte/Kconfig
>  create mode 100644 drivers/clk/zte/Makefile
>  create mode 100644 drivers/clk/zte/clk-zx279128s.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index f9592fd9ec..5200de26c0 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -547,6 +547,7 @@ source "drivers/clk/uniphier/Kconfig"
>  source "drivers/clk/visconti/Kconfig"
>  source "drivers/clk/x86/Kconfig"
>  source "drivers/clk/xilinx/Kconfig"
> +source "drivers/clk/zte/Kconfig"
>  source "drivers/clk/zynqmp/Kconfig"
>  
>  # Kunit test cases
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index b18af485d7..07393019c3 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -181,5 +181,6 @@ ifeq ($(CONFIG_COMMON_CLK), y)
>  obj-$(CONFIG_X86)			+= x86/
>  endif
>  obj-y					+= xilinx/
> +obj-y					+= 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 0000000000..92fd27ea42
> --- /dev/null
> +++ b/drivers/clk/zte/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config CLK_ZX279128S
> +	bool "ZTE zx279128s clock driver"
> +	depends on SOC_ZX279128S || COMPILE_TEST
> +	depends on OF
> +	default SOC_ZX279128S
> +	select RESET_CONTROLLER
> +	select RESET_SIMPLE

Don't select RESET_SIMPLE without using it.

> +	help
> +	  This driver supports the clocks of the ZTE zx279128s SoC: the
> +	  top clock and reset module (CRM) with the PLLs, the CPU, bus and
> +	  peripheral group clocks, and the CRPMs of the two groups of low
> +	  speed peripherals (UARTs, SPI flash controller, GPIO, MDIO).
> +	  Say Y if you build a kernel for a board with this SoC.
> diff --git a/drivers/clk/zte/Makefile b/drivers/clk/zte/Makefile
> new file mode 100644
> index 0000000000..a55ffbfaf1
> --- /dev/null
> +++ b/drivers/clk/zte/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_CLK_ZX279128S)	+= clk-zx279128s.o
> diff --git a/drivers/clk/zte/clk-zx279128s.c b/drivers/clk/zte/clk-zx279128s.c
> new file mode 100644
> index 0000000000..892e0c22e5
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx279128s.c
> @@ -0,0 +1,758 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ZTE zx279128s clocks: the top CRM and the CRPMs of the two groups of low
> + * speed peripherals
> + *
> + * Copyright (C) 2026 Navid Ghahremani <ghahramani.navid@gmail.com>
> + *
> + * The boot loader sets up the PLLs and the CPU and bus clocks, and the driver
> + * leaves them as they are: it reads them once and registers their rates. The
> + * gates, and the muxes and dividers of the peripherals, are under its
> + * control.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/reset-controller.h>
> +
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#include <dt-bindings/clock/zte,zx279128s-crm.h>
> +#include <dt-bindings/reset/zte,zx279128s-reset.h>
> +
> +/* Top CRM */
> +#define TOP_CLK_SEL		0x0c
> +#define  TOP_MATRIX_SEL_SHIFT	15
> +#define  TOP_CPU_SEL_SHIFT	13
> +#define TOP_CLK_EN		0x14
> +#define TOP_PLL_A9		0x18
> +#define TOP_PLL_AUDIO		0x28
> +#define TOP_USB_EN		0x48
> +
> +#define TOP_NUM_CLKS		(ZX279128S_TOP_USB_REF + 1)
> +
> +struct zx_reset_line {
> +	u16 reg;
> +	u8 bit;
> +};
> +
> +static const struct zx_reset_line zx279128s_top_resets[] = {
> +	[ZX279128S_RESET_USB_B9]  = { 0x4c, 9 },
> +	[ZX279128S_RESET_USB_B10] = { 0x4c, 10 },
> +	[ZX279128S_RESET_USB_B11] = { 0x4c, 11 },
> +	[ZX279128S_RESET_USB_B14] = { 0x4c, 14 },

Why store reg in zx_reset_line when it's all the same value?

On the other hand, keeping reg and adding a bool active_low field would
allow to share ops between the two reset controllers.

> +};
> +
> +static DEFINE_SPINLOCK(zx_top_lock);

Why share the lock? Each reset controller has its own register.

> +
> +struct zx_reset_data {
> +	struct reset_controller_dev rcdev;
> +	void __iomem *base;
> +	spinlock_t *lock;
> +};
> +
> +static inline struct zx_reset_data *to_zx_reset_data(struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct zx_reset_data, rcdev);
> +}
> +
> +static int zx_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct zx_reset_data *data = to_zx_reset_data(rcdev);
> +	const struct zx_reset_line *line;
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (id >= ARRAY_SIZE(zx279128s_top_resets))
> +		return -EINVAL;

This check is unnecessary, fwnode_reset_simple_xlate() already checked
this before the reset control was created.

> +	line = &zx279128s_top_resets[id];
> +	spin_lock_irqsave(data->lock, flags);
> +	val = readl(data->base + line->reg);
> +	val &= ~BIT(line->bit);
> +	writel(val, data->base + line->reg);
> +	spin_unlock_irqrestore(data->lock, flags);

Consider sharing a read-modify-write helper between implementations.

> +	return 0;
> +}
> +
> +static int zx_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct zx_reset_data *data = to_zx_reset_data(rcdev);
> +	const struct zx_reset_line *line;
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (id >= ARRAY_SIZE(zx279128s_top_resets))
> +		return -EINVAL;
> +
> +	line = &zx279128s_top_resets[id];
> +	spin_lock_irqsave(data->lock, flags);
> +	val = readl(data->base + line->reg);
> +	val |= BIT(line->bit);
> +	writel(val, data->base + line->reg);
> +	spin_unlock_irqrestore(data->lock, flags);
> +	return 0;
> +}
> +
> +static int zx_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct zx_reset_data *data = to_zx_reset_data(rcdev);
> +	const struct zx_reset_line *line;
> +	u32 val;
> +
> +	if (id >= ARRAY_SIZE(zx279128s_top_resets))
> +		return -EINVAL;
> +
> +	line = &zx279128s_top_resets[id];
> +	val = readl(data->base + line->reg);
> +	return !(val & BIT(line->bit));
> +}
> +
> +static const struct reset_control_ops zx_reset_ops = {
> +	.assert = zx_reset_assert,
> +	.deassert = zx_reset_deassert,
> +	.status = zx_reset_status,
> +};
> +
> +/*
> + * Reset bits are cleared to hold the reset. The registers also hold other
> + * controls, but no other driver writes the reset registers.
> + */
> +static int __init zx_top_reset_init(struct device_node *np,
> +				    void __iomem *base)

Why is this initialized from CLK_OF_DECLARE_DRIVER()?
Surely USB resets are not required early?

> +{
> +	struct zx_reset_data *rst;
> +	int ret;
> +
> +	rst = kzalloc_obj(*rst, GFP_KERNEL);
> +	if (!rst)
> +		return -ENOMEM;
> +
> +	rst->lock = &zx_top_lock;
> +	rst->base = base;
> +	rst->rcdev.ops = &zx_reset_ops;
> +	rst->rcdev.owner = THIS_MODULE;
> +	rst->rcdev.nr_resets = ARRAY_SIZE(zx279128s_top_resets);
> +	rst->rcdev.of_node = np;
> +
> +	ret = reset_controller_register(&rst->rcdev);
> +	if (ret)
> +		kfree(rst);
> +
> +	return ret;
> +}
[...]
> +static void __init zx279128s_topcrm_init(struct device_node *np)
> +{
[...]
> +}
> +
> +CLK_OF_DECLARE_DRIVER(zx279128s_topcrm, "zte,zx279128s-topcrm",
> +	       zx279128s_topcrm_init);
[...]
> +struct zx_crm2_reset_data {
> +	struct reset_controller_dev rcdev;
> +	void __iomem *base;
> +	spinlock_t *lock;
> +};

This is exactly the same as zx_reset_data, why duplicate it?
Same comments as above apply here.

regards
Philipp

Re: [PATCH v2 06/24] clk: zte: Add ZX279128S CRM clock and reset driver

[Stefan Dösinger]

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

Hi,

Am Mittwoch, 14. Oktober 2026, 09:20:46 Mitteleuropäische Sommerzeit schrieb 
Navid Ghahremani:
> Support the top CRM and low-speed peripheral clock controllers of the
> ZTE ZX279128S SoC. Implement custom reset controller ops mapping
> logical reset IDs to internal hardware register offsets, and provide
> CRM2 clock gating and reset controller support for PCIe.
> 
> Signed-off-by: Navid Ghahremani <ghahramani.navid@gmail.com>

The comments I sent for v1 still apply to v2 I think.

Please try to reuse the infrastructure in place for zx29. Ideally clk-
zx279128s.c should be just static data and a minimalistic _probe function that 
invokes zx_clk_common_probe().

The drivers/clk/zte code is upstream in linux-next already. The MFD and reset 
parts are still missing though.

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

Re: [PATCH v2 06/24] clk: zte: Add ZX279128S CRM clock and reset driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] clk: zte: boot failure when external oscillator is an orphan
- [Medium] clk: zte: incomplete disable and is_enabled implementation for CRM2 clocks
- [Low] clk: zte: unmet dependency risk from unconditional select of RESET_SIMPLE
--

commit 46157610926015842fd1aee202e8fff3cbf39169
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

clk: zte: Add ZX279128S CRM clock and reset driver

This commit adds support for the top CRM and low-speed peripheral clock
controllers for the ZTE ZX279128S SoC. It implements custom reset controller
operations and provides CRM2 clock gating along with reset controller support
for PCIe.

> diff --git a/drivers/clk/zte/Kconfig b/drivers/clk/zte/Kconfig
> new file mode 100644
> index 0000000000000..92fd27ea42b2f
> --- /dev/null
> +++ b/drivers/clk/zte/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config CLK_ZX279128S
> +	bool "ZTE zx279128s clock driver"
> +	depends on SOC_ZX279128S || COMPILE_TEST
> +	depends on OF
> +	default SOC_ZX279128S
> +	select RESET_CONTROLLER
> +	select RESET_SIMPLE

[Severity: Low]
Does unconditionally selecting RESET_SIMPLE cause an unmet dependency
warning on platforms without HAS_IOMEM (like UML)?

Since the driver implements its own custom reset operations (zx_reset_ops,
zx_crm2_reset_ops) and doesn't use the reset-simple framework, is it possible
to drop the select RESET_SIMPLE entirely?

[ ... ]

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

[ ... ]

> +static void __init zx279128s_topcrm_init(struct device_node *np)
> +{
> +	const struct clk_hw *cpu_parents[4], *matrix_parents[4];
> +	struct clk_hw *osc, *lsp, *lsp_500m, *lsp_250m, *lsp_125m, *lsp_100m;
> +	struct clk_hw *audio, *audio_32k, *matrix, *hclk, *pclk, *m200, *m20;
> +	struct clk_hw_onecell_data *data;
> +	struct clk_hw **hws;
> +	void __iomem *base, *en;
> +	int i;
> +
> +	base = of_iomap(np, 0);
> +	if (!base) {
> +		pr_err("%pOF: no registers\n", np);
> +		return;
> +	}
> +
> +	data = kzalloc(struct_size(data, hws, TOP_NUM_CLKS), GFP_KERNEL);
> +	if (!data)
> +		return;
> +	data->num = TOP_NUM_CLKS;
> +	hws = data->hws;
> +
> +	hws[ZX279128S_TOP_PLL_A9] = zx_top_pll(np, "pll_a9", base + TOP_PLL_A9);
> +	if (IS_ERR(hws[ZX279128S_TOP_PLL_A9]))
> +		goto err;
> +	osc = clk_hw_get_parent(hws[ZX279128S_TOP_PLL_A9]);
> +	if (!osc)
> +		goto err;

[Severity: High]
Does treating a missing parent as a fatal error here break the boot process
if the external "osc" clock is not yet registered?

If the external "osc" clock is an orphan due to probe sequence or driver
loading order, clk_hw_get_parent() will return NULL. Treating this as an error
silently aborts the initialization of all CRM clocks.

Could we instead use fw_name = "osc" (or other suitable clk_parent_data
constructs) for downstream clocks, allowing the Common Clock Framework
to handle orphan resolution naturally?

[ ... ]

> +static void zx_crm2_clk_disable(struct clk_hw *hw)
> +{
> +	struct zx_crm2_gate *gate = to_zx_crm2_gate(hw);
> +	unsigned long flags;
> +	u32 val;
> +
> +	spin_lock_irqsave(gate->lock, flags);
> +	val = readl(gate->reg);
> +	val &= ~gate->clk_on;
> +	writel(val, gate->reg);

[Severity: Medium]
In zx_crm2_clk_enable(), the gate->clk_off bits are cleared to enable the
clock. Should zx_crm2_clk_disable() restore those clk_off bits when disabling?

By only clearing the clk_on bits, this might leave the hardware partially
active or fail to properly isolate the clock domain.

> +	spin_unlock_irqrestore(gate->lock, flags);
> +}
> +
> +static int zx_crm2_clk_is_enabled(struct clk_hw *hw)
> +{
> +	struct zx_crm2_gate *gate = to_zx_crm2_gate(hw);
> +	u32 val = readl(gate->reg);
> +
> +	return (val & gate->clk_on) == gate->clk_on;

[Severity: Medium]
Does this status check need to account for the gate->clk_off bits as well?

If the clock is currently in a disabled state (e.g., clk_off bits set by the
bootloader), this function will report it as enabled if the clk_on bits happen
to be set. This would prevent the Common Clock Framework from ever calling
enable() to clear those isolation bits.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014072109.1759502-1-ghahramani.navid@gmail.com?part=6