[PATCH 0/7] Hello, Support for Atmel's AT25F series and some small stuff

[Stefan Tauner]

Hello!

as some of you may already know, i will probably apply for a flashrom
(or coreboot) project for this year's gsoc.

i am studying computer science or more specifically computer
engineering at the technical university in vienna (austria) since 2005
and will hopefully graduate this year :)

i have contributed smaller pieces to different oss projects and i am
doing regular bug reporting from kernel specific issues to daily
annoyances in ubuntu, but i have never been engaged in the cooperative
development of a bigger software project for a longer period.

please see below for the shortlog of the incoming patches.
a forked git tree including all this can be found at
https://github.com/stefanct/flashrom

i hang around in irc in case there are questions or flames you don't
wanna see on the list :)

Stefan Tauner (7):
  whitespace, documentation and other small stuff
  The AT25F512B is quite different from the other (yet unsupported)
    chips in the     AT25F* familiy, so rename 512B-specific stuff.
  generify spi_rdid to allow passing different opcodes. This is needed
    to add support     for Atmel's AT25F series.
  Add support for Atmel's AT25F series of SPI flash chips.     This
    includes a new probing method (probe_spi_rdid_at25f), block erase
    method     (spi_block_erase_62),
    spi_prettyprint_status_register_at25f and    
    spi_disable_blockprotect_at25f.
  add support for 8086:1076 (82541GI) to nicintel_spi.c
  use getpagesize() to determine physmap's length in nicintel_spi.c
  check if write enable is really set in nicintel_spi_init (and minor
    comment changes).

chipdrivers.h    |    3 +
 chipset_enable.c |    4 +-
 drkaiser.c       |    2 +-
 flash.h          |    7 ++-
 flashchips.c     |   89 +++++++++++++++++++++++++++
 flashchips.h     |   14 ++---
 flashrom.c       |    4 +-
 nicintel_spi.c   |   23 ++++++-
 print.c          |    2 +-
 print_wiki.c     |    2 +-
 spi.h            |   14 +++-
 spi25.c          |  177
 ++++++++++++++++++++++++++++++++++++++++++++++------- wbsio_spi.c
 |    2 +- 13 files changed, 294 insertions(+), 49 deletions(-)

[Paul Menzel]

Dear Stefan,

Am Montag, den 14.03.2011, 21:40 +0100 schrieb Stefan Tauner:

[…]
> please see below for the shortlog of the incoming patches.
> a forked git tree including all this can be found at
> https://github.com/stefanct/flashrom
>
> i hang around in irc in case there are questions or flames you don't
> wanna see on the list :)

thank you for your patches.
> Stefan Tauner (7):
>   whitespace, documentation and other small stuff
>   The AT25F512B is quite different from the other (yet unsupported)
>     chips in the     AT25F* familiy, so rename 512B-specific stuff.
>   generify spi_rdid to allow passing different opcodes. This is needed
>     to add support     for Atmel's AT25F series.
>   Add support for Atmel's AT25F series of SPI flash chips.     This
>     includes a new probing method (probe_spi_rdid_at25f), block erase
>     method     (spi_block_erase_62),
>     spi_prettyprint_status_register_at25f and    
>     spi_disable_blockprotect_at25f.
>   add support for 8086:1076 (82541GI) to nicintel_spi.c
>   use getpagesize() to determine physmap's length in nicintel_spi.c
>   check if write enable is really set in nicintel_spi_init (and minor
>     comment changes).
>
> chipdrivers.h    |    3 +
>  chipset_enable.c |    4 +-
>  drkaiser.c       |    2 +-
>  flash.h          |    7 ++-
>  flashchips.c     |   89 +++++++++++++++++++++++++++
>  flashchips.h     |   14 ++---
>  flashrom.c       |    4 +-
>  nicintel_spi.c   |   23 ++++++-
>  print.c          |    2 +-
>  print_wiki.c     |    2 +-
>  spi.h            |   14 +++-
>  spi25.c          |  177
>  ++++++++++++++++++++++++++++++++++++++++++++++------- wbsio_spi.c
>  |    2 +- 13 files changed, 294 insertions(+), 49 deletions(-)

