[flashrom] [PATCH] adding support for AT25DQ321

Hatim Kanchwala hatim at hatimak.me
Sun Mar 27 04:30:40 CEST 2016


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 <hatim at hatimak.me>

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 <seanboree at gmail.com <mailto:seanboree at gmail.com>>
> 
> 
> 
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> https://www.flashrom.org/mailman/listinfo/flashrom
> 

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




More information about the flashrom mailing list