diff mbox series

[v6,3/5] dt-bindings: bus: add device tree bindings for qcom,ssc-block-bus

Message ID 20220217113004.22757-3-michael.srba@seznam.cz (mailing list archive)
State Superseded
Headers show
Series [v6,1/5] dt-bindings: clock: gcc-msm8998: Add definitions of SSC-related clocks | expand

Commit Message

Michael Srba Feb. 17, 2022, 11:30 a.m. UTC
From: Michael Srba <Michael.Srba@seznam.cz>

This patch adds bindings for the AHB bus which exposes the SCC block in
the global address space. This bus (and the SSC block itself) is present
on certain qcom SoCs.

In typical configuration, this bus (as some of the clocks and registers
that we need to manipulate) is not accessible to the OS, and the
resources on this bus are indirectly accessed by communicating with a
hexagon CPU core residing in the SSC block. In this configuration, the
hypervisor is the one performing the bus initialization for the purposes
of bringing the haxagon CPU core out of reset.

However, it is possible to change the configuration, in which case this
binding serves to allow the OS to initialize the bus.

Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
---
 NOTE: this applies against v5.17-rc4 just fine; dt_binding_check seems
       to have an issue with indentation, but the indentation looks correct
       to me as well as to local dt_binding_check; also, it seems that the
       rest of this series doesn't get applied before checking for compile
       errors on the example, which results in missing defines for
       GCC_IM_SLEEP, AGGRE2_SNOC_NORTH_AXI, SSC_XO, and SSC_CNOC_AHBS_CLK.

 CHANGES:
 - v2: fix issues caught by by dt-schema
 - v3: none
 - v4: address the issues pointed out in the review
 - v5: clarify type of additional properties; remove ssc_tlmm node for now
 - v6: none
---
 .../bindings/bus/qcom,ssc-block-bus.yaml      | 143 ++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml

Comments

