mtd: spinand: add support for ESMT F50x1G41LB

PATCH In review Chuanhong Guo 3 participants 7 messages Apr 13, 2022 – Jan 26, 2024

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Chuanhong Guo

author

2022-04-13 08:39 UTC #0 lore raw

PATCH v2 1/1 mtd: spinand: add support for ESMT F50x1G41LB

This patch adds support for ESMT F50L1G41LB and F50D1G41LB. It seems that ESMT likes to use random JEDEC ID from other vendors. Their 1G chips uses 0xc8 from GigaDevice and 2G/4G chips uses 0x2c from Micron. For this reason, the ESMT entry is named esmt_c8 with explicit JEDEC ID in variable name.

Datasheets:
https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50L1G41LB(2M).pdf
https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50D1G41LB(2M).pdf

Signed-off-by Chuanhong Guo <[email protected]>

---
This patch is made purely based on datasheet info without testing on any actual chips.

Change since v1: drop 0x7f padding from SPINAND_ID.

 drivers/mtd/nand/spi/Makefile |  2 +-
 drivers/mtd/nand/spi/core.c   |  1 +
 drivers/mtd/nand/spi/esmt.c   | 94 +++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |  1 +
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/esmt.c

drivers/mtd/nand/spi/Makefile +1 -1

index 9662b9c1d5a9..7e3ab8a9aec7 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o
+spinand-objs := core.o esmt.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o

drivers/mtd/nand/spi/core.c +1 -0

index ff8336870bc0..6c5d79ec3501 100644
@@ -927,6 +927,7 @@ static const struct nand_ops spinand_ops = {
 };
 
 static const struct spinand_manufacturer *spinand_manufacturers[] = {
+	&esmt_c8_spinand_manufacturer,
 	&gigadevice_spinand_manufacturer,
 	&macronix_spinand_manufacturer,
 	&micron_spinand_manufacturer,

drivers/mtd/nand/spi/esmt.c new file +94 -0

94 lines not on this page · raw message

include/linux/mtd/spinand.h +1 -0

index 3aa28240a77f..e19a1554a1e9 100644
@@ -260,6 +260,7 @@ struct spinand_manufacturer {
 };
 
 /* SPI NAND manufacturers */
+extern const struct spinand_manufacturer esmt_c8_spinand_manufacturer;
 extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
 extern const struct spinand_manufacturer macronix_spinand_manufacturer;
 extern const struct spinand_manufacturer micron_spinand_manufacturer;

Signature · 1 line

2.35.1

Miquel Raynal

maintainer

2022-04-13 12:58 UTC #1 lore raw

Hi,

[email protected] wrote on Wed, 13 Apr 2022 16:38:19 +0800:

Quoted 6 lines

This patch adds support for ESMT F50L1G41LB and F50D1G41LB. It seems that ESMT likes to use random JEDEC ID from other vendors. Their 1G chips uses 0xc8 from GigaDevice and 2G/4G chips uses 0x2c from Micron. For this reason, the ESMT entry is named esmt_c8 with explicit JEDEC ID in variable name.

Quoted 8 lines

Datasheets:
https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50L1G41LB(2M).pdf
https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50D1G41LB(2M).pdf
Signed-off-by Chuanhong Guo <[email protected]>
---
This patch is made purely based on datasheet info without testing on any actual chips.

Do you plan to get one of these any time soon?

I am not really confident merging a 100% non-tested driver :)

Quoted 38 lines

Change since v1: drop 0x7f padding from SPINAND_ID.

 drivers/mtd/nand/spi/Makefile |  2 +-
 drivers/mtd/nand/spi/core.c   |  1 +
 drivers/mtd/nand/spi/esmt.c   | 94 +++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |  1 +
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/esmt.c

drivers/mtd/nand/spi/Makefile +1 -1

index 9662b9c1d5a9..7e3ab8a9aec7 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o
+spinand-objs := core.o esmt.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o

drivers/mtd/nand/spi/core.c +1 -0

