Re: [PATCH v10 2/6] dt-bindings: opp: v2-qcom-level: Document CPR3 open/closed loop volt adjustment

From: AngeloGioacchino Del Regno
Date: Mon Feb 20 2023 - 06:27:38 EST


Il 18/02/23 01:26, Konrad Dybcio ha scritto:


On 18.02.2023 00:13, Rob Herring wrote:
On Fri, Feb 17, 2023 at 12:08:25PM +0100, Konrad Dybcio wrote:
CPR3 and newer can be fed per-OPP voltage adjustment values for both
open- and closed-loop paths to make better decisions about settling
on the final voltage offset target. Document these properties.

Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
.../devicetree/bindings/opp/opp-v2-qcom-level.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
index a30ef93213c0..93cc88434dfe 100644
--- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
+++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
@@ -34,6 +34,20 @@ patternProperties:
minItems: 1
maxItems: 2
+ qcom,opp-cloop-vadj:
+ description: |
+ A value representing the closed-loop voltage adjustment value

A value?

+ associated with this OPP node.
+ $ref: /schemas/types.yaml#/definitions/int32-array
+ maxItems: 2

Or 2 values?
Right, this description doesn't make any sense if you're just
looking at the documentation without looking at the driver..

Generally, each CPR3 instance can have multiple "threads"
(each one of which regulates voltage for some on-SoC IP or
part of it). The nth entry in the qcom,opp-[co]loop-vadj
array corresponds to a voltage offset for the nth thread.

If the nth entry in the array is missing, the driver assumes
the arr[0] one is "global" to this CPR3 instance at this OPP
level and applies it to all threads. ...and looking at it
again, this is sorta just bad design, especially if you
take into account that there's no known user of CPR3 that
employs more than 2 threads.

I'll remove that from the driver and make the description clearer.


description:
Represents the closed-loop voltage adjustment associated with
this OPP node.

P.S.: Drop '|' here and on oloop!

This binding is intended to support either single or multiple CPR threads;
the driver's behavior is unimportant as bindings describe the hardware,
not the driver.

Regards,
Angelo


Also, only noticed now.. "qcom,sdm630-cprh" was not documented,
so that's to be fixed for the next submission as well!


Konrad

+
+ qcom,opp-oloop-vadj:
+ description: |
+ A value representing the open-loop voltage adjustment value
+ associated with this OPP node.
+ $ref: /schemas/types.yaml#/definitions/int32-array
+ maxItems: 2
+
required:
- opp-level
- qcom,opp-fuse-level

--
2.39.1