[PATCH] arm/io.h: add macros to read/write big/little endian register

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Viresh Kumar]

From: Pratyush Anand <pratyush.anand@st.com>

There are some peripheral (e.g dwc otg) whose registers can be configured to
work in either little or big endian mode. Therefore macros like out_be32,
in_be32, out_le32 and in_le32 have been added to support such peripherals.

Signed-off-by: Pratyush Anand <pratyush.anand@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/include/asm/io.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 9275828..a1ccac0 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -192,6 +192,16 @@ static inline void __iomem *__typesafe_io(unsigned long addr)
 #define insw_p(port,to,len)	insw(port,to,len)
 #define insl_p(port,to,len)	insl(port,to,len)
 
+/* Big Endian */
+#define out_be32(v, p)	({ __iowmb(); __raw_writel((__force __u32) \
+					cpu_to_be32(v), __io(p)); })
+#define in_be32(p)	({ __u32 __v = be32_to_cpu((__force __be32) \
+			__raw_readl(__io(p))); __iormb(); __v; })
+
+/* Little endian */
+#define out_le32(v, p)		outl(v, p)
+#define in_le32(p)		inl(p)
+
 /*
  * String version of IO memory access ops:
  */
-- 
1.7.8.110.g4cb5d

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 02:47:40PM +0530, Viresh Kumar wrote:
> From: Pratyush Anand <pratyush.anand@st.com>
> 
> There are some peripheral (e.g dwc otg) whose registers can be configured to
> work in either little or big endian mode. Therefore macros like out_be32,
> in_be32, out_le32 and in_le32 have been added to support such peripherals.

NAK.

1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
2. We should have readb_be() etc instead.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Viresh Kumar]

On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
> 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly

We mistakenly placed it outside #ifdef __io, #endif
Sorry, i am still missing your point.

> 2. We should have readb_be() etc instead.

Actually, existing drivers are using out_be32(), etc in their implementation.
What do you suggest in order to use these drivers for SPEAr/ARM.

-- 
viresh

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 04:23:11PM +0530, Viresh Kumar wrote:
> On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
> > 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
> 
> We mistakenly placed it outside #ifdef __io, #endif
> Sorry, i am still missing your point.

inb() et.al. are for PCI/ISA IO, not for general platform MMIO.

> > 2. We should have readb_be() etc instead.
> 
> Actually, existing drivers are using out_be32(), etc in their implementation.
> What do you suggest in order to use these drivers for SPEAr/ARM.

Which drivers?

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 11:19:06AM +0000, Russell King - ARM Linux wrote:
> On Thu, Feb 23, 2012 at 04:23:11PM +0530, Viresh Kumar wrote:
> > On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
> > > 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
> > 
> > We mistakenly placed it outside #ifdef __io, #endif
> > Sorry, i am still missing your point.
> 
> inb() et.al. are for PCI/ISA IO, not for general platform MMIO.
> 
> > > 2. We should have readb_be() etc instead.
> > 
> > Actually, existing drivers are using out_be32(), etc in their implementation.
> > What do you suggest in order to use these drivers for SPEAr/ARM.
> 
> Which drivers?

I see, some PPC folk allowed their private IO accessors to spread into
generic drivers despite a comment in their io.h saying that they shouldn't.

The right solution is that this crap needs to get fixed, rather than
spreading around yet another IO accessor which was supposed to be private
to an arch.  I've mailed the PPC folk about it.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Pratyush Anand]

On 2/23/2012 4:49 PM, Russell King - ARM Linux wrote:
> On Thu, Feb 23, 2012 at 04:23:11PM +0530, Viresh Kumar wrote:
>> On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
>>> 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
>>
>> We mistakenly placed it outside #ifdef __io, #endif
>> Sorry, i am still missing your point.
>
> inb() et.al. are for PCI/ISA IO, not for general platform MMIO.
>

Ok.. So, will it be fine if we keep it outside #ifdef __io and do not 
use __io macro in their implementation?

>>> 2. We should have readb_be() etc instead.
>>
>> Actually, existing drivers are using out_be32(), etc in their implementation.
>> What do you suggest in order to use these drivers for SPEAr/ARM.
>
> Which drivers?
> .
>

