On Monday 20 June 2011, Nicolas Pitre wrote:
> On Mon, 20 Jun 2011, Arnd Bergmann wrote:
>
> > On Saturday 18 June 2011, Nicolas Pitre wrote:
> > Well, in case of readl/writel, we need the memory clobber for other reasons:
> > They might trigger a DMA operation, or the readl might be used to wait for
> > completion of a DMA operation. In either case, you need to have a barrier
> > to prevent the reordering of the MMIO instruction w.r.t the DMA memory that
> > it is referring to. On in-order CPUs, this does not always require a
> > synchronisation instruction, but it strill requires a compiler barrier,
> > which is typically expressed as a full memory clobber.
>
> We don't have any of that now though.
We do have __iormb() in there, which is both a CPU and a compiler barrier.
Unfortunately, in case of !CONFIG_ARM_DMA_MEM_BUFFERABLE it degrades to
no barrier at all, which is a bug.
> And I think this would be much more sensible if the barrier was
> explicit, so not to generate awful code as I demonstrated, in the vast
> majority of the cases where there is no such issues.
Yes, we can recommend to people to use the _relaxed versions, especially
for drivers that don't do any DMA.
> And the fact that the inline asm is marked volatile should be enough to
> prevent reordering already, isn't it?
No. It only makes sure that the asm will get executed, but does not
prevent the compiler from reordering other statements around it.
> > > static inline unsigned int readl(volatile unsigned int *a)
> > > {
> > > unsigned int v;
> > > asm volatile ("ldr\t%1, %0" : : "+o" (*a), "=r" (v));
> > > return v;
> > > }
> > >
> > > The bidirectional qualifier indicates that we're going to modify the
> > > memory pointed by a, and yet it is going to be different after that,
> > > which pretty much matches how hardware registers behave.
> >
> > I don't see how the "+" is needed here, because the compiler won't
> > be accessing the pointers otherwise, and doesn't have the option of
> > omitting any accesses either.
>
> It could prevents the compiler from assuming that the memory content at
> that location is still containing the same data that was just
> written/read, even if the volatile was omitted (note I'm not suggesting
> to remove 'volatile').
But the compiler doesn't know teh contents of the pointer anyway, so
even if it assumes that it hasn't changed, that won't mean anything at
all.
Arnd
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-toolchain
