On Sat, 12 Mar 2016 02:33:13 +0530
Hatim Kanchwala <ha...@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 <ha...@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



-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

_______________________________________________
flashrom mailing list
flashrom@flashrom.org
https://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to