[U-Boot] [PATCH 0/4 V2] Add basic NVIDIA Tegra2 SoC support

[Peter Tyser]

On Sat, 2011-01-15 at 00:00 +0100, Albert ARIBAUD wrote:
> Hello,
> 
> Le 14/01/2011 23:39, Tom Warren a écrit :
> 
> > So instead of, say uart->lcr = 0, you'd prefer writel(0, uart->lcr),
> > where writel = __arch_putl(v, a) = (*(volatile unsigned int *)(a) =
> > (v))?
> > Is that different enough from 'uart->lcr = 0' to warrant the change?
> > Does it add some HW barriers or forced read-before-write that the
> > 'volatile' struct doesn't?
> 
> writel() and readl() do not introduce "read-before-write", that is, they 
> do not perform any more than what their names imply, but yes they do 
> introduce barriers, or more precisely, they force the compiler to do so.

Agreed, I should have dug deeper.  On PPC we use out_be32() or similar
to access memory mapped registers, which does have an explicit barrier.
I'm not familiar with ARM so don't know what the proper access functions
are, but it looks like the defacto standard writel()/readl().

I'd personally use writel()/readl() in this patch.  Functionally it may
be the same as your volatile accesses, but its the generally recommended
practice.  It looks like most of the Tegra support in the Linux kernel
also uses writel()/readl() for what its worth.  Ultimately its up to the
ARM U-Boot maintainer to decide if they require the access functions or
not - I'm just giving my opinions from PPC experience.

Regards,
Peter