On 19/04/16 15:41, Boris Brezillon wrote:

On Tue, 19 Apr 2016 15:30:39 +0300
Roger Quadros [off-list ref] wrote:

quoted

On 19/04/16 14:22, Boris Brezillon wrote:

quoted

Hi Roger,

On Tue, 19 Apr 2016 13:28:50 +0300
Roger Quadros [off-list ref] wrote:

quoted

quoted

@@ -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;

Shouldn't this have been
		if (!(nand_chip->options & NAND_BUSWIDTH_16)
			min_oobbytes		= 1;
?

Yep.

quoted

quoted

 		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",

After the above changes BCH with HW ECC worked fine but BCH with SW ECC still failed.
I had to fix it up with the below patch. This is mainly because chip->ecc.steps wasn't
yet initialized before calling nand_bch_init().

After the below patch it worked fine with bch4 (hw & sw), bch8 (hw & sw) and ham1.
I couldn't yet verify bch16 though.

I just verified that bch16 works as well.

quoted

Thanks for the fix, but I'd prefer fixing the bug for all soft BCH
users.

Could you try this patch?

I tried your patch and it worked fine.

Thanks, I'll provide a reworked nand/next branch soon.
BTW, is there anything to fix in my merge commit (the commit merging
your GPMC/OMAP changes in nand/next)?

I just replied in the other thread that the conflict resolution is fine.

quoted

You will still need the below change to omap2.c

--
cheers,
-roger

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 0abfba6..33c8fde 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c

@@ -1715,7 +1715,7 @@ static int omap_sw_ooblayout_free(struct mtd_info *mtd, int section,
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	int off = BADBLOCK_MARKER_LENGTH;
 
-	if (section)
+	if (section >= chip->ecc.steps)
 		return -ERANGE;

Sorry but I don't get why we need that one. Don't we have a single
oobfree section starting at the end of the ECC sections?

You are right. Nothing needs to be changed there then. Thanks :)

cheers,
-roger