[PATCH v2 1/2] armv8: Fix get_sctlr() return type

[Sam Protsenko]

On Mon, Nov 11, 2024 at 8:48 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Thu, 7 Nov 2024 at 04:58, Sam Protsenko <semen.protsenko at linaro.org> wrote:
> >
> > SCTLR_EL2 is a 64-bit register [1]. Return its value as long (64 bit)
> > instead of int (32 bit) in get_sctlr() to make sure it's not trimmed.
> >
> > [1] https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/SCTLR-EL2--System-Control-Register--EL2-?lang=en
> >
> > Fixes: 0ae7653128c8 ("arm64: core support")
> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > ---
> > Changes in v2:
> >   - None (this patch was introduced in v2)
> >
> >  arch/arm/cpu/armv8/cache_v8.c | 2 +-
> >  arch/arm/include/asm/system.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> > index e6be6359c5d9..5d6953ffedd1 100644
> > --- a/arch/arm/cpu/armv8/cache_v8.c
> > +++ b/arch/arm/cpu/armv8/cache_v8.c
> > @@ -825,7 +825,7 @@ void dcache_enable(void)
> >
> >  void dcache_disable(void)
> >  {
> > -       uint32_t sctlr;
> > +       unsigned long sctlr;
>
> Although that's correct since it's a 64bit platform, isn't it better
> to define it as u64 to be sure we'll have at least 64 bits?
>

Thanks for reviewing this! I considered making it uint64_t while
reworking this bit. This 'sctlr' variable is assigned to get_sctlr()
return value, and then passed to set_sctlr() as a parameter, both of
which are 'unsigned long'. So I made 'sctlr' unsigned long too, just
to match the signatures of mentioned functions. As for get_sctlr(), I
just followed the style already existed inside and around that
function. As you said, both u64 and unsigned long are 64-bit long
types on ARMv8, and that's definitely ARMv8 specific file, so I guess
that should be fine? In any case, if you have a strong opinion about
this, please let me know and I'll rework it; both u64 and unsigned
long look ok to me in this context.

> Thanks
> /Ilias
> >
> >         sctlr = get_sctlr();
> >
> > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > index 52f6c9b934d7..dbf9ab43e280 100644
> > --- a/arch/arm/include/asm/system.h
> > +++ b/arch/arm/include/asm/system.h
> > @@ -171,7 +171,7 @@ static inline unsigned int current_el(void)
> >         return 3 & (el >> 2);
> >  }
> >
> > -static inline unsigned int get_sctlr(void)
> > +static inline unsigned long get_sctlr(void)
> >  {
> >         unsigned int el;
> >         unsigned long val;
> > --
> > 2.39.5
> >