| Message ID | 1335295105-7981-7-git-send-email-mikedunn@newsguy.com |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mike, On Tue, Apr 24, 2012 at 12:18 PM, Mike Dunn <mikedunn@newsguy.com> wrote: > This patch adds sanity checks that ensure that drivers for controllers with > hardware ECC set the 'strength' element in struct nand_ecc_ctrl. Also stylistic > changes to the line that calculates strength for software ECC. Both suggested > during discussion [1]. > > [1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040467.html Thanks for addressing these issues! Note my comment below though. > --- > drivers/mtd/nand/nand_base.c | 28 ++++++++++++++++++++++------ > 1 files changed, 22 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 3137abd..275c43f 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -3302,10 +3302,21 @@ int nand_scan_tail(struct mtd_info *mtd) > "hardware ECC not possible\n"); > BUG(); > } > + > + if (!chip->ecc.strength) { > + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); > + BUG(); > + } > + > if (!chip->ecc.read_page) > chip->ecc.read_page = nand_read_page_hwecc_oob_first; > > case NAND_ECC_HW: > + if (!chip->ecc.strength) { > + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); > + BUG(); > + } > + > /* Use standard hwecc read page function? */ > if (!chip->ecc.read_page) > chip->ecc.read_page = nand_read_page_hwecc; > @@ -3345,12 +3356,17 @@ int nand_scan_tail(struct mtd_info *mtd) > if (!chip->ecc.write_oob) > chip->ecc.write_oob = nand_write_oob_syndrome; > > - if (mtd->writesize >= chip->ecc.size) > - break; > - pr_warn("%d byte HW ECC not possible on " > - "%d byte page size, fallback to SW ECC\n", > - chip->ecc.size, mtd->writesize); > + if (mtd->writesize >= chip->ecc.size) { > + if (!chip->ecc.strength) { > + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); > + BUG(); > + } > + break; /* all's well */ > + } > + pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n", > + chip->ecc.size, mtd->writesize); > chip->ecc.mode = NAND_ECC_SOFT; > + /* fall through */ > > case NAND_ECC_SOFT: > chip->ecc.calculate = nand_calculate_ecc; I think you only need this last check (under ECC_HW_SYNDROME), since the ECC_HW and ECC_HW_OOB_FIRST cases necessarily fall through to ECC_HW_SYNDROME case. (Note the lack of 'break;' statements in the first two 'case's.) Brian
On 04/24/2012 09:09 PM, Brian Norris wrote: > > I think you only need this last check (under ECC_HW_SYNDROME), since > the ECC_HW and ECC_HW_OOB_FIRST cases necessarily fall through to > ECC_HW_SYNDROME case. (Note the lack of 'break;' statements in the > first two 'case's.) Oops... you're right, I did miss that. This should have been a much simpler patch. Thanks! Mike
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 3137abd..275c43f 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3302,10 +3302,21 @@ int nand_scan_tail(struct mtd_info *mtd) "hardware ECC not possible\n"); BUG(); } + + if (!chip->ecc.strength) { + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); + BUG(); + } + if (!chip->ecc.read_page) chip->ecc.read_page = nand_read_page_hwecc_oob_first; case NAND_ECC_HW: + if (!chip->ecc.strength) { + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); + BUG(); + } + /* Use standard hwecc read page function? */ if (!chip->ecc.read_page) chip->ecc.read_page = nand_read_page_hwecc; @@ -3345,12 +3356,17 @@ int nand_scan_tail(struct mtd_info *mtd) if (!chip->ecc.write_oob) chip->ecc.write_oob = nand_write_oob_syndrome; - if (mtd->writesize >= chip->ecc.size) - break; - pr_warn("%d byte HW ECC not possible on " - "%d byte page size, fallback to SW ECC\n", - chip->ecc.size, mtd->writesize); + if (mtd->writesize >= chip->ecc.size) { + if (!chip->ecc.strength) { + pr_warn("Driver must set ecc.strength when using hardware ECC\n"); + BUG(); + } + break; /* all's well */ + } + pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n", + chip->ecc.size, mtd->writesize); chip->ecc.mode = NAND_ECC_SOFT; + /* fall through */ case NAND_ECC_SOFT: chip->ecc.calculate = nand_calculate_ecc; @@ -3401,7 +3417,7 @@ int nand_scan_tail(struct mtd_info *mtd) BUG(); } chip->ecc.strength = - chip->ecc.bytes*8 / fls(8*chip->ecc.size); + chip->ecc.bytes * 8 / fls(8 * chip->ecc.size); break; case NAND_ECC_NONE:
This patch adds sanity checks that ensure that drivers for controllers with hardware ECC set the 'strength' element in struct nand_ecc_ctrl. Also stylistic changes to the line that calculates strength for software ECC. Both suggested during discussion [1]. [1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040467.html Signed-off-by: Mike Dunn <mikedunn@newsguy.com> --- drivers/mtd/nand/nand_base.c | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-)