[PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[Maulik Shah]

From: Mahesh Sivasubramanian <msivasub@codeaurora.org>

Add device binding documentation for Qualcomm Technology Inc's (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>
---
 .../bindings/soc/qcom/soc-sleep-stats.yaml         | 47 ++++++++++++++++++++++
 1 file changed, 47 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 00000000..50352a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/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.
+  SoC sleep stats driver provides debugfs interface to show this information.
+
+properties:
+  compatible:
+    enum:
+      - qcom,rpmh-sleep-stats
+      - qcom,rpm-sleep-stats
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  # Example of rpmh sleep stats
+  - |
+    rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
+      compatible = "qcom,rpmh-sleep-stats";
+      reg = <0 0xc3f0000 0 0x400>;
+    };
+  # Example of rpm sleep stats
+  - |
+    rpm_sleep_stats: soc-sleep-stats@4690000 {
+      compatible = "qcom,rpm-sleep-stats";
+      reg = <0 0x04690000 0 0x400>;
+    };
+...
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 3/4] arm64: dts: qcom: sc7180: Enable soc sleep stats

[Maulik Shah]

SoC sleep stats provides various low power mode stats.

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

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 8011c5f..eee6d92 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -745,6 +745,11 @@
 			};
 		};
 
+		rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
+			compatible = "qcom,rpmh-sleep-stats";
+			reg = <0 0xc3f0000 0 0x400>;
+		};
+
 		tcsr_mutex_regs: syscon@1f40000 {
 			compatible = "syscon";
 			reg = <0 0x01f40000 0 0x40000>;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[Rob Herring]

On Fri, 21 Feb 2020 14:19:43 +0530, Maulik Shah wrote:
> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> 
> Add device binding documentation for Qualcomm Technology Inc's (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>
> ---
>  .../bindings/soc/qcom/soc-sleep-stats.yaml         | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 3/4] arm64: dts: qcom: sc7180: Enable soc sleep stats

[Bjorn Andersson]

On Fri 21 Feb 00:49 PST 2020, Maulik Shah wrote:

> SoC sleep stats provides various low power mode stats.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 8011c5f..eee6d92 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -745,6 +745,11 @@
>  			};
>  		};
>  
> +		rpmh_sleep_stats: soc-sleep-stats@c3f0000 {

I don't see any reason to reference this node, so you should be able to
omit the label(?)

> +			compatible = "qcom,rpmh-sleep-stats";
> +			reg = <0 0xc3f0000 0 0x400>;

Please pad the address to 8 digits and sort the nodes by address.

Regards,
Bjorn

> +		};
> +
>  		tcsr_mutex_regs: syscon@1f40000 {
>  			compatible = "syscon";
>  			reg = <0 0x01f40000 0 0x40000>;
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[Bjorn Andersson]

On Fri 21 Feb 00:49 PST 2020, Maulik Shah wrote:

> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> 
> Add device binding documentation for Qualcomm Technology Inc's (QTI)
> SoC sleep stats driver. The driver is used for displaying SoC sleep
> statistic maintained by Always On Processor or Resource Power Manager.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> 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>
> ---
>  .../bindings/soc/qcom/soc-sleep-stats.yaml         | 47 ++++++++++++++++++++++
>  1 file changed, 47 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 00000000..50352a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/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.
> +  SoC sleep stats driver provides debugfs interface to show this information.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,rpmh-sleep-stats
> +      - qcom,rpm-sleep-stats
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  # Example of rpmh sleep stats
> +  - |
> +    rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
> +      compatible = "qcom,rpmh-sleep-stats";
> +      reg = <0 0xc3f0000 0 0x400>;
> +    };
> +  # Example of rpm sleep stats
> +  - |
> +    rpm_sleep_stats: soc-sleep-stats@4690000 {
> +      compatible = "qcom,rpm-sleep-stats";
> +      reg = <0 0x04690000 0 0x400>;
> +    };
> +...
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Maulik Shah (2020-02-21 00:49:43)
> 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 00000000..50352a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/soc/qcom/soc-sleep-stats.yaml#

Drop 'bindings' from above?

> +$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.
> +  SoC sleep stats driver provides debugfs interface to show this information.

Please remove this last line. It is a Linuxism that doesn't belong in DT
bindings. And then make it one paragraph and drop the | because
formatting doesn't need to be maintained.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,rpmh-sleep-stats
> +      - qcom,rpm-sleep-stats
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  # Example of rpmh sleep stats
> +  - |
> +    rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
> +      compatible = "qcom,rpmh-sleep-stats";
> +      reg = <0 0xc3f0000 0 0x400>;
> +    };

