[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Nicolas Lacressonnière]

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 at atmel.fr
Phone: 33 (0) 442 53 72 54
-----------------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20030603/129ff0ef/attachment.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-dataflash.gz
Type: application/x-gzip
Size: 24007 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20030603/129ff0ef/attachment.bin 

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Wolfgang Denk]

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 at denx.de
There is an order of things in this universe.
	-- Apollo, "Who Mourns for Adonais?" stardate 3468.1

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Nicolas Lacressonnière]

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 at 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 at 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>
>

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Wolfgang Denk]

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 at denx.de
Always try to do things in chronological order; it's  less  confusing
that way.

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Hamid IKDOUMI]

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 at 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 at denx.de
Always try to do things in chronological order; it's  less  confusing
that way.

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Wolfgang Denk]

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 at denx.de
In general, they do what you want, unless you want consistency.
                                    - Larry Wall in the perl man page

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Hamid IKDOUMI]

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 at denx.de]
Sent: mercredi 4 juin 2003 13:53
To: hikdoumi at 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 at denx.de
In general, they do what you want, unless you want consistency.
                                    - Larry Wall in the perl man page

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Nicolas Lacressonnière]

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 at 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 at denx.de
> In general, they do what you want, unless you want consistency.
>                                     - Larry Wall in the perl man page
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-dataflash.gz
Type: application/x-gzip
Size: 25584 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20030611/ef2eab26/attachment.bin 

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Wolfgang Denk]

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

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
grep me no patterns and I'll tell you no lines.

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Nicolas Lacressonnière]

Hi,

Please find attached a patch applied against the 1.0.0 release.

It implements some changes for Atmel AT91RM9200DK development kit. It adds:
- the possibility to set the environment variables in DataFlash
- Atmel DataFlash AT45DB1282 support

Tell me if there is something wrong

Nicolas.
-----------------------------------------------------------------
LACRESSONNIERE Nicolas

ARM-Based Products Application Group

ATMEL Rousset Zone Industrielle
13106 Rousset Cedex FRANCE
T?l: +33 (0)4 42 53 72 54
Fax: +33 (0)4 42 53 60 01
-----------------------------------------------------------------

-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-u-boot-AT91RM9200-dataflash.dat
Type: application/octet-stream
Size: 33046 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20031112/e7eb2af8/attachment.obj 

[U-Boot-Users] [PATCH] DataFlash for AT91RM9200DK board

[Wolfgang Denk]

In message <001901c3a8fc$6bde46a0$91f59f0a@pc0752> you wrote:
> 
> Please find attached a patch applied against the 1.0.0 release.
> 
> It implements some changes for Atmel AT91RM9200DK development kit. It add=
> s:
> - the possibility to set the environment variables in DataFlash
> - Atmel DataFlash AT45DB1282 support

Thanks, checked in.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Swap read error.  You lose your mind.