Re: [PATCH 4/6] arm64/sysreg: Annotate signed enumerations

From: Mark Brown <broonie@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	James Morse <james.morse@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 4/6] arm64/sysreg: Annotate signed enumerations
Date: Fri, 9 Dec 2022 13:33:49 +0000	[thread overview]
Message-ID: <Y5M5PSdd8BmPBkbt@sirena.org.uk> (raw)
In-Reply-To: <Y5MtIskhPZGqjA1u@FVFF77S0Q05N>

[-- Attachment #1.1: Type: text/plain, Size: 1629 bytes --]

On Fri, Dec 09, 2022 at 12:42:10PM +0000, Mark Rutland wrote:
> On Thu, Dec 08, 2022 at 04:03:25PM +0000, Mark Brown wrote:

> > ID_AA64PFR0_EL1.FP and ID_AA64PFR0_EL1.AdvSIMD are both signed enumerations,
> > specify them as such in sysreg. There are other signed enumerations in the
> > registers but these are the only ones for which we currently use FTR_SIGNED,
> > others can be fixed up incrementally.

> Can we please do that in one go (either in one patch or a set of patches in
> this series)?

> I appreciate that's more work up-front, but doing that will mean that all the
> definitions are in a consistent state, which'll be far less error prone going
> forwards -- people will *definitely* forget to change the other existing
> definitions to be SIGNED if all that is hidden at the point-of-use.

I am not sure I will get round to doing all that at once in a reasonable
timeframe on what is basically a low priority background task.  I'd much
rather just leave the use of FTR_SIGNED/UNSIGNED in the C code, I only
did this because I initially did that conversion and the repetitiveness
was jumping out as obvious.

> If we do that, we may as well explicitly annotate the UNSIGNED enums (and those
> which are purely enums without a sign) at the same time. That'll indicate that
> we've reviewed each entry, and it'll make it far more obvious one must do so
> when adding new entries in future.

We could also just leave Enum as unspecified, do all the UnsignedEnums,
and leave enum as unspecified and not generating a sign constant, that
any users that care about the sign of an enum won't get an incorrect
default.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel