Re: lib/iomap.c mmio_{in,out}s* vs. __raw_* accessors

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Sat, 2006-11-04 at 19:46 -0800, Linus Torvalds wrote:
> 
> On Sun, 5 Nov 2006, Benjamin Herrenschmidt wrote:
> >
> > Make the generic lib/iomap.c use arch provided MMIO accessors when
> > available for big endian and repeat operations. Also while at it,
> > fix the *_be version which are currently broken for PIO
> 
> Just rip the _be versions out, methinks.
> 
> Also, what does your "writesb()" actually look like? I assume it's the 
> exact same thing as the generic one, with just another barrier. No? 

With our old naming (we used _ins* for the base operations, the PIO ones
just calling them with a magic offset) :

void _insb(const volatile u8 __iomem *port, void *buf, long count)
{
        u8 *tbuf = buf;
        u8 tmp;

        if (unlikely(count <= 0))
                return;
        asm volatile("sync");
        do {
                tmp = *port;
                asm volatile("eieio");
                *tbuf++ = tmp;
        } while (--count != 0);
        asm volatile("twi 0,%0,0; isync" : : "r" (tmp));
}
EXPORT_SYMBOL(_insb);

void _outsb(volatile u8 __iomem *port, const void *buf, long count)
{
        const u8 *tbuf = buf;

        if (unlikely(count <= 0))
                return;
        asm volatile("sync");
        do {
                *port = *tbuf++;
        } while (--count != 0);
        asm volatile("sync");
}
EXPORT_SYMBOL(_outsb);

I'm not even entirely sure that the insb one is correct, maybe it should
or-in all the values to get a proper data dependency that acutally
depends on all previous loads to pass to the twi instructions (it's a
conditional trap instruction with the condition set as never, it forces
the CPU to think the data has been consumed and thus, in conjunction
with the subsequent isync, forces the loads to be performed before any
further code can execute).

The store also has two barriers as you can see. That's mostly related to
the need for very synchronous IOs to please drivers, though the second
one could be replaced with

	get_paca()->io_sync = 1;	

(Which is a new mecanism we introduced recently to tell spin_unlock that
one or previous MMIO stores might need a sync before we unlock. Looks
like we didn't update the "s" functions when adding that).

Ben.