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 <[email protected]>

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 <[email protected] <mailto:[email protected]>>
> 
> 
> 
> _______________________________________________
> flashrom mailing list
> [email protected]
> https://www.flashrom.org/mailman/listinfo/flashrom
> 

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

_______________________________________________
flashrom mailing list
[email protected]
https://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to