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

Albert ARIBAUD
2010-12-21T10:38:44Z

Le 21/12/2010 09:46, John Rigby a ?crit :
> On Tue, Dec 21, 2010 at 1:35 AM, Dirk Behme<dirk.behme@googlemail.com> wrote:
>>
>> (Resend with corrected broken example)
>>
>> On 21.12.2010 08:21, Albert ARIBAUD wrote:
>>> Hi Dirk,
>>>
>>> Le 21/12/2010 08:11, Dirk Behme a ?crit :
>>>
>>>> But the issue with drivers/mtd/nand/omap_gpmc.c (i.e. the additional
>>>> ldrb r3, [r3]) is still open? Has anybody tried to replace it with
>>>> a nop in the binary to be sure this is the root cause?
>>>
>>> Can you try and preprocess the C file for both the broken and working
>>> cases, then post the preprocessed C extract? Differences at the C level
>>> may help understanding differences at the asm level.
>>
>> gcc version 4.5.1 (Sourcery G++ Lite 2010.09-50)
>>
>> Work:
>> ====
>>
>> static void omap_nand_hwcontrol(struct mtd_info *mtd, int32_t cmd,
>> uint32_t ctrl)
>> {
>> register struct nand_chip *this = mtd->priv;
>> ...
>> if (cmd != -1)
>>
>> (*(volatile unsigned char *)(this->IO_ADDR_W) = (cmd));
>> }
>>
>> if (cmd != NAND_CMD_NONE)
>> 84: e3710001 cmn r1, #1
>> origwriteb(cmd, this->IO_ADDR_W);
>> 88: 15933004 ldrne r3, [r3, #4]
>> 8c: 120110ff andne r1, r1, #255 ; 0xff
>> 90: 15c31000 strbne r1, [r3]
>> 94: e12fff1e bx lr
>> ...
>>
>>
>> Broken:
>> ======
>>
>> static void omap_nand_hwcontrol(struct mtd_info *mtd, int32_t cmd,
>> uint32_t ctrl)
>> {
>> register struct nand_chip *this = mtd->priv;
>>
>> ...
>>
>> if (cmd != -1)
>> ({ __asm__ __volatile__ ("" : : : "memory"); (*(volatile unsigned
>> char *)(this->IO_ADDR_W) = (cmd)); });
>> }
>>
>> if (cmd != NAND_CMD_NONE)
>> 84: e3710001 cmn r1, #1
>> 88: 012fff1e bxeq lr
>> writeb(cmd, this->IO_ADDR_W);
>> 8c: e5933004 ldr r3, [r3, #4]
>> 90: e20110ff and r1, r1, #255 ; 0xff
>> 94: e5c31000 strb r1, [r3]
>> 98: e5d33000 ldrb r3, [r3]
>> 9c: e12fff1e bx lr
>>
>>
>> The issue seems to be the additional 'ldrb r3, [r3]' added by the
>> compiler in the broken version.
>>
>
> And I at your suggestion tried modifying the binary changing the extra
> ldrb to a nop and it works.

Seems like a compiler issue to me, as the preprocessed C source is the
same for the register access and does not call for a re-read (that is
what I wanted to see in the preprocessed version), yet the ASM sequence
does the re-read.

Amicalement,
--
Albert.