AT49F010 patch is basically a copy of the existing AT49F020 code, but with half the size and the correct ID. The log was from a write of random data, after the chip already contained different random data. I did separate read, erase, write tests before but this log seems to show that PREW all work.
CAT28F512 I have successfully probed and read data from, but erase fails. Could be because it needs 12v VPP. The CAT28F512 I have is soldered on to a NIC. I haven't investigated if VPP is connected at all yet. Thought I'd include the patch as-is anyway.
Both patches: Signed-off-by: Andrew Morgan ziltro@ziltro.com
On Sat, 27 Aug 2011 04:28:28 +0100 Andrew Morgan ziltro@ziltro.com wrote:
AT49F010 patch is basically a copy of the existing AT49F020 code, but with half the size and the correct ID. The log was from a write of random data, after the chip already contained different random data. I did separate read, erase, write tests before but this log seems to show that PREW all work.
CAT28F512 I have successfully probed and read data from, but erase fails. Could be because it needs 12v VPP. The CAT28F512 I have is soldered on to a NIC. I haven't investigated if VPP is connected at all yet. Thought I'd include the patch as-is anyway.
Both patches: Signed-off-by: Andrew Morgan ziltro@ziltro.com
hello andrew and thanks for the patches!
btw patch_es_: please post one per mail in the future, because patchwork seems to be too dumb to recognize multiple patches in one mail (http://patchwork.coreboot.org/patch/3395/).
review of the AT49F010 patch follows:
Index: flashchips.c
--- flashchips.c (revision 1422) +++ flashchips.c (working copy) @@ -2218,6 +2218,30 @@
{ .vendor = "Atmel",
.name = "AT49F010",
.name = "AT49(H)F010", The H version is just a faster version (but with same VCC).
.bustype = BUS_PARALLEL,.manufacture_id = ATMEL_ID,.model_id = ATMEL_AT49F010,.total_size = 128,.page_size = 128,
should probably be 256 for now (semantics are different for each chip driver for parallel chips afaik and it does not matter for jedec routines iirc. NB: page_size is in bytes, so syncing it with total_size does not make sense.)...
.feature_bits = FEATURE_EITHER_RESET,.tested = TEST_OK_PREW,.probe = probe_jedec,.probe_timing = TIMING_ZERO, /* Datasheet has no timing info specified */.block_erasers ={{.eraseblocks = { {128 * 1024, 1} },.block_erase = erase_chip_block_jedec,}},.write = write_jedec_1,.read = read_memmapped,.voltage = {4500, 5500},- },
- {
.vendor = "Atmel",Index: flashchips.h
--- flashchips.h (revision 1422) +++ flashchips.h (working copy) @@ -181,6 +181,7 @@ #define ATMEL_AT45DB642 /* No ID available */ #define ATMEL_AT45DB642D 0x2800 #define ATMEL_AT49BV512 0x03 +#define ATMEL_AT49F010 0x17 /* Also AT49HF010 */
correct id, but the comment should be /* Same as AT49HF010 */ to be consistent.
#define ATMEL_AT49F020 0x0B #define ATMEL_AT49F002N 0x07 /* for AT49F002(N) */ #define ATMEL_AT49F002NT 0x08 /* for AT49F002(N)T */
this chip and also the 2 Mb and 4 Mb versions support a boot block protection that can be detected by software. it would be nice to add a .printlock function to do this and inform the user. i have seen the scheme before. Maybe there is already code in flashrom... but it is not that important. Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be appreciated. Nevertheless after addressing the in-line comments this patch is: Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
do you have commit rights?
On Wed, 31 Aug 2011 00:25:36 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
i have seen the scheme before. Maybe there is already code in flashrom... but it is not that important. Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be appreciated.
ahem. [PATCH] Add support for AT49F040 http://patchwork.coreboot.org/patch/3338/ explains that part ;) you could review and/or combine that patch with yours.
On Wed, 31 Aug 2011 00:35:53 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
On Wed, 31 Aug 2011 00:25:36 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
i have seen the scheme before. Maybe there is already code in flashrom... but it is not that important. Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be appreciated.
ahem. [PATCH] Add support for AT49F040 http://patchwork.coreboot.org/patch/3338/ explains that part ;) you could review and/or combine that patch with yours.
aaaand there is a 8 Mb version too; two actually (with top and bottom bootblocks respectively): 23H (AT49F080), 27H (AT49F080T).
PS: please repost the CAT patch as separate mail so that it wont get lost, but archived in patchwork.
On 30/08/11 23:25, Stefan Tauner wrote:
review of the AT49F010 patch follows:
Index: flashchips.c
--- flashchips.c (revision 1422) +++ flashchips.c (working copy) @@ -2218,6 +2218,30 @@
{ .vendor = "Atmel",
.name = "AT49F010",.name = "AT49(H)F010",The H version is just a faster version (but with same VCC).
Are you suggesting I change the name to include (H)? If so does AT49F020 have an H version too and should that be changed? If it wants to be changed what should flashchips.h have for the name?
.bustype = BUS_PARALLEL,.manufacture_id = ATMEL_ID,.model_id = ATMEL_AT49F010,.total_size = 128,.page_size = 128,should probably be 256 for now (semantics are different for each chip driver for parallel chips afaik and it does not matter for jedec routines iirc. NB: page_size is in bytes, so syncing it with total_size does not make sense.)...
Ok, changed. I didn't know where to find information on what to set page_size, feature_bits or probe_timing to.
this chip and also the 2 Mb and 4 Mb versions support a boot block protection that can be detected by software. it would be nice to add a .printlock function to do this and inform the user. i have seen the scheme before. Maybe there is already code in flashrom... but it is not that important.
I've looked at the patch mentioned in your later messages, and modified w39.c and chipdrivers.h, and added a .printlock = line to the AT49F010. The code compiled and ran and it told me the boot block lock was not active. This was just a test so I have left this out of the attached patch. I didn't do any tests to see if the printlock code affected read/write at all.
Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be appreciated. Nevertheless after addressing the in-line comments this patch is: Acked-by: Stefan Taunerstefan.tauner@student.tuwien.ac.at
do you have commit rights?
I do not. Updated patch attached. I hope I got all the changes. :)
If I get a chance and can find datasheets I'll have a go at adding the larger AT49F chips.
On 30/08/11 23:25, Stefan Tauner wrote:
btw patch_es_: please post one per mail in the future, because patchwork seems to be too dumb to recognize multiple patches in one mail (http://patchwork.coreboot.org/patch/3395/).
CAT28F512 patch on its own.
On Wed, 31 Aug 2011 01:52:42 +0100 Andrew Morgan ziltro@ziltro.com wrote:
On 30/08/11 23:25, Stefan Tauner wrote:
review of the AT49F010 patch follows:
Index: flashchips.c
--- flashchips.c (revision 1422) +++ flashchips.c (working copy) @@ -2218,6 +2218,30 @@
{ .vendor = "Atmel",
.name = "AT49F010",.name = "AT49(H)F010",The H version is just a faster version (but with same VCC).
Are you suggesting I change the name to include (H)?
yes. this is the way we designate chip variations in one entry. (x) means an optional part x in the name. x/y means alternative x or y.
look at the output of flashrom -L for some examples, a good one is AT49F002(N) and AT49F002(N)T. The N variants have a pin not connected and can be combined. but the T versions need their own entry due to the erase block layout.
If so does AT49F020 have an H version too and should that be changed?
i have not found one/a datasheet.... neither (for) a HF040 and HF080, so no.
If it wants to be changed what should flashchips.h have for the name?
this is handled by the comment. we usually take the name that matches the other names of a series. in this case without the H because the 010 is the exception.
.bustype = BUS_PARALLEL,.manufacture_id = ATMEL_ID,.model_id = ATMEL_AT49F010,.total_size = 128,.page_size = 128,should probably be 256 for now (semantics are different for each chip driver for parallel chips afaik and it does not matter for jedec routines iirc. NB: page_size is in bytes, so syncing it with total_size does not make sense.)...
Ok, changed. I didn't know where to find information on what to set page_size, feature_bits or probe_timing to.
jup that's all a bit of a mystery... in the feature bits various chip properties are saved. these are retrieved by programmer and chip drivers to change the behavior of generic functions in them e.g. getaddrmask and its users. page_size will be removed soon(tm). probe_timing... i am not sure myself. from what i have read in the code, some chips need some time to "startup" and be ready for more commands after probing. if in doubt leave them alone. the defaults are usually sane. in the at49f case one could add FEATURE_ADDR_FULL to the feature bits, but that's the default anyway (all 0s in the respective bits).
this chip and also the 2 Mb and 4 Mb versions support a boot block protection that can be detected by software. it would be nice to add a .printlock function to do this and inform the user. i have seen the scheme before. Maybe there is already code in flashrom... but it is not that important.
I've looked at the patch mentioned in your later messages, and modified w39.c and chipdrivers.h, and added a .printlock = line to the AT49F010. The code compiled and ran and it told me the boot block lock was not active. This was just a test so I have left this out of the attached patch. I didn't do any tests to see if the printlock code affected read/write at all.
of course it would be more interesting to see if it would detect if the lock is engaged, but the nature of the lock (afaics it is permanent? not that clear in the datasheets imho), it's not a good idea to tamper with it :) the printlock is just executed once when the chip is found in the probing method hence it should have almost no effect. i think it is safe and good to include it now. the only question is, if the location and name of the function is right (it is not.) and how to change it.
Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be appreciated. Nevertheless after addressing the in-line comments this patch is: Acked-by: Stefan Taunerstefan.tauner@student.tuwien.ac.at
do you have commit rights?
I do not. Updated patch attached. I hope I got all the changes. :)
apart from the (H) i explained above, i think it was only the 128 page size and the .h comment, which are in, thanks.
If I get a chance and can find datasheets I'll have a go at adding the larger AT49F chips.
http://www.alldatasheet.com/datasheet-pdf/pdf/56185/ATMEL/AT49F080.html
if you combine the patches please move the entries like i have done in the 040 patch, add yours in the right order and attach all signed-off-bys involved. if you don't want to do it, just say so and i will do it when i have time. thanks for your effort!
On Sat, Aug 27, 2011 at 04:28:28AM +0100, Andrew Morgan wrote:
CAT28F512 I have successfully probed and read data from, but erase fails. Could be because it needs 12v VPP. The CAT28F512 I have is soldered on to a NIC. I haven't investigated if VPP is connected at all yet. Thought I'd include the patch as-is anyway.
Both patches: Signed-off-by: Andrew Morgan ziltro@ziltro.com
Thanks, r1439 with some changes.
I changed "CATALYST" to "Catalyst", as used in most of their docs and websites, e.g.
http://www.catsemi.com http://www.onsemi.com/PowerSolutions/content.do?id=16700
(they've been bought)
I tested the patch on an Intel NIC with this chip too, probe and read works fine indeed.
Probe only works due to sheer luck though, probe_jedec() is not really correct here, it only works because:
"Signature Mode: The signature mode allows the user to identify the IC manufacturer and the type of device while the device resides in the target system. This mode can be activated in either of two ways; through the conventional method of applying a high voltage (12V) to address pin A9 or by sending an instruction to the command register (see Write Operations)."
I have verified on my NIC that VPP is indeed 12V always, hence probe works even though we don't supply the correct probe commands etc. (yet).
Index: flashchips.c
--- flashchips.c (revision 1421) +++ flashchips.c (working copy) @@ -2305,6 +2305,30 @@
[...]
.page_size = 256,
Killed, not used.
.feature_bits = FEATURE_EITHER_RESET,
Is now 0, don't think this feature applies (neither do the other ones AFAICS).
.probe = probe_jedec,
Added a TODO.
.block_erasers ={{.eraseblocks = { {64 * 1024, 1} },.block_erase = erase_chip_block_jedec,},},.write = write_jedec_1,
I set the write/erase functions to NULL for now, the functions above are incorrect and will not work. Let me know if you want to have a look at the datasheet and implement the correct ones. I might have some time sooner or later to do it, dunno.
Uwe.
On Wed, 31 Aug 2011 11:50:51 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
if you don't want to do it, just say so and i will do it when i have time. thanks for your effort!
hi andrew!
may i presume that you wont refine the patch? :)
On Thu, 9 Aug 2012 22:52:09 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
On Wed, 31 Aug 2011 11:50:51 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
if you don't want to do it, just say so and i will do it when i have time. thanks for your effort!
hi andrew!
may i presume that you wont refine the patch? :)
i have added some stuff and committed the patch in r1572. thanks!
On Wed, 31 Aug 2011 01:53:07 +0100 Andrew Morgan ziltro@ziltro.com wrote:
On 30/08/11 23:25, Stefan Tauner wrote:
btw patch_es_: please post one per mail in the future, because patchwork seems to be too dumb to recognize multiple patches in one mail (http://patchwork.coreboot.org/patch/3395/).
CAT28F512 patch on its own.
just to get this into patchwork at the appropriate location: this patch was refined and committed by uwe, see 20110913221148.GH17397@greenwood
-
Andrew Morgan -
Stefan Tauner -
Uwe Hermann