mbox series

[v2,00/12] Add dedicated device tree node for RPM processor/subsystem

Message ID 20230531-rpm-rproc-v2-0-56a4a00c8260@gerhold.net
Headers show
Series Add dedicated device tree node for RPM processor/subsystem | expand

Message

Stephan Gerhold June 8, 2023, 7:10 a.m. UTC
The Resource Power Manager (RPM) currently does not have a dedicated 
device tree node that represents the remoteproc/subsystem. The 
functionality exposed through the SMD/GLINK channels is described in 
top-level nodes of the device tree. This makes it hard to group other 
functionality provided by the RPM together in the device tree. This 
series adds a single top-level remoteproc-rpm/rpm-proc device tree node 
that groups all RPM functionality together.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v2:
- Pick up review/test tags from Konrad and Krzysztof
- Rename "remoteproc-rpm" -> "remoteproc" everywhere (Krzysztof/Konrad)
- "dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem"
  - Squash with other dt-bindings changes to have atomic refactoring (Krzysztof)
  - Add diagrams from discussion as clarification
- "soc: qcom: smem: Add qcom_smem_is_available()"
  - Add return documentation in qcom_smem_is_available() (Konrad)
- "soc: qcom: Add RPM processor/subsystem driver"
  - Add missing of_node_put(), fix children (Konrad)
  - Add depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n to fix build 
    error in weird kernel configurations (kernel test robot)
- Link to v1: https://lore.kernel.org/r/20230531-rpm-rproc-v1-0-e0a3b6de1f14@gerhold.net

---
Stephan Gerhold (12):
      dt-bindings soc: qcom: smd-rpm: Fix sort order
      dt-bindings: soc: qcom: smd-rpm: Add MSM8909 to qcom,smd-channels
      dt-bindings: soc: qcom: smd-rpm: Add some more compatibles
      soc: qcom: smd-rpm: Match rpmsg channel instead of compatible
      dt-bindings: remoteproc: Add Qualcomm RPM processor/subsystem
      soc: qcom: smem: Add qcom_smem_is_available()
      rpmsg: qcom_smd: Use qcom_smem_is_available()
      soc: qcom: Add RPM processor/subsystem driver
      arm64: dts: qcom: Add rpm-proc node for SMD platforms
      arm64: dts: qcom: Add rpm-proc node for GLINK gplatforms
      ARM: dts: qcom: Add rpm-proc node for SMD platforms
      ARM: dts: qcom: apq8064: Drop redundant /smd node

 .../bindings/remoteproc/qcom,rpm-proc.yaml         | 171 +++++++++++++++++++++
 .../devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml |  23 ++-
 .../devicetree/bindings/soc/qcom/qcom,smd.yaml     |   7 +
 arch/arm/boot/dts/qcom-apq8064.dtsi                |  40 -----
 arch/arm/boot/dts/qcom-apq8084.dtsi                |   6 +-
 arch/arm/boot/dts/qcom-msm8226.dtsi                |  38 ++---
 arch/arm/boot/dts/qcom-msm8974.dtsi                |  44 +++---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              |  48 +++---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi              |  28 ++--
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |   6 +-
 arch/arm64/boot/dts/qcom/msm8939.dtsi              | 112 +++++++-------
 arch/arm64/boot/dts/qcom/msm8953.dtsi              | 136 ++++++++--------
 arch/arm64/boot/dts/qcom/msm8976.dtsi              | 152 +++++++++---------
 arch/arm64/boot/dts/qcom/msm8994.dtsi              |  99 ++++++------
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 113 +++++++-------
 arch/arm64/boot/dts/qcom/msm8998.dtsi              |  98 ++++++------
 arch/arm64/boot/dts/qcom/qcm2290.dtsi              | 126 +++++++--------
 arch/arm64/boot/dts/qcom/qcs404.dtsi               | 152 +++++++++---------
 arch/arm64/boot/dts/qcom/sdm630.dtsi               | 132 ++++++++--------
 arch/arm64/boot/dts/qcom/sm6115.dtsi               | 128 +++++++--------
 arch/arm64/boot/dts/qcom/sm6125.dtsi               | 140 +++++++++--------
 arch/arm64/boot/dts/qcom/sm6375.dtsi               | 126 +++++++--------
 drivers/rpmsg/qcom_smd.c                           |  10 +-
 drivers/soc/qcom/Kconfig                           |   1 +
 drivers/soc/qcom/Makefile                          |   2 +-
 drivers/soc/qcom/rpm-proc.c                        |  77 ++++++++++
 drivers/soc/qcom/smd-rpm.c                         |  35 ++---
 drivers/soc/qcom/smem.c                            |  11 ++
 include/linux/soc/qcom/smem.h                      |   1 +
 29 files changed, 1161 insertions(+), 901 deletions(-)
