Message-ID: <0df01d9863f72df3bebff1868f010954@manjaro.org>
Date: Fri, 07 Mar 2025 10:50:16 +0100
From: Dragan Simic <dsimic@...jaro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 heiko@...ech.de, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] docs: dt-bindings: Specify ordering for properties
 within groups

Hello Krzysztof,

On 2025-03-07 10:28, Krzysztof Kozlowski wrote:
> On Fri, Mar 07, 2025 at 05:33:37AM +0100, Dragan Simic wrote:
>> When it comes to ordering of the individual properties inside each 
>> property
>> group, applying natural sort order to multi-digit numbers [1] found 
>> inside
>> the property names can result in more logical and more usable property 
>> lists,
>> similarly to what's already the case with the alpha-numerical ordering 
>> of
>> the nodes without unit addresses.
>> 
>> Let's have this clearly specified in the DTS coding style, together 
>> with
>> a brief description of the natural sort order for those readers who 
>> aren't
>> familiar with it already.  Also expand the provided node example a 
>> bit, to
>> actually show the results of applying natural sort order.
>> 
>> Applying strict alpha-numerical ordering can result in property lists 
>> that
>> are suboptimal from the usability standpoint.  For the provided 
>> example,
>> which stems from a real-world DT, [2][3][4] applying strict 
>> alpha-numerical
>> ordering produces the following undesirable result:
> 
> BTW, your entire commit msg still has incorrect wrapping. Please use
> standard editors which understand Git commit msg style (which I believe
> is equal to Linux submitting patches).

Sorry, but what makes the wrapping wrong?  I wrap patch descriptions
at or well before 78 columns, which I belive is the desired way to do
so.  The longest line in this patch description is 77 characters long,
which is still below 78 characters.

Also, please note that I don't "just use" some random editor, but I use
a highly customized Vim setup, in which I over time wrote about 46 KB
of Vimscript code in my ~/.vimrc, all by hand.  I've even contributed
smaller patches to Vim.

>>   vdd-0v9-supply = <&board_vreg1>;
>>   vdd-12v-supply = <&board_vreg3>;
>>   vdd-1v8-supply = <&board_vreg4>;
>>   vdd-3v3-supply = <&board_vreg2>;
>> 
>> Having the properties sorted in natural order by their associated 
>> voltages
>> is more logical, more usable, and a bit more consistent.
>> 
>> [1] https://en.wikipedia.org/wiki/Natural_sort_order
>> [2] 
>> https://lore.kernel.org/linux-rockchip/b39cfd7490d8194f053bf3971f13a43472d1769e.1740941097.git.dsimic@manjaro.org/
>> [3] 
>> https://lore.kernel.org/linux-rockchip/174104113599.8946.16805724674396090918.b4-ty@sntech.de/
>> [4] 
>> https://lore.kernel.org/linux-rockchip/757afa87255212dfa5abf4c0e31deb08@manjaro.org/
>> 
>> Signed-off-by: Dragan Simic <dsimic@...jaro.org>
>> ---
>> 
>> Notes:
>>     Changes in v3:
>>       - Rewrote the part of the changes that describes natural sort 
>> order
>>         and its relation to "standard" alpha-numerical ordering, to 
>> make
>>         it more understandable, as suggested by Krzysztof [6]
>>       - Slightly expanded the patch description, to clarify the 
>> additional
>>         goal of explaining the natural sort order briefly
>> 
>>     Changes in v2:
>>       - Changed the additions to the coding style to specify natural 
>> sort
>>         order, which avoids amibguity, as suggested by Krzysztof [5]
>>       - Adjusted and expanded the patch description appropriately, 
>> together
>>         with including one more reference for the natural sort order
>> 
>>     Link to v1: 
>> https://lore.kernel.org/linux-kernel/09d6f2fc111b3d6e58987336944f93ec36b65118.1741071107.git.dsimic@manjaro.org/T/#u
>>     Link to v2: 
>> https://lore.kernel.org/linux-kernel/47c51c10098f089e52fb14c5c5527611dc8daf32.1741164239.git.dsimic@manjaro.org/T/#u
>> 
>>     [5] 
>> https://lore.kernel.org/linux-kernel/20250305-defiant-serious-newt-b7c5ea@krzk-bin/
>>     [6] 
>> https://lore.kernel.org/linux-kernel/20250306-dexterous-goshawk-of-aptitude-e4f1f6@krzk-bin/
>> 
>>  .../devicetree/bindings/dts-coding-style.rst       | 14 
>> +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/dts-coding-style.rst 
>> b/Documentation/devicetree/bindings/dts-coding-style.rst
>> index 8a68331075a0..7d183c1ade74 100644
>> --- a/Documentation/devicetree/bindings/dts-coding-style.rst
>> +++ b/Documentation/devicetree/bindings/dts-coding-style.rst
>> @@ -133,6 +133,15 @@ The above-described ordering follows this 
>> approach:
>>  3. Status is the last information to annotate that device node is or 
>> is not
>>     finished (board resources are needed).
>> 
>> +The above-described ordering specifies the preferred ordering of 
>> property
>> +groups, while the individual properties inside each group shall use 
>> natural
>> +sort order by the property name.
> 
> I guess you sent it after my today's reply. Full stop, done. Such
> trivial thing as sorting should have one line. Three lines is already
> longish, but fine, especially that it can be one line.
> 
> "Properties within each of above groups should use natural sort order."

If you insist that the brief sentence you provided is to be used,
I can live with it, but I have to note that it could be improved
from the style and grammar viewpoints.

Could we, please, settle on using the version I proposed above,
together with dropping the additional description, visible below?

>> +
>> +The natural sort order differs from alpha-numerical ordering only by 
>> treating
>> +any single- and multi-digit numbers found inside the property names 
>> atomically,
>> +and by taking their actual numerical values and comparing those 
>> values between
>> +themselves to determine the order of property names.
> 
> 8 lines is 8 times too long.

Alright, I'll drop that additional description.  I can agree that it
might be seen as redundant.

Powered by blists - more mailing lists