On Fri, 15 Apr 2016 21:54:10 +0200 Rafał Miłecki <[email protected]> wrote:
> Now we have all drivers properly setting this new field we can start > using it and proceed with deprecating NAND_ECC_SOFT_BCH. > > Signed-off-by: Rafał Miłecki <[email protected]> > --- > drivers/mtd/nand/nand_base.c | 98 > ++++++++++++++++++++++++-------------------- > 1 file changed, 53 insertions(+), 45 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index e1f3cf8..ffd1b32 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd) > /* > * If no default placement scheme is given, select an appropriate one. > */ > - if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) { > + if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) { Should be: if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT && ecc->algo != NAND_ECC_BCH) { Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into account. > switch (mtd->oobsize) { > case 8: > case 16: > @@ -4248,51 +4248,59 @@ int nand_scan_tail(struct mtd_info *mtd) > ecc->algo = NAND_ECC_HAMMING; > > case NAND_ECC_SOFT: > - ecc->calculate = nand_calculate_ecc; > - ecc->correct = nand_correct_data; > - ecc->read_page = nand_read_page_swecc; > - ecc->read_subpage = nand_read_subpage; > - ecc->write_page = nand_write_page_swecc; > - ecc->read_page_raw = nand_read_page_raw; > - ecc->write_page_raw = nand_write_page_raw; > - ecc->read_oob = nand_read_oob_std; > - ecc->write_oob = nand_write_oob_std; > - if (!ecc->size) > - ecc->size = 256; > - ecc->bytes = 3; > - ecc->strength = 1; > - break; > - > case NAND_ECC_SOFT_BCH: Shouldn't we drop this case? > - if (!mtd_nand_has_bch()) { > - WARN(1, "CONFIG_MTD_NAND_ECC_BCH not enabled\n"); > - ret = -EINVAL; > - goto err_free; > - } > - ecc->calculate = nand_bch_calculate_ecc; > - ecc->correct = nand_bch_correct_data; > - ecc->read_page = nand_read_page_swecc; > - ecc->read_subpage = nand_read_subpage; > - ecc->write_page = nand_write_page_swecc; > - ecc->read_page_raw = nand_read_page_raw; > - ecc->write_page_raw = nand_write_page_raw; > - ecc->read_oob = nand_read_oob_std; > - ecc->write_oob = nand_write_oob_std; > - /* > - * Board driver should supply ecc.size and ecc.strength values > - * to select how many bits are correctable. Otherwise, default > - * to 4 bits for large page devices. > - */ > - if (!ecc->size && (mtd->oobsize >= 64)) { > - ecc->size = 512; > - ecc->strength = 4; > - } > + switch (ecc->algo) { Please put this logic in a sub-function, nand_scan_tail() is already quite long and hard to read, let's try to not define sub switch-case blocks inside existing ones. > + case NAND_ECC_HAMMING: > + ecc->calculate = nand_calculate_ecc; > + ecc->correct = nand_correct_data; > + ecc->read_page = nand_read_page_swecc; > + ecc->read_subpage = nand_read_subpage; > + ecc->write_page = nand_write_page_swecc; > + ecc->read_page_raw = nand_read_page_raw; > + ecc->write_page_raw = nand_write_page_raw; > + ecc->read_oob = nand_read_oob_std; > + ecc->write_oob = nand_write_oob_std; > + if (!ecc->size) > + ecc->size = 256; > + ecc->bytes = 3; > + ecc->strength = 1; > + break; > + case NAND_ECC_BCH: > + if (!mtd_nand_has_bch()) { > + WARN(1, "CONFIG_MTD_NAND_ECC_BCH not > enabled\n"); > + ret = -EINVAL; > + goto err_free; > + } > + ecc->calculate = nand_bch_calculate_ecc; > + ecc->correct = nand_bch_correct_data; > + ecc->read_page = nand_read_page_swecc; > + ecc->read_subpage = nand_read_subpage; > + ecc->write_page = nand_write_page_swecc; > + ecc->read_page_raw = nand_read_page_raw; > + ecc->write_page_raw = nand_write_page_raw; > + ecc->read_oob = nand_read_oob_std; > + ecc->write_oob = nand_write_oob_std; > + /* > + * Board driver should supply ecc.size and ecc.strength > + * values to select how many bits are correctable. > + * Otherwise, default to 4 bits for large page devices. > + */ > + if (!ecc->size && (mtd->oobsize >= 64)) { > + ecc->size = 512; > + ecc->strength = 4; > + } > > - /* See nand_bch_init() for details. */ > - ecc->bytes = 0; > - ecc->priv = nand_bch_init(mtd); > - if (!ecc->priv) { > - WARN(1, "BCH ECC initialization failed!\n"); > + /* See nand_bch_init() for details. */ > + ecc->bytes = 0; > + ecc->priv = nand_bch_init(mtd); > + if (!ecc->priv) { > + WARN(1, "BCH ECC initialization failed!\n"); > + ret = -EINVAL; > + goto err_free; > + } > + break; > + default: > + WARN(1, "Unsupported ECC algorithm!\n"); > ret = -EINVAL; > goto err_free; > } > @@ -4478,7 +4486,7 @@ void nand_release(struct mtd_info *mtd) > { > struct nand_chip *chip = mtd_to_nand(mtd); > > - if (chip->ecc.mode == NAND_ECC_SOFT_BCH) > + if (chip->ecc.algo == NAND_ECC_BCH) Again, should be: if (chip->ecc.mode = NAND_ECC_SOFT && chip->ecc.algo == NAND_ECC_BCH) > nand_bch_free((struct nand_bch_control *)chip->ecc.priv); > > mtd_device_unregister(mtd); -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
