2020-03-21 01:32:53

by Rishabh Bhatnagar

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: remoteproc: Add documentation for SPSS remoteproc

Add devicetree binding for Secure Subsystem remote processor
support in remoteproc framework. This describes all the resources
needed by SPSS to boot and handle crash and shutdown scenarios.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
---
.../devicetree/bindings/remoteproc/qcom,spss.yaml | 125 +++++++++++++++++++++
1 file changed, 125 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
new file mode 100644
index 0000000..9ca7947a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/qcom,spss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SPSS Peripheral Image Loader
+
+maintainers:
+ - Rishabh Bhatnagar <[email protected]>
+description: |
+ This document defines the binding for a component that loads and boots firmware
+ on the Qualcomm Secure Peripheral Processor. This processor is booted in the
+ bootloader stage and it attaches itself to linux later on in the boot process.
+
+properties:
+ compatible:
+ enum:
+ "qcom,sm8250-spss-pas"
+
+ reg:
+ items:
+ - description: IRQ status register
+ - description: IRQ clear register
+ - description: IRQ mask register
+ - description: Error register
+ - description: Error spare register
+
+ reg-names:
+ items:
+ - const: sp2soc_irq_status
+ - const: sp2soc_irq_clr
+ - const: sp2soc_irq_mask
+ - const: rmb_err
+ - const: rmb_err_spare2
+
+ interrupts:
+ maxitems: 1
+ items:
+ - description: rx interrupt
+
+ clocks:
+ items:
+ - description:
+ reference to the xo clock to be held on behalf
+ of the booting Hexagon core
+
+ clock-names:
+ items:
+ - const: xo
+
+ cx-supply: true
+
+ px-supply: true
+
+ memory-region: true
+ items:
+ - description: reference to the reserved-memory for the SPSS
+
+ qcom,spss-scsr-bits:
+ - description: Bits that are set by remote processor in the irq status
+ register region to represent different states during
+ boot process
+
+ child-node:
+ description: Subnode named either "smd-edge" or "glink-edge" that
+ describes the communication edge, channels and devices
+ related to the SPSS.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - clocks
+ - clock-names
+ - cx-supply
+ - px-supply
+ - memory-region
+ - qcom,spss-scsr-bits
+
+
+examples:
+ - |
+ spss {
+ compatible = "qcom,sm8250-spss-pil";
+ reg = <0x188101c 0x4>,
+ <0x1881024 0x4>,
+ <0x1881028 0x4>,
+ <0x188103c 0x4>,
+ <0x1882014 0x4>;
+ reg-names = "sp2soc_irq_status", "sp2soc_irq_clr",
+ "sp2soc_irq_mask", "rmb_err", "rmb_err_spare2";
+ interrupts = <0 352 1>;
+
+ cx-supply = <&VDD_CX_LEVEL>;
+ cx-uV-uA = <RPMH_REGULATOR_LEVEL_TURBO 100000>;
+ px-supply = <&VDD_MX_LEVEL>;
+ px-uV = <RPMH_REGULATOR_LEVEL_TURBO 100000>;
+
+ clocks = <&clock_rpmh RPMH_CXO_CLK>;
+ clock-names = "xo";
+ qcom,proxy-clock-names = "xo";
+ status = "ok";
+
+ memory-region = <&pil_spss_mem>;
+ qcom,spss-scsr-bits = <24 25>;
+
+ glink-edge {
+ qcom,remote-pid = <8>;
+ transport = "spss";
+ mboxes = <&sp_scsr 0>;
+ mbox-names = "spss_spss";
+ interrupt-parent = <&intsp>;
+ interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>;
+
+ reg = <0x1885008 0x8>,
+ <0x1885010 0x4>;
+ reg-names = "qcom,spss-addr",
+ "qcom,spss-size";
+
+ label = "spss";
+ qcom,glink-label = "spss";
+ };
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-03-23 16:24:17

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: remoteproc: Add documentation for SPSS remoteproc

On Fri, 20 Mar 2020 18:32:10 -0700, Rishabh Bhatnagar wrote:
> Add devicetree binding for Secure Subsystem remote processor
> support in remoteproc framework. This describes all the resources
> needed by SPSS to boot and handle crash and shutdown scenarios.
>
> Signed-off-by: Rishabh Bhatnagar <[email protected]>
> ---
> .../devicetree/bindings/remoteproc/qcom,spss.yaml | 125 +++++++++++++++++++++
> 1 file changed, 125 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml: mapping values are not allowed in this context
in "<unicode string>", line 57, column 10
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/remoteproc/qcom,spss.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/remoteproc/qcom,spss.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

2020-03-25 05:28:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: remoteproc: Add documentation for SPSS remoteproc

On Fri 20 Mar 18:32 PDT 2020, Rishabh Bhatnagar wrote:

> Add devicetree binding for Secure Subsystem remote processor
> support in remoteproc framework. This describes all the resources
> needed by SPSS to boot and handle crash and shutdown scenarios.
>
> Signed-off-by: Rishabh Bhatnagar <[email protected]>
> ---
> .../devicetree/bindings/remoteproc/qcom,spss.yaml | 125 +++++++++++++++++++++
> 1 file changed, 125 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
> new file mode 100644
> index 0000000..9ca7947a9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,spss.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/qcom,spss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SPSS Peripheral Image Loader
> +
> +maintainers:
> + - Rishabh Bhatnagar <[email protected]>
> +description: |
> + This document defines the binding for a component that loads and boots firmware
> + on the Qualcomm Secure Peripheral Processor. This processor is booted in the
> + bootloader stage and it attaches itself to linux later on in the boot process.
> +
> +properties:
> + compatible:
> + enum:
> + "qcom,sm8250-spss-pas"
> +
> + reg:
> + items:
> + - description: IRQ status register
> + - description: IRQ clear register
> + - description: IRQ mask register
> + - description: Error register
> + - description: Error spare register
> +
> + reg-names:
> + items:
> + - const: sp2soc_irq_status
> + - const: sp2soc_irq_clr
> + - const: sp2soc_irq_mask
> + - const: rmb_err
> + - const: rmb_err_spare2
> +
> + interrupts:
> + maxitems: 1
> + items:
> + - description: rx interrupt
> +
> + clocks:
> + items:
> + - description:
> + reference to the xo clock to be held on behalf
> + of the booting Hexagon core
> +
> + clock-names:
> + items:
> + - const: xo
> +
> + cx-supply: true
> +
> + px-supply: true
> +
> + memory-region: true
> + items:
> + - description: reference to the reserved-memory for the SPSS
> +
> + qcom,spss-scsr-bits:
> + - description: Bits that are set by remote processor in the irq status
> + register region to represent different states during
> + boot process
> +
> + child-node:
> + description: Subnode named either "smd-edge" or "glink-edge" that
> + describes the communication edge, channels and devices
> + related to the SPSS.
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - clocks
> + - clock-names
> + - cx-supply
> + - px-supply
> + - memory-region
> + - qcom,spss-scsr-bits
> +
> +
> +examples:
> + - |
> + spss {

remoteproc@188101c but probably rather remoteproc@1880000



> + compatible = "qcom,sm8250-spss-pil";

s/pil/pas/

> + reg = <0x188101c 0x4>,
> + <0x1881024 0x4>,
> + <0x1881028 0x4>,
> + <0x188103c 0x4>,
> + <0x1882014 0x4>;

As noted in the driver review, these are all from the same block, map it
once.

> + reg-names = "sp2soc_irq_status", "sp2soc_irq_clr",
> + "sp2soc_irq_mask", "rmb_err", "rmb_err_spare2";
> + interrupts = <0 352 1>;

interrupts = <GIC_SPI 352 IRQ_TYPE_EDGE_RISING>;

> +
> + cx-supply = <&VDD_CX_LEVEL>;

These are power domains.

> + cx-uV-uA = <RPMH_REGULATOR_LEVEL_TURBO 100000>;

And we'll just vote for max.

> + px-supply = <&VDD_MX_LEVEL>;
> + px-uV = <RPMH_REGULATOR_LEVEL_TURBO 100000>;
> +
> + clocks = <&clock_rpmh RPMH_CXO_CLK>;
> + clock-names = "xo";
> + qcom,proxy-clock-names = "xo";

We don't specify this in DT.

> + status = "ok";

This can be omitted from the example.

> +
> + memory-region = <&pil_spss_mem>;
> + qcom,spss-scsr-bits = <24 25>;

As requested, just hard code these in the driver instead.

> +
> + glink-edge {

This depends on a separate binding, which we haven't yet discussed.
Perhaps worth omitting it for now?

> + qcom,remote-pid = <8>;
> + transport = "spss";

The registered subdev should always be of "spss" type, shouldn't be a
need to describe that here.

> + mboxes = <&sp_scsr 0>;
> + mbox-names = "spss_spss";
> + interrupt-parent = <&intsp>;
> + interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>;

As you map the entire scsr region in the remoteproc driver, this should
reference the spss rproc node above.

> +
> + reg = <0x1885008 0x8>,
> + <0x1885010 0x4>;
> + reg-names = "qcom,spss-addr",
> + "qcom,spss-size";

And it seems reasonable that we pass this information from the rproc
when we create the subdev, rather than having the glink implementation
dig it out from the scsr.

Regards,
Bjorn

> +
> + label = "spss";
> + qcom,glink-label = "spss";
> + };
> + };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project