[PATCH v8 1/5] dt-bindings: Introduce SoC sleep stats bindings

[PATCH v8 1/5] dt-bindings: Introduce SoC sleep stats bindings

[Maulik Shah]

From: Mahesh Sivasubramanian <msivasub@codeaurora.org>

Add device binding documentation for Qualcomm Technologies, Inc. (QTI)
SoC sleep stats driver. The driver is used for displaying SoC sleep
statistic maintained by Always On Processor or Resource Power Manager.

Cc: devicetree@vger.kernel.org
Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 .../bindings/soc/qcom/soc-sleep-stats.yaml         | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml

diff --git a/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
new file mode 100644
index 0000000..9078c4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/qcom/soc-sleep-stats.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. (QTI) SoC sleep stats bindings
+
+maintainers:
+  - Maulik Shah <mkshah@codeaurora.org>
+  - Lina Iyer <ilina@codeaurora.org>
+
+description:
+  Always On Processor/Resource Power Manager maintains statistics of the SoC
+  sleep modes involving powering down of the rails and oscillator clock.
+
+  Statistics includes SoC sleep mode type, number of times low power mode were
+  entered, time of last entry, time of last exit and accumulated sleep duration.
+
+properties:
+  compatible:
+    enum:
+      - qcom,rpmh-sleep-stats
+      - qcom,rpm-sleep-stats
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  # Example of rpmh sleep stats
+  - |
+    rpmh-sleep-stats@c3f0000 {
+      compatible = "qcom,rpmh-sleep-stats";
+      reg = <0x0c3f0000 0x400>;
+    };
+  # Example of rpm sleep stats
+  - |
+    rpm-sleep-stats@4690000 {
+      compatible = "qcom,rpm-sleep-stats";
+      reg = <0x04690000 0x400>;
+    };
+...
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Maulik Shah]

Add device node for SoC sleep stats driver which provides various
low power mode stats.

Also update the reg size of aoss_qmp device to 0x400.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 6228ba2..889d04d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -3215,7 +3215,7 @@
 
 		aoss_qmp: power-controller@c300000 {
 			compatible = "qcom,sc7180-aoss-qmp";
-			reg = <0 0x0c300000 0 0x100000>;
+			reg = <0 0x0c300000 0 0x400>;
 			interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
 			mboxes = <&apss_shared 0>;
 
@@ -3223,6 +3223,11 @@
 			#power-domain-cells = <1>;
 		};
 
+		rpmh-sleep-stats@c3f0000 {
+			compatible = "qcom,rpmh-sleep-stats";
+			reg = <0 0x0c3f0000 0 0x400>;
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0 0x0c440000 0 0x1100>,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v8 5/5] arm64: dts: qcom: sc7280: Enable SoC sleep stats

[Maulik Shah]

Add device node for SoC sleep stats driver which provides various
low power mode stats.

Also update the reg size of aoss_qmp device to 0x400.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 2cc4785..47afca8 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -827,7 +827,7 @@
 
 		aoss_qmp: power-controller@c300000 {
 			compatible = "qcom,sc7280-aoss-qmp";
-			reg = <0 0x0c300000 0 0x100000>;
+			reg = <0 0x0c300000 0 0x400>;
 			interrupts-extended = <&ipcc IPCC_CLIENT_AOP
 						     IPCC_MPROC_SIGNAL_GLINK_QMP
 						     IRQ_TYPE_EDGE_RISING>;
@@ -838,6 +838,11 @@
 			#power-domain-cells = <1>;
 		};
 
+		rpmh-sleep-stats@c3f0000 {
+			compatible = "qcom,rpmh-sleep-stats";
+			reg = <0 0x0c3f0000 0 0x400>;
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0 0x0c440000 0 0x1100>,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Stephen Boyd]

Quoting Maulik Shah (2021-05-21 04:26:09)
> Add device node for SoC sleep stats driver which provides various
> low power mode stats.
>
> Also update the reg size of aoss_qmp device to 0x400.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 6228ba2..889d04d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -3215,7 +3215,7 @@
>
>                 aoss_qmp: power-controller@c300000 {
>                         compatible = "qcom,sc7180-aoss-qmp";
> -                       reg = <0 0x0c300000 0 0x100000>;
> +                       reg = <0 0x0c300000 0 0x400>;
>                         interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
>                         mboxes = <&apss_shared 0>;
>
> @@ -3223,6 +3223,11 @@
>                         #power-domain-cells = <1>;
>                 };
>
> +               rpmh-sleep-stats@c3f0000 {
> +                       compatible = "qcom,rpmh-sleep-stats";
> +                       reg = <0 0x0c3f0000 0 0x400>;
> +               };
> +

Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
aux-bus and stick the sleep stats device on there?

