linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Ungerer <gerg@snapgear.com>
To: David Howells <dhowells@redhat.com>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 18/22] Fix M68K irqflags
Date: Wed, 1 Sep 2010 21:36:32 +1000	[thread overview]
Message-ID: <4C7E3AC0.9000203@snapgear.com> (raw)
In-Reply-To: <15143.1283340624@redhat.com>

On 01/09/10 21:30, David Howells wrote:
>> Here is a spin of the same patch, only with a single irqflags.h.
>> It is closely modeled on the mm version, merging (and cleaning)
>> the non-mmu bits as needed. There is a fair bit of common code
>> in here.
>>
>> It does refactor arch_local_irq_save() but I am pretty sure that
>> makes no impact (performance or code size). I have left
>> arch_irq_enable as-is for all, it is not clear to me why it is
>> the way it is for the mm case (Geert?)
>
> Here's my take on your patch.  By removing a couple of apparently unnecessary
> refs to asm/system.h I can turn the #define of arch_local_irq_enable() into an
> inline function.
>
> Should I take it that Coldfire chips lack the AND/OR directly to SR
> instructions?

Yes, thats right.

The patch looks good to me.
I'll test it and respond with an ack.

Regards
Greg



> David
> ---
>   arch/m68k/include/asm/irqflags.h  |   76 ++++++++++++++++++++++++++++++++++++++
>   arch/m68k/include/asm/system_mm.h |   25 ------------
>   arch/m68k/include/asm/system_no.h |   49 ------------------------
>   include/linux/hardirq.h           |    1
>   include/linux/list.h              |    1
>   5 files changed, 78 insertions(+), 74 deletions(-)
>
> diff --git a/arch/m68k/include/asm/irqflags.h b/arch/m68k/include/asm/irqflags.h
> new file mode 100644
> index 0000000..b186de8
> --- /dev/null
> +++ b/arch/m68k/include/asm/irqflags.h
> @@ -0,0 +1,76 @@
> +#ifndef _M68K_IRQFLAGS_MM_H
> +#define _M68K_IRQFLAGS_MM_H
> +
> +#include<linux/types.h>
> +#include<linux/hardirq.h>
> +#include<linux/preempt.h>
> +#include<asm/thread_info.h>
> +#include<asm/entry.h>
> +
> +static inline unsigned long arch_local_save_flags(void)
> +{
> +	unsigned long flags;
> +	asm volatile ("movew %%sr,%0": "=d" (flags) : : "memory");
> +	return flags;
> +}
> +
> +static inline void arch_local_irq_disable(void)
> +{
> +#ifdef CONFIG_COLDFIRE
> +	asm volatile (
> +		"move	%/sr,%%d0	\n\t"
> +		"ori.l	#0x0700,%%d0	\n\t"
> +		"move	%%d0,%/sr	\n"
> +		: /* no outputs */
> +		:
> +		: "cc", "%d0", "memory");
> +#else
> +	asm volatile ("oriw  #0x0700,%%sr" : : : "memory");
> +#endif
> +}
> +
> +static inline void arch_local_irq_enable(void)
> +{
> +#if defined(CONFIG_COLDFIRE)
> +	asm volatile (
> +		"move	%/sr,%%d0	\n\t"
> +		"andi.l	#0xf8ff,%%d0	\n\t"
> +		"move	%%d0,%/sr	\n"
> +		: /* no outputs */
> +		:
> +		: "cc", "%d0", "memory");
> +#else
> +# if defined(CONFIG_MMU)
> +	if (MACH_IS_Q40 || !hardirq_count())
> +# endif
> +		asm volatile (
> +			"andiw %0,%%sr"
> +			:
> +			: "i" (ALLOWINT)
> +			: "memory")
> +#endif
> +}
> +
> +static inline unsigned long arch_local_irq_save(void)
> +{
> +	unsigned long flags = arch_local_save_flags();
> +	arch_local_irq_disable();
> +	return flags;
> +}
> +
> +static inline void arch_local_irq_restore(unsigned long flags)
> +{
> +	asm volatile ("movew %0,%%sr": : "d" (flags) : "memory");
> +}
> +
> +static inline bool arch_irqs_disabled_flags(unsigned long flags)
> +{
> +	return (flags&  ~ALLOWINT) != 0;
> +}
> +
> +static inline bool arch_irqs_disabled(void)
> +{
> +	return arch_irqs_disabled_flags(arch_local_save_flags());
> +}
> +
> +#endif /* _M68K_IRQFLAGS_MM_H */
> diff --git a/arch/m68k/include/asm/system_mm.h b/arch/m68k/include/asm/system_mm.h
> index dbb6515..12053c4 100644
> --- a/arch/m68k/include/asm/system_mm.h
> +++ b/arch/m68k/include/asm/system_mm.h
> @@ -3,6 +3,7 @@
>
>   #include<linux/linkage.h>
>   #include<linux/kernel.h>
> +#include<linux/irqflags.h>
>   #include<asm/segment.h>
>   #include<asm/entry.h>
>
> @@ -62,30 +63,6 @@ asmlinkage void resume(void);
>   #define smp_wmb()	barrier()
>   #define smp_read_barrier_depends()	((void)0)
>
> -/* interrupt control.. */
> -#if 0
> -#define local_irq_enable() asm volatile ("andiw %0,%%sr": : "i" (ALLOWINT) : "memory")
> -#else
> -#include<linux/hardirq.h>
> -#define local_irq_enable() ({							\
> -	if (MACH_IS_Q40 || !hardirq_count())					\
> -		asm volatile ("andiw %0,%%sr": : "i" (ALLOWINT) : "memory");	\
> -})
> -#endif
> -#define local_irq_disable() asm volatile ("oriw  #0x0700,%%sr": : : "memory")
> -#define local_save_flags(x) asm volatile ("movew %%sr,%0":"=d" (x) : : "memory")
> -#define local_irq_restore(x) asm volatile ("movew %0,%%sr": :"d" (x) : "memory")
> -
> -static inline int irqs_disabled(void)
> -{
> -	unsigned long flags;
> -	local_save_flags(flags);
> -	return flags&  ~ALLOWINT;
> -}
> -
> -/* For spinlocks etc */
> -#define local_irq_save(x)	({ local_save_flags(x); local_irq_disable(); })
> -
>   #define xchg(ptr,x) ((__typeof__(*(ptr)))__xchg((unsigned long)(x),(ptr),sizeof(*(ptr))))
>
>   struct __xchg_dummy { unsigned long a[100]; };
> diff --git a/arch/m68k/include/asm/system_no.h b/arch/m68k/include/asm/system_no.h
> index 3c0718d..52ca8da 100644
> --- a/arch/m68k/include/asm/system_no.h
> +++ b/arch/m68k/include/asm/system_no.h
> @@ -2,6 +2,7 @@
>   #define _M68KNOMMU_SYSTEM_H
>
>   #include<linux/linkage.h>
> +#include<linux/irqflags.h>
>   #include<asm/segment.h>
>   #include<asm/entry.h>
>
> @@ -46,54 +47,6 @@ asmlinkage void resume(void);
>     (last) = _last;						\
>   }
>
> -#ifdef CONFIG_COLDFIRE
> -#define local_irq_enable() __asm__ __volatile__ (		\
> -	"move %/sr,%%d0\n\t"					\
> -	"andi.l #0xf8ff,%%d0\n\t"				\
> -	"move %%d0,%/sr\n"					\
> -	: /* no outputs */					\
> -	:							\
> -        : "cc", "%d0", "memory")
> -#define local_irq_disable() __asm__ __volatile__ (		\
> -	"move %/sr,%%d0\n\t"					\
> -	"ori.l #0x0700,%%d0\n\t"				\
> -	"move %%d0,%/sr\n"					\
> -	: /* no outputs */					\
> -	:							\
> -	: "cc", "%d0", "memory")
> -/* For spinlocks etc */
> -#define local_irq_save(x) __asm__ __volatile__ (		\
> -	"movew %%sr,%0\n\t"					\
> -	"movew #0x0700,%%d0\n\t"				\
> -	"or.l  %0,%%d0\n\t"					\
> -	"movew %%d0,%/sr"					\
> -	: "=d" (x)						\
> -	:							\
> -	: "cc", "%d0", "memory")
> -#else
> -
> -/* portable version */ /* FIXME - see entry.h*/
> -#define ALLOWINT 0xf8ff
> -
> -#define local_irq_enable() asm volatile ("andiw %0,%%sr": : "i" (ALLOWINT) : "memory")
> -#define local_irq_disable() asm volatile ("oriw  #0x0700,%%sr": : : "memory")
> -#endif
> -
> -#define local_save_flags(x) asm volatile ("movew %%sr,%0":"=d" (x) : : "memory")
> -#define local_irq_restore(x) asm volatile ("movew %0,%%sr": :"d" (x) : "memory")
> -
> -/* For spinlocks etc */
> -#ifndef local_irq_save
> -#define local_irq_save(x) do { local_save_flags(x); local_irq_disable(); } while (0)
> -#endif
> -
> -#define	irqs_disabled()			\
> -({					\
> -	unsigned long flags;		\
> -	local_save_flags(flags);	\
> -	((flags&  0x0700) == 0x0700);	\
> -})
> -
>   #define iret() __asm__ __volatile__ ("rte": : :"memory", "sp", "cc")
>
>   /*
> diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
> index d5b3876..7dfdc06 100644
> --- a/include/linux/hardirq.h
> +++ b/include/linux/hardirq.h
> @@ -8,7 +8,6 @@
>   #include<linux/lockdep.h>
>   #include<linux/ftrace_irq.h>
>   #include<asm/hardirq.h>
> -#include<asm/system.h>
>
>   /*
>    * We put the hardirq and softirq counter into the preemption
> diff --git a/include/linux/list.h b/include/linux/list.h
> index d167b5d..88a0006 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -5,7 +5,6 @@
>   #include<linux/stddef.h>
>   #include<linux/poison.h>
>   #include<linux/prefetch.h>
> -#include<asm/system.h>
>
>   /*
>    * Simple doubly linked list implementation.
>

  reply	other threads:[~2010-09-01 11:37 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-27  1:59 [PATCH 00/22] Name the irq flag handling functions sanely David Howells
2010-08-27  1:59 ` [PATCH 01/22] Alpha: Fix a missing comma in sys_osf_statfs() David Howells
2010-08-27  1:59   ` David Howells
2010-08-27  1:59 ` [PATCH 02/22] Blackfin: Split PLL code from mach-specific cdef headers David Howells
2010-08-27  1:59 ` [PATCH 03/22] Blackfin: Don't redefine blackfin serial port symbols David Howells
2010-08-27  1:59 ` [PATCH 04/22] Blackfin: Split the BF532 BFIN_*_FIO_FLAG() functions to their own header David Howells
2010-08-27  1:59   ` David Howells
2010-08-27  1:59 ` [PATCH 05/22] Blackfin: Rename DES PC2() symbol to avoid collision David Howells
2010-08-27  1:59   ` David Howells
2010-08-27  1:59 ` [PATCH 06/22] Blackfin: Add missing dep to asm/irqflags.h David Howells
2010-08-27  1:59   ` David Howells
2010-08-27  1:59 ` [PATCH 07/22] Blackfin: Rename IRQ flags handling functions David Howells
2010-08-27  1:59 ` [PATCH 08/22] h8300: IRQ flags should be stored in an unsigned long David Howells
2010-08-27  1:59 ` [PATCH 09/22] h8300: Fix die() David Howells
2010-08-27  1:59 ` [PATCH 10/22] h8300: Fix missing consts in kernel_execve() David Howells
2010-08-27  1:59   ` David Howells
2010-08-27  2:00 ` [PATCH 11/22] SH: Add missing consts to sys_execve() declaration David Howells
2010-08-27  2:00 ` [PATCH 12/22] Fix IRQ flag handling naming David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 13/22] MIPS: Fix IRQ flags handling David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 14/22] Fix Alpha irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 15/22] Fix H8300 arch David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 16/22] Fix IA64 irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 17/22] Fix m32r irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 18/22] Fix M68K irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-30  7:12   ` Greg Ungerer
2010-08-30  7:12     ` Greg Ungerer
2010-08-30 20:50   ` David Howells
2010-08-31  7:36     ` Geert Uytterhoeven
2010-08-31 16:00     ` David Howells
2010-09-01  6:38       ` Greg Ungerer
2010-09-01  6:33     ` Greg Ungerer
2010-09-01 10:43     ` David Howells
2010-09-01 10:43       ` David Howells
2010-09-01 11:21       ` Greg Ungerer
2010-09-01 10:46     ` David Howells
2010-09-01 11:22       ` Greg Ungerer
2010-09-01 11:31       ` David Howells
2010-09-01 11:30     ` David Howells
2010-09-01 11:36       ` Greg Ungerer [this message]
2010-09-01 12:03       ` David Howells
2010-09-02  1:46       ` Greg Ungerer
2010-09-02 10:20       ` David Howells
2010-09-02 10:21         ` [PATCH 1/3] Drop a couple of unnecessary asm/include.h inclusions David Howells
2010-09-03  1:29           ` Greg Ungerer
2010-09-02 10:21         ` [PATCH 2/3] M68K: Use CONFIG_MMU not __uClinux__ to select m68knommu contributions David Howells
2010-09-02 10:21           ` David Howells
2010-09-02 13:54           ` Sam Ravnborg
2010-09-02 14:15             ` Andreas Schwab
2010-09-03  3:29             ` Greg Ungerer
2010-09-02 14:21           ` Andreas Schwab
2010-09-02 19:53             ` Geert Uytterhoeven
2010-09-03  3:30               ` Greg Ungerer
2010-09-03  3:30                 ` Greg Ungerer
2010-09-03  8:41               ` Andreas Schwab
2010-09-03  9:18               ` David Howells
2010-09-03  9:31                 ` Greg Ungerer
2010-09-03  9:33                   ` Geert Uytterhoeven
2010-09-03  9:52                   ` David Howells
2010-09-03 10:02                     ` Geert Uytterhoeven
2010-09-03 10:41                     ` Greg Ungerer
2010-09-03 10:41                       ` Greg Ungerer
2010-09-02 15:11           ` David Howells
2010-09-02 15:12           ` David Howells
2010-09-02 15:12             ` David Howells
2010-09-02 16:50             ` Sam Ravnborg
2010-09-02 10:22         ` [PATCH 3/3] Fix M68K irqflags David Howells
2010-09-02 10:22           ` David Howells
2010-09-03  1:28           ` Greg Ungerer
2010-09-03  6:44           ` David Howells
2010-09-03  6:53             ` Greg Ungerer
2010-09-03  7:28             ` David Howells
2010-09-03  8:05             ` David Howells
2010-09-03  8:05               ` David Howells
2010-09-03  9:28               ` Greg Ungerer
2010-09-03 10:05               ` David Howells
2010-09-03 10:38                 ` Greg Ungerer
2010-09-02 10:48         ` [PATCH 1/3] Drop a couple of unnecessary asm/include.h inclusions David Howells
2010-08-27  2:00 ` [PATCH 19/22] Fix PA-RISC irqflags David Howells
2010-09-03  1:40   ` Kyle McMartin
2010-09-03  6:48   ` David Howells
2010-09-03 13:31     ` Kyle McMartin
2010-08-27  2:00 ` [PATCH 20/22] Fix powerpc irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 21/22] Fix SH irqflags David Howells
2010-08-27  2:00   ` David Howells
2010-08-27  2:00 ` [PATCH 22/22] Fix Sparc irqflags David Howells
2010-08-27  2:00   ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C7E3AC0.9000203@snapgear.com \
    --to=gerg@snapgear.com \
    --cc=dhowells@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).