index ff8336870bc0..6c5d79ec3501 100644
@@ -927,6 +927,7 @@ static const struct nand_ops spinand_ops = {
 };
 
 static const struct spinand_manufacturer *spinand_manufacturers[] = {
+	&esmt_c8_spinand_manufacturer,
 	&gigadevice_spinand_manufacturer,
 	&macronix_spinand_manufacturer,
 	&micron_spinand_manufacturer,

drivers/mtd/nand/spi/esmt.c new file +2 -0

new file mode 100644
index 000000000000..f86716332893
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+/*

drivers/mtd/nand/spi/esmt.c line 12

Quoted 10 lines

drivers/mtd/nand/spi/esmt.c new file +10 -0

+ * Author:
+ *	Chuanhong Guo <[email protected]>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+/* ESMT uses GigaDevice 0xc8 JECDEC ID on some SPI NANDs */
+#define SPINAND_MFR_ESMT_C8			0xc8

What happens if the gigadevice driver probes first?

drivers/mtd/nand/spi/esmt.c line 18

Quoted 6 lines

drivers/mtd/nand/spi/esmt.c new file +6 -0

+
+#define F50L2G41XA_ECC_STATUS_MASK		GENMASK(6, 4)
+#define F50L2G41XA_STATUS_ECC_1_3_BITFLIPS	(1 << 4)
+#define F50L2G41XA_STATUS_ECC_4_6_BITFLIPS	(3 << 4)
+#define F50L2G41XA_STATUS_ECC_7_8_BITFLIPS	(5 << 4)
+

Thanks,
Miquèl

Chuanhong Guo

author

2022-04-13 14:50 UTC #2 lore raw

Hi!

Quoted 5 lines · Miquel Raynal

[...]
Quoted 2 lines
This patch is made purely based on datasheet info without testing on any actual chips.
Do you plan to get one of these any time soon?

No. I already have way more spi-nand chips than I possibly need due to my last GigaDevice submission :)

I need to replace the spi-nand driver for mediatek in OpenWrt with my recent submission, and this chip is used in one of the currently supported router. So I made this patch from datasheet for it.

BTW Rockchip added identical chip support in their u-boot.[0] I assume they've tested it.

Quoted 1 line

I am not really confident merging a 100% non-tested driver :)

I can understand that.
I'll roll this patch out in OpenWrt anyway. I can wait for a test there and resubmit piled downstream patches for chip supports after next OpenWrt stable release.

Quoted 7 lines

[...]
drivers/mtd/nand/spi/esmt.c line 12
Quoted 3 lines
drivers/mtd/nand/spi/esmt.c new file +3 -0
+
+/* ESMT uses GigaDevice 0xc8 JECDEC ID on some SPI NANDs */
+#define SPINAND_MFR_ESMT_C8                  0xc8
What happens if the gigadevice driver probes first?

Their device ID aren't conflicting yet, so nothing will happen at the moment.

There is a solution for future conflict: Recent SPI-NAND chips contain a parameter page which has the exact chip vendor and model. We can do one more detection with the parameter page content.
Winbond W25N01KV is a 2k+96 SPI-NAND with 4-bit ECC. It uses the exact same chip id as the current W25N01GV (2k+64 1-bit ECC). We need to support detection using parameter page for this crazy decision by Winbond anyway. I'll try to code something for this with my free time.

My current idea is: We first do a detection based on chip id. If that failed, try to read the parameter page. If we got a valid one, match the chip vendor and model string.

Any thoughts?

