Hello Stefan,
On Monday 01 February 2016 05:51 AM, Stefan Tauner wrote:
On Fri, 29 Jan 2016 20:33:46 +0530 Hatim Kanchwala hatim@hatimak.me wrote:
Updated comments in spi.h to reflect ESI chip opcodes. Updated patch follows.
Index: spi.h
--- spi.h (revision 1919) +++ spi.h (working copy) @@ -72,5 +72,5 @@ #define JEDEC_CE_62_INSIZE 0x00
-/* Chip Erase 0xc7 is supported by SST/ST/EON/Macronix chips. */ +/* Chip Erase 0xc7 is supported by SST/ST/EON/Macronix/ESI chips. */ #define JEDEC_CE_C7 0xc7 #define JEDEC_CE_C7_OUTSIZE 0x01 @@ -97,5 +97,5 @@ #define JEDEC_BE_C4_INSIZE 0x00
-/* Block Erase 0xd8 is supported by EON/Macronix chips. */ +/* Block Erase 0xd8 is supported by EON/Macronix/ESI chips. */ #define JEDEC_BE_D8 0xd8 #define JEDEC_BE_D8_OUTSIZE 0x04
Hi again,
thanks for your patch. It had similar problems like the first one but its contents are almost perfect again, good work!
Thanks!
After receiving your reviews on both the patches, I spent time setting up git-svn and trying out a bunch of stuff. Indeed git-svn and git send-email are the easiest way to send patches. I wasn't aware of the 112 characters per line rule (it is not mentioned in the Development Guidelines in the wiki). I'll update the wiki (along with other pending edits I had proposed).
I have left out the spi.h changes because these comments should rather be deleted than extended. They are remains from the early days of SPI support in flashrom when we did not know yet how common some of the opcodes actually are. It turned out that d8 and c7 are almost universally understood by any flash chip.
I am working adding support for Eon's EN25P* series and these understand the 0xd8 and 0xc7 opcodes as well.
One other nitpick: the order of the three flash chips was wrong. They should be ordered by total size within any family as per comment on top of flashchips.c So the order of chips after applying your patches should have been (as shown by flashrom -L): ... Eon EN29GL128 16384 Parallel ESI ES25P40 512 SPI ESI ES25P80 1024 SPI ESI ES25P16 2048 SPI Fujitsu MBM29F004BC 512 Parallel ...
My bad. I had seen the comment and had made a note of it. I had also gone through the context-lines guideline for flashchips.c, but I guess in a hurry to submit my first patch, I was careless.
I'll commit these two patches in a few as r1923. Thank you very much and congratulations to your first contribution to flashrom.
Thanks again! I'll follow the guidelines carefully in all the future patches I send.