> 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 _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
