public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot-Users] [patch] add support for "eeprom info"
@ 2008-01-23 16:13 Mike Frysinger
  2008-01-23 21:06 ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-23 16:13 UTC (permalink / raw)
  To: u-boot

This patch adds a new sub command to eeprom called "info".  This allows eeprom
driver writers to implement a way of querying the device.  For example, SPI
flashes have status commands, jedec ids, part ids, and other fun stuff.  It's
useful to be able to quickly probe this data (so you know things are detected
properly and all that jazz).

For example, on Blackfin boards, you can do:
bfin> eeprom info
SPI Device: m25p128 0x20 (ST) 0x20 0x18
Parameters: num sectors = 64, sector size = 262144, write size = 256
Flash Size: 128 mbit (16 mbyte)
Status: 0x00

I made the function weak so that people aren't required to implement this
function (mostly so that it does not break all the SPI drivers out there right
now).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c
index e5000e9..cc8c277 100644
--- a/common/cmd_eeprom.c
+++ b/common/cmd_eeprom.c
@@ -49,6 +49,7 @@ extern int  eeprom_read  (unsigned dev_addr, unsigned offset,
 			  uchar *buffer, unsigned cnt);
 extern int  eeprom_write (unsigned dev_addr, unsigned offset,
 			  uchar *buffer, unsigned cnt);
+extern int  eeprom_info (void) __attribute__((weak));
 #if defined(CFG_EEPROM_WREN)
 extern int eeprom_write_enable (unsigned dev_addr, int state);
 #endif
@@ -104,7 +105,8 @@ int do_eeprom ( cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 
 			puts ("done\n");
 			return rcode;
-		}
+		} else if (argc == 2 && eeprom_info && strcmp (argv[1], "info") == 0)
+			return eeprom_info ();
 	}
 
 	printf ("Usage:\n%s\n", cmdtp->usage);
