mtd: fsl_elbc_nand: Add SOFT_BCH ECC mode selection via DT

Message ID 20170714162812.2348-1-marek.behun@nic.cz
State Superseded
Headers show

Commit Message

Marek Behun July 14, 2017, 4:28 p.m.
Add RNDOUT operation which is required for SOFT and SOFT_BCH modes.

Restructure the code so that nand_scan_tail can correctly set
parameters for SOFT_BCH ECC mode.

Do not set read/write page operations from the driver when in SOFT
or SOFT_BCH modes.

This code is based on the patch by Tomas Hlavacek, which can be
found at
http://lists.infradead.org/pipermail/linux-mtd/2015-May/059365.html

Signed-off-by: Marek Behun <marek.behun@nic.cz>
---
 drivers/mtd/nand/fsl_elbc_nand.c | 132 ++++++++++++++++++++++++---------------
 1 file changed, 82 insertions(+), 50 deletions(-)

Comments

Boris Brezillon July 17, 2017, 8:27 p.m. | #1
Hi Marek,

Subject prefix should be "mtd: nand: fsl_elbc: "

Le Fri, 14 Jul 2017 18:28:12 +0200,
Marek Behún <marek.behun@nic.cz> a écrit :

> Add RNDOUT operation which is required for SOFT and SOFT_BCH modes.
> 
> Restructure the code so that nand_scan_tail can correctly set
> parameters for SOFT_BCH ECC mode.
> 
> Do not set read/write page operations from the driver when in SOFT
> or SOFT_BCH modes.
> 
> This code is based on the patch by Tomas Hlavacek, which can be
> found at
> http://lists.infradead.org/pipermail/linux-mtd/2015-May/059365.html
> 
> Signed-off-by: Marek Behun <marek.behun@nic.cz>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c | 132 ++++++++++++++++++++++++---------------
>  1 file changed, 82 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index b9ac16f05057..1df4a3b45642 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -355,6 +355,14 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
>  		fsl_elbc_run_command(mtd);
>  		return;
>  
> +	case NAND_CMD_RNDOUT:
> +		dev_vdbg(priv->dev,
> +			  "fsl_elbc_cmdfunc: NAND_CMD_RNDOUT, column: 0x%x.\n",
> +			  column);
> +
> +		elbc_fcm_ctrl->index = column;
> +		return;

I'd prefer to have it split in 2 patches: one adding support for RNDOUT
and the other one adding support for soft ECC.

> +
>  	/* READOOB reads only the OOB because no ECC is performed. */
>  	case NAND_CMD_READOOB:
>  		dev_vdbg(priv->dev,
> @@ -637,22 +645,10 @@ static int fsl_elbc_wait(struct mtd_info *mtd, struct nand_chip *chip)
>  	return (elbc_fcm_ctrl->mdr & 0xff) | NAND_STATUS_WP;
>  }
>  
> -static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
> +static inline void fsl_elbc_chip_init_dbg(struct mtd_info *mtd)

Drop the inline specifier here, the compiler should be smart enough to
know when to inline things.

