Re: [PATCH ARM v8 2/4] mini-os: arm: interrupt controller

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2014-10-21 at 23:54 +0200, Samuel Thibault wrote:
> Ian Campbell, le Tue 21 Oct 2014 12:00:18 +0100, a écrit :
> > On Fri, 2014-10-03 at 10:20 +0100, Thomas Leonard wrote:
> > > +static inline uint32_t REG_READ32(volatile uint32_t *addr)
> > > +{
> > > +    uint32_t value;
> > > +    __asm__ __volatile__("ldr %0, [%1]":"=&r"(value):"r"(addr));
> > > +    rmb();
> > 
> > I'm not 100% convinced that you need this rmb().
> > 
> > > +    return value;
> > > +}
> > > +
> > > +static inline void REG_WRITE32(volatile uint32_t *addr, unsigned int value)
> > > +{
> > > +    __asm__ __volatile__("str %0, [%1]"::"r"(value), "r"(addr));
> > > +    wmb();
> > > +}
> 
> I don't really see why such barriers are needed indeed. Are they needed
> to actually push the values out?

That would, I think, require an isb() (instruction barrier) whereas
wmb() turns into a dsb() (data barrier). I expect you are write and
these rmb/wmb are not needed, but an isb may be needed in the caller if
they want to rely on the affect of a write (e.g. enabling the
controller)

Ian.