diff mbox series

[v3,2/3] dt-bindings: soc: spacemit: Add spacemit,k1-syscon

Message ID 20241126143125.9980-5-heylenay@4d2.org (mailing list archive)
State Handled Elsewhere
Headers show
Series Add clock controller support for Spacemit K1 | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR fail PR summary
conchuod/patch-2-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 205.04s
conchuod/patch-2-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 1310.38s
conchuod/patch-2-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1498.45s
conchuod/patch-2-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 75.27s
conchuod/patch-2-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 77.65s
conchuod/patch-2-test-6 warning .github/scripts/patches/tests/checkpatch.sh took 0.58s
conchuod/patch-2-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 47.38s
conchuod/patch-2-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-2-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.58s
conchuod/patch-2-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-2-test-11 success .github/scripts/patches/tests/verify_fixes.sh took 0.00s
conchuod/patch-2-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.03s

Commit Message

Haylen Chu Nov. 26, 2024, 2:31 p.m. UTC
Add documentation to describe Spacemit K1 system controller registers.

Signed-off-by: Haylen Chu <heylenay@4d2.org>
---
 .../soc/spacemit/spacemit,k1-syscon.yaml      | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml

Comments

Krzysztof Kozlowski Nov. 26, 2024, 2:46 p.m. UTC | #1
On 26/11/2024 15:31, Haylen Chu wrote:
> Add documentation to describe Spacemit K1 system controller registers.
> 
> Signed-off-by: Haylen Chu <heylenay@4d2.org>
> ---
>  .../soc/spacemit/spacemit,k1-syscon.yaml      | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml b/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml
> new file mode 100644
> index 000000000000..b9f20190a70a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/spacemit/spacemit,k1-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Spacemit K1 SoC System Controller
> +
> +maintainers:
> +  - Haylen Chu <heylenay@4d2.org>
> +
> +description:
> +  The Spacemit K1 SoC system controller provides access to shared register files
> +  for related SoC modules, such as clock controller and reset controller.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - spacemit,k1-apbc-syscon
> +          - spacemit,k1-apbs-syscon
> +          - spacemit,k1-apmu-syscon
> +          - spacemit,k1-mpmu-syscon
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  clock-controller:
> +    $ref: /schemas/clock/spacemit,k1-ccu.yaml#
> +    type: object
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
Nothing improved. That's a no-go.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 26, 2024, 2:50 p.m. UTC | #2
On 26/11/2024 15:31, Haylen Chu wrote:
> +examples:
> +  - |
> +    osc_32k: clock-32k {
> +        compatible = "fixed-clock";
> +        clock-frequency = <32000>;
> +        clock-output-names = "osc_32k";
> +        #clock-cells = <0>;
> +    };
> +
> +    vctcxo_1m: clock-1m {
> +        compatible = "fixed-clock";
> +        clock-frequency = <1000000>;
> +        clock-output-names = "vctcxo_1m";
> +        #clock-cells = <0>;
> +    };
> +
> +    vctcxo_3m: clock-3m {
> +        compatible = "fixed-clock";
> +        clock-frequency = <3000000>;
> +        clock-output-names = "vctcxo_3m";
> +        #clock-cells = <0>;
> +    };
> +
> +    vctcxo_24m: clock-24m {
> +        compatible = "fixed-clock";
> +        clock-frequency = <24000000>;
> +        clock-output-names = "vctcxo_24m";
> +        #clock-cells = <0>;
> +    };

Drop all above. Your changelog is poor - does not explain this at all.
Write changelogs which detail what you did and why.

Best regards,
Krzysztof
Haylen Chu Nov. 27, 2024, 11:01 a.m. UTC | #3
On Tue, Nov 26, 2024 at 03:50:30PM +0100, Krzysztof Kozlowski wrote:
> On 26/11/2024 15:31, Haylen Chu wrote:
> > +examples:
> > +  - |
> > +    osc_32k: clock-32k {
> > +        compatible = "fixed-clock";
> > +        clock-frequency = <32000>;
> > +        clock-output-names = "osc_32k";
> > +        #clock-cells = <0>;
> > +    };
> > +
> > +    vctcxo_1m: clock-1m {
> > +        compatible = "fixed-clock";
> > +        clock-frequency = <1000000>;
> > +        clock-output-names = "vctcxo_1m";
> > +        #clock-cells = <0>;
> > +    };
> > +
> > +    vctcxo_3m: clock-3m {
> > +        compatible = "fixed-clock";
> > +        clock-frequency = <3000000>;
> > +        clock-output-names = "vctcxo_3m";
> > +        #clock-cells = <0>;
> > +    };
> > +
> > +    vctcxo_24m: clock-24m {
> > +        compatible = "fixed-clock";
> > +        clock-frequency = <24000000>;
> > +        clock-output-names = "vctcxo_24m";
> > +        #clock-cells = <0>;
> > +    };
> 
> Drop all above. Your changelog is poor - does not explain this at all.
> Write changelogs which detail what you did and why.

Thanks, I forgot that missing phandle references are acceptable in
dt-binding examples. This and other required changes in v2 that I forgot
to apply will be adapted in v4.

> 
> Best regards,
> Krzysztof

Thanks,
Haylen Chu
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml b/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml
new file mode 100644
index 000000000000..b9f20190a70a
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/spacemit/spacemit,k1-syscon.yaml
@@ -0,0 +1,86 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/spacemit/spacemit,k1-syscon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Spacemit K1 SoC System Controller
+
+maintainers:
+  - Haylen Chu <heylenay@4d2.org>
+
+description:
+  The Spacemit K1 SoC system controller provides access to shared register files
+  for related SoC modules, such as clock controller and reset controller.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - spacemit,k1-apbc-syscon
+          - spacemit,k1-apbs-syscon
+          - spacemit,k1-apmu-syscon
+          - spacemit,k1-mpmu-syscon
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  clock-controller:
+    $ref: /schemas/clock/spacemit,k1-ccu.yaml#
+    type: object
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    osc_32k: clock-32k {
+        compatible = "fixed-clock";
+        clock-frequency = <32000>;
+        clock-output-names = "osc_32k";
+        #clock-cells = <0>;
+    };
+
+    vctcxo_1m: clock-1m {
+        compatible = "fixed-clock";
+        clock-frequency = <1000000>;
+        clock-output-names = "vctcxo_1m";
+        #clock-cells = <0>;
+    };
+
+    vctcxo_3m: clock-3m {
+        compatible = "fixed-clock";
+        clock-frequency = <3000000>;
+        clock-output-names = "vctcxo_3m";
+        #clock-cells = <0>;
+    };
+
+    vctcxo_24m: clock-24m {
+        compatible = "fixed-clock";
+        clock-frequency = <24000000>;
+        clock-output-names = "vctcxo_24m";
+        #clock-cells = <0>;
+    };
+
+    system-controller@d4050000 {
+        compatible = "spacemit,k1-mpmu-syscon", "syscon", "simple-mfd";
+        reg = <0xd4050000 0x209c>;
+
+        clock-controller {
+            compatible = "spacemit,k1-ccu-mpmu";
+            clocks = <&osc_32k>, <&vctcxo_1m>, <&vctcxo_3m>, <&vctcxo_24m>;
+            clock-names = "osc_32k", "vctcxo_1m", "vctcxo_3m", "vctcxo_24m";
+            #clock-cells = <1>;
+        };
+    };