The following list shows the additional features that are required to support Skyhighmemory S35ML0xG3 SPI Nand:
[Always ECC On]
Always keep the ECC On during Bad Block Marking and Bad Block Checking
1. The on-die ECC feature is totally transparent to the host. The ECC parity bits used for this feature do not occupy the NAND spare areas.
2. The host is free to have its own ECC engine by using the spare areas that have standard size.
3. We provide this patch to enable users who have limited ECC capabilities on the host side to use the NAND flash. This patch has been tested thoroughly on Linux.
[Change ECC Status information]
This patch changes the ECC status information as follows to maintain compatibility.
00 (normal)
01(1-2 errors corrected)
10(3-6 errors corrected)
11(uncorrectable)
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 14 +++-
drivers/mtd/nand/spi/skyhigh.c | 145 +++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 3 +
4 files changed, 162 insertions(+), 2 deletions(-)
mode change 100644 => 100755 drivers/mtd/nand/spi/Makefile
mode change 100644 => 100755 drivers/mtd/nand/spi/core.c
create mode 100644 drivers/mtd/nand/spi/skyhigh.c
mode change 100644 => 100755 include/linux/mtd/spinand.h
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
old mode 100644
new mode 100755
index 19cc77288ebb..1e61ab21893a
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o gigadevice.o macronix.o
-spinand-objs += micron.o paragon.o toshiba.o winbond.o xtx.o
+spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o xtx.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
old mode 100644
new mode 100755
index e0b6715e5dfe..d09b2bd05284
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -196,6 +196,17 @@ static int spinand_init_quad_enable(struct spinand_device *spinand)
static int spinand_ecc_enable(struct spinand_device *spinand,
bool enable)
{
+ /*
+ * SkyHigh Memory : always ECC on
+ * The on-die ECC feature is totally transparent to the host.
+ * The ECC parity bits used for this feature do not occupy the NAND spare areas.
+ * The host is free to have its own ECC engine by using the spare areas that have standard size.
+ * We provide this patch to enable users who have limited ECC capabilities on the host side to use the NAND flash.
+ * This patch has been tested thoroughly on Linux.
+ */
+ if (spinand->flags & SPINAND_ON_DIE_ECC_MANDATORY)
+ return 0;
+
return spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
enable ? CFG_ECC_ENABLE : 0);
}
@@ -561,7 +572,7 @@ static int spinand_reset_op(struct spinand_device *spinand)
NULL);
}
-static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
+int spinand_lock_block(struct spinand_device *spinand, u8 lock)
{
return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
}
@@ -945,6 +956,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
¯onix_spinand_manufacturer,
µn_spinand_manufacturer,
¶gon_spinand_manufacturer,
+ &skyhigh_spinand_manufacturer,
&toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
&xtx_spinand_manufacturer,
diff --git a/drivers/mtd/nand/spi/skyhigh.c b/drivers/mtd/nand/spi/skyhigh.c
new file mode 100644
index 000000000000..f001357b4d85
--- /dev/null
+++ b/drivers/mtd/nand/spi/skyhigh.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 SkyHigh Memory Limited
+ *
+ * Author: Takahiro Kuwano <[email protected]>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_SKYHIGH 0x01
+
+#define SKYHIGH_STATUS_ECC_1TO2_BITFLIPS (1 << 4)
+#define SKYHIGH_STATUS_ECC_3TO6_BITFLIPS (2 << 4)
+#define SKYHIGH_STATUS_ECC_UNCOR_ERROR (3 << 4)
+
+#define SKYHIGH_CONFIG_PROTECT_EN BIT(1)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 4, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 2, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+ SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
+ SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int skyhigh_spinand_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section)
+ return -ERANGE;
+
+ /* SkyHigh's ecc parity is stored in the internal hidden area */
+ region->length = 0;
+ region->offset = mtd->oobsize;
+
+ return 0;
+}
+
+static int skyhigh_spinand_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section)
+ return -ERANGE;
+
+ region->length = mtd->oobsize - 2;
+ region->offset = 2;
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops skyhigh_spinand_ooblayout = {
+ .ecc = skyhigh_spinand_ooblayout_ecc,
+ .free = skyhigh_spinand_ooblayout_free,
+};
+
+static int skyhigh_spinand_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ switch (status & STATUS_ECC_MASK) {
+ case STATUS_ECC_NO_BITFLIPS:
+ return 0;
+
+ case SKYHIGH_STATUS_ECC_1TO2_BITFLIPS:
+ return 2;
+
+ case SKYHIGH_STATUS_ECC_3TO6_BITFLIPS:
+ return 6;
+
+ case SKYHIGH_STATUS_ECC_UNCOR_ERROR:
+ return -EBADMSG;;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct spinand_info skyhigh_spinand_table[] = {
+ SPINAND_INFO("S35ML01G301",
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15),
+ NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
+ NAND_ECCREQ(6, 32),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_ON_DIE_ECC_MANDATORY,
+ SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
+ skyhigh_spinand_ecc_get_status)),
+ SPINAND_INFO("S35ML01G300",
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14),
+ NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
+ NAND_ECCREQ(6, 32),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_ON_DIE_ECC_MANDATORY,
+ SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
+ skyhigh_spinand_ecc_get_status)),
+ SPINAND_INFO("S35ML02G300",
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25),
+ NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
+ NAND_ECCREQ(6, 32),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_ON_DIE_ECC_MANDATORY,
+ SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
+ skyhigh_spinand_ecc_get_status)),
+ SPINAND_INFO("S35ML04G300",
+ SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35),
+ NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 2, 1, 1),
+ NAND_ECCREQ(6, 32),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_ON_DIE_ECC_MANDATORY,
+ SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
+ skyhigh_spinand_ecc_get_status)),
+};
+
+static int skyhigh_spinand_init(struct spinand_device *spinand)
+{
+ return spinand_lock_block(spinand, SKYHIGH_CONFIG_PROTECT_EN);
+}
+
+static const struct spinand_manufacturer_ops skyhigh_spinand_manuf_ops = {
+ .init = skyhigh_spinand_init,
+ };
+
+const struct spinand_manufacturer skyhigh_spinand_manufacturer = {
+ .id = SPINAND_MFR_SKYHIGH,
+ .name = "SkyHigh",
+ .chips = skyhigh_spinand_table,
+ .nchips = ARRAY_SIZE(skyhigh_spinand_table),
+ .ops = &skyhigh_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
old mode 100644
new mode 100755
index 5c19ead60499..2856eff28bea
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -268,6 +268,7 @@ extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
extern const struct spinand_manufacturer paragon_spinand_manufacturer;
+extern const struct spinand_manufacturer skyhigh_spinand_manufacturer;
extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
extern const struct spinand_manufacturer xtx_spinand_manufacturer;
@@ -312,6 +313,7 @@ struct spinand_ecc_info {
#define SPINAND_HAS_QE_BIT BIT(0)
#define SPINAND_HAS_CR_FEAT_BIT BIT(1)
+#define SPINAND_ON_DIE_ECC_MANDATORY BIT(2) /* SHM */
/**
* struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure
@@ -518,5 +520,6 @@ int spinand_match_and_init(struct spinand_device *spinand,
int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
int spinand_select_target(struct spinand_device *spinand, unsigned int target);
+int spinand_lock_block(struct spinand_device *spinand, u8 lock); /* SHM */
#endif /* __LINUX_MTD_SPINAND_H */
--
2.34.1
+Takahiro, I saw he was quoted as author
Hi, KR,
Thank you for the patch.
Plese run "./scripts/checkpatch.pl --strict" on your patch and fix the
errors and warning that are shown.
Subject is wrong, it should follow how other drivers were introduced.
You may use the following in the next version:
"mtd: spinand: add support for SkyHigh S35ML flashes"
Other comments below.
On 4/26/24 08:20, KR Kim wrote:
> The following list shows the additional features that are required to support Skyhighmemory S35ML0xG3 SPI Nand:
>
> [Always ECC On]
> Always keep the ECC On during Bad Block Marking and Bad Block Checking
> 1. The on-die ECC feature is totally transparent to the host. The ECC parity bits used for this feature do not occupy the NAND spare areas.
> 2. The host is free to have its own ECC engine by using the spare areas that have standard size.
> 3. We provide this patch to enable users who have limited ECC capabilities on the host side to use the NAND flash. This patch has been tested thoroughly on Linux.
>
> [Change ECC Status information]
> This patch changes the ECC status information as follows to maintain compatibility.
> 00 (normal)
> 01(1-2 errors corrected)
> 10(3-6 errors corrected)
> 11(uncorrectable)
Please read the guide on submitting patches before sending v2:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
It describes how one shall describe the changes. You missed to add your
Signed-of-by tag.
> ---
> drivers/mtd/nand/spi/Makefile | 2 +-
> drivers/mtd/nand/spi/core.c | 14 +++-
> drivers/mtd/nand/spi/skyhigh.c | 145 +++++++++++++++++++++++++++++++++
> include/linux/mtd/spinand.h | 3 +
> 4 files changed, 162 insertions(+), 2 deletions(-)
> mode change 100644 => 100755 drivers/mtd/nand/spi/Makefile
> mode change 100644 => 100755 drivers/mtd/nand/spi/core.c
> create mode 100644 drivers/mtd/nand/spi/skyhigh.c
> mode change 100644 => 100755 include/linux/mtd/spinand.h
all these file mode changes are wrong, keep the files as they were.
>
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> old mode 100644
> new mode 100755
> index 19cc77288ebb..1e61ab21893a
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o gigadevice.o macronix.o
> -spinand-objs += micron.o paragon.o toshiba.o winbond.o xtx.o
> +spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o xtx.o
> obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> old mode 100644
> new mode 100755
> index e0b6715e5dfe..d09b2bd05284
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -196,6 +196,17 @@ static int spinand_init_quad_enable(struct spinand_device *spinand)
> static int spinand_ecc_enable(struct spinand_device *spinand,
> bool enable)
> {
> + /*
> + * SkyHigh Memory : always ECC on
how does the configuration register look like, would you please tell us
its fields?
> + * The on-die ECC feature is totally transparent to the host.
this line brings no benefit, remove it
> + * The ECC parity bits used for this feature do not occupy the NAND spare areas.
its already implied, remove the line
> + * The host is free to have its own ECC engine by using the spare areas that have standard size.
Why would one want to have both the on-die and on-host ecc engine work
in parallel?
> + * We provide this patch to enable users who have limited ECC capabilities on the host side to use the NAND flash.
remove this line, it brings no value
> + * This patch has been tested thoroughly on Linux.
all code is considered tested, remove this line
> + */
> + if (spinand->flags & SPINAND_ON_DIE_ECC_MANDATORY)
> + return 0;
the always on on-die ecc shall be discussed in a dedicated patch, as it
touches the core.
> +
> return spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
> enable ? CFG_ECC_ENABLE : 0);
> }
> @@ -561,7 +572,7 @@ static int spinand_reset_op(struct spinand_device *spinand)
> NULL);
> }
>
> -static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
> +int spinand_lock_block(struct spinand_device *spinand, u8 lock)
> {
> return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
> }
> @@ -945,6 +956,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
> ¯onix_spinand_manufacturer,
> µn_spinand_manufacturer,
> ¶gon_spinand_manufacturer,
> + &skyhigh_spinand_manufacturer,
> &toshiba_spinand_manufacturer,
> &winbond_spinand_manufacturer,
> &xtx_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/skyhigh.c b/drivers/mtd/nand/spi/skyhigh.c
> new file mode 100644
> index 000000000000..f001357b4d85
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/skyhigh.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 SkyHigh Memory Limited
Copyright followed by (c) is redundant. Update years.
> + *
> + * Author: Takahiro Kuwano <[email protected]>
You shall consider adding Takahiro as author, or co-author, or at least
specify that the patch is derived from Takahiro's work.
> + */
> +
cut
> +static int skyhigh_spinand_ooblayout_ecc(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *region)
> +{
> + if (section)
> + return -ERANGE;
> +
> + /* SkyHigh's ecc parity is stored in the internal hidden area */
> + region->length = 0;
what happens when length is zero?
> + region->offset = mtd->oobsize;
> +
> + return 0;
> +}
cut
> +};
> +
> +static int skyhigh_spinand_init(struct spinand_device *spinand)
> +{
> + return spinand_lock_block(spinand, SKYHIGH_CONFIG_PROTECT_EN);
I see the core unlocks all blocks. Thus I assume you can as well remove
the init method altogether, it brings no functional change.
Cheers,
ta