(BTW this ESMT chip is POWERCHIP PSU1GS20DX according to the parameter page in their datasheet. But I can't find a datasheet for this model number.)

Quoted 6 lines

drivers/mtd/nand/spi/esmt.c line 17

Quoted 6 lines

drivers/mtd/nand/spi/esmt.c new file +5 -0

+
+#define F50L2G41XA_ECC_STATUS_MASK           GENMASK(6, 4)
+#define F50L2G41XA_STATUS_ECC_1_3_BITFLIPS   (1 << 4)
+#define F50L2G41XA_STATUS_ECC_4_6_BITFLIPS   (3 << 4)
+#define F50L2G41XA_STATUS_ECC_7_8_BITFLIPS   (5 << 4)

Oops. These are left-over defines when I discovered that ESMT 2G/4G SPI-NANDs are repackaged micron parts and dropped their support from this file.
A v3 is needed anyway. I'll wait for a test before submitting it.

[0]: https://github.com/rockchip-linux/u-boot/commit/52b0060178285488737854a48ddecd381f8b236e

Signature · 2 lines

Regards,
Chuanhong Guo

Miquel Raynal

maintainer

2022-04-13 15:28 UTC #3 lore raw

Hello,

[email protected] wrote on Wed, 13 Apr 2022 22:50:43 +0800:

Quoted 11 lines

Hi!
Quoted 5 lines · Miquel Raynal
[...]
Quoted 2 lines
This patch is made purely based on datasheet info without testing on any actual chips.
Do you plan to get one of these any time soon?
No. I already have way more spi-nand chips than I possibly need due to my last GigaDevice submission :)

Quoted 9 lines

I need to replace the spi-nand driver for mediatek in OpenWrt with my recent submission, and this chip is used in one of the currently supported router. So I made this patch from datasheet for it.
BTW Rockchip added identical chip support in their u-boot.[0] I assume they've tested it.

Quoted 6 lines

Quoted 1 line
I am not really confident merging a 100% non-tested driver :)
I can understand that.
I'll roll this patch out in OpenWrt anyway. I can wait for a test there and resubmit piled downstream patches for chip supports after next OpenWrt stable release.

Yes, please, any feedback from the OpenWrt community would be good. If you can get a Tested-by I'll take the patch (resend it with the tag so that I don't miss it).

Quoted 10 lines

Quoted 6 lines
[...]
drivers/mtd/nand/spi/esmt.c line 12
Quoted 3 lines
drivers/mtd/nand/spi/esmt.c new file +3 -0
+
+/* ESMT uses GigaDevice 0xc8 JECDEC ID on some SPI NANDs */
+#define SPINAND_MFR_ESMT_C8                  0xc8  
What happens if the gigadevice driver probes first?
Their device ID aren't conflicting yet, so nothing will happen at the moment.

Quoted 8 lines

There is a solution for future conflict: Recent SPI-NAND chips contain a parameter page which has the exact chip vendor and model. We can do one more detection with the parameter page content.
Winbond W25N01KV is a 2k+96 SPI-NAND with 4-bit ECC. It uses the exact same chip id as the current W25N01GV (2k+64 1-bit ECC). We need to support detection using parameter page for this crazy decision by Winbond anyway.

:')

Quoted 6 lines

I'll try to code something for this with my free time. My current idea is: We first do a detection based on chip id. If that failed, try to read the parameter page. If we got a valid one, match the chip vendor and model string.
Any thoughts?

Yeah that looks reasonable.

Quoted 16 lines

