* [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M @ 2015-06-08 22:24 Maxime Coquelin 2015-06-08 22:47 ` Russell King - ARM Linux 0 siblings, 1 reply; 9+ messages in thread From: Maxime Coquelin @ 2015-06-08 22:24 UTC (permalink / raw) To: Russell King, Arnd Bergmann, u.kleine-koenig Cc: linux-arm-kernel, linux-kernel, Uwe Kleine-König, Russell King Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes build failure on ARMv7-M machines: CC arch/arm/kernel/asm-offsets.s In file included from include/linux/sem.h:5:0, from include/linux/sched.h:35, from arch/arm/kernel/asm-offsets.c:14: include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held': include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration] return preempt_count() != 0 || irqs_disabled(); ^ Commit cb1293e2f594 was initially introduced to fix build errors with randconfig, as ARMv7-m selects TRACE_IRQFLAGS_SUPPORT but breaks build when TRACE_IRQFLAGS is selected. As there is no proper support yet for TRACE_IRQFLAGS, this patch partially reverts commit cb1293e2f594, with the downside of build errors with randconfig when TRACE_IRQFLAGS gets selected. Cc: Arnd Bergmann <arnd@arndb.de> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Cc: Russell King <rmk+kernel@arm.linux.org.uk> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com> --- arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b47457d..e4e1694 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -171,7 +171,7 @@ config LOCKDEP_SUPPORT config TRACE_IRQFLAGS_SUPPORT bool - default !CPU_V7M + default y config RWSEM_XCHGADD_ALGORITHM bool -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-08 22:24 [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M Maxime Coquelin @ 2015-06-08 22:47 ` Russell King - ARM Linux 2015-06-09 9:14 ` Maxime Coquelin 0 siblings, 1 reply; 9+ messages in thread From: Russell King - ARM Linux @ 2015-06-08 22:47 UTC (permalink / raw) To: Maxime Coquelin Cc: Arnd Bergmann, u.kleine-koenig, linux-arm-kernel, linux-kernel On Tue, Jun 09, 2015 at 12:24:48AM +0200, Maxime Coquelin wrote: > Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes > build failure on ARMv7-M machines: > > CC arch/arm/kernel/asm-offsets.s > In file included from include/linux/sem.h:5:0, > from include/linux/sched.h:35, > from arch/arm/kernel/asm-offsets.c:14: > include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held': > include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration] > return preempt_count() != 0 || irqs_disabled(); > ^ The real solution is to provide a definition _in asm-generic_ for arch_irqs_disabled(), rather than having almost every arch doing: static inline bool arch_irqs_disabled(void) { return arch_irqs_disabled_flags(arch_local_save_flags()); } I'm personally refusing to take a patch for ARM which adds yet another copy of the above. This is, after all, exactly the kind of stuff that should be in asm-generic, or if not, in include/linux but overridable by arch stuff. We keep going between the two extremes of "lets push lots of stuff into arch stuff" and "lets try to extract the common bits out of arch code". Let's try and settle on one approach, and apply it universally. In the mean time, I think the right answer is to drop Arnd's patch - subsituting a randconfig build error for a useful-config build error is not something we want to do - and even partially reverting the patch results in randconfig build errors returning, so... -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-08 22:47 ` Russell King - ARM Linux @ 2015-06-09 9:14 ` Maxime Coquelin 2015-06-09 11:41 ` Daniel Thompson 0 siblings, 1 reply; 9+ messages in thread From: Maxime Coquelin @ 2015-06-09 9:14 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel 2015-06-09 0:47 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Tue, Jun 09, 2015 at 12:24:48AM +0200, Maxime Coquelin wrote: >> Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes >> build failure on ARMv7-M machines: >> >> CC arch/arm/kernel/asm-offsets.s >> In file included from include/linux/sem.h:5:0, >> from include/linux/sched.h:35, >> from arch/arm/kernel/asm-offsets.c:14: >> include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held': >> include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration] >> return preempt_count() != 0 || irqs_disabled(); >> ^ > > The real solution is to provide a definition _in asm-generic_ for > arch_irqs_disabled(), rather than having almost every arch doing: > > static inline bool arch_irqs_disabled(void) > { > return arch_irqs_disabled_flags(arch_local_save_flags()); > } > > I'm personally refusing to take a patch for ARM which adds yet another > copy of the above. This is, after all, exactly the kind of stuff that > should be in asm-generic, or if not, in include/linux but overridable > by arch stuff. > > We keep going between the two extremes of "lets push lots of stuff into > arch stuff" and "lets try to extract the common bits out of arch code". > > Let's try and settle on one approach, and apply it universally. I agree on the idea but I don't measure all the impacts it would have. > > In the mean time, I think the right answer is to drop Arnd's patch - > subsituting a randconfig build error for a useful-config build error > is not something we want to do - and even partially reverting the > patch results in randconfig build errors returning, so... Ok, should I send a revert, or you can still drop Arnd's patch directly? Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 9:14 ` Maxime Coquelin @ 2015-06-09 11:41 ` Daniel Thompson 2015-06-09 14:42 ` Maxime Coquelin 2015-06-09 15:01 ` Russell King - ARM Linux 0 siblings, 2 replies; 9+ messages in thread From: Daniel Thompson @ 2015-06-09 11:41 UTC (permalink / raw) To: Maxime Coquelin, Russell King - ARM Linux Cc: Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel On 09/06/15 10:14, Maxime Coquelin wrote: >> The real solution is to provide a definition _in asm-generic_ for >> arch_irqs_disabled(), rather than having almost every arch doing: >> >> static inline bool arch_irqs_disabled(void) >> { >> return arch_irqs_disabled_flags(arch_local_save_flags()); >> } >> >> I'm personally refusing to take a patch for ARM which adds yet another >> copy of the above. This is, after all, exactly the kind of stuff that >> should be in asm-generic, or if not, in include/linux but overridable >> by arch stuff. >> >> We keep going between the two extremes of "lets push lots of stuff into >> arch stuff" and "lets try to extract the common bits out of arch code". >> >> Let's try and settle on one approach, and apply it universally. > > I agree on the idea but I don't measure all the impacts it would have. Hey Maxime, just out of interest... Does the following patch, which makes the arch_irqs_disabled() implementation from asm-generic available on arm, fix the build for you? I've only done a real quick 'n dirty check for regression: multi_v7_defconfig still works ;-) If the patch is useful I can test it a bit harder... From d86ef4466cfdbe622a65bcc7e72a8ca0d1dd2879 Mon Sep 17 00:00:00 2001 From: Daniel Thompson <daniel.thompson@linaro.org> Date: Tue, 9 Jun 2015 12:35:24 +0100 Subject: [PATCH] ARM: irqflags: Get arch_irqs_disabled from asm-generic asm-generic/irqflags.h provides an implementation of arch_irqs_disabled(). Lets grab that implementation. Suggested-by: Russell King <linux@arm.linux.org.uk> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- arch/arm/include/asm/irqflags.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/include/asm/irqflags.h b/arch/arm/include/asm/irqflags.h index 3b763d6652a0..43908146a5cf 100644 --- a/arch/arm/include/asm/irqflags.h +++ b/arch/arm/include/asm/irqflags.h @@ -20,6 +20,7 @@ #if __LINUX_ARM_ARCH__ >= 6 +#define arch_local_irq_save arch_local_irq_save static inline unsigned long arch_local_irq_save(void) { unsigned long flags; @@ -31,6 +32,7 @@ static inline unsigned long arch_local_irq_save(void) return flags; } +#define arch_local_irq_enable arch_local_irq_enable static inline void arch_local_irq_enable(void) { asm volatile( @@ -40,6 +42,7 @@ static inline void arch_local_irq_enable(void) : "memory", "cc"); } +#define arch_local_irq_disable arch_local_irq_disable static inline void arch_local_irq_disable(void) { asm volatile( @@ -56,6 +59,7 @@ static inline void arch_local_irq_disable(void) /* * Save the current interrupt enable state & disable IRQs */ +#define arch_local_irq_save arch_local_irq_save static inline unsigned long arch_local_irq_save(void) { unsigned long flags, temp; @@ -73,6 +77,7 @@ static inline unsigned long arch_local_irq_save(void) /* * Enable IRQs */ +#define arch_local_irq_enable arch_local_irq_enable static inline void arch_local_irq_enable(void) { unsigned long temp; @@ -88,6 +93,7 @@ static inline void arch_local_irq_enable(void) /* * Disable IRQs */ +#define arch_local_irq_disable arch_local_irq_disable static inline void arch_local_irq_disable(void) { unsigned long temp; @@ -135,6 +141,7 @@ static inline void arch_local_irq_disable(void) /* * Save the current interrupt enable state. */ +#define arch_local_save_flags arch_local_save_flags static inline unsigned long arch_local_save_flags(void) { unsigned long flags; @@ -147,6 +154,7 @@ static inline unsigned long arch_local_save_flags(void) /* * restore saved IRQ & FIQ state */ +#define arch_local_irq_restore arch_local_irq_restore static inline void arch_local_irq_restore(unsigned long flags) { asm volatile( @@ -156,10 +164,13 @@ static inline void arch_local_irq_restore(unsigned long flags) : "memory", "cc"); } +#define arch_irqs_disabled_flags arch_irqs_disabled_flags static inline int arch_irqs_disabled_flags(unsigned long flags) { return flags & IRQMASK_I_BIT; } +#include <asm-generic/irqflags.h> + #endif /* ifdef __KERNEL__ */ #endif /* ifndef __ASM_ARM_IRQFLAGS_H */ -- 2.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 11:41 ` Daniel Thompson @ 2015-06-09 14:42 ` Maxime Coquelin 2015-06-09 15:01 ` Russell King - ARM Linux 1 sibling, 0 replies; 9+ messages in thread From: Maxime Coquelin @ 2015-06-09 14:42 UTC (permalink / raw) To: Daniel Thompson Cc: Russell King - ARM Linux, Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel Hi Daniel, 2015-06-09 13:41 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>: > On 09/06/15 10:14, Maxime Coquelin wrote: >>> >>> The real solution is to provide a definition _in asm-generic_ for >>> arch_irqs_disabled(), rather than having almost every arch doing: >>> >>> static inline bool arch_irqs_disabled(void) >>> { >>> return arch_irqs_disabled_flags(arch_local_save_flags()); >>> } >>> >>> I'm personally refusing to take a patch for ARM which adds yet another >>> copy of the above. This is, after all, exactly the kind of stuff that >>> should be in asm-generic, or if not, in include/linux but overridable >>> by arch stuff. >>> >>> We keep going between the two extremes of "lets push lots of stuff into >>> arch stuff" and "lets try to extract the common bits out of arch code". >>> >>> Let's try and settle on one approach, and apply it universally. > > Does the following patch, which makes the arch_irqs_disabled() > implementation from asm-generic available on arm, fix the build for you? I confirm it fixes the build on Russell's for-next branch with efm32_defconfig. I have no efm32 HW to test it though. > > I've only done a real quick 'n dirty check for regression: > multi_v7_defconfig still works ;-) > > If the patch is useful I can test it a bit harder... I can also test it this evening with my stm32 config. Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 11:41 ` Daniel Thompson 2015-06-09 14:42 ` Maxime Coquelin @ 2015-06-09 15:01 ` Russell King - ARM Linux 2015-06-09 17:37 ` Daniel Thompson 1 sibling, 1 reply; 9+ messages in thread From: Russell King - ARM Linux @ 2015-06-09 15:01 UTC (permalink / raw) To: Daniel Thompson Cc: Maxime Coquelin, Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel On Tue, Jun 09, 2015 at 12:41:50PM +0100, Daniel Thompson wrote: > Does the following patch, which makes the arch_irqs_disabled() > implementation from asm-generic available on arm, fix the build for you? Yes, this is exactly the kind of fix for this I'm looking for. Once everyone's happy with it, it can find it's way to the patch system. Thanks. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 15:01 ` Russell King - ARM Linux @ 2015-06-09 17:37 ` Daniel Thompson 2015-06-10 10:03 ` Maxime Coquelin 2015-06-10 14:07 ` Joachim Eastwood 0 siblings, 2 replies; 9+ messages in thread From: Daniel Thompson @ 2015-06-09 17:37 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Maxime Coquelin, Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel On 09/06/15 16:01, Russell King - ARM Linux wrote: > On Tue, Jun 09, 2015 at 12:41:50PM +0100, Daniel Thompson wrote: >> Does the following patch, which makes the arch_irqs_disabled() >> implementation from asm-generic available on arm, fix the build for you? > > Yes, this is exactly the kind of fix for this I'm looking for. Once > everyone's happy with it, it can find it's way to the patch system. From my side, using v4.1-rc6 plus the patch, I can build all defconfigs and both versatile_defconfig and multi_v7_defconfig remain bootable (so both ways through the arch #ifdef in irqflag.h). Similarly working on linux-next-20150608 plus the patch I am able to build and boot Maxime's stm32 code (and show that without the patch it doesn't build). From my side I think that makes it good to go. So... in the absense of objections I will send it to the patch tracker tomorrow. Daniel. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 17:37 ` Daniel Thompson @ 2015-06-10 10:03 ` Maxime Coquelin 2015-06-10 14:07 ` Joachim Eastwood 1 sibling, 0 replies; 9+ messages in thread From: Maxime Coquelin @ 2015-06-10 10:03 UTC (permalink / raw) To: Daniel Thompson Cc: Russell King - ARM Linux, Arnd Bergmann, Uwe Kleine-König, linux-arm-kernel, linux-kernel 2015-06-09 19:37 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>: > On 09/06/15 16:01, Russell King - ARM Linux wrote: >> >> On Tue, Jun 09, 2015 at 12:41:50PM +0100, Daniel Thompson wrote: >>> >>> Does the following patch, which makes the arch_irqs_disabled() >>> implementation from asm-generic available on arm, fix the build for you? >> >> >> Yes, this is exactly the kind of fix for this I'm looking for. Once >> everyone's happy with it, it can find it's way to the patch system. > > > From my side, using v4.1-rc6 plus the patch, I can build all defconfigs and > both versatile_defconfig and multi_v7_defconfig remain bootable (so both > ways through the arch #ifdef in irqflag.h). > > Similarly working on linux-next-20150608 plus the patch I am able to build > and boot Maxime's stm32 code (and show that without the patch it doesn't > build). Ok, thanks for having tested on STM32. > > From my side I think that makes it good to go. > > So... in the absense of objections I will send it to the patch tracker > tomorrow. For what it is worth, you can add: Acked-by: Maxime Coquelin <maxime.coquelin@st.com> Regards, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M 2015-06-09 17:37 ` Daniel Thompson 2015-06-10 10:03 ` Maxime Coquelin @ 2015-06-10 14:07 ` Joachim Eastwood 1 sibling, 0 replies; 9+ messages in thread From: Joachim Eastwood @ 2015-06-10 14:07 UTC (permalink / raw) To: Daniel Thompson Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel, Arnd Bergmann, Maxime Coquelin, Uwe Kleine-König Hi Daniel, On 9 June 2015 at 19:37, Daniel Thompson <daniel.thompson@linaro.org> wrote: > On 09/06/15 16:01, Russell King - ARM Linux wrote: >> >> On Tue, Jun 09, 2015 at 12:41:50PM +0100, Daniel Thompson wrote: >>> >>> Does the following patch, which makes the arch_irqs_disabled() >>> implementation from asm-generic available on arm, fix the build for you? >> >> >> Yes, this is exactly the kind of fix for this I'm looking for. Once >> everyone's happy with it, it can find it's way to the patch system. > > > From my side, using v4.1-rc6 plus the patch, I can build all defconfigs and > both versatile_defconfig and multi_v7_defconfig remain bootable (so both > ways through the arch #ifdef in irqflag.h). > > Similarly working on linux-next-20150608 plus the patch I am able to build > and boot Maxime's stm32 code (and show that without the patch it doesn't > build). > > From my side I think that makes it good to go. Since next didn't build for me either I applied this and it works for the LPC18xx (Cortex-M4) platform. Tested-by: Joachim Eastwood <manabian@gmail.com> regards, Joachim Eastwood ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-06-10 14:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-08 22:24 [PATCH] ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M Maxime Coquelin 2015-06-08 22:47 ` Russell King - ARM Linux 2015-06-09 9:14 ` Maxime Coquelin 2015-06-09 11:41 ` Daniel Thompson 2015-06-09 14:42 ` Maxime Coquelin 2015-06-09 15:01 ` Russell King - ARM Linux 2015-06-09 17:37 ` Daniel Thompson 2015-06-10 10:03 ` Maxime Coquelin 2015-06-10 14:07 ` Joachim Eastwood
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).