I think your MUA mangled the patches, i. e. added line breaks. Please
resend either by turning preformatting on or by using `git send-email`.

(I know you have a Git tree published, but (unfortunately ;-)) not all
people are using Git.)

Thanks,

Paul

[Stefan Tauner]

On Mon, 14 Mar 2011 22:19:01 +0100
Paul Menzel  wrote:
> I think your MUA mangled the patches, i. e. added line breaks. Please
> resend either by turning preformatting on or by using `git
> send-email`.

sorry for that. one should not try something new in such situations.
so... lets try something new and use git send-email... :)

[Peter Lemenkov]

2011/3/15 Paul Menzel :
> (but (unfortunately ;-)) not all
> people are using Git.)

Fortunately that's the only issue in flashrom :)

[Carl-Daniel Hailfinger]

Hi Stefan,

thanks for your patches!

Auf 14.03.2011 21:40, Stefan Tauner schrieb:
> Stefan Tauner (7):
>    whitespace, documentation and other small stuff
>    The AT25F512B is quite different from the other (yet unsupported)
>      chips in the     AT25F* familiy, so rename 512B-specific stuff.
>    generify spi_rdid to allow passing different opcodes. This is needed
>      to add support     for Atmel's AT25F series.
>    Add support for Atmel's AT25F series of SPI flash chips.     This
>      includes a new probing method (probe_spi_rdid_at25f), block erase
>      method     (spi_block_erase_62),
>      spi_prettyprint_status_register_at25f and
>      spi_disable_blockprotect_at25f.
>    add support for 8086:1076 (82541GI) to nicintel_spi.c
>    use getpagesize() to determine physmap's length in nicintel_spi.c
>    check if write enable is really set in nicintel_spi_init (and minor
>      comment changes).

I will review the patches individually, but a small comment beforehand:
There are some pending patches for AT25* already, and we should make 
sure to avoid conflicts between those patches and your patches. I'll dig 
up those AT25* patches again and repost them for review.
Given that you looked at AT25* in detail, maybe you can review those 
older AT25 changes?

Regards,
Carl-Daniel

