[flashrom] [PATCH] Add flash chip definition for ESI ES25P40 and ES25P80

[Hatim Kanchwala]

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 at 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.

-- 

Regards,
Hatim Kanchwala
http://hatimak.me
B. Tech. Electrical Engineering
Indian Institute of Technology Patna