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

[James Clark]

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@xxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxx/