Christophe Leroy's on August 27, 2019 6:13 pm: > SET_MSR_EE() is just use in this file and doesn't provide > any added value compared to mtmsr(). Drop it. > > Add macros to use wrtee/wrteei insn. > > Replace #ifdefs by IS_ENABLED() > > Signed-off-by: Christophe Leroy <christophe.le...@c-s.fr> > --- > arch/powerpc/include/asm/hw_irq.h | 57 > ++++++++++++++++++--------------------- > arch/powerpc/include/asm/reg.h | 2 ++ > 2 files changed, 28 insertions(+), 31 deletions(-) > > diff --git a/arch/powerpc/include/asm/hw_irq.h > b/arch/powerpc/include/asm/hw_irq.h > index 32a18f2f49bc..c25d57f25a8b 100644 > --- a/arch/powerpc/include/asm/hw_irq.h > +++ b/arch/powerpc/include/asm/hw_irq.h > @@ -226,8 +226,8 @@ static inline bool arch_irqs_disabled(void) > #endif /* CONFIG_PPC_BOOK3S */ > > #ifdef CONFIG_PPC_BOOK3E > -#define __hard_irq_enable() asm volatile("wrteei 1" : : : "memory") > -#define __hard_irq_disable() asm volatile("wrteei 0" : : : "memory") > +#define __hard_irq_enable() wrteei(1) > +#define __hard_irq_disable() wrteei(0) > #else > #define __hard_irq_enable() __mtmsrd(MSR_EE|MSR_RI, 1) > #define __hard_irq_disable() __mtmsrd(MSR_RI, 1) > @@ -280,8 +280,6 @@ extern void force_external_irq_replay(void); > > #else /* CONFIG_PPC64 */ > > -#define SET_MSR_EE(x) mtmsr(x) > - > static inline unsigned long arch_local_save_flags(void) > { > return mfmsr(); > @@ -289,47 +287,44 @@ static inline unsigned long arch_local_save_flags(void) > > static inline void arch_local_irq_restore(unsigned long flags) > { > -#if defined(CONFIG_BOOKE) > - asm volatile("wrtee %0" : : "r" (flags) : "memory"); > -#else > - mtmsr(flags); > -#endif > + if (IS_ENABLED(CONFIG_BOOKE)) > + wrtee(flags); > + else > + mtmsr(flags); > } > > static inline unsigned long arch_local_irq_save(void) > { > unsigned long flags = arch_local_save_flags(); > -#ifdef CONFIG_BOOKE > - asm volatile("wrteei 0" : : : "memory"); > -#elif defined(CONFIG_PPC_8xx) > - wrtspr(SPRN_EID); > -#else > - SET_MSR_EE(flags & ~MSR_EE); > -#endif > + > + if (IS_ENABLED(CONFIG_BOOKE)) > + wrteei(0); > + else if (IS_ENABLED(CONFIG_PPC_8xx)) > + wrtspr(SPRN_EID); > + else > + mtmsr(flags & ~MSR_EE); > + > return flags; > } > > static inline void arch_local_irq_disable(void) > { > -#ifdef CONFIG_BOOKE > - asm volatile("wrteei 0" : : : "memory"); > -#elif defined(CONFIG_PPC_8xx) > - wrtspr(SPRN_EID); > -#else > - arch_local_irq_save(); > -#endif > + if (IS_ENABLED(CONFIG_BOOKE)) > + wrteei(0); > + else if (IS_ENABLED(CONFIG_PPC_8xx)) > + wrtspr(SPRN_EID); > + else > + mtmsr(mfmsr() & ~MSR_EE); > } > > static inline void arch_local_irq_enable(void) > { > -#ifdef CONFIG_BOOKE > - asm volatile("wrteei 1" : : : "memory"); > -#elif defined(CONFIG_PPC_8xx) > - wrtspr(SPRN_EIE); > -#else > - unsigned long msr = mfmsr(); > - SET_MSR_EE(msr | MSR_EE); > -#endif > + if (IS_ENABLED(CONFIG_BOOKE)) > + wrteei(1); > + else if (IS_ENABLED(CONFIG_PPC_8xx)) > + wrtspr(SPRN_EIE); > + else > + mtmsr(mfmsr() | MSR_EE); > } > > static inline bool arch_irqs_disabled_flags(unsigned long flags) > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h > index b17ee25df226..04896dd09455 100644 > --- a/arch/powerpc/include/asm/reg.h > +++ b/arch/powerpc/include/asm/reg.h > @@ -1361,6 +1361,8 @@ static inline void mtmsr_isync(unsigned long val) > #endif > #define wrtspr(rn) asm volatile("mtspr " __stringify(rn) ",0" : \ > : : "memory") > +#define wrtee(val) asm volatile("wrtee %0" : : "r" (val) : "memory") > +#define wrteei(val) asm volatile("wrteei %0" : : "i" (val) : "memory")
Can you implement just one macro that uses __builtin_constant_p to select between the imm and reg versions? I forgot if there's some corner cases that prevent that working with inline asm i constraints. Otherwise, looks like a nice cleanup. Thanks, Nick