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

From: Krzysztof Kozlowski
Date: Tue Jun 06 2023 - 02:36:25 EST

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@xxxxxxxxxxx>
> ---
> .../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@xxxxxxxxxx>
> + - Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> +
> +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