Re: [PATCH v2] docs: dt-bindings: add DTS Coding Style document

Krzysztof Kozlowski
2023-11-25T18:45:24Z

On 22/11/2023 15:55, Rob Herring wrote:
> On Wed, Nov 22, 2023 at 1:05 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/11/2023 14:50, Rafał Miłecki wrote:
>>>> +Order of Nodes
>>>> +--------------
>>>> +
>>>> +1. Nodes within any bus, thus using unit addresses for children, shall be
>>>> + ordered incrementally by unit address.
>>>> + Alternatively for some sub-architectures, nodes of the same type can be
>>>> + grouped together (e.g. all I2C controllers one after another even if this
>>>> + breaks unit address ordering).
>>>> +
>>>> +2. Nodes without unit addresses should be ordered alpha-numerically by the node
>>>> + name. For a few types of nodes, they can be ordered by the main property
>>>> + (e.g. pin configuration states ordered by value of "pins" property).
>>>> +
>>>> +3. When extending nodes in the board DTS via &label, the entries should be
>>>> + ordered alpha-numerically.
>>>
>>> Just an idea. Would that make (more) sense to make &label-like entries
>>> match order of nodes in included .dts(i)?
>>>
>>> Adventages:
>>> 1. We keep unit address incremental order that is unlikely to change
>>>
>>> Disadventages:
>>> 1. More difficult to verify
>>
>> Rob also proposed this and I believe above disadvantage here is crucial.
>> If you add new SoC with board DTS you are fine. But if you add only new
>> board, the order of entries look random in the diff hunk. Reviewer must
>> open SoC DTSI to be able to review the patch with board DTS.
>>
>> If review is tricky and we do not have tool to perform it automatically,
>> I am sure submissions will have disordered board DTS.
>
> I'm certainly in favor of only (or mostly?) specifying things we can
> check with tools. I don't need more to check manually...
>
> It wouldn't be too hard to get path from labels. dtc generates that
> with -@ already. So I don't buy "we don't have a tool" if a tool to
> check seems feasible.

OK, then tool is not an argument. In Qualcomm and Samsung we already use
alphabetical order in board DTS, so keeping that existing style could be
an argument. I don't have strong preference, except the need to
re-shuffle all DTS files which would be a quite disastrous for future
stable-backports. I can mention both styles.

Best regards,
Krzysztof