[U-Boot] [RFC PATCH v2] ARM: Avoid compiler optimization for usages of readb, writeb and friends.

[Dirk Behme]

On 20.12.2010 07:07, John Rigby wrote:
> On Sun, Dec 19, 2010 at 9:18 PM, John Rigby<john.rigby at linaro.org>  wrote:
>> On Sun, Dec 19, 2010 at 5:56 PM, Alexander Holler<holler at ahsoftware.de>  wrote:
>>> Am 20.12.2010 01:39, schrieb John Rigby:
>>>>
>>>> On Sun, Dec 19, 2010 at 12:59 PM, Alexander Holler<holler at ahsoftware.de>
>>>>   wrote:
>>>> ...
>>>>>
>>>>> No EEPROM on expansion board
>>>>> Die ID #062a000400000000040365fa16019019
>>>>> Hit any key to stop autoboot:  0
>>>>> OMAP3 beagleboard.org # nand info
>>>>>
>>>>> Device 0: nand0, sector size 16 KiB
>>>>> --------------------------------
>>>>>
>>>> I get the same output  without your change.  My gcc is linaro 4.4.5.
>>>> I'll do some bisecting and try to find out what is going on.
>>>
>>> Bisecting won't help you here. Not if the problem was always there (which is
>>> what I assume
>> Sorry, I was confused about my results.
>>
>> If I replace include<asm/io.h>  in drivers/mtd/nand/omap_gpmc.c with a
>> copy of the original called orig_io.h:
>> #include "orig_io.h"
>>
>> Nand starts working again.  So the problem seems to be isolated to this file.
>>>
>>> Regards,
>>>
>>> Alexander
>>>
>>
>
> With your patch and the following hack nand works:
>
>
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index 99b9cef..5e94155 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -29,6 +29,8 @@
>   #include<linux/mtd/nand_ecc.h>
>   #include<nand.h>
>
> +#define origwriteb(v,a)                        __arch_putb(v,a)
> +
>   static uint8_t cs;
>   static struct nand_ecclayout hw_nand_oob = GPMC_NAND_HW_ECC_LAYOUT;
>
> @@ -58,7 +60,7 @@ static void omap_nand_hwcontrol(struct mtd_info
> *mtd, int32_t cmd,
>          }
>
>          if (cmd != NAND_CMD_NONE)
> -               writeb(cmd, this->IO_ADDR_W);
> +               origwriteb(cmd, this->IO_ADDR_W);
>   }
>
>   /*
>
> The working assembly looks like this:
>
> 	if (cmd != NAND_CMD_NONE)
> 80024d28:	e3710001 	cmn	r1, #1
> 		origwriteb(cmd, this->IO_ADDR_W);
> 80024d2c:	15933004 	ldrne	r3, [r3, #4]
> 80024d30:	120110ff 	andne	r1, r1, #255	; 0xff
> 80024d34:	15c31000 	strbne	r1, [r3]
> 80024d38:	e8bd8010 	pop	{r4, pc}
>
> The broken assembly looks like this:
>
> 	if (cmd != NAND_CMD_NONE)
> 80024d28:	e3710001 	cmn	r1, #1
> 80024d2c:	08bd8010 	popeq	{r4, pc}
> 		writeb(cmd, this->IO_ADDR_W);
> 80024d30:	e5933004 	ldr	r3, [r3, #4]
> 80024d34:	e20110ff 	and	r1, r1, #255	; 0xff
> 80024d38:	e5c31000 	strb	r1, [r3]
> 80024d3c:	e5d33000 	ldrb	r3, [r3]
> 80024d40:	e8bd8010 	pop	{r4, pc}

Hmm. From functionality point of view, the 'broken' assembly below 
should to the same as the working assembly, above. The main difference 
is the 'popeq	{r4, pc}' and the additional 'ldrb	r3, [r3]'. The write 
to the HW 'strb	r1, [r3]' is there, so it should work. Is this 
understanding correct?

If it's correct, the question is, what breaks the below assembly? The 
popeq or the additional ldrb? The popeq looks correct, but why is the 
additional ldrb there?

For some further debugging, I had two ideas: Modifying the resulting 
binary with a hex editor and replacing the ldrb with a nop (if I 
remember correctly the hex code for a nop is ffffffff?). Or modifying 
the the C code and adding a barrier behind the writeb(). E.g.

if (cmd != NAND_CMD_NONE);
         writeb(cmd, this->IO_ADDR_W);
         __asm__ __volatile__ ("dsb" : : : "memory");
}

Best regards

Dirk