diff mbox series

[RFC,24/27] mtd: spinand: Instantiate a SPI-NAND on-die ECC engine

Message ID 20190221125806.28875-12-miquel.raynal@bootlin.com (mailing list archive)
State RFC
Headers show
Series None | expand

Commit Message

Miquel Raynal Feb. 21, 2019, 12:58 p.m. UTC 
Make use of the existing functions taken from the SPI-NAND core to
instantiate an on-die ECC engine specific to the SPI-NAND core. The
next step will be to tweak the core to use this object instead of
calling the helpers directly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/spi/on-die-ecc-engine.c   | 105 +++++++++++++++++++++
 include/linux/mtd/nand-spi-on-die-engine.h |  35 +++++++
 include/linux/mtd/nand.h                   |   1 +
 3 files changed, 141 insertions(+)
 create mode 100644 include/linux/mtd/nand-spi-on-die-engine.h

Comments

Boris Brezillon Feb. 22, 2019, 2:38 p.m. UTC | #1 
On Thu, 21 Feb 2019 13:58:03 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Make use of the existing functions taken from the SPI-NAND core to
> instantiate an on-die ECC engine specific to the SPI-NAND core. The
> next step will be to tweak the core to use this object instead of
> calling the helpers directly.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/spi/on-die-ecc-engine.c   | 105 +++++++++++++++++++++

Maybe on-die-ecc.c is enough. To be honest, given the amount of code,
I'd even prefer to have this code directly in core.c.


>  include/linux/mtd/nand-spi-on-die-engine.h |  35 +++++++

Hm, let's make those defs part of the spinand.h file instead of
creating a new one.


