All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH] Fixed cfi flash read uchar bug.
Date: Sun, 07 Jan 2007 11:40:08 +0100	[thread overview]
Message-ID: <20070107104008.C0083352636@atlas.denx.de> (raw)
In-Reply-To: Your message of "Sun, 07 Jan 2007 04:12:07 CST." <45A0C777.1000508@orkun.us>

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.

  reply	other threads:[~2007-01-07 10:40 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-22  7:27 [U-Boot-Users] [PATCH] Fixed cfi flash read uchar bug wei.zhang at freescale.com
2006-12-22 10:00 ` Wolfgang Denk
2006-12-22 11:02   ` Zhang Wei
2006-12-22 14:11     ` Wolfgang Denk
2006-12-22 16:34       ` [U-Boot-Users] 答复: " Zhang Wei-r63237
2006-12-22 17:17         ` Wolfgang Denk
2006-12-25  7:03           ` Zhang Wei
2006-12-25 23:24             ` Wolfgang Denk
2006-12-26  6:04               ` Zhang Wei
2007-01-04  2:07                 ` Zhang Wei-r63237
2007-01-04  8:17                   ` Wolfgang Denk
2007-01-04  8:36               ` Zhang Wei-r63237
2007-01-04  9:23                 ` Wolfgang Denk
2007-01-04 11:19                   ` Stefan Roese
2007-01-05 13:27                   ` Stefan Roese
2007-01-07 10:12                     ` Tolunay Orkun
2007-01-07 10:40                       ` Wolfgang Denk [this message]
2007-01-13  7:11                         ` Tolunay Orkun
2007-01-13 17:53                           ` Håvard Skinnemoen
2007-01-16  6:52                             ` Stefan Roese
2007-01-25  4:38                           ` Zhang Wei-r63237
2007-01-25 16:10                             ` Timur Tabi
2007-01-25 20:33                               ` Wolfgang Denk
2007-01-26  2:13                                 ` Zhang Wei-r63237
2007-01-29 11:29                                   ` Tolunay Orkun
2007-01-30  3:36                                     ` Wang Haiying-r54964
2007-01-31  9:01                                       ` Tolunay Orkun
2007-01-31 19:25                                         ` Haiying Wang
2007-02-01  5:26                                           ` Tolunay Orkun
2007-02-01 22:06                                             ` Haiying Wang
2007-02-01 22:52                                               ` Chris Fester
2007-02-09 17:47                                             ` [U-Boot-Users] [PATCH0/2] Re-do the patch for adding DO_SYNC in flash_write_cmd Haiying Wang
2007-02-09 19:42                                               ` Wolfgang Denk
2007-02-09 19:48                                                 ` Haiying Wang
2007-02-10  1:04                                                   ` Wolfgang Denk
2007-02-10  7:23                                                     ` Tolunay Orkun
2007-02-10  7:40                                                       ` Stefan Roese
2007-02-10  7:57                                                         ` Tolunay Orkun
2007-02-10  8:07                                                           ` Stefan Roese
2007-02-10 21:55                                                             ` Wolfgang Denk
2007-02-10 21:54                                                         ` Wolfgang Denk
2007-02-11  2:34                                                         ` Timur Tabi
2007-02-11 10:23                                                           ` Wolfgang Denk
2007-02-09 19:59                                                 ` Haavard Skinnemoen
2007-02-10  1:02                                                   ` Wolfgang Denk
2007-02-09 17:47                                             ` [U-Boot-Users] [PATCH 1/2] Add DO_SYNC at the end of flash_write_cmd Haiying Wang
2007-02-09 19:45                                               ` Wolfgang Denk
2007-02-09 17:47                                             ` [U-Boot-Users] [PATCH 2/2] Define DO_SYNC in each CPU's header file Haiying Wang
2007-02-09 19:46                                               ` Wolfgang Denk
2007-02-10  7:17                                                 ` Tolunay Orkun
2007-01-08  2:41                     ` [U-Boot-Users] [PATCH] Fixed cfi flash read uchar bug Zhang Wei-r63237
2006-12-22 17:07       ` Chris Fester
2006-12-22 17:24         ` Wolfgang Denk
2006-12-22 17:42           ` Chris Fester
2006-12-22 21:33             ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070107104008.C0083352636@atlas.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.