mtd: fsl_elbc_nand: Add SOFT_BCH ECC mode selection via DT

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

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