Messages in this thread | | | Date | Mon, 10 Nov 2008 00:10:34 +0900 | From | "Hitoshi Mitake" <> | Subject | Re: [PATCH 1/1] edac x38: new MC driver module |
| |
I'm a same person as mitake@clustcom.com. h.mitake@gmail.com is my private mail address.
On Fri, Nov 7, 2008 at 4:11 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 7 Nov 2008 15:38:24 +0000 Hitoshi Mitake <mitake@clustcom.com> wrote: > >> >> This patch makes x38_edac.c to use kernel's >> readq() function when it is compiled for x86_64. >> >> Signed-off-by: Hitoshi Mitake <mitake@clustcom.com> >> Signed-off-by: Doug Thompson <dougthompson@xmission.com> >> --- >> >> Index: linux-2.6.28-rc3-git2/drivers/edac/Kconfig >> =================================================================== >> --- linux-2.6.28-rc3-git2.orig/drivers/edac/Kconfig 2008-11-07 11:27:05.000000000 +0000 >> +++ linux-2.6.28-rc3-git2/drivers/edac/Kconfig 2008-11-07 11:27:14.000000000 +0000 >> @@ -104,7 +104,7 @@ >> >> config EDAC_X38 >> tristate "Intel X38" >> - depends on EDAC_MM_EDAC && PCI && X86 >> + depends on EDAC_MM_EDAC && PCI && (X86 || X86_64) > > CONFIG_X86 is true for both CONFIG_X86_32=y amek CONFIG_X86_64=y, so > this change isn't needed. I'll fix that up.
Thanks for your fix up. This was redundancy, I should not change that point...
> >> --- linux-2.6.28-rc3-git2.orig/drivers/edac/x38_edac.c 2008-11-07 11:27:06.000000000 +0000 >> +++ linux-2.6.28-rc3-git2/drivers/edac/x38_edac.c 2008-11-07 11:27:29.000000000 +0000 >> @@ -162,10 +162,12 @@ >> X38_ERRSTS_BITS); >> } >> >> -static u64 x38_readq(const void __iomem *addr) >> +#ifndef CONFIG_X86_64 >> +static u64 readq(const void __iomem *addr) > > hm, it'd be nice if there was some more general way of determining > whether the architecture provides readq/writeq. >
I found this code in include/asm-x86/io.h
#ifdef CONFIG_X86_64
...
/* Let people know we have them */ #define readq readq #define writeq writeq #endif
x86 programmers are able to know existence of readq/writeq by using this definition.
And I grepped,
% grep readq `find include/asm-* -name "*.h"` include/asm-mips/io.h: ".set mips3" "\t\t# __readq" "\n\t" \ include/asm-mips/io.h:#define readq_relaxed readq include/asm-mips/io.h:#define readq readq include/asm-mips/txx9/tx4927.h: ((__u32)__raw_readq(&tx4927_ccfgptr->crir) >> 16) include/asm-mips/txx9/tx4927.h:#define TX4927_SDRAMC_CR(ch) __raw_readq(&tx4927_sdramcptr->cr[(ch)]) include/asm-mips/txx9/tx4927.h:#define TX4927_EBUSC_CR(ch) __raw_readq(&tx4927_ebuscptr->cr[(ch)]) include/asm-mips/txx9/tx4927.h: ____raw_writeq(____raw_readq(adr) & ~bits, adr); include/asm-mips/txx9/tx4927.h: ____raw_writeq(____raw_readq(adr) | bits, adr); include/asm-mips/txx9/tx4927.h: ____raw_writeq(____raw_readq(&tx4927_ccfgptr->ccfg) include/asm-mips/txx9/tx4927.h: ____raw_writeq((____raw_readq(&tx4927_ccfgptr->ccfg) include/asm-mips/txx9/tx4927.h: ____raw_writeq((____raw_readq(&tx4927_ccfgptr->ccfg) include/asm-mips/txx9/tx4938.h: ((__u32)__raw_readq(&tx4938_ccfgptr->crir) >> 16) include/asm-parisc/io.h:static inline unsigned long long gsc_readq(unsigned long addr) include/asm-parisc/io.h:static inline unsigned long long __raw_readq(const volatile void __iomem *addr) include/asm-parisc/io.h:#define readq(addr) __fswab64(__raw_readq(addr)) include/asm-parisc/io.h:#define readq_relaxed(addr) readq(addr) include/asm-x86/io.h:build_mmio_read(readq, "q", unsigned long, "=r", :"memory") include/asm-x86/io.h:build_mmio_read(__readq, "q", unsigned long, "=r", ) include/asm-x86/io.h:#define readq_relaxed(a) __readq(a) include/asm-x86/io.h:#define __raw_readq __readq include/asm-x86/io.h:#define readq readq
It seems that architectures that provide readq/writeq are mips, parisc and x86 (and x86_64).
mips and x86 provides this line #define readq readq to let user know existence of readq (and writeq), and parisc doesn't provide. But there is, #define readq(addr) __fswab64(__raw_readq(addr)) in parisc.
There is a difference between mips and x86's readq/writeq and parisc's readq/writeq. mips and x86's definition is only token, but parisc's definition is macro function.
But these defines can be used to determine existence of readq/writeq by common preprocessor like this, #ifndef readq /* programmer can define private version of readq (or writeq) */ #endif
Is this way enough general for our requirement? (If we use this as general way to determine existence of readq/writeq, I want other architecture's developer (whose architecture provides readq/writeq in the future) to support same way.)
| |