>                 spmi_bus: spmi@c440000 {
>                         compatible = "qcom,spmi-pmic-arb";
>                         reg = <0 0x0c440000 0 0x1100>,

Re: [PATCH v8 5/5] arm64: dts: qcom: sc7280: Enable SoC sleep stats

[Stephen Boyd]

Quoting Maulik Shah (2021-05-21 04:26:11)
> Add device node for SoC sleep stats driver which provides various
> low power mode stats.
>
> Also update the reg size of aoss_qmp device to 0x400.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Same question as in sc7180.

>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 2cc4785..47afca8 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -827,7 +827,7 @@
>
>                 aoss_qmp: power-controller@c300000 {
>                         compatible = "qcom,sc7280-aoss-qmp";
> -                       reg = <0 0x0c300000 0 0x100000>;
> +                       reg = <0 0x0c300000 0 0x400>;
>                         interrupts-extended = <&ipcc IPCC_CLIENT_AOP
>                                                      IPCC_MPROC_SIGNAL_GLINK_QMP
>                                                      IRQ_TYPE_EDGE_RISING>;
> @@ -838,6 +838,11 @@
>                         #power-domain-cells = <1>;
>                 };
>
> +               rpmh-sleep-stats@c3f0000 {
> +                       compatible = "qcom,rpmh-sleep-stats";
> +                       reg = <0 0x0c3f0000 0 0x400>;
> +               };
> +
>                 spmi_bus: spmi@c440000 {
>                         compatible = "qcom,spmi-pmic-arb";
>                         reg = <0 0x0c440000 0 0x1100>,
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Bjorn Andersson]

On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:

> Quoting Maulik Shah (2021-05-21 04:26:09)
> > Add device node for SoC sleep stats driver which provides various
> > low power mode stats.
> >
> > Also update the reg size of aoss_qmp device to 0x400.
> >
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > index 6228ba2..889d04d 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > @@ -3215,7 +3215,7 @@
> >
> >                 aoss_qmp: power-controller@c300000 {
> >                         compatible = "qcom,sc7180-aoss-qmp";
> > -                       reg = <0 0x0c300000 0 0x100000>;
> > +                       reg = <0 0x0c300000 0 0x400>;
> >                         interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
> >                         mboxes = <&apss_shared 0>;
> >
> > @@ -3223,6 +3223,11 @@
> >                         #power-domain-cells = <1>;
> >                 };
> >
> > +               rpmh-sleep-stats@c3f0000 {
> > +                       compatible = "qcom,rpmh-sleep-stats";
> > +                       reg = <0 0x0c3f0000 0 0x400>;
> > +               };
> > +
> 
> Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> aux-bus and stick the sleep stats device on there?
> 

The AOSS memory space has N chunks of "message ram", one is used for the
QMP protocol (presumably the APSS specific one), a different one is used
for the sleep stats.

I presume we could have come up with a binding for the entire AOSS/AOP
and then describe (either implicit or explicitly) the QMP and
debug-stats under that.

But we'd also have to come up with the same container-device for the RPM
case.

Regards,
Bjorn

> >                 spmi_bus: spmi@c440000 {
> >                         compatible = "qcom,spmi-pmic-arb";
> >                         reg = <0 0x0c440000 0 0x1100>,

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Stephen Boyd]

Quoting Bjorn Andersson (2021-05-31 10:57:03)
> On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:
>
> > Quoting Maulik Shah (2021-05-21 04:26:09)
> > > Add device node for SoC sleep stats driver which provides various
> > > low power mode stats.
> > >
> > > Also update the reg size of aoss_qmp device to 0x400.
> > >
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > index 6228ba2..889d04d 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > @@ -3215,7 +3215,7 @@
> > >
> > >                 aoss_qmp: power-controller@c300000 {
> > >                         compatible = "qcom,sc7180-aoss-qmp";
> > > -                       reg = <0 0x0c300000 0 0x100000>;
> > > +                       reg = <0 0x0c300000 0 0x400>;
> > >                         interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
> > >                         mboxes = <&apss_shared 0>;
> > >
> > > @@ -3223,6 +3223,11 @@
> > >                         #power-domain-cells = <1>;
> > >                 };
> > >
> > > +               rpmh-sleep-stats@c3f0000 {
> > > +                       compatible = "qcom,rpmh-sleep-stats";
> > > +                       reg = <0 0x0c3f0000 0 0x400>;
> > > +               };
> > > +
> >
> > Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> > aux-bus and stick the sleep stats device on there?
> >
>
> The AOSS memory space has N chunks of "message ram", one is used for the
> QMP protocol (presumably the APSS specific one), a different one is used
> for the sleep stats.
>
> I presume we could have come up with a binding for the entire AOSS/AOP
> and then describe (either implicit or explicitly) the QMP and
> debug-stats under that.
>
> But we'd also have to come up with the same container-device for the RPM
> case.

Because the rpm node doesn't include this region of memory today? I
still fail to see why we're changing the existing binding and adding a
DT node for this new region that is basically a debug feature.

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Bjorn Andersson]