>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct fsl_elbc_mtd *priv = nand_get_controller_data(chip);
> -	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
> -	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> -	unsigned int al;
> -
> -	/* calculate FMR Address Length field */
> -	al = 0;
> -	if (chip->pagemask & 0xffff0000)
> -		al++;
> -	if (chip->pagemask & 0xff000000)
> -		al++;
> -
> -	priv->fmr |= al << FMR_AL_SHIFT;
>  
>  	dev_dbg(priv->dev, "fsl_elbc_init: nand->numchips = %d\n",
>  	        chip->numchips);
> @@ -688,22 +684,6 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>  	        mtd->writesize);
>  	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize = %d\n",
>  	        mtd->oobsize);
> -
> -	/* adjust Option Register and ECC to match Flash page size */
> -	if (mtd->writesize == 512) {
> -		priv->page_size = 0;
> -		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> -	} else if (mtd->writesize == 2048) {
> -		priv->page_size = 1;
> -		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> -	} else {
> -		dev_err(priv->dev,
> -		        "fsl_elbc_init: page size %d is not supported\n",
> -		        mtd->writesize);
> -		return -1;
> -	}
> -
> -	return 0;
>  }
>  
>  static int fsl_elbc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> @@ -748,6 +728,34 @@ static int fsl_elbc_write_subpage(struct mtd_info *mtd, struct nand_chip *chip,
>  	return 0;
>  }
>  
> +static int fsl_elbc_ecc_init(struct fsl_elbc_mtd *priv)
> +{
> +	struct nand_chip *chip = &priv->chip;
> +
> +	switch (chip->ecc.mode) {
> +	case NAND_ECC_SOFT_BCH:

I don't know which branch you're basing your work on but
NAND_ECC_SOFT_BCH has been dropped in 4.7. Please rebase your work on
top of nand/next [1].

> +		break;
> +	case NAND_ECC_SOFT:
> +		chip->ecc.algo = NAND_ECC_HAMMING;

Why not let the user decides which algorithm to use, and fall back to
hamming if nothing is specified (algo == UNKNOWN)?

> +		break;
> +	case NAND_ECC_HW:
> +		chip->ecc.read_page = fsl_elbc_read_page;
> +		chip->ecc.write_page = fsl_elbc_write_page;
> +		chip->ecc.write_subpage = fsl_elbc_write_subpage;
> +
> +		/* put in small page settings and adjust later if needed */
> +		mtd_set_ooblayout(mtd, &fsl_elbc_ooblayout_ops);
> +		chip->ecc.size = 512;
> +		chip->ecc.bytes = 3;
> +		chip->ecc.strength = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
>  {
>  	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
> @@ -755,6 +763,8 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
>  	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
>  	struct nand_chip *chip = &priv->chip;
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	int ret;
> +	unsigned int al;
>  
>  	dev_dbg(priv->dev, "eLBC Set Information for bank %d\n", priv->bank);
>  
> @@ -787,24 +797,58 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
>  	chip->controller = &elbc_fcm_ctrl->controller;
>  	nand_set_controller_data(chip, priv);
>  
> -	chip->ecc.read_page = fsl_elbc_read_page;
> -	chip->ecc.write_page = fsl_elbc_write_page;
> -	chip->ecc.write_subpage = fsl_elbc_write_subpage;
> -
>  	/* If CS Base Register selects full hardware ECC then use it */
>  	if ((in_be32(&lbc->bank[priv->bank].br) & BR_DECC) ==
>  	    BR_DECC_CHK_GEN) {
>  		chip->ecc.mode = NAND_ECC_HW;
> -		mtd_set_ooblayout(mtd, &fsl_elbc_ooblayout_ops);
> -		chip->ecc.size = 512;
> -		chip->ecc.bytes = 3;
> -		chip->ecc.strength = 1;
>  	} else {
>  		/* otherwise fall back to default software ECC */
>  		chip->ecc.mode = NAND_ECC_SOFT;
> -		chip->ecc.algo = NAND_ECC_HAMMING;

Probably more explicit to set algo to NAND_ECC_UNNKOWN.

>  	}
>  
> +	ret = nand_scan_ident(mtd, 1, NULL);
> +	if (ret) {
> +		dev_err(priv->dev, "nand_scan_ident failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = fsl_elbc_ecc_init(priv);
> +	if (ret) {
> +		dev_err(priv->dev, "ECC init failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* calculate FMR Address Length field */
> +	al = 0;
> +	if (chip->pagemask & 0xffff0000)
> +		al++;
> +	if (chip->pagemask & 0xff000000)
> +		al++;
> +
> +	priv->fmr |= al << FMR_AL_SHIFT;
> +
> +	/* adjust Option Register and ECC to match Flash page size */
> +	if (mtd->writesize == 512) {
> +		priv->page_size = 0;
> +		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> +	} else if (mtd->writesize == 2048) {
> +		priv->page_size = 1;
> +		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> +	} else {
> +		dev_err(priv->dev,
> +		        "fsl_elbc_init: page size %d is not supported\n",
> +		        mtd->writesize);
> +		return -1;
> +	}
> +
> +	ret = nand_scan_tail(mtd);
> +	if (ret) {
> +		dev_err(priv->dev, "nand_scan_tail failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	fsl_elbc_chip_init_dbg(mtd);
> +
>  	return 0;
>  }
>  
> @@ -912,18 +956,6 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err;
>  
> -	ret = nand_scan_ident(mtd, 1, NULL);
> -	if (ret)
> -		goto err;
> -
> -	ret = fsl_elbc_chip_init_tail(mtd);
> -	if (ret)
> -		goto err;
> -
> -	ret = nand_scan_tail(mtd);
> -	if (ret)
> -		goto err;
> -
>  	/* First look for RedBoot table or partitions on the command
>  	 * line, these take precedence over device tree information */
>  	mtd_device_parse_register(mtd, part_probe_types, NULL,

I suggested to split the patch in 2, but it could be even clearer if
you split it in 3:
- RNDOUT support
- init code re-organization
- soft BCH support

Regards,

Boris

[1]http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/nand/next

Patch

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index b9ac16f05057..1df4a3b45642 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -355,6 +355,14 @@  static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		fsl_elbc_run_command(mtd);
 		return;
 
+	case NAND_CMD_RNDOUT:
+		dev_vdbg(priv->dev,
+			  "fsl_elbc_cmdfunc: NAND_CMD_RNDOUT, column: 0x%x.\n",
+			  column);
+
+		elbc_fcm_ctrl->index = column;
+		return;
+
 	/* READOOB reads only the OOB because no ECC is performed. */
 	case NAND_CMD_READOOB:
 		dev_vdbg(priv->dev,
@@ -637,22 +645,10 @@  static int fsl_elbc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	return (elbc_fcm_ctrl->mdr & 0xff) | NAND_STATUS_WP;
 }
 
-static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
+static inline void fsl_elbc_chip_init_dbg(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct fsl_elbc_mtd *priv = nand_get_controller_data(chip);
-	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
-	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
-	unsigned int al;
-
-	/* calculate FMR Address Length field */
-	al = 0;
-	if (chip->pagemask & 0xffff0000)
-		al++;
-	if (chip->pagemask & 0xff000000)
-		al++;
-
-	priv->fmr |= al << FMR_AL_SHIFT;
 
 	dev_dbg(priv->dev, "fsl_elbc_init: nand->numchips = %d\n",
 	        chip->numchips);
@@ -688,22 +684,6 @@  static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	        mtd->writesize);
 	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize = %d\n",
 	        mtd->oobsize);
-
-	/* adjust Option Register and ECC to match Flash page size */
-	if (mtd->writesize == 512) {
-		priv->page_size = 0;
-		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
-	} else if (mtd->writesize == 2048) {
-		priv->page_size = 1;
-		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
-	} else {
-		dev_err(priv->dev,
-		        "fsl_elbc_init: page size %d is not supported\n",
-		        mtd->writesize);
-		return -1;
-	}
-
-	return 0;
 }
 
 static int fsl_elbc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
@@ -748,6 +728,34 @@  static int fsl_elbc_write_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
+static int fsl_elbc_ecc_init(struct fsl_elbc_mtd *priv)
+{
+	struct nand_chip *chip = &priv->chip;
+
+	switch (chip->ecc.mode) {
+	case NAND_ECC_SOFT_BCH:
+		break;
+	case NAND_ECC_SOFT:
+		chip->ecc.algo = NAND_ECC_HAMMING;
+		break;
+	case NAND_ECC_HW:
+		chip->ecc.read_page = fsl_elbc_read_page;
+		chip->ecc.write_page = fsl_elbc_write_page;
+		chip->ecc.write_subpage = fsl_elbc_write_subpage;
+
+		/* put in small page settings and adjust later if needed */
+		mtd_set_ooblayout(mtd, &fsl_elbc_ooblayout_ops);
+		chip->ecc.size = 512;
+		chip->ecc.bytes = 3;
+		chip->ecc.strength = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 {
 	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
@@ -755,6 +763,8 @@  static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
 	struct nand_chip *chip = &priv->chip;
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret;
+	unsigned int al;
 
 	dev_dbg(priv->dev, "eLBC Set Information for bank %d\n", priv->bank);
 
@@ -787,24 +797,58 @@  static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	chip->controller = &elbc_fcm_ctrl->controller;
 	nand_set_controller_data(chip, priv);
 
-	chip->ecc.read_page = fsl_elbc_read_page;
-	chip->ecc.write_page = fsl_elbc_write_page;
-	chip->ecc.write_subpage = fsl_elbc_write_subpage;
-
 	/* If CS Base Register selects full hardware ECC then use it */
 	if ((in_be32(&lbc->bank[priv->bank].br) & BR_DECC) ==
 	    BR_DECC_CHK_GEN) {
 		chip->ecc.mode = NAND_ECC_HW;
-		mtd_set_ooblayout(mtd, &fsl_elbc_ooblayout_ops);
-		chip->ecc.size = 512;
-		chip->ecc.bytes = 3;
-		chip->ecc.strength = 1;
 	} else {
 		/* otherwise fall back to default software ECC */
 		chip->ecc.mode = NAND_ECC_SOFT;
-		chip->ecc.algo = NAND_ECC_HAMMING;
 	}
 
+	ret = nand_scan_ident(mtd, 1, NULL);
+	if (ret) {
+		dev_err(priv->dev, "nand_scan_ident failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = fsl_elbc_ecc_init(priv);
+	if (ret) {
+		dev_err(priv->dev, "ECC init failed: %d\n", ret);
+		return ret;
+	}
+
+	/* calculate FMR Address Length field */
+	al = 0;
+	if (chip->pagemask & 0xffff0000)
+		al++;
+	if (chip->pagemask & 0xff000000)
+		al++;
+
+	priv->fmr |= al << FMR_AL_SHIFT;
+
+	/* adjust Option Register and ECC to match Flash page size */
+	if (mtd->writesize == 512) {
+		priv->page_size = 0;
+		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
+	} else if (mtd->writesize == 2048) {
+		priv->page_size = 1;
+		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
+	} else {
+		dev_err(priv->dev,
+		        "fsl_elbc_init: page size %d is not supported\n",
+		        mtd->writesize);
+		return -1;
+	}
+
+	ret = nand_scan_tail(mtd);
+	if (ret) {
+		dev_err(priv->dev, "nand_scan_tail failed: %d\n", ret);
+		return ret;
+	}
+
+	fsl_elbc_chip_init_dbg(mtd);
+
 	return 0;
 }
 
@@ -912,18 +956,6 @@  static int fsl_elbc_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
-	ret = nand_scan_ident(mtd, 1, NULL);
-	if (ret)
-		goto err;
-
-	ret = fsl_elbc_chip_init_tail(mtd);
-	if (ret)
-		goto err;
-
-	ret = nand_scan_tail(mtd);
-	if (ret)
-		goto err;
-
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
 	mtd_device_parse_register(mtd, part_probe_types, NULL,