We have used these macros in dwc_otg driver, which is still to be added 
in the main line , but is under discussion.

http://comments.gmane.org/gmane.linux.usb.general/53348

However, I do see that these macros have been used in several mainline 
driver also, e.g. drivers/usb/gadget/fsl_qe_udc.c.

Regards
Pratyush

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 05:04:28PM +0530, Pratyush Anand wrote:
> On 2/23/2012 4:49 PM, Russell King - ARM Linux wrote:
>> On Thu, Feb 23, 2012 at 04:23:11PM +0530, Viresh Kumar wrote:
>>> On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
>>>> 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
>>>
>>> We mistakenly placed it outside #ifdef __io, #endif
>>> Sorry, i am still missing your point.
>>
>> inb() et.al. are for PCI/ISA IO, not for general platform MMIO.
>>
>
> Ok.. So, will it be fine if we keep it outside #ifdef __io and do not  
> use __io macro in their implementation?

No, I refuse to add another set of accessors to ARMs io.h.  They're private
to PPC - it even says so in their header file.  They should not have leaked
out of PPC.

The fact that 73 drivers in the kernel source use their accessors is their
problem to solve.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Stefan Roese]

Hi Pratyush,

On Thursday 23 February 2012 12:34:28 Pratyush Anand wrote:
> >> Actually, existing drivers are using out_be32(), etc in their
> >> implementation. What do you suggest in order to use these drivers for
> >> SPEAr/ARM.
> > 
> > Which drivers?
> > .
> 
> We have used these macros in dwc_otg driver, which is still to be added
> in the main line , but is under discussion.
> 
> http://comments.gmane.org/gmane.linux.usb.general/53348

Did you notice this recent thread on the USB devel list?

http://www.spinics.net/lists/linux-usb/msg58420.html

I just wanted to make sure, that you don't continue working on this big and 
ugly dwc_otg driver (yes, I worked on it as well), when it has small to zero 
chances for upstreaming. Better to consolidate all efforts on this "smaller" 
driver with the other SoC vendors (Samsung etc).

Thanks,
Stefan

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Arnd Bergmann]

On Thursday 23 February 2012, Russell King - ARM Linux wrote:
> On Thu, Feb 23, 2012 at 05:04:28PM +0530, Pratyush Anand wrote:
> > On 2/23/2012 4:49 PM, Russell King - ARM Linux wrote:
> >> On Thu, Feb 23, 2012 at 04:23:11PM +0530, Viresh Kumar wrote:
> >>> On 2/23/2012 3:23 PM, Russell King - ARM Linux wrote:
> >>>> 1. Using the PCI/ISA IO macros for non-PCI/ISA IO purposes is silly
> >>>
> >>> We mistakenly placed it outside #ifdef __io, #endif
> >>> Sorry, i am still missing your point.
> >>
> >> inb() et.al. are for PCI/ISA IO, not for general platform MMIO.
> >>
> >
> > Ok.. So, will it be fine if we keep it outside #ifdef __io and do not  
> > use __io macro in their implementation?

It would be technically correct unlike an implementation based on inb/outb,
the question is whether it would be a good solution for the problem at
hand. We've had too many ad-hoc fixes in this area leading us to the
mess we're in now, so we should be very careful about getting it right
this time.

> No, I refuse to add another set of accessors to ARMs io.h.  They're private
> to PPC - it even says so in their header file.  They should not have leaked
> out of PPC.
> 
> The fact that 73 drivers in the kernel source use their accessors is their
> problem to solve.

The reason why for ppc to have introduced these is the need for accessors
that work for on-chip devices on machines where the PCI readl/writel and
ioread32/iowrite32 do something funny like using an indirect register
or a hypercall. We have a similar problem on ARM with
CONFIG_IXP4XX_INDIRECT_PCI and possibly the new highbank platforms
that might be doing the same thing in order to get the most out of the
4GB address space.

