2022-10-06 03:47:13

by mikhail.kshevetskiy

[permalink] [raw]
Subject: [PATCH] spi-nand: add Winbond W25N02KV flash support, fix Winbond flashes identifications

From: Mikhail Kshevetskiy <[email protected]>

Winbond uses 3 bytes to identify flash: vendor_id, dev_id_0, dev_id_1,
but current driver uses only first 2 bytes of it for devices
idenfification. As result Winbond W25N02KV flash (id_bytes: EF, AA, 22)
is identified as W25N01GV (id_bytes: EF, AA, 21).

Fix this and add W25N02KV flash support.

Signed-off-by: Mikhail Kshevetskiy <[email protected]>
---
drivers/mtd/nand/spi/winbond.c | 79 +++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
index 76684428354e..3ad58cd284d8 100644
--- a/drivers/mtd/nand/spi/winbond.c
+++ b/drivers/mtd/nand/spi/winbond.c
@@ -74,9 +74,75 @@ static int w25m02gv_select_target(struct spinand_device *spinand,
return spi_mem_exec_op(spinand->spimem, &op);
}

+static int w25n02kv_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 3)
+ return -ERANGE;
+
+ region->offset = 64 + (16 * section);
+ region->length = 13;
+
+ return 0;
+}
+
+static int w25n02kv_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 3)
+ return -ERANGE;
+
+ region->offset = (16 * section) + 2;
+ region->length = 14;
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops w25n02kv_ooblayout = {
+ .ecc = w25n02kv_ooblayout_ecc,
+ .free = w25n02kv_ooblayout_free,
+};
+
+static int w25n02kv_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ case STATUS_ECC_NO_BITFLIPS:
+ return 0;
+
+ case STATUS_ECC_UNCOR_ERROR:
+ return -EBADMSG;
+
+ case STATUS_ECC_HAS_BITFLIPS:
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nanddev_get_ecc_conf(nand)->strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nanddev_get_ecc_conf(nand)->strength || !mbf))
+ return nanddev_get_ecc_conf(nand)->strength;
+
+ return mbf;
+
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
static const struct spinand_info winbond_spinand_table[] = {
SPINAND_INFO("W25M02GV",
- SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xab),
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xab, 0x21),
NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 2),
NAND_ECCREQ(1, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
@@ -86,7 +152,7 @@ static const struct spinand_info winbond_spinand_table[] = {
SPINAND_ECCINFO(&w25m02gv_ooblayout, NULL),
SPINAND_SELECT_TARGET(w25m02gv_select_target)),
SPINAND_INFO("W25N01GV",
- SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xaa),
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xaa, 0x21),
NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
NAND_ECCREQ(1, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
@@ -94,6 +160,15 @@ static const struct spinand_info winbond_spinand_table[] = {
&update_cache_variants),
0,
SPINAND_ECCINFO(&w25m02gv_ooblayout, NULL)),
+ SPINAND_INFO("W25N02KV",
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xaa, 0x22),
+ NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ 0,
+ SPINAND_ECCINFO(&w25n02kv_ooblayout, w25n02kv_ecc_get_status)),
};

static int winbond_spinand_init(struct spinand_device *spinand)
--
2.35.1


2022-10-06 09:59:34

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] spi-nand: add Winbond W25N02KV flash support, fix Winbond flashes identifications

Hi Frieder,

[email protected] wrote on Thu, 6 Oct 2022 11:26:45 +0200:

> Am 06.10.22 um 05:32 schrieb [email protected]:
> >
> > From: Mikhail Kshevetskiy <[email protected]>
> >
> > Winbond uses 3 bytes to identify flash: vendor_id, dev_id_0, dev_id_1,
> > but current driver uses only first 2 bytes of it for devices
> > idenfification. As result Winbond W25N02KV flash (id_bytes: EF, AA, 22)
> > is identified as W25N01GV (id_bytes: EF, AA, 21).
> >
> > Fix this and add W25N02KV flash support.
> >
> > Signed-off-by: Mikhail Kshevetskiy <[email protected]>
>
> Thanks for the patch. We have a similar change in our downstream kernel,
> that I failed to push upstream so far. But your version looks even better.
>
> Can you change the subject prefix to "mtd: spinand: winbond:", please?
>
> And I also would like to ask you to split this into two patches. The
> first one to add the third byte in the ids of the existing devices and
> the second one to add support for the new device.

Agreed on both comments, thanks Frieder!

> With these two changes you can add:
>
> Reviewed-by: Frieder Schrempf <[email protected]>

Cheers,
Miquèl

2022-10-06 10:00:01

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH] spi-nand: add Winbond W25N02KV flash support, fix Winbond flashes identifications

Am 06.10.22 um 05:32 schrieb [email protected]:
>
> From: Mikhail Kshevetskiy <[email protected]>
>
> Winbond uses 3 bytes to identify flash: vendor_id, dev_id_0, dev_id_1,
> but current driver uses only first 2 bytes of it for devices
> idenfification. As result Winbond W25N02KV flash (id_bytes: EF, AA, 22)
> is identified as W25N01GV (id_bytes: EF, AA, 21).
>
> Fix this and add W25N02KV flash support.
>
> Signed-off-by: Mikhail Kshevetskiy <[email protected]>

Thanks for the patch. We have a similar change in our downstream kernel,
that I failed to push upstream so far. But your version looks even better.

Can you change the subject prefix to "mtd: spinand: winbond:", please?

And I also would like to ask you to split this into two patches. The
first one to add the third byte in the ids of the existing devices and
the second one to add support for the new device.

With these two changes you can add:

Reviewed-by: Frieder Schrempf <[email protected]>