Re: [PATCH v4 1/4] dt-bindings: Introduce SoC sleep stats bindings

[Maulik Shah]

On 3/10/2020 12:31 AM, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2020-03-09 11:51:20)
>> On Mon 09 Mar 11:23 PDT 2020, Stephen Boyd wrote:
>>
>>> Quoting Maulik Shah (2020-03-09 04:14:14)
>>>> From: Mahesh Sivasubramanian <msivasub@xxxxxxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxxxxxxx
>>>> Signed-off-by: Mahesh Sivasubramanian <msivasub@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx>
>>>> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
>>>> Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>>>> ---
>>>>  .../bindings/soc/qcom/soc-sleep-stats.yaml         | 46 ++++++++++++++++++++++
>>>>  1 file changed, 46 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..7c29c61
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/qcom/soc-sleep-stats.yaml
>>>> @@ -0,0 +1,46 @@
>>>> +# 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@xxxxxxxxxxxxxx>
>>>> +  - Lina Iyer <ilina@xxxxxxxxxxxxxx>
>>>> +
>>>> +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
>>>> +
>>>> +examples:
>>>> +  # Example of rpmh sleep stats
>>>> +  - |
>>>> +    rpmh_sleep_stats@c3f0000 {
>>>> +      compatible = "qcom,rpmh-sleep-stats";
>>>> +      reg = <0 0xc3f0000 0 0x400>;
>>>> +    };
>>>> +  # Example of rpm sleep stats
>>>> +  - |
>>>> +    rpm_sleep_stats@4690000 {
>>> Node names don't have underscores. It really feels like we should be able
>>> to get away with not having this device node at all. Why can't we have
>>> the rpm message ram be a node that covers the entire range and then have
>>> that either create a platform device for debugfs stats or just have it
>>> register the stat information from whatever driver attaches to that
>>> node?
>>>
>>> Carving this up into multiple nodes and making compatible strings
>>> doesn't seem very useful here because we're essentially making device
>>> nodes in DT for logical software components that exist in the rpm
>>> message ram.
>> It's been a while since I discussed this with Lina, but iirc I opted for
>> the model you suggest and we concluded that it wouldn't fit with the RPM
>> case.
>>
>> And given that, for reasons unknown to me, msgram isn't a single region,
>> but a set of adjacent memory regions, this does seem to represent
>> hardware better.
>>
> I guess there's message ram and code ram or something like that? Maybe
> that's the problem? Either way it sounds like the node name needs to be
> fixed to have dashes and then this is fine to keep. Describing memory
> like this in DT just makes me sad.
Hi,

I will spin v5 with fixing node name.

Thanks,
Maulik

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