The mistake on ppc was to make them private to one architecture instead of
coming up with a well-defined set of accessors that are provided on all
architectures. We certainly have too many accessors already between
inl/readl/ioread32/__raw_readl/readl_relaxed/in_le32, which makes this
rather confusing, but IMHO there is still the need for cleanup and
addition of missing variants like cpu-endian non-relaxed and
big-endian relaxed as well as a replacement for __raw_* that actually
defines them to something useful across architectures.

In case of the dgw otg driver, we can add a simple hack like

#if defined(CONFIG_PPC) || defined(CONFIG_MICROBLAZE) || defined(CONFIG_M68K)
#define dwg_readl(x)	in_le32(x)
#else
#define dwg_readl(x)	readl(x)
#endif

But there is a danger of spreading such hacks everywhere across the
kernel and making the mess bigger. I think a better direction would be
to define one set of accessors across architectures that can be
safely used in this driver. The choices that I can see are:

1. define the {in,out}_{le,be}{8,16,32,64} accessors for *all* architectures
following the ppc semantics (fixed endianess, strict ordering, __iomem but
not PCI). On ARM, this would be the same as the readl() family for all
platforms that have a memory mapped PCI memory space.

2. change the ppc definition of the readl_relaxed() family to something
useful (fixed endian, relaxed ordering, iomem (possibly including PCI
-- same as in_le32 but without the ordering guarantee) and make any
driver that wants to be portable use those accessors with explicit I/O
barriers on spinlocks and DMA.

3. Add a new set of consistent accessors that are explicitly meant for
non-PCI on-chip components and make sure we really get them right this
time and can retire some of the other ones eventually. This means it
needs to be completely arch independent, include relaxed and strict
accesses, little-endian/cpu-endian/native-endian and a well-documented
address space (or more than one).

	Arnd

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 12:33:04PM +0000, Arnd Bergmann wrote:
> 3. Add a new set of consistent accessors that are explicitly meant for
> non-PCI on-chip components and make sure we really get them right this
> time and can retire some of the other ones eventually. This means it
> needs to be completely arch independent, include relaxed and strict
> accesses, little-endian/cpu-endian/native-endian and a well-documented
> address space (or more than one).

That won't work.  What's behind PCI in one area is on-chip on another.
See that with various PXA peripherals appearing behind PCI devices on
x86.  That's also happening with AMBA peripherals as well.

We *really* need to get away from that "PCI is special and has its own
accessors" or "on chip is special and has its own accessors" madness.
That's already starting to bite as being wrong.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Arnd Bergmann]

On Thursday 23 February 2012, Russell King - ARM Linux wrote:
> On Thu, Feb 23, 2012 at 12:33:04PM +0000, Arnd Bergmann wrote:
> > 3. Add a new set of consistent accessors that are explicitly meant for
> > non-PCI on-chip components and make sure we really get them right this
> > time and can retire some of the other ones eventually. This means it
> > needs to be completely arch independent, include relaxed and strict
> > accesses, little-endian/cpu-endian/native-endian and a well-documented
> > address space (or more than one).
> 
> That won't work.  What's behind PCI in one area is on-chip on another.
> See that with various PXA peripherals appearing behind PCI devices on
> x86.  That's also happening with AMBA peripherals as well.
> 
> We really need to get away from that "PCI is special and has its own
> accessors" or "on chip is special and has its own accessors" madness.
> That's already starting to bite as being wrong.

This would mean going through an indirect function call for platforms
where PCI is fundamentally different in hardware. We already do this
for CONFIG_PPC_INDIRECT_PCI, but I'd hope we can avoid this on ARM.

Since devices that can bei either PCI or not are relatively rare, I'd
rather put them in the same category as those that can either use
PCI I/O space or some form of memory space and require that the
drivers use the "I don't care what you are, just do what I want"
accessors in the ioread/iowrite family that are already going through
an indirect function call on most architectures.

Also, the PCI aware readl/writel functions are already more heavyweight
on most architectures (not on x86 though) than we want them to be,
because they have to do extra synchronizing operations against DMA
and/or spinlocks on weakly ordered architectures. We now have
the readl_relaxed family here, but I believe they are only used
for on-chip components at the moment.

	Arnd

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 01:35:04PM +0000, Arnd Bergmann wrote:
> Since devices that can bei either PCI or not are relatively rare, I'd
> rather put them in the same category as those that can either use
> PCI I/O space or some form of memory space and require that the
> drivers use the "I don't care what you are, just do what I want"
> accessors in the ioread/iowrite family that are already going through
> an indirect function call on most architectures.

It's not a case of whether they themselves are PCI or not.  It's a case
that manufacturers are starting to take existing ARM IP and sticking a
PCI bridge in front of them.

That's why there's someone looking at a set of PCI drivers which create
AMBA primecell devices to allow the existing primecell drivers to bind
to these peripherals.

What that means is that in theory, you could have an AMBA primecell
peripheral behind a PCI device on an IXP4xx platform with its weird
windows, and that would necessitate the AMBA primecell drivers to use the
special PCI space accessors.

I would not be surprised if some of these PPC freescale peripheral IPs
using their private in_xxx() accessors end up in the same situation.
That seems to be the direction the hardware folk are going.

So, I think it's a big mistake to distinguish bus types by their
accessors.  Or put it another way, it's a big mistake to have bus
specific accessors.

Consider the AMBA primecell behind a PCI device, and if AMBA had its own
bus specific accessors.  Should we litter the drivers with ifdefs to
select between the PCI accessors or the AMBA accessors?  Or something
equally horrible?

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Benjamin Herrenschmidt]

