* [PATCH 1/2] powerpc: permanently include 8xx registers in reg.h @ 2019-08-27 8:13 Christophe Leroy 2019-08-27 8:13 ` [PATCH 2/2] powerpc: cleanup hw_irq.h Christophe Leroy 0 siblings, 1 reply; 8+ messages in thread From: Christophe Leroy @ 2019-08-27 8:13 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman Cc: linux-kernel, linuxppc-dev Most 8xx registers have specific names, so just include reg_8xx.h all the time in reg.h in order to have them defined even when CONFIG_PPC_8xx is not selected. This will avoid the need for #ifdefs in C code. Guard SPRN_ICTRL in an #ifdef CONFIG_PPC_8xx as this register has same name but different meaning and different spr number as another register in the mpc7450. Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/include/asm/reg.h | 2 -- arch/powerpc/include/asm/reg_8xx.h | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 10caa145f98b..b17ee25df226 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -25,9 +25,7 @@ #include <asm/reg_fsl_emb.h> #endif -#ifdef CONFIG_PPC_8xx #include <asm/reg_8xx.h> -#endif /* CONFIG_PPC_8xx */ #define MSR_SF_LG 63 /* Enable 64 bit mode */ #define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */ diff --git a/arch/powerpc/include/asm/reg_8xx.h b/arch/powerpc/include/asm/reg_8xx.h index 7192eece6c3e..abc663c0f1db 100644 --- a/arch/powerpc/include/asm/reg_8xx.h +++ b/arch/powerpc/include/asm/reg_8xx.h @@ -38,7 +38,9 @@ #define SPRN_CMPF 153 #define SPRN_LCTRL1 156 #define SPRN_LCTRL2 157 +#ifdef CONFIG_PPC_8xx #define SPRN_ICTRL 158 +#endif #define SPRN_BAR 159 /* Commands. Only the first few are available to the instruction cache. -- 2.13.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 8:13 [PATCH 1/2] powerpc: permanently include 8xx registers in reg.h Christophe Leroy @ 2019-08-27 8:13 ` Christophe Leroy 2019-08-27 12:48 ` Nicholas Piggin 0 siblings, 1 reply; 8+ messages in thread From: Christophe Leroy @ 2019-08-27 8:13 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman Cc: linux-kernel, linuxppc-dev 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.leroy@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") extern unsigned long msr_check_and_set(unsigned long bits); extern bool strict_msr_control; -- 2.13.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 8:13 ` [PATCH 2/2] powerpc: cleanup hw_irq.h Christophe Leroy @ 2019-08-27 12:48 ` Nicholas Piggin 2019-08-27 17:29 ` Segher Boessenkool 0 siblings, 1 reply; 8+ messages in thread From: Nicholas Piggin @ 2019-08-27 12:48 UTC (permalink / raw) To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman, Paul Mackerras Cc: linux-kernel, linuxppc-dev 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.leroy@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 12:48 ` Nicholas Piggin @ 2019-08-27 17:29 ` Segher Boessenkool 2019-08-27 17:36 ` Christophe Leroy 0 siblings, 1 reply; 8+ messages in thread From: Segher Boessenkool @ 2019-08-27 17:29 UTC (permalink / raw) To: Nicholas Piggin Cc: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel On Tue, Aug 27, 2019 at 10:48:24PM +1000, Nicholas Piggin wrote: > Christophe Leroy's on August 27, 2019 6:13 pm: > > +#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. static inline void wrtee(long val) { asm volatile("wrtee%I0 %0" : : "n"(val) : "memory"); } (This output modifier goes back to the dark ages, some 2.4 or something). Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 17:29 ` Segher Boessenkool @ 2019-08-27 17:36 ` Christophe Leroy 2019-08-27 18:26 ` Segher Boessenkool 0 siblings, 1 reply; 8+ messages in thread From: Christophe Leroy @ 2019-08-27 17:36 UTC (permalink / raw) To: Segher Boessenkool, Nicholas Piggin Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel Le 27/08/2019 à 19:29, Segher Boessenkool a écrit : > On Tue, Aug 27, 2019 at 10:48:24PM +1000, Nicholas Piggin wrote: >> Christophe Leroy's on August 27, 2019 6:13 pm: >>> +#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. > > static inline void wrtee(long val) > { > asm volatile("wrtee%I0 %0" : : "n"(val) : "memory"); > } Great, didn't know that possibility. Can it be used with any insn, for instance with add/addi ? Or with mr/li ? > > (This output modifier goes back to the dark ages, some 2.4 or something). Hope Clang support it ... Christophe > > > Segher > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 17:36 ` Christophe Leroy @ 2019-08-27 18:26 ` Segher Boessenkool 2019-08-27 18:33 ` Christophe Leroy 0 siblings, 1 reply; 8+ messages in thread From: Segher Boessenkool @ 2019-08-27 18:26 UTC (permalink / raw) To: Christophe Leroy Cc: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel On Tue, Aug 27, 2019 at 07:36:35PM +0200, Christophe Leroy wrote: > Le 27/08/2019 à 19:29, Segher Boessenkool a écrit : > >On Tue, Aug 27, 2019 at 10:48:24PM +1000, Nicholas Piggin wrote: > >>Christophe Leroy's on August 27, 2019 6:13 pm: > >>>+#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. > > > >static inline void wrtee(long val) > >{ > > asm volatile("wrtee%I0 %0" : : "n"(val) : "memory"); > >} > > Great, didn't know that possibility. > > Can it be used with any insn, for instance with add/addi ? > Or with mr/li ? Any instruction, yes. %I<n> simply outputs an "i" if operand n is a constant integer, and nothing otherwise. So asm("add%I2 %0,%1,%2" : "=r"(dst) : "r"(src1), "ri"(src1)); works well. I don't see how you would use it for li/mr... You can do asm("add%I1 %0,0,%1" : "=r"(dst) : "ri"(src)); I suppose, but that is not really an mr. > >(This output modifier goes back to the dark ages, some 2.4 or something). > > Hope Clang support it ... I don't know, sorry. But it is used all over the place, see sfp-machine.h for example, so maybe? Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 18:26 ` Segher Boessenkool @ 2019-08-27 18:33 ` Christophe Leroy 2019-08-27 19:11 ` Segher Boessenkool 0 siblings, 1 reply; 8+ messages in thread From: Christophe Leroy @ 2019-08-27 18:33 UTC (permalink / raw) To: Segher Boessenkool Cc: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel Le 27/08/2019 à 20:26, Segher Boessenkool a écrit : > On Tue, Aug 27, 2019 at 07:36:35PM +0200, Christophe Leroy wrote: >> Le 27/08/2019 à 19:29, Segher Boessenkool a écrit : >>> On Tue, Aug 27, 2019 at 10:48:24PM +1000, Nicholas Piggin wrote: >>>> Christophe Leroy's on August 27, 2019 6:13 pm: >>>>> +#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. >>> >>> static inline void wrtee(long val) >>> { >>> asm volatile("wrtee%I0 %0" : : "n"(val) : "memory"); >>> } >> >> Great, didn't know that possibility. >> >> Can it be used with any insn, for instance with add/addi ? >> Or with mr/li ? > > Any instruction, yes. %I<n> simply outputs an "i" if operand n is a > constant integer, and nothing otherwise. Thinking about it once more, I'm not sure this form is possible, because wrteei expect 0 or 1. If someone calls wrtee(MSR_EE); (or any constant containing MSR_EE) wrteei 1 is expected. And any constant with MSR_EE cleared should result in wrteei 0. > > So > asm("add%I2 %0,%1,%2" : "=r"(dst) : "r"(src1), "ri"(src1)); "ri", not "n" as for wrtee ? Christophe > works well. I don't see how you would use it for li/mr... You can do > asm("add%I1 %0,0,%1" : "=r"(dst) : "ri"(src)); > I suppose, but that is not really an mr. > >>> (This output modifier goes back to the dark ages, some 2.4 or something). >> >> Hope Clang support it ... > > I don't know, sorry. But it is used all over the place, see sfp-machine.h > for example, so maybe? > > > Segher > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc: cleanup hw_irq.h 2019-08-27 18:33 ` Christophe Leroy @ 2019-08-27 19:11 ` Segher Boessenkool 0 siblings, 0 replies; 8+ messages in thread From: Segher Boessenkool @ 2019-08-27 19:11 UTC (permalink / raw) To: Christophe Leroy Cc: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel On Tue, Aug 27, 2019 at 08:33:45PM +0200, Christophe Leroy wrote: > >So > > asm("add%I2 %0,%1,%2" : "=r"(dst) : "r"(src1), "ri"(src1)); > > "ri", not "n" as for wrtee ? "n" means a number. "i" means any constant integer. The difference is mostly that "n" does not allow relocations. This probably does not matter for this asm, not if you call it with correct values anyway. (If you want to pass other than small numbers here, you need different constraints; let's not go there). Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-27 19:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-27 8:13 [PATCH 1/2] powerpc: permanently include 8xx registers in reg.h Christophe Leroy 2019-08-27 8:13 ` [PATCH 2/2] powerpc: cleanup hw_irq.h Christophe Leroy 2019-08-27 12:48 ` Nicholas Piggin 2019-08-27 17:29 ` Segher Boessenkool 2019-08-27 17:36 ` Christophe Leroy 2019-08-27 18:26 ` Segher Boessenkool 2019-08-27 18:33 ` Christophe Leroy 2019-08-27 19:11 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).