On Sun, Nov 14, 2010 at 10:03:56PM +0100, Gabor Juhos wrote:
> >> +static inline u32 ath79_spi_rr(struct ath79_spi *sp, unsigned reg)
> >> +{
> >> + return __raw_readl(sp->base + reg);
> >> +}
> >> +
> >> +static inline void ath79_spi_wr(struct ath79_spi *sp, unsigned reg, u32 val)
> >> +{
> >> + __raw_writel(val, sp->base + reg);
> >> +}
> >
> > This is suspect. Why is __raw_{readl,writel} being used instead of
> > ioread32/iowrite32? The __raw versions don't provide any kind of
> > ordering barriers.
>
> Mainly because the resulting code is smaller, and the performance is a bit
> better with the use of the __raw versions. The controller is embedded into the
> SoC and the registers are memory mapped, so i think it is safe to access them
> with __raw_{readl,writel}. However I can change it if that is the preferred method.
>
Smaller, yes, because it doesn't have any io barriers; but is it safe?
Do you know whether or not the CPU will reorder the instructions on
you? Being embedded into the SoC doesn't really mean anything in this
regard. Unless you really understand all the behaviour of the CPU and
bus, then the safe versions must be used.
If you *do* really understand all the behaviour and decide it is safe
to use the __raw versions, then the driver needs to be well documented
as to the reasons why the __raw versions are safe to use.
g.
