From mboxrd@z Thu Jan  1 00:00:00 1970
From: Albert ARIBAUD <albert.aribaud@free.fr>
Date: Tue, 21 Dec 2010 11:38:44 +0100
Subject: [U-Boot] [RFC PATCH v2] ARM: Avoid compiler optimization for
 usages of readb, writeb and friends.
In-Reply-To: <AANLkTikN-02Eay6u90DMKHuL8uvy7e0VLFSebxDsdoEo@mail.gmail.com>
References: <1292711230-3234-1-git-send-email-holler@ahsoftware.de>	<4D0DB994.1020703@googlemail.com>	<4D0DDCEF.2040806@ahsoftware.de>	<AANLkTik3-EL1VDpn1L5YY01Amv6VzSEKduxU=R9nPJrx@mail.gmail.com>	<4D0E643A.9040001@ahsoftware.de>	<AANLkTinWsOgqOXyLk+nN0AB8Kw33txG9Ot3Ji7Ckd-SO@mail.gmail.com>	<4D0EA9BB.4050002@ahsoftware.de>	<AANLkTikxZM3G2HTXhf1BNy28zkiBdJ-=Q4RWSv13ZQcS@mail.gmail.com>	<AANLkTiksYKRt7DMsVwqA--WAachCfJuC65v-XgotLLyS@mail.gmail.com>	<4D0F8E75.2010107@ahsoftware.de>	<AANLkTinXN-as+0-k9HZmh571vZPdzOh8hmq3zX4XSQta@mail.gmail.com>	<AANLkTinMK=gzYgyUTjkCg2y1g9YuRAu_P-DS0GARrfPR@mail.gmail.com>	<4D105324.2080201@googlemail.com>	<4D105583.8070703@free.fr>	<4D1066CD.2040409@googlemail.com>
	<AANLkTikN-02Eay6u90DMKHuL8uvy7e0VLFSebxDsdoEo@mail.gmail.com>
Message-ID: <4D1083B4.2060704@free.fr>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.