Re: [PATCH] Fixes for dw_dmac and atmel-mci for AP700x

[Arnd Bergmann <arnd.bergmann@linaro.org>]

On Tuesday 21 August 2012, Viresh Kumar wrote:
> On 21 August 2012 11:42, Hein Tibosch <hein_tibosch@yahoo.es> wrote:
> 
> > On 8/21/2012 12:42 PM, viresh kumar wrote:
> > > Ahhhh!! Firstly we can't use __raw* for architectures >= ARMv6. It is not
> > > only for endianess but for memory barriers. Why are they getting swapped
> > > for your case? Does your processor and dw_dmac have different endianess?
> >
> > If I'm not wrong: the __raw_* functions will access the i/o memory in
> > native endianess.
> > As far as I know, all AVR32 drivers are currently using the __raw*
> > functions. I never
> > encountered  a problem with that.
> >
> 
> I don't have the best knowledge in this area :( and so cc-ing Arnd who can
> help us here.
> So my perception of these routines is:
> 
> readl is defined generically as:
> 
> #define readl(addr) __le32_to_cpu(__raw_readl(addr))
> 
> Which converts to a simple __raw_readl() for little endian systems and
> swapped bytes for a big endian system.

readl does much more than that:

* It guarantees that read accesses arrive at the device in the order they
  are written in the source code
* It maintains ordering between DMA and MMIO accesses
* It computes the correct address to dereference from an __iomem token.
* It guarantees that the access is done in a single atomic load, rather
  than a series of byte accesses.

All this is necessary to make device drivers work in general, although
on many simple (strictly ordered) architectures a pointer dereference
will do the same thing. Device drivers should never use the __raw_*
accessors themselves. Some architectures define their own bus-specific
accessors, but not everyone thinks that is a good idea.

> You wrote in the beginning
> >> - After 2.6.39, the registers were accessed using readl/writel
> >>   in stead of the __raw_readl and __raw_writel causing a 16-bit
> >>   swap of all values (little endian access)
> 
> What's this 16-bit swap here? It should be a 8 bit swap by __swab32()
> routine.
> 
> Is AVR32 a big-endian system? Probably big-endian, that's why values are
> getting
> swapped. And dw_dmac is the standard one, can call it little endian for the
> time being.
> 
> @Arnd: What should we do here?

Yes, AVR32 is big-endian. I assume that dw_dmac can be either configured
as little-endian or big-endian and that it is configured as big-endian
on AVR32.

To solve this, we can either make the endianess of dw_dmac run-time
detected, or we add a configuration symbol for it and use either
ioread32_be() or readl(), depending how this is set. This symbol
can be hardwired by architectures on which it is known in advance.
I've also seen some devices that can be configured into either
endianess, so if this is true for the dw_dmac, we could just force
it to be little-endian all the time.

	Arnd