On Sat, 12 Mar 2016 02:33:13 +0530 Hatim Kanchwala hatim@hatimak.me wrote:
Hello,
I referred to two datasheets for these chips (both from GigaDevice) and I noticed a mismatch in voltage range for GD25LQ20B. However the following values were fine when I performed the tests.
Signed-off-by: Hatim Kanchwala hatim@hatimak.me
flashchips.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ flashchips.h | 6 ++- 2 files changed, 167 insertions(+), 2 deletions(-)
Hi,
thanks for your patch(es)! In general we want to keep the number of chip definitions with the same ID as low as possible. If chips only differ in minor details that often are not even reflected by the definitions within flashrom we usually combine their definitions. I did not spot any major differences between the 40B, the 80B and there respective non-B counterparts, did you? If not I'd rather simply update/comment the existing definitions, e.g. the name for the GD25LQ40 definition should be changed to "GD25LQ40(B)". There are many similar examples in flashchips.c too.
diff --git a/flashchips.h b/flashchips.h index 9ffb30f..3e2b18c 100644 --- a/flashchips.h +++ b/flashchips.h @@ -364,32 +364,34 @@ #define GIGADEVICE_ID 0xC8 /* GigaDevice */ #define GIGADEVICE_GD25T80 0x3114 #define GIGADEVICE_GD25Q512 0x4010 #define GIGADEVICE_GD25Q10 0x4011 #define GIGADEVICE_GD25Q20 0x4012 /* Same as GD25QB */ #define GIGADEVICE_GD25Q40 0x4013 /* Same as GD25QB */ #define GIGADEVICE_GD25Q80 0x4014 /* Same as GD25Q80B (which has OTP) */ #define GIGADEVICE_GD25Q16 0x4015 /* Same as GD25Q16B (which has OTP) */ #define GIGADEVICE_GD25Q32 0x4016 /* Same as GD25Q32B */ #define GIGADEVICE_GD25Q64 0x4017 /* Same as GD25Q64B */ #define GIGADEVICE_GD25Q128 0x4018 /* GD25Q128B and GD25Q128C only, can be distinguished by SFDP */ #define GIGADEVICE_GD25VQ21B 0x4212 #define GIGADEVICE_GD25VQ41B 0x4213 /* Same as GD25VQ40C, can be distinguished by SFDP */ #define GIGADEVICE_GD25VQ80C 0x4214 #define GIGADEVICE_GD25VQ16C 0x4215 -#define GIGADEVICE_GD25LQ40 0x6013 -#define GIGADEVICE_GD25LQ80 0x6014 +#define GIGADEVICE_GD25LQ10 0x6011 /* Same as GD25LQ10B, can be distinguished by SFDP */ +#define GIGADEVICE_GD25LQ20 0x6012 /* Same as GD25LQ20B, can be distinguished by SFDP */
There does not seem to be a non-b LQ10 or LQ20 AFAICT thus these comments seem to be wrong (and we should probably change the define to include the B as well). However, there is an LQ05B that is missing yet: #define GIGADEVICE_GD25LQ05B 0x6010
+#define GIGADEVICE_GD25LQ40 0x6013 /* Same as GD25LQ40B, can be distinguished by SFDP */ +#define GIGADEVICE_GD25LQ80 0x6014 /* Same as GD25LQ80B, can be distinguished by SFDP */ #define GIGADEVICE_GD25LQ16 0x6015 #define GIGADEVICE_GD25LQ32 0x6016 #define GIGADEVICE_GD25LQ64 0x6017 /* Same as GD25LQ64B (which is faster) */ #define GIGADEVICE_GD25LQ128 0x6018