Rob Herring (Arm) Feb. 17, 2022, 8:33 p.m. UTC | #1
On Thu, 17 Feb 2022 12:30:02 +0100, michael.srba@seznam.cz wrote:
> From: Michael Srba <Michael.Srba@seznam.cz>
> 
> This patch adds bindings for the AHB bus which exposes the SCC block in
> the global address space. This bus (and the SSC block itself) is present
> on certain qcom SoCs.
> 
> In typical configuration, this bus (as some of the clocks and registers
> that we need to manipulate) is not accessible to the OS, and the
> resources on this bus are indirectly accessed by communicating with a
> hexagon CPU core residing in the SSC block. In this configuration, the
> hypervisor is the one performing the bus initialization for the purposes
> of bringing the haxagon CPU core out of reset.
> 
> However, it is possible to change the configuration, in which case this
> binding serves to allow the OS to initialize the bus.
> 
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> ---
>  NOTE: this applies against v5.17-rc4 just fine; dt_binding_check seems
>        to have an issue with indentation, but the indentation looks correct
>        to me as well as to local dt_binding_check; also, it seems that the
>        rest of this series doesn't get applied before checking for compile
>        errors on the example, which results in missing defines for
>        GCC_IM_SLEEP, AGGRE2_SNOC_NORTH_AXI, SSC_XO, and SSC_CNOC_AHBS_CLK.
> 
>  CHANGES:
>  - v2: fix issues caught by by dt-schema
>  - v3: none
>  - v4: address the issues pointed out in the review
>  - v5: clarify type of additional properties; remove ssc_tlmm node for now
>  - v6: none
> ---
>  .../bindings/bus/qcom,ssc-block-bus.yaml      | 143 ++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml:86:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.example.dts:39.32-33 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1594179

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring Feb. 17, 2022, 8:37 p.m. UTC | #2
On Thu, Feb 17, 2022 at 5:32 AM <michael.srba@seznam.cz> wrote:
>
> From: Michael Srba <Michael.Srba@seznam.cz>
>
> This patch adds bindings for the AHB bus which exposes the SCC block in
> the global address space. This bus (and the SSC block itself) is present
> on certain qcom SoCs.
>
> In typical configuration, this bus (as some of the clocks and registers
> that we need to manipulate) is not accessible to the OS, and the
> resources on this bus are indirectly accessed by communicating with a
> hexagon CPU core residing in the SSC block. In this configuration, the
> hypervisor is the one performing the bus initialization for the purposes
> of bringing the haxagon CPU core out of reset.
>
> However, it is possible to change the configuration, in which case this
> binding serves to allow the OS to initialize the bus.
>
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> ---
>  NOTE: this applies against v5.17-rc4 just fine; dt_binding_check seems
>        to have an issue with indentation, but the indentation looks correct
>        to me as well as to local dt_binding_check; also, it seems that the
>        rest of this series doesn't get applied before checking for compile
>        errors on the example, which results in missing defines for
>        GCC_IM_SLEEP, AGGRE2_SNOC_NORTH_AXI, SSC_XO, and SSC_CNOC_AHBS_CLK.
>
>  CHANGES:
>  - v2: fix issues caught by by dt-schema
>  - v3: none
>  - v4: address the issues pointed out in the review
>  - v5: clarify type of additional properties; remove ssc_tlmm node for now
>  - v6: none
> ---
>  .../bindings/bus/qcom,ssc-block-bus.yaml      | 143 ++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml b/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
> new file mode 100644
> index 000000000000..4044af0afda8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/qcom,ssc-block-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: The AHB Bus Providing a Global View of the SSC Block on (some) qcom SoCs
> +
> +maintainers:
> +  - Michael Srba <Michael.Srba@seznam.cz>
> +
> +description: |
> +  This binding describes the dependencies (clocks, resets, power domains) which
> +  need to be turned on in a sequence before communication over the AHB bus
> +  becomes possible.
> +
> +  Additionally, the reg property is used to pass to the driver the location of
> +  two sadly undocumented registers which need to be poked as part of the sequence.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: qcom,msm8998-ssc-block-bus
> +      - const: qcom,ssc-block-bus
> +
> +  reg:
> +    description: |
> +      Shall contain the addresses of the SSCAON_CONFIG0 and SSCAON_CONFIG1
> +      registers
> +    minItems: 2
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: mpm_sscaon_config0
> +      - const: mpm_sscaon_config1
> +
> +  '#address-cells':
> +    enum: [ 1, 2 ]
> +
> +  '#size-cells':
> +    enum: [ 1, 2 ]
> +
> +  ranges: true
> +
> +  clocks:
> +    minItems: 6
> +    maxItems: 6
> +
> +  clock-names:
> +    items:
> +      - const: xo
> +      - const: aggre2
> +      - const: gcc_im_sleep
> +      - const: aggre2_north
> +      - const: ssc_xo
> +      - const: ssc_ahbs
> +
> +  power-domains:
> +    description: Power domain phandles for the ssc_cx and ssc_mx power domains
> +    minItems: 2
> +    maxItems: 2
> +
> +  power-domain-names:
> +    items:
> +      - const: ssc_cx
> +      - const: ssc_mx
> +
> +  resets:
> +    description: |
> +      Reset phandles for the ssc_reset and ssc_bcr resets (note: ssc_bcr is the
> +      branch control register associated with the ssc_xo and ssc_ahbs clocks)
> +    minItems: 2
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: ssc_reset
> +      - const: ssc_bcr
> +
> +  qcom,halt-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: describes how to locate the ssc AXI halt register
> +    items:
> +      - items:
> +        - description: Phandle reference to a syscon representing TCSR
> +        - description: offset for the ssc AXI halt register

These need indenting 2 more spaces.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>
Rob Herring Feb. 17, 2022, 8:41 p.m. UTC | #3
On Thu, Feb 17, 2022 at 2:33 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, 17 Feb 2022 12:30:02 +0100, michael.srba@seznam.cz wrote:
> > From: Michael Srba <Michael.Srba@seznam.cz>
> >
> > This patch adds bindings for the AHB bus which exposes the SCC block in
> > the global address space. This bus (and the SSC block itself) is present
> > on certain qcom SoCs.
> >
> > In typical configuration, this bus (as some of the clocks and registers
> > that we need to manipulate) is not accessible to the OS, and the
> > resources on this bus are indirectly accessed by communicating with a
> > hexagon CPU core residing in the SSC block. In this configuration, the
> > hypervisor is the one performing the bus initialization for the purposes
> > of bringing the haxagon CPU core out of reset.
> >
> > However, it is possible to change the configuration, in which case this
> > binding serves to allow the OS to initialize the bus.
> >
> > Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> > ---
> >  NOTE: this applies against v5.17-rc4 just fine; dt_binding_check seems
> >        to have an issue with indentation, but the indentation looks correct
> >        to me as well as to local dt_binding_check; also, it seems that the
> >        rest of this series doesn't get applied before checking for compile
> >        errors on the example, which results in missing defines for
> >        GCC_IM_SLEEP, AGGRE2_SNOC_NORTH_AXI, SSC_XO, and SSC_CNOC_AHBS_CLK.
> >
> >  CHANGES:
> >  - v2: fix issues caught by by dt-schema
> >  - v3: none
> >  - v4: address the issues pointed out in the review
> >  - v5: clarify type of additional properties; remove ssc_tlmm node for now
> >  - v6: none
> > ---
> >  .../bindings/bus/qcom,ssc-block-bus.yaml      | 143 ++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml:86:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