@@ -435,6 +437,7 @@ U_BOOT_CMD(
 	"read  devaddr addr off cnt\n"
 	"eeprom write devaddr addr off cnt\n"
 	"       - read/write `cnt' bytes from `devaddr` EEPROM at offset `off'\n"
+	"eeprom info\n"
 );
 #else /* One EEPROM */
 U_BOOT_CMD(
@@ -443,6 +446,7 @@ U_BOOT_CMD(
 	"read  addr off cnt\n"
 	"eeprom write addr off cnt\n"
 	"       - read/write `cnt' bytes at EEPROM offset `off'\n"
+	"eeprom info\n"
 );
 #endif /* CFG_I2C_MULTI_EEPROMS */
 
diff --git a/include/common.h b/include/common.h
index 54083f1..511e2f6 100644
--- a/include/common.h
+++ b/include/common.h
@@ -298,6 +298,7 @@ int  eeprom_probe (unsigned dev_addr, unsigned offset);
 #endif
 int  eeprom_read  (unsigned dev_addr, unsigned offset, uchar *buffer, unsigned cnt);
 int  eeprom_write (unsigned dev_addr, unsigned offset, uchar *buffer, unsigned cnt);
+int  eeprom_info  (void) __attribute__((weak)); /* optional interface */
 #ifdef CONFIG_LWMON
 extern uchar pic_read  (uchar reg);
 extern void  pic_write (uchar reg, uchar val);

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-23 16:13 [U-Boot-Users] [patch] add support for "eeprom info" Mike Frysinger
@ 2008-01-23 21:06 ` Wolfgang Denk
  2008-01-23 21:15   ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-23 21:06 UTC (permalink / raw)
  To: u-boot

In message <200801231113.38864.vapier@gentoo.org> you wrote:
> This patch adds a new sub command to eeprom called "info".  This allows eeprom
> driver writers to implement a way of querying the device.  For example, SPI
> flashes have status commands, jedec ids, part ids, and other fun stuff.  It's
> useful to be able to quickly probe this data (so you know things are detected
> properly and all that jazz).

How is this suppoesed to work on the "normal" EEPROm devices which are
typically attached to the I2C bus?

> I made the function weak so that people aren't required to implement this
> function (mostly so that it does not break all the SPI drivers out there right
> now).

... and I2C.

> +extern int  eeprom_info (void) __attribute__((weak));
>  #if defined(CFG_EEPROM_WREN)
>  extern int eeprom_write_enable (unsigned dev_addr, int state);
>  #endif
> @@ -104,7 +105,8 @@ int do_eeprom ( cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  
>  			puts ("done\n");
>  			return rcode;
> -		}
> +		} else if (argc == 2 && eeprom_info && strcmp (argv[1], "info") == 0)
> +			return eeprom_info ();

... && eeprom_info && ...?

Does that mean that a weak function resolves to a NULL pointer? Is
this guaranteed?

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
The IQ of the group is the lowest IQ of a member of the group divided
by the number of people in the group.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-23 21:06 ` Wolfgang Denk
@ 2008-01-23 21:15   ` Mike Frysinger
  2008-01-23 21:23     ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-23 21:15 UTC (permalink / raw)
  To: u-boot

On Wednesday 23 January 2008, Wolfgang Denk wrote:
> In message <200801231113.38864.vapier@gentoo.org> you wrote:
> > This patch adds a new sub command to eeprom called "info".  This allows
> > eeprom driver writers to implement a way of querying the device.  For
> > example, SPI flashes have status commands, jedec ids, part ids, and other
> > fun stuff.  It's useful to be able to quickly probe this data (so you
> > know things are detected properly and all that jazz).
>
> How is this suppoesed to work on the "normal" EEPROm devices which are
> typically attached to the I2C bus?
>
> > I made the function weak so that people aren't required to implement this
> > function (mostly so that it does not break all the SPI drivers out there
> > right now).
>
> ... and I2C.

i have no idea, i dont use i2c flashes.  i dont know if there is any standard 
for them.  if there isnt, easy enough to protect with CONFIG_SPI.

> > +extern int  eeprom_info (void) __attribute__((weak));
> >  #if defined(CFG_EEPROM_WREN)
> >  extern int eeprom_write_enable (unsigned dev_addr, int state);
> >  #endif
> > @@ -104,7 +105,8 @@ int do_eeprom ( cmd_tbl_t * cmdtp, int flag, int
> > argc, char *argv[])
> >
> >  			puts ("done\n");
> >  			return rcode;
> > -		}
> > +		} else if (argc == 2 && eeprom_info && strcmp (argv[1], "info") == 0)
> > +			return eeprom_info ();
>
> ... && eeprom_info && ...?
>
> Does that mean that a weak function resolves to a NULL pointer? Is
> this guaranteed?

of course.  an undefined weak function resolves to 0.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080123/75f4d2bd/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-23 21:15   ` Mike Frysinger
@ 2008-01-23 21:23     ` Wolfgang Denk
  2008-01-23 22:29       ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-23 21:23 UTC (permalink / raw)
  To: u-boot

In message <200801231615.09447.vapier@gentoo.org> you wrote:
>
> > ... and I2C.
> 
> i have no idea, i dont use i2c flashes.  i dont know if there is any standard 
> for them.  if there isnt, easy enough to protect with CONFIG_SPI.

We're not talking about flashes. We're talking about EEPROM. For
example things like a AT24C164 or similar, see for example
http://www.atmel.com/dyn/resources/prod_documents/doc0105.pdf

> > Does that mean that a weak function resolves to a NULL pointer? Is
> > this guaranteed?
> 
> of course.  an undefined weak function resolves to 0.

Ah. I didn't know that. Thanks.

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
The existence of god implies a violation of causality.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-23 21:23     ` Wolfgang Denk
@ 2008-01-23 22:29       ` Mike Frysinger
  2008-01-24  0:44         ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-23 22:29 UTC (permalink / raw)
  To: u-boot

On Wednesday 23 January 2008, Wolfgang Denk wrote:
> In message <200801231615.09447.vapier@gentoo.org> you wrote:
> > > ... and I2C.
> >
> > i have no idea, i dont use i2c flashes.  i dont know if there is any
> > standard for them.  if there isnt, easy enough to protect with
> > CONFIG_SPI.
>
> We're not talking about flashes. We're talking about EEPROM. For
> example things like a AT24C164 or similar, see for example
> http://www.atmel.com/dyn/resources/prod_documents/doc0105.pdf

the semantics are irrelevant.  it's an external non-volatile memory device 
accessed via the eeprom command.  the document you posted looks like that 
device doesnt have any way of querying it, so eeprom_info() for that device 
would be useless.  if this is normal among i2c eeproms, i'll repost the patch 
with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support the 
jedec id command which allows for dynamic detection (which is how the 
Blackfin SPI driver that i wrote works).  hook up any SPI flash and it "just 
works".
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080123/92cf3990/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-23 22:29       ` Mike Frysinger
@ 2008-01-24  0:44         ` Wolfgang Denk
  2008-01-24  3:39           ` Mike Frysinger
  2008-01-24  9:13           ` Mike Frysinger
  0 siblings, 2 replies; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-24  0:44 UTC (permalink / raw)
  To: u-boot

In message <200801231729.18316.vapier@gentoo.org> you wrote:
> 
> the semantics are irrelevant.  it's an external non-volatile memory device
> accessed via the eeprom command.  the document you posted looks like that 
> device doesnt have any way of querying it, so eeprom_info() for that device 

Indeed. There is no way to query such devices.

> would be useless.  if this is normal among i2c eeproms, i'll repost the patch 

Thsi is normal for basicly *all* EEPROM and FRAM devices I know with
I2C or SPI interfaces.

> with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support the
> jedec id command which allows for dynamic detection (which is how the 
> Blackfin SPI driver that i wrote works).  hook up any SPI flash and it "jus> t 
> works".

I think you are doing something wrong when you try to use "eeprom" to
access "SPI flash" - these are differnt entities...

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
"I find this a nice feature but it is not according to  the  documen-
tation. Or is it a BUG?"   "Let's call it an accidental feature. :-)"
                       - Larry Wall in <6909@jpl-devvax.JPL.NASA.GOV>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  0:44         ` Wolfgang Denk
@ 2008-01-24  3:39           ` Mike Frysinger
  2008-01-24  4:24             ` Ben Warren
  2008-01-24  9:13           ` Mike Frysinger
  1 sibling, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-24  3:39 UTC (permalink / raw)
  To: u-boot

On Wednesday 23 January 2008, Wolfgang Denk wrote:
> In message <200801231729.18316.vapier@gentoo.org> you wrote:
> > with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support
> > the jedec id command which allows for dynamic detection (which is how the
> > Blackfin SPI driver that i wrote works).  hook up any SPI flash and it
> > "just works".
>
> I think you are doing something wrong when you try to use "eeprom" to
> access "SPI flash" - these are differnt entities...

then what is the "correct" method ?  i dont see any other usable option in 
u-boot for hooking up SPI flashes without writing my own subsystem.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080123/7355212e/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  3:39           ` Mike Frysinger
@ 2008-01-24  4:24             ` Ben Warren
  2008-01-24  5:17               ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Ben Warren @ 2008-01-24  4:24 UTC (permalink / raw)
  To: u-boot

Hey Mike,

Mike Frysinger wrote:
> On Wednesday 23 January 2008, Wolfgang Denk wrote:
>   
>> In message <200801231729.18316.vapier@gentoo.org> you wrote:
>>     
>>> with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support
>>> the jedec id command which allows for dynamic detection (which is how the
>>> Blackfin SPI driver that i wrote works).  hook up any SPI flash and it
>>> "just works".
>>>       
>> I think you are doing something wrong when you try to use "eeprom" to
>> access "SPI flash" - these are differnt entities...
>>     
>
> then what is the "correct" method ?  i dont see any other usable option in 
> u-boot for hooking up SPI flashes without writing my own subsystem.
> -mike
>   
Until recently SPI in U-boot was synonymous with 'SPI-based EEPROM'.  In 
fact, CONFIG_SPI doesn't mean 'configure support for a SPI controller', 
it means 'configure support for a SPI EEPROM'.  This is of course yet 
another artifact from another day that it would be nice to clean up.

I introduced CONFIG_HARD_SPI in the latest release cycle, and included a 
driver for a Freescale controller.  It would be nice to have other 
drivers added for generic SPI support.  My preference would be for you 
to expand the 'spi' command with something like:

'spi eeprom info'

regards,
Ben

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  4:24             ` Ben Warren
@ 2008-01-24  5:17               ` Mike Frysinger
  2008-01-24 11:13                 ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-24  5:17 UTC (permalink / raw)
  To: u-boot

On Wednesday 23 January 2008, Ben Warren wrote:
> Hey Mike,
>
> Mike Frysinger wrote:
> > On Wednesday 23 January 2008, Wolfgang Denk wrote:
> >> In message <200801231729.18316.vapier@gentoo.org> you wrote:
> >>> with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support
> >>> the jedec id command which allows for dynamic detection (which is how
> >>> the Blackfin SPI driver that i wrote works).  hook up any SPI flash and
> >>> it "just works".
> >>
> >> I think you are doing something wrong when you try to use "eeprom" to
> >> access "SPI flash" - these are differnt entities...
> >
> > then what is the "correct" method ?  i dont see any other usable option
> > in u-boot for hooking up SPI flashes without writing my own subsystem.
>
> Until recently SPI in U-boot was synonymous with 'SPI-based EEPROM'.  In
> fact, CONFIG_SPI doesn't mean 'configure support for a SPI controller',
> it means 'configure support for a SPI EEPROM'.  This is of course yet
> another artifact from another day that it would be nice to clean up.

i'm aware of this

> I introduced CONFIG_HARD_SPI in the latest release cycle, and included a
> driver for a Freescale controller.  It would be nice to have other
> drivers added for generic SPI support.  My preference would be for you
> to expand the 'spi' command with something like:
>
> 'spi eeprom info'

i dont see how in the long run, this is any different from today.  Wolfgang 
can still argue that the SPI flash is not an eeprom device, so "spi 
eeprom ..." is not appropriate.  which means there is still no subsystem 
whatsoever for accessing SPI flashes.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080124/210854d5/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  0:44         ` Wolfgang Denk
  2008-01-24  3:39           ` Mike Frysinger
@ 2008-01-24  9:13           ` Mike Frysinger
  2008-01-24 11:31             ` Wolfgang Denk
  1 sibling, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-24  9:13 UTC (permalink / raw)
  To: u-boot

On Wednesday 23 January 2008, Wolfgang Denk wrote:
> In message <200801231729.18316.vapier@gentoo.org> you wrote:
> > with eeprom_info behind CONFIG_SPI.  most SPI flashes nowadays support
> > the jedec id command which allows for dynamic detection (which is how the
> > Blackfin SPI driver that i wrote works).  hook up any SPI flash and it
> > "just works".
>
> I think you are doing something wrong when you try to use "eeprom" to
> access "SPI flash" - these are differnt entities...

let's make sure we're talking about the same thing.  SPI flashes are eeproms 
that have a SPI interface.  so Spansion's S25FLxxxx, ST's m25pxtmels 
AT45DBxxxx, Winbond's W25Xxx/W25Pxx, and such.  they need to be erased before 
writing, are split up into some unit size, etc...  all SPI flashes nowadays 
conform to the JEDEC standard (JEP106) which allows for querying of 
manufacturer/device ids so that they can be dynamically detected.  sounds to 
me like "eeprom" is the correct interface for utilizing these devices.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080124/48907962/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  5:17               ` Mike Frysinger
@ 2008-01-24 11:13                 ` Wolfgang Denk
  2008-01-25 13:31                   ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-24 11:13 UTC (permalink / raw)
  To: u-boot

In message <200801240017.46171.vapier@gentoo.org> you wrote:
>
> > drivers added for generic SPI support.  My preference would be for you
> > to expand the 'spi' command with something like:
> >
> > 'spi eeprom info'
> 
> i dont see how in the long run, this is any different from today.  Wolfgang
> can still argue that the SPI flash is not an eeprom device, so "spi 
> eeprom ..." is not appropriate.  which means there is still no subsystem 
> whatsoever for accessing SPI flashes.

You are right - I think "spi eeprom" is not a good choice - if it is
an EEPROM, then the "eeprom" command should work, no matter what the
actual bus interface is.

If the device is SPI flash, then why not call it like that, i. e. 

	spi flash info

Or
	spiflash info

to make it similar to the "nand" commands?

On the other hand, it might be still too shortsighted to tie this  to
the  bus  interface.  For example, what about dataflash? As discussed
many times before, we have similar issues with dataflash devices. 

I don't have a good idea how to do it, but maybe this is a chance to
get a good solution for NAND, dataflash, SPI flash and who knows what
else?

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
"To IBM, 'open' means there is a modicum  of  interoperability  among
some of their equipment."                            - Harv Masterson

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24  9:13           ` Mike Frysinger
@ 2008-01-24 11:31             ` Wolfgang Denk
  2008-01-25 13:33               ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-24 11:31 UTC (permalink / raw)
  To: u-boot

In message <200801240413.04318.vapier@gentoo.org> you wrote:
> 
> > I think you are doing something wrong when you try to use "eeprom" to
> > access "SPI flash" - these are differnt entities...
> 
> let's make sure we're talking about the same thing.  SPI flashes are eeproms 
> that have a SPI interface.  so Spansion's S25FLxxxx, ST's m25pxtmels 
> AT45DBxxxx, Winbond's W25Xxx/W25Pxx, and such.  they need to be erased before 
> writing, are split up into some unit size, etc...  all SPI flashes nowadays 
> conform to the JEDEC standard (JEP106) which allows for querying of 
> manufacturer/device ids so that they can be dynamically detected.  sounds to 
> me like "eeprom" is the correct interface for utilizing these devices.

Sounds to me as if you were talking about flash devices with a SPI bus
interface.

The original SPI eeprom support was implemented som 7+ years ago  for
the  Siemens  CCM  board;  this  is  where  the "CONFIG_SPI" stuff in
common/cmd_eeprom.c  comes  from,  and  this  was  supported  by  the
cpu/mpc8xx/spi.c SPI driver. Note that this was a real EEPROM device,
i.  e.  we just needed spi_read() and spi_write() functions to access
it. No erase, no sectors or any such stuff.

That was an EEPROM - what you have looks like a flash device to me.

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
In the pitiful, multipage, connection-boxed form to which  the  flow-
chart  has  today  been  elaborated, it has proved to be useless as a
design tool -- programmers draw flowcharts after, not before, writing
the programs they describe.                        - Fred Brooks, Jr.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24 11:13                 ` Wolfgang Denk
@ 2008-01-25 13:31                   ` Mike Frysinger
  0 siblings, 0 replies; 18+ messages in thread
From: Mike Frysinger @ 2008-01-25 13:31 UTC (permalink / raw)
  To: u-boot

On Thursday 24 January 2008, Wolfgang Denk wrote:
> If the device is SPI flash, then why not call it like that, i. e.
>
> 	spi flash info

i dont like this because of the integration it'd require with the generic spi 
command

> Or
> 	spiflash info
>
> to make it similar to the "nand" commands?

this sounds easy enough that i could probably do this ...

> On the other hand, it might be still too shortsighted to tie this  to
> the  bus  interface.  For example, what about dataflash? As discussed
> many times before, we have similar issues with dataflash devices.

i'm not sure i follow.  what do you mean by dataflash ?  the atmel 
dataflashes ?  the AT45DBxxx parts ?

> I don't have a good idea how to do it, but maybe this is a chance to
> get a good solution for NAND, dataflash, SPI flash and who knows what
> else?

i can see the motivation for this, but i'm not sure if it's worth the 
overhead ... SPI flash is more like parallel NOR flash than NAND.  there are 
no bad blocks, ecc, etc... with SPI flashes.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080125/35ea22d1/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-24 11:31             ` Wolfgang Denk
@ 2008-01-25 13:33               ` Mike Frysinger
  2008-01-25 15:09                 ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-25 13:33 UTC (permalink / raw)
  To: u-boot

On Thursday 24 January 2008, Wolfgang Denk wrote:
> In message <200801240413.04318.vapier@gentoo.org> you wrote:
> > > I think you are doing something wrong when you try to use "eeprom" to
> > > access "SPI flash" - these are differnt entities...
> >
> > let's make sure we're talking about the same thing.  SPI flashes are
> > eeproms that have a SPI interface.  so Spansion's S25FLxxxx, ST's
> > m25pxtmels AT45DBxxxx, Winbond's W25Xxx/W25Pxx, and such.  they need to
> > be erased before writing, are split up into some unit size, etc...  all
> > SPI flashes nowadays conform to the JEDEC standard (JEP106) which allows
> > for querying of manufacturer/device ids so that they can be dynamically
> > detected.  sounds to me like "eeprom" is the correct interface for
> > utilizing these devices.
>
> Sounds to me as if you were talking about flash devices with a SPI bus
> interface.
>
> The original SPI eeprom support was implemented som 7+ years ago  for
> the  Siemens  CCM  board;  this  is  where  the "CONFIG_SPI" stuff in
> common/cmd_eeprom.c  comes  from,  and  this  was  supported  by  the
> cpu/mpc8xx/spi.c SPI driver. Note that this was a real EEPROM device,
> i.  e.  we just needed spi_read() and spi_write() functions to access
> it. No erase, no sectors or any such stuff.
>
> That was an EEPROM - what you have looks like a flash device to me.

OK, thanks for the explanation.  this is much before my time ;).

at the moment, the Blackfin SPI flash driver hides all of the erasing/writing 
stuff for the user so that it integrates easily into the eeprom command.  i'm 
going to guess that you dont like the sound of that at all ;).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080125/e7fb6924/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-25 13:33               ` Mike Frysinger
@ 2008-01-25 15:09                 ` Wolfgang Denk
  2008-01-25 16:02                   ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2008-01-25 15:09 UTC (permalink / raw)
  To: u-boot

In message <200801250833.05605.vapier@gentoo.org> you wrote:
>
> at the moment, the Blackfin SPI flash driver hides all of the erasing/writing 
> stuff for the user so that it integrates easily into the eeprom command.  i'm 
> going to guess that you dont like the sound of that at all ;).

You know me pretty well.

Well, there is a big difference between NOR flash  and  your  device:
NOR  flash is memory, but your's is a storage device attached to some
bus interface (SPI here). The problem is that we don't  have  a  good
solution to interface with such devices yet.

Using the eeprom command for such devices is misleading, IMO. If  you
can  live  with the spiflash command, then this is not perfect, but I
guess it will have to do for now as we don't have a  better  solution
yet.

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
I have never understood the female capacity to avoid a direct  answer
to any question.
	-- Spock, "This Side of Paradise", stardate 3417.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-25 15:09                 ` Wolfgang Denk
@ 2008-01-25 16:02                   ` Mike Frysinger
  2008-01-25 16:54                     ` J. William Campbell
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2008-01-25 16:02 UTC (permalink / raw)
  To: u-boot

On Friday 25 January 2008, Wolfgang Denk wrote:
> Well, there is a big difference between NOR flash  and  your  device:
> NOR  flash is memory, but your's is a storage device attached to some
> bus interface (SPI here). The problem is that we don't  have  a  good
> solution to interface with such devices yet.
>
> Using the eeprom command for such devices is misleading, IMO. If  you
> can  live  with the spiflash command, then this is not perfect, but I
> guess it will have to do for now as we don't have a  better  solution
> yet.

so we're clear, the proposed interface is:
spiflash info
spiflash <read|write> address offset count
spiflash erase offset count

and once this comes together (i dont think it'll take long), we can consider a 
generic "flash" command.  i'm still not sure what it'll gain us (if 
anything) ... but i guess once we have more things to look at and compare, we 
can make a better decision
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080125/284d7690/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-25 16:02                   ` Mike Frysinger
@ 2008-01-25 16:54                     ` J. William Campbell
  2008-01-25 17:21                       ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: J. William Campbell @ 2008-01-25 16:54 UTC (permalink / raw)
  To: u-boot

Mike Frysinger wrote:
> On Friday 25 January 2008, Wolfgang Denk wrote:
>   
>> Well, there is a big difference between NOR flash  and  your  device:
>> NOR  flash is memory, but your's is a storage device attached to some
>> bus interface (SPI here). The problem is that we don't  have  a  good
>> solution to interface with such devices yet.
>>
>> Using the eeprom command for such devices is misleading, IMO. If  you
>> can  live  with the spiflash command, then this is not perfect, but I
>> guess it will have to do for now as we don't have a  better  solution
>> yet.
>>     
>
> so we're clear, the proposed interface is:
> spiflash info
> spiflash <read|write> address offset count
> spiflash erase offset count
>
>   
This sounds fine to me. The address is obviously a ram 
source/destination address. I vote for an offset in bytes and a count in 
bytes. If the offset is not an appropriate sector boundary, it is an 
error. The count can be a partial sector without problems. Having the 
offset and count in different units is confusing IMHO, so bytes for 
both. Also, Mike, I assume you will also add a module similar to 
env_eeprom.c to support environment in spiflash?

Best Regards,
Bill Campbell
> and once this comes together (i dont think it'll take long), we can consider a 
> generic "flash" command.  i'm still not sure what it'll gain us (if 
> anything) ... but i guess once we have more things to look at and compare, we 
> can make a better decision
> -mike
>   
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> ------------------------------------------------------------------------
>
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>   

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot-Users] [patch] add support for "eeprom info"
  2008-01-25 16:54                     ` J. William Campbell
@ 2008-01-25 17:21                       ` Mike Frysinger
  0 siblings, 0 replies; 18+ messages in thread
From: Mike Frysinger @ 2008-01-25 17:21 UTC (permalink / raw)
  To: u-boot

On Friday 25 January 2008, J. William Campbell wrote:
> Mike Frysinger wrote:
> > On Friday 25 January 2008, Wolfgang Denk wrote:
> >> Well, there is a big difference between NOR flash  and  your  device:
> >> NOR  flash is memory, but your's is a storage device attached to some
> >> bus interface (SPI here). The problem is that we don't  have  a  good
> >> solution to interface with such devices yet.
> >>
> >> Using the eeprom command for such devices is misleading, IMO. If  you
> >> can  live  with the spiflash command, then this is not perfect, but I
> >> guess it will have to do for now as we don't have a  better  solution
> >> yet.
> >
> > so we're clear, the proposed interface is:
> > spiflash info
> > spiflash <read|write> address offset count
> > spiflash erase offset count
>
> This sounds fine to me. The address is obviously a ram
> source/destination address. I vote for an offset in bytes and a count in
> bytes. If the offset is not an appropriate sector boundary, it is an
> error. The count can be a partial sector without problems. Having the
> offset and count in different units is confusing IMHO, so bytes for
> both.

it would be exactly the same as all other interfaces.  we dont hold the users 
hand by calculating pages for them in other places, so we wont here either.  
in other words, yes. ;)

> Also, Mike, I assume you will also add a module similar to 
> env_eeprom.c to support environment in spiflash?

yes
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080125/9352d0aa/attachment.pgp 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2008-01-25 17:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-23 16:13 [U-Boot-Users] [patch] add support for "eeprom info" Mike Frysinger
2008-01-23 21:06 ` Wolfgang Denk
2008-01-23 21:15   ` Mike Frysinger
2008-01-23 21:23     ` Wolfgang Denk
2008-01-23 22:29       ` Mike Frysinger
2008-01-24  0:44         ` Wolfgang Denk
2008-01-24  3:39           ` Mike Frysinger
2008-01-24  4:24             ` Ben Warren
2008-01-24  5:17               ` Mike Frysinger
2008-01-24 11:13                 ` Wolfgang Denk
2008-01-25 13:31                   ` Mike Frysinger
2008-01-24  9:13           ` Mike Frysinger
2008-01-24 11:31             ` Wolfgang Denk
2008-01-25 13:33               ` Mike Frysinger
2008-01-25 15:09                 ` Wolfgang Denk
2008-01-25 16:02                   ` Mike Frysinger
2008-01-25 16:54                     ` J. William Campbell
2008-01-25 17:21                       ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox