[PATCH] Orion NAND: Make dword load asm volatile to avoid GCC optimizing it away

[Nicolas Pitre]

On Thu, 20 Aug 2009, Simon Kagstrom wrote:

> On Wed, 19 Aug 2009 10:47:14 -0400 (EDT)
> Nicolas Pitre <nico at cam.org> wrote:
> 
> > I think that the early clobber buys you nothing, and the memory clobber 
> > is way overkill here.  The volatile ought to be all that is needed.  Can 
> > you confirm that only the addition of volatile makes the code OK?  My 
> > gcc version is 4.3.2 and that makes no difference what so ever as the 
> > code as is produces the correct result already in my case.
> 
> Yes, it works fine with 4.3.3 and 4.4.1 with this change. So the
> updated patch can be found below. I belive the early clobber should be
> there though, since (from the ARM architecture reference manual):
> 
>   If <addressing_mode> performs base register write-back and the base
>   register <Rn> is one of the two destination registers of the
>   instruction, the results are UNPREDICTABLE.
> 
> it works fine without the early clobber as well, but I'd feel more safe
> having it in.

But this isn't the case here.  We don't perform any writeback.
You get a writeback when you have an addressing mode of the form:

	insn	rd, [rn, rm]!
	insn	rd, [rn, #off]!
	insn	rd, [rn], #off

but not with:

	insn	rd, [rn, #off]

Etc.

Still the early clobber shouldn't have any adverse effect either, unlike 
the memory clobber.

Acked-by: Nicolas Pitre <nico at marvell.com>

Unless the MTD guys are going to pick this patch and push it to Linus 
soon I'll carry it in the Orion git repo.


> 
> // Simon
> --
> Orion NAND: Make asm volatile avoid GCC pushing ldrd out of the loop
> 
> GCC 4.3.3 and 4.4.1 happily moves the dword load instruction out of the
> loop in orion_nand_read_buf. This patch makes the instruction volatile
> to avoid the issue. I've discussed this at gcc-help, refer to the thread
> at
> 
>   http://gcc.gnu.org/ml/gcc-help/2009-08/msg00187.html
> 
> The early clobber is added to avoid the destination registers and the
> source register overlapping.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom at netinsight.net>
> ---
>  drivers/mtd/nand/orion_nand.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
> index 7ad9722..0d9d4bc 100644
> --- a/drivers/mtd/nand/orion_nand.c
> +++ b/drivers/mtd/nand/orion_nand.c
> @@ -61,7 +61,7 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  	buf64 = (uint64_t *)buf;
>  	while (i < len/8) {
>  		uint64_t x;
> -		asm ("ldrd\t%0, [%1]" : "=r" (x) : "r" (io_base));
> +		asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
>  		buf64[i++] = x;
>  	}
>  	i *= 8;
> -- 
> 1.6.0.4
>