Complete the addition of Feature Bits for all Jedec based chips. Add Zero entries for FWH, SPI and "Odd Jedec" (FEATURE_ADDR_555) chips. Add FEATURE_SHORT_RESET fix feature_bits to 0 for SST49LF040B
rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
On 23.01.2010 08:46, Sean Nelson wrote:
Complete the addition of Feature Bits for all Jedec based chips. Add Zero entries for FWH, SPI and "Odd Jedec" (FEATURE_ADDR_555) chips. Add FEATURE_SHORT_RESET fix feature_bits to 0 for SST49LF040B
rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
In general, I like it. Some comments, though.
#define FEATURE_REGISTERMAP (1 << 0) #define FEATURE_BYTEWRITES (1 << 1) +#define FEATURE_SHORT_RESET (1 << 4)
Can you please introduce FEATURE_EITHER_RESET as well for chips which support short and long reset? Feel free to define it either to (1<<4) or (0<<4), the point is just to keep this information around in case we ever switch to probe-once.
#define FEATURE_ADDR_FULL (0 << 2) #define FEATURE_ADDR_MASK (3 << 2) +#define FEATURE_ADDR_2AA (1 << 2) +#define FEATURE_ADDR_AAA (2 << 2) +#define FEATURE_ADDR_555 0
FEATURE_ADDR_555 should be FEATURE_ADDR_SHIFTED. Coincidentally, once you evaluate this bit, you can use the _common functions with a mask+shift parameter you posted earlier and which were rejected as too complicated for a first step. Now we're doing the second step and for that mask+shift is entirely appropriate. FEATURE_ADDR_SHIFTED would set shift=1, and all others would set shift=0.
+#define FEATURE_ADDR_FWH 0 +#define FEATURE_ADDR_SPI 0
I don't really get the point of those.
Regards, Carl-Daniel
Complete the addition of Feature Bits for all Jedec based chips. Add Zero entries for FWH, SPI and "Odd Jedec" (FEATURE_ADDR_SHIFTED) chips. Add FEATURE_SHORT_RESET fix feature_bits to 0 for SST49LF040B
rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Updated patch with suggestions and fixes from carldani and uwe.
Signed-off-by: Sean Nelson audiohacked@gmail.com
On 23.01.2010 23:56, Sean Nelson wrote:
Complete the addition of Feature Bits for all Jedec based chips.
Add Zero entries for FWH, SPI and "Odd Jedec" (FEATURE_ADDR_SHIFTED) chips.
I think that part of the changelog can be killed.
Add FEATURE_SHORT_RESET fix feature_bits to 0 for SST49LF040B
rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Very nice. I'd like to keep these files around until we have the locking conversion/refactoring done. My patch to do this probably doesn't apply anymore, and it also can't handle partial locking/unlocking. I can repost next week, but I welcome comments about the interface. One of my new ideas is to have the locking function take an struct lockblock {blocksize, lockstatus}array as parameter. To retrieve the locking status, one would pass action=get array=NULL and the function would allocate and return the lockblock array. To set the locking status, one would first retrieve the array (see previous sentence), then walk it and set the desired status of each lockblock, then pass action=set and the modified array to the locking function. Advantages: You can do lock printing in a generic function which just walks the returned array, you can handle enabling and disabling all locks in the same way. Even something like (un)locking only a specific region can be done with a generic wrapper. Unsolved (well, design is not completely ready): Where should we store lock blocks? In struct flashchip (works mostly OK for LPC/FWH chips because locking status is usually stored in register space at the corresponding block address, will be a nightmare for SPI chips because there are usually just 4 bits in the status reg for this, and undecided for Parallel chips because they have the status reg variant and the corresponding block address variant). In the chip drivers (would avoid cluttering flashchips.c, handle the various locking encodings (bitfield, corresponding address in register space)
Updated patch with suggestions and fixes from carldani and uwe.
Signed-off-by: Sean Nelson audiohacked@gmail.com
Close, but one more iteration please. You changed the write function for a lot of chips, but kept .tested=TEST_OK_PRW instead of changing it to TEST_OK_PR. Yes, I do realize that this means our test matrix will look a lot worse. Unfortunately, we can't do better if we want to stay honest.
--- a/jedec.c +++ b/jedec.c @@ -23,14 +23,15 @@ */
#include "flash.h"
#define MAX_REFLASH_TRIES 0x10 #define MASK_FULL 0xffff #define MASK_2AA 0x7ff +#define MASK_AAA 0xfff
Umm. Does this mean we have 555/2AA/555 and 555/AAA/555 chips?
int write_jedec(struct flashchip *flash, uint8_t *buf) {
- int mask;
- mask = getaddrmask(flash); int i, failed = 0; int total_size = flash->total_size * 1024; int page_size = flash->page_size;
I think this will cause compilation warnings/errors for some compilers because variable declarations and assignments are mixed freely. Can you reorder (move mask=getaddrmask after the declaration block)?
And one last comment: FEATURE_EITHER_RESET is not set for any chip, but a few chips now have FEATURE_SHORT_RESET. I think at least one of them should have FEATURE_EITHER_RESET instead. I would also introduce FEATURE_LONG_RESET (as alias to FEATURE_EITHER_RESET) and fill it in for all chips because you're going through the datasheets anyway.
Yes, I should have mentioned that in my first review, but I just had a few seconds to look over the patch and need to continue with finals preparation.
Note: I did _not_ verify the changes in flashchips.c due to time constraints, but the rest of the code and design looks solid and should work fine if the review is addressed.
Regards, Carl-Daniel
P.S. I need to write shorter sentences and especially, considering the amount of reading you, my dear fellow developers, have to keep up with, the nesting of sentences (and of course the inordinate number of parentheses (which sort of serve as subsitute for even more commas)). :-P
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
---
On 1/23/2010 6:28 PM, Carl-Daniel Hailfinger wrote:
--- a/jedec.c +++ b/jedec.c @@ -23,14 +23,15 @@ */
#include "flash.h"
#define MAX_REFLASH_TRIES 0x10 #define MASK_FULL 0xffff #define MASK_2AA 0x7ff +#define MASK_AAA 0xfff
Umm. Does this mean we have 555/2AA/555 and 555/AAA/555 chips?
Yes, the ST M29F002T/NT/B and the Eon EN29F002A(N) uses the 555/AAA/555 address order scheme. There is one chipdriver that uses a AAA/555/AAA order scheme which is why some chips has FEATURE_ADDR_555 set for them.
We can delete the unused chip-drivers when we are sure that everything we can do generically can be done.
P.S. Oddly enough I can follow everything Carl-Daniel is talking about in his emails.
On 24.01.2010 05:32, Sean Nelson wrote:
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask [...] Signed-off-by: Sean Nelson audiohacked@gmail.com
On 1/23/2010 6:28 PM, Carl-Daniel Hailfinger wrote:
Umm. Does this mean we have 555/2AA/555 and 555/AAA/555 chips?
Yes, the ST M29F002T/NT/B and the Eon EN29F002A(N) uses the 555/AAA/555 address order scheme. There is one chipdriver that uses a AAA/555/AAA order scheme which is why some chips has FEATURE_ADDR_555 set for them.
Good.
As I wrote before, I didn't have time to check the changed entries in flashchips.c, but a random sampling turned up some reset sequence bugs, so maybe there are more of them:
{ .vendor = "SST", .name = "SST39SF010A", .bustype = CHIP_BUSTYPE_PARALLEL, .manufacture_id = SST_ID, .model_id = SST_39SF010, .total_size = 128, .page_size = 4096,
.feature_bits = FEATURE_LONG_RESET,
Should be FEATURE_EITHER_RESET according to my datasheet.
{ .vendor = "SST", .name = "SST39SF020A", .bustype = CHIP_BUSTYPE_PARALLEL, .manufacture_id = SST_ID, .model_id = SST_39SF020, .total_size = 256, .page_size = 4096,
.feature_bits = FEATURE_LONG_RESET,
Should be FEATURE_EITHER_RESET according to my datasheet.
{ .vendor = "SST", .name = "SST39SF040", .bustype = CHIP_BUSTYPE_PARALLEL, .manufacture_id = SST_ID, .model_id = SST_39SF040, .total_size = 512, .page_size = 4096,
.feature_bits = FEATURE_LONG_RESET,
Should be FEATURE_EITHER_RESET according to my datasheet.
SST39SF010A / SST39SF020A / SST39SF040 all support both reset sequences according to this datasheet: http://www.sst.com/dotAsset/40746.pdf
Software ID Exit6 XXH F0H Software ID Exit6 5555H AAH 2AAAH 55H 5555H F0H 6. Both Software ID Exit operations are equivalent
We can delete the unused chip-drivers when we are sure that everything we can do generically can be done.
Yes.
For all stuff outside flashchips.c (I assume you tested compilation): Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
For flashchips.c I'd like someone to cross-check your write_jedec_1/write_jedec and FEATURE_*_RESET values with datasheets. I think macavity or someone else on IRC may be very interested in doing this because it is a good way to get familiar with datasheets.
Please don't get me wrong, the work you're doing is very valuable. It's just that the changes are so big that I wouldn't trust myself to send a 100% correct patch either.
Regards, Carl-Daniel
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
---
Updated, flashchips.c should have correct FEATURE_*_RESET now.
On 25.01.2010 18:29, Sean Nelson wrote:
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask
Signed-off-by: Sean Nelson audiohacked@gmail.com
--- a/flashchips.c +++ b/flashchips.c @@ -58,40 +58,42 @@ struct flashchip flashchips[] = { .vendor = "AMD", .name = "Am29F010A/B", .bustype = CHIP_BUSTYPE_PARALLEL, .manufacture_id = AMD_ID, .model_id = AM_29F010B, /* Same as Am29F010A */ .total_size = 128, .page_size = 16 * 1024,
.tested = TEST_OK_PRW,.probe = probe_29f040b,
.feature_bits = FEATURE_ADDR_2AA|FEATURE_EITHER_RESET,.tested = TEST_OK_PR,.probe = probe_jedec,
Ummm... technically, the probe function changed (well, in practice the command sequence should be identical). Do we want to keep TEST_OK_PR if the probe function changed?
Regards, Carl-Daniel
On 1/25/2010 9:40 AM, Carl-Daniel Hailfinger wrote:
Ummm... technically, the probe function changed (well, in practice the command sequence should be identical). Do we want to keep TEST_OK_PR if the probe function changed?
Regards, Carl-Daniel
On 1/24/2010 1:54 PM, Stefan Reinauer wrote:
I guess that was done wrong before, but all chips touched like that need to be changed back to untested unless you retested each of them. Otherwise the test flags become quite meaningless...
Stefan
I'm thinking that Stefan is right: we should change all the chips to TEST_UNTESTED since the last few patches I've written have had radical changes. This latest version of the patch reflects that.
On 1/25/2010 10:46 AM, Sean Nelson wrote:
On 1/25/2010 9:40 AM, Carl-Daniel Hailfinger wrote:
Ummm... technically, the probe function changed (well, in practice the command sequence should be identical). Do we want to keep TEST_OK_PR if the probe function changed?
Regards, Carl-Daniel
On 1/24/2010 1:54 PM, Stefan Reinauer wrote:
I guess that was done wrong before, but all chips touched like that need to be changed back to untested unless you retested each of them. Otherwise the test flags become quite meaningless...
Stefan
I'm thinking that Stefan is right: we should change all the chips to TEST_UNTESTED since the last few patches I've written have had radical changes. This latest version of the patch reflects that.
Carl asked me to double check as many as i could. Sorry for the somewhat funny notation, but it helped me keep track of where i was. There are only a few not mentioned.. i worked that out with Sean on IRC.
Also, major sorry for the delay.. this was a little tougher than i expected.
The features quoted are the features i have checked. Comments are above. Legend: (C) = Checks out (W) = Warning, sheet says otherwise. (U) = Unsure, please double check and comment. (N) = No datasheet available. (S) = Skipped in this review because $REASON
[AMD Section] (W)(U) I can't find evidence of anything other than write_jedec_1 in the sheet. Please verify. .name = "Am29F010A/B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_EITHER_RESET, .write = write_jedec,
(W)(U) Uhm, the N versions do not support reset at all, but I don't think we can tell them apart with out a runtime test. Also they are ADDR_2AA I think. .name = "Am29F002(N)BB" / "Am29F002(N)BT" .feature_bits = FEATURE_SHORT_RESET, .write = write_jedec_1,
(C) No comments. .name = "Am29F016D", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "Am29F040B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "Am29F080B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "Am29LV040B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
(C) Strictly speaking it doesn't need ADDR_2AA as it has "Don't care" for all addresses (except the last in ID detection) [weird freakin' chip!, red] .name = "Am29LV081B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
[ASD Section] (N) Also has strange comments in flashchips.h .name = "AE49F2008",
[Atmel Section] (C) No comments. .name = "AT29C512", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec,
.name = "AT29C010A", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec,
.name = "AT29C020", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec,
.name = "AT29C040A", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec,
.name = "AT49BV512", .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
(W) N-version does not support reset. .name = "AT49F002(N)" / "AT49F002(N)T" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
[AMIC Section] (C) No remarks. .name = "A29002B" / "A29002T" .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "A29040B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
(C)(U) This one should do write_jedec_1 if that function understands registermap. .name = "A49LF040A", .feature_bits = FEATURE_REGISTERMAP|FEATURE_EITHER_RESET, .write = write_49fl00x,
[EMST Section] (C) No comments. .name = "F49B002UA", .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
[EON Section] (W) Oops, ADDR_555 only. .name = "EN29F002(A)(N)B" / "EN29F002(A)(N)T" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
[Fujitsu Section] (C)(U) *Looks like* it supports write_jedec_1. .name = "MBM29F004BC" / "MBM29F004TC" .feature_bits = FEATURE_ADDR_2AA|FEATURE_EITHER_RESET, .write = NULL,
(U) I have no idea... /* FIXME: this has WORD/BYTE sequences; 2AA for word, 555 for byte */ .name = "MBM29F400BC" / "MBM29F400TC" .feature_bits = FEATURE_ADDR_SHIFTED|FEATURE_EITHER_RESET, .write = write_coreboot_m29f400bt,
[Intel Section] (S) These datasheets are scary. .name = "28F001BX-B" / "28F001BX-T" / "82802AB"
[Macronix Section] (C) These check out: .name = "MX29F001B" / "MX29F001T" .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "MX29F002B" / "MX29F002T" .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
.name = "MX29LV040", .feature_bits = FEATURE_ADDR_2AA|FEATURE_SHORT_RESET, .write = write_jedec_1,
[PMC Section] (W)(U) Looks like write_jedec_1 only. .name = "Pm29F002T" / "Pm29F002B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_EITHER_RESET, .write = write_jedec,
(W) Is ADDR_2AA .name = "Pm39LV010", .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
(W)(U) This looks like plain write_jedec_1 to me .name = "Pm49FL002" / "Pm49FL004" .feature_bits = FEATURE_REGISTERMAP|FEATURE_EITHER_RESET, .write = write_49fl00x
[Sharp Section] (S) Can't read that sheet. Silly people... .name = "LHF00L04",
[SST Section] (W)(U) Can only find short reset in sheet. .name = "SST28SF040A", .feature_bits = FEATURE_EITHER_RESET,
(C) No comment. .name = "SST29EE010" / "SST29LE010", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec
.name = "SST29EE020A" / "SST29LE020", .feature_bits = FEATURE_LONG_RESET, .write = write_jedec
.name = "SST39SF512" / "SST39SF010A" / "SST39SF020A" / "SST39SF040" "SST39VF512" / "SST39VF010" / "SST39VF020" / "SST39VF040" "SST39VF080" / .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
.name = "SST49LF002A/B" / "SST49LF003A/B" / "SST49LF004A/B" "SST49LF008A" .feature_bits = FEATURE_REGISTERMAP|FEATURE_EITHER_RESET
(W) This one is REGISTERMAP (ds page 22) and SHORT_RESET (ds page 14) .name = "SST49LF004C" / "SST49LF008C" .feature_bits = 0,
(W) This one is REGISTERMAP (ds page 19) and SHORT_RESET (ds page 17) .name = "SST49LF016C", .feature_bits = 0,
(W) REGISTERMAP and (ds page 23) and SHORT_RESET (ds page 16) .name = "SST49LF160C", .feature_bits = 0,
(C) No comments. .name = "SST49LF020" / "SST49LF040" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
.name = "SST49LF020A" / "SST49LF080A" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
.name = "SST49LF040B", .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
[ST Section] (U) Generally i have a hard time with ST and write_jedec vs write_jedec_1, so someone else should look over this... (C) Checks out. .name = "M29F002B", .feature_bits = FEATURE_ADDR_AAA|FEATURE_EITHER_RESET, .write = write_jedec,
(W) And once again the N version does not support reset. .name = "M29F002T/NT", .feature_bits = FEATURE_ADDR_AAA|FEATURE_EITHER_RESET, .write = write_jedec,
.name = "M29F040B", .feature_bits = FEATURE_ADDR_2AA|FEATURE_EITHER_RESET, .write = write_jedec_1,
(C) No comment /* FIXME: this has WORD/BYTE sequences; 2AA for word, 555 for byte */ .name = "M29F400BT", .feature_bits = FEATURE_ADDR_SHIFTED|FEATURE_EITHER_RESET,
(W) Is ADDR_2AA .name = "M29W010B" / "M29W040B" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec,
[SyncMOS Section] (C) Home run baby. .name = "S29C31004T" / "S29C51001T" / "S29C51002T" / "S29C51004T" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
[TI Section] (W) Looks ADDR_2AA to me. .name = "TMS29F002RB" / "TMS29F002RT" .feature_bits = FEATURE_EITHER_RESET,
[Winbond Section] (C) No remarks. .name = "W29C011" / "W29C020C" / "W29C040P" / "W29EE011" .feature_bits = FEATURE_LONG_RESET, .write = write_jedec,
.name = "W39V040A" / "W39V040B" / "W39V040C" / "W39V040FA" "W39V080A" / "W49F002U" / "W49V002A" / "W49V002FA" .feature_bits = FEATURE_EITHER_RESET, .write = write_jedec_1,
(W) Is EITHER_RESET (ds page 11) .name = "W39V080FA" / "W39V080FA (dual mode)" .feature_bits = FEATURE_REGISTERMAP|FEATURE_LONG_RESET,
Have a nice day! /Anders
On 28.01.2010 23:58, Anders Juel Jensen wrote:
Carl asked me to double check as many as i could. Sorry for the somewhat funny notation, but it helped me keep track of where i was. There are only a few not mentioned.. i worked that out with Sean on IRC.
Also, major sorry for the delay.. this was a little tougher than i expected.
Awesome review, thank you!
Regards, Carl-Daniel
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
--- Final version of patch
Thanks to Carl-Daniel for: For all stuff outside flashchips.c (I assume you tested compilation): Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks to Anders Juel Jensen for double-checking the flashchips.c (waiting for his Ack, before commit).
Acked-by: Anders Juel Jensen andersjjensen@gmail.com
-- I only have one thing on it, and that is just me being totally anal about it. I think it should be:
#define FEATURE_LONG_RESET (0 << 4) #define FEATURE_SHORT_RESET (1 << 4) #define FEATURE_EITHER_RESET FEATURE_LONG_RESET
As that is what the code really does... but yes, totally cosmetic/pedantic. Maybe i am tired..
/Anders
On Friday 29 January 2010 00:24:45 Sean Nelson wrote:
Complete the addition of Feature Bits for all Jedec based chips. Add FEATURE_SHORT_RESET, FEATURE_LONG_RESET, and FEATURE_EITHER_RESET rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Signed-off-by: Sean Nelson audiohacked@gmail.com
Final version of patch
Thanks to Carl-Daniel for: For all stuff outside flashchips.c (I assume you tested compilation): Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks to Anders Juel Jensen for double-checking the flashchips.c (waiting for his Ack, before commit).
Thank you, Committed in r886.
I guess that was done wrong before, but all chips touched like that need to be changed back to untested unless you retested each of them. Otherwise the test flags become quite meaningless...
Stefan
On 23.01.2010, at 23:56, Sean Nelson audiohacked@gmail.com wrote:
Complete the addition of Feature Bits for all Jedec based chips. Add Zero entries for FWH, SPI and "Odd Jedec" (FEATURE_ADDR_SHIFTED) chips. Add FEATURE_SHORT_RESET fix feature_bits to 0 for SST49LF040B
rewrite jedec functions to use getaddrmask
convert write_49f002 to write_jedec_1 convert write_w39v040c to write_jedec_1 convert probe_w39v040c to probe_jedec convert write_49lf040 to write_jedec_1 convert write_pm29f002 to write_jedec convert write_29f040b to write_jedec_1 convert probe_29f040b to probe_jedec convert erase_chip_29f040b to erase_chip_block_jedec convert erase_sector_29f040b to erase_sector_jedec convert write_m29f002b to write_jedec convert write_m29f002t to write_jedec convert *_29f002 to *_jedec
decouple unused files from Makefile: am29f040b.c en29f002a.c m29f002.c mx29f002.c pm29f002.c sst49lf040.c w39v040c.c w49f002u.c
Updated patch with suggestions and fixes from carldani and uwe.
Signed-off-by: Sean Nelson audiohacked@gmail.com
<feature_bits.diff> _______________________________________________ flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
-
Anders Juel Jensen -
Carl-Daniel Hailfinger -
Sean Nelson -
Stefan Reinauer