| Message ID | 576e53a1d0ef218536da976102b4cc207436ec1d.1679403696.git.quic_schowdhu@quicinc.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | soc: qcom: boot_stats: Add driver support for boot_stats | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 21/03/2023 14:51, Souradeep Chowdhury wrote: > Add the device tree bindings for the module power manager sleep > counter. > > Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> > --- > .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > new file mode 100644 > index 0000000..f9f46b7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > @@ -0,0 +1,40 @@ > +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/qcom/qcom,mpm-sleep-counter.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MPM Sleep Counter > + > +maintainers: > + - Souradeep Chowdhury <quic_schowdhu@quicinc.com> > + > +description: | > + MPM(Module Power Manager) has a sleep counter which is used to track Missing space: MPM (Module > + various stages of the boot process in Qualcomm. in Qualcomm SoC. Because you do not track it in the company... > + > +properties: > + compatible: > + items: Drop items. > + - const: qcom,mpm2-sleep-counter SoC specific compatible. > + > + reg: > + items: > + - description: MPM Sleep Counter Base just maxItems: 1 > + > + clock-frequency: > + description: Frequency for the sleep counter Since this does not have clocks, what frequency you are setting here? > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + mpm2-sleep-counter@c221000{ Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "qcom,mpm2-sleep-counter"; > + reg = <0xc221000 0x1000>; > + clock-frequency = <32768>; > + }; Best regards, Krzysztof
On 21/03/2023 14:51, Souradeep Chowdhury wrote: > Add the device tree bindings for the module power manager sleep > counter. > > Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> > --- > .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > new file mode 100644 > index 0000000..f9f46b7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml Your code a bit explains this... this is not a device but some memory region. Does not look like suitable for bindings as separate device. Best regards, Krzysztof
On 3/21/2023 11:03 PM, Krzysztof Kozlowski wrote: > On 21/03/2023 14:51, Souradeep Chowdhury wrote: >> Add the device tree bindings for the module power manager sleep >> counter. >> >> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >> --- >> .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> new file mode 100644 >> index 0000000..f9f46b7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> @@ -0,0 +1,40 @@ >> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/soc/qcom/qcom,mpm-sleep-counter.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: MPM Sleep Counter >> + >> +maintainers: >> + - Souradeep Chowdhury <quic_schowdhu@quicinc.com> >> + >> +description: | >> + MPM(Module Power Manager) has a sleep counter which is used to track > > Missing space: > MPM (Module > Ack >> + various stages of the boot process in Qualcomm. > > in Qualcomm SoC. Because you do not track it in the company... > Ack >> + >> +properties: >> + compatible: >> + items: > > Drop items. > Ack >> + - const: qcom,mpm2-sleep-counter > > SoC specific compatible. > This is a generic Module Power Manager Sleep Counter which is present in all Qcom Socs, so SoC specific compatible is not given here. >> + >> + reg: >> + items: >> + - description: MPM Sleep Counter Base > > just maxItems: 1 > Ack >> + >> + clock-frequency: >> + description: Frequency for the sleep counter > > Since this does not have clocks, what frequency you are setting here? Module Power Manager(MPM) Sleep Counter is a clock that starts ticking from Primary Boot Loader(PBL) Stage. This is usually a 32 Khz clock and the frequency for the same is stored here. > >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + mpm2-sleep-counter@c221000{ > > Node names should be generic. > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Ack > >> + compatible = "qcom,mpm2-sleep-counter"; >> + reg = <0xc221000 0x1000>; >> + clock-frequency = <32768>; >> + }; > > Best regards, > Krzysztof >
On 3/21/2023 11:09 PM, Krzysztof Kozlowski wrote: > On 21/03/2023 14:51, Souradeep Chowdhury wrote: >> Add the device tree bindings for the module power manager sleep >> counter. >> >> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >> --- >> .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> new file mode 100644 >> index 0000000..f9f46b7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml > > Your code a bit explains this... this is not a device but some memory > region. Does not look like suitable for bindings as separate device. Ack. Can you let me know the suitable place to put the bindings for this? > > Best regards, > Krzysztof >
On 22/03/2023 14:46, Souradeep Chowdhury wrote: >>> + - const: qcom,mpm2-sleep-counter >> >> SoC specific compatible. >> > > This is a generic Module Power Manager Sleep Counter which is present in > all Qcom Socs, so SoC specific compatible is not given here. Not really a good reason and it is actually very difficult to verify this. If I understand correctly any moment FW can change and this will stop being valid, so no. > >>> + >>> + reg: >>> + items: >>> + - description: MPM Sleep Counter Base >> >> just maxItems: 1 >> > > Ack >>> + >>> + clock-frequency: >>> + description: Frequency for the sleep counter >> >> Since this does not have clocks, what frequency you are setting here? > > Module Power Manager(MPM) Sleep Counter is a clock that starts ticking > from Primary Boot Loader(PBL) Stage. This is usually a 32 Khz clock and > the frequency for the same is stored here. You just said all devices have the same MPM, so I would argue that all devices have the same clock. Anyway, this is a legacy property. Best regards, Krzysztof
On 22/03/2023 15:02, Souradeep Chowdhury wrote: > > > On 3/21/2023 11:09 PM, Krzysztof Kozlowski wrote: >> On 21/03/2023 14:51, Souradeep Chowdhury wrote: >>> Add the device tree bindings for the module power manager sleep >>> counter. >>> >>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >>> --- >>> .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ >>> 1 file changed, 40 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >>> new file mode 100644 >>> index 0000000..f9f46b7 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >> >> Your code a bit explains this... this is not a device but some memory >> region. Does not look like suitable for bindings as separate device. > > Ack. Can you let me know the suitable place to put the bindings for this? Reserved memory region? Or drop from bindings entirely. Best regards, Krzysztof
On 3/22/2023 9:59 PM, Krzysztof Kozlowski wrote: > On 22/03/2023 14:46, Souradeep Chowdhury wrote: >>>> + - const: qcom,mpm2-sleep-counter >>> >>> SoC specific compatible. >>> >> >> This is a generic Module Power Manager Sleep Counter which is present in >> all Qcom Socs, so SoC specific compatible is not given here. > > Not really a good reason and it is actually very difficult to verify > this. If I understand correctly any moment FW can change and this will > stop being valid, so no. Ack > >> >>>> + >>>> + reg: >>>> + items: >>>> + - description: MPM Sleep Counter Base >>> >>> just maxItems: 1 >>> >> >> Ack >>>> + >>>> + clock-frequency: >>>> + description: Frequency for the sleep counter >>> >>> Since this does not have clocks, what frequency you are setting here? >> >> Module Power Manager(MPM) Sleep Counter is a clock that starts ticking >> from Primary Boot Loader(PBL) Stage. This is usually a 32 Khz clock and >> the frequency for the same is stored here. > > You just said all devices have the same MPM, so I would argue that all > devices have the same clock. > > Anyway, this is a legacy property. Ack. Will drop clock frequency. > > Best regards, > Krzysztof >
On 3/22/2023 10:01 PM, Krzysztof Kozlowski wrote: > On 22/03/2023 15:02, Souradeep Chowdhury wrote: >> >> >> On 3/21/2023 11:09 PM, Krzysztof Kozlowski wrote: >>> On 21/03/2023 14:51, Souradeep Chowdhury wrote: >>>> Add the device tree bindings for the module power manager sleep >>>> counter. >>>> >>>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >>>> --- >>>> .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ >>>> 1 file changed, 40 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >>>> new file mode 100644 >>>> index 0000000..f9f46b7 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml >>> >>> Your code a bit explains this... this is not a device but some memory >>> region. Does not look like suitable for bindings as separate device. >> >> Ack. Can you let me know the suitable place to put the bindings for this? > > Reserved memory region? Or drop from bindings entirely. Ack. Will drop this for now. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml new file mode 100644 index 0000000..f9f46b7 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/qcom/qcom,mpm-sleep-counter.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MPM Sleep Counter + +maintainers: + - Souradeep Chowdhury <quic_schowdhu@quicinc.com> + +description: | + MPM(Module Power Manager) has a sleep counter which is used to track + various stages of the boot process in Qualcomm. + +properties: + compatible: + items: + - const: qcom,mpm2-sleep-counter + + reg: + items: + - description: MPM Sleep Counter Base + + clock-frequency: + description: Frequency for the sleep counter + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + mpm2-sleep-counter@c221000{ + compatible = "qcom,mpm2-sleep-counter"; + reg = <0xc221000 0x1000>; + clock-frequency = <32768>; + };
Add the device tree bindings for the module power manager sleep counter. Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> --- .../bindings/soc/qcom/qcom,mpm-sleep-counter.yaml | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mpm-sleep-counter.yaml