I see that on sc7180 aoss-qmp is overlapping with this region.


	aoss_qmp: qmp@c300000 {
		compatible = "qcom,sc7180-aoss-qmp";
		reg = <0 0x0c300000 0 0x100000>;

So is this register region really something more like a TCM or RAM area
where rpmh combines multiple software concepts into one hardware memory
region? The aoss-qmp driver talks about message RAM, so I think this
sleep stats stuff is a carveout of the RPMh message RAM. It seems OK if
we want to split that message RAM up into multiple DT nodes, but then
we'll need to reduce the reg size for the aoss-qmp node.

Finally, the node name 'soc-sleep-stats' is generic, but I wonder why we
couldn't name the node 'tcm' or 'memory' or 'msgram'. Similarly for the
qmp node it fits better DT style to have the node be something generic.

> +  # Example of rpm sleep stats
> +  - |
> +    rpm_sleep_stats: soc-sleep-stats@4690000 {
> +      compatible = "qcom,rpm-sleep-stats";
> +      reg = <0 0x04690000 0 0x400>;
> +    };
> +...

Re: [PATCH v2 3/4] arm64: dts: qcom: sc7180: Enable soc sleep stats

[Maulik Shah]

On 2/28/2020 12:04 PM, Bjorn Andersson wrote:
> On Fri 21 Feb 00:49 PST 2020, Maulik Shah wrote:
>
>> SoC sleep stats provides various low power mode stats.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7180.dtsi | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 8011c5f..eee6d92 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -745,6 +745,11 @@
>>   			};
>>   		};
>>   
>> +		rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
> I don't see any reason to reference this node, so you should be able to
> omit the label(?)
Done. Will update in next revision.

Thanks,
Maulik
>
>> +			compatible = "qcom,rpmh-sleep-stats";
>> +			reg = <0 0xc3f0000 0 0x400>;
> Please pad the address to 8 digits and sort the nodes by address.
>
> Regards,
> Bjorn
Done. Will update in next revision.

Thanks,
Maulik
>
>> +		};
>> +
>>   		tcsr_mutex_regs: syscon@1f40000 {
>>   			compatible = "syscon";
>>   			reg = <0 0x01f40000 0 0x40000>;
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation

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

Re: [PATCH v2 1/4] dt-bindings: Introduce soc sleep stats bindings for Qualcomm SoCs

[Maulik Shah]

On 2/28/2020 10:17 PM, Stephen Boyd wrote:
> Quoting Maulik Shah (2020-02-21 00:49:43)
>> 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 00000000..50352a4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/soc/qcom/soc-sleep-stats.yaml#
> Drop 'bindings' from above?
Done.
>> +$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.
>> +  SoC sleep stats driver provides debugfs interface to show this information.
> Please remove this last line. It is a Linuxism that doesn't belong in DT
> bindings. And then make it one paragraph and drop the | because
> formatting doesn't need to be maintained.
Done.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,rpmh-sleep-stats
>> +      - qcom,rpm-sleep-stats
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +examples:
>> +  # Example of rpmh sleep stats
>> +  - |
>> +    rpmh_sleep_stats: soc-sleep-stats@c3f0000 {
>> +      compatible = "qcom,rpmh-sleep-stats";
>> +      reg = <0 0xc3f0000 0 0x400>;
>> +    };
> I see that on sc7180 aoss-qmp is overlapping with this region.
>
>
> 	aoss_qmp: qmp@c300000 {
> 		compatible = "qcom,sc7180-aoss-qmp";
> 		reg = <0 0x0c300000 0 0x100000>;
>
> So is this register region really something more like a TCM or RAM area
> where rpmh combines multiple software concepts into one hardware memory
> region? The aoss-qmp driver talks about message RAM, so I think this
> sleep stats stuff is a carveout of the RPMh message RAM. It seems OK if
> we want to split that message RAM up into multiple DT nodes, but then
> we'll need to reduce the reg size for the aoss-qmp node.
Yes, i discussed this long back with bjorn, we will limit aoss_qmp size to 0x400.
Will include in next revision in DTSI change.
> Finally, the node name 'soc-sleep-stats' is generic, but I wonder why we
> couldn't name the node 'tcm' or 'memory' or 'msgram'. Similarly for the
> qmp node it fits better DT style to have the node be something generic.
following DTSI change in this series has comment to drop the label, since no one
references this node. if we drop label than just naming it msgram@c3f0000 will also
be too generic IMO.

To address both the comments i am planning to drop label and also soc-sleep-stats
and just keep rpmh_sleep_stats@c3f0000.

Thanks,
Maulik

>> +  # Example of rpm sleep stats
>> +  - |
>> +    rpm_sleep_stats: soc-sleep-stats@4690000 {
>> +      compatible = "qcom,rpm-sleep-stats";
>> +      reg = <0 0x04690000 0 0x400>;
>> +    };
>> +...

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