Re: [PATCH v2 3/4] arm64/sysreg: Sort sysreg by encoding

[James Clark <james.clark@linaro.org>]

On 13/03/2025 9:58 pm, Will Deacon wrote:
> On Wed, Jan 15, 2025 at 04:25:57PM +0000, James Clark wrote:
>> It's mostly been sorted by sysreg encoding, but not 100%. Sort it so
>> new entries can be added without wondering where to put them.
>>
>> The following python script was used to sort, keeping the top level
>> SysregFields and comments next to their current Sysreg entries by
>> splitting on "EndSysreg":
>>
>>    # cat arch/arm64/tools/sysreg | python3 sort.py > sorted-sysreg
>>    import sys, re
>>    def key(block):
>>            reg = r"\w+\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)"
>>            match = re.search(reg, block)
>>            sort_val = ''.join(f"{int(n):02d}" for n in match.groups())
>>            return (sort_val, block)
>>    sysreg = sys.stdin.read().split("\nEndSysreg\n")[:-1]
>>    sysreg = sorted(sysreg, key=key)
>>    print("\nEndSysreg\n".join(sysreg) + "\nEndSysreg")
>>
>> Tested by diffing sorted outputs:
>>
>>    $ diff <(sort arch/arm64/include/generated/asm/sysreg-defs.h)  \
>>           <(sort before-sysreg-defs.h) -s
>>
>>    Files /dev/fd/63 and /dev/fd/62 are identical
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>>   arch/arm64/tools/sysreg | 1006 +++++++++++++++++++--------------------
>>   1 file changed, 503 insertions(+), 503 deletions(-)
> 
> This looks like unnecessary pain for backporting...
> 
> What do we gain from sorting this?
> 
> Will

It's from the discussion here [1]. But yeah backporting wasn't mentioned 
as a possible issue, it's something to think about.

The summary is:

  * I added one out of order because it wasn't obvious from the unsorted
    file that they were supposed to be in order
  * To avoid more of the same review comments and save time I wanted to
    put a "keep this file sorted" comment
  * But the comment wasn't ok because the file wasn't already sorted, so
    we ended up with sorting it

To be fair adding a "keep it sorted" comment is a bit awkward because 
there are sometimes multiple places you can insert something if it's not 
already fully sorted.

[1]: 
https://lore.kernel.org/all/996c7843-7f51-49a0-9122-e688e37f9902@sirena.org.uk/