diff mbox series

[05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem

Message ID 20230531-rpm-rproc-v1-5-e0a3b6de1f14@gerhold.net (mailing list archive)
State Superseded
Headers show
Series Add dedicated device tree node for RPM processor/subsystem | expand

Commit Message

Stephan Gerhold June 5, 2023, 7:08 a.m. UTC
On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
described as remote processors in the device tree, with a dedicated
node where properties and services related to them can be described.

The Resource Power Manager (RPM) is also such a subsystem, with a
remote processor that is running a special firmware. Unfortunately,
the RPM never got a dedicated node representing it properly in the
device tree. Most of the RPM services are described below a top-level
/smd or /rpm-glink node.

However, SMD/GLINK is just one of the communication channels to the RPM
firmware. For example, the MPM interrupt functionality provided by the
RPM does not use SMD/GLINK but writes directly to a special memory
region allocated by the RPM firmware in combination with a mailbox.
Currently there is no good place in the device tree to describe this
functionality. It doesn't belong below SMD/GLINK but it's not an
independent top-level device either.

Introduce a new "qcom,rpm-proc" compatible that allows describing the
RPM as a remote processor/subsystem like all others. The SMD/GLINK node
is moved to a "smd-edge"/"glink-edge" subnode consistent with other
existing bindings. Additional subnodes (e.g. interrupt-controller for
MPM, rpm-master-stats) can be also added there.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
 1 file changed, 125 insertions(+)

Comments

Rob Herring June 5, 2023, 8:33 a.m. UTC | #1
On Mon, 05 Jun 2023 09:08:21 +0200, Stephan Gerhold wrote:
> On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
> described as remote processors in the device tree, with a dedicated
> node where properties and services related to them can be described.
> 
> The Resource Power Manager (RPM) is also such a subsystem, with a
> remote processor that is running a special firmware. Unfortunately,
> the RPM never got a dedicated node representing it properly in the
> device tree. Most of the RPM services are described below a top-level
> /smd or /rpm-glink node.
> 
> However, SMD/GLINK is just one of the communication channels to the RPM
> firmware. For example, the MPM interrupt functionality provided by the
> RPM does not use SMD/GLINK but writes directly to a special memory
> region allocated by the RPM firmware in combination with a mailbox.
> Currently there is no good place in the device tree to describe this
> functionality. It doesn't belong below SMD/GLINK but it's not an
> independent top-level device either.
> 
> Introduce a new "qcom,rpm-proc" compatible that allows describing the
> RPM as a remote processor/subsystem like all others. The SMD/GLINK node
> is moved to a "smd-edge"/"glink-edge" subnode consistent with other
> existing bindings. Additional subnodes (e.g. interrupt-controller for
> MPM, rpm-master-stats) can be also added there.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 

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:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/soc/qcom/qcom,rpm-master-stats.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230531-rpm-rproc-v1-5-e0a3b6de1f14@gerhold.net

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Stephan Gerhold June 5, 2023, 9:16 a.m. UTC | #2
On Mon, Jun 05, 2023 at 02:33:58AM -0600, Rob Herring wrote:
> On Mon, 05 Jun 2023 09:08:21 +0200, Stephan Gerhold wrote:
> > On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
> > described as remote processors in the device tree, with a dedicated
> > node where properties and services related to them can be described.
> > 
> > The Resource Power Manager (RPM) is also such a subsystem, with a
> > remote processor that is running a special firmware. Unfortunately,
> > the RPM never got a dedicated node representing it properly in the
> > device tree. Most of the RPM services are described below a top-level
> > /smd or /rpm-glink node.
> > 
> > However, SMD/GLINK is just one of the communication channels to the RPM
> > firmware. For example, the MPM interrupt functionality provided by the
> > RPM does not use SMD/GLINK but writes directly to a special memory
> > region allocated by the RPM firmware in combination with a mailbox.
> > Currently there is no good place in the device tree to describe this
> > functionality. It doesn't belong below SMD/GLINK but it's not an
> > independent top-level device either.
> > 
> > Introduce a new "qcom,rpm-proc" compatible that allows describing the
> > RPM as a remote processor/subsystem like all others. The SMD/GLINK node
> > is moved to a "smd-edge"/"glink-edge" subnode consistent with other
> > existing bindings. Additional subnodes (e.g. interrupt-controller for
> > MPM, rpm-master-stats) can be also added there.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
> >  1 file changed, 125 insertions(+)
> > 
> 
> 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:
> 
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/soc/qcom/qcom,rpm-master-stats.yaml
> 

I think we can ignore this error: The qcom,rpm-master-stats.yaml schema
exists only in the qcom for-next branch at the moment, which is what
this series targets. The base-commit in the cover letter also points
there (although I guess it might be tricky to resolve it reliably for
automated testing).

Before sending this series I verified that there are no dt_binding_check
and dtbs_check warnings or errors when applied to the correct branch.

Thanks,
Stephan
Krzysztof Kozlowski June 6, 2023, 6:36 a.m. UTC | #3
On 05/06/2023 09:08, Stephan Gerhold wrote:
> On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
> described as remote processors in the device tree, with a dedicated
> node where properties and services related to them can be described.
> 
> The Resource Power Manager (RPM) is also such a subsystem, with a
> remote processor that is running a special firmware. Unfortunately,
> the RPM never got a dedicated node representing it properly in the
> device tree. Most of the RPM services are described below a top-level
> /smd or /rpm-glink node.

Then what is rpm-requests? This is the true RPM. It looks like you now
duplicate half of it in a node above. Unless you want here to describe
ways to communicate with the RPM, not the RPM itself.


> However, SMD/GLINK is just one of the communication channels to the RPM
> firmware. For example, the MPM interrupt functionality provided by the
> RPM does not use SMD/GLINK but writes directly to a special memory
> region allocated by the RPM firmware in combination with a mailbox.
> Currently there is no good place in the device tree to describe this
> functionality. It doesn't belong below SMD/GLINK but it's not an
> independent top-level device either.
> 
> Introduce a new "qcom,rpm-proc" compatible that allows describing the
> RPM as a remote processor/subsystem like all others. The SMD/GLINK node
> is moved to a "smd-edge"/"glink-edge" subnode consistent with other
> existing bindings. Additional subnodes (e.g. interrupt-controller for
> MPM, rpm-master-stats) can be also added there.

If this was about to stay, you should also update the qcom,smd.yaml, so
there will be no duplication.

> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
> new file mode 100644
> index 000000000000..c06dd4f66503
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,rpm-proc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Resource Power Manager (RPM) Processor/Subsystem
> +
> +maintainers:
> +  - Bjorn Andersson <andersson@kernel.org>
> +  - Konrad Dybcio <konrad.dybcio@linaro.org>
> +
> +description:
> +  Resource Power Manager (RPM) subsystem found in various Qualcomm platforms.
> +  The RPM allows each component in the system to vote for state of the system
> +  resources, such as clocks, regulators and bus frequencies. rpm-proc is the
> +  top-level device tree node that groups all the RPM functionality together.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,apq8084-rpm-proc
> +          - qcom,ipq6018-rpm-proc
> +          - qcom,ipq9574-rpm-proc
> +          - qcom,mdm9607-rpm-proc
> +          - qcom,msm8226-rpm-proc
> +          - qcom,msm8610-rpm-proc
> +          - qcom,msm8909-rpm-proc
> +          - qcom,msm8916-rpm-proc
> +          - qcom,msm8917-rpm-proc
> +          - qcom,msm8936-rpm-proc
> +          - qcom,msm8937-rpm-proc
> +          - qcom,msm8952-rpm-proc
> +          - qcom,msm8953-rpm-proc
> +          - qcom,msm8974-rpm-proc
> +          - qcom,msm8976-rpm-proc
> +          - qcom,msm8994-rpm-proc
> +          - qcom,msm8996-rpm-proc
> +          - qcom,msm8998-rpm-proc
> +          - qcom,qcm2290-rpm-proc
> +          - qcom,qcs404-rpm-proc
> +          - qcom,sdm660-rpm-proc
> +          - qcom,sm6115-rpm-proc
> +          - qcom,sm6125-rpm-proc
> +          - qcom,sm6375-rpm-proc
> +      - const: qcom,rpm-proc
> +
> +  smd-edge:
> +    $ref: /schemas/remoteproc/qcom,smd-edge.yaml#
> +    description:
> +      Qualcomm Shared Memory subnode which represents communication edge,
> +      channels and devices related to the RPM subsystem.
> +
> +  glink-rpm:
> +    $ref: /schemas/remoteproc/qcom,glink-rpm-edge.yaml#
> +    description:
> +      Qualcomm G-Link subnode which represents communication edge,
> +      channels and devices related to the RPM subsystem.
> +
> +  interrupt-controller:
> +    type: object
> +    $ref: /schemas/interrupt-controller/qcom,mpm.yaml#
> +    description:
> +      MSM Power Manager (MPM) interrupt controller that monitors interrupts
> +      when the system is asleep.

Isn't this a service of RPM?

> +
> +  master-stats:
> +    $ref: /schemas/soc/qcom/qcom,rpm-master-stats.yaml#
> +    description:
> +      Subsystem-level low-power mode statistics provided by RPM.

The same question.

> +
> +required:
> +  - compatible
> +
> +oneOf:
> +  - required:
> +      - smd-edge
> +  - required:
> +      - glink-rpm
> +
> +additionalProperties: false
> +
> +examples:
> +  # SMD
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    remoteproc-rpm {

remoteproc

> +      compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc";
> +
> +      smd-edge {
> +        interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> +        qcom,ipc = <&apcs 8 0>;
> +        qcom,smd-edge = <15>;
> +
> +        rpm-requests {
> +          compatible = "qcom,rpm-msm8916";
> +          qcom,smd-channels = "rpm_requests";
> +          /* ... */
> +        };
> +      };
> +    };
> +  # GLINK
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    remoteproc-rpm {

remoteproc

> +      compatible = "qcom,qcm2290-rpm-proc", "qcom,rpm-proc";
> +
> +      glink-rpm {
> +        compatible = "qcom,glink-rpm";
> +        interrupts = <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>;
> +        qcom,rpm-msg-ram = <&rpm_msg_ram>;
> +        mboxes = <&apcs_glb 0>;
> +
> +        rpm-requests {
> +          compatible = "qcom,rpm-qcm2290";
> +          qcom,glink-channels = "rpm_requests";
> +          /* ... */
> +        };
> +      };
> +    };
> 

Best regards,
Krzysztof
Stephan Gerhold June 6, 2023, 8:55 a.m. UTC | #4
<On Tue, Jun 06, 2023 at 08:36:10AM +0200, Krzysztof Kozlowski wrote:
> On 05/06/2023 09:08, Stephan Gerhold wrote:
> > On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
> > described as remote processors in the device tree, with a dedicated
> > node where properties and services related to them can be described.
> > 
> > The Resource Power Manager (RPM) is also such a subsystem, with a
> > remote processor that is running a special firmware. Unfortunately,
> > the RPM never got a dedicated node representing it properly in the
> > device tree. Most of the RPM services are described below a top-level
> > /smd or /rpm-glink node.
> 
> Then what is rpm-requests? This is the true RPM. It looks like you now
> duplicate half of it in a node above. Unless you want here to describe
> ways to communicate with the RPM, not the RPM itself.
>

I think you're confusing hardware and firmware here. The rpm-proc node
represents the RPM hardware block (or perhaps the RPM firmware overall),
while rpm-requests is just *one* communication interface provided by the
RPM firmware. Here is a rough picture of the RPM "subsystem" I'm trying
to describe:

                +--------------------------------------------+      
                |       RPM subsystem (qcom,rpm-proc)        |      
                |                                            |      
          reset | +---------------+     +-----+  +-----+     |      
        --------->|               |     | MPM |  | CPR | ... |      
 IPC interrupts | | ARM Cortex-M3 |     +-----+  +-----+     |      
----------------->|               |---     |        |        |      
                | +---------------+  |---------------------- |      
                | +---------------+  |                       |      
                | |   Code RAM    |--|  +------------------+ |      
                | +---------------+  |  |                  | |      
                | +---------------+  |  |   Message RAM    | |      
                | |   Data RAM    |--|--|                  | |      
                | +---------------+  |  +------------------+ |      
                +--------------------|-----------------------+      
                                     v                              
                                    NoC                             

(Somewhat adapted from Figure 8-1 RPM overview in the APQ8016E Technical
 Reference Manual, but I added some extra stuff.)

As you can see neither "SMD" nor "rpm-requests" exist in hardware.
Again, it's just one communication protocol implemented by the RPM
firmware running on the Cortex-M3 processor, much like a webserver
running on a PC.

When the SoC is started only the hardware block exists. Usually a
component in the boot chain loads firmware into the code/data RAM and
then de-asserts the reset line to start the Cortex-M3 processor.

This is not guaranteed to happen. For example, I have a special firmware
version where the firmware is only loaded but not brought out of reset.
In this case Linux is responsible to de-assert the reset line.
In follow-up patches I add that to the outer qcom,rpm-proc node:

	remoteproc {
		compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc";
		resets = <&gcc GCC_RPM_RESET>;
		iommus = <&apps_smmu 0x0040>;

		smd-edge {
			/* ... */
			rpm-requests {
				qcom,smd-channels = "rpm_requests";
			};
		};
	};

This reset line cannot be described on the rpm-requests node. Until the
firmware is started there is no such thing as "SMD" or "rpm-requests".

When the RPM firmware is started it sets up some data structures in the
message RAM and then begins serving requests, perhaps like this:

               +----------------------------------+                                    
               |          ARM Cortex-M3           |                                    
               | +------------------------------+ |  +--------------------------------+
               | |  RPM Firmware                | |  |          Message RAM           |
 IPC Interrupt | | +----------------------+     | |  | +----------------------------+ |
------------------>| SMD Server           |     | |  | | SMD Data Structures & FIFO | |
               | | | +--------------+     |     | |  | | +--------------+           | |
               | | | | rpm_requests | ... |     | |  | | | rpm_requests |    ...    | |
               | | | +--------------+     | ... | |  | | +--------------+           | |
               | | +----------------------+     | |  | +----------------------------+ |
               | +------------------------------+ |  |                ...             |
               +----------------------------------+  +--------------------------------+

The "smd-edge" node represents the SMD part and "rpm_requests" is one
of the communication channels that can be used to talk to the RPM firmware.

Everything below rpm-requests: clocks, regulators, power domains are
pure firmware constructions. They do not exist in the RPM hardware block,
the firmware just acts as a proxy to collect votes from different
clients and then configures the actual hardware.
 
> 
> > However, SMD/GLINK is just one of the communication channels to the RPM
> > firmware. For example, the MPM interrupt functionality provided by the
> > RPM does not use SMD/GLINK but writes directly to a special memory
> > region allocated by the RPM firmware in combination with a mailbox.
> > Currently there is no good place in the device tree to describe this
> > functionality. It doesn't belong below SMD/GLINK but it's not an
> > independent top-level device either.
> > 
> > Introduce a new "qcom,rpm-proc" compatible that allows describing the
> > RPM as a remote processor/subsystem like all others. The SMD/GLINK node
> > is moved to a "smd-edge"/"glink-edge" subnode consistent with other
> > existing bindings. Additional subnodes (e.g. interrupt-controller for
> > MPM, rpm-master-stats) can be also added there.
> 
> If this was about to stay, you should also update the qcom,smd.yaml, so
> there will be no duplication.
> 

qcom,smd.yaml is deprecated in the next patch (PATCH 07/14).

> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
> >  1 file changed, 125 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
> > new file mode 100644
> > index 000000000000..c06dd4f66503
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
> > @@ -0,0 +1,125 @@
> > [...]
> > +  interrupt-controller:
> > +    type: object
> > +    $ref: /schemas/interrupt-controller/qcom,mpm.yaml#
> > +    description:
> > +      MSM Power Manager (MPM) interrupt controller that monitors interrupts
> > +      when the system is asleep.
> 
> Isn't this a service of RPM?
> 
> > +
> > +  master-stats:
> > +    $ref: /schemas/soc/qcom/qcom,rpm-master-stats.yaml#
> > +    description:
> > +      Subsystem-level low-power mode statistics provided by RPM.
> 
> The same question.
> 

Yes, they are services of the RPM firmware. But they have no relation to
the SMD/GLINK channel.

To clarify this I extend my picture from above with MPM:

                 +----------------------------------+                                    
                 |          ARM Cortex-M3           |                                    
                 | +------------------------------+ |  +--------------------------------+
                 | |  RPM Firmware                | |  |          Message RAM           |
 IPC Interrupt 0 | | +----------------------+     | |  | +----------------------------+ |
 ------------------->| SMD Server           |     | |  | | SMD Data Structures & FIFO | |
                 | | | +--------------+     |     | |  | | +--------------+           | |
                 | | | | rpm_requests | ... |     | |  | | | rpm_requests |    ...    | |
                 | | | +--------------+     | ... | |  | | +--------------+           | |
                 | | +----------------------+     | |  | +----------------------------+ |
 IPC Interrupt 1 | | +----------------------+     | |  | +----------------------------+ |
 ------------------->| MPM virtualization   |<-----------| MPM register copy (vMPM)   | |
                 | | +----------------------+     | |  | +----------------------------+ |
                 | +-----------|------------------+ |  |              ...               |
                 +-------------v--------------------+  +--------------------------------+
                       +--------------+                                                  
                       | MPM Hardware |                                                  
                       +--------------+                                                  

As you can see, SMD and MPM are siblings. The MPM functionality
implemented by the RPM firmware is not a child of the SMD server.

It's a bit like a webserver and emailserver running on the same PC.
Two separate services accessible via different ports and protocols.

Thanks,
Stephan

NOTE: All of this is just my interpretation based on the public hints
that exist. I have no access to internal documentation or source code
that would prove that all of this is correct.
Krzysztof Kozlowski June 7, 2023, 8:32 a.m. UTC | #5
On 06/06/2023 10:55, Stephan Gerhold wrote:
> <On Tue, Jun 06, 2023 at 08:36:10AM +0200, Krzysztof Kozlowski wrote:
>> On 05/06/2023 09:08, Stephan Gerhold wrote:
>>> On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
>>> described as remote processors in the device tree, with a dedicated
>>> node where properties and services related to them can be described.
>>>
>>> The Resource Power Manager (RPM) is also such a subsystem, with a
>>> remote processor that is running a special firmware. Unfortunately,
>>> the RPM never got a dedicated node representing it properly in the
>>> device tree. Most of the RPM services are described below a top-level
>>> /smd or /rpm-glink node.
>>
>> Then what is rpm-requests? This is the true RPM. It looks like you now
>> duplicate half of it in a node above. Unless you want here to describe
>> ways to communicate with the RPM, not the RPM itself.
>>
> 
> I think you're confusing hardware and firmware here. The rpm-proc node
> represents the RPM hardware block (or perhaps the RPM firmware overall),
> while rpm-requests is just *one* communication interface provided by the
> RPM firmware. Here is a rough picture of the RPM "subsystem" I'm trying
> to describe:
> 
>                 +--------------------------------------------+      
>                 |       RPM subsystem (qcom,rpm-proc)        |      
>                 |                                            |      
>           reset | +---------------+     +-----+  +-----+     |      
>         --------->|               |     | MPM |  | CPR | ... |      
>  IPC interrupts | | ARM Cortex-M3 |     +-----+  +-----+     |      
> ----------------->|               |---     |        |        |      
>                 | +---------------+  |---------------------- |      
>                 | +---------------+  |                       |      
>                 | |   Code RAM    |--|  +------------------+ |      
>                 | +---------------+  |  |                  | |      
>                 | +---------------+  |  |   Message RAM    | |      
>                 | |   Data RAM    |--|--|                  | |      
>                 | +---------------+  |  +------------------+ |      
>                 +--------------------|-----------------------+      
>                                      v                              
>                                     NoC                             
> 
> (Somewhat adapted from Figure 8-1 RPM overview in the APQ8016E Technical
>  Reference Manual, but I added some extra stuff.)
> 
> As you can see neither "SMD" nor "rpm-requests" exist in hardware.
> Again, it's just one communication protocol implemented by the RPM
> firmware running on the Cortex-M3 processor, much like a webserver
> running on a PC.
> 
> When the SoC is started only the hardware block exists. Usually a
> component in the boot chain loads firmware into the code/data RAM and
> then de-asserts the reset line to start the Cortex-M3 processor.
> 
> This is not guaranteed to happen. For example, I have a special firmware
> version where the firmware is only loaded but not brought out of reset.
> In this case Linux is responsible to de-assert the reset line.
> In follow-up patches I add that to the outer qcom,rpm-proc node:
> 
> 	remoteproc {
> 		compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc";
> 		resets = <&gcc GCC_RPM_RESET>;
> 		iommus = <&apps_smmu 0x0040>;
> 
> 		smd-edge {
> 			/* ... */
> 			rpm-requests {
> 				qcom,smd-channels = "rpm_requests";
> 			};
> 		};
> 	};
> 
> This reset line cannot be described on the rpm-requests node. Until the
> firmware is started there is no such thing as "SMD" or "rpm-requests".

OK, that makes sense, thank you for clarifying. It would be nice to
include it in the binding description (especially that you already wrote
it for me in the email :) ).

