From: Janusz Krzysztofik <jmkrzyszt@gmail.com>
To: Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
Tudor Ambarus <Tudor.Ambarus@microchip.com>,
linux-mtd@lists.infradead.org
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH v13 17/20] mtd: rawnand: Use the ECC framework OOB layouts
Date: Sun, 20 Sep 2020 14:21:44 +0200 [thread overview]
Message-ID: <3339465.iIbC2pHGDl@z50> (raw)
In-Reply-To: <20200827085208.16276-18-miquel.raynal@bootlin.com>
On Thursday, August 27, 2020 10:52:05 A.M. CEST Miquel Raynal wrote:
> No need to have our own in the raw NAND core.
Hi Miquel,
This patch introduced a regression for me, at least on ams-delta which sets
nand_chip->ecc.engine_type to NAND_ECC_ENGINE_TYPE_SOFT and
nand_chip.ecc->algo to NAND_ECC_ALGO_HAMMING. No related kernel messages
could be observed, but nanddump fails with "pread: Numerical result out of
range" for me.
The ECC framework OOB layouts refer to nand_device->ecc.ctx.total which no
driver nor ECC engine initializes so far, while the old layouts used to refer
to nand_chip->ecc.total, still initialized by nand_scan_tail(). Is there any
work in progress which will address the issue (initialization of
nand_device->ecc.ctx.total at least)?
Thanks,
Janusz
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/arasan-nand-controller.c | 2 +-
> drivers/mtd/nand/raw/atmel/nand-controller.c | 2 +-
> drivers/mtd/nand/raw/davinci_nand.c | 3 +-
> .../mtd/nand/raw/ingenic/ingenic_nand_drv.c | 6 +-
> drivers/mtd/nand/raw/nand_base.c | 171 +-----------------
> drivers/mtd/nand/raw/nand_toshiba.c | 2 +-
> drivers/mtd/nand/raw/vf610_nfc.c | 2 +-
> include/linux/mtd/rawnand.h | 4 +-
> 8 files changed, 16 insertions(+), 176 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/
nand/raw/arasan-nand-controller.c
> index 006bc3d3ce20..fbb4ea751be8 100644
> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c
> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c
> @@ -980,7 +980,7 @@ static int anfc_init_hw_ecc_controller(struct arasan_nfc
*nfc,
> return -EINVAL;
> }
>
> - mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
>
> ecc->steps = mtd->writesize / ecc->size;
> ecc->algo = NAND_ECC_ALGO_BCH;
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/
raw/atmel/nand-controller.c
> index 3d162fdc9ce2..7fcc5e67163e 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1108,7 +1108,7 @@ static int atmel_nand_pmecc_init(struct nand_chip
*chip)
>
> chip->options |= NAND_NO_SUBPAGE_WRITE;
>
> - mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
>
> return 0;
> }
> diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/
davinci_nand.c
> index 58966a9706b1..427f320fb79b 100644
> --- a/drivers/mtd/nand/raw/davinci_nand.c
> +++ b/drivers/mtd/nand/raw/davinci_nand.c
> @@ -645,7 +645,8 @@ static int davinci_nand_attach_chip(struct nand_chip
*chip)
> mtd_set_ooblayout(mtd,
>
&hwecc4_small_ooblayout_ops);
> } else if (chunks == 4 || chunks == 8) {
> - mtd_set_ooblayout(mtd,
&nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd,
> +
nand_get_large_page_ooblayout());
> info->chip.ecc.read_page =
nand_davinci_read_page_hwecc_oob_first;
> } else {
> return -EIO;
> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/
nand/raw/ingenic/ingenic_nand_drv.c
> index 70309f18124c..0e9d426fe4f2 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> @@ -243,8 +243,10 @@ static int ingenic_nand_attach_chip(struct nand_chip
*chip)
> /* For legacy reasons we use a different layout on the qi,lb60
board. */
> if (of_machine_is_compatible("qi,lb60"))
> mtd_set_ooblayout(mtd, &qi_lb60_ooblayout_ops);
> - else
> + else if (nfc->soc_info->oob_layout)
> mtd_set_ooblayout(mtd, nfc->soc_info->oob_layout);
> + else
> + mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
>
> return 0;
> }
> @@ -532,7 +534,6 @@ static const struct jz_soc_info jz4740_soc_info = {
> .data_offset = 0x00000000,
> .cmd_offset = 0x00008000,
> .addr_offset = 0x00010000,
> - .oob_layout = &nand_ooblayout_lp_ops,
> };
>
> static const struct jz_soc_info jz4725b_soc_info = {
> @@ -546,7 +547,6 @@ static const struct jz_soc_info jz4780_soc_info = {
> .data_offset = 0x00000000,
> .cmd_offset = 0x00400000,
> .addr_offset = 0x00800000,
> - .oob_layout = &nand_ooblayout_lp_ops,
> };
>
> static const struct of_device_id ingenic_nand_dt_match[] = {
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/
nand_base.c
> index b4ea831ecd85..77f60849dfb4 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -46,166 +46,6 @@
>
> #include "internals.h"
>
> -/* Define default oob placement schemes for large and small page devices */
> -static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
> - struct mtd_oob_region
*oobregion)
> -{
> - struct nand_chip *chip = mtd_to_nand(mtd);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> -
> - if (section > 1)
> - return -ERANGE;
> -
> - if (!section) {
> - oobregion->offset = 0;
> - if (mtd->oobsize == 16)
> - oobregion->length = 4;
> - else
> - oobregion->length = 3;
> - } else {
> - if (mtd->oobsize == 8)
> - return -ERANGE;
> -
> - oobregion->offset = 6;
> - oobregion->length = ecc->total - 4;
> - }
> -
> - return 0;
> -}
> -
> -static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
> - struct mtd_oob_region
*oobregion)
> -{
> - if (section > 1)
> - return -ERANGE;
> -
> - if (mtd->oobsize == 16) {
> - if (section)
> - return -ERANGE;
> -
> - oobregion->length = 8;
> - oobregion->offset = 8;
> - } else {
> - oobregion->length = 2;
> - if (!section)
> - oobregion->offset = 3;
> - else
> - oobregion->offset = 6;
> - }
> -
> - return 0;
> -}
> -
> -const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
> - .ecc = nand_ooblayout_ecc_sp,
> - .free = nand_ooblayout_free_sp,
> -};
> -EXPORT_SYMBOL_GPL(nand_ooblayout_sp_ops);
> -
> -static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
> - struct mtd_oob_region
*oobregion)
> -{
> - struct nand_chip *chip = mtd_to_nand(mtd);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> -
> - if (section || !ecc->total)
> - return -ERANGE;
> -
> - oobregion->length = ecc->total;
> - oobregion->offset = mtd->oobsize - oobregion->length;
> -
> - return 0;
> -}
> -
> -static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
> - struct mtd_oob_region
*oobregion)
> -{
> - struct nand_chip *chip = mtd_to_nand(mtd);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> -
> - if (section)
> - return -ERANGE;
> -
> - oobregion->length = mtd->oobsize - ecc->total - 2;
> - oobregion->offset = 2;
> -
> - return 0;
> -}
> -
> -const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
> - .ecc = nand_ooblayout_ecc_lp,
> - .free = nand_ooblayout_free_lp,
> -};
> -EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops);
> -
> -/*
> - * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
> - * are placed at a fixed offset.
> - */
> -static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section,
> - struct mtd_oob_region
*oobregion)
> -{
> - struct nand_chip *chip = mtd_to_nand(mtd);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> -
> - if (section)
> - return -ERANGE;
> -
> - switch (mtd->oobsize) {
> - case 64:
> - oobregion->offset = 40;
> - break;
> - case 128:
> - oobregion->offset = 80;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - oobregion->length = ecc->total;
> - if (oobregion->offset + oobregion->length > mtd->oobsize)
> - return -ERANGE;
> -
> - return 0;
> -}
> -
> -static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int
section,
> - struct
mtd_oob_region *oobregion)
> -{
> - struct nand_chip *chip = mtd_to_nand(mtd);
> - struct nand_ecc_ctrl *ecc = &chip->ecc;
> - int ecc_offset = 0;
> -
> - if (section < 0 || section > 1)
> - return -ERANGE;
> -
> - switch (mtd->oobsize) {
> - case 64:
> - ecc_offset = 40;
> - break;
> - case 128:
> - ecc_offset = 80;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - if (section == 0) {
> - oobregion->offset = 2;
> - oobregion->length = ecc_offset - 2;
> - } else {
> - oobregion->offset = ecc_offset + ecc->total;
> - oobregion->length = mtd->oobsize - oobregion->offset;
> - }
> -
> - return 0;
> -}
> -
> -static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
> - .ecc = nand_ooblayout_ecc_lp_hamming,
> - .free = nand_ooblayout_free_lp_hamming,
> -};
> -
> static int nand_pairing_dist3_get_info(struct mtd_info *mtd, int page,
> struct mtd_pairing_info
*info)
> {
> @@ -5377,7 +5217,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip
*chip)
> return -EINVAL;
> }
>
> - mtd_set_ooblayout(mtd,
&nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd,
nand_get_large_page_ooblayout());
>
> }
>
> @@ -5386,7 +5226,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip
*chip)
> * used, otherwise we don't know how many bytes can
really be
> * used.
> */
> - if (mtd->ooblayout == &nand_ooblayout_lp_ops &&
> + if (mtd->ooblayout == nand_get_large_page_ooblayout() &&
> ecc->options & NAND_ECC_MAXIMIZE) {
> int steps, bytes;
>
> @@ -5788,11 +5628,12 @@ static int nand_scan_tail(struct nand_chip *chip)
> switch (mtd->oobsize) {
> case 8:
> case 16:
> - mtd_set_ooblayout(mtd,
&nand_ooblayout_sp_ops);
> + mtd_set_ooblayout(mtd,
nand_get_small_page_ooblayout());
> break;
> case 64:
> case 128:
> - mtd_set_ooblayout(mtd,
&nand_ooblayout_lp_hamming_ops);
> + mtd_set_ooblayout(mtd,
> +
nand_get_large_page_hamming_ooblayout());
> break;
> default:
> /*
> @@ -5804,7 +5645,7 @@ static int nand_scan_tail(struct nand_chip *chip)
> */
> if (ecc->engine_type ==
NAND_ECC_ENGINE_TYPE_NONE) {
> mtd_set_ooblayout(mtd,
> -
&nand_ooblayout_lp_ops);
> +
nand_get_large_page_ooblayout());
> break;
> }
>
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/
nand_toshiba.c
> index fb6662337b90..cf4f37959421 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -140,7 +140,7 @@ static void toshiba_nand_benand_init(struct nand_chip
*chip)
>
> chip->options |= NAND_SUBPAGE_READ;
>
> - mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
> }
>
> static void toshiba_nand_decode_id(struct nand_chip *chip)
> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/
vf610_nfc.c
> index 8ee2c1f539c4..50dc0c93140c 100644
> --- a/drivers/mtd/nand/raw/vf610_nfc.c
> +++ b/drivers/mtd/nand/raw/vf610_nfc.c
> @@ -779,7 +779,7 @@ static int vf610_nfc_attach_chip(struct nand_chip *chip)
> mtd->oobsize = 64;
>
> /* Use default large page ECC layout defined in NAND core */
> - mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> + mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
> if (chip->ecc.strength == 32) {
> nfc->ecc_mode = ECC_60_BYTE;
> chip->ecc.bytes = 60;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 967b616c50df..1bbce6fa1b08 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -14,6 +14,7 @@
> #define __LINUX_MTD_RAWNAND_H
>
> #include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand.h>
> #include <linux/mtd/flashchip.h>
> #include <linux/mtd/bbm.h>
> #include <linux/mtd/jedec.h>
> @@ -1156,9 +1157,6 @@ struct nand_chip {
> void *priv;
> };
>
> -extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
> -extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops;
> -
> static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
> {
> return container_of(mtd, struct nand_chip, base.mtd);
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-09-20 12:23 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-27 8:51 [PATCH v13 00/20] Introduction of the generic ECC framework Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 01/20] mtd: rawnand: Add a kernel doc to the ECC algorithm enumeration Miquel Raynal
2020-09-07 7:25 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 02/20] mtd: rawnand: Rename the ECC algorithm enumeration items Miquel Raynal
2020-09-07 7:25 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 03/20] mtd: rawnand: Move the nand_ecc_algo enum to the generic NAND layer Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 04/20] mtd: nand: Add a NAND page I/O request type Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 05/20] dt-bindings: mtd: Document nand-ecc-placement Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 06/20] dt-bindings: mtd: Document nand-ecc-engine Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 07/20] dt-bindings: mtd: Document boolean NAND ECC properties Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 08/20] mtd: nand: Introduce the ECC engine framework Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 09/20] mtd: rawnand: Separate the ECC engine type and the ECC byte placement Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-10-14 21:36 ` Han Xu
2020-10-15 1:35 ` Fabio Estevam
2020-10-15 3:09 ` Han Xu
2020-10-15 7:25 ` Miquel Raynal
2020-10-16 17:09 ` Han Xu
2020-10-16 17:14 ` Miquel Raynal
2020-10-16 17:51 ` Han Xu
2020-10-16 18:00 ` Miquel Raynal
2020-10-16 21:03 ` Fabio Estevam
2020-10-19 15:51 ` Han Xu
2020-10-19 16:52 ` Fabio Estevam
2020-08-27 8:51 ` [PATCH v13 10/20] mtd: rawnand: Use the new ECC engine type enumeration Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:51 ` [PATCH v13 11/20] mtd: nand: Create a helper to extract the ECC configuration Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 12/20] mtd: spinand: Use nanddev_get_ecc_conf() when relevant Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 13/20] mtd: nand: Create helpers to set/extract the ECC requirements Miquel Raynal
2020-09-07 7:24 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 14/20] mtd: rawnand: Use nanddev_get/set_ecc_requirements() when relevant Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 15/20] mtd: nand: Use the new generic ECC object Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 16/20] mtd: rawnand: Make use of the ECC framework Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 17/20] mtd: rawnand: Use the ECC framework OOB layouts Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-09-20 12:21 ` Janusz Krzysztofik [this message]
2020-09-29 8:15 ` Miquel Raynal
2020-09-29 21:26 ` Janusz Krzysztofik
2020-08-27 8:52 ` [PATCH v13 18/20] mtd: rawnand: Use the ECC framework nand_ecc_is_strong_enough() helper Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 19/20] mtd: rawnand: Use the ECC framework user input parsing bits Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
2020-09-21 9:36 ` Sascha Hauer
2020-09-28 15:18 ` Miquel Raynal
2020-08-27 8:52 ` [PATCH v13 20/20] mtd: rawnand: Use the NAND framework user_conf object for ECC flags Miquel Raynal
2020-09-07 7:23 ` Miquel Raynal
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=3339465.iIbC2pHGDl@z50 \
--to=jmkrzyszt@gmail.com \
--cc=Tudor.Ambarus@microchip.com \
--cc=linux-mtd@lists.infradead.org \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--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).