[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