On Tuesday 21 August 2012, Viresh Kumar wrote:
> On 21 August 2012 14:17, Arnd Bergmann <[email protected]> wrote:
>
> > On Tuesday 21 August 2012, Viresh Kumar wrote:
> > > On 21 August 2012 14:04, Arnd Bergmann <[email protected]> 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
On 8/21/2012 5:05 PM, Arnd Bergmann wrote:
> 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.
I already thought the 16-bit swap might be related to the 16-bit SMC
configuration. SMC will fetch each u32 word in 2 different 16-bit banks
of SDRAM.
In a meanwhile I tried dw_dmac using iowrite32be/ioread32be, and it worked
equally well! Which isn't surprising because for AVR32 they were defined as:
#define iowrite32be(v,p) __raw_writel(v, p)
#define ioread32be(p) ((unsigned int)__raw_readl(p))
The 'relaxed' versions won't work because:
#define readl_relaxed readl
The object code confirms what was expected:
u32 val = ioread32be (((unsigned*)0x100)); /* Same as __raw_readl */
ld.w r8,pc[256]
iowrite32be (val, ((unsigned*)0x104)); /* same as __raw_writel */
st.w pc[260],r8
val = readl (((unsigned*)0x108)); /* __raw_readl + swahb32 */
ld.w r8,pc[264]
lsl r9,r8,0x10 /* 16-bit swap */
or r9,r9,r8>>0x10
writel (val, ((unsigned*)0x10C)); /* swahb32 + __raw_writel */
lsl r8,r9,0x10 /* 16-bit swap */
or r8,r8,r9>>0x10
st.w pc[268],r8
Hein