On Wed 02 Jun 19:26 CDT 2021, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2021-05-31 10:57:03)
> > On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:
> >
> > > Quoting Maulik Shah (2021-05-21 04:26:09)
> > > > Add device node for SoC sleep stats driver which provides various
> > > > low power mode stats.
> > > >
> > > > Also update the reg size of aoss_qmp device to 0x400.
> > > >
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> > > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > > index 6228ba2..889d04d 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > > @@ -3215,7 +3215,7 @@
> > > >
> > > >                 aoss_qmp: power-controller@c300000 {
> > > >                         compatible = "qcom,sc7180-aoss-qmp";
> > > > -                       reg = <0 0x0c300000 0 0x100000>;
> > > > +                       reg = <0 0x0c300000 0 0x400>;
> > > >                         interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
> > > >                         mboxes = <&apss_shared 0>;
> > > >
> > > > @@ -3223,6 +3223,11 @@
> > > >                         #power-domain-cells = <1>;
> > > >                 };
> > > >
> > > > +               rpmh-sleep-stats@c3f0000 {
> > > > +                       compatible = "qcom,rpmh-sleep-stats";
> > > > +                       reg = <0 0x0c3f0000 0 0x400>;
> > > > +               };
> > > > +
> > >
> > > Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> > > aux-bus and stick the sleep stats device on there?
> > >
> >
> > The AOSS memory space has N chunks of "message ram", one is used for the
> > QMP protocol (presumably the APSS specific one), a different one is used
> > for the sleep stats.
> >
> > I presume we could have come up with a binding for the entire AOSS/AOP
> > and then describe (either implicit or explicitly) the QMP and
> > debug-stats under that.
> >
> > But we'd also have to come up with the same container-device for the RPM
> > case.
> 
> Because the rpm node doesn't include this region of memory today? I
> still fail to see why we're changing the existing binding and adding a
> DT node for this new region that is basically a debug feature.

We're not changing the binding, the memory region for the "AOSS QMP"
thing was never larger than 0x400.

0x100000 is the size of all the AOSS "msg_ram" regions. We don't have
this whole thing described in a binding and we don't have an
implementation for the whole thing.

If we're going for that we'd need to extend the binding to indicate
which of the msg_ram regions are used for APSS QMP and for debug stats
on particular platform (either by compatible, explicit properties or as
some subnodes).


That said, as I looked into my other objection, for the RPM
(non-hardened) case it seems that we're actually describing the RPM
region. So there it would make sense to describe it as such in DT - but
we don't have any other code (that I'm aware of) that would implement
the "qcom,<platform>-rpm".

Regards,
Bjorn

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Stephen Boyd]

Quoting Bjorn Andersson (2021-06-02 19:44:40)
> On Wed 02 Jun 19:26 CDT 2021, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2021-05-31 10:57:03)
> > > On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:
> > >
> > > > Quoting Maulik Shah (2021-05-21 04:26:09)
> > > > > @@ -3223,6 +3223,11 @@
> > > > >                         #power-domain-cells = <1>;
> > > > >                 };
> > > > >
> > > > > +               rpmh-sleep-stats@c3f0000 {
> > > > > +                       compatible = "qcom,rpmh-sleep-stats";
> > > > > +                       reg = <0 0x0c3f0000 0 0x400>;
> > > > > +               };
> > > > > +
> > > >
> > > > Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> > > > aux-bus and stick the sleep stats device on there?
> > > >
> > >
> > > The AOSS memory space has N chunks of "message ram", one is used for the
> > > QMP protocol (presumably the APSS specific one), a different one is used
> > > for the sleep stats.
> > >
> > > I presume we could have come up with a binding for the entire AOSS/AOP
> > > and then describe (either implicit or explicitly) the QMP and
> > > debug-stats under that.
> > >
> > > But we'd also have to come up with the same container-device for the RPM
> > > case.
> >
> > Because the rpm node doesn't include this region of memory today? I
> > still fail to see why we're changing the existing binding and adding a
> > DT node for this new region that is basically a debug feature.
>
> We're not changing the binding, the memory region for the "AOSS QMP"
> thing was never larger than 0x400.
>
> 0x100000 is the size of all the AOSS "msg_ram" regions. We don't have
> this whole thing described in a binding and we don't have an
> implementation for the whole thing.
>
> If we're going for that we'd need to extend the binding to indicate
> which of the msg_ram regions are used for APSS QMP and for debug stats
> on particular platform (either by compatible, explicit properties or as
> some subnodes).

