[flashrom] [PATCH/FULL/SVN] Support for 4-bytes addressing SPI flash chips 32MB+ (256Mb+)

[Boris Baykov]

> Boris,
> Thank you for your effort! This is indeed a very useful feature that flashrom has been
> lacking for too long.

I wonder if this is helpful to may people. Thanks.

> - Use of a fishing-world word 'BAIT' many times throughout the code.
>   Probably, 'BYTE' was intended. This typo is quite alarming for me.

4BAIT is not BYTE )))
It's just the short form of 4-Byte Addressing Instruction Table, a part of SFDP 1.6

> - In walk_eraseregions() variables percent_last and percent_current are
>   initialized conditionally, which makes some compilers (e.g., mingw32msvc-gcc v4.2.1)
>   complain about possible use of those variables uninitialized.

I used gcc Linux so I haven't checked it with mingw. I'm sorry for these warnings.

> - Happy New Year!
>   I believe that the 'History of changes' in sfdp.c should contain dates
>   from 2015, not from 2014. In other newly added places I guess it is also 2015, not 2014.
>   ;)

O, yeah, it's funny :-) Should be 2015 of course.

> - Looks like currently one can define a chip with size over 16MB but without
>   FEATURE_4BA_SUPPORT and get strange results. Maybe it would be better to imply
>   4BA support for any chips over 16MB and get rid of FEATURE_4BA_SUPPORT bit?

Good idea!

> - What is this 'eraser.type' field is for? I don't see any actual use for it.
>   Looks like an index field to me if I understand JESD216B right.
>   Wouldn't it be more correct to search for a proper eraser using block size
>   rather than 'eraser type' ? Current implementation forces developers to
>   think about those 'eraser types' each time they add a new 4BA chip.
>   Am I overlooking anything here?

This field is used to change the required eraser from 4BA (or EREG) to
4BA_direct in sfdp_change_uniform_eraser_4ba_direct in sfdp.c

> +       tmp32 =  ((unsigned int)buf[(4 * 0) + 0]);
> +       tmp32 |= ((unsigned int)buf[(4 * 0) + 1]) << 8;
> +       tmp32 |= ((unsigned int)buf[(4 * 0) + 2]) << 16;
> +       tmp32 |= ((unsigned int)buf[(4 * 0) + 3]) << 24;
>   make my eyes bleed. Why not use a loop or at least a macro?

This code is just a copy-paste from sfdp.c to sfdp.c to make parsing of new
dwords same as all others. All previous dwords have same code. Macro is better.

> Also, I couldn't manage to make this patch work with MXIC MX25L25635F chip which
> supports all the 4BA modes (direct, EN4B, EAR). I tried various combinations,
> but it couldn't properly erase the chip anyway. Didn't work in SFDP mode as well
> (only SFDP 1.0 is supported by the chip). Hence, I couldn't check if it reads
> back properly what was written. I didn't do any debugging though to find out
> the reason.

Try to uncomment FBA_USE_EXT_ADDR_REG_BY_DEFAULT define in sfdp.c It may work after.
Surely it should be checked more deeply with the chip.

> Thanks again.
Thank you, Alexander

Boris Baykov