On Wed, Mar 30, 2011 at 8:16 PM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
Am 22.03.2011 01:22 schrieb Steven Zakulec:

On Tue, Oct 12, 2010 at 8:51 PM, Carl-Daniel Hailfinger wrote:
 
Hi Steven,

thanks a lot for this comprehensive list of flash chip voltages and form
factors..

On 13.10.2010 01:49, Steven Zakulec wrote:
   
Hello, some weeks ago I asked carldani what I could do to help the
flashrom project, and he mentioned that they could really use someone
to go thru all the chips in flashchips.c and get the voltage range and
form factor(s) for each chip listed there.

This is the result of that project: a spreadsheet with the chip name,
voltage range, form factors, connector type (pins, leads, etc).  This
covers all the chips as of v1143.
If anyone else is interested, I also have a short text file with all
of the form factor acronyms I came across and what they mean (I
grabbed the definition from the datasheet).

There's a few missing chips, and some values that I wasn't quite sure
about.  Comments would be greatly appreciated.

     
I thought I had seen some SPI chips which were available for disjoint
voltage ranges, e.g. 1.8-2.5V and 2.7-3.6V. Maybe the names were
different for the various versions with different voltages. I hope I'll
stumble over such a datasheet again.

OK, now the next question is how we can store this info in struct
flashchip. Suggestion:

struct voltage {
uint8_t minvoltage_decivolt;
uint8_t maxvoltage_decivolt;
};

Values would be in 1/10 volts. I haven't seen any flash chip datasheets
with greater precision, and with such an encoding even 12V can be
expressed.

For the form factor, I'd just use a large bitfield with one bit per
available form factor.
uint32_t formfactors;

#define PLCC32 (1<<  0)
#define TSOP32 (1<<  1)
#define TSOP40 (1<<  2)
...

Example:

       {
               .vendor         = "SST",
               .name           = "SST49LF008A",
               .bustype        = CHIP_BUSTYPE_FWH, /* A/A Mux */
               .manufacture_id = SST_ID,
               .model_id       = SST_SST49LF008A,
               .total_size     = 1024,
               .page_size      = 64 * 1024,
               .feature_bits   = FEATURE_REGISTERMAP |
FEATURE_EITHER_RESET,
               .tested         = TEST_OK_PRE,
               .probe          = probe_jedec,
               .probe_timing   = 1,        /* 150 ns */
               .block_erasers  =
               {
                       {
                               .eraseblocks = { {4 * 1024, 256} },
                               .block_erase = erase_sector_jedec,
                       }, {
                               .eraseblocks = { {64 * 1024, 16} },
                               .block_erase = erase_block_jedec,
                       }, {
                               .eraseblocks = { {1024 * 1024, 1} },
                               .block_erase = NULL, /* AA 55 80 AA 55 10,
only in A/A mux mode */
                       }
               },
               .printlock      = printlock_sst_fwhub,
               .unlock         = unlock_sst_fwhub,
               .write          = write_jedec_1,
               .read           = read_memmapped,
               .voltages       = { 30, 36 },
               .formfactors    = TSOP32|TSOP40|PLCC32,
         },

Regards,
Carl-Daniel

--
http://www.hailfinger.org/

I've included a patch (not working currently) that does pretty much what
   
you indicated above (just the voltage part).  It fails to compile correctly,
and has the struct called voltage instead of voltages, but should otherwise
be identical to what you proposed.  While filling in what I had, I did come
across some chips that offered separate ranges (usually 5 V +-10% and a 12V
fast erase or program).  I've listed those in comments next to the voltage
field.
 

Right. Please note that those chips usually can't handle 12V on data/address pins, and have a separate pin for supplying 12V for write/erase.

So it is correct to list them like other 5V chips, and add a comment about separate 12V programming voltage (please be precise if you describe when the chip needs 12V).  So far I've seen the following ways of using 12V:
- never/optional/always for erase
- never/optional/always for write
We could handle that either with feature bits or with separate voltage feature bits. Not sure. A comment will suffice for now until we can actually handle this.




The form-factor half is more complicated- in my original spreadsheet,
there's 29 form factors- I listed the pins/etc separate from the form
factor, so there are a very large amount of form factors consequently.
Ideas on how to handle this would be appreciated.
 

Can we postpone form factors and concentrate on voltages first?

