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

Benjamin Herrenschmidt
2004-09-22T01:37:58Z

On Wed, 2004-09-22 at 08:16, Linus Torvalds wrote:
> On Tue, 21 Sep 2004, Roland Dreier wrote:
> >
> > That means using __raw_writel() is pretty much guaranteed to blow up
> > on IBM pSeries (and I do care about pSeries for my driver).
>
> Oh, that's true. And that's pretty clearly a bug, since it just means that
> __raw_writel() can't even work in general.

Only when eeh is enabled for a device, never happens on your g5 :)

> > Maybe something like the patch below would make sense? (Reordering of
> > code is to make sure IO_TOKEN_TO_ADDR() is defined before the
> > __raw_*() functions; eeh.h has to be included after the in_*() and
> > out_*() functions are defined)
>
> I wonder if we could just remove the TOKEN/ADDR games. I think they were
> done entirely as a debugging aid (but I could be wrong). In particular,
> the compile-time type safefy should hopefully be better at finding these
> things in the long run, and in the short run the TOKEN games have
> obviously played their part.
>
> I wasn't using pp64 back when, maybe there's some other reason for playing
> games with the tokens? Who's the guity/knowledgeable party? Ben?

Well, g5 will never play token games on you. I need to investigate a bit
about what's up with pSeries, in the meantim, Roland patch looks fine.

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.

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).

Petr ?

Ben.