| Message ID | 20230115195312.1477845-2-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | Backport BRCMNAND changes from Linux | expand |
Hi Linus,
Unfortunately the u-boot nand base code still uses nand_ecclayout
structure because it was based on old kernel nand driver. Your change
cause bugcheck in the nand_scan_tail at line 4978 when mtd->oobsize is
not one of the default size (i.e. some large nand with BCH-8 ecc
requirement and has 224 bytes oobsize per 4K page) because ecc->layout
is never set. Also certainly any data built based on nand_cclayout like
mtd->oobavail will not be correct.
I actually converted back to nand_ecclayout structure from mtd_ooblayout
with this fix to solve the above issues.
Fixes: e365de90517b ("drivers: nand: brcmnand: fix nand_chip ecc layout
structure")
I can see your point to get the latest from the brcmnand linux kernel
driver but this requires updating the u-boot nand base driver to use
mtd_ooblayout as well and all others nand controller drivers too. I am
not sure if this is something you want to tackle right now.
As far as I can see, all other oob/ecc layout setting patches you back
ported in this series are not in the original brcmstb_choose_ecc_layout
code you replaced. So I am not worried about that we must switch to
mtd_ooblayout_ops at this point. If indeed there is bug in
brcmstb_choose_ecc_layout, we can port and convert the fix to
nand_ecclayout structure from kernel code.
Was the bug you were hunting down in the code related to this patch?
Thanks,
William
On 01/15/2023 11:52 AM, Linus Walleij wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
>
> Implementing the mtd_ooblayout_ops interface is the new way of exposing
> ECC/OOB layout to MTD users.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> [Ported to U-Boot from the Linux kernel]
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 260 ++++++++++++++---------
> 1 file changed, 156 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 74c9348f7fc4..8ea33e861354 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -894,131 +894,183 @@ static inline bool is_hamming_ecc(struct brcmnand_controller *ctrl,
> }
>
> /*
> - * Returns a nand_ecclayout strucutre for the given layout/configuration.
> - * Returns NULL on failure.
> + * Set mtd->ooblayout to the appropriate mtd_ooblayout_ops given
> + * the layout/configuration.
> + * Returns -ERRCODE on failure.
> */
> -static struct nand_ecclayout *brcmnand_create_layout(int ecc_level,
> - struct brcmnand_host *host)
> +static int brcmnand_hamming_ooblayout_ecc(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> {
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct brcmnand_host *host = nand_get_controller_data(chip);
> struct brcmnand_cfg *cfg = &host->hwcfg;
> - int i, j;
> - struct nand_ecclayout *layout;
> - int req;
> - int sectors;
> - int sas;
> - int idx1, idx2;
>
> -#ifndef __UBOOT__
> - layout = devm_kzalloc(&host->pdev->dev, sizeof(*layout), GFP_KERNEL);
> -#else
> - layout = devm_kzalloc(host->pdev, sizeof(*layout), GFP_KERNEL);
> -#endif
> - if (!layout)
> - return NULL;
> -
> - sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> - sas = cfg->spare_area_size << cfg->sector_size_1k;
> -
> - /* Hamming */
> - if (is_hamming_ecc(host->ctrl, cfg)) {
> - for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) {
> - /* First sector of each page may have BBI */
> - if (i == 0) {
> - layout->oobfree[idx2].offset = i * sas + 1;
> - /* Small-page NAND use byte 6 for BBI */
> - if (cfg->page_size == 512)
> - layout->oobfree[idx2].offset--;
> - layout->oobfree[idx2].length = 5;
> - } else {
> - layout->oobfree[idx2].offset = i * sas;
> - layout->oobfree[idx2].length = 6;
> - }
> - idx2++;
> - layout->eccpos[idx1++] = i * sas + 6;
> - layout->eccpos[idx1++] = i * sas + 7;
> - layout->eccpos[idx1++] = i * sas + 8;
> - layout->oobfree[idx2].offset = i * sas + 9;
> - layout->oobfree[idx2].length = 7;
> - idx2++;
> - /* Leave zero-terminated entry for OOBFREE */
> - if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE ||
> - idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1)
> - break;
> - }
> + int sas = cfg->spare_area_size << cfg->sector_size_1k;
> + int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
>
> - return layout;
> - }
> + if (section >= sectors)
> + return -ERANGE;
> + oobregion->offset = (section * sas) + 6;
> + oobregion->length = 3;
>
> - /*
> - * CONTROLLER_VERSION:
> - * < v5.0: ECC_REQ = ceil(BCH_T * 13/8)
> - * >= v5.0: ECC_REQ = ceil(BCH_T * 14/8)
> - * But we will just be conservative.
> - */
> - req = DIV_ROUND_UP(ecc_level * 14, 8);
> - if (req >= sas) {
> - dev_err(host->pdev,
> - "error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n",
> - req, sas);
> - return NULL;
> - }
> + return 0;
> +}
> +
> +static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct brcmnand_host *host = nand_get_controller_data(chip);
> + struct brcmnand_cfg *cfg = &host->hwcfg;
> + int sas = cfg->spare_area_size << cfg->sector_size_1k;
> + int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> +
> + if (section >= sectors * 2)
> + return -ERANGE;
> +
> + oobregion->offset = (section / 2) * sas;
>
> - layout->eccbytes = req * sectors;
> - for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) {
> - for (j = sas - req; j < sas && idx1 <
> - MTD_MAX_ECCPOS_ENTRIES_LARGE; j++, idx1++)
> - layout->eccpos[idx1] = i * sas + j;
> + if (section & 1) {
> + oobregion->offset += 9;
> + oobregion->length = 7;
> + } else {
> + oobregion->length = 6;
>
> /* First sector of each page may have BBI */
> - if (i == 0) {
> - if (cfg->page_size == 512 && (sas - req >= 6)) {
> - /* Small-page NAND use byte 6 for BBI */
> - layout->oobfree[idx2].offset = 0;
> - layout->oobfree[idx2].length = 5;
> - idx2++;
> - if (sas - req > 6) {
> - layout->oobfree[idx2].offset = 6;
> - layout->oobfree[idx2].length =
> - sas - req - 6;
> - idx2++;
> - }
> - } else if (sas > req + 1) {
> - layout->oobfree[idx2].offset = i * sas + 1;
> - layout->oobfree[idx2].length = sas - req - 1;
> - idx2++;
> - }
> - } else if (sas > req) {
> - layout->oobfree[idx2].offset = i * sas;
> - layout->oobfree[idx2].length = sas - req;
> - idx2++;
> + if (!section) {
> + /*
> + * Small-page NAND use byte 6 for BBI while large-page
> + * NAND use byte 0.
> + */
> + if (cfg->page_size > 512)
> + oobregion->offset++;
> + oobregion->length--;
> }
> - /* Leave zero-terminated entry for OOBFREE */
> - if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE ||
> - idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1)
> - break;
> }
>
> - return layout;
> + return 0;
> }
>
> -static struct nand_ecclayout *brcmstb_choose_ecc_layout(
> - struct brcmnand_host *host)
> +static const struct mtd_ooblayout_ops brcmnand_hamming_ooblayout_ops = {
> + .ecc = brcmnand_hamming_ooblayout_ecc,
> + .rfree = brcmnand_hamming_ooblayout_free,
> +};
> +
> +static int brcmnand_bch_ooblayout_ecc(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct brcmnand_host *host = nand_get_controller_data(chip);
> + struct brcmnand_cfg *cfg = &host->hwcfg;
> + int sas = cfg->spare_area_size << cfg->sector_size_1k;
> + int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> +
> + if (section >= sectors)
> + return -ERANGE;
> +
> + oobregion->offset = (section * (sas + 1)) - chip->ecc.bytes;
> + oobregion->length = chip->ecc.bytes;
> +
> + return 0;
> +}
> +
> +static int brcmnand_bch_ooblayout_free_lp(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct brcmnand_host *host = nand_get_controller_data(chip);
> + struct brcmnand_cfg *cfg = &host->hwcfg;
> + int sas = cfg->spare_area_size << cfg->sector_size_1k;
> + int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> +
> + if (section >= sectors)
> + return -ERANGE;
> +
> + if (sas <= chip->ecc.bytes)
> + return 0;
> +
> + oobregion->offset = section * sas;
> + oobregion->length = sas - chip->ecc.bytes;
> +
> + if (!section) {
> + oobregion->offset++;
> + oobregion->length--;
> + }
> +
> + return 0;
> +}
> +
> +static int brcmnand_bch_ooblayout_free_sp(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct brcmnand_host *host = nand_get_controller_data(chip);
> + struct brcmnand_cfg *cfg = &host->hwcfg;
> + int sas = cfg->spare_area_size << cfg->sector_size_1k;
> +
> + if (section > 1 || sas - chip->ecc.bytes < 6 ||
> + (section && sas - chip->ecc.bytes == 6))
> + return -ERANGE;
> +
> + if (!section) {
> + oobregion->offset = 0;
> + oobregion->length = 5;
> + } else {
> + oobregion->offset = 6;
> + oobregion->length = sas - chip->ecc.bytes - 6;
> + }
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops brcmnand_bch_lp_ooblayout_ops = {
> + .ecc = brcmnand_bch_ooblayout_ecc,
> + .rfree = brcmnand_bch_ooblayout_free_lp,
> +};
> +
> +static const struct mtd_ooblayout_ops brcmnand_bch_sp_ooblayout_ops = {
> + .ecc = brcmnand_bch_ooblayout_ecc,
> + .rfree = brcmnand_bch_ooblayout_free_sp,
> +};
> +
> +static int brcmstb_choose_ecc_layout(struct brcmnand_host *host)
> {
> - struct nand_ecclayout *layout;
> struct brcmnand_cfg *p = &host->hwcfg;
> + struct mtd_info *mtd = nand_to_mtd(&host->chip);
> + struct nand_ecc_ctrl *ecc = &host->chip.ecc;
> unsigned int ecc_level = p->ecc_level;
> + int sas = p->spare_area_size << p->sector_size_1k;
> + int sectors = p->page_size / (512 << p->sector_size_1k);
>
> if (p->sector_size_1k)
> ecc_level <<= 1;
>
> - layout = brcmnand_create_layout(ecc_level, host);
> - if (!layout) {
> + if (is_hamming_ecc(host->ctrl, p)) {
> + ecc->bytes = 3 * sectors;
> + mtd_set_ooblayout(mtd, &brcmnand_hamming_ooblayout_ops);
> + return 0;
> + }
> +
> + /*
> + * CONTROLLER_VERSION:
> + * < v5.0: ECC_REQ = ceil(BCH_T * 13/8)
> + * >= v5.0: ECC_REQ = ceil(BCH_T * 14/8)
> + * But we will just be conservative.
> + */
> + ecc->bytes = DIV_ROUND_UP(ecc_level * 14, 8);
> + if (p->page_size == 512)
> + mtd_set_ooblayout(mtd, &brcmnand_bch_sp_ooblayout_ops);
> + else
> + mtd_set_ooblayout(mtd, &brcmnand_bch_lp_ooblayout_ops);
> +
> + if (ecc->bytes >= sas) {
> dev_err(host->pdev,
> - "no proper ecc_layout for this NAND cfg\n");
> - return NULL;
> + "error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n",
> + ecc->bytes, sas);
> + return -EINVAL;
> }
>
> - return layout;
> + return 0;
> }
>
> static void brcmnand_wp(struct mtd_info *mtd, int wp)
> @@ -2329,9 +2381,9 @@ static int brcmnand_init_cs(struct brcmnand_host *host, ofnode dn)
> /* only use our internal HW threshold */
> mtd->bitflip_threshold = 1;
>
> - chip->ecc.layout = brcmstb_choose_ecc_layout(host);
> - if (!chip->ecc.layout)
> - return -ENXIO;
> + ret = brcmstb_choose_ecc_layout(host);
> + if (ret)
> + return ret;
>
> ret = nand_scan_tail(mtd);
> if (ret)
>
On Thu, Jan 26, 2023 at 2:02 AM William Zhang <william.zhang@broadcom.com> wrote: > Unfortunately the u-boot nand base code still uses nand_ecclayout > structure because it was based on old kernel nand driver. Your change > cause bugcheck in the nand_scan_tail at line 4978 when mtd->oobsize is > not one of the default size (i.e. some large nand with BCH-8 ecc > requirement and has 224 bytes oobsize per 4K page) because ecc->layout > is never set. Also certainly any data built based on nand_cclayout like > mtd->oobavail will not be correct. > > I actually converted back to nand_ecclayout structure from mtd_ooblayout > with this fix to solve the above issues. > Fixes: e365de90517b ("drivers: nand: brcmnand: fix nand_chip ecc layout > structure") Argh yeah I see. Let's hold this series off then. It was worth a try! > I can see your point to get the latest from the brcmnand linux kernel > driver but this requires updating the u-boot nand base driver to use > mtd_ooblayout as well and all others nand controller drivers too. I am > not sure if this is something you want to tackle right now. No I can't do that I do not have a big enough experience with NAND flash and no testbed for it either. > As far as I can see, all other oob/ecc layout setting patches you back > ported in this series are not in the original brcmstb_choose_ecc_layout > code you replaced. So I am not worried about that we must switch to > mtd_ooblayout_ops at this point. If indeed there is bug in > brcmstb_choose_ecc_layout, we can port and convert the fix to > nand_ecclayout structure from kernel code. OK no they seemed to be mostly improvements of the algorithm so we can certainly live without. > Was the bug you were hunting down in the code related to this patch? Actually not, I think, it's one of the other patches I sent, the one enabling BCH-1 by reading the proper ECC properties from the device tree. That made it finally work. The iproc NAND driver I sent should also work pretty much as-is, nothing depends on these backports. Yours, Linus Walleij
On 01/26/2023 12:43 AM, Linus Walleij wrote: > On Thu, Jan 26, 2023 at 2:02 AM William Zhang > <william.zhang@broadcom.com> wrote: > >> Unfortunately the u-boot nand base code still uses nand_ecclayout >> structure because it was based on old kernel nand driver. Your change >> cause bugcheck in the nand_scan_tail at line 4978 when mtd->oobsize is >> not one of the default size (i.e. some large nand with BCH-8 ecc >> requirement and has 224 bytes oobsize per 4K page) because ecc->layout >> is never set. Also certainly any data built based on nand_cclayout like >> mtd->oobavail will not be correct. >> >> I actually converted back to nand_ecclayout structure from mtd_ooblayout >> with this fix to solve the above issues. >> Fixes: e365de90517b ("drivers: nand: brcmnand: fix nand_chip ecc layout >> structure") > > Argh yeah I see. Let's hold this series off then. > It was worth a try! > >> I can see your point to get the latest from the brcmnand linux kernel >> driver but this requires updating the u-boot nand base driver to use >> mtd_ooblayout as well and all others nand controller drivers too. I am >> not sure if this is something you want to tackle right now. > > No I can't do that I do not have a big enough experience with NAND > flash and no testbed for it either. > >> As far as I can see, all other oob/ecc layout setting patches you back >> ported in this series are not in the original brcmstb_choose_ecc_layout >> code you replaced. So I am not worried about that we must switch to >> mtd_ooblayout_ops at this point. If indeed there is bug in >> brcmstb_choose_ecc_layout, we can port and convert the fix to >> nand_ecclayout structure from kernel code. > > OK no they seemed to be mostly improvements of the algorithm so we > can certainly live without. > >> Was the bug you were hunting down in the code related to this patch? > > Actually not, I think, it's one of the other patches I sent, the one enabling > BCH-1 by reading the proper ECC properties from the device tree. > That made it finally work. > > The iproc NAND driver I sent should also work pretty much as-is, nothing > depends on these backports. > Yes the patches that are not relate to the ooblayout should still be good to have. > Yours, > Linus Walleij >
Hi William On Thu, Jan 26, 2023 at 6:39 PM William Zhang <william.zhang@broadcom.com> wrote: > > > > On 01/26/2023 12:43 AM, Linus Walleij wrote: > > On Thu, Jan 26, 2023 at 2:02 AM William Zhang > > <william.zhang@broadcom.com> wrote: > > Can you add your review-by? Michael > >> Unfortunately the u-boot nand base code still uses nand_ecclayout > >> structure because it was based on old kernel nand driver. Your change > >> cause bugcheck in the nand_scan_tail at line 4978 when mtd->oobsize is > >> not one of the default size (i.e. some large nand with BCH-8 ecc > >> requirement and has 224 bytes oobsize per 4K page) because ecc->layout > >> is never set. Also certainly any data built based on nand_cclayout like > >> mtd->oobavail will not be correct. > >> > >> I actually converted back to nand_ecclayout structure from mtd_ooblayout > >> with this fix to solve the above issues. > >> Fixes: e365de90517b ("drivers: nand: brcmnand: fix nand_chip ecc layout > >> structure") > > > > Argh yeah I see. Let's hold this series off then. > > It was worth a try! > > > >> I can see your point to get the latest from the brcmnand linux kernel > >> driver but this requires updating the u-boot nand base driver to use > >> mtd_ooblayout as well and all others nand controller drivers too. I am > >> not sure if this is something you want to tackle right now. > > > > No I can't do that I do not have a big enough experience with NAND > > flash and no testbed for it either. > > > >> As far as I can see, all other oob/ecc layout setting patches you back > >> ported in this series are not in the original brcmstb_choose_ecc_layout > >> code you replaced. So I am not worried about that we must switch to > >> mtd_ooblayout_ops at this point. If indeed there is bug in > >> brcmstb_choose_ecc_layout, we can port and convert the fix to > >> nand_ecclayout structure from kernel code. > > > > OK no they seemed to be mostly improvements of the algorithm so we > > can certainly live without. > > > >> Was the bug you were hunting down in the code related to this patch? > > > > Actually not, I think, it's one of the other patches I sent, the one enabling > > BCH-1 by reading the proper ECC properties from the device tree. > > That made it finally work. > > > > The iproc NAND driver I sent should also work pretty much as-is, nothing > > depends on these backports. > > > Yes the patches that are not relate to the ooblayout should still be > good to have. > > > Yours, > > Linus Walleij > >
On Fri, Feb 3, 2023 at 9:48 AM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > On Thu, Jan 26, 2023 at 6:39 PM William Zhang > <william.zhang@broadcom.com> wrote: > > > > > > > > On 01/26/2023 12:43 AM, Linus Walleij wrote: > > > On Thu, Jan 26, 2023 at 2:02 AM William Zhang > > > <william.zhang@broadcom.com> wrote: > > > > > Can you add your review-by? I think maybe I need to rebase the series and take out all the changes that does not relate to patch 1 that changes the way we handle the OOB layout so William can test the result? Yours, Linus Walleij
Hi Linus and Michael, On 02/03/2023 03:10 AM, Linus Walleij wrote: > On Fri, Feb 3, 2023 at 9:48 AM Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: >> On Thu, Jan 26, 2023 at 6:39 PM William Zhang >> <william.zhang@broadcom.com> wrote: >>> >>> >>> >>> On 01/26/2023 12:43 AM, Linus Walleij wrote: >>>> On Thu, Jan 26, 2023 at 2:02 AM William Zhang >>>> <william.zhang@broadcom.com> wrote: >>>> >> >> Can you add your review-by? > > I think maybe I need to rebase the series and take out all the changes > that does not relate to patch 1 that changes the way we handle the > OOB layout so William can test the result? > Yeah I think that is a good idea and I can add my review-by and test-by tag if everything is good. > Yours, > Linus Walleij >
Hi Linus On Fri, Feb 3, 2023 at 6:23 PM William Zhang <william.zhang@broadcom.com> wrote: > > Hi Linus and Michael, > > On 02/03/2023 03:10 AM, Linus Walleij wrote: > > On Fri, Feb 3, 2023 at 9:48 AM Michael Nazzareno Trimarchi > > <michael@amarulasolutions.com> wrote: > >> On Thu, Jan 26, 2023 at 6:39 PM William Zhang > >> <william.zhang@broadcom.com> wrote: > >>> > >>> > >>> > >>> On 01/26/2023 12:43 AM, Linus Walleij wrote: > >>>> On Thu, Jan 26, 2023 at 2:02 AM William Zhang > >>>> <william.zhang@broadcom.com> wrote: > >>>> > >> > >> Can you add your review-by? > > > > I think maybe I need to rebase the series and take out all the changes > > that does not relate to patch 1 that changes the way we handle the > > OOB layout so William can test the result? > > > Yeah I think that is a good idea and I can add my review-by and test-by > tag if everything is good. > > > Yours, > > Linus Walleij > > Can you repost your patchset? Thanks Michael
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 74c9348f7fc4..8ea33e861354 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -894,131 +894,183 @@ static inline bool is_hamming_ecc(struct brcmnand_controller *ctrl, } /* - * Returns a nand_ecclayout strucutre for the given layout/configuration. - * Returns NULL on failure. + * Set mtd->ooblayout to the appropriate mtd_ooblayout_ops given + * the layout/configuration. + * Returns -ERRCODE on failure. */ -static struct nand_ecclayout *brcmnand_create_layout(int ecc_level, - struct brcmnand_host *host) +static int brcmnand_hamming_ooblayout_ecc(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) { + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_cfg *cfg = &host->hwcfg; - int i, j; - struct nand_ecclayout *layout; - int req; - int sectors; - int sas; - int idx1, idx2; -#ifndef __UBOOT__ - layout = devm_kzalloc(&host->pdev->dev, sizeof(*layout), GFP_KERNEL); -#else - layout = devm_kzalloc(host->pdev, sizeof(*layout), GFP_KERNEL); -#endif - if (!layout) - return NULL; - - sectors = cfg->page_size / (512 << cfg->sector_size_1k); - sas = cfg->spare_area_size << cfg->sector_size_1k; - - /* Hamming */ - if (is_hamming_ecc(host->ctrl, cfg)) { - for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) { - /* First sector of each page may have BBI */ - if (i == 0) { - layout->oobfree[idx2].offset = i * sas + 1; - /* Small-page NAND use byte 6 for BBI */ - if (cfg->page_size == 512) - layout->oobfree[idx2].offset--; - layout->oobfree[idx2].length = 5; - } else { - layout->oobfree[idx2].offset = i * sas; - layout->oobfree[idx2].length = 6; - } - idx2++; - layout->eccpos[idx1++] = i * sas + 6; - layout->eccpos[idx1++] = i * sas + 7; - layout->eccpos[idx1++] = i * sas + 8; - layout->oobfree[idx2].offset = i * sas + 9; - layout->oobfree[idx2].length = 7; - idx2++; - /* Leave zero-terminated entry for OOBFREE */ - if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE || - idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1) - break; - } + int sas = cfg->spare_area_size << cfg->sector_size_1k; + int sectors = cfg->page_size / (512 << cfg->sector_size_1k); - return layout; - } + if (section >= sectors) + return -ERANGE; + oobregion->offset = (section * sas) + 6; + oobregion->length = 3; - /* - * CONTROLLER_VERSION: - * < v5.0: ECC_REQ = ceil(BCH_T * 13/8) - * >= v5.0: ECC_REQ = ceil(BCH_T * 14/8) - * But we will just be conservative. - */ - req = DIV_ROUND_UP(ecc_level * 14, 8); - if (req >= sas) { - dev_err(host->pdev, - "error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n", - req, sas); - return NULL; - } + return 0; +} + +static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_cfg *cfg = &host->hwcfg; + int sas = cfg->spare_area_size << cfg->sector_size_1k; + int sectors = cfg->page_size / (512 << cfg->sector_size_1k); + + if (section >= sectors * 2) + return -ERANGE; + + oobregion->offset = (section / 2) * sas; - layout->eccbytes = req * sectors; - for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) { - for (j = sas - req; j < sas && idx1 < - MTD_MAX_ECCPOS_ENTRIES_LARGE; j++, idx1++) - layout->eccpos[idx1] = i * sas + j; + if (section & 1) { + oobregion->offset += 9; + oobregion->length = 7; + } else { + oobregion->length = 6; /* First sector of each page may have BBI */ - if (i == 0) { - if (cfg->page_size == 512 && (sas - req >= 6)) { - /* Small-page NAND use byte 6 for BBI */ - layout->oobfree[idx2].offset = 0; - layout->oobfree[idx2].length = 5; - idx2++; - if (sas - req > 6) { - layout->oobfree[idx2].offset = 6; - layout->oobfree[idx2].length = - sas - req - 6; - idx2++; - } - } else if (sas > req + 1) { - layout->oobfree[idx2].offset = i * sas + 1; - layout->oobfree[idx2].length = sas - req - 1; - idx2++; - } - } else if (sas > req) { - layout->oobfree[idx2].offset = i * sas; - layout->oobfree[idx2].length = sas - req; - idx2++; + if (!section) { + /* + * Small-page NAND use byte 6 for BBI while large-page + * NAND use byte 0. + */ + if (cfg->page_size > 512) + oobregion->offset++; + oobregion->length--; } - /* Leave zero-terminated entry for OOBFREE */ - if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE || - idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1) - break; } - return layout; + return 0; } -static struct nand_ecclayout *brcmstb_choose_ecc_layout( - struct brcmnand_host *host) +static const struct mtd_ooblayout_ops brcmnand_hamming_ooblayout_ops = { + .ecc = brcmnand_hamming_ooblayout_ecc, + .rfree = brcmnand_hamming_ooblayout_free, +}; + +static int brcmnand_bch_ooblayout_ecc(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_cfg *cfg = &host->hwcfg; + int sas = cfg->spare_area_size << cfg->sector_size_1k; + int sectors = cfg->page_size / (512 << cfg->sector_size_1k); + + if (section >= sectors) + return -ERANGE; + + oobregion->offset = (section * (sas + 1)) - chip->ecc.bytes; + oobregion->length = chip->ecc.bytes; + + return 0; +} + +static int brcmnand_bch_ooblayout_free_lp(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_cfg *cfg = &host->hwcfg; + int sas = cfg->spare_area_size << cfg->sector_size_1k; + int sectors = cfg->page_size / (512 << cfg->sector_size_1k); + + if (section >= sectors) + return -ERANGE; + + if (sas <= chip->ecc.bytes) + return 0; + + oobregion->offset = section * sas; + oobregion->length = sas - chip->ecc.bytes; + + if (!section) { + oobregion->offset++; + oobregion->length--; + } + + return 0; +} + +static int brcmnand_bch_ooblayout_free_sp(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_cfg *cfg = &host->hwcfg; + int sas = cfg->spare_area_size << cfg->sector_size_1k; + + if (section > 1 || sas - chip->ecc.bytes < 6 || + (section && sas - chip->ecc.bytes == 6)) + return -ERANGE; + + if (!section) { + oobregion->offset = 0; + oobregion->length = 5; + } else { + oobregion->offset = 6; + oobregion->length = sas - chip->ecc.bytes - 6; + } + + return 0; +} + +static const struct mtd_ooblayout_ops brcmnand_bch_lp_ooblayout_ops = { + .ecc = brcmnand_bch_ooblayout_ecc, + .rfree = brcmnand_bch_ooblayout_free_lp, +}; + +static const struct mtd_ooblayout_ops brcmnand_bch_sp_ooblayout_ops = { + .ecc = brcmnand_bch_ooblayout_ecc, + .rfree = brcmnand_bch_ooblayout_free_sp, +}; + +static int brcmstb_choose_ecc_layout(struct brcmnand_host *host) { - struct nand_ecclayout *layout; struct brcmnand_cfg *p = &host->hwcfg; + struct mtd_info *mtd = nand_to_mtd(&host->chip); + struct nand_ecc_ctrl *ecc = &host->chip.ecc; unsigned int ecc_level = p->ecc_level; + int sas = p->spare_area_size << p->sector_size_1k; + int sectors = p->page_size / (512 << p->sector_size_1k); if (p->sector_size_1k) ecc_level <<= 1; - layout = brcmnand_create_layout(ecc_level, host); - if (!layout) { + if (is_hamming_ecc(host->ctrl, p)) { + ecc->bytes = 3 * sectors; + mtd_set_ooblayout(mtd, &brcmnand_hamming_ooblayout_ops); + return 0; + } + + /* + * CONTROLLER_VERSION: + * < v5.0: ECC_REQ = ceil(BCH_T * 13/8) + * >= v5.0: ECC_REQ = ceil(BCH_T * 14/8) + * But we will just be conservative. + */ + ecc->bytes = DIV_ROUND_UP(ecc_level * 14, 8); + if (p->page_size == 512) + mtd_set_ooblayout(mtd, &brcmnand_bch_sp_ooblayout_ops); + else + mtd_set_ooblayout(mtd, &brcmnand_bch_lp_ooblayout_ops); + + if (ecc->bytes >= sas) { dev_err(host->pdev, - "no proper ecc_layout for this NAND cfg\n"); - return NULL; + "error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n", + ecc->bytes, sas); + return -EINVAL; } - return layout; + return 0; } static void brcmnand_wp(struct mtd_info *mtd, int wp) @@ -2329,9 +2381,9 @@ static int brcmnand_init_cs(struct brcmnand_host *host, ofnode dn) /* only use our internal HW threshold */ mtd->bitflip_threshold = 1; - chip->ecc.layout = brcmstb_choose_ecc_layout(host); - if (!chip->ecc.layout) - return -ENXIO; + ret = brcmstb_choose_ecc_layout(host); + if (ret) + return ret; ret = nand_scan_tail(mtd); if (ret)