> 
> When the RPM firmware is started it sets up some data structures in the
> message RAM and then begins serving requests, perhaps like this:
> 
>                +----------------------------------+                                    
>                |          ARM Cortex-M3           |                                    
>                | +------------------------------+ |  +--------------------------------+
>                | |  RPM Firmware                | |  |          Message RAM           |
>  IPC Interrupt | | +----------------------+     | |  | +----------------------------+ |
> ------------------>| SMD Server           |     | |  | | SMD Data Structures & FIFO | |
>                | | | +--------------+     |     | |  | | +--------------+           | |
>                | | | | rpm_requests | ... |     | |  | | | rpm_requests |    ...    | |
>                | | | +--------------+     | ... | |  | | +--------------+           | |
>                | | +----------------------+     | |  | +----------------------------+ |
>                | +------------------------------+ |  |                ...             |
>                +----------------------------------+  +--------------------------------+
> 
> The "smd-edge" node represents the SMD part and "rpm_requests" is one
> of the communication channels that can be used to talk to the RPM firmware.
> 
> Everything below rpm-requests: clocks, regulators, power domains are
> pure firmware constructions. They do not exist in the RPM hardware block,
> the firmware just acts as a proxy to collect votes from different
> clients and then configures the actual hardware.

OK

>  
>>
>>> However, SMD/GLINK is just one of the communication channels to the RPM
>>> firmware. For example, the MPM interrupt functionality provided by the
>>> RPM does not use SMD/GLINK but writes directly to a special memory
>>> region allocated by the RPM firmware in combination with a mailbox.
>>> Currently there is no good place in the device tree to describe this
>>> functionality. It doesn't belong below SMD/GLINK but it's not an
>>> independent top-level device either.
>>>
>>> Introduce a new "qcom,rpm-proc" compatible that allows describing the
>>> RPM as a remote processor/subsystem like all others. The SMD/GLINK node
>>> is moved to a "smd-edge"/"glink-edge" subnode consistent with other
>>> existing bindings. Additional subnodes (e.g. interrupt-controller for
>>> MPM, rpm-master-stats) can be also added there.
>>
>> If this was about to stay, you should also update the qcom,smd.yaml, so
>> there will be no duplication.
>>
> 
> qcom,smd.yaml is deprecated in the next patch (PATCH 07/14).

