From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Mon, 10 Jan 2011 17:13:42 +0100 Subject: [U-Boot] [PATCH v3] ARM: Avoid compiler optimization for usages of readb, writeb and friends. In-Reply-To: <20110109222550.81536150A44@gemini.denx.de> References: <1293015862-3678-1-git-send-email-holler@ahsoftware.de> <4D1F1841.5060508@googlemail.com> <20110109222550.81536150A44@gemini.denx.de> Message-ID: <4D2B3036.4010506@googlemail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Wolfgang, On 09.01.2011 23:25, Wolfgang Denk wrote: > Dear Dirk Behme, > > In message<4D1F1841.5060508@googlemail.com> you wrote: >> >> Do you like to test the patch in the attachment? I named it 'v4'. > > Please send patches inline. > >> After some thinking and testing, it seems to me that the volatile >> optimization issue this patch shall fix is only with the readx() >> macros. So the idea is to drop all writex() changes done in the v3 >> version of this patch. With dropping the writex() changes, we would >> drop all issues we discussed with e.g. the GCC statement-expression >> and the do while workaround, too. > > This makes no sense. Even if we experience problems only with read*() > at the moment, we should to the Rigth Thing (TM) and fix both the > read*() and write*() functions. The question I was thinking about with my patch was "what's Right Thing?" ;) It's my understanding that we don't fix read*() and write*() because they are broken. We touch them to work around a broken tool chain. We saw that this specific tool chain has issues with read*(). While working around this, we touched write*(), too. This was done in the wrong way. So while read*() was fine, write*() was accidentally broken (with all tool chains), then. So we could (a) do write*() correctly, too (as you do in your patch below) or (b) just don't touch write*() as it isn't needed to work around the read*() tool chain issue (as I proposed in my patch v4) Anyway: > Please have a look a the patch I just posted, > http://patchwork.ozlabs.org/patch/78056/ I'm fine with that patch. Thanks Dirk