---
base-commit: 8d5a57ea6a0b1722725170e32e511701ca7c454c
change-id: 20230531-rpm-rproc-758364839cdd

Best regards,

Comments

Rob Herring (Arm) June 8, 2023, 8:27 a.m. UTC | #1
On Thu, 08 Jun 2023 09:10:25 +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.
> 
> Deprecate using the old top-level /smd node since all SMD edges
> are now specified as subnodes of the remote processor.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> This patch is based on qcom/for-next, since it needs the new
> qcom,rpm-master-stats.yaml schema that is only applied there.
> ---
>  .../bindings/remoteproc/qcom,rpm-proc.yaml         | 171 +++++++++++++++++++++
>  .../devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml |   6 +-
>  .../devicetree/bindings/soc/qcom/qcom,smd.yaml     |   7 +
>  3 files changed, 181 insertions(+), 3 deletions(-)
> 

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-v2-5-56a4a00c8260@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.
Konrad Dybcio June 9, 2023, 3:26 p.m. UTC | #2
On 8.06.2023 09:10, Stephan Gerhold wrote:
> Rather than having the RPM SMD channels as the only child of a dummy
> SMD node, switch to representing the RPM as remoteproc like all the
> other remoteprocs (WCNSS, modem DSP).
> 
> This allows assigning additional subdevices to it like the MPM
> interrupt-controller or rpm-master-stats.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm/boot/dts/qcom-apq8084.dtsi |  6 ++---
>  arch/arm/boot/dts/qcom-msm8226.dtsi | 38 ++++++++++++++++----------------
>  arch/arm/boot/dts/qcom-msm8974.dtsi | 44 ++++++++++++++++++-------------------
>  3 files changed, 44 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
> index 8f178bc87e1d..2b1f9d0fb510 100644
> --- a/arch/arm/boot/dts/qcom-apq8084.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
> @@ -784,10 +784,10 @@ spmi_bus: spmi@fc4cf000 {
>  		};
>  	};
>  
> -	smd {
> -		compatible = "qcom,smd";
> +	rpm: remoteproc {
> +		compatible = "qcom,apq8084-rpm-proc", "qcom,rpm-proc";
>  
> -		rpm {
> +		smd-edge {
>  			interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
>  			qcom,ipc = <&apcs 8 0>;
>  			qcom,smd-edge = <15>;
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
> index a3a9162e9c28..a3e8d023d0e6 100644
> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -53,26 +53,10 @@ pmu {
>  					 IRQ_TYPE_LEVEL_HIGH)>;
>  	};
>  
> -	reserved-memory {
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -		ranges;
> -
> -		smem_region: smem@3000000 {
> -			reg = <0x3000000 0x100000>;
> -			no-map;
> -		};
> -
> -		adsp_region: adsp@dc00000 {
> -			reg = <0x0dc00000 0x1900000>;
> -			no-map;
> -		};
> -	};
> -
> -	smd {
> -		compatible = "qcom,smd";
> +	rpm: remoteproc {
> +		compatible = "qcom,msm8226-rpm-proc", "qcom,rpm-proc";
>  
> -		rpm {
> +		smd-edge {
>  			interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
>  			qcom,ipc = <&apcs 8 0>;
>  			qcom,smd-edge = <15>;
> @@ -120,6 +104,22 @@ rpmpd_opp_super_turbo: opp6 {
>  		};
>  	};
>  
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		smem_region: smem@3000000 {
> +			reg = <0x3000000 0x100000>;
> +			no-map;
> +		};
> +
> +		adsp_region: adsp@dc00000 {
> +			reg = <0x0dc00000 0x1900000>;
> +			no-map;
> +		};
> +	};
> +
>  	smem {
>  		compatible = "qcom,smem";
>  
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 58e144957c5d..0a5b5ecb5dfa 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -113,6 +113,28 @@ pmu {
>  		interrupts = <GIC_PPI 7 0xf04>;
>  	};
>  
> +	rpm: remoteproc {
> +		compatible = "qcom,msm8974-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: rpm-requests {
> +				compatible = "qcom,rpm-msm8974";
> +				qcom,smd-channels = "rpm_requests";
> +
> +				rpmcc: clock-controller {
> +					compatible = "qcom,rpmcc-msm8974", "qcom,rpmcc";
> +					#clock-cells = <1>;
> +					clocks = <&xo_board>;
> +					clock-names = "xo";
> +				};
> +			};
> +		};
> +	};
> +
>  	reserved-memory {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> @@ -293,28 +315,6 @@ wcnss_smsm: wcnss@7 {
>  		};
>  	};
>  
> -	smd {
> -		compatible = "qcom,smd";
> -
> -		rpm {
> -			interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> -			qcom,ipc = <&apcs 8 0>;
> -			qcom,smd-edge = <15>;
> -
> -			rpm_requests: rpm-requests {
> -				compatible = "qcom,rpm-msm8974";
> -				qcom,smd-channels = "rpm_requests";
> -
> -				rpmcc: clock-controller {
> -					compatible = "qcom,rpmcc-msm8974", "qcom,rpmcc";
> -					#clock-cells = <1>;
> -					clocks = <&xo_board>;
> -					clock-names = "xo";
> -				};
> -			};
> -		};
> -	};
> -
>  	soc: soc {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>
Krzysztof Kozlowski June 10, 2023, 4:43 p.m. UTC | #3
On 08/06/2023 09:10, 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.


Thank you for your patch. There is something to discuss/improve.


> +
> +  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:

This should be "glink-edge", to be a bit more generic and match existing
smd-edge.

> +    $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
> +


Best regards,
Krzysztof
Stephan Gerhold June 10, 2023, 4:52 p.m. UTC | #4
On Sat, Jun 10, 2023 at 06:43:17PM +0200, Krzysztof Kozlowski wrote:
> On 08/06/2023 09:10, 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.
> > +
> > +  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:
> 
> This should be "glink-edge", to be a bit more generic and match existing
> smd-edge.
> 

I fully agree and I actually made that change at some point before
sending v1. Unfortunately, it doesn't work: The node name "glink-edge"
is already reserved by qcom,glink-edge.yaml. While it's very similar it
has some subtle differences to glink-rpm-edge.yaml. For example, there
is no qcom,remote-pid in the RPM variant which is required by the normal
glink-edge.

Would "glink-rpm-edge" sound better?

Thanks,
Stephan
Krzysztof Kozlowski June 12, 2023, 6:19 p.m. UTC | #5
On 10/06/2023 18:52, Stephan Gerhold wrote:
> On Sat, Jun 10, 2023 at 06:43:17PM +0200, Krzysztof Kozlowski wrote:
>> On 08/06/2023 09:10, 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.
>>> +
>>> +  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:
>>
>> This should be "glink-edge", to be a bit more generic and match existing
>> smd-edge.
>>
> 
> I fully agree and I actually made that change at some point before
> sending v1. Unfortunately, it doesn't work: The node name "glink-edge"
> is already reserved by qcom,glink-edge.yaml. While it's very similar it
> has some subtle differences to glink-rpm-edge.yaml. For example, there
> is no qcom,remote-pid in the RPM variant which is required by the normal
> glink-edge.

But the other variant has, so they are pretty similar. It could be one
binding or some common part. Anyway we can as well drop the nodename
from the qcom,glink-edge.yaml binding. Anyway the binding is referenced
by each specific remote proc, so this nodename brings nothing.

> Would "glink-rpm-edge" sound better?
> 
> Thanks,
> Stephan
> 

Best regards,
Krzysztof