W29EE011-15 (was: Hello, Support for Atmel's AT25F series and some small stuff)

[Stefan Tauner]

On Mon, 14 Mar 2011 22:29:56 +0100
Carl-Daniel Hailfinger  wrote:
> I will review the patches individually, but a small comment
> beforehand: There are some pending patches for AT25* already, and we
> should make sure to avoid conflicts between those patches and your
> patches. I'll dig up those AT25* patches again and repost them for
> review. Given that you looked at AT25* in detail, maybe you can
> review those older AT25 changes?

i have only looked at AT25F*, but yes sure i can...

i started to test my old bios/etherboot chips. sadly i had to notice
that the 3com nics i gathered for this (already marked to be ok) only
support 128kB and most of the chips are 256kB. btw this would be
something i'd like to be reported in the wiki list.
anyway... there is one 128kB chip and this might be quite interesting
to play with... because there seem to be some serious wtf in the
codebase. at least i dont understand it.
the chip is a W29EE011-15.
it uses the WINBOND_W29C010 define and there are two elements in the
flashchips.c table that use them, both with the name
"W29C010(M)/W29C011A/W29EE011/W29EE012". they only differ in their
probing function which was modified because of problems with A49LF040A
(http://www.mail-archive.com/[e-mail]/msg02644.html).

i did not dig into the revision history but it seems someone missed
some side effects when merging the chips or so (after r1091:
http://www.flashrom.org/pipermail/flashrom/2010-August/004345.html).

executing ./flashrom -V -p nic3com -c
"W29C010(M)/W29C011A/W29EE011/W29EE012" results in:
...
Initializing nic3com programmer
Found "3COM 3C90xB: PCI 10/100 Mbps; shared 10BASE-T/100BASE-TX" (10b7:9055, BDF 01:00.0).
Requested BAR is I/O
Probing for Winbond W29C010(M)/W29C011A/W29EE011/W29EE012, 128 kB: probe_jedec_common: id1 0xda, id2 0xc1
Found chip "Winbond W29C010(M)/W29C011A/W29EE011/W29EE012" (128 kB, Parallel) on nic3com.
Probing for Winbond W29C010(M)/W29C011A/W29EE011/W29EE012, 128 kB: Probing disabled for Winbond W29EE011 because the probing sequence puts the AMIC A49LF040A in a funky state. Use 'flashrom -c W29EE011' if you have a board with this chip.
...

the message suggesting 'flashrom -c W29EE011' is not even correct
since this would fail with:
Error: Unknown chip 'W29EE011' specified.

if no one can shed some light on this, i would suggest that we
remove the alternative probing because jedec works for me (tested
PR only yet).

W29EE011-15 (was: Hello, Support for Atmel's AT25F series and some small stuff)

[Stefan Tauner]

even more puzzling (at least for me atm) are these erase verify failures...
./flashrom -V -p nic3com -c "W29C010(M)/W29C011A/W29EE011/W29EE012" -w ../testimages/128kB.rand.img
...
Reading old flash chip contents...
Erasing and writing flash chip... Looking at blockwise erase function 0... trying... 0x000000-0x01ffff:EWVERIFY FAILED at 0x00008400! Expected=0xa6, Read=0x00, failed byte count from 0x00008400-0x0000847f: 0x80
retrying.
VERIFY FAILED at 0x0000f180! Expected=0xb9, Read=0x00, failed byte count from 0x0000f180-0x0000f1ff: 0x7d
retrying.
VERIFY FAILED at 0x00015f00! Expected=0x83, Read=0x00, failed byte count from 0x00015f00-0x00015f7f: 0x80
retrying.
VERIFY FAILED at 0x00018880! Expected=0x75, Read=0x07, failed byte count from 0x00018880-0x000188ff: 0x7f
retrying.
VERIFY FAILED at 0x00018880! Expected=0x75, Read=0x05, failed byte count from 0x00018880-0x000188ff: 0x28
retrying.
VERIFY FAILED at 0x00018880! Expected=0x75, Read=0x00, failed byte count from 0x00018880-0x000188ff: 0x7f
retrying.
VERIFY FAILED at 0x0001cb80! Expected=0x0f, Read=0x00, failed byte count from 0x0001cb80-0x0001cbff: 0x7f
retrying.

Done.
Verifying flash... VERIFIED.

and just to be really sure:
./flashrom -V -p nic3com -c "W29C010(M)/W29C011A/W29EE011/W29EE012" -r ../testimages/128kB-W29EE011.rand.img
...

md5sum ...
b2b9911ae898a52adfe55e73b816af1c  128kB.rand.img
b2b9911ae898a52adfe55e73b816af1c  128kB-W29EE011.rand.img

W29EE011-15 (was: Hello, Support for Atmel's AT25F series and some small stuff)

[Michael Karcher]

Am Dienstag, den 15.03.2011, 02:05 +0100 schrieb Stefan Tauner:
> even more puzzling (at least for me atm) are these erase verify failures...
> ./flashrom -V -p nic3com -c "W29C010(M)/W29C011A/W29EE011/W29EE012" -w ../testimages/128kB.rand.img
> ...
> Reading old flash chip contents...
> Erasing and writing flash chip... Looking at blockwise erase function 0... trying... 0x000000-0x01ffff:EWVERIFY FAILED at 0x00008400! Expected=0xa6, Read=0x00, failed byte count from 0x00008400-0x0000847f: 0x80
> retrying.
> VERIFY FAILED at 0x0000f180! Expected=0xb9, Read=0x00, failed byte count from 0x0000f180-0x0000f1ff: 0x7d
> retrying.

The key is "retrying". We really need to suppress the frightening
"VERIFY FAILED" messages for these chips. It retries a page up to ten
times, and only bails out then.

BTW: These failures don't look like the typical "we didn't manage to get
the whole page without timeout" problem. Maybe the need for retrying on
these chips is normal, or our toggle-bit code terminates early...

Regards,
  Michael Karcher