>  include/linux/mtd/nand.h                   |   1 +
>  3 files changed, 141 insertions(+)
>  create mode 100644 include/linux/mtd/nand-spi-on-die-engine.h
> 
> diff --git a/drivers/mtd/nand/spi/on-die-ecc-engine.c b/drivers/mtd/nand/spi/on-die-ecc-engine.c
> index 3d5b83238e14..515b9e3efc93 100644
> --- a/drivers/mtd/nand/spi/on-die-ecc-engine.c
> +++ b/drivers/mtd/nand/spi/on-die-ecc-engine.c
> @@ -45,3 +45,108 @@ int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
>  
>  	return -EINVAL;
>  }
> +
> +static int spinand_noecc_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				       struct mtd_oob_region *region)
> +{
> +	return -ERANGE;
> +}
> +
> +static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section,
> +					struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	/* Reserve 2 bytes for the BBM. */
> +	region->offset = 2;
> +	region->length = 62;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = {
> +	.ecc = spinand_noecc_ooblayout_ecc,
> +	.free = spinand_noecc_ooblayout_free,
> +};
> +
> +static int ecc_spinand_ondie_init_ctx(struct nand_device *nand)
> +{
> +	struct spinand_device *spinand = nand_to_spinand(nand);
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	struct ecc_sw_hamming_conf *engine_conf;
> +
> +	nand->ecc.ctx.conf.step_size = nand->ecc.requirements.step_size;
> +	nand->ecc.ctx.conf.strength = nand->ecc.requirements.strength;
> +
> +	engine_conf = kzalloc(sizeof(*engine_conf), GFP_KERNEL);
> +	if (!engine_conf)
> +		return -ENOMEM;
> +
> +	nand->ecc.ctx.priv = engine_conf;
> +
> +	if (spinand->eccinfo.ooblayout)
> +		mtd_set_ooblayout(mtd, spinand->eccinfo.ooblayout);
> +	else
> +		mtd_set_ooblayout(mtd, &spinand_noecc_ooblayout);
> +
> +	return 0;
> +}
> +
> +static void ecc_spinand_ondie_cleanup_ctx(struct nand_device *nand)
> +{
> +	kfree(nand->ecc.ctx.priv);
> +}
> +
> +static int ecc_spinand_ondie_prepare_io_req(struct nand_device *nand,
> +					    struct nand_page_io_req *req,
> +					    void *oobbuf)
> +{
> +	struct spinand_device *spinand = nand_to_spinand(nand);
> +	bool enable = (req->mode != MTD_OPS_RAW);
> +
> +	/* Only enable or disable the engine */
> +	return spinand_ecc_enable(spinand, enable);
> +}
> +
> +static int ecc_spinand_ondie_finish_io_req(struct nand_device *nand,
> +					   struct nand_page_io_req *req,
> +					   void *oobbuf)
> +{
> +	struct ecc_spinand_ondie_conf *engine_conf = nand->ecc.ctx.priv;
> +	struct spinand_device *spinand = nand_to_spinand(nand);
> +
> +	if (req->mode == MTD_OPS_RAW)
> +		return 0;
> +
> +	/* Nothing to do when finishing a page write */
> +	if (req->type == NAND_PAGE_WRITE)
> +		return 0;
> +
> +	/* Finish a page write: check the status, report errors/bitflips */
> +	return spinand_check_ecc_status(spinand, engine_conf->status);
> +}
> +
> +static struct nand_ecc_engine_ops spinand_ondie_engine_ops = {
> +	.init_ctx = ecc_spinand_ondie_init_ctx,
> +	.cleanup_ctx = ecc_spinand_ondie_cleanup_ctx,
> +	.prepare_io_req = ecc_spinand_ondie_prepare_io_req,
> +	.finish_io_req = ecc_spinand_ondie_finish_io_req,
> +};
> +
> +static struct nand_ecc_engine spinand_ondie_engine = {
> +	.ops = &spinand_ondie_engine_ops,
> +};
> +
> +void spinand_ondie_ecc_save_status(struct nand_device *nand, u8 status)
> +{
> +	struct ecc_spinand_ondie_conf *engine_conf = nand->ecc.ctx.priv;
> +
> +	if (nand->ecc.engine == &spinand_ondie_engine && engine_conf)
> +		engine_conf->status = status;
> +}
> +
> +struct nand_ecc_engine *spinand_ondie_ecc_get_engine(void)
> +{
> +	return &spinand_ondie_engine;

Hm, I was expecting to have one engine instance directly embedded in
spinand_device. The ECC engine is not a global thing here, it's
actually tied to the SPI NAND device itself.

> +}
diff mbox series

Patch

diff --git a/drivers/mtd/nand/spi/on-die-ecc-engine.c b/drivers/mtd/nand/spi/on-die-ecc-engine.c
index 3d5b83238e14..515b9e3efc93 100644
--- a/drivers/mtd/nand/spi/on-die-ecc-engine.c
+++ b/drivers/mtd/nand/spi/on-die-ecc-engine.c
@@ -45,3 +45,108 @@  int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
 
 	return -EINVAL;
 }
