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@denx.de [mailto:wd@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@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@denx.de Always leave room to add an explanation if it doesn't work out.

OK Wolfgang,

We will use the same commands for flash and dataflash parts. I found an existing flag CFG_NO_FLASH that can be used to prevent from compiling some flash code so that we can use same commands without compiling specific flash part. Does that way seem OK for you?

I also have a question concerning new patches I have to submit. These CFG_NO_FLASH modifications have some impact on one of the patch ([Patch 1/5] Add support for AT91SAM9261EK board) I submitted yesterday and which was not rejected... Do I have to submit 2 new patches (previous one will be cancelled) or do I have to make a diff on impacted files and submit only the new patch (previous one will be keeped)?

Thanks in advance. Best regards.

Nicolas.

-----Original Message----- From: u-boot-users-admin@lists.sourceforge.net [mailto:u-boot-users-admin@lists.sourceforge.net]On Behalf Of Wolfgang Denk Sent: mercredi 25 janvier 2006 12:22 To: Lacressonniere Nicolas Cc: U-Boot-Users Subject: Re: [U-Boot-Users] [Patch 5/5] Add DataFlash support for AT91SAM9261EK board

Dear Nicolas,

in message KAEELLICOFHDAEPIACDEKEPOCFAA.nicolas.lacressonniere@rfo.atmel.com you wrote:

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?

You don;t need a file flash.c in your board directory; there are many boards without such a file - for example all boards using the CFI flash driver. It does not matter where the function flash_init() gets implemented - and you will need some function like that, too.

The question if we want to have special commands for dataflash was discussed a long time ago when the first board added support for such devices. By then it was decided that the standard flash commands (protect, erase, cp, flinfo) shall be used for dataflash, too, so that the behaviour is completely transparent for the user. I still think that was a good decision.

...

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.

Just rename this code into flash_init() and one of the problems mentioned above just disappears...

...

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

See first part...

See above. Please support the standard flinfo command.

...

• 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.

Thanks.

Best regards,

Wolfgang Denk

-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de The perversity of nature is nowhere better demonstrated by the fact that, when exposed to the same atmosphere, bread becomes hard while crackers become soft.

------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://sel.as-us.falkag.net/sel?cmd=lnk&kid%103432&bid#0486&dat%... _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users

Dear Wolfgang,

Do you really think this is needed? I think that the existing dataflash implementation (for the AT91RM9200) does not require any NOR flash either, and it does not need to do this.

Yes, I think it is necessary. I just had a look at AT91RM9200 implementation, and it includes all NOR flash stuff even if NOR flash is not used... (cmd_flash.c requires a board/at91rm9200dk/flash.c and so on...) As you told me before, we could use the same flash functions for NOR Flash and DataFlash but it would break all existing code using CONFIG_HAS_DATAFLASH flag. That's why I think the best thing is to use CFG_NO_FLASH flag which will avoid embedding all NOR Flash stuff...

If things are interdependent on such a level it's probably best to fix this first and then submit everything again, telling me to drop the previous set of patches.

OK, you can drop previous set of patches.

One more thing which is not clear to me. Concerning NAND development, you told me to use testing-NAND git branch. But do I have to submit every patches (board, lcd, usb...) against that branch or only the NAND part? If I submit everything against testing-NAND branch, will it be included in a future u-boot-1.1.5 release, for example?

Best regards. Nicolas.

-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: mercredi 25 janvier 2006 21:49 To: Lacressonniere Nicolas Cc: U-Boot-Users Subject: Re: [U-Boot-Users] [Patch 5/5] Add DataFlash support for AT91SAM9261EK board

Dear Nicalas,

in message KAEELLICOFHDAEPIACDEEEAFCGAA.nicolas.lacressonniere@rfo.atmel.com you wrote:

We will use the same commands for flash and dataflash parts. I found an existing flag CFG_NO_FLASH that can be used to prevent from compiling some flash code so that we can use same commands without

compiling

specific flash part. Does that way seem OK for you?

Do you really think this is needed? I think that the existing dataflash implementation (for the AT91RM9200) does not require any NOR flash either, and it does not need to do this.

I also have a question concerning new patches I have to submit. These CFG_NO_FLASH modifications have some impact on one of the patch ([Patch

1/5]

Add support for AT91SAM9261EK board) I submitted yesterday and which was

not

rejected... Do I have to submit 2 new patches (previous one will be cancelled) or do I have to make a diff on impacted files and submit only

the

new patch (previous one will be keeped)?

If things are interdependent on such a level it's probably best to fix this first and then submit everything again, telling me to drop the previous set of patches.

But then, your patches should not be dependent on each other in such a way in the first place.

Best regards,

Wolfgang Denk

-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Put your Nose to the Grindstone! -- Amalgamated Plastic Surgeons and Toolmakers, Ltd.

Dear Nicolas,

in message KAEELLICOFHDAEPIACDECEBHCGAA.nicolas.lacressonniere@rfo.atmel.com you wrote:

Concerning NAND development, you told me to use testing-NAND git branch. No problem with that.

Fine.

But do I have to submit every AT91SAM9261 patches (board, lcd, usb...) against that branch or only the NAND part? If I submit everything against testing-NAND branch, will it be included in a future u-boot-1.1.5 release, for example?

Please use the testing-NAND branch only foir NAND related patches; everything else should be based on the main branch (top of tree).

Best regards,

Wolfgang Denk