public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Linus Torvalds <torvalds@osdl.org>
Cc: Russell King <rmk+lkml@arm.linux.org.uk>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	Jeff Garzik <jgarzik@pobox.com>, Andrew Morton <akpm@osdl.org>,
	"David S. Miller" <davem@davemloft.net>,
	Paul Mackerras <paulus@samba.org>
Subject: Re: lib/iomap.c mmio_{in,out}s* vs. __raw_* accessors
Date: Sun, 05 Nov 2006 14:49:43 +1100	[thread overview]
Message-ID: <1162698583.28571.140.camel@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.64.0611041918560.25218@g5.osdl.org>


> > 	- "be" versions of MMIO accesses. Example:
> 
> Now, this one we should probably just drop, because nobody uses it. 
> Big-endian IO devices basically don't exist any more, and if they do, they 
> wouldn't use any generic structures.

I'd be happy to do so :) I just added _be versions for PowerPC for that
(writew_be, etc...) and an ARCH_HAVE_* to tell iomap that they exit. I
also found out that the _be implementation in lib/iomap is broken for
PIO (it just uses the LE versions without swap).

I just sent you a patch that allows the arch to provide it's own
versions. But if you want, I can redo it dropping the "be" thingies.

There are BE devices though... just generally not on PCI (Heh, I've just
had to deal with some folks doing a BE version of EHCI !). For plaform
specific stuffs, PowerPC has it's own low level accessors
(in_le16/in_be16/etc....) so it doesn't really matter unless we start
wanting drivers for those weird animals to get a bit more generic.

> > 	- repeat versions of MMIO accessors. Example:
> 
> So this one is the only one that makes sense. But it DOES NOT MAKE SENSE 
> in light of your second email that talked about "transparently use MMIO or 
> PIO". That's what made me think that you were just blathering.

I think I just badly expressed myself. My appologies.

> But I also don't want to add _yet_another_ bogus IO accessor function to 
> all architectures that nobody actually uses except for this one little 
> thing. Quite frankly, I'd rather just add something like
> 
> 	#ifndef iobarrier_w
> 	# define iobarrier_w() do { } while (0)
> 	#endif
> 
> and then use iobarrier_w()" in the "mmio" version of the __raw_writel loop 
> (and same for iobarrier_r() for the __raw_readl).

Unfortunately, that is not enough for us... 

In the case of reads. I need a barrier before the read and I need to
create a data dependency on the read data to force the CPU to perform
the read followed by an instruction barrier...

Also, with our new ultra-hard-like-x86 semantics we have now (at least
until I try again sorting that other mess out), our stores also need
additional barriers before the stores and setting a magic per-CPU bit
after... That sort of crap isn't easy to expose in the form of a couple
of barrier macros so I'd rather keep it out of the way safely inside
asm-powerpc/io.h

I've actually come accross buggy drivers that could use some "repeat"
version of the MMIO accessors a couple of times in the past in fact
(they used a loop instead and got the endian wrong of course). So I'm
not convinced they are that useless.... That sort of shit is actually
fairly common in the embedded world.

Anyway, the patch I sent you earlier allows the arch to provide them and
just falls back to the "hand made" versions if not. You might still want
to add this iobarrier_w() thingy in addition to that for other archs
that might want to use the fallback and need barriers though...

Cheers
Ben.



      reply	other threads:[~2006-11-05  3:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-04  7:52 lib/iomap.c mmio_{in,out}s* vs. __raw_* accessors Benjamin Herrenschmidt
2006-11-04 14:06 ` Russell King
2006-11-04 22:17   ` Benjamin Herrenschmidt
2006-11-04 23:52     ` Linus Torvalds
2006-11-05  1:10       ` Benjamin Herrenschmidt
2006-11-05  3:32         ` Benjamin Herrenschmidt
2006-11-05  3:46           ` Linus Torvalds
2006-11-05  3:55             ` Benjamin Herrenschmidt
2006-11-05  4:00             ` Benjamin Herrenschmidt
2006-11-05  4:16               ` Linus Torvalds
2006-11-05  4:38                 ` Benjamin Herrenschmidt
2006-11-05  5:00                   ` Linus Torvalds
2006-11-05  5:08                     ` Benjamin Herrenschmidt
2006-11-05  5:28                       ` Benjamin Herrenschmidt
2006-11-06  2:56                         ` Benjamin Herrenschmidt
2006-11-06  3:13                           ` Linus Torvalds
2006-11-06  4:50                             ` Benjamin Herrenschmidt
2006-11-05  3:34         ` Linus Torvalds
2006-11-05  3:49           ` Benjamin Herrenschmidt [this message]

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=1162698583.28571.140.camel@localhost.localdomain \
    --to=benh@kernel.crashing.org \
    --cc=akpm@osdl.org \
    --cc=davem@davemloft.net \
    --cc=jgarzik@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=rmk+lkml@arm.linux.org.uk \
    --cc=torvalds@osdl.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