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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
