<br><br><div class="gmail_quote">On Sun, Apr 3, 2011 at 10:27 PM, Steven Zakulec <span dir="ltr"><<a href="mailto:spzakulec@gmail.com">spzakulec@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div></div><div class="h5"><br><br><div class="gmail_quote">On Wed, Mar 30, 2011 at 8:16 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net" target="_blank">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Am 22.03.2011 01:22 schrieb Steven Zakulec:<div><div></div><div><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Tue, Oct 12, 2010 at 8:51 PM, Carl-Daniel Hailfinger wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Steven,<br>
<br>
thanks a lot for this comprehensive list of flash chip voltages and form<br>
factors..<br>
<br>
On <a href="tel:13.10.2010" value="+13102010" target="_blank">13.10.2010</a> 01:49, Steven Zakulec wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello, some weeks ago I asked carldani what I could do to help the<br>
flashrom project, and he mentioned that they could really use someone<br>
to go thru all the chips in flashchips.c and get the voltage range and<br>
form factor(s) for each chip listed there.<br>
<br>
This is the result of that project: a spreadsheet with the chip name,<br>
voltage range, form factors, connector type (pins, leads, etc). This<br>
covers all the chips as of v1143.<br>
If anyone else is interested, I also have a short text file with all<br>
of the form factor acronyms I came across and what they mean (I<br>
grabbed the definition from the datasheet).<br>
<br>
There's a few missing chips, and some values that I wasn't quite sure<br>
about. Comments would be greatly appreciated.<br>
<br>
<br>
</blockquote>
I thought I had seen some SPI chips which were available for disjoint<br>
voltage ranges, e.g. 1.8-2.5V and 2.7-3.6V. Maybe the names were<br>
different for the various versions with different voltages. I hope I'll<br>
stumble over such a datasheet again.<br>
<br>
OK, now the next question is how we can store this info in struct<br>
flashchip. Suggestion:<br>
<br>
struct voltage {<br>
uint8_t minvoltage_decivolt;<br>
uint8_t maxvoltage_decivolt;<br>
};<br>
<br>
Values would be in 1/10 volts. I haven't seen any flash chip datasheets<br>
with greater precision, and with such an encoding even 12V can be<br>
expressed.<br>
<br>
For the form factor, I'd just use a large bitfield with one bit per<br>
available form factor.<br>
uint32_t formfactors;<br>
<br>
#define PLCC32 (1<< 0)<br>
#define TSOP32 (1<< 1)<br>
#define TSOP40 (1<< 2)<br>
...<br>
<br>
Example:<br>
<br>
{<br>
.vendor = "SST",<br>
.name = "SST49LF008A",<br>
.bustype = CHIP_BUSTYPE_FWH, /* A/A Mux */<br>
.manufacture_id = SST_ID,<br>
.model_id = SST_SST49LF008A,<br>
.total_size = 1024,<br>
.page_size = 64 * 1024,<br>
.feature_bits = FEATURE_REGISTERMAP |<br>
FEATURE_EITHER_RESET,<br>
.tested = TEST_OK_PRE,<br>
.probe = probe_jedec,<br>
.probe_timing = 1, /* 150 ns */<br>
.block_erasers =<br>
{<br>
{<br>
.eraseblocks = { {4 * 1024, 256} },<br>
.block_erase = erase_sector_jedec,<br>
}, {<br>
.eraseblocks = { {64 * 1024, 16} },<br>
.block_erase = erase_block_jedec,<br>
}, {<br>
.eraseblocks = { {1024 * 1024, 1} },<br>
.block_erase = NULL, /* AA 55 80 AA 55 10,<br>
only in A/A mux mode */<br>
}<br>
},<br>
.printlock = printlock_sst_fwhub,<br>
.unlock = unlock_sst_fwhub,<br>
.write = write_jedec_1,<br>
.read = read_memmapped,<br>
.voltages = { 30, 36 },<br>
.formfactors = TSOP32|TSOP40|PLCC32,<br>
},<br>
<br>
Regards,<br>
Carl-Daniel<br>
<br>
--<br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a><br>
<br>
I've included a patch (not working currently) that does pretty much what<br>
<br>
</blockquote>
you indicated above (just the voltage part). It fails to compile correctly,<br>
and has the struct called voltage instead of voltages, but should otherwise<br>
be identical to what you proposed. While filling in what I had, I did come<br>
across some chips that offered separate ranges (usually 5 V +-10% and a 12V<br>
fast erase or program). I've listed those in comments next to the voltage<br>
field.<br>
<br>
</blockquote>
<br></div></div>
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.<br>
<br>
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:<br>
- never/optional/always for erase<br>
- never/optional/always for write<br>
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.<div><br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The form-factor half is more complicated- in my original spreadsheet,<br>
there's 29 form factors- I listed the pins/etc separate from the form<br>
factor, so there are a very large amount of form factors consequently.<br>
Ideas on how to handle this would be appreciated.<br>
<br>
</blockquote>
<br></div>
Can we postpone form factors and concentrate on voltages first?<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Index: flash.h<br>
===================================================================<br>
--- flash.h (revision 1282)<br>
+++ flash.h (working copy)<br>
@@ -138,7 +138,12 @@<br>
int (*unlock) (struct flashchip *flash);<br>
int (*write) (struct flashchip *flash, uint8_t *buf, int start, int len);<br>
int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len);<br>
+ struct voltage {<div><br>
+ uint8_t minvoltage_decivolt;<br>
+ uint8_t maxvoltage_decivolt;<br></div>
+ };<br>
<br>
</blockquote>
<br>
Please remove one tab above so the } is below s of struct voltage.<br>
<br>
By the way, I'd prefer<br>
<br>
+ struct {<br>
+ uint8_t min;<br>
+ uint8_t max;<br>
+ } voltage;<br>
<br>
<br>
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.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
+<br>
<br>
</blockquote>
<br>
No additional empty line.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
/* Some flash devices have an additional register space. */<br>
chipaddr virtual_memory;<br>
chipaddr virtual_registers;<br>
<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Index: flashchips.c<br>
===================================================================<br>
--- flashchips.c (revision 1282)<br>
+++ flashchips.c (working copy)<br>
@@ -54,6 +54,7 @@<br>
* .unlock = Chip unlock function<br>
* .write = Chip write function<br>
* .read = Chip read function<br>
+ * .voltage = Voltage min and max range<br>
<br>
</blockquote>
<br>
Voltage min and max range in decivolt<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
*/<br>
<br>
{<br>
@@ -111,6 +113,7 @@<div><br>
},<br>
.write = write_jedec_1,<br>
.read = read_memmapped,<br></div>
+ .voltage = { 50 },<br>
<br>
</blockquote>
<br>
If this chip indeed has zero tolerance, please write {50, 50} explicitly.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -1283,6 +1313,7 @@<br>
.unlock = spi_disable_blockprotect_at25df,<br>
.write = spi_chip_write_256,<br>
.read = spi_chip_read,<br>
+ .voltage = { 23, 36 }, /* Datasheet says 2.3-3.6V or 2.7-3.6V */<br>
<br>
</blockquote>
<br>
Another feature bit maybe? FEATURE_VOLTAGE_SUBRANGE or something like that. You can postpone this.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -1320,6 +1351,7 @@<br>
.unlock = spi_disable_blockprotect_at25df,<br>
.write = spi_chip_write_256,<br>
.read = spi_chip_read,<br>
+ .voltage = { 23, 36 }, /* Datasheet says 2.3-3.6V or 2.7-3.6V */<br>
<br>
</blockquote>
<br>
Stylistic choice: All other data in curly brackets has no space after { and no space before }.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -1357,6 +1389,7 @@<br>
.unlock = spi_disable_blockprotect_at25df,<br>
.write = spi_chip_write_256,<br>
.read = spi_chip_read,<br>
+ .voltage = { 17, 19 }, /* Datasheet says range is 1.65-1.95 V */<br>
<br>
</blockquote>
<br>
Maybe use 16,20 instead?<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -2144,6 +2204,7 @@<div><br>
},<br>
.write = write_jedec_1,<br>
.read = read_memmapped,<br></div>
+ .voltage = { 5, 5 },<br>
<br>
</blockquote>
<br>
Really 0.5V or did you mean 5.0V here?<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -2175,6 +2236,7 @@<div><br>
},<br>
.write = write_jedec_1,<br>
.read = read_memmapped,<br></div>
+ .voltage = { 5, 5 },<br>
<br>
</blockquote>
<br>
Same.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -2206,6 +2268,7 @@<div><br>
},<br>
.write = write_jedec_1,<br>
.read = read_memmapped,<br></div>
+ .voltage = { 5, 5 },<br>
<br>
</blockquote>
<br>
Same.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
@@ -3460,6 +3560,7 @@<br>
},<br>
.write = write_82802ab,<br>
.read = read_memmapped,<br>
+ .voltage = { 114, 126 } /*Offers 5V read in addition, some chips offer 12V +-10% read/write */<br>
<br>
</blockquote>
<br>
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.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
@@ -3483,6 +3584,7 @@<br>
.unlock = unlock_28f004s5,<br>
.write = write_82802ab,<br>
.read = read_memmapped,<br>
+ .voltage = { 5, 5 }, /*Also offers 12V write */<br>
<br>
</blockquote>
<br>
Please use the same phrasing as for the other chips:<br>
"Also has 12V fast program"<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
},<br>
<br>
{<br>
<br>
</blockquote>
<br>
Looks good otherwise. Some of my comments apply to multiple lines, but I picked only one line for each to keep the mail short.<br>
<br>
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.<div><div></div><div><br>
<br>
Regards,<br>
Carl-Daniel<br>
<br>
-- <br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a><br>
<br>
</div></div></blockquote></div><br></div></div><div>I believe I've addressed all the comments.</div><div>Signed-off-by: Steven Zakulec <<a href="mailto:spzakulec@gmail.com" target="_blank">spzakulec@gmail.com</a>></div>
</blockquote><div><br>Hi, this is the latest version of the patch, taking into account Peter's request to me to change the struct to uint16_t and the values to milliwatts.<br>Signed-off-by: Steven Zakulec <<a href="mailto:spzakulec@gmail.com">spzakulec@gmail.com</a>><br>
</div></div><br>