[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqLVjWH-x3jxGQpJ3ax_ddvBqHd-P2DE=DEnrknoHuq+Qg@mail.gmail.com>
Date: Thu, 20 Oct 2022 14:51:02 -0500
From: Rob Herring <robh@...nel.org>
To: Mark Brown <broonie@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>,
James Morse <james.morse@....com>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Marc Zyngier <maz@...nel.org>,
Suzuki K Poulose <suzuki.poulose@....com>,
Will Deacon <will@...nel.org>,
Alexandru Elisei <alexandru.elisei@....com>,
Catalin Marinas <catalin.marinas@....com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Oliver Upton <oliver.upton@...ux.dev>,
Jiri Olsa <jolsa@...nel.org>, kvmarm@...ts.cs.columbia.edu,
kvmarm@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/7] arm64/sysreg: Convert SPE registers to automatic generation
On Thu, Oct 20, 2022 at 9:33 AM Mark Brown <broonie@...nel.org> wrote:
>
> On Wed, Oct 19, 2022 at 02:11:26PM -0500, Rob Herring wrote:
> > Convert all the SPE register defines to automatic generation. No
> > functional changes.
> >
> > New registers and fields for SPEv1.2 are added with the conversion.
> >
> > Some of the PMBSR MSS field defines are kept as the automatic generation
> > has no way to create multiple names for the same register bits. The
> > meaning of the MSS field depends on other bits.
>
> A few small things below from checking against DDI0487I.a, nothing
> major:
[...]
> > +Sysreg PMSIDR_EL1 3 0 9 9 7
>
> > +Field 23:20 FORMAT
> > +Field 19:16 COUNTSIZE
> > +Field 15:12 MAXSIZE
> > +Field 11:8 INTERVAL
>
> These should really be enums.
Okay for COUNTSIZE and INTERVAL.
The only defined value for FORMAT is 'Format 0'. I assume next will be
'Format 1'. I don't think defines with the value in the name are too
useful. When this is used, we'll just be passing the value to
userspace via either sysfs or perf aux buffer.
MAXSIZE is the power of 2 encoded size and we just convert to bytes
(1<<MAXSIZE). So I can add them, but they will never be used.
> > +Sysreg PMBLIMITR_EL1 3 0 9 10 0
> > +Enum 2:1 FM
> > + 0b0000 STOP_IRQ
> > +EndEnum
>
> DDI0487I.a also defines
>
> 0b01 DISCARD
STOP_IRQ doesn't seem like the best name either. It's not used, so I
changed it to 'FILL'.
> > +Sysreg PMBIDR_EL1 3 0 9 10 7
> > +Res0 63:12
> > +Field 11:8 EA
>
> This looks like it should be described as an enum.
0b0000 Not_Described
0b0001 Ignored
0b0010 SError
What's the preferred case here?
>
> > +Field 3:0 ALIGN
>
> This could potentially also be an enum.
Another power of 2 encoding.
> > +Sysreg PMSCR_EL2 3 4 9 9 0
> > +Res0 63:8
> > +Field 7:6 PCT
>
> This lookslike it should be an enum.
Humm, PCT is a bit tricky. Here's what I came up with:
Enum 7:6 PCT
0b00 VIRT
0b01 PHYS
0b11 GUEST
EndEnum
Again, CAPS or CarrotCase for new things?
Rob
Powered by blists - more mailing lists