Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

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