Re: [flashrom] [PATCH] adding support for AT25DQ321

[Hatim Kanchwala]

Dear Sean,

Thanks for your patch. Here's a few issues I found -
1. For patches that touch struct flashchip it is wise to include more context lines. Have a look here (https://www.flashrom.org/Development_Guidelines#Patch_submission).
2. Tabs were converted to spaces. You can have a look at the AT25F512 entry just below it to understand the differences.
3. Regarding total_size, we write it usually as "x * 1024" that is to be read as "x kB".
4. This particular chip supports dual and quad I/O, so the FEATURE_QPI bit should be set and you can also include a comment. Just have a look at other entries in flashchips.c.
5. IMHO, the FIXME comment in spi25_statusreg.c should only come at spi_prettyprint_status_register_at25df_sec and not spi_prettyprint_status_register_at25df, because AT25DQ321 really uses only the former.

Apart from the above, the content looks fine to me. Thanks for the patch. :)

Acked-by: Hatim Kanchwala

On Sunday 27 March 2016 05:15 AM, sean boree wrote:
> This patch adds support for the AT25DQ321 chip, as this is my first patch, I apologise 
> for any rookie mistakes I made.
>
> Signed-off-by: Sean Boree <[e-mail] >
>
> flashrom mailing list
> [e-mail]
> https://www.flashrom.org/mailman/listinfo/flashrom