From: James Clark <james.clark@arm.com>
There are a few entries particularly at the end of the file that aren't
in order. To avoid confusion, add a comment that might help new entries
to be added in the right place.
Reviewed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: James Clark <james.clark@arm.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
arch/arm64/tools/sysreg | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index b081b54d6d22..4ba167089e2a 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -48,6 +48,8 @@
# feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration
# item ACCDATA) though it may be more taseful to do something else.
+# Please try to keep entries in this file sorted by sysreg encoding.
+
Sysreg OSDTRRX_EL1 2 0 0 0 2
Res0 63:32
Field 31:0 DTRRX
--
2.34.1On Tue, 07 Jan 2025 11:32:41 +0000, James Clark <james.clark@linaro.org> wrote: > > From: James Clark <james.clark@arm.com> > > There are a few entries particularly at the end of the file that aren't > in order. To avoid confusion, add a comment that might help new entries > to be added in the right place. > > Reviewed-by: Mark Brown <broonie@kernel.org> > Signed-off-by: James Clark <james.clark@arm.com> > Signed-off-by: James Clark <james.clark@linaro.org> > --- > arch/arm64/tools/sysreg | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index b081b54d6d22..4ba167089e2a 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -48,6 +48,8 @@ > # feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration > # item ACCDATA) though it may be more taseful to do something else. > > +# Please try to keep entries in this file sorted by sysreg encoding. > + > Sysreg OSDTRRX_EL1 2 0 0 0 2 > Res0 63:32 > Field 31:0 DTRRX "Do as I say, don't do as I do". I don't think this makes any sense if we don't actually sort the file the first place. M. -- Without deviation from the norm, progress is not possible.
On 12/01/2025 12:49 pm, Marc Zyngier wrote: > On Tue, 07 Jan 2025 11:32:41 +0000, > James Clark <james.clark@linaro.org> wrote: >> >> From: James Clark <james.clark@arm.com> >> >> There are a few entries particularly at the end of the file that aren't >> in order. To avoid confusion, add a comment that might help new entries >> to be added in the right place. >> >> Reviewed-by: Mark Brown <broonie@kernel.org> >> Signed-off-by: James Clark <james.clark@arm.com> >> Signed-off-by: James Clark <james.clark@linaro.org> >> --- >> arch/arm64/tools/sysreg | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg >> index b081b54d6d22..4ba167089e2a 100644 >> --- a/arch/arm64/tools/sysreg >> +++ b/arch/arm64/tools/sysreg >> @@ -48,6 +48,8 @@ >> # feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration >> # item ACCDATA) though it may be more taseful to do something else. >> >> +# Please try to keep entries in this file sorted by sysreg encoding. >> + >> Sysreg OSDTRRX_EL1 2 0 0 0 2 >> Res0 63:32 >> Field 31:0 DTRRX > > "Do as I say, don't do as I do". > > I don't think this makes any sense if we don't actually sort the file > the first place. > > M. > I think it's ok if it avoids review comments that new entries should be sorted. Or maybe we do the opposite and the comment should say this file is allowed to be unsorted...
On Mon, 13 Jan 2025 15:43:39 +0000, James Clark <james.clark@linaro.org> wrote: > > > > On 12/01/2025 12:49 pm, Marc Zyngier wrote: > > On Tue, 07 Jan 2025 11:32:41 +0000, > > James Clark <james.clark@linaro.org> wrote: > >> > >> From: James Clark <james.clark@arm.com> > >> > >> There are a few entries particularly at the end of the file that aren't > >> in order. To avoid confusion, add a comment that might help new entries > >> to be added in the right place. > >> > >> Reviewed-by: Mark Brown <broonie@kernel.org> > >> Signed-off-by: James Clark <james.clark@arm.com> > >> Signed-off-by: James Clark <james.clark@linaro.org> > >> --- > >> arch/arm64/tools/sysreg | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > >> index b081b54d6d22..4ba167089e2a 100644 > >> --- a/arch/arm64/tools/sysreg > >> +++ b/arch/arm64/tools/sysreg > >> @@ -48,6 +48,8 @@ > >> # feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration > >> # item ACCDATA) though it may be more taseful to do something else. > >> +# Please try to keep entries in this file sorted by sysreg > >> encoding. > >> + > >> Sysreg OSDTRRX_EL1 2 0 0 0 2 > >> Res0 63:32 > >> Field 31:0 DTRRX > > > > "Do as I say, don't do as I do". > > > > I don't think this makes any sense if we don't actually sort the file > > the first place. > > > > M. > > > > I think it's ok if it avoids review comments that new entries should > be sorted. Or maybe we do the opposite and the comment should say this > file is allowed to be unsorted... The better option would be to add the comment *and* sort the file. Leading by example has some value, it seems. M. -- Without deviation from the norm, progress is not possible.
On Mon, Jan 13, 2025 at 10:49 AM Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 13 Jan 2025 15:43:39 +0000, > James Clark <james.clark@linaro.org> wrote: > > > > > > > > On 12/01/2025 12:49 pm, Marc Zyngier wrote: > > > On Tue, 07 Jan 2025 11:32:41 +0000, > > > James Clark <james.clark@linaro.org> wrote: > > >> > > >> From: James Clark <james.clark@arm.com> > > >> > > >> There are a few entries particularly at the end of the file that aren't > > >> in order. To avoid confusion, add a comment that might help new entries > > >> to be added in the right place. > > >> > > >> Reviewed-by: Mark Brown <broonie@kernel.org> > > >> Signed-off-by: James Clark <james.clark@arm.com> > > >> Signed-off-by: James Clark <james.clark@linaro.org> > > >> --- > > >> arch/arm64/tools/sysreg | 2 ++ > > >> 1 file changed, 2 insertions(+) > > >> > > >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > > >> index b081b54d6d22..4ba167089e2a 100644 > > >> --- a/arch/arm64/tools/sysreg > > >> +++ b/arch/arm64/tools/sysreg > > >> @@ -48,6 +48,8 @@ > > >> # feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration > > >> # item ACCDATA) though it may be more taseful to do something else. > > >> +# Please try to keep entries in this file sorted by sysreg > > >> encoding. > > >> + > > >> Sysreg OSDTRRX_EL1 2 0 0 0 2 > > >> Res0 63:32 > > >> Field 31:0 DTRRX > > > > > > "Do as I say, don't do as I do". > > > > > > I don't think this makes any sense if we don't actually sort the file > > > the first place. > > > > > > M. > > > > > > > I think it's ok if it avoids review comments that new entries should > > be sorted. Or maybe we do the opposite and the comment should say this > > file is allowed to be unsorted... > > The better option would be to add the comment *and* sort the file. > Leading by example has some value, it seems. IME, it's better if documentation just states what the tools enforce. Can't we add something like this to the header generation: $ grep '^Sysreg\s' arch/arm64/tools/sysreg | sort -n -k3 -k4 -k5 -k6 -k7 -c sort: -:22: disorder: Sysreg ID_MMFR4_EL1 3 0 0 2 6 Rob
On 14/01/2025 6:16 pm, Rob Herring wrote: > On Mon, Jan 13, 2025 at 10:49 AM Marc Zyngier <maz@kernel.org> wrote: >> >> On Mon, 13 Jan 2025 15:43:39 +0000, >> James Clark <james.clark@linaro.org> wrote: >>> >>> >>> >>> On 12/01/2025 12:49 pm, Marc Zyngier wrote: >>>> On Tue, 07 Jan 2025 11:32:41 +0000, >>>> James Clark <james.clark@linaro.org> wrote: >>>>> >>>>> From: James Clark <james.clark@arm.com> >>>>> >>>>> There are a few entries particularly at the end of the file that aren't >>>>> in order. To avoid confusion, add a comment that might help new entries >>>>> to be added in the right place. >>>>> >>>>> Reviewed-by: Mark Brown <broonie@kernel.org> >>>>> Signed-off-by: James Clark <james.clark@arm.com> >>>>> Signed-off-by: James Clark <james.clark@linaro.org> >>>>> --- >>>>> arch/arm64/tools/sysreg | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg >>>>> index b081b54d6d22..4ba167089e2a 100644 >>>>> --- a/arch/arm64/tools/sysreg >>>>> +++ b/arch/arm64/tools/sysreg >>>>> @@ -48,6 +48,8 @@ >>>>> # feature that introduces them (eg, FEAT_LS64_ACCDATA introduces enumeration >>>>> # item ACCDATA) though it may be more taseful to do something else. >>>>> +# Please try to keep entries in this file sorted by sysreg >>>>> encoding. >>>>> + >>>>> Sysreg OSDTRRX_EL1 2 0 0 0 2 >>>>> Res0 63:32 >>>>> Field 31:0 DTRRX >>>> >>>> "Do as I say, don't do as I do". >>>> >>>> I don't think this makes any sense if we don't actually sort the file >>>> the first place. >>>> >>>> M. >>>> >>> >>> I think it's ok if it avoids review comments that new entries should >>> be sorted. Or maybe we do the opposite and the comment should say this >>> file is allowed to be unsorted... >> >> The better option would be to add the comment *and* sort the file. >> Leading by example has some value, it seems. > > IME, it's better if documentation just states what the tools enforce. > > Can't we add something like this to the header generation: > > $ grep '^Sysreg\s' arch/arm64/tools/sysreg | sort -n -k3 -k4 -k5 -k6 -k7 -c > sort: -:22: disorder: Sysreg ID_MMFR4_EL1 3 0 0 > 2 6 > > Rob Actually I updated gen-sysreg.awk to fail the build if it's not sorted, was just about to post it. I don't know if a build failure or a warning is preferred but I can do either. James
On Wed, Jan 15, 2025 at 10:43:07AM +0000, James Clark wrote: > Actually I updated gen-sysreg.awk to fail the build if it's not sorted, was > just about to post it. I don't know if a build failure or a warning is > preferred but I can do either. A build failure is probably safer, people are more likely to notice it and warnings from things that aren't the compiler can get a bit shaky with tooling sometimes.
On Tue, Jan 14, 2025 at 12:16:27PM -0600, Rob Herring wrote: > On Mon, Jan 13, 2025 at 10:49 AM Marc Zyngier <maz@kernel.org> wrote: > > The better option would be to add the comment *and* sort the file. > > Leading by example has some value, it seems. > IME, it's better if documentation just states what the tools enforce. > Can't we add something like this to the header generation: > $ grep '^Sysreg\s' arch/arm64/tools/sysreg | sort -n -k3 -k4 -k5 -k6 -k7 -c > sort: -:22: disorder: Sysreg ID_MMFR4_EL1 3 0 0 > 2 6 Sounds good to me.
On Mon, Jan 13, 2025 at 04:49:49PM +0000, Marc Zyngier wrote: > James Clark <james.clark@linaro.org> wrote: > > I think it's ok if it avoids review comments that new entries should > > be sorted. Or maybe we do the opposite and the comment should say this > > file is allowed to be unsorted... > The better option would be to add the comment *and* sort the file. > Leading by example has some value, it seems. Yes, please.
© 2016 - 2025 Red Hat, Inc.