From: Boris Brezillon <bbrezillon@kernel.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Mason Yang <masonccyang@mxic.com.tw>, Vignesh R <vigneshr@ti.com>,
Tudor Ambarus <Tudor.Ambarus@microchip.com>,
Julien Su <juliensu@mxic.com.tw>,
Richard Weinberger <richard@nod.at>,
Schrempf Frieder <frieder.schrempf@kontron.de>,
Marek Vasut <marek.vasut@gmail.com>,
linux-mtd@lists.infradead.org,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Brian Norris <computersforpeace@gmail.com>,
David Woodhouse <dwmw2@infradead.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 24/27] mtd: spinand: Instantiate a SPI-NAND on-die ECC engine
Date: Fri, 22 Feb 2019 15:38:53 +0100 [thread overview]
Message-ID: <20190222153853.3347b253@kernel.org> (raw)
In-Reply-To: <20190221125806.28875-12-miquel.raynal@bootlin.com>
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.
> +}
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2019-02-22 14:39 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 12:57 [RFC PATCH 13/27] mtd: nand: ecc: Clarify the software Hamming introductory line Miquel Raynal
2019-02-21 12:57 ` [RFC PATCH 14/27] mtd: nand: ecc: Turn the software Hamming implementation generic Miquel Raynal
2019-02-21 13:22 ` Boris Brezillon
2019-02-21 12:57 ` [RFC PATCH 15/27] mtd: nand: Remove useless include about software Hamming ECC Miquel Raynal
2019-02-21 12:57 ` [RFC PATCH 16/27] mtd: nand: ecc: Let the software BCH ECC engine be a module Miquel Raynal
2019-02-21 13:48 ` Adam Ford
2019-02-21 14:02 ` Miquel Raynal
2019-02-22 14:24 ` Boris Brezillon
2019-02-21 12:57 ` [RFC PATCH 17/27] mtd: nand: ecc: Let the software Hamming ECC engine be unselected Miquel Raynal
2019-02-21 13:20 ` Boris Brezillon
2019-02-21 13:35 ` Miquel Raynal
2019-02-21 13:41 ` Boris Brezillon
2019-02-21 13:46 ` Miquel Raynal
2019-02-21 12:57 ` [RFC PATCH 18/27] mtd: nand: ecc: Create the software BCH engine instance Miquel Raynal
2019-02-21 12:57 ` [RFC PATCH 19/27] mtd: nand: ecc: Create the software Hamming " Miquel Raynal
2019-02-21 12:57 ` [RFC PATCH 20/27] mtd: nand: Let software ECC engines be retrieved from the NAND core Miquel Raynal
2019-02-22 14:29 ` Boris Brezillon
2019-02-25 15:49 ` Miquel Raynal
2019-02-25 16:13 ` Boris Brezillon
2019-02-26 15:54 ` Miquel Raynal
2019-02-21 12:58 ` [RFC PATCH 21/27] mtd: spinand: Fix typo in comment Miquel Raynal
2019-02-22 14:31 ` Boris Brezillon
2019-02-21 12:58 ` [RFC PATCH 22/27] mtd: spinand: Let the SPI-NAND core flag a SPI-NAND chip Miquel Raynal
2019-02-22 14:33 ` Boris Brezillon
2019-02-21 12:58 ` [RFC PATCH 23/27] mtd: spinand: Move the ECC helper functions into a separate file Miquel Raynal
2019-02-21 12:58 ` [RFC PATCH 24/27] mtd: spinand: Instantiate a SPI-NAND on-die ECC engine Miquel Raynal
2019-02-22 14:38 ` Boris Brezillon [this message]
2019-02-21 12:58 ` [RFC PATCH 25/27] mtd: nand: Add helpers to manage ECC engines and configurations Miquel Raynal
2019-02-22 14:44 ` Boris Brezillon
2019-02-25 16:01 ` Miquel Raynal
2019-02-25 16:34 ` Boris Brezillon
2019-02-25 18:48 ` Boris Brezillon
2019-02-26 15:59 ` [RFC PATCH 25/27] mtd: nand: Add helpers to manage ECC engines and configurationsND Miquel Raynal
2019-02-26 16:04 ` Boris Brezillon
2019-02-27 14:07 ` [RFC PATCH 25/27] mtd: nand: Add helpers to manage ECC engines and configurations Miquel Raynal
2019-02-27 14:30 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190222153853.3347b253@kernel.org \
--to=bbrezillon@kernel.org \
--cc=Tudor.Ambarus@microchip.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=frieder.schrempf@kontron.de \
--cc=juliensu@mxic.com.tw \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=masonccyang@mxic.com.tw \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=thomas.petazzoni@bootlin.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).