[flashrom] [PATCH 1/3] Add support for GD25LQ10B, GD25LQ20B, GD25LQ40B and GD25LQ80B

[Stefan Tauner]

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