Please squash it, because it is logically one change - you rework one
solution and deprecate other.

> 
>>>
>>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>>> ---
>>>  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 125 +++++++++++++++++++++
>>>  1 file changed, 125 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
>>> new file mode 100644
>>> index 000000000000..c06dd4f66503
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
>>> @@ -0,0 +1,125 @@
>>> [...]
>>> +  interrupt-controller:
>>> +    type: object
>>> +    $ref: /schemas/interrupt-controller/qcom,mpm.yaml#
>>> +    description:
>>> +      MSM Power Manager (MPM) interrupt controller that monitors interrupts
>>> +      when the system is asleep.
>>
>> Isn't this a service of RPM?
>>
>>> +
>>> +  master-stats:
>>> +    $ref: /schemas/soc/qcom/qcom,rpm-master-stats.yaml#
>>> +    description:
>>> +      Subsystem-level low-power mode statistics provided by RPM.
>>
>> The same question.
>>
> 
> Yes, they are services of the RPM firmware. But they have no relation to
> the SMD/GLINK channel.
> 
> To clarify this I extend my picture from above with MPM:
> 
>                  +----------------------------------+                                    
>                  |          ARM Cortex-M3           |                                    
>                  | +------------------------------+ |  +--------------------------------+
>                  | |  RPM Firmware                | |  |          Message RAM           |
>  IPC Interrupt 0 | | +----------------------+     | |  | +----------------------------+ |
>  ------------------->| SMD Server           |     | |  | | SMD Data Structures & FIFO | |
>                  | | | +--------------+     |     | |  | | +--------------+           | |
>                  | | | | rpm_requests | ... |     | |  | | | rpm_requests |    ...    | |
>                  | | | +--------------+     | ... | |  | | +--------------+           | |
>                  | | +----------------------+     | |  | +----------------------------+ |
>  IPC Interrupt 1 | | +----------------------+     | |  | +----------------------------+ |
>  ------------------->| MPM virtualization   |<-----------| MPM register copy (vMPM)   | |
>                  | | +----------------------+     | |  | +----------------------------+ |
>                  | +-----------|------------------+ |  |              ...               |
>                  +-------------v--------------------+  +--------------------------------+
>                        +--------------+                                                  
>                        | MPM Hardware |                                                  
>                        +--------------+                                                  
> 
> As you can see, SMD and MPM are siblings. The MPM functionality
> implemented by the RPM firmware is not a child of the SMD server.

