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

From: Mark Rutland <mark.rutland@arm.com>
To: Mark Brown <broonie@kernel.org>
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 16:03:46 +0000	[thread overview]
Message-ID: <Y5NcYs3UZMNn5bJB@FVFF77S0Q05N> (raw)
In-Reply-To: <Y5M5PSdd8BmPBkbt@sirena.org.uk>

On Fri, Dec 09, 2022 at 01:33:49PM +0000, Mark Brown wrote:
> 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.

Sure, that works for me!

Thanks,
Mark.

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