| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160416095807.112d2f59@bbrezillon>
Date: Sat, 16 Apr 2016 09:58:07 +0200
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
To: Rafał Miłecki <zajec5@...il.com>
Cc: linux-mtd@...ts.infradead.org, Richard Weinberger <richard@....at>,
David Woodhouse <dwmw2@...radead.org>,
Brian Norris <computersforpeace@...il.com>,
linux-kernel@...r.kernel.org (open list)
Subject: Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field
On Fri, 15 Apr 2016 21:54:10 +0200
Rafał Miłecki <zajec5@...il.com> 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 <zajec5@...il.com>
> ---
> 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
Powered by blists - more mailing lists