Index: flash.h
===================================================================
--- flash.h     (revision 1282)
+++ flash.h     (working copy)
@@ -138,7 +138,12 @@
       int (*unlock) (struct flashchip *flash);
       int (*write) (struct flashchip *flash, uint8_t *buf, int start, int len);
       int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len);
+       struct voltage {

+               uint8_t minvoltage_decivolt;
+               uint8_t maxvoltage_decivolt;
+               };
 

Please remove one tab above so the } is below s of struct voltage.

By the way, I'd prefer

+       struct {
+               uint8_t min;
+               uint8_t max;
+       } voltage;


We do not use long variable names elsewhere. And yes, I know that I suggested this code some time ago. Since then I have learned a few things about the usefulness of short code. And please move the word "voltage" to the end of the struct declaration so it works.



+
 

No additional empty line.


       /* Some flash devices have an additional register space. */
       chipaddr virtual_memory;
       chipaddr virtual_registers;
 

Index: flashchips.c
===================================================================
--- flashchips.c        (revision 1282)
+++ flashchips.c        (working copy)
@@ -54,6 +54,7 @@
        * .unlock              = Chip unlock function
        * .write               = Chip write function
        * .read                = Chip read function
+        * .voltage             = Voltage min and max range
 

Voltage min and max range in decivolt


       */

       {
@@ -111,6 +113,7 @@

               },
               .write          = write_jedec_1,
               .read           = read_memmapped,
+               .voltage        = { 50 },
 

If this chip indeed has zero tolerance, please write {50, 50} explicitly.


       },

       {
  @@ -1283,6 +1313,7 @@
               .unlock         = spi_disable_blockprotect_at25df,
               .write          = spi_chip_write_256,
               .read           = spi_chip_read,
+               .voltage        = { 23, 36 }, /* Datasheet says 2.3-3.6V or 2.7-3.6V */
 

Another feature bit maybe? FEATURE_VOLTAGE_SUBRANGE or something like that. You can postpone this.


       },

       {
@@ -1320,6 +1351,7 @@
               .unlock         = spi_disable_blockprotect_at25df,
               .write          = spi_chip_write_256,
               .read           = spi_chip_read,
+               .voltage        = { 23, 36 }, /* Datasheet says 2.3-3.6V or 2.7-3.6V */
 

Stylistic choice: All other data in curly brackets has no space after { and no space before }.


       },

       {
@@ -1357,6 +1389,7 @@
               .unlock         = spi_disable_blockprotect_at25df,
               .write          = spi_chip_write_256,
               .read           = spi_chip_read,
+               .voltage        = { 17, 19 }, /* Datasheet says range is 1.65-1.95 V */
 

Maybe use 16,20 instead?



       },

       {
  @@ -2144,6 +2204,7 @@

               },
               .write          = write_jedec_1,
               .read           = read_memmapped,
+               .voltage        = { 5, 5 },
 

Really 0.5V or did you mean 5.0V here?


       },

       {
@@ -2175,6 +2236,7 @@

               },
               .write          = write_jedec_1,
               .read           = read_memmapped,
+               .voltage        = { 5, 5 },
 

Same.


       },

       {
@@ -2206,6 +2268,7 @@

               },
               .write          = write_jedec_1,
               .read           = read_memmapped,
+               .voltage        = { 5, 5 },
 

Same.


       },

       {
 

@@ -3460,6 +3560,7 @@
               },
               .write          = write_82802ab,
               .read           = read_memmapped,
+               .voltage        = { 114, 126 } /*Offers 5V read in addition, some chips offer 12V +-10% read/write */
 

Are you sure the chip is a pure 12V chip? Most "12V" chips I know are actually 5V chips and the 12V is only required for write/erase and sometimes special forms of ID.


       },

       {
@@ -3483,6 +3584,7 @@
               .unlock         = unlock_28f004s5,
               .write          = write_82802ab,
               .read           = read_memmapped,
+               .voltage        = { 5, 5 }, /*Also offers 12V write */
 

Please use the same phrasing as for the other chips:
"Also has 12V fast program"


       },

       {
 

Looks good otherwise. Some of my comments apply to multiple lines, but I picked only one line for each to keep the mail short.

Please resend this with the comments addressed and a Signed-off-by statement. Then we just need someone to cross-check the voltage values and we're good to go.


Regards,
Carl-Daniel

--
http://www.hailfinger.org/


I believe I've addressed all the comments.
Signed-off-by: Steven Zakulec <spzakulec@gmail.com>