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

[Ilias Apalodimas]

On Wed, 13 Nov 2024 at 04:30, Sam Protsenko <semen.protsenko at linaro.org> wrote:
>
> 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.

Yea perhaps I am nitpicking too much
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>
> > 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
> > >