> The reason why for ppc to have introduced these is the need for accessors
> that work for on-chip devices on machines where the PCI readl/writel and
> ioread32/iowrite32 do something funny like using an indirect register
> or a hypercall. We have a similar problem on ARM with
> CONFIG_IXP4XX_INDIRECT_PCI and possibly the new highbank platforms
> that might be doing the same thing in order to get the most out of the
> 4GB address space.

Well, the "generic" ones can do the workarounds conditionally based on
bits of the address. However it does add overhead to SoC drivers that
don't need it.

That's indeed one of the reasons for some drivers to use our specific
ones.

> The mistake on ppc was to make them private to one architecture instead of
> coming up with a well-defined set of accessors that are provided on all
> architectures.

That was never going to fly, we already have too many.

>  We certainly have too many accessors already between
> inl/readl/ioread32/__raw_readl/readl_relaxed/in_le32, which makes this
> rather confusing, but IMHO there is still the need for cleanup and
> addition of missing variants like cpu-endian non-relaxed and
> big-endian relaxed as well as a replacement for __raw_* that actually
> defines them to something useful across architectures.

Right, the _relaxed don't have clear semantics and we do need "real"
relaxed ones badly as in "don't have memory barriers". Currently, a
driver that wants to avoid the sync's we have all over the place in our
accessors for performance reasons has to use __raw_* and thus handle
endian manually as well, which sucks since it also cannot use the
reverse-load/store instructions in that case.

I tried to propose a uniform set a while back, but that never went
anywhere, I should have fought harder. You are welcome to bring that
back on the table :-)

> In case of the dgw otg driver, we can add a simple hack like
> 
> #if defined(CONFIG_PPC) || defined(CONFIG_MICROBLAZE) || defined(CONFIG_M68K)
> #define dwg_readl(x)	in_le32(x)
> #else
> #define dwg_readl(x)	readl(x)
> #endif
> 
> But there is a danger of spreading such hacks everywhere across the
> kernel and making the mess bigger. I think a better direction would be
> to define one set of accessors across architectures that can be
> safely used in this driver. The choices that I can see are:

Yes. We used to have that sort of hacks in the early days as well, I'd
rather not go back down that path.

> 1. define the {in,out}_{le,be}{8,16,32,64} accessors for *all* architectures
> following the ppc semantics (fixed endianess, strict ordering, __iomem but
> not PCI). On ARM, this would be the same as the readl() family for all
> platforms that have a memory mapped PCI memory space.

I'm sure there's going to be resistance here. A better option would be
to properly define __readX/__writeX or something like that (maybe using
iomap variants) as having no built-in PCI specific workarounds.

That still leaves us with the need for properly defined relaxed ones.