Please fix your yamllint install (or lack of).

> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.example.dts:39.32-33 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1398: dt_binding_check] Error 2

This failure can be ignored. I believe the problem is with my script
applying series without a cover letter. However, series should have a
cover letter.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml b/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
new file mode 100644
index 000000000000..4044af0afda8
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/qcom,ssc-block-bus.yaml
@@ -0,0 +1,143 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/qcom,ssc-block-bus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The AHB Bus Providing a Global View of the SSC Block on (some) qcom SoCs
+
+maintainers:
+  - Michael Srba <Michael.Srba@seznam.cz>
+
+description: |
+  This binding describes the dependencies (clocks, resets, power domains) which
+  need to be turned on in a sequence before communication over the AHB bus
+  becomes possible.
+
+  Additionally, the reg property is used to pass to the driver the location of
+  two sadly undocumented registers which need to be poked as part of the sequence.
+
+properties:
+  compatible:
+    items:
+      - const: qcom,msm8998-ssc-block-bus
+      - const: qcom,ssc-block-bus
+
+  reg:
+    description: |
+      Shall contain the addresses of the SSCAON_CONFIG0 and SSCAON_CONFIG1
+      registers
+    minItems: 2
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: mpm_sscaon_config0
+      - const: mpm_sscaon_config1
+
+  '#address-cells':
+    enum: [ 1, 2 ]
+
+  '#size-cells':
+    enum: [ 1, 2 ]
+
+  ranges: true
+
+  clocks:
+    minItems: 6
+    maxItems: 6
+
+  clock-names:
+    items:
+      - const: xo
+      - const: aggre2
+      - const: gcc_im_sleep
+      - const: aggre2_north
+      - const: ssc_xo
+      - const: ssc_ahbs
+
+  power-domains:
+    description: Power domain phandles for the ssc_cx and ssc_mx power domains
+    minItems: 2
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: ssc_cx
+      - const: ssc_mx
+
+  resets:
+    description: |
+      Reset phandles for the ssc_reset and ssc_bcr resets (note: ssc_bcr is the
+      branch control register associated with the ssc_xo and ssc_ahbs clocks)
+    minItems: 2
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: ssc_reset
+      - const: ssc_bcr
+
+  qcom,halt-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: describes how to locate the ssc AXI halt register
+    items:
+      - items:
+        - description: Phandle reference to a syscon representing TCSR
+        - description: offset for the ssc AXI halt register
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+  - clocks
+  - clock-names
+  - power-domains
+  - power-domain-names
+  - resets
+  - reset-names
+  - qcom,halt-regs
+
+additionalProperties:
+  type: object
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-msm8998.h>
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        // devices under this node are physically located in the SSC block, connected to an ssc-internal bus;
+        ssc_ahb_slave: bus@10ac008 {
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges;
+
+            compatible = "qcom,msm8998-ssc-block-bus", "qcom,ssc-block-bus";
+            reg = <0x10ac008 0x4>, <0x10ac010 0x4>;
+            reg-names = "mpm_sscaon_config0", "mpm_sscaon_config1";
+
+            clocks = <&xo>,
+                     <&rpmcc RPM_SMD_AGGR2_NOC_CLK>,
+                     <&gcc GCC_IM_SLEEP>,
+                     <&gcc AGGRE2_SNOC_NORTH_AXI>,
+                     <&gcc SSC_XO>,
+                     <&gcc SSC_CNOC_AHBS_CLK>;
+            clock-names = "xo", "aggre2", "gcc_im_sleep", "aggre2_north", "ssc_xo", "ssc_ahbs";
+
+            resets = <&gcc GCC_SSC_RESET>, <&gcc GCC_SSC_BCR>;
+            reset-names = "ssc_reset", "ssc_bcr";
+
+            power-domains = <&rpmpd MSM8998_SSCCX>, <&rpmpd MSM8998_SSCMX>;
+            power-domain-names = "ssc_cx", "ssc_mx";
+
+            qcom,halt-regs = <&tcsr_mutex_regs 0x26000>;
+        };
+    };