Re: [PATCH] ppc64: Fix __raw_* IO accessors

From: Benjamin Herrenschmidt
Date: Wed Sep 22 2004 - 19:53:50 EST


On Thu, 2004-09-23 at 04:58, Petr Vandrovec wrote:

> > There still is that issue with __raw_* doing both barrier-less and
> > endianswap-less accesses though. I think there is a fundamental problem
> > here with drivers like matroxfb using them to get endian-less access and
> > losing barriers at the same time.
>
> Before I put __raw_* there, code was using direct *(u_int32_t*)(mmio +
> reg) = value, and nobody complained... (and it worked on my PReP box).
> It seems that PPC does not reorder concurrent writes targetting one
> device.

It may re-order write vs. reads, nobody complained because on most old
machines, the CPU would be too dumb to do really heavy re-ordering but
that is no longer the case. This is definitely a bug.

> > I'd rather have matroxfb use writel with an explicit swap, or better, the
> > driver could maybe disable big endian register access and switch the card
> > to little endian, provided it can do that while keeping the frame buffer
> > itself set to BE (which is necessary most of the time).
>
> It is due to compatibility with XFree (or at least I was told) - they want
> both framebuffer and accelerator in big-endian mode, so there is really no
> choice (other than not supporting ppc...).

Hrm... having a quick look at mga driver in current Xorg tree, it uses
the MMIO_IN/OUT macros directly, those are not byteswapping ?

It also does this at one point (ugh !) :

#if X_BYTE_ORDER == X_BIG_ENDIAN
/* Disable byte-swapping for big-endian architectures - the XFree
driver seems to like a little-endian framebuffer -ReneR */
/* pReg->Option |= 0x80000000; */
pReg->Option &= ~0x80000000;
#endif

Weird... I think the X driver just lacks any "knowledge" of what's going
on with endianness...

> But of course, I can use writel(swab(...)) to get big-endian PCI
> accesses if __raw_* does not work on your hardware...

It's not that "__raw_*" does not work for my hardware ... it's that __raw_*
is always wrong to use on MMIO register accesses (unless you know _exactly_
what you are doing, for example it may be acceptable for filling a fifo in
some cases provided the first & last writes are not __raw)

Ben.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/