From: Praveenkumar I <[email protected]>
Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
Signed-off-by: Praveenkumar I <[email protected]>
Signed-off-by: Varadarajan Narayanan <[email protected]>
---
[v3]:
Fix dt_binding_check & dtbs_check errors (Used
Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
as reference/example)
Drop 'Acked-by: Rob Herring' as suggested in review
[v2]:
Thanks to Krzysztof Kozlowski <[email protected]>
for the tip to make qcom,ipq8074-tsens as fallback.
---
Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index d9aa54c..57e3908 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -19,6 +19,11 @@ description: |
properties:
compatible:
oneOf:
+ - const: qcom,tsens-v0_1
+ - const: qcom,tsens-v1
+ - const: qcom,tsens-v2
+ - const: qcom,ipq8074-tsens
+
- description: msm8960 TSENS based
items:
- enum:
@@ -66,8 +71,10 @@ properties:
- const: qcom,tsens-v2
- description: v2 of TSENS with combined interrupt
- enum:
- - qcom,ipq8074-tsens
+ items:
+ - enum:
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
reg:
items:
@@ -279,6 +286,7 @@ allOf:
contains:
enum:
- qcom,ipq8074-tsens
+ - qcom,ipq9574-tsens
then:
properties:
interrupts:
@@ -294,6 +302,7 @@ allOf:
contains:
enum:
- qcom,ipq8074-tsens
+ - qcom,ipq9574-tsens
- qcom,tsens-v0_1
- qcom,tsens-v1
- qcom,tsens-v2
--
2.7.4
On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> From: Praveenkumar I <[email protected]>
>
> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
>
> Signed-off-by: Praveenkumar I <[email protected]>
> Signed-off-by: Varadarajan Narayanan <[email protected]>
> ---
> [v3]:
> Fix dt_binding_check & dtbs_check errors (Used
> Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> as reference/example)
>
> Drop 'Acked-by: Rob Herring' as suggested in review
>
> [v2]:
> Thanks to Krzysztof Kozlowski <[email protected]>
> for the tip to make qcom,ipq8074-tsens as fallback.
> ---
> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index d9aa54c..57e3908 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -19,6 +19,11 @@ description: |
> properties:
> compatible:
> oneOf:
> + - const: qcom,tsens-v0_1
> + - const: qcom,tsens-v1
> + - const: qcom,tsens-v2
Nope, these are not correct.
> + - const: qcom,ipq8074-tsens
Also nope, this is already there.
> +
> - description: msm8960 TSENS based
> items:
> - enum:
> @@ -66,8 +71,10 @@ properties:
> - const: qcom,tsens-v2
>
> - description: v2 of TSENS with combined interrupt
> - enum:
> - - qcom,ipq8074-tsens
Why?
> + items:
> + - enum:
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
>
> reg:
> items:
> @@ -279,6 +286,7 @@ allOf:
> contains:
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
Not needed, drop.
> then:
> properties:
> interrupts:
> @@ -294,6 +302,7 @@ allOf:
> contains:
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
Ditto.
Best regards,
Krzysztof
On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> > From: Praveenkumar I <[email protected]>
> >
> > Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> >
> > Signed-off-by: Praveenkumar I <[email protected]>
> > Signed-off-by: Varadarajan Narayanan <[email protected]>
> > ---
> > [v3]:
> > Fix dt_binding_check & dtbs_check errors (Used
> > Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> > as reference/example)
> >
> > Drop 'Acked-by: Rob Herring' as suggested in review
> >
> > [v2]:
> > Thanks to Krzysztof Kozlowski <[email protected]>
> > for the tip to make qcom,ipq8074-tsens as fallback.
> > ---
> > Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > index d9aa54c..57e3908 100644
> > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > @@ -19,6 +19,11 @@ description: |
> > properties:
> > compatible:
> > oneOf:
> > + - const: qcom,tsens-v0_1
> > + - const: qcom,tsens-v1
> > + - const: qcom,tsens-v2
>
> Nope, these are not correct.
>
> > + - const: qcom,ipq8074-tsens
>
> Also nope, this is already there.
>
> > +
> > - description: msm8960 TSENS based
> > items:
> > - enum:
> > @@ -66,8 +71,10 @@ properties:
> > - const: qcom,tsens-v2
> >
> > - description: v2 of TSENS with combined interrupt
> > - enum:
> > - - qcom,ipq8074-tsens
>
> Why?
>
> > + items:
> > + - enum:
> > + - qcom,ipq9574-tsens
> > + - const: qcom,ipq8074-tsens
Without changing it like this either dtbs_check or
dt_binding_check kept failing.
- description: v2 of TSENS with combined interrupt
enum:
- qcom,ipq8074-tsens
- qcom,ipq9574-tsens
dtbs_check gave this kind of error
['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
- description: v2 of TSENS with combined interrupt
const: qcom,ipq8074-tsens
- enum:
- qcom,ipq9574-tsens
- const: qcom,ipq8074-tsens
dt_binding_check gives the following error
Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
and dtbs_check gives
./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
CHKDT Documentation/devicetree/bindings/processed-schema.json
./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
If i change it like below,
- description: v2 of TSENS with combined interrupt
enum:
- qcom,ipq9574-tsens
- const: qcom,ipq8074-tsens
dt_binding_check and dtbs_check gives same error as above.
Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
which seemed to do something similar to what is wanted in this
case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
file. After which dt_binding_check and dtbs_check passed. Please
let me know if there is a better way to solve this. Will go with
that.
Thanks
Varada
> >
> > reg:
> > items:
> > @@ -279,6 +286,7 @@ allOf:
> > contains:
> > enum:
> > - qcom,ipq8074-tsens
> > + - qcom,ipq9574-tsens
>
> Not needed, drop.
>
> > then:
> > properties:
> > interrupts:
> > @@ -294,6 +302,7 @@ allOf:
> > contains:
> > enum:
> > - qcom,ipq8074-tsens
> > + - qcom,ipq9574-tsens
>
> Ditto.
>
>
> Best regards,
> Krzysztof
>
On 16/05/2023 14:04, Varadarajan Narayanan wrote:
> On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
>> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
>>> From: Praveenkumar I <[email protected]>
>>>
>>> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
>>>
>>> Signed-off-by: Praveenkumar I <[email protected]>
>>> Signed-off-by: Varadarajan Narayanan <[email protected]>
>>> ---
>>> [v3]:
>>> Fix dt_binding_check & dtbs_check errors (Used
>>> Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
>>> as reference/example)
>>>
>>> Drop 'Acked-by: Rob Herring' as suggested in review
>>>
>>> [v2]:
>>> Thanks to Krzysztof Kozlowski <[email protected]>
>>> for the tip to make qcom,ipq8074-tsens as fallback.
>>> ---
>>> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> index d9aa54c..57e3908 100644
>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> @@ -19,6 +19,11 @@ description: |
>>> properties:
>>> compatible:
>>> oneOf:
>>> + - const: qcom,tsens-v0_1
>>> + - const: qcom,tsens-v1
>>> + - const: qcom,tsens-v2
>>
>> Nope, these are not correct.
>>
>>> + - const: qcom,ipq8074-tsens
>>
>> Also nope, this is already there.
>>
>>> +
>>> - description: msm8960 TSENS based
>>> items:
>>> - enum:
>>> @@ -66,8 +71,10 @@ properties:
>>> - const: qcom,tsens-v2
>>>
>>> - description: v2 of TSENS with combined interrupt
>>> - enum:
>>> - - qcom,ipq8074-tsens
>>
>> Why?
>>
>>> + items:
>>> + - enum:
>>> + - qcom,ipq9574-tsens
>>> + - const: qcom,ipq8074-tsens
>
> Without changing it like this either dtbs_check or
> dt_binding_check kept failing.
>
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> - qcom,ipq9574-tsens
But we do not talk about this... Look, I commented out under specific
hunks which are not correct. Not under the hunk which is correct.
>
> dtbs_check gave this kind of error
> ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
>
> After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
>
> - description: v2 of TSENS with combined interrupt
> const: qcom,ipq8074-tsens
> - enum:
> - qcom,ipq9574-tsens
> - const: qcom,ipq8074-tsens
>
> dt_binding_check gives the following error
>
> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
Because it is not even valid syntax.
>
> and dtbs_check gives
>
> ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
> CHKDT Documentation/devicetree/bindings/processed-schema.json
> ./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> ./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
> SCHEMA Documentation/devicetree/bindings/processed-schema.json
> /local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
>
> If i change it like below,
>
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq9574-tsens
> - const: qcom,ipq8074-tsens
>
> dt_binding_check and dtbs_check gives same error as above.
>
> Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> which seemed to do something similar to what is wanted in this
> case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
> file. After which dt_binding_check and dtbs_check passed. Please
> let me know if there is a better way to solve this. Will go with
Changing one valid syntax to another valid syntax is not related to the
patch. If you think such change as reasonable, please split it, but to
me it does not look justified. As for actual change, so adding new
compatible, it's not really related to the others. Why you cannot add
the proper list (so the only valid hunk) and that's it?
Best regards,
Krzysztof
On Tue, May 16, 2023 at 03:06:40PM +0200, Krzysztof Kozlowski wrote:
> On 16/05/2023 14:04, Varadarajan Narayanan wrote:
> > On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
> >> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> >>> From: Praveenkumar I <[email protected]>
> >>>
> >>> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> >>>
> >>> Signed-off-by: Praveenkumar I <[email protected]>
> >>> Signed-off-by: Varadarajan Narayanan <[email protected]>
> >>> ---
> >>> [v3]:
> >>> Fix dt_binding_check & dtbs_check errors (Used
> >>> Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> >>> as reference/example)
> >>>
> >>> Drop 'Acked-by: Rob Herring' as suggested in review
> >>>
> >>> [v2]:
> >>> Thanks to Krzysztof Kozlowski <[email protected]>
> >>> for the tip to make qcom,ipq8074-tsens as fallback.
> >>> ---
> >>> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> >>> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> index d9aa54c..57e3908 100644
> >>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> @@ -19,6 +19,11 @@ description: |
> >>> properties:
> >>> compatible:
> >>> oneOf:
> >>> + - const: qcom,tsens-v0_1
> >>> + - const: qcom,tsens-v1
> >>> + - const: qcom,tsens-v2
> >>
> >> Nope, these are not correct.
> >>
> >>> + - const: qcom,ipq8074-tsens
> >>
> >> Also nope, this is already there.
> >>
> >>> +
> >>> - description: msm8960 TSENS based
> >>> items:
> >>> - enum:
> >>> @@ -66,8 +71,10 @@ properties:
> >>> - const: qcom,tsens-v2
> >>>
> >>> - description: v2 of TSENS with combined interrupt
> >>> - enum:
> >>> - - qcom,ipq8074-tsens
> >>
> >> Why?
> >>
> >>> + items:
> >>> + - enum:
> >>> + - qcom,ipq9574-tsens
> >>> + - const: qcom,ipq8074-tsens
> >
> > Without changing it like this either dtbs_check or
> > dt_binding_check kept failing.
> >
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq8074-tsens
> > - qcom,ipq9574-tsens
>
> But we do not talk about this... Look, I commented out under specific
> hunks which are not correct. Not under the hunk which is correct.
>
> >
> > dtbs_check gave this kind of error
> > ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> >
> > After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
> >
> > - description: v2 of TSENS with combined interrupt
> > const: qcom,ipq8074-tsens
> > - enum:
> > - qcom,ipq9574-tsens
> > - const: qcom,ipq8074-tsens
> >
> > dt_binding_check gives the following error
> >
> > Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
>
> Because it is not even valid syntax.
>
> >
> > and dtbs_check gives
> >
> > ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
> > CHKDT Documentation/devicetree/bindings/processed-schema.json
> > ./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > ./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
> > SCHEMA Documentation/devicetree/bindings/processed-schema.json
> > /local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
> >
> > If i change it like below,
> >
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq9574-tsens
> > - const: qcom,ipq8074-tsens
> >
> > dt_binding_check and dtbs_check gives same error as above.
> >
> > Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> > which seemed to do something similar to what is wanted in this
> > case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
> > file. After which dt_binding_check and dtbs_check passed. Please
> > let me know if there is a better way to solve this. Will go with
>
> Changing one valid syntax to another valid syntax is not related to the
> patch. If you think such change as reasonable, please split it, but to
> me it does not look justified. As for actual change, so adding new
> compatible, it's not really related to the others. Why you cannot add
> the proper list (so the only valid hunk) and that's it?
Not sure if I didn't express properly. There are two parts to this patch.
Part-1 is adding the 'const' entries at the beginning i.e.
+ - const: qcom,tsens-v0_1
+ - const: qcom,tsens-v1
+ - const: qcom,tsens-v2
+ - const: qcom,ipq8074-tsens
Part-2 is changing from one valid syntax to another i.e.
+ items:
+ - enum:
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
Without both of the above changes, either or both of dtbs_check
& dt_binding_check fails. So, it is not possible to just add the
"valid hunk" (part-2) alone.
If having both part-1 and part-2 in the same patch is not
acceptable, shall I split them into two patches? Please let me know.
Thanks
Varada
On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> Part-1 is adding the 'const' entries at the beginning i.e.
>
> + - const: qcom,tsens-v0_1
> + - const: qcom,tsens-v1
> + - const: qcom,tsens-v2
> + - const: qcom,ipq8074-tsens
>
> Part-2 is changing from one valid syntax to another i.e.
>
> + items:
> + - enum:
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
>
> Without both of the above changes, either or both of dtbs_check
> & dt_binding_check fails. So, it is not possible to just add the
> "valid hunk" (part-2) alone.
Of course it is. All schema files work like that...
>
> If having both part-1 and part-2 in the same patch is not
> acceptable, shall I split them into two patches? Please let me know.
No, hunk one is not justified.
Best regards,
Krzysztof
On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> > Part-1 is adding the 'const' entries at the beginning i.e.
> >
> > + - const: qcom,tsens-v0_1
> > + - const: qcom,tsens-v1
> > + - const: qcom,tsens-v2
> > + - const: qcom,ipq8074-tsens
> >
> > Part-2 is changing from one valid syntax to another i.e.
> >
> > + items:
> > + - enum:
> > + - qcom,ipq9574-tsens
> > + - const: qcom,ipq8074-tsens
> >
> > Without both of the above changes, either or both of dtbs_check
> > & dt_binding_check fails. So, it is not possible to just add the
> > "valid hunk" (part-2) alone.
>
> Of course it is. All schema files work like that...
> >
> > If having both part-1 and part-2 in the same patch is not
> > acceptable, shall I split them into two patches? Please let me know.
>
> No, hunk one is not justified.
For the other compatibles, the enum entries and const/fallback
entries are different. For the 9574 & 8074 case, we want to have
qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
if we don't have the first hunk, dtbs_check fails for 8074
related dtbs
ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
['qcom,ipq8074-tsens'] is too short
ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
['qcom,ipq8074-tsens'] is too short
ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
['qcom,ipq8074-tsens'] is too short
I'm not sure of the correct solution. Having the first hunk
solves the above dtbs_check errors, so went with it. I'm able to
avoid dtbs_check errors with just one entry in the first hunk.
+ - const: qcom,ipq8074-tsens
Please let me know if there is a better way to resolve this or we
can have just the 8074 entry in the first hunk.
Thanks
Varada
On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>
>>> + - const: qcom,tsens-v0_1
>>> + - const: qcom,tsens-v1
>>> + - const: qcom,tsens-v2
>>> + - const: qcom,ipq8074-tsens
>>>
>>> Part-2 is changing from one valid syntax to another i.e.
>>>
>>> + items:
>>> + - enum:
>>> + - qcom,ipq9574-tsens
>>> + - const: qcom,ipq8074-tsens
>>>
>>> Without both of the above changes, either or both of dtbs_check
>>> & dt_binding_check fails. So, it is not possible to just add the
>>> "valid hunk" (part-2) alone.
>>
>> Of course it is. All schema files work like that...
>>>
>>> If having both part-1 and part-2 in the same patch is not
>>> acceptable, shall I split them into two patches? Please let me know.
>>
>> No, hunk one is not justified.
>
> For the other compatibles, the enum entries and const/fallback
> entries are different. For the 9574 & 8074 case, we want to have
> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> if we don't have the first hunk, dtbs_check fails for 8074
> related dtbs
>
> ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> ['qcom,ipq8074-tsens'] is too short
Why? It is already there. Open the file and you will see that this is
already covered.
If you remove it, then yes, you will see errors and the answer is: do
not remove it.
>
> ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> ['qcom,ipq8074-tsens'] is too short
>
> ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> ['qcom,ipq8074-tsens'] is too short
>
> I'm not sure of the correct solution. Having the first hunk
> solves the above dtbs_check errors, so went with it. I'm able to
> avoid dtbs_check errors with just one entry in the first hunk.
You made multiple changes in one patch which is not correct. Your goal
is to add only one change - ipq9574 followed by ipq8074. Add this one.
Don't touch others.
>
> + - const: qcom,ipq8074-tsens
>
> Please let me know if there is a better way to resolve this or we
> can have just the 8074 entry in the first hunk.
You only need to add new item on the oneOf list:
- enum
- ipq9574
- const: ipq8074
Best regards,
Krzysztof
On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> >> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> >>> Part-1 is adding the 'const' entries at the beginning i.e.
> >>>
> >>> + - const: qcom,tsens-v0_1
> >>> + - const: qcom,tsens-v1
> >>> + - const: qcom,tsens-v2
> >>> + - const: qcom,ipq8074-tsens
> >>>
> >>> Part-2 is changing from one valid syntax to another i.e.
> >>>
> >>> + items:
> >>> + - enum:
> >>> + - qcom,ipq9574-tsens
> >>> + - const: qcom,ipq8074-tsens
> >>>
> >>> Without both of the above changes, either or both of dtbs_check
> >>> & dt_binding_check fails. So, it is not possible to just add the
> >>> "valid hunk" (part-2) alone.
> >>
> >> Of course it is. All schema files work like that...
> >>>
> >>> If having both part-1 and part-2 in the same patch is not
> >>> acceptable, shall I split them into two patches? Please let me know.
> >>
> >> No, hunk one is not justified.
> >
> > For the other compatibles, the enum entries and const/fallback
> > entries are different. For the 9574 & 8074 case, we want to have
> > qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> > if we don't have the first hunk, dtbs_check fails for 8074
> > related dtbs
> >
> > ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > ['qcom,ipq8074-tsens'] is too short
>
> Why? It is already there. Open the file and you will see that this is
> already covered.
I guess dtbs_check doesn't like the same value being a const and
a oneof entry. Have attached the file, please see if something is
not in order.
> If you remove it, then yes, you will see errors and the answer is: do
> not remove it.
I haven't removed it. For this patch, ipq8074-tsens changed from
being an oneof enum entry to a const entry. Probably, that is why
dtbs_check is giving these errors.
> > ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > ['qcom,ipq8074-tsens'] is too short
> >
> > ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > ['qcom,ipq8074-tsens'] is too short
> >
> > I'm not sure of the correct solution. Having the first hunk
> > solves the above dtbs_check errors, so went with it. I'm able to
> > avoid dtbs_check errors with just one entry in the first hunk.
>
> You made multiple changes in one patch which is not correct. Your goal
> is to add only one change - ipq9574 followed by ipq8074. Add this one.
> Don't touch others.
But that breaks dtbs_check.
> > + - const: qcom,ipq8074-tsens
> >
> > Please let me know if there is a better way to resolve this or we
> > can have just the 8074 entry in the first hunk.
>
> You only need to add new item on the oneOf list:
> - enum
> - ipq9574
> - const: ipq8074
The "['qcom,ipq8074-tsens'] is too short" errors were generated
with the above snippet only. Please see the attachment
Thanks
Varada
On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
>> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
>>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>>>
>>>>> + - const: qcom,tsens-v0_1
>>>>> + - const: qcom,tsens-v1
>>>>> + - const: qcom,tsens-v2
>>>>> + - const: qcom,ipq8074-tsens
>>>>>
>>>>> Part-2 is changing from one valid syntax to another i.e.
>>>>>
>>>>> + items:
>>>>> + - enum:
>>>>> + - qcom,ipq9574-tsens
>>>>> + - const: qcom,ipq8074-tsens
>>>>>
>>>>> Without both of the above changes, either or both of dtbs_check
>>>>> & dt_binding_check fails. So, it is not possible to just add the
>>>>> "valid hunk" (part-2) alone.
>>>>
>>>> Of course it is. All schema files work like that...
>>>>>
>>>>> If having both part-1 and part-2 in the same patch is not
>>>>> acceptable, shall I split them into two patches? Please let me know.
>>>>
>>>> No, hunk one is not justified.
>>>
>>> For the other compatibles, the enum entries and const/fallback
>>> entries are different. For the 9574 & 8074 case, we want to have
>>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
>>> if we don't have the first hunk, dtbs_check fails for 8074
>>> related dtbs
>>>
>>> ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> ['qcom,ipq8074-tsens'] is too short
>>
>> Why? It is already there. Open the file and you will see that this is
>> already covered.
>
> I guess dtbs_check doesn't like the same value being a const and
> a oneof entry.
I don't understand.
> Have attached the file, please see if something is
> not in order.
I don't know what changed there. Please work on patches.
>
>> If you remove it, then yes, you will see errors and the answer is: do
>> not remove it.
>
> I haven't removed it.
You did. Look:
- description: v2 of TSENS with combined interrupt
- enum:
- - qcom,ipq8074-tsens
The first character in the diff (-) means removal.
> For this patch, ipq8074-tsens changed from
> being an oneof enum entry to a const entry. Probably, that is why
> dtbs_check is giving these errors.
You removed the entry which you should not have touched.
>
>>> ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> ['qcom,ipq8074-tsens'] is too short
>>>
>>> ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> ['qcom,ipq8074-tsens'] is too short
>>>
>>> I'm not sure of the correct solution. Having the first hunk
>>> solves the above dtbs_check errors, so went with it. I'm able to
>>> avoid dtbs_check errors with just one entry in the first hunk.
>>
>> You made multiple changes in one patch which is not correct. Your goal
>> is to add only one change - ipq9574 followed by ipq8074. Add this one.
>> Don't touch others.
>
> But that breaks dtbs_check.
All other cases, hundreds of other binding files, do not have problem.
Only this one "breaks dtbs_check". No, it does not.
Whatever is broken is result of your removal of unrelated pieces.
>
>>> + - const: qcom,ipq8074-tsens
>>>
>>> Please let me know if there is a better way to resolve this or we
>>> can have just the 8074 entry in the first hunk.
>>
>> You only need to add new item on the oneOf list:
>> - enum
>> - ipq9574
>> - const: ipq8074
>
> The "['qcom,ipq8074-tsens'] is too short" errors were generated
> with the above snippet only. Please see the attachment
It's not true. The error you see is result because you removed something
you should not. I did not ask you to remove anything. So repeating -
"add new item". Adding is not "removal and adding". Adding is just "adding".
Best regards,
Krzysztof
On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> >>>>> Part-1 is adding the 'const' entries at the beginning i.e.
> >>>>>
> >>>>> + - const: qcom,tsens-v0_1
> >>>>> + - const: qcom,tsens-v1
> >>>>> + - const: qcom,tsens-v2
> >>>>> + - const: qcom,ipq8074-tsens
> >>>>>
> >>>>> Part-2 is changing from one valid syntax to another i.e.
> >>>>>
> >>>>> + items:
> >>>>> + - enum:
> >>>>> + - qcom,ipq9574-tsens
> >>>>> + - const: qcom,ipq8074-tsens
> >>>>>
> >>>>> Without both of the above changes, either or both of dtbs_check
> >>>>> & dt_binding_check fails. So, it is not possible to just add the
> >>>>> "valid hunk" (part-2) alone.
> >>>>
> >>>> Of course it is. All schema files work like that...
> >>>>>
> >>>>> If having both part-1 and part-2 in the same patch is not
> >>>>> acceptable, shall I split them into two patches? Please let me know.
> >>>>
> >>>> No, hunk one is not justified.
> >>>
> >>> For the other compatibles, the enum entries and const/fallback
> >>> entries are different. For the 9574 & 8074 case, we want to have
> >>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> >>> if we don't have the first hunk, dtbs_check fails for 8074
> >>> related dtbs
> >>>
> >>> ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> ['qcom,ipq8074-tsens'] is too short
> >>
> >> Why? It is already there. Open the file and you will see that this is
> >> already covered.
> >
> > I guess dtbs_check doesn't like the same value being a const and
> > a oneof entry.
>
> I don't understand.
- description: v2 of TSENS with combined interrupt
items:
- enum:
- qcom,ipq9574-tsens <--- one of the compatible entries
- const: qcom,ipq8074-tsens <--- fallback entry
In this patch, we want 8074 to act as a compatible entry for
ipq8074*.dts and fallback entry for ipq9574.dtsi. That is why I
believe we are not able to just add 9574 and get it to pass
dtbs_check and dt_binding_check.
> > Have attached the file, please see if something is
> > not in order.
>
> I don't know what changed there. Please work on patches.
>
> >
> >> If you remove it, then yes, you will see errors and the answer is: do
> >> not remove it.
> >
> > I haven't removed it.
>
> You did. Look:
>
> - description: v2 of TSENS with combined interrupt
> - enum:
> - - qcom,ipq8074-tsens
>
> The first character in the diff (-) means removal.
It changed from 'enum' to 'const', that is why I said it is not
removed.
> > For this patch, ipq8074-tsens changed from
> > being an oneof enum entry to a const entry. Probably, that is why
> > dtbs_check is giving these errors.
>
> You removed the entry which you should not have touched.
>
> >
> >>> ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> ['qcom,ipq8074-tsens'] is too short
> >>>
> >>> ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> ['qcom,ipq8074-tsens'] is too short
> >>>
> >>> I'm not sure of the correct solution. Having the first hunk
> >>> solves the above dtbs_check errors, so went with it. I'm able to
> >>> avoid dtbs_check errors with just one entry in the first hunk.
> >>
> >> You made multiple changes in one patch which is not correct. Your goal
> >> is to add only one change - ipq9574 followed by ipq8074. Add this one.
> >> Don't touch others.
> >
> > But that breaks dtbs_check.
>
> All other cases, hundreds of other binding files, do not have problem.
> Only this one "breaks dtbs_check". No, it does not.
>
> Whatever is broken is result of your removal of unrelated pieces.
Not sure about other binding files. Probably they don't have the
same value for fallback and normal compatible. If there is such
an example binding file, will replicate that syntax/structure for
ipq9574 too.
In the 'nvidia,tegra210-ope' example (https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L25)
too 'nvidia,tegra210-ope' is listed twice
- const: nvidia,tegra210-ope <===
- items:
- enum:
- nvidia,tegra234-ope
- nvidia,tegra194-ope
- nvidia,tegra186-ope
- const: nvidia,tegra210-ope <===
> >>> + - const: qcom,ipq8074-tsens
> >>>
> >>> Please let me know if there is a better way to resolve this or we
> >>> can have just the 8074 entry in the first hunk.
> >>
> >> You only need to add new item on the oneOf list:
> >> - enum
> >> - ipq9574
> >> - const: ipq8074
> >
> > The "['qcom,ipq8074-tsens'] is too short" errors were generated
> > with the above snippet only. Please see the attachment
>
> It's not true. The error you see is result because you removed something
> you should not. I did not ask you to remove anything. So repeating -
> "add new item". Adding is not "removal and adding". Adding is just "adding".
See below for the changes that were tried and the corresponding errors.
(1) No lines removed
@@ -66,6 +66,7 @@
- description: v2 of TSENS with combined interrupt
enum:
- qcom,ipq8074-tsens
+ - qcom,ipq9574-tsens
reg:
items:
dt_binding_check: No errors
dtbs_check :
arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
(2) No lines removed
@@ -66,6 +66,8 @@
- description: v2 of TSENS with combined interrupt
enum:
- qcom,ipq8074-tsens
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
reg:
items:
dt_binding_check: No errors
dtbs_check : Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
(3) No lines removed
@@ -19,6 +19,7 @@
properties:
compatible:
oneOf:
+ - const: qcom,ipq8074-tsens
- description: msm8960 TSENS based
items:
- enum:
@@ -66,6 +67,8 @@
- description: v2 of TSENS with combined interrupt
enum:
- qcom,ipq8074-tsens
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
reg:
items:
dt_binding_check: Same as above
dtbs_check : Same as above
(4) Change 8074 from enum to const
@@ -19,6 +19,7 @@
properties:
compatible:
oneOf:
+ - const: qcom,ipq8074-tsens
- description: msm8960 TSENS based
items:
- enum:
@@ -64,8 +65,10 @@
- const: qcom,tsens-v2
- description: v2 of TSENS with combined interrupt
- enum:
- - qcom,ipq8074-tsens
+ items:
+ - enum:
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
reg:
items:
dt_binding_check: No errors
dtbs_check : No errors
But (4) doesn't seem acceptable. Any other alternative to resolve this?
Thanks
Varada
On Tue, May 23, 2023 at 03:49:04PM +0530, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> > On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> > >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> > >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> > It's not true. The error you see is result because you removed something
> > you should not. I did not ask you to remove anything. So repeating -
> > "add new item". Adding is not "removal and adding". Adding is just "adding".
>
> See below for the changes that were tried and the corresponding errors.
>
> (1) No lines removed
>
> @@ -66,6 +66,7 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check :
> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
Which I figure you understand makes sense.
> (2) No lines removed
>
> @@ -66,6 +66,8 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check : Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
I think you've missed an earlier error that points out the entire
binding is invalid.
> (3) No lines removed
> @@ -19,6 +19,7 @@
> properties:
> compatible:
> oneOf:
> + - const: qcom,ipq8074-tsens
> - description: msm8960 TSENS based
> items:
> - enum:
> @@ -66,6 +67,8 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
>
> reg:
> items:
>
> dt_binding_check: Same as above
>
> dtbs_check : Same as above
Ditto here.
> (4) Change 8074 from enum to const
> @@ -19,6 +19,7 @@
> properties:
> compatible:
> oneOf:
> + - const: qcom,ipq8074-tsens
> - description: msm8960 TSENS based
> items:
> - enum:
> @@ -64,8 +65,10 @@
> - const: qcom,tsens-v2
>
> - description: v2 of TSENS with combined interrupt
> - enum:
> - - qcom,ipq8074-tsens
> + items:
> + - enum:
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check : No errors
>
> But (4) doesn't seem acceptable. Any other alternative to resolve this?
It now has a "random" entry up at the top w/ no description, not
matching the existing style. Can you just fix that up & send a v(N+1)
so that the discussion can restart in a less confusing way? I am trying
to fill in for Krzysztof but I am struggling to follow the thread.
Thanks,
Conor.
On Tue, May 23, 2023 at 05:44:02PM +0100, Conor Dooley wrote:
> On Tue, May 23, 2023 at 03:49:04PM +0530, Varadarajan Narayanan wrote:
> > On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> > > On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > > > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> > > >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > > >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> > > >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>
> > > It's not true. The error you see is result because you removed something
> > > you should not. I did not ask you to remove anything. So repeating -
> > > "add new item". Adding is not "removal and adding". Adding is just "adding".
> >
> > See below for the changes that were tried and the corresponding errors.
> >
> > (1) No lines removed
> >
> > @@ -66,6 +66,7 @@
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq8074-tsens
> > + - qcom,ipq9574-tsens
> >
> > reg:
> > items:
> >
> > dt_binding_check: No errors
> >
> > dtbs_check :
> > arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> > ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
>
> Which I figure you understand makes sense.
>
> > (2) No lines removed
> >
> > @@ -66,6 +66,8 @@
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq8074-tsens
> > + - qcom,ipq9574-tsens
> > + - const: qcom,ipq8074-tsens
> >
> > reg:
> > items:
> >
> > dt_binding_check: No errors
> >
> > dtbs_check : Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> > arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> > arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> > arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
>
> I think you've missed an earlier error that points out the entire
> binding is invalid.
>
> > (3) No lines removed
> > @@ -19,6 +19,7 @@
> > properties:
> > compatible:
> > oneOf:
> > + - const: qcom,ipq8074-tsens
> > - description: msm8960 TSENS based
> > items:
> > - enum:
> > @@ -66,6 +67,8 @@
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq8074-tsens
> > + - qcom,ipq9574-tsens
> > + - const: qcom,ipq8074-tsens
> >
> > reg:
> > items:
> >
> > dt_binding_check: Same as above
> >
> > dtbs_check : Same as above
>
> Ditto here.
>
> > (4) Change 8074 from enum to const
> > @@ -19,6 +19,7 @@
> > properties:
> > compatible:
> > oneOf:
> > + - const: qcom,ipq8074-tsens
> > - description: msm8960 TSENS based
> > items:
> > - enum:
> > @@ -64,8 +65,10 @@
> > - const: qcom,tsens-v2
> >
> > - description: v2 of TSENS with combined interrupt
> > - enum:
> > - - qcom,ipq8074-tsens
> > + items:
> > + - enum:
> > + - qcom,ipq9574-tsens
> > + - const: qcom,ipq8074-tsens
> >
> > reg:
> > items:
> >
> > dt_binding_check: No errors
> >
> > dtbs_check : No errors
> >
> > But (4) doesn't seem acceptable. Any other alternative to resolve this?
>
> It now has a "random" entry up at the top w/ no description, not
> matching the existing style. Can you just fix that up & send a v(N+1)
> so that the discussion can restart in a less confusing way? I am trying
> to fill in for Krzysztof but I am struggling to follow the thread.
Sure. Will post the next version.
Thanks
Varada
On 23/05/2023 12:19, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
>> On 18/05/2023 11:05, Varadarajan Narayanan wrote:
>>> On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
>>>> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
>>>>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>>>>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>>>>>
>>>>>>> + - const: qcom,tsens-v0_1
>>>>>>> + - const: qcom,tsens-v1
>>>>>>> + - const: qcom,tsens-v2
>>>>>>> + - const: qcom,ipq8074-tsens
>>>>>>>
>>>>>>> Part-2 is changing from one valid syntax to another i.e.
>>>>>>>
>>>>>>> + items:
>>>>>>> + - enum:
>>>>>>> + - qcom,ipq9574-tsens
>>>>>>> + - const: qcom,ipq8074-tsens
>>>>>>>
>>>>>>> Without both of the above changes, either or both of dtbs_check
>>>>>>> & dt_binding_check fails. So, it is not possible to just add the
>>>>>>> "valid hunk" (part-2) alone.
>>>>>>
>>>>>> Of course it is. All schema files work like that...
>>>>>>>
>>>>>>> If having both part-1 and part-2 in the same patch is not
>>>>>>> acceptable, shall I split them into two patches? Please let me know.
>>>>>>
>>>>>> No, hunk one is not justified.
>>>>>
>>>>> For the other compatibles, the enum entries and const/fallback
>>>>> entries are different. For the 9574 & 8074 case, we want to have
>>>>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
>>>>> if we don't have the first hunk, dtbs_check fails for 8074
>>>>> related dtbs
>>>>>
>>>>> ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> ['qcom,ipq8074-tsens'] is too short
>>>>
>>>> Why? It is already there. Open the file and you will see that this is
>>>> already covered.
>>>
>>> I guess dtbs_check doesn't like the same value being a const and
>>> a oneof entry.
>>
>> I don't understand.
>
> - description: v2 of TSENS with combined interrupt
> items:
> - enum:
> - qcom,ipq9574-tsens <--- one of the compatible entries
> - const: qcom,ipq8074-tsens <--- fallback entry
>
> In this patch, we want 8074 to act as a compatible entry for
> ipq8074*.dts and fallback entry for ipq9574.dtsi. That is why I
> believe we are not able to just add 9574 and get it to pass
> dtbs_check and dt_binding_check.
Nope, no other bindings have any problems with that. Fix your syntax as
I said.
>
>>> Have attached the file, please see if something is
>>> not in order.
>>
>> I don't know what changed there. Please work on patches.
>>
>>>
>>>> If you remove it, then yes, you will see errors and the answer is: do
>>>> not remove it.
>>>
>>> I haven't removed it.
>>
>> You did. Look:
>>
>> - description: v2 of TSENS with combined interrupt
>> - enum:
>> - - qcom,ipq8074-tsens
>>
>> The first character in the diff (-) means removal.
>
> It changed from 'enum' to 'const', that is why I said it is not
> removed.
You removed that hunk. Diff hunk. It does not matter that you added
similar one in different place. You removed this one. Don't.
>
>>> For this patch, ipq8074-tsens changed from
>>> being an oneof enum entry to a const entry. Probably, that is why
>>> dtbs_check is giving these errors.
>>
>> You removed the entry which you should not have touched.
>>
>>>
>>>>> ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> ['qcom,ipq8074-tsens'] is too short
>>>>>
>>>>> ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> ['qcom,ipq8074-tsens'] is too short
>>>>>
>>>>> I'm not sure of the correct solution. Having the first hunk
>>>>> solves the above dtbs_check errors, so went with it. I'm able to
>>>>> avoid dtbs_check errors with just one entry in the first hunk.
>>>>
>>>> You made multiple changes in one patch which is not correct. Your goal
>>>> is to add only one change - ipq9574 followed by ipq8074. Add this one.
>>>> Don't touch others.
>>>
>>> But that breaks dtbs_check.
>>
>> All other cases, hundreds of other binding files, do not have problem.
>> Only this one "breaks dtbs_check". No, it does not.
>>
>> Whatever is broken is result of your removal of unrelated pieces.
>
> Not sure about other binding files. Probably they don't have the
> same value for fallback and normal compatible. If there is such
> an example binding file, will replicate that syntax/structure for
> ipq9574 too.
No. There are many examples of other bindings which do not have any
problems with it and what we talk here is common pattern. You created
some fake problem of wrong syntax and then fixed it with different
approach...
>
> In the 'nvidia,tegra210-ope' example (https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L25)
> too 'nvidia,tegra210-ope' is listed twice
>
> - const: nvidia,tegra210-ope <===
> - items:
> - enum:
> - nvidia,tegra234-ope
> - nvidia,tegra194-ope
> - nvidia,tegra186-ope
> - const: nvidia,tegra210-ope <===
>
>>>>> + - const: qcom,ipq8074-tsens
>>>>>
>>>>> Please let me know if there is a better way to resolve this or we
>>>>> can have just the 8074 entry in the first hunk.
>>>>
>>>> You only need to add new item on the oneOf list:
>>>> - enum
>>>> - ipq9574
>>>> - const: ipq8074
>>>
>>> The "['qcom,ipq8074-tsens'] is too short" errors were generated
>>> with the above snippet only. Please see the attachment
>>
>> It's not true. The error you see is result because you removed something
>> you should not. I did not ask you to remove anything. So repeating -
>> "add new item". Adding is not "removal and adding". Adding is just "adding".
>
> See below for the changes that were tried and the corresponding errors.
>
> (1) No lines removed
>
> @@ -66,6 +66,7 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
This does not make sense, right? Let's not discuss solutions which do
not make sense...
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check :
> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
>
> (2) No lines removed
>
> @@ -66,6 +66,8 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
You change existing entry, which breaks it. Don't.
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check : Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
>
>
> (3) No lines removed
> @@ -19,6 +19,7 @@
> properties:
> compatible:
> oneOf:
> + - const: qcom,ipq8074-tsens
> - description: msm8960 TSENS based
> items:
> - enum:
> @@ -66,6 +67,8 @@
> - description: v2 of TSENS with combined interrupt
> enum:
> - qcom,ipq8074-tsens
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
Don't change existing entry.
>
> reg:
> items:
>
> dt_binding_check: Same as above
>
> dtbs_check : Same as above
>
> (4) Change 8074 from enum to const
> @@ -19,6 +19,7 @@
> properties:
> compatible:
> oneOf:
> + - const: qcom,ipq8074-tsens
> - description: msm8960 TSENS based
> items:
> - enum:
> @@ -64,8 +65,10 @@
> - const: qcom,tsens-v2
>
> - description: v2 of TSENS with combined interrupt
> - enum:
> - - qcom,ipq8074-tsens
> + items:
> + - enum:
> + - qcom,ipq9574-tsens
> + - const: qcom,ipq8074-tsens
Don't touch existing entry. Third time.
>
> reg:
> items:
>
> dt_binding_check: No errors
>
> dtbs_check : No errors
>
> But (4) doesn't seem acceptable. Any other alternative to resolve this?
Don't touch existing entry. Fourth time. Add a new one matching your
combination.
Best regards,
Krzysztof