linux-kernel - Re: [PATCH 1/1] edac x38: new MC driver module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081129180144.GA15533@uranus.ravnborg.org>
Date:	Sat, 29 Nov 2008 19:01:44 +0100
From:	Sam Ravnborg <sam@...nborg.org>
To:	Hitoshi Mitake <h.mitake@...il.com>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"Luck, Tony" <tony.luck@...el.com>,
	Russell King <rmk+lkml@....linux.org.uk>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Doug Thompson <norsk5@...oo.com>, dougthompson@...ssion.com,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH 1/1] edac x38: new MC driver module

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@...nborg.org> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/