Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 03/04/2023 12:00, Konrad Dybcio wrote:
> 
> 
> On 3.04.2023 11:50, Dmitry Baryshkov wrote:
>> On 03/04/2023 12:09, Krzysztof Kozlowski wrote:
>>> On 02/04/2023 13:02, Dmitry Baryshkov wrote:
>>>> On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
>>>>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>>>>> Correct the thermal zone node name to remove the clash with
>>>>>> pm8350c.dtsi. Remove unused labels.
>>>>>>
>>>>>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
>>>>>
>>>>> Please describe the observable bug from that commit being fixed here.
>>>>> Any future clash, which did not exist that time, is not a bug. It's future.
>>>>>
>>>>> Naming changes here are more a matter of style, because the old names
>>>>> were correct according to our coding guidelines, just not precise (c
>>>>> instead of b). But node names anyway are not important from the point of
>>>>> view fixes and adding such tag will cause a needless backport.
>>>>
>>>> It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result
>>>> in one thermal zone overriding another one.
>>>
>>> I don't understand. You used future tense "will", but we talk about
>>> past. So where is the bug in commit 5c1399299d9d?
>>
>> At that time there already existed sm8350-mtp which included both of dtsi files.
>>
> pm8350[bc] both have a node named /pm8350c-thermal, which means one will
> get overriden with the other. Since we sort the includes alphabetically,
> the one in pm8350b.dtsi will never be taken into account and hence the
> temp alarm will never be associated to any thermal zone.

OK, if I understand correctly, this overwrite of thermal node was
already happening at commit 5c1399299d9d. It looks good then.

Best regards,
Krzysztof