[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board
Find attached a patch applied against the 0.3.0 release.
It implements several changes for Atmel AT91RM9200DK development kit. - Add Atmel DataFlash support for reading and writing (new entry CFG_CMD_DATAFLASH in cmd_confdefs.h file) - Add Flash detection between AT49BV1614 and AT49BV1614A flashes. - Replace old Ethernet PHY configuration functions - New link address
Tell me if there is something wrong.
Nicolas. ----------------------------------------------------------------- Nicolas Lacressonniere ARM-based Products Application Group ATMEL Rousset - Zone Industrielle Fab7 - 13106 Rousset Cedex nlacressonniere@atmel.fr Phone: 33 (0) 442 53 72 54 -----------------------------------------------------------------
Dear Nicolas,
in message 031601c329e5$0c9fba70$91f59f0a@pc0752 you wrote:
Find attached a patch applied against the 0.3.0 release.
It implements several changes for Atmel AT91RM9200DK development kit.
- Add Atmel DataFlash support for reading and writing (new entry CFG_CMD_=
DATAFLASH in cmd_confdefs.h file)
Why do we need a special command to access the DataFlash? I would like to see the same interface as for all other flash devices.
Also, why did you place drivers/at45.c in the (common) drivers/ directory? It seems to be very CPU-specific code to me?
Thinking twice, the same is true for drivers/at91rm9200_ether.c: this code should IMHO go to a CPU dependend directory, but not to the common drivers/ directory.
- Add Flash detection between AT49BV1614 and AT49BV1614A flashes.
Your flash protection mechanism seems to be based on some #define'd CFG_* parameters; please check our changes in the current CVS version to get rid of such constants (like CFG_MON_LEN). Maybe you want to adjust your code?
- Replace old Ethernet PHY configuration functions
- New link address
Some files (drivers/at91rm9200_ether.c, include/AT91C_SPI_DataFlash.h, include/asm-arm/arch-at91rm9200/AT91RM9200.h, include/dataflash.h) do not contain GPL headers and/or copyright notices. Can you please add / fix these?
And please remove all C++ style comments (//), and trailing white space.
Best regards,
Wolfgang Denk
Wolfgang,
Why do we need a special command to access the DataFlash? I would like to see the same interface as for all other flash devices.
The DataFlash is a serial flash which is accessed like a serial eeprom. That is why we decided to create a special command to access it.
Concerning the other points, we will make the corrections.
Best regards.
Nicolas. ----------------------------------------------------------------- Nicolas Lacressonniere ARM-based Products Application Group ATMEL Rousset - Zone Industrielle Fab7 - 13106 Rousset Cedex nlacressonniere@atmel.fr Phone: 33 (0) 442 53 72 54 ----------------------------------------------------------------- ----- Original Message ----- From: "Wolfgang Denk" wd@denx.de To: "Nicolas Lacressonnière" nlacressonniere@atmel.fr Cc: "u-boot Mailing List" u-boot-users@lists.sourceforge.net; "Hamid Ikdoumi" hikdoumi@atmel.fr; "Olivier Debicki" odebicki@atmel.fr Sent: Tuesday, June 03, 2003 6:56 PM Subject: Re: [U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board
Dear Nicolas,
in message 031601c329e5$0c9fba70$91f59f0a@pc0752 you wrote:
Find attached a patch applied against the 0.3.0 release.
It implements several changes for Atmel AT91RM9200DK development kit.
- Add Atmel DataFlash support for reading and writing (new entry
CFG_CMD_=
DATAFLASH in cmd_confdefs.h file)
Why do we need a special command to access the DataFlash? I would like to see the same interface as for all other flash devices.
Also, why did you place drivers/at45.c in the (common) drivers/ directory? It seems to be very CPU-specific code to me?
Thinking twice, the same is true for drivers/at91rm9200_ether.c: this code should IMHO go to a CPU dependend directory, but not to the common drivers/ directory.
- Add Flash detection between AT49BV1614 and AT49BV1614A flashes.
Your flash protection mechanism seems to be based on some #define'd CFG_* parameters; please check our changes in the current CVS version to get rid of such constants (like CFG_MON_LEN). Maybe you want to adjust your code?
- Replace old Ethernet PHY configuration functions
- New link address
Some files (drivers/at91rm9200_ether.c, include/AT91C_SPI_DataFlash.h, include/asm-arm/arch-at91rm9200/AT91RM9200.h, include/dataflash.h) do not contain GPL headers and/or copyright notices. Can you please add / fix these?
And please remove all C++ style comments (//), and trailing white space.
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd@denx.de There is an order of things in this universe. -- Apollo, "Who Mourns for Adonais?" stardate 3468.1
This SF.net email is sponsored by: eBay Get office equipment for less on eBay! http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5 _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
In message 004f01c32a6f$63dcd860$91f59f0a@pc0752 you wrote:
Why do we need a special command to access the DataFlash? I would like to see the same interface as for all other flash devices.
The DataFlash is a serial flash which is accessed like a serial eeprom. That is why we decided to create a special command to access it.
What is it being used for, then?
Concerning the other points, we will make the corrections.
Thanks.
Best regards,
Wolfgang Denk
Hi Wolfgang,
I'm working with Nicholas on the dataflash driver for U-Boot. The dataflash is used to store big amount of data(up to 128 Mbits for the AT45DB128). In our case, we use it to store and boot an image of linux(the dataflash contains the linux zImage and the ramdisk). This dataflash can be used also for a filesystem.
regards, Hamid,
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: mercredi 4 juin 2003 11:56 To: Nicolas Lacressonnière Cc: u-boot Mailing List; Hamid Ikdoumi; Olivier Debicki Subject: Re: [U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board
In message 004f01c32a6f$63dcd860$91f59f0a@pc0752 you wrote:
Why do we need a special command to access the DataFlash? I would like to see the same interface as for all other flash devices.
The DataFlash is a serial flash which is accessed like a serial eeprom.
That
is why we decided to create a special command to access it.
What is it being used for, then?
Concerning the other points, we will make the corrections.
Thanks.
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd@denx.de Always try to do things in chronological order; it's less confusing that way.
In message 000d01c32a8d$a615aea0$90f59f0a@pc0709 you wrote:
I'm working with Nicholas on the dataflash driver for U-Boot. The dataflash is used to store big amount of data(up to 128 Mbits for the AT45DB128).
OK, so my understanding was correct.
In our case, we use it to store and boot an image of linux(the dataflash contains the linux zImage and the ramdisk). This dataflash can be used also for a filesystem.
In this case I think we should offer an interface which looks to the user like mmeory.
Instead of providing separate commands to read and write from such "memory" I think we should integrate this into the existing code.
For example, writing to "normal" flash memory is an implicit operation to the "cp" (copy) command when it detects that the target address is in flash memory.
The same should be done for dataflash.
OK, with "normal" flash memory we don't have to special-case read accesses like used by "md" or "bootm" commands - where in case of dataflash such handling will be necessary.
But especially if you boot Linux from dataflash it seems more logical to me to use "bootm" directly instead of using "dataflash read" + "bootm" in separate steps.
Do you t hink tis is feasible?
Best regards,
Wolfgang Denk
Ok! We will modify the plug the dataflash functions in the cmd_mem.c file and add the dataflash info in the flinfo command.
Best regards,
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: mercredi 4 juin 2003 13:53 To: hikdoumi@atmel.fr Cc: 'Nicolas Lacressonnière'; 'u-boot Mailing List'; 'Olivier Debicki' Subject: Re: [U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board
In message 000d01c32a8d$a615aea0$90f59f0a@pc0709 you wrote:
I'm working with Nicholas on the dataflash driver for U-Boot. The dataflash is used to store big amount of data(up to 128 Mbits for the AT45DB128).
OK, so my understanding was correct.
In our case, we use it to store and boot an image of linux(the dataflash contains the linux zImage and the ramdisk). This dataflash can be used also for a filesystem.
In this case I think we should offer an interface which looks to the user like mmeory.
Instead of providing separate commands to read and write from such "memory" I think we should integrate this into the existing code.
For example, writing to "normal" flash memory is an implicit operation to the "cp" (copy) command when it detects that the target address is in flash memory.
The same should be done for dataflash.
OK, with "normal" flash memory we don't have to special-case read accesses like used by "md" or "bootm" commands - where in case of dataflash such handling will be necessary.
But especially if you boot Linux from dataflash it seems more logical to me to use "bootm" directly instead of using "dataflash read" + "bootm" in separate steps.
Do you t hink tis is feasible?
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd@denx.de In general, they do what you want, unless you want consistency. - Larry Wall in the perl man page
Hi Wolfgang,
In that new patch (still applied on u-boot-0.3.0 version), we made the modifications concerning DataFlash support as you advise us to do below. It is now possible to boot Linux from DataFlash with Bootm command and to access DataFlash with cp, and md commands...
Also, why did you place drivers/at45.c in the (common) drivers/ directory? It seems to be very CPU-specific code to me?
Thinking twice, the same is true for drivers/at91rm9200_ether.c: this code should IMHO go to a CPU dependend directory, but not to the common drivers/ directory.
These two files are now placed in the cpu/at91rm9200/ directory.
- Add Flash detection between AT49BV1614 and AT49BV1614A flashes.
Your flash protection mechanism seems to be based on some #define'd CFG_* parameters; please check our changes in the current CVS version to get rid of such constants (like CFG_MON_LEN). Maybe you want to adjust your code?
It is not possible for us to use the monitor_flash_len variable because we use a gzipped U-Boot image in Flash which is uncompressed into SDRAM when our primary boot is executing. That's why we are forced to define three specific #define'd CFG_* parameters. I hope that won't be a problem.
Some files (drivers/at91rm9200_ether.c, include/AT91C_SPI_DataFlash.h, include/asm-arm/arch-at91rm9200/AT91RM9200.h, include/dataflash.h) do not contain GPL headers and/or copyright notices. Can you please add / fix
these?
Done!
Tell me if there is something wrong.
Nicolas. ----------------------------------------------------------------- Nicolas Lacressonniere ARM-based Products Application Group ATMEL Rousset - Zone Industrielle Fab7 - 13106 Rousset Cedex nlacressonniere@atmel.fr Phone: 33 (0) 442 53 72 54 ----------------------------------------------------------------- ----- Original Message ----- From: "Wolfgang Denk" wd@denx.de To: hikdoumi@atmel.fr Cc: "'Nicolas Lacressonnière'" nlacressonniere@atmel.fr; "'u-boot Mailing List'" u-boot-users@lists.sourceforge.net; "'Olivier Debicki'" odebicki@atmel.fr Sent: Wednesday, June 04, 2003 1:53 PM Subject: Re: [U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board
In message 000d01c32a8d$a615aea0$90f59f0a@pc0709 you wrote:
I'm working with Nicholas on the dataflash driver for U-Boot. The dataflash is used to store big amount of data(up to 128 Mbits for
the
AT45DB128).
OK, so my understanding was correct.
In our case, we use it to store and boot an image of linux(the dataflash contains the linux zImage and the ramdisk). This dataflash can be used also for a filesystem.
In this case I think we should offer an interface which looks to the user
like mmeory.
Instead of providing separate commands to read and write from such "memory" I think we should integrate this into the existing code.
For example, writing to "normal" flash memory is an implicit operation to the "cp" (copy) command when it detects that the target address is in flash memory.
The same should be done for dataflash.
OK, with "normal" flash memory we don't have to special-case read accesses like used by "md" or "bootm" commands - where in case of dataflash such handling will be necessary.
But especially if you boot Linux from dataflash it seems more logical to me to use "bootm" directly instead of using "dataflash read" + "bootm" in separate steps.
Do you t hink tis is feasible?
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd@denx.de In general, they do what you want, unless you want consistency. - Larry Wall in the perl man page
Hello,
in message 057001c32ffc$ef02fb80$91f59f0a@pc0752 you wrote:
In that new patch (still applied on u-boot-0.3.0 version), we made the modifications concerning DataFlash support as you advise us to do below. It is now possible to boot Linux from DataFlash with Bootm command and to access DataFlash with cp, and md commands...
Thanks.
Tell me if there is something wrong.
I've added your patches, but I had to clean up a couple of compile problems. Did you really run the MAKEALL script as required?
Please clean up ALL compiler warnings before submitting a patch! ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Also, please do not use C++ comments, and use the Linux kernel coding style for your sources.
Finally, I think there is a bug in your code. In "lib_arm/armlinux.c" you write (cast by me to avoid compiler warnings):
132 #ifdef CONFIG_HAS_DATAFLASH 133 if (addr_dataflash(addr)){ 134 read_dataflash(data, len, (char *)CFG_LOAD_ADDR); 135 data = CFG_LOAD_ADDR; 136 } 137 #endif
It is IMHO wrong to use a static, compile-time defined constant here; shouldn't this be the variable load_addr instead?
The same comment applies to "common/cmd_bootm.c"
Also, should "drivers/dataflash.c" not be moved into the at91 directory? It seems it can handle only at91 dataflash anyway? For example, it calls a function AT91F_DataFlashWrite() which is unknown outside the cpu/at91rm9200/ tree...
Checked in locally now, will push to CVS later this night.
Please check out from CVS, and check carefully. If necessary, submit a patch against CVS then (especially for the CFG_LOAD_ADDR issue).
Best regards,
Wolfgang Denk
participants (3)
-
Hamid IKDOUMI -
Nicolas Lacressonnière -
Wolfgang Denk