| Message ID | 20230710103735.1375847-2-quic_ipkumar@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add IPQ5332 TSENS support | expand |
On 10/07/2023 12:37, Praveenkumar I wrote: > Add TSENS V2 calibration nvmem cells for IPQ5332 > > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> > --- > .../bindings/thermal/qcom-tsens.yaml | 26 +++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > index 27e9e16e6455..8b7863c3989e 100644 > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > @@ -91,7 +91,7 @@ properties: > maxItems: 2 > > nvmem-cells: > - oneOf: > + anyOf: > - minItems: 1 > maxItems: 2 > description: > @@ -106,9 +106,13 @@ properties: > description: | > Reference to nvmem cells for the calibration mode, two calibration > bases and two cells per each sensor, main and backup copies, plus use_backup cell > + - maxItems: 17 > + description: | > + V2 of TSENS, reference to nvmem cells for the calibration mode, two calibration > + bases and one cell per each sensor I think this is already included in one of the previous entries. Otherwise, are you sure that all new devices will have exactly 17 entries? > > nvmem-cell-names: > - oneOf: > + anyOf: > - minItems: 1 > items: > - const: calib > @@ -205,6 +209,24 @@ properties: > - const: s9_p2_backup > - const: s10_p1_backup > - const: s10_p2_backup > + - items: > + - const: mode > + - const: base0 > + - const: base1 > + - const: s0_offset > + - const: s3_offset > + - const: s4_offset > + - const: s5_offset > + - const: s6_offset > + - const: s7_offset > + - const: s8_offset > + - const: s9_offset > + - const: s10_offset > + - const: s11_offset > + - const: s12_offset > + - const: s13_offset > + - const: s14_offset > + - const: s15_offset Don't introduce new naming style. Existing uses s[0-9]+, without offset suffix. Why this should be different? > > "#qcom,sensors": > description: Best regards, Krzysztof
On 7/11/2023 1:40 AM, Krzysztof Kozlowski wrote: > On 10/07/2023 12:37, Praveenkumar I wrote: >> Add TSENS V2 calibration nvmem cells for IPQ5332 >> >> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> >> --- >> .../bindings/thermal/qcom-tsens.yaml | 26 +++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> index 27e9e16e6455..8b7863c3989e 100644 >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> @@ -91,7 +91,7 @@ properties: >> maxItems: 2 >> >> nvmem-cells: >> - oneOf: >> + anyOf: >> - minItems: 1 >> maxItems: 2 >> description: >> @@ -106,9 +106,13 @@ properties: >> description: | >> Reference to nvmem cells for the calibration mode, two calibration >> bases and two cells per each sensor, main and backup copies, plus use_backup cell >> + - maxItems: 17 >> + description: | >> + V2 of TSENS, reference to nvmem cells for the calibration mode, two calibration >> + bases and one cell per each sensor > I think this is already included in one of the previous entries. > Otherwise, are you sure that all new devices will have exactly 17 entries? Previous entries does not support TSENS version 2.X.X QFPROM. TSENS V2 QFPROM has mode, base0, base1 and s[0-15]+_offset. Ideally it should be like, - minItems: 4 - maxItems: 19 But dt binding check fails in oneOf / anyOf condition. So added the IPQ5332 properties which is exactly 17. > >> >> nvmem-cell-names: >> - oneOf: >> + anyOf: >> - minItems: 1 >> items: >> - const: calib >> @@ -205,6 +209,24 @@ properties: >> - const: s9_p2_backup >> - const: s10_p1_backup >> - const: s10_p2_backup >> + - items: >> + - const: mode >> + - const: base0 >> + - const: base1 >> + - const: s0_offset >> + - const: s3_offset >> + - const: s4_offset >> + - const: s5_offset >> + - const: s6_offset >> + - const: s7_offset >> + - const: s8_offset >> + - const: s9_offset >> + - const: s10_offset >> + - const: s11_offset >> + - const: s12_offset >> + - const: s13_offset >> + - const: s14_offset >> + - const: s15_offset > Don't introduce new naming style. Existing uses s[0-9]+, without offset > suffix. Why this should be different? As I mentioned above, s[0-9]+_p1 / s[0-9]+p2 is for TSENS V1. TSENS V2 QFPROM layout is different from the existing one. I would like to add mode, base0, base1 and 16 patterns '^s[0-15]+_offset$'. But DT binding check is failing in oneOf/ anyOf condintion. -- Thanks, Praveenkumar > >> >> "#qcom,sensors": >> description: > Best regards, > Krzysztof >
On 11/07/2023 16:13, Praveenkumar I wrote: >>>>> - const: calib >>>>> @@ -205,6 +209,24 @@ properties: >>>>> - const: s9_p2_backup >>>>> - const: s10_p1_backup >>>>> - const: s10_p2_backup >>>>> + - items: >>>>> + - const: mode >>>>> + - const: base0 >>>>> + - const: base1 >>>>> + - const: s0_offset >>>>> + - const: s3_offset >>>>> + - const: s4_offset >>>>> + - const: s5_offset >>>>> + - const: s6_offset >>>>> + - const: s7_offset >>>>> + - const: s8_offset >>>>> + - const: s9_offset >>>>> + - const: s10_offset >>>>> + - const: s11_offset >>>>> + - const: s12_offset >>>>> + - const: s13_offset >>>>> + - const: s14_offset >>>>> + - const: s15_offset >>>> Don't introduce new naming style. Existing uses s[0-9]+, without offset >>>> suffix. Why this should be different? >>> As I mentioned above, s[0-9]+_p1 / s[0-9]+p2 is for TSENS V1. TSENS V2 >>> QFPROM layout is different from the existing one. >> I know, I did not write about p1/p2. >> >>> I would like to add mode, base0, base1 and 16 patterns >>> '^s[0-15]+_offset$'. But DT binding check is failing in oneOf/ anyOf >>> condintion. >> This does not explain why you need different style - this "offset" suffix. > In QFPROM, the BIT field names are s0_offset, s3_offset to s15_offset. > s1_offset and s2_offset not present in IPQ5332. > Hence used the same "offset" suffix here. May I know in this case, which > nvmem-cells-names you are suggesting to use? "Existing uses s[0-9]+" so s[0-9]+ Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml index 27e9e16e6455..8b7863c3989e 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml @@ -91,7 +91,7 @@ properties: maxItems: 2 nvmem-cells: - oneOf: + anyOf: - minItems: 1 maxItems: 2 description: @@ -106,9 +106,13 @@ properties: description: | Reference to nvmem cells for the calibration mode, two calibration bases and two cells per each sensor, main and backup copies, plus use_backup cell + - maxItems: 17 + description: | + V2 of TSENS, reference to nvmem cells for the calibration mode, two calibration + bases and one cell per each sensor nvmem-cell-names: - oneOf: + anyOf: - minItems: 1 items: - const: calib @@ -205,6 +209,24 @@ properties: - const: s9_p2_backup - const: s10_p1_backup - const: s10_p2_backup + - items: + - const: mode + - const: base0 + - const: base1 + - const: s0_offset + - const: s3_offset + - const: s4_offset + - const: s5_offset + - const: s6_offset + - const: s7_offset + - const: s8_offset + - const: s9_offset + - const: s10_offset + - const: s11_offset + - const: s12_offset + - const: s13_offset + - const: s14_offset + - const: s15_offset "#qcom,sensors": description:
Add TSENS V2 calibration nvmem cells for IPQ5332 Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> --- .../bindings/thermal/qcom-tsens.yaml | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)