diff mbox series

[v5,02/20] dt-bindings: thermal: tsens: support per-sensor calibration cells

Message ID 20221229030106.3303205-3-dmitry.baryshkov@linaro.org
State New
Headers show
Series thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually | expand

Commit Message

Dmitry Baryshkov Dec. 29, 2022, 3 a.m. UTC
Allow specifying the exact calibration mode and calibration data as nvmem
cells, rather than specifying just a single calibration data blob.

Note, unlike the vendor kernel the calibration data uses hw_ids rather
than software sensor indices (to match actual tsens usage in
thermal zones).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/thermal/qcom-tsens.yaml          | 95 +++++++++++++++++--
 1 file changed, 85 insertions(+), 10 deletions(-)

Comments

Krzysztof Kozlowski Jan. 1, 2023, 3:56 p.m. UTC | #1
On 29/12/2022 12:49, Dmitry Baryshkov wrote:
> On Thu, 29 Dec 2022 at 10:35, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 29/12/2022 04:00, Dmitry Baryshkov wrote:
>>> Allow specifying the exact calibration mode and calibration data as nvmem
>>> cells, rather than specifying just a single calibration data blob.
>>>
>>> Note, unlike the vendor kernel the calibration data uses hw_ids rather
>>> than software sensor indices (to match actual tsens usage in
>>> thermal zones).
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>  .../bindings/thermal/qcom-tsens.yaml          | 95 +++++++++++++++++--
>>>  1 file changed, 85 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> index f3660af0b3bf..4bb689f4602d 100644
>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> @@ -81,18 +81,63 @@ properties:
>>>      maxItems: 2
>>>
>>>    nvmem-cells:
>>> -    minItems: 1
>>> -    maxItems: 2
>>> -    description:
>>> -      Reference to an nvmem node for the calibration data
>>> +    oneOf:
>>> +      - minItems: 1
>>> +        maxItems: 2
>>> +        description:
>>> +          Reference to an nvmem node for the calibration data
>>> +      - minItems: 5
>>> +        maxItems: 35
>>> +        description: |
>>> +          Reference to nvmem cells for the calibration mode, two calibration
>>> +          bases and two cells per each sensor
>>>
>>>    nvmem-cell-names:
>>> -    minItems: 1
>>> -    items:
>>> -      - const: calib
>>> -      - enum:
>>> -          - calib_backup
>>> -          - calib_sel
>>> +    oneOf:
>>> +      - minItems: 1
>>> +        items:
>>> +          - const: calib
>>> +          - enum:
>>> +              - calib_backup
>>> +              - calib_sel
>>> +      - minItems: 5
>>> +        items:
>>> +          enum:
>>
>> This should not be an enum but a list of const... unless "holes" are
>> expected (e.g. s0_p1 and s5_p2, without ones in between).
> 
> Yes, this is the case. See the msm8916.dtsi changes. There is no
> sensor with hw_id 3, so the sequence is: ... s2_p1, s2_p2, s4_p1,
> s4_p2,....
> 
> Same applies to the msm8939 (no sensor #4).
> 
> Note: if there was support for the prefixItems, I'd have probably
> marked mode/base1/base2 to be the first items of the array.

Then how about list of const items and patterns? Would be similar number
of lines, just a bit more complicated pattern instead of simple string
to enum.

Best regards,
Krzysztof
Dmitry Baryshkov Jan. 1, 2023, 7:18 p.m. UTC | #2
On 01/01/2023 17:56, Krzysztof Kozlowski wrote:
> On 29/12/2022 12:49, Dmitry Baryshkov wrote:
>> On Thu, 29 Dec 2022 at 10:35, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 29/12/2022 04:00, Dmitry Baryshkov wrote:
>>>> Allow specifying the exact calibration mode and calibration data as nvmem
>>>> cells, rather than specifying just a single calibration data blob.
>>>>
>>>> Note, unlike the vendor kernel the calibration data uses hw_ids rather
>>>> than software sensor indices (to match actual tsens usage in
>>>> thermal zones).
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>>   .../bindings/thermal/qcom-tsens.yaml          | 95 +++++++++++++++++--
>>>>   1 file changed, 85 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> index f3660af0b3bf..4bb689f4602d 100644
>>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> @@ -81,18 +81,63 @@ properties:
>>>>       maxItems: 2
>>>>
>>>>     nvmem-cells:
>>>> -    minItems: 1
>>>> -    maxItems: 2
>>>> -    description:
>>>> -      Reference to an nvmem node for the calibration data
>>>> +    oneOf:
>>>> +      - minItems: 1
>>>> +        maxItems: 2
>>>> +        description:
>>>> +          Reference to an nvmem node for the calibration data
>>>> +      - minItems: 5
>>>> +        maxItems: 35
>>>> +        description: |
>>>> +          Reference to nvmem cells for the calibration mode, two calibration
>>>> +          bases and two cells per each sensor
>>>>
>>>>     nvmem-cell-names:
>>>> -    minItems: 1
>>>> -    items:
>>>> -      - const: calib
>>>> -      - enum:
>>>> -          - calib_backup
>>>> -          - calib_sel
>>>> +    oneOf:
>>>> +      - minItems: 1
>>>> +        items:
>>>> +          - const: calib
>>>> +          - enum:
>>>> +              - calib_backup
>>>> +              - calib_sel
>>>> +      - minItems: 5
>>>> +        items:
>>>> +          enum:
>>>
>>> This should not be an enum but a list of const... unless "holes" are
>>> expected (e.g. s0_p1 and s5_p2, without ones in between).
>>
>> Yes, this is the case. See the msm8916.dtsi changes. There is no
>> sensor with hw_id 3, so the sequence is: ... s2_p1, s2_p2, s4_p1,
>> s4_p2,....
>>
>> Same applies to the msm8939 (no sensor #4).
>>
>> Note: if there was support for the prefixItems, I'd have probably
>> marked mode/base1/base2 to be the first items of the array.
> 
> Then how about list of const items and patterns? Would be similar number
> of lines, just a bit more complicated pattern instead of simple string
> to enum.

Ack, this sounds good. I'll send it in the v6.

BTW: do you know if there are any plans to add support for prefixItems?

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 2, 2023, 8:32 a.m. UTC | #3
On 01/01/2023 20:18, Dmitry Baryshkov wrote:

>>> Same applies to the msm8939 (no sensor #4).
>>>
>>> Note: if there was support for the prefixItems, I'd have probably
>>> marked mode/base1/base2 to be the first items of the array.
>>
>> Then how about list of const items and patterns? Would be similar number
>> of lines, just a bit more complicated pattern instead of simple string
>> to enum.
> 
> Ack, this sounds good. I'll send it in the v6.
> 
> BTW: do you know if there are any plans to add support for prefixItems?

Yes, I think Rob is planning to introduce it.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index f3660af0b3bf..4bb689f4602d 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -81,18 +81,63 @@  properties:
     maxItems: 2
 
   nvmem-cells:
-    minItems: 1
-    maxItems: 2
-    description:
-      Reference to an nvmem node for the calibration data
+    oneOf:
+      - minItems: 1
+        maxItems: 2
+        description:
+          Reference to an nvmem node for the calibration data
+      - minItems: 5
+        maxItems: 35
+        description: |
+          Reference to nvmem cells for the calibration mode, two calibration
+          bases and two cells per each sensor
 
   nvmem-cell-names:
-    minItems: 1
-    items:
-      - const: calib
-      - enum:
-          - calib_backup
-          - calib_sel
+    oneOf:
+      - minItems: 1
+        items:
+          - const: calib
+          - enum:
+              - calib_backup
+              - calib_sel
+      - minItems: 5
+        items:
+          enum:
+            - mode
+            - base1
+            - base2
+            - s0_p1
+            - s0_p2
+            - s1_p1
+            - s1_p2
+            - s2_p1
+            - s2_p2
+            - s3_p1
+            - s3_p2
+            - s4_p1
+            - s4_p2
+            - s5_p1
+            - s5_p2
+            - s6_p1
+            - s6_p2
+            - s7_p1
+            - s7_p2
+            - s8_p1
+            - s8_p2
+            - s9_p1
+            - s9_p2
+            - s10_p1
+            - s10_p2
+            - s11_p1
+            - s11_p2
+            - s12_p1
+            - s12_p2
+            - s13_p1
+            - s13_p2
+            - s14_p1
+            - s14_p2
+            - s15_p1
+            - s15_p2
 
   "#qcom,sensors":
     description:
@@ -221,6 +266,36 @@  examples:
           };
     };
 
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    // Example 1 (new calbiration data: for pre v1 IP):
+    thermal-sensor@900000 {
+        compatible = "qcom,msm8916-tsens", "qcom,tsens-v0_1";
+        reg = <0x4a9000 0x1000>, /* TM */
+              <0x4a8000 0x1000>; /* SROT */
+
+        nvmem-cells = <&tsens_mode>,
+                      <&tsens_base1>, <&tsens_base2>,
+                      <&tsens_s0_p1>, <&tsens_s0_p2>,
+                      <&tsens_s1_p1>, <&tsens_s1_p2>,
+                      <&tsens_s2_p1>, <&tsens_s2_p2>,
+                      <&tsens_s4_p1>, <&tsens_s4_p2>,
+                      <&tsens_s5_p1>, <&tsens_s5_p2>;
+        nvmem-cell-names = "mode",
+                           "base1", "base2",
+                           "s0_p1", "s0_p2",
+                           "s1_p1", "s1_p2",
+                           "s2_p1", "s2_p2",
+                           "s4_p1", "s4_p2",
+                           "s5_p1", "s5_p2";
+
+        interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "uplow";
+
+        #qcom,sensors = <5>;
+        #thermal-sensor-cells = <1>;
+    };
+
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     // Example 1 (legacy: for pre v1 IP):