Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1381768 > unrolled thread

Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops

Started byRoger Quadros <rogerq@ti.com>
First post2016-04-18 16:40 +0200
Last post2016-04-19 15:00 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Roger Quadros <rogerq@ti.com> - 2016-04-18 16:40 +0200
    Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-18 17:10 +0200
      Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Roger Quadros <rogerq@ti.com> - 2016-04-19 12:40 +0200
        Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-19 13:30 +0200
          Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Roger Quadros <rogerq@ti.com> - 2016-04-19 14:40 +0200
            Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-19 14:50 +0200
              Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops Roger Quadros <rogerq@ti.com> - 2016-04-19 15:00 +0200

#1381768 — Re: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops

FromRoger Quadros <rogerq@ti.com>
Date2016-04-18 16:40 +0200
SubjectRe: [PATCH v5 39/50] mtd: nand: omap2: switch to mtd_ooblayout_ops
Message-ID<rpivh-79W-45@gated-at.bofh.it>
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@free-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.

> +
> +	if (section)
> +		return -ERANGE;
> +
> +	oobregion->offset = off;
> +	oobregion->length = chip->ecc.total;
> +
> +	return 0;
> +}
> +
> +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;

ditto.

> +
> +	if (section)
> +		return -ERANGE;
> +
> +	off += chip->ecc.total;
> +	if (off >= mtd->oobsize)
> +		return -ERANGE;
> +
> +	oobregion->offset = off;
> +	oobregion->length = mtd->oobsize - off;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops omap_ooblayout_ops = {
> +	.ecc = omap_ooblayout_ecc,
> +	.free = omap_ooblayout_free,
> +};
> +
> +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;
> +
> +	if (section >= chip->ecc.steps)
> +		return -ERANGE;
> +
> +	/*
> +	 * When SW correction is employed, one OMAP specific marker byte is
> +	 * reserved after each ECC step.
> +	 */
> +	oobregion->offset = off + (section * (chip->ecc.bytes + 1));
> +	oobregion->length = chip->ecc.bytes;
> +
> +	return 0;
> +}
> +
> +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;
> +
> +	if (section)
> +		return -ERANGE;
> +
> +	/*
> +	 * When SW correction is employed, one OMAP specific marker byte is
> +	 * reserved after each ECC step.
> +	 */
> +	off += ((chip->ecc.bytes + 1) * chip->ecc.steps);
> +	if (off >= mtd->oobsize)
> +		return -ERANGE;
> +
> +	oobregion->offset = off;
> +	oobregion->length = mtd->oobsize - off;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
> +	.ecc = omap_sw_ooblayout_ecc,
> +	.free = omap_sw_ooblayout_free,
> +};
> +
>  static int omap_nand_probe(struct platform_device *pdev)
>  {
>  	struct omap_nand_info		*info;
>  	struct omap_nand_platform_data	*pdata;
>  	struct mtd_info			*mtd;
>  	struct nand_chip		*nand_chip;
> -	struct nand_ecclayout		*ecclayout;
>  	int				err;
> -	int				i;
>  	dma_cap_mask_t			mask;
>  	unsigned			sig;
> -	unsigned			oob_index;
>  	struct resource			*res;
> +	int				min_oobbytes;
> +	int				oobbytes_per_step;
>  
>  	pdata = dev_get_platdata(&pdev->dev);
>  	if (pdata == NULL) {
> @@ -1810,7 +1897,7 @@ static int omap_nand_probe(struct platform_device *pdev)
>  
>  	/*
>  	 * Bail out earlier to let NAND_ECC_SOFT code create its own
> -	 * ecclayout instead of using ours.
> +	 * ooblayout instead of using ours.
>  	 */
>  	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
>  		nand_chip->ecc.mode = NAND_ECC_SOFT;
> @@ -1818,8 +1905,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	/* populate MTD interface based on ECC scheme */
> -	ecclayout		= &info->oobinfo;
> -	nand_chip->ecc.layout	= ecclayout;
>  	switch (info->ecc_opt) {
>  	case OMAP_ECC_HAM1_CODE_HW:
>  		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
> @@ -1830,19 +1915,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.calculate        = omap_calculate_ecc;
>  		nand_chip->ecc.hwctl            = omap_enable_hwecc;
>  		nand_chip->ecc.correct          = omap_correct_data;
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		if (nand_chip->options & NAND_BUSWIDTH_16)
> -			oob_index		= BADBLOCK_MARKER_LENGTH;
> -		else
> -			oob_index		= 1;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)
> -			ecclayout->eccpos[i]	= oob_index;
> -		/* no reserved-marker in ecclayout for this ecc-scheme */
> -		ecclayout->oobfree->offset	=
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step		= nand_chip->ecc.bytes;
>  		break;
>  
>  	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> @@ -1854,19 +1928,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
>  		nand_chip->ecc.correct		= nand_bch_correct_data;
>  		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		oob_index			= BADBLOCK_MARKER_LENGTH;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {
> -			ecclayout->eccpos[i] = oob_index;
> -			if (((i + 1) % nand_chip->ecc.bytes) == 0)
> -				oob_index++;
> -		}
> -		/* include reserved-marker in ecclayout->oobfree calculation */
> -		ecclayout->oobfree->offset	= 1 +
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step		= nand_chip->ecc.bytes + 1;
>  		/* software bch library is used for locating errors */
>  		nand_chip->ecc.priv		= nand_bch_init(mtd);
>  		if (!nand_chip->ecc.priv) {
> @@ -1888,16 +1952,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
>  		nand_chip->ecc.read_page	= omap_read_page_bch;
>  		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		oob_index			= BADBLOCK_MARKER_LENGTH;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)
> -			ecclayout->eccpos[i]	= oob_index;
> -		/* reserved marker already included in ecclayout->eccbytes */
> -		ecclayout->oobfree->offset	=
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step		= nand_chip->ecc.bytes;
>  
>  		err = elm_config(info->elm_dev, BCH4_ECC,
>  				 mtd->writesize / nand_chip->ecc.size,
> @@ -1915,19 +1971,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
>  		nand_chip->ecc.correct		= nand_bch_correct_data;
>  		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		oob_index			= BADBLOCK_MARKER_LENGTH;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {
> -			ecclayout->eccpos[i] = oob_index;
> -			if (((i + 1) % nand_chip->ecc.bytes) == 0)
> -				oob_index++;
> -		}
> -		/* include reserved-marker in ecclayout->oobfree calculation */
> -		ecclayout->oobfree->offset	= 1 +
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step		= nand_chip->ecc.bytes + 1;
>  		/* software bch library is used for locating errors */
>  		nand_chip->ecc.priv		= nand_bch_init(mtd);
>  		if (!nand_chip->ecc.priv) {
> @@ -1949,6 +1995,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
>  		nand_chip->ecc.read_page	= omap_read_page_bch;
>  		nand_chip->ecc.write_page	= omap_write_page_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step		= nand_chip->ecc.bytes;
>  
>  		err = elm_config(info->elm_dev, BCH8_ECC,
>  				 mtd->writesize / nand_chip->ecc.size,
> @@ -1956,16 +2004,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		if (err < 0)
>  			goto return_error;
>  
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		oob_index			= BADBLOCK_MARKER_LENGTH;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)
> -			ecclayout->eccpos[i]	= oob_index;
> -		/* reserved marker already included in ecclayout->eccbytes */
> -		ecclayout->oobfree->offset	=
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
>  		break;
>  
>  	case OMAP_ECC_BCH16_CODE_HW:
> @@ -1979,6 +2017,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
>  		nand_chip->ecc.read_page	= omap_read_page_bch;
>  		nand_chip->ecc.write_page	= omap_write_page_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step		= nand_chip->ecc.bytes;
>  
>  		err = elm_config(info->elm_dev, BCH16_ECC,
>  				 mtd->writesize / nand_chip->ecc.size,
> @@ -1986,16 +2026,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		if (err < 0)
>  			goto return_error;
>  
> -		/* define ECC layout */
> -		ecclayout->eccbytes		= nand_chip->ecc.bytes *
> -							(mtd->writesize /
> -							nand_chip->ecc.size);
> -		oob_index			= BADBLOCK_MARKER_LENGTH;
> -		for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)
> -			ecclayout->eccpos[i]	= oob_index;
> -		/* reserved marker already included in ecclayout->eccbytes */
> -		ecclayout->oobfree->offset	=
> -				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
>  		break;
>  	default:
>  		dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
> @@ -2003,13 +2033,15 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		goto return_error;
>  	}
>  
> -	/* 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?

> +	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 :(.

cheers,
-roger

[toc] | [next] | [standalone]

#1381794

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-18 17:10 +0200
Message-ID<rpiYh-7Ft-1@gated-at.bofh.it>
In reply to#1381768
On Mon, 18 Apr 2016 17:32:49 +0300
Roger Quadros <rogerq@ti.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@free-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",

[toc] | [prev] | [next] | [standalone]

#1382385

FromRoger Quadros <rogerq@ti.com>
Date2016-04-19 12:40 +0200
Message-ID<rpBez-5pW-29@gated-at.bofh.it>
In reply to#1381794
On 18/04/16 18:05, Boris Brezillon wrote:
> On Mon, 18 Apr 2016 17:32:49 +0300
> Roger Quadros <rogerq@ti.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@free-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;

Shouldn't this have been
		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",
> 

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.

--cheers,
-roger

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index db22c01..1b1a804 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1692,7 +1692,7 @@ static int omap_sw_ooblayout_ecc(struct mtd_info *mtd, int section,
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	int off = BADBLOCK_MARKER_LENGTH;
 
-	if (section >= chip->ecc.steps)
+	if (section >= mtd->writesize / chip->ecc.size)	/* ecc steps */
 		return -ERANGE;
 
 	/*
@@ -1711,7 +1711,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 >= mtd->writesize / chip->ecc.size) /* ecc steps */
 		return -ERANGE;
 
 	/*
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1382410

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-19 13:30 +0200
Message-ID<rpC0X-64Y-21@gated-at.bofh.it>
In reply to#1382385
Hi Roger,

On Tue, 19 Apr 2016 13:28:50 +0300
Roger Quadros <rogerq@ti.com> wrote:

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

> 
> >  		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.

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

Could you try this patch?

--->8---

diff --git a/drivers/mtd/nand/nand_bch.c b/drivers/mtd/nand/nand_bch.c
index ca9b2a4..3ca3d39 100644
--- a/drivers/mtd/nand/nand_bch.c
+++ b/drivers/mtd/nand/nand_bch.c
@@ -177,6 +177,16 @@ struct nand_bch_control *nand_bch_init(struct mtd_info *mtd)
 		goto fail;
 	}
 
+	/*
+	 * ecc->steps and ecc->total might be used by mtd->ooblayout->ecc(),
+	 * which is called by mtd_ooblayout_count_eccbytes().
+	 * Make sure they are properly initialized before calling
+	 * mtd_ooblayout_count_eccbytes().
+	 * FIXME: we should probaly rework the sequencing in nand_scan_tail()
+	 * to avoid setting those fields twice.
+	 */
+	nand->ecc.steps = eccsteps;
+	nand->ecc.total = eccsteps * eccbytes;
 	if (mtd_ooblayout_count_eccbytes(mtd) != (eccsteps*eccbytes)) {
 		printk(KERN_WARNING "invalid ecc layout\n");
 		goto fail;

[toc] | [prev] | [next] | [standalone]

#1382456

FromRoger Quadros <rogerq@ti.com>
Date2016-04-19 14:40 +0200
Message-ID<rpD6G-6Tc-37@gated-at.bofh.it>
In reply to#1382410
On 19/04/16 14:22, Boris Brezillon wrote:
> Hi Roger,
> 
> On Tue, 19 Apr 2016 13:28:50 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
>>> @@ -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.
> 
>>
>>>  		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.

> 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.
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;
 
 	/*
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1382471

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-19 14:50 +0200
Message-ID<rpDgm-6X3-13@gated-at.bofh.it>
In reply to#1382456
On Tue, 19 Apr 2016 15:30:39 +0300
Roger Quadros <rogerq@ti.com> wrote:

> On 19/04/16 14:22, Boris Brezillon wrote:
> > Hi Roger,
> > 
> > On Tue, 19 Apr 2016 13:28:50 +0300
> > Roger Quadros <rogerq@ti.com> wrote:
> > 
> >>> @@ -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.
> > 
> >>
> >>>  		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.
> 
> > 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)?

> 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?


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1382486

FromRoger Quadros <rogerq@ti.com>
Date2016-04-19 15:00 +0200
Message-ID<rpDq3-71o-19@gated-at.bofh.it>
In reply to#1382471
On 19/04/16 15:41, Boris Brezillon wrote:
> On Tue, 19 Apr 2016 15:30:39 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
>> On 19/04/16 14:22, Boris Brezillon wrote:
>>> Hi Roger,
>>>
>>> On Tue, 19 Apr 2016 13:28:50 +0300
>>> Roger Quadros <rogerq@ti.com> wrote:
>>>
>>>>> @@ -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.
>>>
>>>>
>>>>>  		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.
>>
>>> 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.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web