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

[Hein Tibosch <hein_tibosch@yahoo.es>]

On 8/21/2012 2:35 PM, Viresh Kumar wrote:
> On 21 August 2012 11:42, Hein Tibosch <hein_tibosch@yahoo.es <mailto: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.
>
> 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)

It got swapped as 0xAABB.CCDD => 0xCCDD.AABB

> 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.
>
AVR32 (AP700x) is a big-endian system.

It took me many hours to figure out why the DMA controller didn't do
anything at all.

Then I decided to access the i/o memory in different way:

    #include "dmaca_206a.h" // from the AVR32-tools library

    const struct __iomem avr32_dmaca_t *regs =
        (const struct __iomem avr32_dmaca_t *)0xFF200000;

and printk all register values. Each and every u32 field was swapped as
0xAABBCCDD => 0xCCDDAABB

Håvard's comment was: "it's always wrong to use little endian accessors
(i.e. readl/writel) in drivers for chip-internal devices"

Memory barriers: within the AVR32 code, one often sees explicit ways to
introduce memory barriers, e.g.:

    hsmc_readl(hsmc, MODE0); /* I/O barrier */

which means: reading back (just any) register (from the same peripheral)
to assure that the write was successful, especially from within
interrupt handlers.

But as I said, within avr32-linux I never encountered problems
with the __raw* approach, including dw_dmac.c

> @Arnd: What should we do here?

I'm curious too

Hein