+
+static int spinand_noecc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *region)
+{
+	return -ERANGE;
+}
+
+static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	/* Reserve 2 bytes for the BBM. */
+	region->offset = 2;
+	region->length = 62;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = {
+	.ecc = spinand_noecc_ooblayout_ecc,
+	.free = spinand_noecc_ooblayout_free,
+};
+
+static int ecc_spinand_ondie_init_ctx(struct nand_device *nand)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct ecc_sw_hamming_conf *engine_conf;
+
+	nand->ecc.ctx.conf.step_size = nand->ecc.requirements.step_size;
+	nand->ecc.ctx.conf.strength = nand->ecc.requirements.strength;
+
+	engine_conf = kzalloc(sizeof(*engine_conf), GFP_KERNEL);
+	if (!engine_conf)
+		return -ENOMEM;
+
+	nand->ecc.ctx.priv = engine_conf;
+
+	if (spinand->eccinfo.ooblayout)
+		mtd_set_ooblayout(mtd, spinand->eccinfo.ooblayout);
+	else
+		mtd_set_ooblayout(mtd, &spinand_noecc_ooblayout);
+
+	return 0;
+}
+
+static void ecc_spinand_ondie_cleanup_ctx(struct nand_device *nand)
+{
+	kfree(nand->ecc.ctx.priv);
+}
+
+static int ecc_spinand_ondie_prepare_io_req(struct nand_device *nand,
+					    struct nand_page_io_req *req,
+					    void *oobbuf)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	bool enable = (req->mode != MTD_OPS_RAW);
+
+	/* Only enable or disable the engine */
+	return spinand_ecc_enable(spinand, enable);
+}
+
+static int ecc_spinand_ondie_finish_io_req(struct nand_device *nand,
+					   struct nand_page_io_req *req,
+					   void *oobbuf)
+{
+	struct ecc_spinand_ondie_conf *engine_conf = nand->ecc.ctx.priv;
+	struct spinand_device *spinand = nand_to_spinand(nand);
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	/* Nothing to do when finishing a page write */
+	if (req->type == NAND_PAGE_WRITE)
+		return 0;
+
+	/* Finish a page write: check the status, report errors/bitflips */
+	return spinand_check_ecc_status(spinand, engine_conf->status);
+}
+
+static struct nand_ecc_engine_ops spinand_ondie_engine_ops = {
+	.init_ctx = ecc_spinand_ondie_init_ctx,
+	.cleanup_ctx = ecc_spinand_ondie_cleanup_ctx,
+	.prepare_io_req = ecc_spinand_ondie_prepare_io_req,
+	.finish_io_req = ecc_spinand_ondie_finish_io_req,
+};
+
+static struct nand_ecc_engine spinand_ondie_engine = {
+	.ops = &spinand_ondie_engine_ops,
+};
+
+void spinand_ondie_ecc_save_status(struct nand_device *nand, u8 status)
+{
+	struct ecc_spinand_ondie_conf *engine_conf = nand->ecc.ctx.priv;
+
+	if (nand->ecc.engine == &spinand_ondie_engine && engine_conf)
+		engine_conf->status = status;
+}
+
+struct nand_ecc_engine *spinand_ondie_ecc_get_engine(void)
+{
+	return &spinand_ondie_engine;
+}
diff --git a/include/linux/mtd/nand-spi-on-die-engine.h b/include/linux/mtd/nand-spi-on-die-engine.h
new file mode 100644
index 000000000000..33e466b81583
--- /dev/null
+++ b/include/linux/mtd/nand-spi-on-die-engine.h
@@ -0,0 +1,35 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 Macronix
+ * Author:
+ *     Miquèl RAYNAL <miquel.raynal@bootlin.com>
+ *
+ * This file is the header for the SPI-NAND on-die ECC implementation.
+ */
+
+#ifndef __MTD_NAND_SPI_ON_DIE_H__
+#define __MTD_NAND_SPI_ON_DIE_H__
+
+#if IS_ENABLED(CONFIG_MTD_SPI_NAND)
+
+/**
+ * struct ecc_spinand_ondie_conf - private SPI-NAND on-die ECC engine structure
+ * @status: status of the last wait operation that will be used in case
+ *          ->get_status() is not populated by the spinand device.
+ */
+struct ecc_spinand_ondie_conf {
+	u8 status;
+};
+
+struct nand_ecc_engine *spinand_ondie_ecc_get_engine(void);
+void spinand_ondie_ecc_save_status(struct nand_device *nand, u8 status);
+
+#else /* !CONFIG_MTD_SPI_NAND */
+
+static inline struct nand_ecc_engine *spinand_ondie_ecc_get_engine(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_MTD_SPI_NAND */
+
+#endif /* __MTD_NAND_SPI_ON_DIE_H__ */
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3abe113e4f06..534c07fab9de 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -13,6 +13,7 @@ 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/nand-sw-bch-engine.h>
+#include <linux/mtd/nand-spi-on-die-engine.h>
 
 struct nand_device;