[U-Boot] [PATCH] cfi_flash: Micron Nor flash don't support read operation after send

[Wolfgang Denk]

Dear Qi Wang 王起 (qiwang),

In message <71CF8D7F32C5C24C9CD1D0E02D52498A770D149E at NTXXIAMBX02.xacn.micron.com> you wrote:
>
> Yes, I totally agree with you, copy from NOR flash to NOR flash is not support.

Fine.

> But in the flash_write_cfibuffer() function, flash_read16(src) is used to read data
> from src address, I think this "flash_read" function give users much mislead, src address
> can be located in flash address range, not only can be in RAM.

Well, we cannot prevent users from misinterpreting the code.

> And currently, one of micron customers uses "cp.b" command to copy data from NOR flash 
> to Nor flash, such as "cp.b src_addr dest_addr len", both src_addr and dest_addr are 
> located in NOR flash address range. Some manufactory's Nor flash can work fine under 
> this command, but micron nor flash cannot. 

U-Boot is a boot loader, i. e. we have always to find a compromize
between resource consumption and functionality.  Just because a user
tries to do something that is not supported does not mean we should
make the code bigger and more complex - users will find all kinds of
exotic ways to do things.

In this specific case, there is a simple and efficient way to perform
the operation attempted by the user without code changes, so it makes
sense to just educate this user.

> As you said, copy from NOR flash to NOR flash is forbidden. 
> In the cp command function, 
> do_mem_cp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]),
> you can see below code, it just judge if the dest address is located in NOR flash, but
> don't do any detect on src address.

Yes, and this is perfectly OK so.  Consider the case we have more than
one NOR flash device on a system.  It is perfectly legal to copy data
from one NOR flash chip into another one.

> So my suggestion is,
> 1. don't use flash_read function in flash_write_cfibuffer() function, use 
>    readb(),readw(), readl() function instead.
> 2. add detection in do_mem_cp() function, check if src address is locate in NOR flash address range.
> 
> So what's your opinions?

I see no need for any code changes.  Everything is working as
expected.

I repeat: U-Boot is a boot loader, not a general purpose OS.  This
means, that users of the command line interface aresupposed to
understand what they are doing.  There is no such strict error
checking and measures against all possible kinds of incorrect or
illegal input as you might expect from user space applications or from
the OS.

I think the only thing that should be done is to improve the
documentation and explicitly warn that copying of data within the same
bank of NOR flash cannot work.  Please feel free to edit [1] as
needed.  Thanks.

[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.1.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A consultant is a person who borrows your watch, tells you what  time
it is, pockets the watch, and sends you a bill for it.