From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Sun, 07 Jan 2007 11:40:08 +0100 Subject: [U-Boot-Users] [PATCH] Fixed cfi flash read uchar bug. In-Reply-To: Your message of "Sun, 07 Jan 2007 04:12:07 CST." <45A0C777.1000508@orkun.us> Message-ID: <20070107104008.C0083352636@atlas.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Tolunay, in message <45A0C777.1000508@orkun.us> you wrote: > > > Please find attached a small patch that adds fixes this potential problem for > > the 3 functions flash_read_uchar/ushort/long. Please give it a try and let me > > know if this changed the behavior somehow. > > I think this is a good idea given GCC 4.x is agressive in optimizations. I already discussed this internally with Stefan. I *don't* think it's a good idea. I really hate to change bits and pieces of code without really understanding why we are doing this. In the current situation we are accessing flash memory which is supposed to be in read mode, i. e. it behaves like normal system memory. It should be no problem even if the flash memory content was cached. So why would "volatile" improve anything? As long as I don't see (for example in the generated assembler code) how a problem might exist, and how the suggested patch fixes exactly this problem, I'd like to continue researching this problem. > I do not think converting these to use memcpy() is a good idea. I am > with Wolfgang on this. Actually I might have been wrong in my assessment here, when I stated that memcpy() performs a character-wise copy, too. The simple code from lib_generic/string.c is used only if __HAVE_ARCH_MEMCPY is undefined, and especially on PPC we define this (in include/asm-ppc/string.h). So we might use an optimized versions where it *does* make a difference. Checking this idea I see that we are actually using the memcpy() code from lib_ppc/ppcstring.S which *does* implement some optimiation, but I then I don't think this is efffecting us here. > I think we should commit Stefan's patch even if it might or might not > solve the problem Zhang is experiencing. Please explain why you think adding volatile's here would be a good thing, andin which way it improves the code. Best regards, Wolfgang Denk -- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "What happened to the crewman?" "The M-5 computer needed a new power source, the crewman merely got in the way." -- Kirk and Dr. Richard Daystrom, "The Ultimate Computer", stardate 4731.3.