Fair enough. At the least, can we change the name of the node then to
'sram' or 'ram'? The 'rpmh-sleep-stats' node name is nonsense.

>
>
> That said, as I looked into my other objection, for the RPM
> (non-hardened) case it seems that we're actually describing the RPM
> region. So there it would make sense to describe it as such in DT - but
> we don't have any other code (that I'm aware of) that would implement
> the "qcom,<platform>-rpm".
>

I only half parsed this part. Are you saying that because we don't have
a driver for qcom,<platform>-rpm we shouldn't keep it all within the rpm
node?

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Bjorn Andersson]

On Fri 04 Jun 16:53 CDT 2021, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2021-06-02 19:44:40)
> > On Wed 02 Jun 19:26 CDT 2021, Stephen Boyd wrote:
> >
> > > Quoting Bjorn Andersson (2021-05-31 10:57:03)
> > > > On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:
> > > >
> > > > > Quoting Maulik Shah (2021-05-21 04:26:09)
> > > > > > @@ -3223,6 +3223,11 @@
> > > > > >                         #power-domain-cells = <1>;
> > > > > >                 };
> > > > > >
> > > > > > +               rpmh-sleep-stats@c3f0000 {
> > > > > > +                       compatible = "qcom,rpmh-sleep-stats";
> > > > > > +                       reg = <0 0x0c3f0000 0 0x400>;
> > > > > > +               };
> > > > > > +
> > > > >
> > > > > Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> > > > > aux-bus and stick the sleep stats device on there?
> > > > >
> > > >
> > > > The AOSS memory space has N chunks of "message ram", one is used for the
> > > > QMP protocol (presumably the APSS specific one), a different one is used
> > > > for the sleep stats.
> > > >
> > > > I presume we could have come up with a binding for the entire AOSS/AOP
> > > > and then describe (either implicit or explicitly) the QMP and
> > > > debug-stats under that.
> > > >
> > > > But we'd also have to come up with the same container-device for the RPM
> > > > case.
> > >
> > > Because the rpm node doesn't include this region of memory today? I
> > > still fail to see why we're changing the existing binding and adding a
> > > DT node for this new region that is basically a debug feature.
> >
> > We're not changing the binding, the memory region for the "AOSS QMP"
> > thing was never larger than 0x400.
> >
> > 0x100000 is the size of all the AOSS "msg_ram" regions. We don't have
> > this whole thing described in a binding and we don't have an
> > implementation for the whole thing.
> >
> > If we're going for that we'd need to extend the binding to indicate
> > which of the msg_ram regions are used for APSS QMP and for debug stats
> > on particular platform (either by compatible, explicit properties or as
> > some subnodes).
> 
> Fair enough. At the least, can we change the name of the node then to
> 'sram' or 'ram'? The 'rpmh-sleep-stats' node name is nonsense.
> 

Yes, "ram" sounds like a better node name for both the qmp and
sleep-stats region - in the RPMH case.

> >
> >
> > That said, as I looked into my other objection, for the RPM
> > (non-hardened) case it seems that we're actually describing the RPM
> > region. So there it would make sense to describe it as such in DT - but
> > we don't have any other code (that I'm aware of) that would implement
> > the "qcom,<platform>-rpm".
> >
> 
> I only half parsed this part. Are you saying that because we don't have
> a driver for qcom,<platform>-rpm we shouldn't keep it all within the rpm
> node?

What I was trying to say is that in the RPM (non-H) case the described
memory region is not a chunk of "ram" (or "sram"), but seems to rather
be the RPM region. So there it seems more reasonable to have a non-debug
compatible, but I don't think we have any other use for it than the
debug-stats...

Regards,
Bjorn

Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats

[Maulik Shah]

Hi,

On 6/6/2021 9:12 AM, Bjorn Andersson wrote:
> Yes, "ram" sounds like a better node name for both the qmp and
> sleep-stats region - in the RPMH case.

Updated to use "aop_msgram" in v9.  I think saying only "ram" may 
further confuse with DDR.

Thanks,
Maulik

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation