Re: [PATCH 4/9] dtc: dt-check-style: Handle sorting of top-level nodes and properties

Krzysztof Kozlowski <[email protected]> Sat, 1 Aug 2026 23:31:55 +0200
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
On 01/08/2026 23:19, Krzysztof Kozlowski wrote:
> On 26/07/2026 17:57, [email protected] wrote:
>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>> - [Medium] Tree hierarchy tracking in `_walk_bodies()` is broken, incorrectly assigning closed sibling nodes as parents to subsequent siblings.
>> - [Medium] `_property_bucket_root()` collapses standard and vendor properties into a single bucket, breaking canonical property ordering at the root node.
>> - [Medium] Top-level addressed nodes are exempted from address sorting but are never subjected to the node name sorting claimed in the commit message.
>> --
>>
>> --- Patch [4]: [PATCH 4/9] dtc: dt-check-style: Handle sorting of top-level nodes and properties ---
>> commit d752014f34dc3d4a2a9f9617748ec741da0cb1c5
>> Author: Krzysztof Kozlowski <[email protected]>
>>
>> dtc: dt-check-style: Handle sorting of top-level nodes and properties
>>
>> This commit updates the DTS styling rules for top-level nodes and properties.
>> It skips unit-address sorting for root nodes since they mix unit and non-unit
>> addresses. It also adds a special case for the 'model' property to sort before
>> 'compatible' at the root node.
>>
>>> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
>>> index 1937c6feb57ec..db4451e30dfbc 100755
>>> --- a/scripts/dtc/dt-check-style
>>> +++ b/scripts/dtc/dt-check-style
>>
>> [ ... ]
>>
>>> @@ -496,15 +497,21 @@ def _walk_bodies(lines):
>>>      in the input. Skips ref-nodes (&label) since those don't have an
>>>      intrinsic ordering."""
>>>      body_stack = [[]]
>>> +    node_stack = [[]]
>>> +    parent_dl = None
>>>      for dl in lines:
>>>          if dl.linetype == LineType.NODE_OPEN:
>>> +            dl.parent = parent_dl
>>> +            node_stack.append(dl)
>>>              body_stack[-1].append(dl)
>>>              body_stack.append([])
>>> +            parent_dl = dl
>>>              continue
>>>          if dl.linetype == LineType.NODE_CLOSE:
>>>              if len(body_stack) <= 1:
>>>                  # Unbalanced; ignore to avoid crashing on malformed input
>>>                  continue
>>> +            parent_dl = node_stack.pop()
>>
>> [Severity: Medium]
>> Does this pop the node that just closed instead of the actual parent?
>>
>> When node_stack.pop() is called here, it returns the node that just closed
>> because that was the last one appended. Does this cause subsequent siblings
> 
> That's intended, since we go up in the hierarchy. Current parent should
> be now what we left on the stack.

Actually this was right.

Best regards,
Krzysztof