[flashrom] [PATCH/FULL/SVN] Support for 4-bytes addressing SPI flash chips 32MB+ (256Mb+)
dev at borisbaykov.com
Thu Jan 15 13:08:25 CET 2015
> 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?
> - 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
More information about the flashrom