[PATCH v13 17/20] mtd: rawnand: Use the ECC framework OOB layouts
Janusz Krzysztofik
jmkrzyszt at gmail.com
Sun Sep 20 08:21:44 EDT 2020
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 at 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);
>
More information about the linux-mtd
mailing list