diff mbox series

[v2,08/20] mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs

Message ID 20200506174238.15385-9-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Superseded
Headers show
Series None | expand

Commit Message

Serge Semin May 6, 2020, 5:42 p.m. UTC 
From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Commit 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA
allows") updated the cpu_has_mips* macro to be replaced with a constant
expression where it's possible. By mistake it wasn't done correctly
for cpu_has_mips64r1/cpu_has_mips64r2 macro. They are defined to
be replaced with conditional expression __isa_range_or_flag(), which
means either ISA revision being within the range or the corresponding
CPU options flag was set at the probe stage or both being true at the
same time. But the ISA level value doesn't indicate whether the ISA is
MIPS32 or MIPS64. Due to this if we select MIPS32r1 - MIPS32r5
architectures the __isa_range() macro will activate the
cpu_has_mips64rX flags, which is incorrect. In order to fix the
problem we added a new macro __isa_range_and_flag() and use it to
define the cpu_has_mips64r1/cpu_has_mips64r2 flags.

Fixes: 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA allows")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 arch/mips/include/asm/cpu-features.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Thomas Bogendoerfer May 8, 2020, 1:28 p.m. UTC | #1 
On Wed, May 06, 2020 at 08:42:26PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> index e2f31bd6363b..7e22b9c1e279 100644
> --- a/arch/mips/include/asm/cpu-features.h
> +++ b/arch/mips/include/asm/cpu-features.h
> @@ -64,6 +64,8 @@
>  	((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
>  #define __isa_range_or_flag(ge, lt, flag) \
>  	(__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
> +#define __isa_range_and_flag(ge, lt, flag) \
> +	(__isa_range(ge, lt) && __isa(flag))
>  
>  /*
>   * SMP assumption: Options of CPU 0 are a superset of all processors.
> @@ -291,10 +293,10 @@
>  # define cpu_has_mips32r6	__isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
>  #endif
>  #ifndef cpu_has_mips64r1
> -# define cpu_has_mips64r1	__isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
> +# define cpu_has_mips64r1	__isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)

that's not the correct fix. You want to check for cpu_has_64bits here.
Something like 

# define cpu_has_mips64r1    (cpu_has_64bits && __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1))

should do the trick.

Thomas.
Serge Semin May 10, 2020, 11:59 p.m. UTC | #2 
On Fri, May 08, 2020 at 03:28:09PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:26PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> > index e2f31bd6363b..7e22b9c1e279 100644
> > --- a/arch/mips/include/asm/cpu-features.h
> > +++ b/arch/mips/include/asm/cpu-features.h
> > @@ -64,6 +64,8 @@
> >  	((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
> >  #define __isa_range_or_flag(ge, lt, flag) \
> >  	(__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
> > +#define __isa_range_and_flag(ge, lt, flag) \
> > +	(__isa_range(ge, lt) && __isa(flag))
> >  
> >  /*
> >   * SMP assumption: Options of CPU 0 are a superset of all processors.
> > @@ -291,10 +293,10 @@
> >  # define cpu_has_mips32r6	__isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
> >  #endif
> >  #ifndef cpu_has_mips64r1
> > -# define cpu_has_mips64r1	__isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
> > +# define cpu_has_mips64r1	__isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)
> 
> that's not the correct fix. You want to check for cpu_has_64bits here.
> Something like 
> 
> # define cpu_has_mips64r1    (cpu_has_64bits && __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1))
> 
> should do the trick.

Good point. Thanks. I'll fix it in v3.

-Sergey

> 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]
diff mbox series

Patch

diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index e2f31bd6363b..7e22b9c1e279 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -64,6 +64,8 @@ 
 	((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
 #define __isa_range_or_flag(ge, lt, flag) \
 	(__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
+#define __isa_range_and_flag(ge, lt, flag) \
+	(__isa_range(ge, lt) && __isa(flag))
 
 /*
  * SMP assumption: Options of CPU 0 are a superset of all processors.
@@ -291,10 +293,10 @@ 
 # define cpu_has_mips32r6	__isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
 #endif
 #ifndef cpu_has_mips64r1
-# define cpu_has_mips64r1	__isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
+# define cpu_has_mips64r1	__isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)
 #endif
 #ifndef cpu_has_mips64r2
-# define cpu_has_mips64r2	__isa_range_or_flag(2, 6, MIPS_CPU_ISA_M64R2)
+# define cpu_has_mips64r2	__isa_range_and_flag(2, 6, MIPS_CPU_ISA_M64R2)
 #endif
 #ifndef cpu_has_mips64r6
 # define cpu_has_mips64r6	__isa_ge_and_flag(6, MIPS_CPU_ISA_M64R6)