(BTW this ESMT chip is POWERCHIP PSU1GS20DX according to the parameter page in their datasheet. But I can't find a datasheet for this model number.)
Quoted 6 lines
drivers/mtd/nand/spi/esmt.c line 17
Quoted 6 lines
drivers/mtd/nand/spi/esmt.c new file +5 -0
+
+#define F50L2G41XA_ECC_STATUS_MASK           GENMASK(6, 4)
+#define F50L2G41XA_STATUS_ECC_1_3_BITFLIPS   (1 << 4)
+#define F50L2G41XA_STATUS_ECC_4_6_BITFLIPS   (3 << 4)
+#define F50L2G41XA_STATUS_ECC_7_8_BITFLIPS   (5 << 4)  
Oops. These are left-over defines when I discovered that ESMT 2G/4G SPI-NANDs are repackaged micron parts and dropped their support from this file.
A v3 is needed anyway. I'll wait for a test before submitting it.

Quoted 1 line

[0]: https://github.com/rockchip-linux/u-boot/commit/52b0060178285488737854a48ddecd381f8b236e

Thanks,
Miquèl

Ezra Buehler

2024-01-23 17:54 UTC #4 lore raw

In reply to Chuanhong Guo

Hi Chuanhong Guo,

Quoted 2 lines · On Wed, 13 Apr 2022 22:50:43 +0800 Chuanhong Guo <[email protected]> wrote:

Their device ID aren't conflicting yet, so nothing will happen at the moment.

Since commit aa08bf187f32 ("mtd: spinand: esmt: add support for F50D2G41KA") we have a conflict with the GigaDevice GD5F1GQ5UExxG (manufacturer ID 0xC8, device ID 0x51), preventing our board from booting.

Quoted 10 lines

There is a solution for future conflict: Recent SPI-NAND chips contain a parameter page which has the exact chip vendor and model. We can do one more detection with the parameter page content.
Winbond W25N01KV is a 2k+96 SPI-NAND with 4-bit ECC. It uses the exact same chip id as the current W25N01GV (2k+64 1-bit ECC). We need to support detection using parameter page for this crazy decision by Winbond anyway. I'll try to code something for this with my free time.

Quoted 3 lines

My current idea is: We first do a detection based on chip id. If that failed, try to read the parameter page. If we got a valid one, match the chip vendor and model string.

According to the datasheets, the ESMT chips actually have a 5 byte ID, the last 3 bytes being 0x7F (JEDEC Maker Code Continuation Code). Why can't we simply extend the ID in esmt.c (as you had it in your original patch) and increase SPINAND_MAX_ID_LEN to 5? Or, alternatively, only extend the ID to 4 bytes?

If that is the way to go, I would be happy to provide the patches.

Cheers,
Ezra.

Miquel Raynal

maintainer

2024-01-24 17:08 UTC #5 lore raw

Hi Ezra,

[email protected] wrote on Tue, 23 Jan 2024 19:54:06 +0200:

Quoted 25 lines

Hi Chuanhong Guo,
Quoted 2 lines · On Wed, 13 Apr 2022 22:50:43 +0800 Chuanhong Guo <[email protected]> wrote:
Their device ID aren't conflicting yet, so nothing will happen at the moment.
Since commit aa08bf187f32 ("mtd: spinand: esmt: add support for F50D2G41KA") we have a conflict with the GigaDevice GD5F1GQ5UExxG (manufacturer ID 0xC8, device ID 0x51), preventing our board from booting.
Quoted 10 lines
There is a solution for future conflict: Recent SPI-NAND chips contain a parameter page which has the exact chip vendor and model. We can do one more detection with the parameter page content.
Winbond W25N01KV is a 2k+96 SPI-NAND with 4-bit ECC. It uses the exact same chip id as the current W25N01GV (2k+64 1-bit ECC). We need to support detection using parameter page for this crazy decision by Winbond anyway. I'll try to code something for this with my free time.
Quoted 3 lines
My current idea is: We first do a detection based on chip id. If that failed, try to read the parameter page. If we got a valid one, match the chip vendor and model string.

Quoted 7 lines

According to the datasheets, the ESMT chips actually have a 5 byte ID, the last 3 bytes being 0x7F (JEDEC Maker Code Continuation Code). Why can't we simply extend the ID in esmt.c (as you had it in your original patch) and increase SPINAND_MAX_ID_LEN to 5? Or, alternatively, only extend the ID to 4 bytes?
If that is the way to go, I would be happy to provide the patches.

Please send the patches. Chuanhong can you test them?

Thanks,
Miquèl

Chuanhong Guo

author

2024-01-26 01:45 UTC #6 lore raw

Hi Miquel!

Quoted 9 lines · Miquel Raynal

Quoted 7 lines
According to the datasheets, the ESMT chips actually have a 5 byte ID, the last 3 bytes being 0x7F (JEDEC Maker Code Continuation Code). Why can't we simply extend the ID in esmt.c (as you had it in your original patch) and increase SPINAND_MAX_ID_LEN to 5? Or, alternatively, only extend the ID to 4 bytes?
If that is the way to go, I would be happy to provide the patches.
Please send the patches. Chuanhong can you test them?

I think it should be fine to set MAX_ID_LEN to 5 and add more 0x7f to the ID definition. Unfortunately I don't have access to an ESMT flash for a test.

Signature · 2 lines

Regards,
Chuanhong Guo