From: Maulik Shah <mkshah@codeaurora.org>
To: Stephan Gerhold <stephan@gerhold.net>
Cc: swboyd@chromium.org, mka@chromium.org, evgreen@chromium.org,
bjorn.andersson@linaro.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, agross@kernel.org,
dianders@chromium.org, linux@roeck-us.net, rnayak@codeaurora.org,
lsrao@codeaurora.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v11 1/5] dt-bindings: Introduce QCOM Sleep stats bindings
Date: Fri, 8 Oct 2021 14:20:24 +0530 [thread overview]
Message-ID: <b4c26217-67b1-516a-325f-a58e321c4e09@codeaurora.org> (raw)
In-Reply-To: <YV9PN1JNPhVQb8jN@gerhold.net>
Hi Stephan,
On 10/8/2021 1:20 AM, Stephan Gerhold wrote:
> On Thu, Oct 07, 2021 at 03:27:25PM +0530, Maulik Shah wrote:
>> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
>>
>> Add device binding documentation for Qualcomm Technologies, Inc. (QTI)
>> Sleep stats driver. The driver is used for displaying 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/qcom-sleep-stats.yaml | 47 ++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom-sleep-stats.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom-sleep-stats.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom-sleep-stats.yaml
>> new file mode 100644
>> index 0000000..5213daf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom-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/soc/qcom/qcom-sleep-stats.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. (QTI) Sleep stats bindings
>> +
>> +maintainers:
>> + - Maulik Shah <mkshah@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
>> + - |
>> + sram@c3f0000 {
>> + compatible = "qcom,rpmh-sleep-stats";
>> + reg = <0x0c3f0000 0x400>;
>> + };
>> + # Example of rpm sleep stats
>> + - |
>> + sram@4690000 {
>> + compatible = "qcom,rpm-sleep-stats";
>> + reg = <0x04690000 0x400>;
>> + };
>
> Does this region really only contain "rpm-*sleep*-stats"? AFAICT this is
> really a more generic memory region where various offsets are read from.
Right the area contains all kind of sleep stats/logs.
>
> These are all the offsets in msm8998-pm.dtsi downstream [1]:
> ...9000c: rpm-rail-stats offset
> ...90014: rpm-sleep-stats offset (RPM_DYNAMIC_ADDR in your driver)
> ...90018: rpm-log offset
> ...9001c: "RPM FREE HEAP SPACE"
>
> How would you set up any of the other drivers if the entire region
> is declared as "rpm-sleep-stats"?
We don't need to setup other drivers.
The idea is to have single stats driver (qcom_sleep_stats) that can be
enahanced to to read other stats also on both RPM/ PRMH targets.
Today this driver reads only sleep stats from offset 0x90014.
if in future say, we want to read rpm-rail-stats from offset 0x9000c
then it can be added in the existing driver.
In the rpm data, add one more entry to indicate reading rpm-rail-stats,
something like below and add new compatible flag for the target, may be
"qcom,rpm-sleep-stats-legacy" and should work like,
#define RPM_RAIL_STATS_ADDR 0xc
static const struct stats_config rpm_data = {
.stats_offset = 0,
.num_records = 2,
.appended_stats_avail = true,
.dynamic_offset = true,
.rpm_rail_stats = true,
};
and check in driver probe like
if (config->rpm_rail_stats) {
//create rpm rail stats related files.
}
Similarly for other rpm-logs / any other stats can be added in same driver.
Hope this clarifies.
>
> Perhaps this region should have a more generic name that represents what
> it actually is and not only one of the information it contains, similar
> to "qcom,rpm-msg-ram"?
sram looks good to me. Actually on RPM targets its RPM data ram and RPMH
targets its msgram. To keep the name common sram is used here.
Thanks,
Maulik
>
> Thanks,
> Stephan
>
> [1]: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=LA.UM.8.4.1.c25#n271
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
next prev parent reply other threads:[~2021-10-08 8:50 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-07 9:57 [PATCH v11 0/5] Introduce SoC sleep stats driver Maulik Shah
2021-10-07 9:57 ` [PATCH v11 1/5] dt-bindings: Introduce QCOM Sleep stats bindings Maulik Shah
2021-10-07 19:50 ` Stephan Gerhold
2021-10-08 8:50 ` Maulik Shah [this message]
2021-10-08 10:59 ` Stephan Gerhold
2021-10-13 5:57 ` Maulik Shah
2021-10-13 6:55 ` Stephan Gerhold
2021-10-07 9:57 ` [PATCH v11 2/5] soc: qcom: Add Sleep stats driver Maulik Shah
2021-10-07 18:29 ` Stephan Gerhold
2021-10-08 9:15 ` Maulik Shah
2021-10-08 10:34 ` Stephan Gerhold
2021-10-13 6:04 ` Maulik Shah
2021-10-07 9:57 ` [PATCH v11 3/5] arm64: defconfig: Enable " Maulik Shah
2021-10-07 9:57 ` [PATCH v11 4/5] arm64: dts: qcom: Enable RPMh Sleep stats Maulik Shah
2021-10-07 9:57 ` [PATCH v11 5/5] arm64: dts: qcom: Enable RPM " Maulik Shah
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b4c26217-67b1-516a-325f-a58e321c4e09@codeaurora.org \
--to=mkshah@codeaurora.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=evgreen@chromium.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lsrao@codeaurora.org \
--cc=mka@chromium.org \
--cc=rnayak@codeaurora.org \
--cc=stephan@gerhold.net \
--cc=swboyd@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.