On 12/05/2022 11:32, Sireesh Kodali wrote:
>>>>> + - enum:
>>>>> + - qcom,pronto-v2-pil
>>>>> + - enum:
>>>>> + - qcom,pronto
>>>>
>>>> This does not look correct. The fallback compatible should not change.
>>>> What is more, it was not documented in original binding, so this should
>>>> be done in separate patch.
>>>>
>>>
>>> This was not a change to the fallback compatible.
>>
>> You made it an enum, so you expect it to use different fallback for
>> different cases.
>>
>>> msm8916.dtsi's wcnss
>>> node has "qcom,pronto" as the compatible string, which is why this was
>>> added. It is however not documented in the txt file. Is it sufficient to
>>> add a note in the commit message, or should it be split into a separate
>>> commit?
>>
>> Please split it, assuming that fallback is correct. Maybe the fallback
>> is wrong?
>
> The code doesn't recognize "qcom,pronto", so perhaps the best solution
> is to just remove that compatible from msm8916.dtsi?
Eh, I don't know. You need to check, maybe also in downstream sources.
(...)
>>>>
>>>>> +
>>>>> + iris:
>>>>
>>>> Generic node name... what is "iris"?
>>>>
>>> Iris is the RF module, I'll make the description better
>>
>> RF like wifi? Then the property name should be "wifi".
>
> RF like wifi and bluetooth. However there are wifi and bt subnodes in
> the smd-edge subnode. Iris is just the antenna hardware if I understand
> correctly. Also this is just a documentation of the existing nodes that
> are present in msm8916.dtsi, but for whatever reason their documentation
> was missing in the txt file. Without adding this node in the YAML
> dtb_check fails.
It seems commit fd52bdae9ab0 ("wcn36xx: Disable 5GHz for wcn3620")
added usage of "iris" property but did not document it in the bindings.
You can fix it by documenting (separate patch) existing practice or
document with changing the node name. I am not sure if it is worth the
effort, so just new patch please.
Best regards,
Krzysztof
