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

Stephan Gerhold posted 14 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Stephan Gerhold 2 years, 3 months ago
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(+)

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";
+          /* ... */
+        };
+      };
+    };

-- 
2.40.1
Re: [PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Krzysztof Kozlowski 2 years, 3 months ago
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
Re: [PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Stephan Gerhold 2 years, 3 months ago
<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.
Re: [PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Krzysztof Kozlowski 2 years, 3 months ago
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
Re: [PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Rob Herring 2 years, 3 months ago
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.
Re: [PATCH 05/14] dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
Posted by Stephan Gerhold 2 years, 3 months ago
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