> 2. change the ppc definition of the readl_relaxed() family to something
> useful (fixed endian, relaxed ordering, iomem (possibly including PCI
> -- same as in_le32 but without the ordering guarantee) and make any
> driver that wants to be portable use those accessors with explicit I/O
> barriers on spinlocks and DMA.

Last I looked, __relaxed() had a completely different meaning (relaxed
ordering -on-the-bus- which is a totally different thing, so iirc only
one arch ever implemented it).

> 3. Add a new set of consistent accessors that are explicitly meant for
> non-PCI on-chip components and make sure we really get them right this
> time and can retire some of the other ones eventually. This means it
> needs to be completely arch independent, include relaxed and strict
> accesses, little-endian/cpu-endian/native-endian and a well-documented

I don't mind either, it's trivial for us to add them, it's all macro
generated anyway :-)

Cheers,
Ben.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Benjamin Herrenschmidt]

On Thu, 2012-02-23 at 13:35 +0000, Arnd Bergmann wrote:
> 
> This would mean going through an indirect function call for platforms
> where PCI is fundamentally different in hardware. We already do this
> for CONFIG_PPC_INDIRECT_PCI, but I'd hope we can avoid this on ARM.

On PPC we do it a bit more sneakily using a bit in the address. It
causes the inline accessor to be a big bigger but avoids the cost of the
indirect call for the "normal" case.

> Since devices that can bei either PCI or not are relatively rare, I'd
> rather put them in the same category as those that can either use
> PCI I/O space or some form of memory space and require that the
> drivers use the "I don't care what you are, just do what I want"
> accessors in the ioread/iowrite family that are already going through
> an indirect function call on most architectures.
> 
> Also, the PCI aware readl/writel functions are already more
> heavyweight
> on most architectures (not on x86 though) than we want them to be,
> because they have to do extra synchronizing operations against DMA
> and/or spinlocks on weakly ordered architectures. We now have
> the readl_relaxed family here, but I believe they are only used
> for on-chip components at the moment.

On-chip have the same issue vs. DMA and we wouldn't want the defaults to
let them leak out of locks etc...

Cheers,
Ben.

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Pratyush Anand]

On 2/23/2012 5:31 PM, Stefan Roese wrote:
> Did you notice this recent thread on the USB devel list?
>
> http://www.spinics.net/lists/linux-usb/msg58420.html
>
> I just wanted to make sure, that you don't continue working on this big and
> ugly dwc_otg driver (yes, I worked on it as well), when it has small to zero
> chances for upstreaming. Better to consolidate all efforts on this "smaller"
> driver with the other SoC vendors (Samsung etc).

Nice to know that a driver exists in main line under different name.
Will discuss about it on linux-usb mailing list.

Thanks & Regards
Pratyush

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Arnd Bergmann]

On Thursday 23 February 2012, Benjamin Herrenschmidt wrote:

> > The mistake on ppc was to make them private to one architecture instead of
> > coming up with a well-defined set of accessors that are provided on all
> > architectures.
> 
> That was never going to fly, we already have too many.

It's certainly not easy to get consensus across arch maintainers, especially
for a topic that is so complex as this one. On the other hand, we did add
the readl_relaxed family just recently to all architectures, and I think
it was good to do it this way.
 
> >  We certainly have too many accessors already between
> > inl/readl/ioread32/__raw_readl/readl_relaxed/in_le32, which makes this
> > rather confusing, but IMHO there is still the need for cleanup and
> > addition of missing variants like cpu-endian non-relaxed and
> > big-endian relaxed as well as a replacement for __raw_* that actually
> > defines them to something useful across architectures.
> 
> Right, the _relaxed don't have clear semantics and we do need "real"
> relaxed ones badly as in "don't have memory barriers". Currently, a
> driver that wants to avoid the sync's we have all over the place in our
> accessors for performance reasons has to use __raw_* and thus handle
> endian manually as well, which sucks since it also cannot use the
> reverse-load/store instructions in that case.

Using __raw_* is actually worse since it's not really well-defined 
what that means across architectures. We recently had a bug where
gcc would create byte-wise accesses on registers that are marked
"packed" by a driver writer, because the __raw_writel function just
does a cast to a variable of larger alignment, which is undefined
(IIRC, but maybe just implementation defined) in C.

