[U-Boot-Users] [Patch 5/5] Add DataFlash support for AT91SAM9261EK board

[Lacressonniere Nicolas]

Hi Wolfgang,

> I reject this patch. Dataflash support is supposed to be  transparent
> to  the  user. IT should be supported by the existing flash commands.
> Please change the code to use the existing "protect" command like  it
> was done before for dataflash, too.
Our board does not use any parallel flash part... So if I enable
CFG_CMD_FLASH flag, I need to add an unnecessary flash.c file in
board/at91sam9261ek directory in order to have access to low-level flash
functions (flash_init...) So do I have to create such a file with empty
functions? Or can we create another set of command? What do you think is
best?


> Also, I don't  see  any  need  for  a  "dflc  init"  command  -  such
> initialization  should be done when needed and without needing manual
> user interaction.
OK, I will remove it.

> And "dflc info" is  supposed  to  be  part of  the
> "flinfo" output on your hardware.
See first part...

> * Add addr2ram verification in do_mem_cp function.
> I also reject the patch because I think that such "verification" is a
> bad thing.
OK, I will remove it.

Best regards.
Nicolas.

-----Original Message-----
From: wd at denx.de [mailto:wd at denx.de]
Sent: mardi 24 janvier 2006 18:40
To: Lacressonniere Nicolas
Cc: U-Boot-Users
Subject: Re: [U-Boot-Users] [Patch 5/5] Add DataFlash support for
AT91SAM9261EK board


In message
<KAEELLICOFHDAEPIACDEAEPJCFAA.nicolas.lacressonniere at rfo.atmel.com> you
wrote:
>
> DataFlash patch (5/5) adds DataFlash support for ATMEL AT91SAM9261EK
board.
>
> It also adds the following dataflash commands (in cmd_dataflash.c):
> U_BOOT_CMD(
> 	dflc,	5,	1,	do_dataflash_command,
> 	"dflc         - DATAFLASH utility command\n",
> 	"dflc init    - Init connected dataflash cards\n"
> 	"dflc info    - scan for connected dataflash devices\n"
> 	"dflc protect - protect or unprotect sectors\n"
> );

I reject this patch. Dataflash support is supposed to be  transparent
to  the  user. IT should be supported by the existing flash commands.
Please change the code to use the existing "protect" command like  it
was done before for dataflash, too.

Also, I don't  see  any  need  for  a  "dflc  init"  command  -  such
initialization  should be done when needed and without needing manual
user interaction. And "dflc info" is  supposed  to  be  par  tof  the
"flinfo" output on your hardware.

> CHANGELOG
> Patch by Nicolas Lacressonniere 24 January 2006
> * Add DataFlash support for ATMEL AT91SAM9261EK board (arm926ejs)
> * Add DataFlash utility commands
> * Add addr2ram verification in do_mem_cp function.

I also reject the patch because I think that such "verification" is a
bad thing.

"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."       - Doug Gwyn

Your "verification" prevents a lot of "clever things",  like  copying
data to a framebuffer or some dual ported RAM or on-chip memory or...

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
Always leave room to add an explanation if it doesn't work out.