[flashrom] Add support for unlock and write to Atmel AT26F040

Tim Small tim at buttersideup.com
Tue Aug 2 10:30:52 CEST 2011


Hi Stefan,

Thanks for the review, I'll see if I have time to spin another patch.

>> > +	const uint8_t stat_bit_WPP = (1 << 4);
>>     
> carldani has preferred shorter variables in the past. we usually don't
> use such constants and verbose comments (although it would help newbies
> to understand the code).

Yep, that was my aim.

>  i think anyone able to read a datasheet
> would prefer these inline (if not they should be macros instead).
> look at the other functions in at25.c for examples.
>   

OK, cool.  I'm happy to go with house style - tho' macros always seem
like a bit of an anachronism with modern compilers (and are
debugger-hostile of course).

> would that buy us anything if used?
>
> +		/* Supports spi_at26_write_sequential too, but it's untested */
>   


Probably not, but carldani said on IRC (at the time I wrote the original
patch) that he'd like to add a mechanism to support multiple alternative
write modes in the future, so this comment was with the aim of making
that transition easier.

>> > -		.write		= NULL /* Incompatible Page write */,
>> > +		.unlock		= spi_disable_blockprotect_at26f040,
>> > +		/* also supports spi_chip_write_1, but nothing else */
>>     
> have you tested spi_chip_write_1?
>   

Yes I have, and it worked (I implemented the write_sequential code
whilst I was waiting for the spi_chip_write_1 test to complete!).


Thanks for the review.  ACK on all other comments!

Tim.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20110802/9a657819/attachment.html>


More information about the flashrom mailing list