| 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: <20160418170518.363f732d@bbrezillon>
Date: Mon, 18 Apr 2016 17:05:18 +0200
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
To: Roger Quadros <rogerq@...com>
Cc: David Woodhouse <dwmw2@...radead.org>,
Brian Norris <computersforpeace@...il.com>,
<linux-mtd@...ts.infradead.org>,
Richard Weinberger <richard@....at>,
<linux-mips@...ux-mips.org>,
Krzysztof Kozlowski <k.kozlowski@...sung.com>,
Harvey Hunt <harvey.hunt@...tec.com>,
Nicolas Ferre <nicolas.ferre@...el.com>,
Stefan Agner <stefan@...er.ch>, <linux-sunxi@...glegroups.com>,
Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
punnaiah choudary kalluri <punnaia@...inx.com>,
Robert Jarzmik <robert.jarzmik@...e.fr>,
<devel@...verdev.osuosl.org>,
Archit Taneja <architt@...eaurora.org>,
<linux-samsung-soc@...r.kernel.org>,
Kamal Dasu <kdasu.kdev@...il.com>,
Josh Wu <rainyfeeling@...look.com>,
Chen-Yu Tsai <wens@...e.org>, Kukjin Kim <kgene@...nel.org>,
<bcm-kernel-feedback-list@...adcom.com>,
Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
Huang Shijie <shijie.huang@....com>,
Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
Haojian Zhuang <haojian.zhuang@...il.com>,
Han Xu <b45815@...escale.com>,
<linux-arm-kernel@...ts.infradead.org>,
Priit Laes <plaes@...es.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
<linux-kernel@...r.kernel.org>, Ralf Baechle <ralf@...ux-mips.org>,
Wenyou Yang <wenyou.yang@...el.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
<linux-api@...r.kernel.org>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Daniel Mack <daniel@...que.org>
Subject: Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops
On Mon, 18 Apr 2016 17:32:49 +0300
Roger Quadros <rogerq@...com> wrote:
> Boris,
>
> On 30/03/16 19:14, Boris Brezillon wrote:
> > 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@...e-electrons.com>
> > ---
> > drivers/mtd/nand/omap2.c | 194 +++++++++++++++++++++++++++--------------------
> > 1 file changed, 113 insertions(+), 81 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index 4ebf16b..bca154a 100644
> > --- a/drivers/mtd/nand/omap2.c
> > +++ b/drivers/mtd/nand/omap2.c
> > @@ -169,8 +169,6 @@ struct omap_nand_info {
> > u_char *buf;
> > int buf_len;
> > struct gpmc_nand_regs reg;
> > - /* generated at runtime depending on ECC algorithm and layout selected */
> > - struct nand_ecclayout oobinfo;
> > /* fields specific for BCHx_HW ECC scheme */
> > struct device *elm_dev;
> > struct device_node *of_node;
> > @@ -1639,19 +1637,108 @@ static bool omap2_nand_ecc_check(struct omap_nand_info *info,
> > return true;
> > }
> >
> > +static int omap_ooblayout_ecc(struct mtd_info *mtd, int section,
> > + struct mtd_oob_region *oobregion)
> > +{
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + int off = chip->options & NAND_BUSWIDTH_16 ?
> > + BADBLOCK_MARKER_LENGTH : 1;
>
> IMO "off = 1" is valid only for OMAP_ECC_HAM1_CODE_HW and 8-bit NAND.
> For all other layouts it is always set to BADBLOCK_MARKER_LENGTH.
Indeed.
[...]
> > - /* all OOB bytes from oobfree->offset till end off OOB are free */
> > - ecclayout->oobfree->length = mtd->oobsize - ecclayout->oobfree->offset;
> > /* check if NAND device's OOB is enough to store ECC signatures */
> > - if (mtd->oobsize < (ecclayout->eccbytes + BADBLOCK_MARKER_LENGTH)) {
> > + min_oobbytes = (oobbytes_per_step *
> > + (mtd->writesize / nand_chip->ecc.size)) +
> > + (nand_chip->options & NAND_BUSWIDTH_16 ?
> > + BADBLOCK_MARKER_LENGTH : 1);
>
> would it affect this as well?
And here as well.
I've generated a patch (see below) fixing those problems.
>
> > + if (mtd->oobsize < min_oobbytes) {
> > dev_err(&info->pdev->dev,
> > "not enough OOB bytes required = %d, available=%d\n",
> > - ecclayout->eccbytes, mtd->oobsize);
> > + min_oobbytes, mtd->oobsize);
> > err = -EINVAL;
> > goto return_error;
> > }
> >
>
> I will need to test this change with all possible configurations.
> The number of configurations on OMAP is a bit overwhelming :(.
Sorry about that, but at least now I have someone who can test it :).
Thanks,
Boris
--->8---
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 9b96f56..07d4039 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1640,9 +1640,13 @@ static bool omap2_nand_ecc_check(struct omap_nand_info *info,
static int omap_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
- int off = chip->options & NAND_BUSWIDTH_16 ?
- BADBLOCK_MARKER_LENGTH : 1;
+ struct omap_nand_info *info = mtd_to_omap(mtd);
+ struct nand_chip *chip = &info->nand;
+ int off = BADBLOCK_MARKER_LENGTH;
+
+ if (info->ecc_opt == OMAP_ECC_HAM1_CODE_HW &&
+ !(chip->options & NAND_BUSWIDTH_16))
+ off = 1;
if (section)
return -ERANGE;
@@ -1656,9 +1660,13 @@ static int omap_ooblayout_ecc(struct mtd_info *mtd, int section,
static int omap_ooblayout_free(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
- int off = chip->options & NAND_BUSWIDTH_16 ?
- BADBLOCK_MARKER_LENGTH : 1;
+ struct omap_nand_info *info = mtd_to_omap(mtd);
+ struct nand_chip *chip = &info->nand;
+ int off = BADBLOCK_MARKER_LENGTH;
+
+ if (info->ecc_opt == OMAP_ECC_HAM1_CODE_HW &&
+ !(chip->options & NAND_BUSWIDTH_16))
+ off = 1;
if (section)
return -ERANGE;
@@ -1682,8 +1690,7 @@ static int omap_sw_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
struct nand_chip *chip = mtd_to_nand(mtd);
- int off = chip->options & NAND_BUSWIDTH_16 ?
- BADBLOCK_MARKER_LENGTH : 1;
+ int off = BADBLOCK_MARKER_LENGTH;
if (section >= chip->ecc.steps)
return -ERANGE;
@@ -1702,8 +1709,7 @@ static int omap_sw_ooblayout_free(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
struct nand_chip *chip = mtd_to_nand(mtd);
- int off = chip->options & NAND_BUSWIDTH_16 ?
- BADBLOCK_MARKER_LENGTH : 1;
+ int off = BADBLOCK_MARKER_LENGTH;
if (section)
return -ERANGE;
@@ -1737,7 +1743,7 @@ static int omap_nand_probe(struct platform_device *pdev)
dma_cap_mask_t mask;
unsigned sig;
struct resource *res;
- int min_oobbytes;
+ int min_oobbytes = BADBLOCK_MARKER_LENGTH;
int oobbytes_per_step;
pdata = dev_get_platdata(&pdev->dev);
@@ -1921,6 +1927,9 @@ static int omap_nand_probe(struct platform_device *pdev)
nand_chip->ecc.correct = omap_correct_data;
mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
oobbytes_per_step = nand_chip->ecc.bytes;
+
+ if (nand_chip->options & NAND_BUSWIDTH_16)
+ min_oobbytes = 1;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -2038,10 +2047,8 @@ static int omap_nand_probe(struct platform_device *pdev)
}
/* check if NAND device's OOB is enough to store ECC signatures */
- min_oobbytes = (oobbytes_per_step *
- (mtd->writesize / nand_chip->ecc.size)) +
- (nand_chip->options & NAND_BUSWIDTH_16 ?
- BADBLOCK_MARKER_LENGTH : 1);
+ min_oobbytes += (oobbytes_per_step *
+ (mtd->writesize / nand_chip->ecc.size));
if (mtd->oobsize < min_oobbytes) {
dev_err(&info->pdev->dev,
"not enough OOB bytes required = %d, available=%d\n",
Powered by blists - more mailing lists