| Message ID | 20221229030106.3303205-3-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually | expand |
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). > + - mode > + - base1 > + - base2 > + - s0_p1 > + - s0_p2 > + - s1_p1 > + - s1_p2 > + - s2_p1 > + - s2_p2 > + - s3_p1 > + - s3_p2 > + - s4_p1 Best regards, Krzysztof
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. > > > + - mode > > + - base1 > > + - base2 > > + - s0_p1 > > + - s0_p2 > > + - s1_p1 > > + - s1_p2 > > + - s2_p1 > > + - s2_p2 > > + - s3_p1 > > + - s3_p2 > > + - s4_p1 > > > Best regards, > Krzysztof >
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
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 >
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 --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):
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(-)