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

Dirk Behme
2010-12-20T06:49:01Z

On 20.12.2010 07:07, John Rigby wrote:
> On Sun, Dec 19, 2010 at 9:18 PM, John Rigby<john.rigby@linaro.org> wrote:
>> On Sun, Dec 19, 2010 at 5:56 PM, Alexander Holler<holler@ahsoftware.de> wrote:
>>> Am 20.12.2010 01:39, schrieb John Rigby:
>>>>
>>>> On Sun, Dec 19, 2010 at 12:59 PM, Alexander Holler<holler@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