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

login
register
about
Submitter Hatim Kanchwala
Date 2016-03-11 21:03:13
Message ID <1457730195-8900-2-git-send-email-hatim@hatimak.me>
Download mbox | patch
Permalink /patch/4421/
State New
Headers show

Comments

Hatim Kanchwala - 2016-03-11 21:03:13
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(-)
Stefan Tauner - 2016-03-12 03:15:53
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
Hatim Kanchwala - 2016-03-16 15:34:53
On Saturday 12 March 2016 08:45 AM, Stefan Tauner wrote:
> 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.

40B/80B have SFDP and second status register whereas the non-B counterparts do not. They also differ in amount of memory for security registers. These indeed are minor details not reflected by flashrom definitions. I updated the existing definitions and performed tests again (logs attached), works perfectly.

> 
>> 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
> 

Yes you are correct, there is no non-B version. LQ05B was in the same datasheet as LQ20B so I'll add that as well.

>> +#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
> 
> 
> 

The newer patch follows in a separate successive mail along with support for LQ05B. Thanks.

Patch

diff --git a/flashchips.c b/flashchips.c
index 0fc1b7a..5da56d0 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -5628,30 +5628,193 @@  const struct flashchip flashchips[] = {
 					{16 * 1024, 1},
 				},
 				.block_erase = erase_block_jedec,
 			}, {
 				.eraseblocks = { {2048 * 1024, 1} },
 				.block_erase = erase_chip_block_jedec,
 			},
 		},
 		.write		= write_jedec_1, /* Supports a fast mode too */
 		.read		= read_memmapped,
 		.voltage	= {3000, 3600}, /* 3.0-3.6V for type -70, others 2.7-3.6V */
 	},
 
 	{
 		.vendor		= "GigaDevice",
+		.name		= "GD25LQ10B",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= GIGADEVICE_ID,
+		.model_id	= GIGADEVICE_GD25LQ10,
+		.total_size	= 128,
+		.page_size	= 256,
+		/* Supports SFDP */
+		/* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 32} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {32 * 1024, 4} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 2} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {128 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {128 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		/* TODO: 2nd status reg (read 0x35); 3rd status reg (read 0x15) */
+		.printlock	= spi_prettyprint_status_register_bp4_srwd,
+		.unlock		= spi_disable_blockprotect_bp4_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 2100},
+	},
+
+	{
+		.vendor		= "GigaDevice",
+		.name		= "GD25LQ20B",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= GIGADEVICE_ID,
+		.model_id	= GIGADEVICE_GD25LQ20,
+		.total_size	= 256,
+		.page_size	= 256,
+		/* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 64} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {32 * 1024, 8} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 4} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {256 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {256 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		/* TODO: 2nd status reg (read 0x35); 3rd status reg (read 0x15) */
+		.printlock	= spi_prettyprint_status_register_bp4_srwd,
+		.unlock		= spi_disable_blockprotect_bp4_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 2100},
+	},
+
+	{
+		.vendor		= "GigaDevice",
+		.name		= "GD25LQ40B",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= GIGADEVICE_ID,
+		.model_id	= GIGADEVICE_GD25LQ40,
+		.total_size	= 512,
+		.page_size	= 256,
+		/* Supports SFDP */
+		/* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 128} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {32 * 1024, 16} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 8} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {512 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {512 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		/* TODO: 2nd status reg (read 0x35); 3rd status reg (read 0x15) */
+		.printlock	= spi_prettyprint_status_register_bp4_srwd,
+		.unlock		= spi_disable_blockprotect_bp4_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 2100},
+	},
+
+	{
+		.vendor		= "GigaDevice",
+		.name		= "GD25LQ80B",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= GIGADEVICE_ID,
+		.model_id	= GIGADEVICE_GD25LQ80,
+		.total_size	= 1024,
+		.page_size	= 256,
+		/* Supports SFDP */
+		/* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 256} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {32 * 1024, 32} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 16} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {1024 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {1024 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		/* TODO: 2nd status reg (read 0x35); 3rd status reg (read 0x15) */
+		.printlock	= spi_prettyprint_status_register_bp4_srwd,
+		.unlock		= spi_disable_blockprotect_bp4_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 2100},
+	},
+
+	{
+		.vendor		= "GigaDevice",
 		.name		= "GD25LQ40",
 		.bustype	= BUS_SPI,
 		.manufacture_id	= GIGADEVICE_ID,
 		.model_id	= GIGADEVICE_GD25LQ40,
 		.total_size	= 512,
 		.page_size	= 256,
 		/* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */
 		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
 			{
 				.eraseblocks = { {4 * 1024, 128} },
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 */
+#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
 #define GIGADEVICE_GD29GL064CAB	0x7E0601
 
 #define HYUNDAI_ID		0xAD	/* Hyundai */
 #define HYUNDAI_HY29F400T	0x23	/* Same as HY29F400AT */
 #define HYUNDAI_HY29F800B	0x58	/* Same as HY29F800AB */
 #define HYUNDAI_HY29LV800B	0x5B
 #define HYUNDAI_HY29F040A	0xA4
 #define HYUNDAI_HY29F400B	0xAB	/* Same as HY29F400AB */
 #define HYUNDAI_HY29F002B	0x34
 #define HYUNDAI_HY29F002T	0xB0
 #define HYUNDAI_HY29LV400T	0xB9