| Message ID | 20230531-rpm-rproc-v1-0-e0a3b6de1f14@gerhold.net |
|---|---|
| Headers | show |
| Series | Add dedicated device tree node for RPM processor/subsystem | expand |
On Mon, 05 Jun 2023 09:08:22 +0200, Stephan Gerhold wrote: > Use the new top-level rpm-proc node instead of having a dummy top-level > /smd node that only contains the RPM but not other remote processors. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- > 1 file changed, 3 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/soc/qcom/qcom,smd-rpm.example.dtb: /example-0/remoteproc-rpm: failed to match any schema with compatible: ['qcom,msm8916-rpm-proc', 'qcom,rpm-proc'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230531-rpm-rproc-v1-6-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.
On Mon, Jun 05, 2023 at 02:33:58AM -0600, Rob Herring wrote: > On Mon, 05 Jun 2023 09:08:22 +0200, Stephan Gerhold wrote: > > Use the new top-level rpm-proc node instead of having a dummy top-level > > /smd node that only contains the RPM but not other remote processors. > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > --- > > Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- > > 1 file changed, 3 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/soc/qcom/qcom,smd-rpm.example.dtb: /example-0/remoteproc-rpm: failed to match any schema with compatible: ['qcom,msm8916-rpm-proc', 'qcom,rpm-proc'] > Huh? The schema that matches this compatible is in the previous patch. :) Perhaps this error is related to the dt_binding_check problem on the patch before (which is caused by applying the patches to the wrong base branch). 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
On 5.06.2023 09:08, Stephan Gerhold wrote: > Avoid having to look up a dummy item from SMEM to detect if it is > already available or if we need to defer probing. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > drivers/soc/qcom/smem.c | 9 +++++++++ > include/linux/soc/qcom/smem.h | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index b0d59e815c3b..3d93a6681494 100644 > --- a/drivers/soc/qcom/smem.c > +++ b/drivers/soc/qcom/smem.c > @@ -359,6 +359,15 @@ static struct qcom_smem *__smem; > /* Timeout (ms) for the trylock of remote spinlocks */ > #define HWSPINLOCK_TIMEOUT 1000 > > +/** > + * qcom_smem_is_available() - Checks if SMEM is available > + */ Shouldn't kerneldoc explicitly say "returns x if y else z"? Modulo that: Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > +bool qcom_smem_is_available(void) > +{ > + return !!__smem; > +} > +EXPORT_SYMBOL(qcom_smem_is_available); > + > static int qcom_smem_alloc_private(struct qcom_smem *smem, > struct smem_partition *part, > unsigned item, > diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h > index 223db6a9c733..a36a3b9d4929 100644 > --- a/include/linux/soc/qcom/smem.h > +++ b/include/linux/soc/qcom/smem.h > @@ -4,6 +4,7 @@ > > #define QCOM_SMEM_HOST_ANY -1 > > +bool qcom_smem_is_available(void); > int qcom_smem_alloc(unsigned host, unsigned item, size_t size); > void *qcom_smem_get(unsigned host, unsigned item, size_t *size); > >
On 5.06.2023 09:08, Stephan Gerhold wrote: > The "smd-edge"s for remote processors are typically specified below the > remoteproc nodes. For some reason apq8064 also has them all listed in a > top-level /smd node, disabled by default. None of the boards enable them. > > Right now apq8064 only has support for WCNSS/riva, but there the > smd-edge is already defined with the same interrupt etc below the > riva-pil node. > > Drop these redundant definitions since the /smd top-level node is now > deprecated. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- Hm, 2012 is calling.. it wants it dead code back! Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > arch/arm/boot/dts/qcom-apq8064.dtsi | 40 ------------------------------------- > 1 file changed, 40 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi > index d2289205ff81..e0adf237fc5c 100644 > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi > @@ -226,46 +226,6 @@ smem { > hwlocks = <&sfpb_mutex 3>; > }; > > - smd { > - compatible = "qcom,smd"; > - > - modem-edge { > - interrupts = <0 37 IRQ_TYPE_EDGE_RISING>; > - > - qcom,ipc = <&l2cc 8 3>; > - qcom,smd-edge = <0>; > - > - status = "disabled"; > - }; > - > - q6-edge { > - interrupts = <0 90 IRQ_TYPE_EDGE_RISING>; > - > - qcom,ipc = <&l2cc 8 15>; > - qcom,smd-edge = <1>; > - > - status = "disabled"; > - }; > - > - dsps-edge { > - interrupts = <0 138 IRQ_TYPE_EDGE_RISING>; > - > - qcom,ipc = <&sps_sic_non_secure 0x4080 0>; > - qcom,smd-edge = <3>; > - > - status = "disabled"; > - }; > - > - riva-edge { > - interrupts = <0 198 IRQ_TYPE_EDGE_RISING>; > - > - qcom,ipc = <&l2cc 8 25>; > - qcom,smd-edge = <6>; > - > - status = "disabled"; > - }; > - }; > - > smsm { > compatible = "qcom,smsm"; > >
On 05/06/2023 09:08, Stephan Gerhold wrote: > Some of the enum entries are not properly ordered, fix that. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 05/06/2023 09:08, Stephan Gerhold wrote: > MSM8909 is using qcom,smd-channels but is missing in the list, add it > there as well. > > Fixes: 709d473dd5e1 ("dt-bindings: soc: qcom: smd-rpm: Add MSM8909") > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 05/06/2023 09:08, Stephan Gerhold wrote: > To avoid several more small patches adding new RPM compatibles in the > future, add MDM9607, MSM8610, MSM8917, MSM8937 and MSM8952 at once. > All of these have been worked on over the time by some people and are > definitely compatible as-is with the smd-rpm driver. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
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
On 05/06/2023 09:08, Stephan Gerhold wrote: > Use the new top-level rpm-proc node instead of having a dummy top-level > /smd node that only contains the RPM but not other remote processors. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > index c6930706bfa9..06e574239bd4 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > @@ -120,10 +120,10 @@ examples: > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/interrupt-controller/irq.h> > > - smd { > - compatible = "qcom,smd"; > + remoteproc-rpm { remoteproc > + compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc"; > > - rpm { > + smd-edge { What about binding updates? Anyway, this should be squashed with previous one. > interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>; > qcom,ipc = <&apcs 8 0>; > qcom,smd-edge = <15>; > Best regards, Krzysztof
<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.
On Tue, Jun 06, 2023 at 08:37:04AM +0200, Krzysztof Kozlowski wrote: > On 05/06/2023 09:08, Stephan Gerhold wrote: > > Use the new top-level rpm-proc node instead of having a dummy top-level > > /smd node that only contains the RPM but not other remote processors. > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > --- > > Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > > index c6930706bfa9..06e574239bd4 100644 > > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml > > @@ -120,10 +120,10 @@ examples: > > [...] > > + compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc"; > > > > - rpm { > > + smd-edge { > > What about binding updates? > The binding for this is in PATCH 05/14. The old binding replaced here is deprecated in PATCH 07/14. > Anyway, this should be squashed with previous one. > Sure, I can squash in v2. Thanks, Stephan
On 06/06/2023 11:06, Stephan Gerhold wrote: > On Tue, Jun 06, 2023 at 08:37:04AM +0200, Krzysztof Kozlowski wrote: >> On 05/06/2023 09:08, Stephan Gerhold wrote: >>> Use the new top-level rpm-proc node instead of having a dummy top-level >>> /smd node that only contains the RPM but not other remote processors. >>> >>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net> >>> --- >>> Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml >>> index c6930706bfa9..06e574239bd4 100644 >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd-rpm.yaml >>> @@ -120,10 +120,10 @@ examples: >>> [...] >>> + compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc"; >>> >>> - rpm { >>> + smd-edge { >> >> What about binding updates? >> > > The binding for this is in PATCH 05/14. The old binding replaced here is > deprecated in PATCH 07/14. So changing example without changing binding is not an atomic change. Best regards, Krzysztof
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
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> --- Stephan Gerhold (14): 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 dt-bindings: soc: qcom: smd-rpm: Use qcom,rpm-proc in example dt-bindings: qcom: smd: Mark as deprecated 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 | 125 +++++++++++++++++ .../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/Makefile | 2 +- drivers/soc/qcom/rpm-proc.c | 76 +++++++++++ drivers/soc/qcom/smd-rpm.c | 35 ++--- drivers/soc/qcom/smem.c | 9 ++ include/linux/soc/qcom/smem.h | 1 + 28 files changed, 1111 insertions(+), 901 deletions(-) --- base-commit: 8d5a57ea6a0b1722725170e32e511701ca7c454c change-id: 20230531-rpm-rproc-758364839cdd Best regards,