On Sat, Nov 29, 2008 at 10:24:49PM +0900, Hitoshi Mitake wrote: > On Sat, 29 Nov 2008 11:52:29 +0100 > Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > > > > > > But this is old way. ARCH_HAS_READQ and ARCH_HAS_WRITEQ are new ways > > > to determine existence of readq/writeq. Drivers which use readq/writeq should > > > depend on these values in their Kconfig file. > > > > If we look at arch/x86/Kconfig we see: > > ### Arch settings > > config X86 > > def_bool y > > select HAVE_AOUT if X86_32 > > select HAVE_UNSTABLE_SCHED_CLOCK > > select HAVE_IDE > > select HAVE_OPROFILE > > select HAVE_IOREMAP_PROT > > select HAVE_KPROBES > > select ARCH_WANT_OPTIONAL_GPIOLIB > > ... > > > > So the normal syntax here is "HAVE_XXX_XXX" - not ARCH_HAS_XXX_XXX > > > > If you update your patch please use this syntax, > > and locate the select under X86 - not under the 32/64 entries. > > Thanks for your notification. I didn't notice that syntax rule. > > > > > But I do not see why adding these in the first place. > > > > Andrew Morton told that drivers which need readq/writeq should use ones of kernel, > and if architecture part of kernel does not provide readq/writeq, drivers should be disabled. > > This is Andrew's mail: > http://marc.info/?l=linux-kernel&m=122625885124798&w=2 > ===> Quote: > > #ifdef readq > > Is a suitable way of determining whether the architecture implements > readq and writeq. It isn't pretty, but it will suffice. > > A problem with it is that drivers will then do > > #ifndef readq > <provide a local implementation here> > #endif > > which rather sucks - we don't want lots of little private readq/writeq > implementations all over the tree. > > Perhaps it would be better to have a CONFIG_ARCH_HAS_READQ and to then > disable these drivers on the architectures which don't provide > readq/writeq support. > > <==== I see both rationales and you combine them in your patch - OK. And the reason why you cannot just add this to include/linux/io.h is that not all architectures provide a readl()/writel() I assume. Feel free to add my Acked-by: to the patch. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html