OK, also please include it in the description.

> 
> It's a bit like a webserver and emailserver running on the same PC.
> Two separate services accessible via different ports and protocols.
> 
> Thanks,
> Stephan
> 
> NOTE: All of this is just my interpretation based on the public hints
> that exist. I have no access to internal documentation or source code
> that would prove that all of this is correct.

Sounds good enough for me :). Thank you.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
new file mode 100644
index 000000000000..c06dd4f66503
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
@@ -0,0 +1,125 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,rpm-proc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Resource Power Manager (RPM) Processor/Subsystem
+
+maintainers:
+  - Bjorn Andersson <andersson@kernel.org>
+  - Konrad Dybcio <konrad.dybcio@linaro.org>
+
+description:
+  Resource Power Manager (RPM) subsystem found in various Qualcomm platforms.
+  The RPM allows each component in the system to vote for state of the system
+  resources, such as clocks, regulators and bus frequencies. rpm-proc is the
+  top-level device tree node that groups all the RPM functionality together.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,apq8084-rpm-proc
+          - qcom,ipq6018-rpm-proc
+          - qcom,ipq9574-rpm-proc
+          - qcom,mdm9607-rpm-proc
+          - qcom,msm8226-rpm-proc
+          - qcom,msm8610-rpm-proc
+          - qcom,msm8909-rpm-proc
+          - qcom,msm8916-rpm-proc
+          - qcom,msm8917-rpm-proc
+          - qcom,msm8936-rpm-proc
+          - qcom,msm8937-rpm-proc
+          - qcom,msm8952-rpm-proc
+          - qcom,msm8953-rpm-proc
+          - qcom,msm8974-rpm-proc
+          - qcom,msm8976-rpm-proc
+          - qcom,msm8994-rpm-proc
+          - qcom,msm8996-rpm-proc
+          - qcom,msm8998-rpm-proc
+          - qcom,qcm2290-rpm-proc
+          - qcom,qcs404-rpm-proc
+          - qcom,sdm660-rpm-proc
+          - qcom,sm6115-rpm-proc
+          - qcom,sm6125-rpm-proc
+          - qcom,sm6375-rpm-proc
+      - const: qcom,rpm-proc
+
+  smd-edge:
+    $ref: /schemas/remoteproc/qcom,smd-edge.yaml#
+    description:
+      Qualcomm Shared Memory subnode which represents communication edge,
+      channels and devices related to the RPM subsystem.
+
+  glink-rpm:
+    $ref: /schemas/remoteproc/qcom,glink-rpm-edge.yaml#
+    description:
+      Qualcomm G-Link subnode which represents communication edge,
+      channels and devices related to the RPM subsystem.
+
+  interrupt-controller:
+    type: object
+    $ref: /schemas/interrupt-controller/qcom,mpm.yaml#
+    description:
+      MSM Power Manager (MPM) interrupt controller that monitors interrupts
+      when the system is asleep.
+
+  master-stats:
+    $ref: /schemas/soc/qcom/qcom,rpm-master-stats.yaml#
+    description:
+      Subsystem-level low-power mode statistics provided by RPM.
+
+required:
+  - compatible
+
+oneOf:
+  - required:
+      - smd-edge
+  - required:
+      - glink-rpm
+
+additionalProperties: false
+
+examples:
+  # SMD
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    remoteproc-rpm {
+      compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc";
+
+      smd-edge {
+        interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
+        qcom,ipc = <&apcs 8 0>;
+        qcom,smd-edge = <15>;
+
+        rpm-requests {
+          compatible = "qcom,rpm-msm8916";
+          qcom,smd-channels = "rpm_requests";
+          /* ... */
+        };
+      };
+    };
+  # GLINK
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    remoteproc-rpm {
+      compatible = "qcom,qcm2290-rpm-proc", "qcom,rpm-proc";
+
+      glink-rpm {
+        compatible = "qcom,glink-rpm";
+        interrupts = <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>;
+        qcom,rpm-msg-ram = <&rpm_msg_ram>;
+        mboxes = <&apcs_glb 0>;
+
+        rpm-requests {
+          compatible = "qcom,rpm-qcm2290";
+          qcom,glink-channels = "rpm_requests";
+          /* ... */
+        };
+      };
+    };