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

Arnd Bergmann
2012-08-21T09:06:01Z

On Tuesday 21 August 2012, Viresh Kumar wrote:
> On 21 August 2012 14:17, Arnd Bergmann <arnd.bergmann@linaro.org> wrote:
>
> > On Tuesday 21 August 2012, Viresh Kumar wrote:
> > > On 21 August 2012 14:04, Arnd Bergmann <arnd.bergmann@linaro.org> wrote:
> > >
> > > > Yes, this is very strange. Maybe the compiler already splits the
> > > > access into two 16-byte loads and that confuses the device?
> > >
> > > @Arnd: Is compiler allowed to do that even when we have volatile
> > specified
> > > for the access? It shouldn't optimize the access at all i believe.
> >
> > Yes. The "volatile" keyword implies that the compiler has to do the access
> > exactly once and that it cannot reorder the access with others on the same
> > address, or read more data than is specified, but it can (and does) split
> > the access if there is a reason for that, e.g. when the address might
> > be misaligned and the architecture cannot do misaligned loads.
>
>
> But that can't be the case here. Isn't it? So we shouldn't have got such
> results.

It should be easy to tell from the object code whether this happened
or not. If it did, then we can investigate why gcc did that, otherwise
something else caused the strange byte swap.

The safe way to define the readl() function in asm/io.h is to
use an inline assembly that prevents the access from getting split,
but avr32 just uses a pointer dereference here.

I think I just found the answer elsewhere in
arch/avr32/mach-at32ap/include/mach/io.h, which defines

# define __mem_ioswabl(a, x) swahb32(x)

and that apparently does the halfword swap when CONFIG_AP700X_16_BIT_SMC
is set. This explains why Havard said it's wrong to use readl on
internal deviceson avr32, but unfortunately that rule conflicts with how
we define the accessors on ARM.

Arnd