> I tried to propose a uniform set a while back, but that never went
> anywhere, I should have fought harder. You are welcome to bring that
> back on the table :-)

Since the _relaxed version comes from ARM, it's basically defined
to be convenient there, which means it is synchronized against the
instruction stream (spinlock, most importantly) but not against
DMA, because that tends to be expensive.

> > 2. change the ppc definition of the readl_relaxed() family to something
> > useful (fixed endian, relaxed ordering, iomem (possibly including PCI
> > -- same as in_le32 but without the ordering guarantee) and make any
> > driver that wants to be portable use those accessors with explicit I/O
> > barriers on spinlocks and DMA.
> 
> Last I looked, __relaxed() had a completely different meaning (relaxed
> ordering -on-the-bus- which is a totally different thing, so iirc only
> one arch ever implemented it).

I think you may be confusing it with relaxed ordering on DMA. Having
MMIO with relaxed ordering between the accesses would be silly. The
only reason we have it on ARM is to remove the need to do an expensive
synchronization against a DMA operation on the same bus. IIRC that
would let you do an "eieio" instead of a full "sync" on powerpc but
should only be used in drivers that don't do DMA.

	Arnd

[PATCH] arm/io.h: add macros to read/write big/little endian register

[Benjamin Herrenschmidt]

On Fri, 2012-02-24 at 16:22 +0000, Arnd Bergmann wrote:
> 
> It's certainly not easy to get consensus across arch maintainers, especially
> for a topic that is so complex as this one. On the other hand, we did add
> the readl_relaxed family just recently to all architectures, and I think
> it was good to do it this way.

Well, nobody added _relaxed ones to powerpc (at least not meaningful
ones) nor do I remember being CCed on the patch going in :-) In any
case, see below.

> Using __raw_* is actually worse since it's not really well-defined 
> what that means across architectures. We recently had a bug where
> gcc would create byte-wise accesses on registers that are marked
> "packed" by a driver writer, because the __raw_writel function just
> does a cast to a variable of larger alignment, which is undefined
> (IIRC, but maybe just implementation defined) in C.

Right, __raw_* is basically useless as an abstraction.

> > I tried to propose a uniform set a while back, but that never went
> > anywhere, I should have fought harder. You are welcome to bring that
> > back on the table :-)
> 
> Since the _relaxed version comes from ARM, it's basically defined
> to be convenient there, which means it is synchronized against the
> instruction stream (spinlock, most importantly) but not against
> DMA, because that tends to be expensive.

Argh. Crap. And why did you synchronize it with locks ? Seriously,
synchronizing vs. instruction stream is horribly costly for IO and
generally non-sensical. (The sync. vs a lock isn't even vs. instruction
stream, it's also a storage ordering issue).

Anyways, if you require the semantic of synchronizing against locks then
power will have to keep sync's in there which totally defeats the
purpose for us :-(

Was that ever discussed with CC on the most likely to be affected archs
such as us ? Looks like not...

> > > 2. change the ppc definition of the readl_relaxed() family to something
> > > useful (fixed endian, relaxed ordering, iomem (possibly including PCI
> > > -- same as in_le32 but without the ordering guarantee) and make any
> > > driver that wants to be portable use those accessors with explicit I/O
> > > barriers on spinlocks and DMA.
> > 
> > Last I looked, __relaxed() had a completely different meaning (relaxed
> > ordering -on-the-bus- which is a totally different thing, so iirc only
> > one arch ever implemented it).
> 
> I think you may be confusing it with relaxed ordering on DMA. Having
> MMIO with relaxed ordering between the accesses would be silly.

Well, my memory might be blurry, but back when I tried to get that
sorted at least one arch (and it wasn't ARM from memory) had _relaxed
accessors and the meaning of _relaxed was ... something else. Really.
Unless I was lied to :-)

> only reason we have it on ARM is to remove the need to do an expensive
> synchronization against a DMA operation on the same bus. IIRC that
> would let you do an "eieio" instead of a full "sync" on powerpc but
> should only be used in drivers that don't do DMA.

That wouldn't solve the problem with locks. Besides, we'd want "relaxed"
to not have eieio either wouldn't we ?

Cheers,
Ben.