[PATCH v5 00/17] Allow dynamic allocations during NAND chip identification phase

[PATCH v5 00/17] Allow dynamic allocations during NAND chip identification phase

[Miquel Raynal]

Hello,

This series make a quite deep change in the NAND framework. Until now,
the NAND chip identification phase could be done in two manners from the
controller driver perspective:

1/ Call nand_scan()

  or

1/ Call nand_scan_ident()
2/ Do some controller-dependent configuration
3/ Call nand_scan_tail().

The fact that the identifaction could be split in two operations
involved that in the NAND framework, it was not possible to do any
dynamic allocation without risking a memory leak. What if the core
allocates a structure, then the driver between nand_scan_ident() and
nand_scan_tail() decides it cannot handle the chip and errors out?
The structure allocated by the core is lost: it is a memory leak. One
solution could have been to add a nand_scan_ident_cleanup() function,
but that would mean patching all the drivers anyway to make them call
this function when something fails between nand_scan_ident() and
nand_scan_tail().

To avoid this situation, we migrate all drivers to use nand_scan() in
conjuction with the recently added hooks ->attach_chip() and
->detach_chip() that are part of the nand_controller structure
operations. Drivers that need to tweak their configuration after
nand_scan_ident() should implement it. Any dynamically allocated space
in ->attach_chip() must be freed in the second hook: ->detach_chip().

The ->detach_chip() does not have to be called upon error in the
controller driver probe function. The nand_cleanup() helper already
exists for that and will do the call if needed. Of course, this helper
must be called on error after a successful nand_scan(), just like
before.

Once all drivers not using nand_scan() are migrated, nand_scan_ident()
and nand_scan_tail() are unexported and only available internally.

A previous work [1] removed the ONFI/JEDEC parameter pages and instead
allocated a nand_parameters structure in nand_chip, embedding both
generic entries and ONFI-related ones. The deal was, once dynamic
allocation possible, allocate in nand_scan_ident() the ONFI strcuture
only if actually needed. This is done in the last patches.

This series applies on top of nand/next.

Thank you,
Miquèl

[1] http://lists.infradead.org/pipermail/linux-mtd/2018-March/079456.html

Changes since v4:
=================
* Applied patches have been trimmed out of the series.
* Commit logs have been reworded a bit as suggested by Boris (done as
  well with applied commits).
* Always assigned the controller operations where the controller
  structure was initialized.
* brcmnand: moved the whole chunk between _ident() and _tail() in
  brcmnand_attach_chip().
* cafe: fixed the use of the useddma module parameter.
* denali: removed kfree(denali->buf) from denali_remove() as it is
  already done in ->detach_chip().
* lpc32xx_mlc: moved the IRQ handling out of the ->attach_chip()
  callback.
* txx9ndfmc: moved the block about ecc.size/bytes in
  txx9ndfmc_nand_scan() for more clarity.
* atmel: moved all the code in atmel_nand_register() directly in
  atmel_nand_controller_add_nand() + renamed atmel_nand_unregister()
  into atmel_nand_controller_remove_nand().
* nand_scan_ident: move the check on maxchips in nand_scan_with_ids()
  and the comment about it in the kernel doc (will no run
  nand_scan_ident() if zero).
* docg4: added a ->detach_chip() to fee the allocated BCH context.
* jz4740: fixed the error path and added a patch to clarify the
  transition to nand_scan(). Also removed unneeded curly braces
  and removed useless pdev parameters from the driver private
  structure (added in this series).
* core: moved the definition of nand_release() close to nand_cleanup()
  in rawnand.h while updating the comments.
* micron: only check for the ->onfi pointer to be present, not the
  vendor revision which is redundant.

Changes since v3:
=================
* Constified all the nand_controller_ops structure definitions.
* Fixed a build issue in fsl_elbc.
* Added a patch in the core to prevent executing nand_scan_ident if
  maxchips is NULL.
* Fixed the regression around the model name.
* Used kstrdup to allocate the model.
* The migration from char model[] to const char *model is done in a
  separate patch.

Changes since v2:
=================
* Rebased on top of nand/next.
* Adapted all drivers to declare statically a nand_controller_ops
  structure and assign it in the probe().
* Added the migration of the tegra_nand.c driver.
* Moved brcmnand controller ops affectation in the probe().

Changes since v1:
=================
* Rebased on top of nand/next.
* Light rewording of the cover letter about the possibility to have a
  nand_scan_ident_cleanup() function (just as example of how this series
  could have been done differently).
* Changed the hooks to reside in the nand_hw_ctrl structure instead of
  being part of nand_ecc_ctrl as these hooks are more
  controller-related.


Miquel Raynal (17):
  mtd: rawnand: brcmnand: convert driver to nand_scan()
  mtd: rawnand: cafe: convert driver to nand_scan()
  mtd: rawnand: lpc32xx_mlc: convert driver to nand_scan()
  mtd: rawnand: omap2: convert driver to nand_scan()
  mtd: rawnand: atmel: clarify NAND addition/removal paths
  mtd: rawnand: atmel: convert driver to nand_scan()
  mtd: rawnand: do not execute nand_scan_ident() if maxchips is zero
  mtd: rawnand: docg4: convert driver to nand_scan()
  mtd: rawnand: jz4740: fix probe function error path
  mtd: rawnand: jz4740: group nand_scan_{ident,tail} calls
  mtd: rawnand: jz4740: convert driver to nand_scan()
  mtd: rawnand: tegra: convert driver to nand_scan()
  mtd: rawnand: txx9ndfmc: clarify ECC parameters assignation
  mtd: rawnand: txx9ndfmc: convert driver to nand_scan()
  mtd: rawnand: do not export nand_scan_[ident|tail]() anymore
  mtd: rawnand: allocate model parameter dynamically
  mtd: rawnand: allocate dynamically ONFI parameters during detection

 drivers/mtd/nand/raw/atmel/nand-controller.c | 149 ++++----
 drivers/mtd/nand/raw/brcmnand/brcmnand.c     |  63 ++--
 drivers/mtd/nand/raw/cafe_nand.c             | 135 ++++---
 drivers/mtd/nand/raw/docg4.c                 |  70 ++--
 drivers/mtd/nand/raw/jz4740_nand.c           |  40 +-
 drivers/mtd/nand/raw/lpc32xx_mlc.c           |  61 +--
 drivers/mtd/nand/raw/nand_base.c             | 134 ++++---
 drivers/mtd/nand/raw/nand_micron.c           |   6 +-
 drivers/mtd/nand/raw/nand_timings.c          |  12 +-
 drivers/mtd/nand/raw/omap2.c                 | 533 +++++++++++++--------------
 drivers/mtd/nand/raw/tegra_nand.c            | 162 ++++----
 drivers/mtd/nand/raw/txx9ndfmc.c             |  32 +-
 include/linux/mtd/rawnand.h                  |  26 +-
 13 files changed, 762 insertions(+), 661 deletions(-)

-- 
2.14.1

[PATCH v5 01/17] mtd: rawnand: brcmnand: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 63 ++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 2e5efa0f9ea2..4b90d5b380c2 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2208,6 +2208,40 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 	return 0;
 }
 
+static int brcmnand_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct brcmnand_host *host = nand_get_controller_data(chip);
+	int ret;
+
+	chip->options |= NAND_NO_SUBPAGE_WRITE;
+	/*
+	 * Avoid (for instance) kmap()'d buffers from JFFS2, which we can't DMA
+	 * to/from, and have nand_base pass us a bounce buffer instead, as
+	 * needed.
+	 */
+	chip->options |= NAND_USE_BOUNCE_BUFFER;
+
+	if (chip->bbt_options & NAND_BBT_USE_FLASH)
+		chip->bbt_options |= NAND_BBT_NO_OOB;
+
+	if (brcmnand_setup_dev(host))
+		return -ENXIO;
+
+	chip->ecc.size = host->hwcfg.sector_size_1k ? 1024 : 512;
+
+	/* only use our internal HW threshold */
+	mtd->bitflip_threshold = 1;
+
+	ret = brcmstb_choose_ecc_layout(host);
+
+	return ret;
+}
+
+static const struct nand_controller_ops brcmnand_controller_ops = {
+	.attach_chip = brcmnand_attach_chip,
+};
+
 static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
@@ -2267,33 +2301,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
 	nand_writereg(ctrl, cfg_offs,
 		      nand_readreg(ctrl, cfg_offs) & ~CFG_BUS_WIDTH);
 
-	ret = nand_scan_ident(mtd, 1, NULL);
-	if (ret)
-		return ret;
-
-	chip->options |= NAND_NO_SUBPAGE_WRITE;
-	/*
-	 * Avoid (for instance) kmap()'d buffers from JFFS2, which we can't DMA
-	 * to/from, and have nand_base pass us a bounce buffer instead, as
-	 * needed.
-	 */
-	chip->options |= NAND_USE_BOUNCE_BUFFER;
-
-	if (chip->bbt_options & NAND_BBT_USE_FLASH)
-		chip->bbt_options |= NAND_BBT_NO_OOB;
-
-	if (brcmnand_setup_dev(host))
-		return -ENXIO;
-
-	chip->ecc.size = host->hwcfg.sector_size_1k ? 1024 : 512;
-	/* only use our internal HW threshold */
-	mtd->bitflip_threshold = 1;
-
-	ret = brcmstb_choose_ecc_layout(host);
-	if (ret)
-		return ret;
-
-	ret = nand_scan_tail(mtd);
+	ret = nand_scan(mtd, 1);
 	if (ret)
 		return ret;
 
@@ -2434,6 +2442,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 	init_completion(&ctrl->done);
 	init_completion(&ctrl->dma_done);
 	nand_controller_init(&ctrl->controller);
+	ctrl->controller.ops = &brcmnand_controller_ops;
 	INIT_LIST_HEAD(&ctrl->host_list);
 
 	/* NAND register range */
-- 
2.14.1

Re: [PATCH v5 01/17] mtd: rawnand: brcmnand: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 63 ++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 2e5efa0f9ea2..4b90d5b380c2 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2208,6 +2208,40 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  	return 0;
>  }
>  
> +static int brcmnand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct brcmnand_host *host = nand_get_controller_data(chip);
> +	int ret;
> +
> +	chip->options |= NAND_NO_SUBPAGE_WRITE;
> +	/*
> +	 * Avoid (for instance) kmap()'d buffers from JFFS2, which we can't DMA
> +	 * to/from, and have nand_base pass us a bounce buffer instead, as
> +	 * needed.
> +	 */
> +	chip->options |= NAND_USE_BOUNCE_BUFFER;
> +
> +	if (chip->bbt_options & NAND_BBT_USE_FLASH)
> +		chip->bbt_options |= NAND_BBT_NO_OOB;
> +
> +	if (brcmnand_setup_dev(host))
> +		return -ENXIO;
> +
> +	chip->ecc.size = host->hwcfg.sector_size_1k ? 1024 : 512;
> +
> +	/* only use our internal HW threshold */
> +	mtd->bitflip_threshold = 1;
> +
> +	ret = brcmstb_choose_ecc_layout(host);
> +
> +	return ret;
> +}
> +
> +static const struct nand_controller_ops brcmnand_controller_ops = {
> +	.attach_chip = brcmnand_attach_chip,
> +};
> +
>  static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
>  {
>  	struct brcmnand_controller *ctrl = host->ctrl;
> @@ -2267,33 +2301,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
>  	nand_writereg(ctrl, cfg_offs,
>  		      nand_readreg(ctrl, cfg_offs) & ~CFG_BUS_WIDTH);
>  
> -	ret = nand_scan_ident(mtd, 1, NULL);
> -	if (ret)
> -		return ret;
> -
> -	chip->options |= NAND_NO_SUBPAGE_WRITE;
> -	/*
> -	 * Avoid (for instance) kmap()'d buffers from JFFS2, which we can't DMA
> -	 * to/from, and have nand_base pass us a bounce buffer instead, as
> -	 * needed.
> -	 */
> -	chip->options |= NAND_USE_BOUNCE_BUFFER;
> -
> -	if (chip->bbt_options & NAND_BBT_USE_FLASH)
> -		chip->bbt_options |= NAND_BBT_NO_OOB;
> -
> -	if (brcmnand_setup_dev(host))
> -		return -ENXIO;
> -
> -	chip->ecc.size = host->hwcfg.sector_size_1k ? 1024 : 512;
> -	/* only use our internal HW threshold */
> -	mtd->bitflip_threshold = 1;
> -
> -	ret = brcmstb_choose_ecc_layout(host);
> -	if (ret)
> -		return ret;
> -
> -	ret = nand_scan_tail(mtd);
> +	ret = nand_scan(mtd, 1);
>  	if (ret)
>  		return ret;
>  
> @@ -2434,6 +2442,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  	init_completion(&ctrl->done);
>  	init_completion(&ctrl->dma_done);
>  	nand_controller_init(&ctrl->controller);
> +	ctrl->controller.ops = &brcmnand_controller_ops;
>  	INIT_LIST_HEAD(&ctrl->host_list);
>  
>  	/* NAND register range */

[PATCH v5 02/17] mtd: rawnand: cafe: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/cafe_nand.c | 135 ++++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 57 deletions(-)

diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
index ac0be933a490..1dbe43adcfe7 100644
--- a/drivers/mtd/nand/raw/cafe_nand.c
+++ b/drivers/mtd/nand/raw/cafe_nand.c
@@ -67,6 +67,7 @@ struct cafe_priv {
 	int nr_data;
 	int data_pos;
 	int page_addr;
+	bool usedma;
 	dma_addr_t dmaaddr;
 	unsigned char *dmabuf;
 };
@@ -121,7 +122,7 @@ static void cafe_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct cafe_priv *cafe = nand_get_controller_data(chip);
 
-	if (usedma)
+	if (cafe->usedma)
 		memcpy(cafe->dmabuf + cafe->datalen, buf, len);
 	else
 		memcpy_toio(cafe->mmio + CAFE_NAND_WRITE_DATA + cafe->datalen, buf, len);
@@ -137,7 +138,7 @@ static void cafe_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct cafe_priv *cafe = nand_get_controller_data(chip);
 
-	if (usedma)
+	if (cafe->usedma)
 		memcpy(buf, cafe->dmabuf + cafe->datalen, len);
 	else
 		memcpy_fromio(buf, cafe->mmio + CAFE_NAND_READ_DATA + cafe->datalen, len);
@@ -253,7 +254,7 @@ static void cafe_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	/* NB: The datasheet lies -- we really should be subtracting 1 here */
 	cafe_writel(cafe, cafe->datalen, NAND_DATA_LEN);
 	cafe_writel(cafe, 0x90000000, NAND_IRQ);
-	if (usedma && (ctl1 & (3<<25))) {
+	if (cafe->usedma && (ctl1 & (3<<25))) {
 		uint32_t dmactl = 0xc0000000 + cafe->datalen;
 		/* If WR or RD bits set, set up DMA */
 		if (ctl1 & (1<<26)) {
@@ -593,6 +594,76 @@ static int cafe_mul(int x)
 	return gf4096_mul(x, 0xe01);
 }
 
+static int cafe_nand_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct cafe_priv *cafe = nand_get_controller_data(chip);
+	int err = 0;
+
+	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112,
+					  &cafe->dmaaddr, GFP_KERNEL);
+	if (!cafe->dmabuf)
+		return -ENOMEM;
+
+	/* Set up DMA address */
+	cafe_writel(cafe, lower_32_bits(cafe->dmaaddr), NAND_DMA_ADDR0);
+	cafe_writel(cafe, upper_32_bits(cafe->dmaaddr), NAND_DMA_ADDR1);
+
+	cafe_dev_dbg(&cafe->pdev->dev, "Set DMA address to %x (virt %p)\n",
+		     cafe_readl(cafe, NAND_DMA_ADDR0), cafe->dmabuf);
+
+	/* Restore the DMA flag */
+	cafe->usedma = usedma;
+
+	cafe->ctl2 = BIT(27); /* Reed-Solomon ECC */
+	if (mtd->writesize == 2048)
+		cafe->ctl2 |= BIT(29); /* 2KiB page size */
+
+	/* Set up ECC according to the type of chip we found */
+	mtd_set_ooblayout(mtd, &cafe_ooblayout_ops);
+	if (mtd->writesize == 2048) {
+		cafe->nand.bbt_td = &cafe_bbt_main_descr_2048;
+		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_2048;
+	} else if (mtd->writesize == 512) {
+		cafe->nand.bbt_td = &cafe_bbt_main_descr_512;
+		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_512;
+	} else {
+		dev_warn(&cafe->pdev->dev,
+			 "Unexpected NAND flash writesize %d. Aborting\n",
+			 mtd->writesize);
+		err = -ENOTSUPP;
+		goto out_free_dma;
+	}
+
+	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
+	cafe->nand.ecc.size = mtd->writesize;
+	cafe->nand.ecc.bytes = 14;
+	cafe->nand.ecc.strength = 4;
+	cafe->nand.ecc.write_page = cafe_nand_write_page_lowlevel;
+	cafe->nand.ecc.write_oob = cafe_nand_write_oob;
+	cafe->nand.ecc.read_page = cafe_nand_read_page;
+	cafe->nand.ecc.read_oob = cafe_nand_read_oob;
+
+	return 0;
+
+ out_free_dma:
+	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
+
+	return err;
+}
+
+static void cafe_nand_detach_chip(struct nand_chip *chip)
+{
+	struct cafe_priv *cafe = nand_get_controller_data(chip);
+
+	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
+}
+
+static const struct nand_controller_ops cafe_nand_controller_ops = {
+	.attach_chip = cafe_nand_attach_chip,
+	.detach_chip = cafe_nand_detach_chip,
+};
+
 static int cafe_nand_probe(struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {
@@ -600,7 +671,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	struct cafe_priv *cafe;
 	uint32_t ctrl;
 	int err = 0;
-	int old_dma;
 
 	/* Very old versions shared the same PCI ident for all three
 	   functions on the chip. Verify the class too... */
@@ -708,62 +778,15 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 		cafe_readl(cafe, GLOBAL_CTRL),
 		cafe_readl(cafe, GLOBAL_IRQ_MASK));
 
-	/* Do not use the DMA for the nand_scan_ident() */
-	old_dma = usedma;
-	usedma = 0;
+	/* Do not use the DMA during the NAND identification */
+	cafe->usedma = 0;
 
 	/* Scan to find existence of the device */
-	err = nand_scan_ident(mtd, 2, NULL);
+	cafe->nand.dummy_controller.ops = &cafe_nand_controller_ops;
+	err = nand_scan(mtd, 2);
 	if (err)
 		goto out_irq;
 
-	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112,
-					  &cafe->dmaaddr, GFP_KERNEL);
-	if (!cafe->dmabuf) {
-		err = -ENOMEM;
-		goto out_irq;
-	}
-
-	/* Set up DMA address */
-	cafe_writel(cafe, lower_32_bits(cafe->dmaaddr), NAND_DMA_ADDR0);
-	cafe_writel(cafe, upper_32_bits(cafe->dmaaddr), NAND_DMA_ADDR1);
-
-	cafe_dev_dbg(&cafe->pdev->dev, "Set DMA address to %x (virt %p)\n",
-		cafe_readl(cafe, NAND_DMA_ADDR0), cafe->dmabuf);
-
-	/* Restore the DMA flag */
-	usedma = old_dma;
-
-	cafe->ctl2 = 1<<27; /* Reed-Solomon ECC */
-	if (mtd->writesize == 2048)
-		cafe->ctl2 |= 1<<29; /* 2KiB page size */
-
-	/* Set up ECC according to the type of chip we found */
-	mtd_set_ooblayout(mtd, &cafe_ooblayout_ops);
-	if (mtd->writesize == 2048) {
-		cafe->nand.bbt_td = &cafe_bbt_main_descr_2048;
-		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_2048;
-	} else if (mtd->writesize == 512) {
-		cafe->nand.bbt_td = &cafe_bbt_main_descr_512;
-		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_512;
-	} else {
-		pr_warn("Unexpected NAND flash writesize %d. Aborting\n",
-			mtd->writesize);
-		goto out_free_dma;
-	}
-	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
-	cafe->nand.ecc.size = mtd->writesize;
-	cafe->nand.ecc.bytes = 14;
-	cafe->nand.ecc.strength = 4;
-	cafe->nand.ecc.write_page = cafe_nand_write_page_lowlevel;
-	cafe->nand.ecc.write_oob = cafe_nand_write_oob;
-	cafe->nand.ecc.read_page = cafe_nand_read_page;
-	cafe->nand.ecc.read_oob = cafe_nand_read_oob;
-
-	err = nand_scan_tail(mtd);
-	if (err)
-		goto out_free_dma;
-
 	pci_set_drvdata(pdev, mtd);
 
 	mtd->name = "cafe_nand";
@@ -775,8 +798,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 
  out_cleanup_nand:
 	nand_cleanup(&cafe->nand);
- out_free_dma:
-	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
  out_irq:
 	/* Disable NAND IRQ in global IRQ mask register */
 	cafe_writel(cafe, ~1 & cafe_readl(cafe, GLOBAL_IRQ_MASK), GLOBAL_IRQ_MASK);
-- 
2.14.1

Re: [PATCH v5 02/17] mtd: rawnand: cafe: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:37 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/cafe_nand.c | 135 ++++++++++++++++++++++-----------------
>  1 file changed, 78 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
> index ac0be933a490..1dbe43adcfe7 100644
> --- a/drivers/mtd/nand/raw/cafe_nand.c
> +++ b/drivers/mtd/nand/raw/cafe_nand.c
> @@ -67,6 +67,7 @@ struct cafe_priv {
>  	int nr_data;
>  	int data_pos;
>  	int page_addr;
> +	bool usedma;
>  	dma_addr_t dmaaddr;
>  	unsigned char *dmabuf;
>  };
> @@ -121,7 +122,7 @@ static void cafe_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct cafe_priv *cafe = nand_get_controller_data(chip);
>  
> -	if (usedma)
> +	if (cafe->usedma)
>  		memcpy(cafe->dmabuf + cafe->datalen, buf, len);
>  	else
>  		memcpy_toio(cafe->mmio + CAFE_NAND_WRITE_DATA + cafe->datalen, buf, len);
> @@ -137,7 +138,7 @@ static void cafe_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct cafe_priv *cafe = nand_get_controller_data(chip);
>  
> -	if (usedma)
> +	if (cafe->usedma)
>  		memcpy(buf, cafe->dmabuf + cafe->datalen, len);
>  	else
>  		memcpy_fromio(buf, cafe->mmio + CAFE_NAND_READ_DATA + cafe->datalen, len);
> @@ -253,7 +254,7 @@ static void cafe_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>  	/* NB: The datasheet lies -- we really should be subtracting 1 here */
>  	cafe_writel(cafe, cafe->datalen, NAND_DATA_LEN);
>  	cafe_writel(cafe, 0x90000000, NAND_IRQ);
> -	if (usedma && (ctl1 & (3<<25))) {
> +	if (cafe->usedma && (ctl1 & (3<<25))) {
>  		uint32_t dmactl = 0xc0000000 + cafe->datalen;
>  		/* If WR or RD bits set, set up DMA */
>  		if (ctl1 & (1<<26)) {
> @@ -593,6 +594,76 @@ static int cafe_mul(int x)
>  	return gf4096_mul(x, 0xe01);
>  }
>  
> +static int cafe_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct cafe_priv *cafe = nand_get_controller_data(chip);
> +	int err = 0;
> +
> +	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112,
> +					  &cafe->dmaaddr, GFP_KERNEL);
> +	if (!cafe->dmabuf)
> +		return -ENOMEM;
> +
> +	/* Set up DMA address */
> +	cafe_writel(cafe, lower_32_bits(cafe->dmaaddr), NAND_DMA_ADDR0);
> +	cafe_writel(cafe, upper_32_bits(cafe->dmaaddr), NAND_DMA_ADDR1);
> +
> +	cafe_dev_dbg(&cafe->pdev->dev, "Set DMA address to %x (virt %p)\n",
> +		     cafe_readl(cafe, NAND_DMA_ADDR0), cafe->dmabuf);
> +
> +	/* Restore the DMA flag */
> +	cafe->usedma = usedma;
> +
> +	cafe->ctl2 = BIT(27); /* Reed-Solomon ECC */
> +	if (mtd->writesize == 2048)
> +		cafe->ctl2 |= BIT(29); /* 2KiB page size */
> +
> +	/* Set up ECC according to the type of chip we found */
> +	mtd_set_ooblayout(mtd, &cafe_ooblayout_ops);
> +	if (mtd->writesize == 2048) {
> +		cafe->nand.bbt_td = &cafe_bbt_main_descr_2048;
> +		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_2048;
> +	} else if (mtd->writesize == 512) {
> +		cafe->nand.bbt_td = &cafe_bbt_main_descr_512;
> +		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_512;
> +	} else {
> +		dev_warn(&cafe->pdev->dev,
> +			 "Unexpected NAND flash writesize %d. Aborting\n",
> +			 mtd->writesize);
> +		err = -ENOTSUPP;
> +		goto out_free_dma;
> +	}
> +
> +	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
> +	cafe->nand.ecc.size = mtd->writesize;
> +	cafe->nand.ecc.bytes = 14;
> +	cafe->nand.ecc.strength = 4;
> +	cafe->nand.ecc.write_page = cafe_nand_write_page_lowlevel;
> +	cafe->nand.ecc.write_oob = cafe_nand_write_oob;
> +	cafe->nand.ecc.read_page = cafe_nand_read_page;
> +	cafe->nand.ecc.read_oob = cafe_nand_read_oob;
> +
> +	return 0;
> +
> + out_free_dma:
> +	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
> +
> +	return err;
> +}
> +
> +static void cafe_nand_detach_chip(struct nand_chip *chip)
> +{
> +	struct cafe_priv *cafe = nand_get_controller_data(chip);
> +
> +	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
> +}
> +
> +static const struct nand_controller_ops cafe_nand_controller_ops = {
> +	.attach_chip = cafe_nand_attach_chip,
> +	.detach_chip = cafe_nand_detach_chip,
> +};
> +
>  static int cafe_nand_probe(struct pci_dev *pdev,
>  				     const struct pci_device_id *ent)
>  {
> @@ -600,7 +671,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  	struct cafe_priv *cafe;
>  	uint32_t ctrl;
>  	int err = 0;
> -	int old_dma;
>  
>  	/* Very old versions shared the same PCI ident for all three
>  	   functions on the chip. Verify the class too... */
> @@ -708,62 +778,15 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  		cafe_readl(cafe, GLOBAL_CTRL),
>  		cafe_readl(cafe, GLOBAL_IRQ_MASK));
>  
> -	/* Do not use the DMA for the nand_scan_ident() */
> -	old_dma = usedma;
> -	usedma = 0;
> +	/* Do not use the DMA during the NAND identification */
> +	cafe->usedma = 0;
>  
>  	/* Scan to find existence of the device */
> -	err = nand_scan_ident(mtd, 2, NULL);
> +	cafe->nand.dummy_controller.ops = &cafe_nand_controller_ops;
> +	err = nand_scan(mtd, 2);
>  	if (err)
>  		goto out_irq;
>  
> -	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112,
> -					  &cafe->dmaaddr, GFP_KERNEL);
> -	if (!cafe->dmabuf) {
> -		err = -ENOMEM;
> -		goto out_irq;
> -	}
> -
> -	/* Set up DMA address */
> -	cafe_writel(cafe, lower_32_bits(cafe->dmaaddr), NAND_DMA_ADDR0);
> -	cafe_writel(cafe, upper_32_bits(cafe->dmaaddr), NAND_DMA_ADDR1);
> -
> -	cafe_dev_dbg(&cafe->pdev->dev, "Set DMA address to %x (virt %p)\n",
> -		cafe_readl(cafe, NAND_DMA_ADDR0), cafe->dmabuf);
> -
> -	/* Restore the DMA flag */
> -	usedma = old_dma;
> -
> -	cafe->ctl2 = 1<<27; /* Reed-Solomon ECC */
> -	if (mtd->writesize == 2048)
> -		cafe->ctl2 |= 1<<29; /* 2KiB page size */
> -
> -	/* Set up ECC according to the type of chip we found */
> -	mtd_set_ooblayout(mtd, &cafe_ooblayout_ops);
> -	if (mtd->writesize == 2048) {
> -		cafe->nand.bbt_td = &cafe_bbt_main_descr_2048;
> -		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_2048;
> -	} else if (mtd->writesize == 512) {
> -		cafe->nand.bbt_td = &cafe_bbt_main_descr_512;
> -		cafe->nand.bbt_md = &cafe_bbt_mirror_descr_512;
> -	} else {
> -		pr_warn("Unexpected NAND flash writesize %d. Aborting\n",
> -			mtd->writesize);
> -		goto out_free_dma;
> -	}
> -	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
> -	cafe->nand.ecc.size = mtd->writesize;
> -	cafe->nand.ecc.bytes = 14;
> -	cafe->nand.ecc.strength = 4;
> -	cafe->nand.ecc.write_page = cafe_nand_write_page_lowlevel;
> -	cafe->nand.ecc.write_oob = cafe_nand_write_oob;
> -	cafe->nand.ecc.read_page = cafe_nand_read_page;
> -	cafe->nand.ecc.read_oob = cafe_nand_read_oob;
> -
> -	err = nand_scan_tail(mtd);
> -	if (err)
> -		goto out_free_dma;
> -
>  	pci_set_drvdata(pdev, mtd);
>  
>  	mtd->name = "cafe_nand";
> @@ -775,8 +798,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  
>   out_cleanup_nand:
>  	nand_cleanup(&cafe->nand);
> - out_free_dma:
> -	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
>   out_irq:
>  	/* Disable NAND IRQ in global IRQ mask register */
>  	cafe_writel(cafe, ~1 & cafe_readl(cafe, GLOBAL_IRQ_MASK), GLOBAL_IRQ_MASK);

[PATCH v5 03/17] mtd: rawnand: lpc32xx_mlc: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/lpc32xx_mlc.c | 61 +++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/nand/raw/lpc32xx_mlc.c b/drivers/mtd/nand/raw/lpc32xx_mlc.c
index 052d123a8304..e82abada130a 100644
--- a/drivers/mtd/nand/raw/lpc32xx_mlc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_mlc.c
@@ -184,6 +184,7 @@ static struct nand_bbt_descr lpc32xx_nand_bbt_mirror = {
 };
 
 struct lpc32xx_nand_host {
+	struct platform_device	*pdev;
 	struct nand_chip	nand_chip;
 	struct lpc32xx_mlc_platform_data *pdata;
 	struct clk		*clk;
@@ -653,6 +654,32 @@ static struct lpc32xx_nand_cfg_mlc *lpc32xx_parse_dt(struct device *dev)
 	return ncfg;
 }
 
+static int lpc32xx_nand_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct lpc32xx_nand_host *host = nand_get_controller_data(chip);
+	struct device *dev = &host->pdev->dev;
+
+	host->dma_buf = devm_kzalloc(dev, mtd->writesize, GFP_KERNEL);
+	if (!host->dma_buf)
+		return -ENOMEM;
+
+	host->dummy_buf = devm_kzalloc(dev, mtd->writesize, GFP_KERNEL);
+	if (!host->dummy_buf)
+		return -ENOMEM;
+
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.size = 512;
+	mtd_set_ooblayout(mtd, &lpc32xx_ooblayout_ops);
+	host->mlcsubpages = mtd->writesize / 512;
+
+	return 0;
+}
+
+static const struct nand_controller_ops lpc32xx_nand_controller_ops = {
+	.attach_chip = lpc32xx_nand_attach_chip,
+};
+
 /*
  * Probe for NAND controller
  */
@@ -669,6 +696,8 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	if (!host)
 		return -ENOMEM;
 
+	host->pdev = pdev;
+
 	rc = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	host->io_base = devm_ioremap_resource(&pdev->dev, rc);
 	if (IS_ERR(host->io_base))
@@ -748,31 +777,6 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 		}
 	}
 
-	/*
-	 * Scan to find existance of the device and
-	 * Get the type of NAND device SMALL block or LARGE block
-	 */
-	res = nand_scan_ident(mtd, 1, NULL);
-	if (res)
-		goto release_dma_chan;
-
-	host->dma_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
-	if (!host->dma_buf) {
-		res = -ENOMEM;
-		goto release_dma_chan;
-	}
-
-	host->dummy_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
-	if (!host->dummy_buf) {
-		res = -ENOMEM;
-		goto release_dma_chan;
-	}
-
-	nand_chip->ecc.mode = NAND_ECC_HW;
-	nand_chip->ecc.size = 512;
-	mtd_set_ooblayout(mtd, &lpc32xx_ooblayout_ops);
-	host->mlcsubpages = mtd->writesize / 512;
-
 	/* initially clear interrupt status */
 	readb(MLC_IRQ_SR(host->io_base));
 
@@ -794,10 +798,11 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	}
 
 	/*
-	 * Fills out all the uninitialized function pointers with the defaults
-	 * And scans for a bad block table if appropriate.
+	 * Scan to find existence of the device and get the type of NAND device:
+	 * SMALL block or LARGE block.
 	 */
-	res = nand_scan_tail(mtd);
+	nand_chip->dummy_controller.ops = &lpc32xx_nand_controller_ops;
+	res = nand_scan(mtd, 1);
 	if (res)
 		goto free_irq;
 
-- 
2.14.1

Re: [PATCH v5 03/17] mtd: rawnand: lpc32xx_mlc: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:38 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/lpc32xx_mlc.c | 61 +++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/lpc32xx_mlc.c b/drivers/mtd/nand/raw/lpc32xx_mlc.c
> index 052d123a8304..e82abada130a 100644
> --- a/drivers/mtd/nand/raw/lpc32xx_mlc.c
> +++ b/drivers/mtd/nand/raw/lpc32xx_mlc.c
> @@ -184,6 +184,7 @@ static struct nand_bbt_descr lpc32xx_nand_bbt_mirror = {
>  };
>  
>  struct lpc32xx_nand_host {
> +	struct platform_device	*pdev;

You don't need pdev. With this field removed

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

>  	struct nand_chip	nand_chip;
>  	struct lpc32xx_mlc_platform_data *pdata;
>  	struct clk		*clk;
> @@ -653,6 +654,32 @@ static struct lpc32xx_nand_cfg_mlc *lpc32xx_parse_dt(struct device *dev)
>  	return ncfg;
>  }
>  
> +static int lpc32xx_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct lpc32xx_nand_host *host = nand_get_controller_data(chip);
> +	struct device *dev = &host->pdev->dev;
> +
> +	host->dma_buf = devm_kzalloc(dev, mtd->writesize, GFP_KERNEL);
> +	if (!host->dma_buf)
> +		return -ENOMEM;
> +
> +	host->dummy_buf = devm_kzalloc(dev, mtd->writesize, GFP_KERNEL);
> +	if (!host->dummy_buf)
> +		return -ENOMEM;
> +
> +	chip->ecc.mode = NAND_ECC_HW;
> +	chip->ecc.size = 512;
> +	mtd_set_ooblayout(mtd, &lpc32xx_ooblayout_ops);
> +	host->mlcsubpages = mtd->writesize / 512;
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops lpc32xx_nand_controller_ops = {
> +	.attach_chip = lpc32xx_nand_attach_chip,
> +};
> +
>  /*
>   * Probe for NAND controller
>   */
> @@ -669,6 +696,8 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>  	if (!host)
>  		return -ENOMEM;
>  
> +	host->pdev = pdev;
> +
>  	rc = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	host->io_base = devm_ioremap_resource(&pdev->dev, rc);
>  	if (IS_ERR(host->io_base))
> @@ -748,31 +777,6 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	/*
> -	 * Scan to find existance of the device and
> -	 * Get the type of NAND device SMALL block or LARGE block
> -	 */
> -	res = nand_scan_ident(mtd, 1, NULL);
> -	if (res)
> -		goto release_dma_chan;
> -
> -	host->dma_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
> -	if (!host->dma_buf) {
> -		res = -ENOMEM;
> -		goto release_dma_chan;
> -	}
> -
> -	host->dummy_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
> -	if (!host->dummy_buf) {
> -		res = -ENOMEM;
> -		goto release_dma_chan;
> -	}
> -
> -	nand_chip->ecc.mode = NAND_ECC_HW;
> -	nand_chip->ecc.size = 512;
> -	mtd_set_ooblayout(mtd, &lpc32xx_ooblayout_ops);
> -	host->mlcsubpages = mtd->writesize / 512;
> -
>  	/* initially clear interrupt status */
>  	readb(MLC_IRQ_SR(host->io_base));
>  
> @@ -794,10 +798,11 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	/*
> -	 * Fills out all the uninitialized function pointers with the defaults
> -	 * And scans for a bad block table if appropriate.
> +	 * Scan to find existence of the device and get the type of NAND device:
> +	 * SMALL block or LARGE block.
>  	 */
> -	res = nand_scan_tail(mtd);
> +	nand_chip->dummy_controller.ops = &lpc32xx_nand_controller_ops;
> +	res = nand_scan(mtd, 1);
>  	if (res)
>  		goto free_irq;
>  

[PATCH v5 04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/omap2.c | 533 +++++++++++++++++++++----------------------
 1 file changed, 265 insertions(+), 268 deletions(-)

diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index e943b2e5a5e2..4546ac0bed4a 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -144,12 +144,6 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
 	0xac, 0x6b, 0xff, 0x99, 0x7b};
 static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
 
-/* Shared among all NAND instances to synchronize access to the ECC Engine */
-static struct nand_controller omap_gpmc_controller = {
-	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
-	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
-};
-
 struct omap_nand_info {
 	struct nand_chip		nand;
 	struct platform_device		*pdev;
@@ -1915,17 +1909,278 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
 	.free = omap_sw_ooblayout_free,
 };
 
+static int omap_nand_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct omap_nand_info *info = mtd_to_omap(mtd);
+	struct device *dev = &info->pdev->dev;
+	int min_oobbytes = BADBLOCK_MARKER_LENGTH;
+	int oobbytes_per_step;
+	dma_cap_mask_t mask;
+	int err;
+
+	if (chip->bbt_options & NAND_BBT_USE_FLASH)
+		chip->bbt_options |= NAND_BBT_NO_OOB;
+	else
+		chip->options |= NAND_SKIP_BBTSCAN;
+
+	/* Re-populate low-level callbacks based on xfer modes */
+	switch (info->xfer_type) {
+	case NAND_OMAP_PREFETCH_POLLED:
+		chip->read_buf = omap_read_buf_pref;
+		chip->write_buf = omap_write_buf_pref;
+		break;
+
+	case NAND_OMAP_POLLED:
+		/* Use nand_base defaults for {read,write}_buf */
+		break;
+
+	case NAND_OMAP_PREFETCH_DMA:
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+		info->dma = dma_request_chan(dev, "rxtx");
+
+		if (IS_ERR(info->dma)) {
+			dev_err(dev, "DMA engine request failed\n");
+			return PTR_ERR(info->dma);
+		} else {
+			struct dma_slave_config cfg;
+
+			memset(&cfg, 0, sizeof(cfg));
+			cfg.src_addr = info->phys_base;
+			cfg.dst_addr = info->phys_base;
+			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			cfg.src_maxburst = 16;
+			cfg.dst_maxburst = 16;
+			err = dmaengine_slave_config(info->dma, &cfg);
+			if (err) {
+				dev_err(dev,
+					"DMA engine slave config failed: %d\n",
+					err);
+				return err;
+			}
+			chip->read_buf = omap_read_buf_dma_pref;
+			chip->write_buf = omap_write_buf_dma_pref;
+		}
+		break;
+
+	case NAND_OMAP_PREFETCH_IRQ:
+		info->gpmc_irq_fifo = platform_get_irq(info->pdev, 0);
+		if (info->gpmc_irq_fifo <= 0) {
+			dev_err(dev, "Error getting fifo IRQ\n");
+			return -ENODEV;
+		}
+		err = devm_request_irq(dev, info->gpmc_irq_fifo,
+				       omap_nand_irq, IRQF_SHARED,
+				       "gpmc-nand-fifo", info);
+		if (err) {
+			dev_err(dev, "Requesting IRQ %d, error %d\n",
+				info->gpmc_irq_fifo, err);
+			info->gpmc_irq_fifo = 0;
+			return err;
+		}
+
+		info->gpmc_irq_count = platform_get_irq(info->pdev, 1);
+		if (info->gpmc_irq_count <= 0) {
+			dev_err(dev, "Error getting IRQ count\n");
+			return -ENODEV;
+		}
+		err = devm_request_irq(dev, info->gpmc_irq_count,
+				       omap_nand_irq, IRQF_SHARED,
+				       "gpmc-nand-count", info);
+		if (err) {
+			dev_err(dev, "Requesting IRQ %d, error %d\n",
+				info->gpmc_irq_count, err);
+			info->gpmc_irq_count = 0;
+			return err;
+		}
+
+		chip->read_buf = omap_read_buf_irq_pref;
+		chip->write_buf = omap_write_buf_irq_pref;
+
+		break;
+
+	default:
+		dev_err(dev, "xfer_type %d not supported!\n", info->xfer_type);
+		return -EINVAL;
+	}
+
+	if (!omap2_nand_ecc_check(info))
+		return -EINVAL;
+
+	/*
+	 * Bail out earlier to let NAND_ECC_SOFT code create its own
+	 * ooblayout instead of using ours.
+	 */
+	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
+		chip->ecc.mode = NAND_ECC_SOFT;
+		chip->ecc.algo = NAND_ECC_HAMMING;
+		return 0;
+	}
+
+	/* Populate MTD interface based on ECC scheme */
+	switch (info->ecc_opt) {
+	case OMAP_ECC_HAM1_CODE_HW:
+		dev_info(dev, "nand: using OMAP_ECC_HAM1_CODE_HW\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.bytes		= 3;
+		chip->ecc.size		= 512;
+		chip->ecc.strength	= 1;
+		chip->ecc.calculate	= omap_calculate_ecc;
+		chip->ecc.hwctl		= omap_enable_hwecc;
+		chip->ecc.correct	= omap_correct_data;
+		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+		oobbytes_per_step	= chip->ecc.bytes;
+
+		if (!(chip->options & NAND_BUSWIDTH_16))
+			min_oobbytes	= 1;
+
+		break;
+
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.size		= 512;
+		chip->ecc.bytes		= 7;
+		chip->ecc.strength	= 4;
+		chip->ecc.hwctl		= omap_enable_hwecc_bch;
+		chip->ecc.correct	= nand_bch_correct_data;
+		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
+		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
+		/* Reserve one byte for the OMAP marker */
+		oobbytes_per_step	= chip->ecc.bytes + 1;
+		/* Software BCH library is used for locating errors */
+		chip->ecc.priv		= nand_bch_init(mtd);
+		if (!chip->ecc.priv) {
+			dev_err(dev, "Unable to use BCH library\n");
+			return -EINVAL;
+		}
+		break;
+
+	case OMAP_ECC_BCH4_CODE_HW:
+		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.size		= 512;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		chip->ecc.bytes		= 7 + 1;
+		chip->ecc.strength	= 4;
+		chip->ecc.hwctl		= omap_enable_hwecc_bch;
+		chip->ecc.correct	= omap_elm_correct_data;
+		chip->ecc.read_page	= omap_read_page_bch;
+		chip->ecc.write_page	= omap_write_page_bch;
+		chip->ecc.write_subpage	= omap_write_subpage_bch;
+		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+		oobbytes_per_step	= chip->ecc.bytes;
+
+		err = elm_config(info->elm_dev, BCH4_ECC,
+				 mtd->writesize / chip->ecc.size,
+				 chip->ecc.size, chip->ecc.bytes);
+		if (err < 0)
+			return err;
+		break;
+
+	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.size		= 512;
+		chip->ecc.bytes		= 13;
+		chip->ecc.strength	= 8;
+		chip->ecc.hwctl		= omap_enable_hwecc_bch;
+		chip->ecc.correct	= nand_bch_correct_data;
+		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
+		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
+		/* Reserve one byte for the OMAP marker */
+		oobbytes_per_step	= chip->ecc.bytes + 1;
+		/* Software BCH library is used for locating errors */
+		chip->ecc.priv		= nand_bch_init(mtd);
+		if (!chip->ecc.priv) {
+			dev_err(dev, "unable to use BCH library\n");
+			return -EINVAL;
+		}
+		break;
+
+	case OMAP_ECC_BCH8_CODE_HW:
+		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.size		= 512;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		chip->ecc.bytes		= 13 + 1;
+		chip->ecc.strength	= 8;
+		chip->ecc.hwctl		= omap_enable_hwecc_bch;
+		chip->ecc.correct	= omap_elm_correct_data;
+		chip->ecc.read_page	= omap_read_page_bch;
+		chip->ecc.write_page	= omap_write_page_bch;
+		chip->ecc.write_subpage	= omap_write_subpage_bch;
+		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+		oobbytes_per_step	= chip->ecc.bytes;
+
+		err = elm_config(info->elm_dev, BCH8_ECC,
+				 mtd->writesize / chip->ecc.size,
+				 chip->ecc.size, chip->ecc.bytes);
+		if (err < 0)
+			return err;
+
+		break;
+
+	case OMAP_ECC_BCH16_CODE_HW:
+		pr_info("Using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
+		chip->ecc.mode		= NAND_ECC_HW;
+		chip->ecc.size		= 512;
+		chip->ecc.bytes		= 26;
+		chip->ecc.strength	= 16;
+		chip->ecc.hwctl		= omap_enable_hwecc_bch;
+		chip->ecc.correct	= omap_elm_correct_data;
+		chip->ecc.read_page	= omap_read_page_bch;
+		chip->ecc.write_page	= omap_write_page_bch;
+		chip->ecc.write_subpage	= omap_write_subpage_bch;
+		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
+		oobbytes_per_step	= chip->ecc.bytes;
+
+		err = elm_config(info->elm_dev, BCH16_ECC,
+				 mtd->writesize / chip->ecc.size,
+				 chip->ecc.size, chip->ecc.bytes);
+		if (err < 0)
+			return err;
+
+		break;
+	default:
+		dev_err(dev, "Invalid or unsupported ECC scheme\n");
+		return -EINVAL;
+	}
+
+	/* Check if NAND device's OOB is enough to store ECC signatures */
+	min_oobbytes += (oobbytes_per_step *
+			 (mtd->writesize / chip->ecc.size));
+	if (mtd->oobsize < min_oobbytes) {
+		dev_err(dev,
+			"Not enough OOB bytes: required = %d, available=%d\n",
+			min_oobbytes, mtd->oobsize);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct nand_controller_ops omap_nand_controller_ops = {
+	.attach_chip = omap_nand_attach_chip,
+};
+
+/* Shared among all NAND instances to synchronize access to the ECC Engine */
+static struct nand_controller omap_gpmc_controller = {
+	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
+	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
+	.ops = &omap_nand_controller_ops,
+};
+
 static int omap_nand_probe(struct platform_device *pdev)
 {
 	struct omap_nand_info		*info;
 	struct mtd_info			*mtd;
 	struct nand_chip		*nand_chip;
 	int				err;
-	dma_cap_mask_t			mask;
 	struct resource			*res;
 	struct device			*dev = &pdev->dev;
-	int				min_oobbytes = BADBLOCK_MARKER_LENGTH;
-	int				oobbytes_per_step;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
 				GFP_KERNEL);
@@ -1998,266 +2253,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	/* scan NAND device connected to chip controller */
 	nand_chip->options |= info->devsize & NAND_BUSWIDTH_16;
-	err = nand_scan_ident(mtd, 1, NULL);
-	if (err) {
-		dev_err(&info->pdev->dev,
-			"scan failed, may be bus-width mismatch\n");
-		goto return_error;
-	}
 
-	if (nand_chip->bbt_options & NAND_BBT_USE_FLASH)
-		nand_chip->bbt_options |= NAND_BBT_NO_OOB;
-	else
-		nand_chip->options |= NAND_SKIP_BBTSCAN;
-
-	/* re-populate low-level callbacks based on xfer modes */
-	switch (info->xfer_type) {
-	case NAND_OMAP_PREFETCH_POLLED:
-		nand_chip->read_buf   = omap_read_buf_pref;
-		nand_chip->write_buf  = omap_write_buf_pref;
-		break;
-
-	case NAND_OMAP_POLLED:
-		/* Use nand_base defaults for {read,write}_buf */
-		break;
-
-	case NAND_OMAP_PREFETCH_DMA:
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-		info->dma = dma_request_chan(pdev->dev.parent, "rxtx");
-
-		if (IS_ERR(info->dma)) {
-			dev_err(&pdev->dev, "DMA engine request failed\n");
-			err = PTR_ERR(info->dma);
-			goto return_error;
-		} else {
-			struct dma_slave_config cfg;
-
-			memset(&cfg, 0, sizeof(cfg));
-			cfg.src_addr = info->phys_base;
-			cfg.dst_addr = info->phys_base;
-			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-			cfg.src_maxburst = 16;
-			cfg.dst_maxburst = 16;
-			err = dmaengine_slave_config(info->dma, &cfg);
-			if (err) {
-				dev_err(&pdev->dev, "DMA engine slave config failed: %d\n",
-					err);
-				goto return_error;
-			}
-			nand_chip->read_buf   = omap_read_buf_dma_pref;
-			nand_chip->write_buf  = omap_write_buf_dma_pref;
-		}
-		break;
-
-	case NAND_OMAP_PREFETCH_IRQ:
-		info->gpmc_irq_fifo = platform_get_irq(pdev, 0);
-		if (info->gpmc_irq_fifo <= 0) {
-			dev_err(&pdev->dev, "error getting fifo irq\n");
-			err = -ENODEV;
-			goto return_error;
-		}
-		err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
-					omap_nand_irq, IRQF_SHARED,
-					"gpmc-nand-fifo", info);
-		if (err) {
-			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
-						info->gpmc_irq_fifo, err);
-			info->gpmc_irq_fifo = 0;
-			goto return_error;
-		}
-
-		info->gpmc_irq_count = platform_get_irq(pdev, 1);
-		if (info->gpmc_irq_count <= 0) {
-			dev_err(&pdev->dev, "error getting count irq\n");
-			err = -ENODEV;
-			goto return_error;
-		}
-		err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
-					omap_nand_irq, IRQF_SHARED,
-					"gpmc-nand-count", info);
-		if (err) {
-			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
-						info->gpmc_irq_count, err);
-			info->gpmc_irq_count = 0;
-			goto return_error;
-		}
-
-		nand_chip->read_buf  = omap_read_buf_irq_pref;
-		nand_chip->write_buf = omap_write_buf_irq_pref;
-
-		break;
-
-	default:
-		dev_err(&pdev->dev,
-			"xfer_type(%d) not supported!\n", info->xfer_type);
-		err = -EINVAL;
-		goto return_error;
-	}
-
-	if (!omap2_nand_ecc_check(info)) {
-		err = -EINVAL;
-		goto return_error;
-	}
-
-	/*
-	 * Bail out earlier to let NAND_ECC_SOFT code create its own
-	 * ooblayout instead of using ours.
-	 */
-	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
-		nand_chip->ecc.mode = NAND_ECC_SOFT;
-		nand_chip->ecc.algo = NAND_ECC_HAMMING;
-		goto scan_tail;
-	}
-
-	/* populate MTD interface based on ECC scheme */
-	switch (info->ecc_opt) {
-	case OMAP_ECC_HAM1_CODE_HW:
-		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
-		nand_chip->ecc.mode             = NAND_ECC_HW;
-		nand_chip->ecc.bytes            = 3;
-		nand_chip->ecc.size             = 512;
-		nand_chip->ecc.strength         = 1;
-		nand_chip->ecc.calculate        = omap_calculate_ecc;
-		nand_chip->ecc.hwctl            = omap_enable_hwecc;
-		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:
-		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
-		nand_chip->ecc.mode		= NAND_ECC_HW;
-		nand_chip->ecc.size		= 512;
-		nand_chip->ecc.bytes		= 7;
-		nand_chip->ecc.strength		= 4;
-		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		nand_chip->ecc.correct		= nand_bch_correct_data;
-		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
-		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) {
-			dev_err(&info->pdev->dev, "unable to use BCH library\n");
-			err = -EINVAL;
-			goto return_error;
-		}
-		break;
-
-	case OMAP_ECC_BCH4_CODE_HW:
-		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
-		nand_chip->ecc.mode		= NAND_ECC_HW;
-		nand_chip->ecc.size		= 512;
-		/* 14th bit is kept reserved for ROM-code compatibility */
-		nand_chip->ecc.bytes		= 7 + 1;
-		nand_chip->ecc.strength		= 4;
-		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		nand_chip->ecc.correct		= omap_elm_correct_data;
-		nand_chip->ecc.read_page	= omap_read_page_bch;
-		nand_chip->ecc.write_page	= omap_write_page_bch;
-		nand_chip->ecc.write_subpage	= omap_write_subpage_bch;
-		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,
-				 nand_chip->ecc.size, nand_chip->ecc.bytes);
-		if (err < 0)
-			goto return_error;
-		break;
-
-	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
-		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
-		nand_chip->ecc.mode		= NAND_ECC_HW;
-		nand_chip->ecc.size		= 512;
-		nand_chip->ecc.bytes		= 13;
-		nand_chip->ecc.strength		= 8;
-		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		nand_chip->ecc.correct		= nand_bch_correct_data;
-		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
-		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) {
-			dev_err(&info->pdev->dev, "unable to use BCH library\n");
-			err = -EINVAL;
-			goto return_error;
-		}
-		break;
-
-	case OMAP_ECC_BCH8_CODE_HW:
-		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
-		nand_chip->ecc.mode		= NAND_ECC_HW;
-		nand_chip->ecc.size		= 512;
-		/* 14th bit is kept reserved for ROM-code compatibility */
-		nand_chip->ecc.bytes		= 13 + 1;
-		nand_chip->ecc.strength		= 8;
-		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		nand_chip->ecc.correct		= omap_elm_correct_data;
-		nand_chip->ecc.read_page	= omap_read_page_bch;
-		nand_chip->ecc.write_page	= omap_write_page_bch;
-		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
-				 nand_chip->ecc.size, nand_chip->ecc.bytes);
-		if (err < 0)
-			goto return_error;
-
-		break;
-
-	case OMAP_ECC_BCH16_CODE_HW:
-		pr_info("using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
-		nand_chip->ecc.mode		= NAND_ECC_HW;
-		nand_chip->ecc.size		= 512;
-		nand_chip->ecc.bytes		= 26;
-		nand_chip->ecc.strength		= 16;
-		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		nand_chip->ecc.correct		= omap_elm_correct_data;
-		nand_chip->ecc.read_page	= omap_read_page_bch;
-		nand_chip->ecc.write_page	= omap_write_page_bch;
-		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
-				 nand_chip->ecc.size, nand_chip->ecc.bytes);
-		if (err < 0)
-			goto return_error;
-
-		break;
-	default:
-		dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
-		err = -EINVAL;
-		goto return_error;
-	}
-
-	/* check if NAND device's OOB is enough to store ECC signatures */
-	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",
-			min_oobbytes, mtd->oobsize);
-		err = -EINVAL;
-		goto return_error;
-	}
-
-scan_tail:
-	/* second phase scan */
-	err = nand_scan_tail(mtd);
+	err = nand_scan(mtd, 1);
 	if (err)
 		goto return_error;
 
-- 
2.14.1

Re: [PATCH v5 04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:39 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/omap2.c | 533 +++++++++++++++++++++----------------------
>  1 file changed, 265 insertions(+), 268 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
> index e943b2e5a5e2..4546ac0bed4a 100644
> --- a/drivers/mtd/nand/raw/omap2.c
> +++ b/drivers/mtd/nand/raw/omap2.c
> @@ -144,12 +144,6 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
>  	0xac, 0x6b, 0xff, 0x99, 0x7b};
>  static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
>  
> -/* Shared among all NAND instances to synchronize access to the ECC Engine */
> -static struct nand_controller omap_gpmc_controller = {
> -	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
> -	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
> -};
> -
>  struct omap_nand_info {
>  	struct nand_chip		nand;
>  	struct platform_device		*pdev;
> @@ -1915,17 +1909,278 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
>  	.free = omap_sw_ooblayout_free,
>  };
>  
> +static int omap_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct omap_nand_info *info = mtd_to_omap(mtd);
> +	struct device *dev = &info->pdev->dev;
> +	int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> +	int oobbytes_per_step;
> +	dma_cap_mask_t mask;
> +	int err;
> +
> +	if (chip->bbt_options & NAND_BBT_USE_FLASH)
> +		chip->bbt_options |= NAND_BBT_NO_OOB;
> +	else
> +		chip->options |= NAND_SKIP_BBTSCAN;
> +
> +	/* Re-populate low-level callbacks based on xfer modes */
> +	switch (info->xfer_type) {
> +	case NAND_OMAP_PREFETCH_POLLED:
> +		chip->read_buf = omap_read_buf_pref;
> +		chip->write_buf = omap_write_buf_pref;
> +		break;
> +
> +	case NAND_OMAP_POLLED:
> +		/* Use nand_base defaults for {read,write}_buf */
> +		break;
> +
> +	case NAND_OMAP_PREFETCH_DMA:
> +		dma_cap_zero(mask);
> +		dma_cap_set(DMA_SLAVE, mask);
> +		info->dma = dma_request_chan(dev, "rxtx");
> +
> +		if (IS_ERR(info->dma)) {
> +			dev_err(dev, "DMA engine request failed\n");
> +			return PTR_ERR(info->dma);
> +		} else {
> +			struct dma_slave_config cfg;
> +
> +			memset(&cfg, 0, sizeof(cfg));
> +			cfg.src_addr = info->phys_base;
> +			cfg.dst_addr = info->phys_base;
> +			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			cfg.src_maxburst = 16;
> +			cfg.dst_maxburst = 16;
> +			err = dmaengine_slave_config(info->dma, &cfg);
> +			if (err) {
> +				dev_err(dev,
> +					"DMA engine slave config failed: %d\n",
> +					err);
> +				return err;
> +			}
> +			chip->read_buf = omap_read_buf_dma_pref;
> +			chip->write_buf = omap_write_buf_dma_pref;
> +		}
> +		break;
> +
> +	case NAND_OMAP_PREFETCH_IRQ:
> +		info->gpmc_irq_fifo = platform_get_irq(info->pdev, 0);
> +		if (info->gpmc_irq_fifo <= 0) {
> +			dev_err(dev, "Error getting fifo IRQ\n");
> +			return -ENODEV;
> +		}
> +		err = devm_request_irq(dev, info->gpmc_irq_fifo,
> +				       omap_nand_irq, IRQF_SHARED,
> +				       "gpmc-nand-fifo", info);
> +		if (err) {
> +			dev_err(dev, "Requesting IRQ %d, error %d\n",
> +				info->gpmc_irq_fifo, err);
> +			info->gpmc_irq_fifo = 0;
> +			return err;
> +		}
> +
> +		info->gpmc_irq_count = platform_get_irq(info->pdev, 1);
> +		if (info->gpmc_irq_count <= 0) {
> +			dev_err(dev, "Error getting IRQ count\n");
> +			return -ENODEV;
> +		}
> +		err = devm_request_irq(dev, info->gpmc_irq_count,
> +				       omap_nand_irq, IRQF_SHARED,
> +				       "gpmc-nand-count", info);
> +		if (err) {
> +			dev_err(dev, "Requesting IRQ %d, error %d\n",
> +				info->gpmc_irq_count, err);
> +			info->gpmc_irq_count = 0;
> +			return err;
> +		}
> +
> +		chip->read_buf = omap_read_buf_irq_pref;
> +		chip->write_buf = omap_write_buf_irq_pref;
> +
> +		break;
> +
> +	default:
> +		dev_err(dev, "xfer_type %d not supported!\n", info->xfer_type);
> +		return -EINVAL;
> +	}
> +
> +	if (!omap2_nand_ecc_check(info))
> +		return -EINVAL;
> +
> +	/*
> +	 * Bail out earlier to let NAND_ECC_SOFT code create its own
> +	 * ooblayout instead of using ours.
> +	 */
> +	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
> +		chip->ecc.mode = NAND_ECC_SOFT;
> +		chip->ecc.algo = NAND_ECC_HAMMING;
> +		return 0;
> +	}
> +
> +	/* Populate MTD interface based on ECC scheme */
> +	switch (info->ecc_opt) {
> +	case OMAP_ECC_HAM1_CODE_HW:
> +		dev_info(dev, "nand: using OMAP_ECC_HAM1_CODE_HW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.bytes		= 3;
> +		chip->ecc.size		= 512;
> +		chip->ecc.strength	= 1;
> +		chip->ecc.calculate	= omap_calculate_ecc;
> +		chip->ecc.hwctl		= omap_enable_hwecc;
> +		chip->ecc.correct	= omap_correct_data;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		if (!(chip->options & NAND_BUSWIDTH_16))
> +			min_oobbytes	= 1;
> +
> +		break;
> +
> +	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> +		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 7;
> +		chip->ecc.strength	= 4;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= nand_bch_correct_data;
> +		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step	= chip->ecc.bytes + 1;
> +		/* Software BCH library is used for locating errors */
> +		chip->ecc.priv		= nand_bch_init(mtd);
> +		if (!chip->ecc.priv) {
> +			dev_err(dev, "Unable to use BCH library\n");
> +			return -EINVAL;
> +		}
> +		break;
> +
> +	case OMAP_ECC_BCH4_CODE_HW:
> +		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		/* 14th bit is kept reserved for ROM-code compatibility */
> +		chip->ecc.bytes		= 7 + 1;
> +		chip->ecc.strength	= 4;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH4_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +		break;
> +
> +	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> +		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 13;
> +		chip->ecc.strength	= 8;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= nand_bch_correct_data;
> +		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step	= chip->ecc.bytes + 1;
> +		/* Software BCH library is used for locating errors */
> +		chip->ecc.priv		= nand_bch_init(mtd);
> +		if (!chip->ecc.priv) {
> +			dev_err(dev, "unable to use BCH library\n");
> +			return -EINVAL;
> +		}
> +		break;
> +
> +	case OMAP_ECC_BCH8_CODE_HW:
> +		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		/* 14th bit is kept reserved for ROM-code compatibility */
> +		chip->ecc.bytes		= 13 + 1;
> +		chip->ecc.strength	= 8;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH8_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +
> +	case OMAP_ECC_BCH16_CODE_HW:
> +		pr_info("Using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 26;
> +		chip->ecc.strength	= 16;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH16_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +	default:
> +		dev_err(dev, "Invalid or unsupported ECC scheme\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check if NAND device's OOB is enough to store ECC signatures */
> +	min_oobbytes += (oobbytes_per_step *
> +			 (mtd->writesize / chip->ecc.size));
> +	if (mtd->oobsize < min_oobbytes) {
> +		dev_err(dev,
> +			"Not enough OOB bytes: required = %d, available=%d\n",
> +			min_oobbytes, mtd->oobsize);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops omap_nand_controller_ops = {
> +	.attach_chip = omap_nand_attach_chip,
> +};
> +
> +/* Shared among all NAND instances to synchronize access to the ECC Engine */
> +static struct nand_controller omap_gpmc_controller = {
> +	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
> +	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
> +	.ops = &omap_nand_controller_ops,
> +};
> +
>  static int omap_nand_probe(struct platform_device *pdev)
>  {
>  	struct omap_nand_info		*info;
>  	struct mtd_info			*mtd;
>  	struct nand_chip		*nand_chip;
>  	int				err;
> -	dma_cap_mask_t			mask;
>  	struct resource			*res;
>  	struct device			*dev = &pdev->dev;
> -	int				min_oobbytes = BADBLOCK_MARKER_LENGTH;
> -	int				oobbytes_per_step;
>  
>  	info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
>  				GFP_KERNEL);
> @@ -1998,266 +2253,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  
>  	/* scan NAND device connected to chip controller */
>  	nand_chip->options |= info->devsize & NAND_BUSWIDTH_16;
> -	err = nand_scan_ident(mtd, 1, NULL);
> -	if (err) {
> -		dev_err(&info->pdev->dev,
> -			"scan failed, may be bus-width mismatch\n");
> -		goto return_error;
> -	}
>  
> -	if (nand_chip->bbt_options & NAND_BBT_USE_FLASH)
> -		nand_chip->bbt_options |= NAND_BBT_NO_OOB;
> -	else
> -		nand_chip->options |= NAND_SKIP_BBTSCAN;
> -
> -	/* re-populate low-level callbacks based on xfer modes */
> -	switch (info->xfer_type) {
> -	case NAND_OMAP_PREFETCH_POLLED:
> -		nand_chip->read_buf   = omap_read_buf_pref;
> -		nand_chip->write_buf  = omap_write_buf_pref;
> -		break;
> -
> -	case NAND_OMAP_POLLED:
> -		/* Use nand_base defaults for {read,write}_buf */
> -		break;
> -
> -	case NAND_OMAP_PREFETCH_DMA:
> -		dma_cap_zero(mask);
> -		dma_cap_set(DMA_SLAVE, mask);
> -		info->dma = dma_request_chan(pdev->dev.parent, "rxtx");
> -
> -		if (IS_ERR(info->dma)) {
> -			dev_err(&pdev->dev, "DMA engine request failed\n");
> -			err = PTR_ERR(info->dma);
> -			goto return_error;
> -		} else {
> -			struct dma_slave_config cfg;
> -
> -			memset(&cfg, 0, sizeof(cfg));
> -			cfg.src_addr = info->phys_base;
> -			cfg.dst_addr = info->phys_base;
> -			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -			cfg.src_maxburst = 16;
> -			cfg.dst_maxburst = 16;
> -			err = dmaengine_slave_config(info->dma, &cfg);
> -			if (err) {
> -				dev_err(&pdev->dev, "DMA engine slave config failed: %d\n",
> -					err);
> -				goto return_error;
> -			}
> -			nand_chip->read_buf   = omap_read_buf_dma_pref;
> -			nand_chip->write_buf  = omap_write_buf_dma_pref;
> -		}
> -		break;
> -
> -	case NAND_OMAP_PREFETCH_IRQ:
> -		info->gpmc_irq_fifo = platform_get_irq(pdev, 0);
> -		if (info->gpmc_irq_fifo <= 0) {
> -			dev_err(&pdev->dev, "error getting fifo irq\n");
> -			err = -ENODEV;
> -			goto return_error;
> -		}
> -		err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
> -					omap_nand_irq, IRQF_SHARED,
> -					"gpmc-nand-fifo", info);
> -		if (err) {
> -			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
> -						info->gpmc_irq_fifo, err);
> -			info->gpmc_irq_fifo = 0;
> -			goto return_error;
> -		}
> -
> -		info->gpmc_irq_count = platform_get_irq(pdev, 1);
> -		if (info->gpmc_irq_count <= 0) {
> -			dev_err(&pdev->dev, "error getting count irq\n");
> -			err = -ENODEV;
> -			goto return_error;
> -		}
> -		err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
> -					omap_nand_irq, IRQF_SHARED,
> -					"gpmc-nand-count", info);
> -		if (err) {
> -			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
> -						info->gpmc_irq_count, err);
> -			info->gpmc_irq_count = 0;
> -			goto return_error;
> -		}
> -
> -		nand_chip->read_buf  = omap_read_buf_irq_pref;
> -		nand_chip->write_buf = omap_write_buf_irq_pref;
> -
> -		break;
> -
> -	default:
> -		dev_err(&pdev->dev,
> -			"xfer_type(%d) not supported!\n", info->xfer_type);
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	if (!omap2_nand_ecc_check(info)) {
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	/*
> -	 * Bail out earlier to let NAND_ECC_SOFT code create its own
> -	 * ooblayout instead of using ours.
> -	 */
> -	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
> -		nand_chip->ecc.mode = NAND_ECC_SOFT;
> -		nand_chip->ecc.algo = NAND_ECC_HAMMING;
> -		goto scan_tail;
> -	}
> -
> -	/* populate MTD interface based on ECC scheme */
> -	switch (info->ecc_opt) {
> -	case OMAP_ECC_HAM1_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
> -		nand_chip->ecc.mode             = NAND_ECC_HW;
> -		nand_chip->ecc.bytes            = 3;
> -		nand_chip->ecc.size             = 512;
> -		nand_chip->ecc.strength         = 1;
> -		nand_chip->ecc.calculate        = omap_calculate_ecc;
> -		nand_chip->ecc.hwctl            = omap_enable_hwecc;
> -		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:
> -		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 7;
> -		nand_chip->ecc.strength		= 4;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= nand_bch_correct_data;
> -		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> -		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) {
> -			dev_err(&info->pdev->dev, "unable to use BCH library\n");
> -			err = -EINVAL;
> -			goto return_error;
> -		}
> -		break;
> -
> -	case OMAP_ECC_BCH4_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		/* 14th bit is kept reserved for ROM-code compatibility */
> -		nand_chip->ecc.bytes		= 7 + 1;
> -		nand_chip->ecc.strength		= 4;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_bch;
> -		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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -		break;
> -
> -	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> -		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 13;
> -		nand_chip->ecc.strength		= 8;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= nand_bch_correct_data;
> -		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> -		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) {
> -			dev_err(&info->pdev->dev, "unable to use BCH library\n");
> -			err = -EINVAL;
> -			goto return_error;
> -		}
> -		break;
> -
> -	case OMAP_ECC_BCH8_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		/* 14th bit is kept reserved for ROM-code compatibility */
> -		nand_chip->ecc.bytes		= 13 + 1;
> -		nand_chip->ecc.strength		= 8;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -
> -		break;
> -
> -	case OMAP_ECC_BCH16_CODE_HW:
> -		pr_info("using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 26;
> -		nand_chip->ecc.strength		= 16;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -
> -		break;
> -	default:
> -		dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	/* check if NAND device's OOB is enough to store ECC signatures */
> -	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",
> -			min_oobbytes, mtd->oobsize);
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -scan_tail:
> -	/* second phase scan */
> -	err = nand_scan_tail(mtd);
> +	err = nand_scan(mtd, 1);
>  	if (err)
>  		goto return_error;
>  

Re: [v5,04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Alexander Sverdlin]

Hello Miquel, Boris,

On Wed, 25 Jul 2018 15:31:39 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

I've bisected this patch to brake Beagle Bone Black boot
(from NAND, at least in DMA mode):

[    0.243337] edma 49000000.edma: TI EDMA DMA engine driver

... skipped...

[    1.888170] omap-gpmc 50000000.gpmc: GPMC revision 6.0
[    1.893597] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
[    1.901776] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xda
[    1.908589] nand: Micron MT29F2G08ABAEAWP
[    1.912807] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    1.920802] omap2-nand 8000000.nand: DMA engine request failed

... unrelated...

[    1.985544] UBI error: cannot open mtd NAND.root-squashfs2, error -2
[    1.992432] UBI error: cannot open mtd NAND.root-ubifs, error -2
[    1.998897] UBI: block: can't open volume on ubi0_0, err=-19

... unrelated...

[    2.025751] VFS: Cannot open root device "ubiblock0_0" or unknown-block(0,0): error -6
[    2.034168] Please append a correct "root=" boot option; here are the available partitions:
[    2.042997] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)

EDMA driver is here, yet the channel cannot be obtained after
commit e1e6255c31.

> ---
>  drivers/mtd/nand/raw/omap2.c | 533 +++++++++++++++++++++----------------------
>  1 file changed, 265 insertions(+), 268 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
> index e943b2e5a5e2..4546ac0bed4a 100644
> --- a/drivers/mtd/nand/raw/omap2.c
> +++ b/drivers/mtd/nand/raw/omap2.c
> @@ -144,12 +144,6 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
>  	0xac, 0x6b, 0xff, 0x99, 0x7b};
>  static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
>  
> -/* Shared among all NAND instances to synchronize access to the ECC Engine */
> -static struct nand_controller omap_gpmc_controller = {
> -	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
> -	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
> -};
> -
>  struct omap_nand_info {
>  	struct nand_chip		nand;
>  	struct platform_device		*pdev;
> @@ -1915,17 +1909,278 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
>  	.free = omap_sw_ooblayout_free,
>  };
>  
> +static int omap_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct omap_nand_info *info = mtd_to_omap(mtd);
> +	struct device *dev = &info->pdev->dev;
> +	int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> +	int oobbytes_per_step;
> +	dma_cap_mask_t mask;
> +	int err;
> +
> +	if (chip->bbt_options & NAND_BBT_USE_FLASH)
> +		chip->bbt_options |= NAND_BBT_NO_OOB;
> +	else
> +		chip->options |= NAND_SKIP_BBTSCAN;
> +
> +	/* Re-populate low-level callbacks based on xfer modes */
> +	switch (info->xfer_type) {
> +	case NAND_OMAP_PREFETCH_POLLED:
> +		chip->read_buf = omap_read_buf_pref;
> +		chip->write_buf = omap_write_buf_pref;
> +		break;
> +
> +	case NAND_OMAP_POLLED:
> +		/* Use nand_base defaults for {read,write}_buf */
> +		break;
> +
> +	case NAND_OMAP_PREFETCH_DMA:
> +		dma_cap_zero(mask);
> +		dma_cap_set(DMA_SLAVE, mask);
> +		info->dma = dma_request_chan(dev, "rxtx");
> +
> +		if (IS_ERR(info->dma)) {
> +			dev_err(dev, "DMA engine request failed\n");
> +			return PTR_ERR(info->dma);
> +		} else {
> +			struct dma_slave_config cfg;
> +
> +			memset(&cfg, 0, sizeof(cfg));
> +			cfg.src_addr = info->phys_base;
> +			cfg.dst_addr = info->phys_base;
> +			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			cfg.src_maxburst = 16;
> +			cfg.dst_maxburst = 16;
> +			err = dmaengine_slave_config(info->dma, &cfg);
> +			if (err) {
> +				dev_err(dev,
> +					"DMA engine slave config failed: %d\n",
> +					err);
> +				return err;
> +			}
> +			chip->read_buf = omap_read_buf_dma_pref;
> +			chip->write_buf = omap_write_buf_dma_pref;
> +		}
> +		break;
> +
> +	case NAND_OMAP_PREFETCH_IRQ:
> +		info->gpmc_irq_fifo = platform_get_irq(info->pdev, 0);
> +		if (info->gpmc_irq_fifo <= 0) {
> +			dev_err(dev, "Error getting fifo IRQ\n");
> +			return -ENODEV;
> +		}
> +		err = devm_request_irq(dev, info->gpmc_irq_fifo,
> +				       omap_nand_irq, IRQF_SHARED,
> +				       "gpmc-nand-fifo", info);
> +		if (err) {
> +			dev_err(dev, "Requesting IRQ %d, error %d\n",
> +				info->gpmc_irq_fifo, err);
> +			info->gpmc_irq_fifo = 0;
> +			return err;
> +		}
> +
> +		info->gpmc_irq_count = platform_get_irq(info->pdev, 1);
> +		if (info->gpmc_irq_count <= 0) {
> +			dev_err(dev, "Error getting IRQ count\n");
> +			return -ENODEV;
> +		}
> +		err = devm_request_irq(dev, info->gpmc_irq_count,
> +				       omap_nand_irq, IRQF_SHARED,
> +				       "gpmc-nand-count", info);
> +		if (err) {
> +			dev_err(dev, "Requesting IRQ %d, error %d\n",
> +				info->gpmc_irq_count, err);
> +			info->gpmc_irq_count = 0;
> +			return err;
> +		}
> +
> +		chip->read_buf = omap_read_buf_irq_pref;
> +		chip->write_buf = omap_write_buf_irq_pref;
> +
> +		break;
> +
> +	default:
> +		dev_err(dev, "xfer_type %d not supported!\n", info->xfer_type);
> +		return -EINVAL;
> +	}
> +
> +	if (!omap2_nand_ecc_check(info))
> +		return -EINVAL;
> +
> +	/*
> +	 * Bail out earlier to let NAND_ECC_SOFT code create its own
> +	 * ooblayout instead of using ours.
> +	 */
> +	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
> +		chip->ecc.mode = NAND_ECC_SOFT;
> +		chip->ecc.algo = NAND_ECC_HAMMING;
> +		return 0;
> +	}
> +
> +	/* Populate MTD interface based on ECC scheme */
> +	switch (info->ecc_opt) {
> +	case OMAP_ECC_HAM1_CODE_HW:
> +		dev_info(dev, "nand: using OMAP_ECC_HAM1_CODE_HW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.bytes		= 3;
> +		chip->ecc.size		= 512;
> +		chip->ecc.strength	= 1;
> +		chip->ecc.calculate	= omap_calculate_ecc;
> +		chip->ecc.hwctl		= omap_enable_hwecc;
> +		chip->ecc.correct	= omap_correct_data;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		if (!(chip->options & NAND_BUSWIDTH_16))
> +			min_oobbytes	= 1;
> +
> +		break;
> +
> +	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> +		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 7;
> +		chip->ecc.strength	= 4;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= nand_bch_correct_data;
> +		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step	= chip->ecc.bytes + 1;
> +		/* Software BCH library is used for locating errors */
> +		chip->ecc.priv		= nand_bch_init(mtd);
> +		if (!chip->ecc.priv) {
> +			dev_err(dev, "Unable to use BCH library\n");
> +			return -EINVAL;
> +		}
> +		break;
> +
> +	case OMAP_ECC_BCH4_CODE_HW:
> +		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		/* 14th bit is kept reserved for ROM-code compatibility */
> +		chip->ecc.bytes		= 7 + 1;
> +		chip->ecc.strength	= 4;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH4_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +		break;
> +
> +	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> +		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 13;
> +		chip->ecc.strength	= 8;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= nand_bch_correct_data;
> +		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> +		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
> +		/* Reserve one byte for the OMAP marker */
> +		oobbytes_per_step	= chip->ecc.bytes + 1;
> +		/* Software BCH library is used for locating errors */
> +		chip->ecc.priv		= nand_bch_init(mtd);
> +		if (!chip->ecc.priv) {
> +			dev_err(dev, "unable to use BCH library\n");
> +			return -EINVAL;
> +		}
> +		break;
> +
> +	case OMAP_ECC_BCH8_CODE_HW:
> +		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		/* 14th bit is kept reserved for ROM-code compatibility */
> +		chip->ecc.bytes		= 13 + 1;
> +		chip->ecc.strength	= 8;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH8_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +
> +	case OMAP_ECC_BCH16_CODE_HW:
> +		pr_info("Using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
> +		chip->ecc.mode		= NAND_ECC_HW;
> +		chip->ecc.size		= 512;
> +		chip->ecc.bytes		= 26;
> +		chip->ecc.strength	= 16;
> +		chip->ecc.hwctl		= omap_enable_hwecc_bch;
> +		chip->ecc.correct	= omap_elm_correct_data;
> +		chip->ecc.read_page	= omap_read_page_bch;
> +		chip->ecc.write_page	= omap_write_page_bch;
> +		chip->ecc.write_subpage	= omap_write_subpage_bch;
> +		mtd_set_ooblayout(mtd, &omap_ooblayout_ops);
> +		oobbytes_per_step	= chip->ecc.bytes;
> +
> +		err = elm_config(info->elm_dev, BCH16_ECC,
> +				 mtd->writesize / chip->ecc.size,
> +				 chip->ecc.size, chip->ecc.bytes);
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +	default:
> +		dev_err(dev, "Invalid or unsupported ECC scheme\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check if NAND device's OOB is enough to store ECC signatures */
> +	min_oobbytes += (oobbytes_per_step *
> +			 (mtd->writesize / chip->ecc.size));
> +	if (mtd->oobsize < min_oobbytes) {
> +		dev_err(dev,
> +			"Not enough OOB bytes: required = %d, available=%d\n",
> +			min_oobbytes, mtd->oobsize);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops omap_nand_controller_ops = {
> +	.attach_chip = omap_nand_attach_chip,
> +};
> +
> +/* Shared among all NAND instances to synchronize access to the ECC Engine */
> +static struct nand_controller omap_gpmc_controller = {
> +	.lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock),
> +	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq),
> +	.ops = &omap_nand_controller_ops,
> +};
> +
>  static int omap_nand_probe(struct platform_device *pdev)
>  {
>  	struct omap_nand_info		*info;
>  	struct mtd_info			*mtd;
>  	struct nand_chip		*nand_chip;
>  	int				err;
> -	dma_cap_mask_t			mask;
>  	struct resource			*res;
>  	struct device			*dev = &pdev->dev;
> -	int				min_oobbytes = BADBLOCK_MARKER_LENGTH;
> -	int				oobbytes_per_step;
>  
>  	info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
>  				GFP_KERNEL);
> @@ -1998,266 +2253,8 @@ static int omap_nand_probe(struct platform_device *pdev)
>  
>  	/* scan NAND device connected to chip controller */
>  	nand_chip->options |= info->devsize & NAND_BUSWIDTH_16;
> -	err = nand_scan_ident(mtd, 1, NULL);
> -	if (err) {
> -		dev_err(&info->pdev->dev,
> -			"scan failed, may be bus-width mismatch\n");
> -		goto return_error;
> -	}
>  
> -	if (nand_chip->bbt_options & NAND_BBT_USE_FLASH)
> -		nand_chip->bbt_options |= NAND_BBT_NO_OOB;
> -	else
> -		nand_chip->options |= NAND_SKIP_BBTSCAN;
> -
> -	/* re-populate low-level callbacks based on xfer modes */
> -	switch (info->xfer_type) {
> -	case NAND_OMAP_PREFETCH_POLLED:
> -		nand_chip->read_buf   = omap_read_buf_pref;
> -		nand_chip->write_buf  = omap_write_buf_pref;
> -		break;
> -
> -	case NAND_OMAP_POLLED:
> -		/* Use nand_base defaults for {read,write}_buf */
> -		break;
> -
> -	case NAND_OMAP_PREFETCH_DMA:
> -		dma_cap_zero(mask);
> -		dma_cap_set(DMA_SLAVE, mask);
> -		info->dma = dma_request_chan(pdev->dev.parent, "rxtx");
> -
> -		if (IS_ERR(info->dma)) {
> -			dev_err(&pdev->dev, "DMA engine request failed\n");
> -			err = PTR_ERR(info->dma);
> -			goto return_error;
> -		} else {
> -			struct dma_slave_config cfg;
> -
> -			memset(&cfg, 0, sizeof(cfg));
> -			cfg.src_addr = info->phys_base;
> -			cfg.dst_addr = info->phys_base;
> -			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -			cfg.src_maxburst = 16;
> -			cfg.dst_maxburst = 16;
> -			err = dmaengine_slave_config(info->dma, &cfg);
> -			if (err) {
> -				dev_err(&pdev->dev, "DMA engine slave config failed: %d\n",
> -					err);
> -				goto return_error;
> -			}
> -			nand_chip->read_buf   = omap_read_buf_dma_pref;
> -			nand_chip->write_buf  = omap_write_buf_dma_pref;
> -		}
> -		break;
> -
> -	case NAND_OMAP_PREFETCH_IRQ:
> -		info->gpmc_irq_fifo = platform_get_irq(pdev, 0);
> -		if (info->gpmc_irq_fifo <= 0) {
> -			dev_err(&pdev->dev, "error getting fifo irq\n");
> -			err = -ENODEV;
> -			goto return_error;
> -		}
> -		err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
> -					omap_nand_irq, IRQF_SHARED,
> -					"gpmc-nand-fifo", info);
> -		if (err) {
> -			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
> -						info->gpmc_irq_fifo, err);
> -			info->gpmc_irq_fifo = 0;
> -			goto return_error;
> -		}
> -
> -		info->gpmc_irq_count = platform_get_irq(pdev, 1);
> -		if (info->gpmc_irq_count <= 0) {
> -			dev_err(&pdev->dev, "error getting count irq\n");
> -			err = -ENODEV;
> -			goto return_error;
> -		}
> -		err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
> -					omap_nand_irq, IRQF_SHARED,
> -					"gpmc-nand-count", info);
> -		if (err) {
> -			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
> -						info->gpmc_irq_count, err);
> -			info->gpmc_irq_count = 0;
> -			goto return_error;
> -		}
> -
> -		nand_chip->read_buf  = omap_read_buf_irq_pref;
> -		nand_chip->write_buf = omap_write_buf_irq_pref;
> -
> -		break;
> -
> -	default:
> -		dev_err(&pdev->dev,
> -			"xfer_type(%d) not supported!\n", info->xfer_type);
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	if (!omap2_nand_ecc_check(info)) {
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	/*
> -	 * Bail out earlier to let NAND_ECC_SOFT code create its own
> -	 * ooblayout instead of using ours.
> -	 */
> -	if (info->ecc_opt == OMAP_ECC_HAM1_CODE_SW) {
> -		nand_chip->ecc.mode = NAND_ECC_SOFT;
> -		nand_chip->ecc.algo = NAND_ECC_HAMMING;
> -		goto scan_tail;
> -	}
> -
> -	/* populate MTD interface based on ECC scheme */
> -	switch (info->ecc_opt) {
> -	case OMAP_ECC_HAM1_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
> -		nand_chip->ecc.mode             = NAND_ECC_HW;
> -		nand_chip->ecc.bytes            = 3;
> -		nand_chip->ecc.size             = 512;
> -		nand_chip->ecc.strength         = 1;
> -		nand_chip->ecc.calculate        = omap_calculate_ecc;
> -		nand_chip->ecc.hwctl            = omap_enable_hwecc;
> -		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:
> -		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 7;
> -		nand_chip->ecc.strength		= 4;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= nand_bch_correct_data;
> -		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> -		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) {
> -			dev_err(&info->pdev->dev, "unable to use BCH library\n");
> -			err = -EINVAL;
> -			goto return_error;
> -		}
> -		break;
> -
> -	case OMAP_ECC_BCH4_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		/* 14th bit is kept reserved for ROM-code compatibility */
> -		nand_chip->ecc.bytes		= 7 + 1;
> -		nand_chip->ecc.strength		= 4;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_bch;
> -		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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -		break;
> -
> -	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> -		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 13;
> -		nand_chip->ecc.strength		= 8;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= nand_bch_correct_data;
> -		nand_chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
> -		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) {
> -			dev_err(&info->pdev->dev, "unable to use BCH library\n");
> -			err = -EINVAL;
> -			goto return_error;
> -		}
> -		break;
> -
> -	case OMAP_ECC_BCH8_CODE_HW:
> -		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		/* 14th bit is kept reserved for ROM-code compatibility */
> -		nand_chip->ecc.bytes		= 13 + 1;
> -		nand_chip->ecc.strength		= 8;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -
> -		break;
> -
> -	case OMAP_ECC_BCH16_CODE_HW:
> -		pr_info("using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
> -		nand_chip->ecc.mode		= NAND_ECC_HW;
> -		nand_chip->ecc.size		= 512;
> -		nand_chip->ecc.bytes		= 26;
> -		nand_chip->ecc.strength		= 16;
> -		nand_chip->ecc.hwctl		= omap_enable_hwecc_bch;
> -		nand_chip->ecc.correct		= omap_elm_correct_data;
> -		nand_chip->ecc.read_page	= omap_read_page_bch;
> -		nand_chip->ecc.write_page	= omap_write_page_bch;
> -		nand_chip->ecc.write_subpage	= omap_write_subpage_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,
> -				 nand_chip->ecc.size, nand_chip->ecc.bytes);
> -		if (err < 0)
> -			goto return_error;
> -
> -		break;
> -	default:
> -		dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -	/* check if NAND device's OOB is enough to store ECC signatures */
> -	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",
> -			min_oobbytes, mtd->oobsize);
> -		err = -EINVAL;
> -		goto return_error;
> -	}
> -
> -scan_tail:
> -	/* second phase scan */
> -	err = nand_scan_tail(mtd);
> +	err = nand_scan(mtd, 1);
>  	if (err)
>  		goto return_error;
>  


-- 
Alexander Sverdlin.

Re: [v5,04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Boris Brezillon]

On Thu, 13 Dec 2018 19:01:11 +0100
Alexander Sverdlin <alexander.sverdlin@gmail.com> wrote:

> Hello Miquel, Boris,
> 
> On Wed, 25 Jul 2018 15:31:39 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Two helpers have been added to the core to do all kind of controller
> > side configuration/initialization between the detection phase and the
> > final NAND scan. Implement these hooks so that we can convert the driver
> > to just use nand_scan() instead of the nand_scan_ident() +
> > nand_scan_tail() pair.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>  
> 
> I've bisected this patch to brake Beagle Bone Black boot
> (from NAND, at least in DMA mode):
> 
> [    0.243337] edma 49000000.edma: TI EDMA DMA engine driver
> 
> ... skipped...
> 
> [    1.888170] omap-gpmc 50000000.gpmc: GPMC revision 6.0
> [    1.893597] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> [    1.901776] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xda
> [    1.908589] nand: Micron MT29F2G08ABAEAWP
> [    1.912807] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> [    1.920802] omap2-nand 8000000.nand: DMA engine request failed
> 
> ... unrelated...
> 
> [    1.985544] UBI error: cannot open mtd NAND.root-squashfs2, error -2
> [    1.992432] UBI error: cannot open mtd NAND.root-ubifs, error -2
> [    1.998897] UBI: block: can't open volume on ubi0_0, err=-19
> 
> ... unrelated...
> 
> [    2.025751] VFS: Cannot open root device "ubiblock0_0" or unknown-block(0,0): error -6
> [    2.034168] Please append a correct "root=" boot option; here are the available partitions:
> [    2.042997] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
> 
> EDMA driver is here, yet the channel cannot be obtained after
> commit e1e6255c31.

Can you provide the full logs before and after this commit?

Re: [v5,04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Boris Brezillon]

On Thu, 13 Dec 2018 19:01:11 +0100
Alexander Sverdlin <alexander.sverdlin@gmail.com> wrote:

> > +	/* Re-populate low-level callbacks based on xfer modes */
> > +	switch (info->xfer_type) {
> > +	case NAND_OMAP_PREFETCH_POLLED:
> > +		chip->read_buf = omap_read_buf_pref;
> > +		chip->write_buf = omap_write_buf_pref;
> > +		break;
> > +
> > +	case NAND_OMAP_POLLED:
> > +		/* Use nand_base defaults for {read,write}_buf */
> > +		break;
> > +
> > +	case NAND_OMAP_PREFETCH_DMA:
> > +		dma_cap_zero(mask);
> > +		dma_cap_set(DMA_SLAVE, mask);
> > +		info->dma = dma_request_chan(dev, "rxtx");
> > +
> > +		if (IS_ERR(info->dma)) {
> > +			dev_err(dev, "DMA engine request failed\n");

Can you print the error code here?

Re: [v5,04/17] mtd: rawnand: omap2: convert driver to nand_scan()

[Boris Brezillon]

On Thu, 13 Dec 2018 19:01:11 +0100
Alexander Sverdlin <alexander.sverdlin@gmail.com> wrote:

> > +static int omap_nand_attach_chip(struct nand_chip *chip)
> > +{
> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct omap_nand_info *info = mtd_to_omap(mtd);
> > +	struct device *dev = &info->pdev->dev;
> > +	int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> > +	int oobbytes_per_step;
> > +	dma_cap_mask_t mask;
> > +	int err;
> > +
> > +	if (chip->bbt_options & NAND_BBT_USE_FLASH)
> > +		chip->bbt_options |= NAND_BBT_NO_OOB;
> > +	else
> > +		chip->options |= NAND_SKIP_BBTSCAN;
> > +
> > +	/* Re-populate low-level callbacks based on xfer modes */
> > +	switch (info->xfer_type) {
> > +	case NAND_OMAP_PREFETCH_POLLED:
> > +		chip->read_buf = omap_read_buf_pref;
> > +		chip->write_buf = omap_write_buf_pref;
> > +		break;
> > +
> > +	case NAND_OMAP_POLLED:
> > +		/* Use nand_base defaults for {read,write}_buf */
> > +		break;
> > +
> > +	case NAND_OMAP_PREFETCH_DMA:
> > +		dma_cap_zero(mask);
> > +		dma_cap_set(DMA_SLAVE, mask);
> > +		info->dma = dma_request_chan(dev, "rxtx");
> > +

Can you try with

		info->dma = dma_request_chan(dev->parent, "rxtx");

?

[PATCH v5 05/17] mtd: rawnand: atmel: clarify NAND addition/removal paths

[Miquel Raynal]

No need for an atmel_nand_register() function, let's move the code in
it directly where the function was called: in
atmel_nand_controller_add_nand(). Also, for more coherence, rename
atmel_nand_unregister() as atmel_nand_controller_remove_nand(), as
opposed as the previously mentioned function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 102 ++++++++++++---------------
 1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 4d27401f1299..143d029710f0 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1573,7 +1573,7 @@ static int atmel_nand_detect(struct atmel_nand *nand)
 	return ret;
 }
 
-static int atmel_nand_unregister(struct atmel_nand *nand)
+static int atmel_nand_controller_remove_nand(struct atmel_nand *nand)
 {
 	struct nand_chip *chip = &nand->base;
 	struct mtd_info *mtd = nand_to_mtd(chip);
@@ -1589,60 +1589,6 @@ static int atmel_nand_unregister(struct atmel_nand *nand)
 	return 0;
 }
 
-static int atmel_nand_register(struct atmel_nand *nand)
-{
-	struct nand_chip *chip = &nand->base;
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct atmel_nand_controller *nc;
-	int ret;
-
-	nc = to_nand_controller(chip->controller);
-
-	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
-		/*
-		 * We keep the MTD name unchanged to avoid breaking platforms
-		 * where the MTD cmdline parser is used and the bootloader
-		 * has not been updated to use the new naming scheme.
-		 */
-		mtd->name = "atmel_nand";
-	} else if (!mtd->name) {
-		/*
-		 * If the new bindings are used and the bootloader has not been
-		 * updated to pass a new mtdparts parameter on the cmdline, you
-		 * should define the following property in your nand node:
-		 *
-		 *	label = "atmel_nand";
-		 *
-		 * This way, mtd->name will be set by the core when
-		 * nand_set_flash_node() is called.
-		 */
-		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
-					   "%s:nand.%d", dev_name(nc->dev),
-					   nand->cs[0].id);
-		if (!mtd->name) {
-			dev_err(nc->dev, "Failed to allocate mtd->name\n");
-			return -ENOMEM;
-		}
-	}
-
-	ret = nand_scan_tail(mtd);
-	if (ret) {
-		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
-		return ret;
-	}
-
-	ret = mtd_device_register(mtd, NULL, 0);
-	if (ret) {
-		dev_err(nc->dev, "Failed to register mtd device: %d\n", ret);
-		nand_cleanup(chip);
-		return ret;
-	}
-
-	list_add_tail(&nand->node, &nc->chips);
-
-	return 0;
-}
-
 static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
 					    struct device_node *np,
 					    int reg_cells)
@@ -1772,7 +1718,49 @@ atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
 	if (ret)
 		return ret;
 
-	return atmel_nand_register(nand);
+	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
+		/*
+		 * We keep the MTD name unchanged to avoid breaking platforms
+		 * where the MTD cmdline parser is used and the bootloader
+		 * has not been updated to use the new naming scheme.
+		 */
+		mtd->name = "atmel_nand";
+	} else if (!mtd->name) {
+		/*
+		 * If the new bindings are used and the bootloader has not been
+		 * updated to pass a new mtdparts parameter on the cmdline, you
+		 * should define the following property in your nand node:
+		 *
+		 *	label = "atmel_nand";
+		 *
+		 * This way, mtd->name will be set by the core when
+		 * nand_set_flash_node() is called.
+		 */
+		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
+					   "%s:nand.%d", dev_name(nc->dev),
+					   nand->cs[0].id);
+		if (!mtd->name) {
+			dev_err(nc->dev, "Failed to allocate mtd->name\n");
+			return -ENOMEM;
+		}
+	}
+
+	ret = nand_scan_tail(mtd);
+	if (ret) {
+		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		dev_err(nc->dev, "Failed to register mtd device: %d\n", ret);
+		nand_cleanup(chip);
+		return ret;
+	}
+
+	list_add_tail(&nand->node, &nc->chips);
+
+	return 0;
 }
 
 static int
@@ -1782,7 +1770,7 @@ atmel_nand_controller_remove_nands(struct atmel_nand_controller *nc)
 	int ret;
 
 	list_for_each_entry_safe(nand, tmp, &nc->chips, node) {
-		ret = atmel_nand_unregister(nand);
+		ret = atmel_nand_controller_remove_nand(nand);
 		if (ret)
 			return ret;
 	}
-- 
2.14.1

Re: [PATCH v5 05/17] mtd: rawnand: atmel: clarify NAND addition/removal paths

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:40 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> No need for an atmel_nand_register() function, let's move the code in
> it directly where the function was called: in
> atmel_nand_controller_add_nand(). Also, for more coherence, rename
> atmel_nand_unregister() as atmel_nand_controller_remove_nand(), as
> opposed as the previously mentioned function.

How about replacing the last sentence (which is not clear at all) by:

"
To make things consistent, also rename atmel_nand_unregister() into
atmel_nand_controller_remove_nand().
"

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 102 ++++++++++++---------------
>  1 file changed, 45 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 4d27401f1299..143d029710f0 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1573,7 +1573,7 @@ static int atmel_nand_detect(struct atmel_nand *nand)
>  	return ret;
>  }
>  
> -static int atmel_nand_unregister(struct atmel_nand *nand)
> +static int atmel_nand_controller_remove_nand(struct atmel_nand *nand)
>  {
>  	struct nand_chip *chip = &nand->base;
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> @@ -1589,60 +1589,6 @@ static int atmel_nand_unregister(struct atmel_nand *nand)
>  	return 0;
>  }
>  
> -static int atmel_nand_register(struct atmel_nand *nand)
> -{
> -	struct nand_chip *chip = &nand->base;
> -	struct mtd_info *mtd = nand_to_mtd(chip);
> -	struct atmel_nand_controller *nc;
> -	int ret;
> -
> -	nc = to_nand_controller(chip->controller);
> -
> -	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
> -		/*
> -		 * We keep the MTD name unchanged to avoid breaking platforms
> -		 * where the MTD cmdline parser is used and the bootloader
> -		 * has not been updated to use the new naming scheme.
> -		 */
> -		mtd->name = "atmel_nand";
> -	} else if (!mtd->name) {
> -		/*
> -		 * If the new bindings are used and the bootloader has not been
> -		 * updated to pass a new mtdparts parameter on the cmdline, you
> -		 * should define the following property in your nand node:
> -		 *
> -		 *	label = "atmel_nand";
> -		 *
> -		 * This way, mtd->name will be set by the core when
> -		 * nand_set_flash_node() is called.
> -		 */
> -		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
> -					   "%s:nand.%d", dev_name(nc->dev),
> -					   nand->cs[0].id);
> -		if (!mtd->name) {
> -			dev_err(nc->dev, "Failed to allocate mtd->name\n");
> -			return -ENOMEM;
> -		}
> -	}
> -
> -	ret = nand_scan_tail(mtd);
> -	if (ret) {
> -		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = mtd_device_register(mtd, NULL, 0);
> -	if (ret) {
> -		dev_err(nc->dev, "Failed to register mtd device: %d\n", ret);
> -		nand_cleanup(chip);
> -		return ret;
> -	}
> -
> -	list_add_tail(&nand->node, &nc->chips);
> -
> -	return 0;
> -}
> -
>  static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
>  					    struct device_node *np,
>  					    int reg_cells)
> @@ -1772,7 +1718,49 @@ atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
>  	if (ret)
>  		return ret;
>  
> -	return atmel_nand_register(nand);
> +	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
> +		/*
> +		 * We keep the MTD name unchanged to avoid breaking platforms
> +		 * where the MTD cmdline parser is used and the bootloader
> +		 * has not been updated to use the new naming scheme.
> +		 */
> +		mtd->name = "atmel_nand";
> +	} else if (!mtd->name) {
> +		/*
> +		 * If the new bindings are used and the bootloader has not been
> +		 * updated to pass a new mtdparts parameter on the cmdline, you
> +		 * should define the following property in your nand node:
> +		 *
> +		 *	label = "atmel_nand";
> +		 *
> +		 * This way, mtd->name will be set by the core when
> +		 * nand_set_flash_node() is called.
> +		 */
> +		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
> +					   "%s:nand.%d", dev_name(nc->dev),
> +					   nand->cs[0].id);
> +		if (!mtd->name) {
> +			dev_err(nc->dev, "Failed to allocate mtd->name\n");
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	ret = nand_scan_tail(mtd);
> +	if (ret) {
> +		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = mtd_device_register(mtd, NULL, 0);
> +	if (ret) {
> +		dev_err(nc->dev, "Failed to register mtd device: %d\n", ret);
> +		nand_cleanup(chip);
> +		return ret;
> +	}
> +
> +	list_add_tail(&nand->node, &nc->chips);
> +
> +	return 0;
>  }
>  
>  static int
> @@ -1782,7 +1770,7 @@ atmel_nand_controller_remove_nands(struct atmel_nand_controller *nc)
>  	int ret;
>  
>  	list_for_each_entry_safe(nand, tmp, &nc->chips, node) {
> -		ret = atmel_nand_unregister(nand);
> +		ret = atmel_nand_controller_remove_nand(nand);
>  		if (ret)
>  			return ret;
>  	}

[PATCH v5 06/17] mtd: rawnand: atmel: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 113 +++++++++++++--------------
 1 file changed, 54 insertions(+), 59 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 143d029710f0..a068b214ebaa 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -201,7 +201,7 @@ struct atmel_nand_controller_ops {
 	int (*remove)(struct atmel_nand_controller *nc);
 	void (*nand_init)(struct atmel_nand_controller *nc,
 			  struct atmel_nand *nand);
-	int (*ecc_init)(struct atmel_nand *nand);
+	int (*ecc_init)(struct nand_chip *chip);
 	int (*setup_data_interface)(struct atmel_nand *nand, int csline,
 				    const struct nand_data_interface *conf);
 };
@@ -1132,9 +1132,8 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 	return 0;
 }
 
-static int atmel_nand_ecc_init(struct atmel_nand *nand)
+static int atmel_nand_ecc_init(struct nand_chip *chip)
 {
-	struct nand_chip *chip = &nand->base;
 	struct atmel_nand_controller *nc;
 	int ret;
 
@@ -1169,12 +1168,11 @@ static int atmel_nand_ecc_init(struct atmel_nand *nand)
 	return 0;
 }
 
-static int atmel_hsmc_nand_ecc_init(struct atmel_nand *nand)
+static int atmel_hsmc_nand_ecc_init(struct nand_chip *chip)
 {
-	struct nand_chip *chip = &nand->base;
 	int ret;
 
-	ret = atmel_nand_ecc_init(nand);
+	ret = atmel_nand_ecc_init(chip);
 	if (ret)
 		return ret;
 
@@ -1557,22 +1555,6 @@ static void atmel_hsmc_nand_init(struct atmel_nand_controller *nc,
 	chip->select_chip = atmel_hsmc_nand_select_chip;
 }
 
-static int atmel_nand_detect(struct atmel_nand *nand)
-{
-	struct nand_chip *chip = &nand->base;
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct atmel_nand_controller *nc;
-	int ret;
-
-	nc = to_nand_controller(chip->controller);
-
-	ret = nand_scan_ident(mtd, nand->numcs, NULL);
-	if (ret)
-		dev_err(nc->dev, "nand_scan_ident() failed: %d\n", ret);
-
-	return ret;
-}
-
 static int atmel_nand_controller_remove_nand(struct atmel_nand *nand)
 {
 	struct nand_chip *chip = &nand->base;
@@ -1700,6 +1682,8 @@ static int
 atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
 			       struct atmel_nand *nand)
 {
+	struct nand_chip *chip = &nand->base;
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
 
 	/* No card inserted, skip this NAND. */
@@ -1710,44 +1694,9 @@ atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
 
 	nc->caps->ops->nand_init(nc, nand);
 
-	ret = atmel_nand_detect(nand);
-	if (ret)
-		return ret;
-
-	ret = nc->caps->ops->ecc_init(nand);
-	if (ret)
-		return ret;
-
-	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
-		/*
-		 * We keep the MTD name unchanged to avoid breaking platforms
-		 * where the MTD cmdline parser is used and the bootloader
-		 * has not been updated to use the new naming scheme.
-		 */
-		mtd->name = "atmel_nand";
-	} else if (!mtd->name) {
-		/*
-		 * If the new bindings are used and the bootloader has not been
-		 * updated to pass a new mtdparts parameter on the cmdline, you
-		 * should define the following property in your nand node:
-		 *
-		 *	label = "atmel_nand";
-		 *
-		 * This way, mtd->name will be set by the core when
-		 * nand_set_flash_node() is called.
-		 */
-		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
-					   "%s:nand.%d", dev_name(nc->dev),
-					   nand->cs[0].id);
-		if (!mtd->name) {
-			dev_err(nc->dev, "Failed to allocate mtd->name\n");
-			return -ENOMEM;
-		}
-	}
-
-	ret = nand_scan_tail(mtd);
+	ret = nand_scan(mtd, nand->numcs);
 	if (ret) {
-		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
+		dev_err(nc->dev, "NAND scan failed: %d\n", ret);
 		return ret;
 	}
 
@@ -1945,6 +1894,51 @@ static const struct of_device_id atmel_matrix_of_ids[] = {
 	{ /* sentinel */ },
 };
 
+static int atmel_nand_attach_chip(struct nand_chip *chip)
+{
+	struct atmel_nand_controller *nc = to_nand_controller(chip->controller);
+	struct atmel_nand *nand = to_atmel_nand(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret;
+
+	ret = nc->caps->ops->ecc_init(chip);
+	if (ret)
+		return ret;
+
+	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
+		/*
+		 * We keep the MTD name unchanged to avoid breaking platforms
+		 * where the MTD cmdline parser is used and the bootloader
+		 * has not been updated to use the new naming scheme.
+		 */
+		mtd->name = "atmel_nand";
+	} else if (!mtd->name) {
+		/*
+		 * If the new bindings are used and the bootloader has not been
+		 * updated to pass a new mtdparts parameter on the cmdline, you
+		 * should define the following property in your nand node:
+		 *
+		 *	label = "atmel_nand";
+		 *
+		 * This way, mtd->name will be set by the core when
+		 * nand_set_flash_node() is called.
+		 */
+		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
+					   "%s:nand.%d", dev_name(nc->dev),
+					   nand->cs[0].id);
+		if (!mtd->name) {
+			dev_err(nc->dev, "Failed to allocate mtd->name\n");
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
+static const struct nand_controller_ops atmel_nand_controller_ops = {
+	.attach_chip = atmel_nand_attach_chip,
+};
+
 static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
 				struct platform_device *pdev,
 				const struct atmel_nand_controller_caps *caps)
@@ -1954,6 +1948,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
 	int ret;
 
 	nand_controller_init(&nc->base);
+	nc->base.ops = &atmel_nand_controller_ops;
 	INIT_LIST_HEAD(&nc->chips);
 	nc->dev = dev;
 	nc->caps = caps;
-- 
2.14.1

Re: [PATCH v5 06/17] mtd: rawnand: atmel: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:41 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 113 +++++++++++++--------------
>  1 file changed, 54 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 143d029710f0..a068b214ebaa 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -201,7 +201,7 @@ struct atmel_nand_controller_ops {
>  	int (*remove)(struct atmel_nand_controller *nc);
>  	void (*nand_init)(struct atmel_nand_controller *nc,
>  			  struct atmel_nand *nand);
> -	int (*ecc_init)(struct atmel_nand *nand);
> +	int (*ecc_init)(struct nand_chip *chip);
>  	int (*setup_data_interface)(struct atmel_nand *nand, int csline,
>  				    const struct nand_data_interface *conf);
>  };
> @@ -1132,9 +1132,8 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
>  	return 0;
>  }
>  
> -static int atmel_nand_ecc_init(struct atmel_nand *nand)
> +static int atmel_nand_ecc_init(struct nand_chip *chip)
>  {
> -	struct nand_chip *chip = &nand->base;
>  	struct atmel_nand_controller *nc;
>  	int ret;
>  
> @@ -1169,12 +1168,11 @@ static int atmel_nand_ecc_init(struct atmel_nand *nand)
>  	return 0;
>  }
>  
> -static int atmel_hsmc_nand_ecc_init(struct atmel_nand *nand)
> +static int atmel_hsmc_nand_ecc_init(struct nand_chip *chip)
>  {
> -	struct nand_chip *chip = &nand->base;
>  	int ret;
>  
> -	ret = atmel_nand_ecc_init(nand);
> +	ret = atmel_nand_ecc_init(chip);
>  	if (ret)
>  		return ret;
>  
> @@ -1557,22 +1555,6 @@ static void atmel_hsmc_nand_init(struct atmel_nand_controller *nc,
>  	chip->select_chip = atmel_hsmc_nand_select_chip;
>  }
>  
> -static int atmel_nand_detect(struct atmel_nand *nand)
> -{
> -	struct nand_chip *chip = &nand->base;
> -	struct mtd_info *mtd = nand_to_mtd(chip);
> -	struct atmel_nand_controller *nc;
> -	int ret;
> -
> -	nc = to_nand_controller(chip->controller);
> -
> -	ret = nand_scan_ident(mtd, nand->numcs, NULL);
> -	if (ret)
> -		dev_err(nc->dev, "nand_scan_ident() failed: %d\n", ret);
> -
> -	return ret;
> -}
> -
>  static int atmel_nand_controller_remove_nand(struct atmel_nand *nand)
>  {
>  	struct nand_chip *chip = &nand->base;
> @@ -1700,6 +1682,8 @@ static int
>  atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
>  			       struct atmel_nand *nand)
>  {
> +	struct nand_chip *chip = &nand->base;
> +	struct mtd_info *mtd = nand_to_mtd(chip);
>  	int ret;
>  
>  	/* No card inserted, skip this NAND. */
> @@ -1710,44 +1694,9 @@ atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
>  
>  	nc->caps->ops->nand_init(nc, nand);
>  
> -	ret = atmel_nand_detect(nand);
> -	if (ret)
> -		return ret;
> -
> -	ret = nc->caps->ops->ecc_init(nand);
> -	if (ret)
> -		return ret;
> -
> -	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
> -		/*
> -		 * We keep the MTD name unchanged to avoid breaking platforms
> -		 * where the MTD cmdline parser is used and the bootloader
> -		 * has not been updated to use the new naming scheme.
> -		 */
> -		mtd->name = "atmel_nand";
> -	} else if (!mtd->name) {
> -		/*
> -		 * If the new bindings are used and the bootloader has not been
> -		 * updated to pass a new mtdparts parameter on the cmdline, you
> -		 * should define the following property in your nand node:
> -		 *
> -		 *	label = "atmel_nand";
> -		 *
> -		 * This way, mtd->name will be set by the core when
> -		 * nand_set_flash_node() is called.
> -		 */
> -		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
> -					   "%s:nand.%d", dev_name(nc->dev),
> -					   nand->cs[0].id);
> -		if (!mtd->name) {
> -			dev_err(nc->dev, "Failed to allocate mtd->name\n");
> -			return -ENOMEM;
> -		}
> -	}
> -
> -	ret = nand_scan_tail(mtd);
> +	ret = nand_scan(mtd, nand->numcs);
>  	if (ret) {
> -		dev_err(nc->dev, "nand_scan_tail() failed: %d\n", ret);
> +		dev_err(nc->dev, "NAND scan failed: %d\n", ret);
>  		return ret;
>  	}
>  
> @@ -1945,6 +1894,51 @@ static const struct of_device_id atmel_matrix_of_ids[] = {
>  	{ /* sentinel */ },
>  };
>  
> +static int atmel_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct atmel_nand_controller *nc = to_nand_controller(chip->controller);
> +	struct atmel_nand *nand = to_atmel_nand(chip);
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	int ret;
> +
> +	ret = nc->caps->ops->ecc_init(chip);
> +	if (ret)
> +		return ret;
> +
> +	if (nc->caps->legacy_of_bindings || !nc->dev->of_node) {
> +		/*
> +		 * We keep the MTD name unchanged to avoid breaking platforms
> +		 * where the MTD cmdline parser is used and the bootloader
> +		 * has not been updated to use the new naming scheme.
> +		 */
> +		mtd->name = "atmel_nand";
> +	} else if (!mtd->name) {
> +		/*
> +		 * If the new bindings are used and the bootloader has not been
> +		 * updated to pass a new mtdparts parameter on the cmdline, you
> +		 * should define the following property in your nand node:
> +		 *
> +		 *	label = "atmel_nand";
> +		 *
> +		 * This way, mtd->name will be set by the core when
> +		 * nand_set_flash_node() is called.
> +		 */
> +		mtd->name = devm_kasprintf(nc->dev, GFP_KERNEL,
> +					   "%s:nand.%d", dev_name(nc->dev),
> +					   nand->cs[0].id);
> +		if (!mtd->name) {
> +			dev_err(nc->dev, "Failed to allocate mtd->name\n");
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops atmel_nand_controller_ops = {
> +	.attach_chip = atmel_nand_attach_chip,
> +};
> +
>  static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>  				struct platform_device *pdev,
>  				const struct atmel_nand_controller_caps *caps)
> @@ -1954,6 +1948,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc,
>  	int ret;
>  
>  	nand_controller_init(&nc->base);
> +	nc->base.ops = &atmel_nand_controller_ops;
>  	INIT_LIST_HEAD(&nc->chips);
>  	nc->dev = dev;
>  	nc->caps = caps;

[PATCH v5 07/17] mtd: rawnand: do not execute nand_scan_ident() if maxchips is zero

[Miquel Raynal]

Some driver (eg. docg4) will need to handle themselves the
identification phase. As part of the migration to use nand_scan()
everywhere (which will unconditionnaly call nand_scan_ident()), we add
a condition at the start of nand_scan_with_ids() to jump over
nand_scan_ident() if the maxchips parameters is zero, meaning that the
driver does not want the core to handle this phase.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index dea41fa25be1..42a7a934a17b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -6735,7 +6735,9 @@ static void nand_detach(struct nand_chip *chip)
 /**
  * nand_scan_with_ids - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
- * @maxchips: number of chips to scan for
+ * @maxchips: number of chips to scan for. @nand_scan_ident() will not be run if
+ *	      this parameter is zero (useful for specific drivers that must
+ *	      handle this part of the process themselves, e.g docg4).
  * @ids: optional flash IDs table
  *
  * This fills out all the uninitialized function pointers with the defaults.
@@ -6748,9 +6750,11 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	int ret;
 
-	ret = nand_scan_ident(mtd, maxchips, ids);
-	if (ret)
-		return ret;
+	if (maxchips) {
+		ret = nand_scan_ident(mtd, maxchips, ids);
+		if (ret)
+			return ret;
+	}
 
 	ret = nand_attach(chip);
 	if (ret)
-- 
2.14.1

Re: [PATCH v5 07/17] mtd: rawnand: do not execute nand_scan_ident() if maxchips is zero

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:42 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Some driver (eg. docg4) will need to handle themselves the
> identification phase. As part of the migration to use nand_scan()
> everywhere (which will unconditionnaly call nand_scan_ident()), we add
> a condition at the start of nand_scan_with_ids() to jump over
> nand_scan_ident() if the maxchips parameters is zero, meaning that the
> driver does not want the core to handle this phase.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/nand_base.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index dea41fa25be1..42a7a934a17b 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -6735,7 +6735,9 @@ static void nand_detach(struct nand_chip *chip)
>  /**
>   * nand_scan_with_ids - [NAND Interface] Scan for the NAND device
>   * @mtd: MTD device structure
> - * @maxchips: number of chips to scan for
> + * @maxchips: number of chips to scan for. @nand_scan_ident() will not be run if
> + *	      this parameter is zero (useful for specific drivers that must
> + *	      handle this part of the process themselves, e.g docg4).
>   * @ids: optional flash IDs table
>   *
>   * This fills out all the uninitialized function pointers with the defaults.
> @@ -6748,9 +6750,11 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	int ret;
>  
> -	ret = nand_scan_ident(mtd, maxchips, ids);
> -	if (ret)
> -		return ret;
> +	if (maxchips) {
> +		ret = nand_scan_ident(mtd, maxchips, ids);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	ret = nand_attach(chip);
>  	if (ret)

[PATCH v5 08/17] mtd: rawnand: docg4: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/docg4.c | 70 ++++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
index 4dccdfba6140..b69a36842605 100644
--- a/drivers/mtd/nand/raw/docg4.c
+++ b/drivers/mtd/nand/raw/docg4.c
@@ -1227,10 +1227,9 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
 	 * required within a nand driver because they are performed by the nand
 	 * infrastructure code as part of nand_scan().  In this case they need
 	 * to be initialized here because we skip call to nand_scan_ident() (the
-	 * first half of nand_scan()).  The call to nand_scan_ident() is skipped
-	 * because for this device the chip id is not read in the manner of a
-	 * standard nand device.  Unfortunately, nand_scan_ident() does other
-	 * things as well, such as call nand_set_defaults().
+	 * first half of nand_scan()).  The call to nand_scan_ident() could be
+	 * skipped because for this device the chip id is not read in the manner
+	 * of a standard nand device.
 	 */
 
 	struct nand_chip *nand = mtd_to_nand(mtd);
@@ -1315,6 +1314,40 @@ static int __init read_id_reg(struct mtd_info *mtd)
 
 static char const *part_probes[] = { "cmdlinepart", "saftlpart", NULL };
 
+static int docg4_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
+	int ret;
+
+	init_mtd_structs(mtd);
+
+	/* Initialize kernel BCH algorithm */
+	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
+	if (!doc->bch)
+		return -EINVAL;
+
+	reset(mtd);
+
+	ret = read_id_reg(mtd);
+	if (ret)
+		free_bch(doc->bch);
+
+	return ret;
+}
+
+static void docg4_detach_chip(struct nand_chip *chip)
+{
+	struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
+
+	free_bch(doc->bch);
+}
+
+static const struct nand_controller_ops docg4_controller_ops = {
+	.attach_chip = docg4_attach_chip,
+	.detach_chip = docg4_detach_chip,
+};
+
 static int __init probe_docg4(struct platform_device *pdev)
 {
 	struct mtd_info *mtd;
@@ -1350,28 +1383,17 @@ static int __init probe_docg4(struct platform_device *pdev)
 	mtd->dev.parent = &pdev->dev;
 	doc->virtadr = virtadr;
 	doc->dev = dev;
-
-	init_mtd_structs(mtd);
-
-	/* initialize kernel bch algorithm */
-	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
-	if (doc->bch == NULL) {
-		retval = -EINVAL;
-		goto free_nand;
-	}
-
 	platform_set_drvdata(pdev, doc);
 
-	reset(mtd);
-	retval = read_id_reg(mtd);
-	if (retval == -ENODEV) {
-		dev_warn(dev, "No diskonchip G4 device found.\n");
-		goto free_bch;
-	}
-
-	retval = nand_scan_tail(mtd);
+	/*
+	 * Running nand_scan() with maxchips == 0 will skip nand_scan_ident(),
+	 * which is a specific operation with this driver and done in the
+	 * ->attach_chip callback.
+	 */
+	nand->dummy_controller.ops = &docg4_controller_ops;
+	retval = nand_scan(mtd, 0);
 	if (retval)
-		goto free_bch;
+		goto free_nand;
 
 	retval = read_factory_bbt(mtd);
 	if (retval)
@@ -1387,8 +1409,6 @@ static int __init probe_docg4(struct platform_device *pdev)
 
 cleanup_nand:
 	nand_cleanup(nand);
-free_bch:
-	free_bch(doc->bch);
 free_nand:
 	kfree(nand);
 unmap:
-- 
2.14.1

Re: [PATCH v5 08/17] mtd: rawnand: docg4: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:43 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/docg4.c | 70 ++++++++++++++++++++++++++++----------------
>  1 file changed, 45 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index 4dccdfba6140..b69a36842605 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1227,10 +1227,9 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
>  	 * required within a nand driver because they are performed by the nand
>  	 * infrastructure code as part of nand_scan().  In this case they need
>  	 * to be initialized here because we skip call to nand_scan_ident() (the
> -	 * first half of nand_scan()).  The call to nand_scan_ident() is skipped
> -	 * because for this device the chip id is not read in the manner of a
> -	 * standard nand device.  Unfortunately, nand_scan_ident() does other
> -	 * things as well, such as call nand_set_defaults().
> +	 * first half of nand_scan()).  The call to nand_scan_ident() could be
> +	 * skipped because for this device the chip id is not read in the manner
> +	 * of a standard nand device.
>  	 */
>  
>  	struct nand_chip *nand = mtd_to_nand(mtd);
> @@ -1315,6 +1314,40 @@ static int __init read_id_reg(struct mtd_info *mtd)
>  
>  static char const *part_probes[] = { "cmdlinepart", "saftlpart", NULL };
>  
> +static int docg4_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
> +	int ret;
> +
> +	init_mtd_structs(mtd);
> +
> +	/* Initialize kernel BCH algorithm */
> +	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> +	if (!doc->bch)
> +		return -EINVAL;
> +
> +	reset(mtd);
> +
> +	ret = read_id_reg(mtd);
> +	if (ret)
> +		free_bch(doc->bch);
> +
> +	return ret;
> +}
> +
> +static void docg4_detach_chip(struct nand_chip *chip)
> +{
> +	struct docg4_priv *doc = (struct docg4_priv *)(chip + 1);
> +
> +	free_bch(doc->bch);
> +}
> +
> +static const struct nand_controller_ops docg4_controller_ops = {
> +	.attach_chip = docg4_attach_chip,
> +	.detach_chip = docg4_detach_chip,
> +};
> +
>  static int __init probe_docg4(struct platform_device *pdev)
>  {
>  	struct mtd_info *mtd;
> @@ -1350,28 +1383,17 @@ static int __init probe_docg4(struct platform_device *pdev)
>  	mtd->dev.parent = &pdev->dev;
>  	doc->virtadr = virtadr;
>  	doc->dev = dev;
> -
> -	init_mtd_structs(mtd);
> -
> -	/* initialize kernel bch algorithm */
> -	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
> -	if (doc->bch == NULL) {
> -		retval = -EINVAL;
> -		goto free_nand;
> -	}
> -
>  	platform_set_drvdata(pdev, doc);
>  
> -	reset(mtd);
> -	retval = read_id_reg(mtd);
> -	if (retval == -ENODEV) {
> -		dev_warn(dev, "No diskonchip G4 device found.\n");
> -		goto free_bch;
> -	}
> -
> -	retval = nand_scan_tail(mtd);
> +	/*
> +	 * Running nand_scan() with maxchips == 0 will skip nand_scan_ident(),
> +	 * which is a specific operation with this driver and done in the
> +	 * ->attach_chip callback.
> +	 */
> +	nand->dummy_controller.ops = &docg4_controller_ops;
> +	retval = nand_scan(mtd, 0);
>  	if (retval)
> -		goto free_bch;
> +		goto free_nand;
>  
>  	retval = read_factory_bbt(mtd);
>  	if (retval)
> @@ -1387,8 +1409,6 @@ static int __init probe_docg4(struct platform_device *pdev)
>  
>  cleanup_nand:
>  	nand_cleanup(nand);
> -free_bch:
> -	free_bch(doc->bch);

You forgot to remove the free_bch() call in the remove path.
With this addressed

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

>  free_nand:
>  	kfree(nand);
>  unmap:

[PATCH v5 09/17] mtd: rawnand: jz4740: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup(), not a
nand_release() as mtd_device_register() (or one of its variants) has not
been called and there is no need to deregister any MTD device yet.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/jz4740_nand.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
index a4052b03249c..3abf87420c10 100644
--- a/drivers/mtd/nand/raw/jz4740_nand.c
+++ b/drivers/mtd/nand/raw/jz4740_nand.c
@@ -472,15 +472,15 @@ static int jz_nand_probe(struct platform_device *pdev)
 
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add mtd device\n");
-		goto err_nand_release;
+		goto err_cleanup_nand;
 	}
 
 	dev_info(&pdev->dev, "Successfully registered JZ4740 NAND driver\n");
 
 	return 0;
 
-err_nand_release:
-	nand_release(mtd);
+err_cleanup_nand:
+	nand_cleanup(chip);
 err_unclaim_banks:
 	while (chipnr--) {
 		unsigned char bank = nand->banks[chipnr];
-- 
2.14.1

Re: [PATCH v5 09/17] mtd: rawnand: jz4740: fix probe function error path

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:44 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup(), not a
> nand_release() as mtd_device_register() (or one of its variants) has not
> been called and there is no need to deregister any MTD device yet.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/jz4740_nand.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
> index a4052b03249c..3abf87420c10 100644
> --- a/drivers/mtd/nand/raw/jz4740_nand.c
> +++ b/drivers/mtd/nand/raw/jz4740_nand.c
> @@ -472,15 +472,15 @@ static int jz_nand_probe(struct platform_device *pdev)
>  
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to add mtd device\n");
> -		goto err_nand_release;
> +		goto err_cleanup_nand;
>  	}
>  
>  	dev_info(&pdev->dev, "Successfully registered JZ4740 NAND driver\n");
>  
>  	return 0;
>  
> -err_nand_release:
> -	nand_release(mtd);
> +err_cleanup_nand:
> +	nand_cleanup(chip);
>  err_unclaim_banks:
>  	while (chipnr--) {
>  		unsigned char bank = nand->banks[chipnr];

[PATCH v5 10/17] mtd: rawnand: jz4740: group nand_scan_{ident, tail} calls

[Miquel Raynal]

Prepare the migration to nand_scan() by moving both calls to
nand_scan_ident() and nand_scan_tail() in a single spot.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/jz4740_nand.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
index 3abf87420c10..09541608a45f 100644
--- a/drivers/mtd/nand/raw/jz4740_nand.c
+++ b/drivers/mtd/nand/raw/jz4740_nand.c
@@ -309,6 +309,7 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
 			       size_t chipnr, uint8_t *nand_maf_id,
 			       uint8_t *nand_dev_id)
 {
+	struct jz_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	int ret;
 	char res_name[6];
 	uint32_t ctrl;
@@ -335,8 +336,19 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
 		if (ret)
 			goto notfound_id;
 
+		if (pdata && pdata->ident_callback)
+			pdata->ident_callback(pdev, mtd, &pdata->partitions,
+					      &pdata->num_partitions);
+
+		ret = nand_scan_tail(mtd);
+		if (ret) {
+			dev_err(&pdev->dev,  "Failed to scan NAND\n");
+			goto notfound_id;
+		}
+
 		/* Retrieve the IDs from the first chip. */
 		chip->select_chip(mtd, 0);
+
 		nand_reset_op(chip);
 		nand_readid_op(chip, 0, id, sizeof(id));
 		*nand_maf_id = id[0];
@@ -456,17 +468,6 @@ static int jz_nand_probe(struct platform_device *pdev)
 		goto err_iounmap_mmio;
 	}
 
-	if (pdata && pdata->ident_callback) {
-		pdata->ident_callback(pdev, mtd, &pdata->partitions,
-					&pdata->num_partitions);
-	}
-
-	ret = nand_scan_tail(mtd);
-	if (ret) {
-		dev_err(&pdev->dev,  "Failed to scan NAND\n");
-		goto err_unclaim_banks;
-	}
-
 	ret = mtd_device_register(mtd, pdata ? pdata->partitions : NULL,
 				  pdata ? pdata->num_partitions : 0);
 
@@ -481,7 +482,6 @@ static int jz_nand_probe(struct platform_device *pdev)
 
 err_cleanup_nand:
 	nand_cleanup(chip);
-err_unclaim_banks:
 	while (chipnr--) {
 		unsigned char bank = nand->banks[chipnr];
 		jz_nand_iounmap_resource(nand->bank_mem[bank - 1],
-- 
2.14.1

Re: [PATCH v5 10/17] mtd: rawnand: jz4740: group nand_scan_{ident,tail} calls

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:45 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Prepare the migration to nand_scan() by moving both calls to
> nand_scan_ident() and nand_scan_tail() in a single spot.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/jz4740_nand.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
> index 3abf87420c10..09541608a45f 100644
> --- a/drivers/mtd/nand/raw/jz4740_nand.c
> +++ b/drivers/mtd/nand/raw/jz4740_nand.c
> @@ -309,6 +309,7 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
>  			       size_t chipnr, uint8_t *nand_maf_id,
>  			       uint8_t *nand_dev_id)
>  {
> +	struct jz_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	int ret;
>  	char res_name[6];
>  	uint32_t ctrl;
> @@ -335,8 +336,19 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
>  		if (ret)
>  			goto notfound_id;
>  
> +		if (pdata && pdata->ident_callback)
> +			pdata->ident_callback(pdev, mtd, &pdata->partitions,
> +					      &pdata->num_partitions);
> +
> +		ret = nand_scan_tail(mtd);
> +		if (ret) {
> +			dev_err(&pdev->dev,  "Failed to scan NAND\n");
> +			goto notfound_id;
> +		}
> +
>  		/* Retrieve the IDs from the first chip. */
>  		chip->select_chip(mtd, 0);
> +

You add an empty line here for no reason. Other than this tiny detail,
LGTM.

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

>  		nand_reset_op(chip);
>  		nand_readid_op(chip, 0, id, sizeof(id));
>  		*nand_maf_id = id[0];
> @@ -456,17 +468,6 @@ static int jz_nand_probe(struct platform_device *pdev)
>  		goto err_iounmap_mmio;
>  	}
>  
> -	if (pdata && pdata->ident_callback) {
> -		pdata->ident_callback(pdev, mtd, &pdata->partitions,
> -					&pdata->num_partitions);
> -	}
> -
> -	ret = nand_scan_tail(mtd);
> -	if (ret) {
> -		dev_err(&pdev->dev,  "Failed to scan NAND\n");
> -		goto err_unclaim_banks;
> -	}
> -
>  	ret = mtd_device_register(mtd, pdata ? pdata->partitions : NULL,
>  				  pdata ? pdata->num_partitions : 0);
>  
> @@ -481,7 +482,6 @@ static int jz_nand_probe(struct platform_device *pdev)
>  
>  err_cleanup_nand:
>  	nand_cleanup(chip);
> -err_unclaim_banks:
>  	while (chipnr--) {
>  		unsigned char bank = nand->banks[chipnr];
>  		jz_nand_iounmap_resource(nand->bank_mem[bank - 1],

[PATCH v5 11/17] mtd: rawnand: jz4740: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/jz4740_nand.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
index 09541608a45f..3816d56b0d01 100644
--- a/drivers/mtd/nand/raw/jz4740_nand.c
+++ b/drivers/mtd/nand/raw/jz4740_nand.c
@@ -309,7 +309,6 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
 			       size_t chipnr, uint8_t *nand_maf_id,
 			       uint8_t *nand_dev_id)
 {
-	struct jz_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	int ret;
 	char res_name[6];
 	uint32_t ctrl;
@@ -332,20 +331,10 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
 
 	if (chipnr == 0) {
 		/* Detect first chip. */
-		ret = nand_scan_ident(mtd, 1, NULL);
+		ret = nand_scan(mtd, 1);
 		if (ret)
 			goto notfound_id;
 
-		if (pdata && pdata->ident_callback)
-			pdata->ident_callback(pdev, mtd, &pdata->partitions,
-					      &pdata->num_partitions);
-
-		ret = nand_scan_tail(mtd);
-		if (ret) {
-			dev_err(&pdev->dev,  "Failed to scan NAND\n");
-			goto notfound_id;
-		}
-
 		/* Retrieve the IDs from the first chip. */
 		chip->select_chip(mtd, 0);
 
@@ -380,6 +369,24 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
 	return ret;
 }
 
+static int jz_nand_attach_chip(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct device *dev = mtd->dev.parent;
+	struct jz_nand_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+
+	if (pdata && pdata->ident_callback)
+		pdata->ident_callback(pdev, mtd, &pdata->partitions,
+				      &pdata->num_partitions);
+
+	return 0;
+}
+
+static const struct nand_controller_ops jz_nand_controller_ops = {
+	.attach_chip = jz_nand_attach_chip,
+};
+
 static int jz_nand_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -423,6 +430,7 @@ static int jz_nand_probe(struct platform_device *pdev)
 	chip->chip_delay = 50;
 	chip->cmd_ctrl = jz_nand_cmd_ctrl;
 	chip->select_chip = jz_nand_select_chip;
+	chip->dummy_controller.ops = &jz_nand_controller_ops;
 
 	if (nand->busy_gpio)
 		chip->dev_ready = jz_nand_dev_ready;
-- 
2.14.1

Re: [PATCH v5 11/17] mtd: rawnand: jz4740: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:46 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/jz4740_nand.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/jz4740_nand.c
> index 09541608a45f..3816d56b0d01 100644
> --- a/drivers/mtd/nand/raw/jz4740_nand.c
> +++ b/drivers/mtd/nand/raw/jz4740_nand.c
> @@ -309,7 +309,6 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
>  			       size_t chipnr, uint8_t *nand_maf_id,
>  			       uint8_t *nand_dev_id)
>  {
> -	struct jz_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	int ret;
>  	char res_name[6];
>  	uint32_t ctrl;
> @@ -332,20 +331,10 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
>  
>  	if (chipnr == 0) {
>  		/* Detect first chip. */
> -		ret = nand_scan_ident(mtd, 1, NULL);
> +		ret = nand_scan(mtd, 1);
>  		if (ret)
>  			goto notfound_id;
>  
> -		if (pdata && pdata->ident_callback)
> -			pdata->ident_callback(pdev, mtd, &pdata->partitions,
> -					      &pdata->num_partitions);
> -
> -		ret = nand_scan_tail(mtd);
> -		if (ret) {
> -			dev_err(&pdev->dev,  "Failed to scan NAND\n");
> -			goto notfound_id;
> -		}
> -
>  		/* Retrieve the IDs from the first chip. */
>  		chip->select_chip(mtd, 0);
>  
> @@ -380,6 +369,24 @@ static int jz_nand_detect_bank(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static int jz_nand_attach_chip(struct nand_chip *chip)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct device *dev = mtd->dev.parent;
> +	struct jz_nand_platform_data *pdata = dev_get_platdata(dev);
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	if (pdata && pdata->ident_callback)
> +		pdata->ident_callback(pdev, mtd, &pdata->partitions,
> +				      &pdata->num_partitions);
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops jz_nand_controller_ops = {
> +	.attach_chip = jz_nand_attach_chip,
> +};
> +
>  static int jz_nand_probe(struct platform_device *pdev)
>  {
>  	int ret;
> @@ -423,6 +430,7 @@ static int jz_nand_probe(struct platform_device *pdev)
>  	chip->chip_delay = 50;
>  	chip->cmd_ctrl = jz_nand_cmd_ctrl;
>  	chip->select_chip = jz_nand_select_chip;
> +	chip->dummy_controller.ops = &jz_nand_controller_ops;
>  
>  	if (nand->busy_gpio)
>  		chip->dev_ready = jz_nand_dev_ready;

[PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/tegra_nand.c | 162 +++++++++++++++++++++-----------------
 1 file changed, 88 insertions(+), 74 deletions(-)

diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index 31c0d9ca9d23..79da1efc88d1 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -906,74 +906,13 @@ static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
 				       bits_per_step, oobsize);
 }
 
-static int tegra_nand_chips_init(struct device *dev,
-				 struct tegra_nand_controller *ctrl)
+static int tegra_nand_attach_chip(struct nand_chip *chip)
 {
-	struct device_node *np = dev->of_node;
-	struct device_node *np_nand;
-	int nsels, nchips = of_get_child_count(np);
-	struct tegra_nand_chip *nand;
-	struct mtd_info *mtd;
-	struct nand_chip *chip;
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	struct tegra_nand_chip *nand = to_tegra_chip(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int bits_per_step;
 	int ret;
-	u32 cs;
-
-	if (nchips != 1) {
-		dev_err(dev, "Currently only one NAND chip supported\n");
-		return -EINVAL;
-	}
-
-	np_nand = of_get_next_child(np, NULL);
-
-	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
-	if (nsels != 1) {
-		dev_err(dev, "Missing/invalid reg property\n");
-		return -EINVAL;
-	}
-
-	/* Retrieve CS id, currently only single die NAND supported */
-	ret = of_property_read_u32(np_nand, "reg", &cs);
-	if (ret) {
-		dev_err(dev, "could not retrieve reg property: %d\n", ret);
-		return ret;
-	}
-
-	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
-	if (!nand)
-		return -ENOMEM;
-
-	nand->cs[0] = cs;
-
-	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
-
-	if (IS_ERR(nand->wp_gpio)) {
-		ret = PTR_ERR(nand->wp_gpio);
-		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
-		return ret;
-	}
-
-	chip = &nand->chip;
-	chip->controller = &ctrl->controller;
-
-	mtd = nand_to_mtd(chip);
-
-	mtd->dev.parent = dev;
-	mtd->owner = THIS_MODULE;
-
-	nand_set_flash_node(chip, np_nand);
-
-	if (!mtd->name)
-		mtd->name = "tegra_nand";
-
-	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
-	chip->exec_op = tegra_nand_exec_op;
-	chip->select_chip = tegra_nand_select_chip;
-	chip->setup_data_interface = tegra_nand_setup_data_interface;
-
-	ret = nand_scan_ident(mtd, 1, NULL);
-	if (ret)
-		return ret;
 
 	if (chip->bbt_options & NAND_BBT_USE_FLASH)
 		chip->bbt_options |= NAND_BBT_NO_OOB;
@@ -982,7 +921,8 @@ static int tegra_nand_chips_init(struct device *dev,
 	chip->ecc.size = 512;
 	chip->ecc.steps = mtd->writesize / chip->ecc.size;
 	if (chip->ecc_step_ds != 512) {
-		dev_err(dev, "Unsupported step size %d\n", chip->ecc_step_ds);
+		dev_err(ctrl->dev, "Unsupported step size %d\n",
+			chip->ecc_step_ds);
 		return -EINVAL;
 	}
 
@@ -1004,14 +944,15 @@ static int tegra_nand_chips_init(struct device *dev,
 	}
 
 	if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048) {
-		dev_err(dev, "BCH supports 2K or 4K page size only\n");
+		dev_err(ctrl->dev, "BCH supports 2K or 4K page size only\n");
 		return -EINVAL;
 	}
 
 	if (!chip->ecc.strength) {
 		ret = tegra_nand_select_strength(chip, mtd->oobsize);
 		if (ret < 0) {
-			dev_err(dev, "No valid strength found, minimum %d\n",
+			dev_err(ctrl->dev,
+				"No valid strength found, minimum %d\n",
 				chip->ecc_strength_ds);
 			return ret;
 		}
@@ -1039,7 +980,7 @@ static int tegra_nand_chips_init(struct device *dev,
 			nand->config_ecc |= CONFIG_TVAL_8;
 			break;
 		default:
-			dev_err(dev, "ECC strength %d not supported\n",
+			dev_err(ctrl->dev, "ECC strength %d not supported\n",
 				chip->ecc.strength);
 			return -EINVAL;
 		}
@@ -1062,17 +1003,17 @@ static int tegra_nand_chips_init(struct device *dev,
 			nand->bch_config |= BCH_TVAL_16;
 			break;
 		default:
-			dev_err(dev, "ECC strength %d not supported\n",
+			dev_err(ctrl->dev, "ECC strength %d not supported\n",
 				chip->ecc.strength);
 			return -EINVAL;
 		}
 		break;
 	default:
-		dev_err(dev, "ECC algorithm not supported\n");
+		dev_err(ctrl->dev, "ECC algorithm not supported\n");
 		return -EINVAL;
 	}
 
-	dev_info(dev, "Using %s with strength %d per 512 byte step\n",
+	dev_info(ctrl->dev, "Using %s with strength %d per 512 byte step\n",
 		 chip->ecc.algo == NAND_ECC_BCH ? "BCH" : "RS",
 		 chip->ecc.strength);
 
@@ -1095,7 +1036,8 @@ static int tegra_nand_chips_init(struct device *dev,
 		nand->config |= CONFIG_PS_4096;
 		break;
 	default:
-		dev_err(dev, "Unsupported writesize %d\n", mtd->writesize);
+		dev_err(ctrl->dev, "Unsupported writesize %d\n",
+			mtd->writesize);
 		return -ENODEV;
 	}
 
@@ -1106,7 +1048,78 @@ static int tegra_nand_chips_init(struct device *dev,
 	nand->config |= CONFIG_TAG_BYTE_SIZE(mtd->oobsize - 1);
 	writel_relaxed(nand->config, ctrl->regs + CONFIG);
 
-	ret = nand_scan_tail(mtd);
+	return 0;
+}
+
+static const struct nand_controller_ops tegra_nand_controller_ops = {
+	.attach_chip = &tegra_nand_attach_chip,
+};
+
+static int tegra_nand_chips_init(struct device *dev,
+				 struct tegra_nand_controller *ctrl)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *np_nand;
+	int nsels, nchips = of_get_child_count(np);
+	struct tegra_nand_chip *nand;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	int ret;
+	u32 cs;
+
+	if (nchips != 1) {
+		dev_err(dev, "Currently only one NAND chip supported\n");
+		return -EINVAL;
+	}
+
+	np_nand = of_get_next_child(np, NULL);
+
+	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
+	if (nsels != 1) {
+		dev_err(dev, "Missing/invalid reg property\n");
+		return -EINVAL;
+	}
+
+	/* Retrieve CS id, currently only single die NAND supported */
+	ret = of_property_read_u32(np_nand, "reg", &cs);
+	if (ret) {
+		dev_err(dev, "could not retrieve reg property: %d\n", ret);
+		return ret;
+	}
+
+	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
+	if (!nand)
+		return -ENOMEM;
+
+	nand->cs[0] = cs;
+
+	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
+
+	if (IS_ERR(nand->wp_gpio)) {
+		ret = PTR_ERR(nand->wp_gpio);
+		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
+		return ret;
+	}
+
+	chip = &nand->chip;
+	chip->controller = &ctrl->controller;
+
+	mtd = nand_to_mtd(chip);
+
+	mtd->dev.parent = dev;
+	mtd->owner = THIS_MODULE;
+
+	nand_set_flash_node(chip, np_nand);
+
+	if (!mtd->name)
+		mtd->name = "tegra_nand";
+
+	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
+	chip->exec_op = tegra_nand_exec_op;
+	chip->select_chip = tegra_nand_select_chip;
+	chip->setup_data_interface = tegra_nand_setup_data_interface;
+
+	ret = nand_scan(mtd, 1);
 	if (ret)
 		return ret;
 
@@ -1137,6 +1150,7 @@ static int tegra_nand_probe(struct platform_device *pdev)
 
 	ctrl->dev = &pdev->dev;
 	nand_controller_init(&ctrl->controller);
+	ctrl->controller.ops = &tegra_nand_controller_ops;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ctrl->regs = devm_ioremap_resource(&pdev->dev, res);
-- 
2.14.1

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:47 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/tegra_nand.c | 162 +++++++++++++++++++++-----------------
>  1 file changed, 88 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
> index 31c0d9ca9d23..79da1efc88d1 100644
> --- a/drivers/mtd/nand/raw/tegra_nand.c
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -906,74 +906,13 @@ static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
>  				       bits_per_step, oobsize);
>  }
>  
> -static int tegra_nand_chips_init(struct device *dev,
> -				 struct tegra_nand_controller *ctrl)
> +static int tegra_nand_attach_chip(struct nand_chip *chip)
>  {
> -	struct device_node *np = dev->of_node;
> -	struct device_node *np_nand;
> -	int nsels, nchips = of_get_child_count(np);
> -	struct tegra_nand_chip *nand;
> -	struct mtd_info *mtd;
> -	struct nand_chip *chip;
> +	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
> +	struct tegra_nand_chip *nand = to_tegra_chip(chip);
> +	struct mtd_info *mtd = nand_to_mtd(chip);
>  	int bits_per_step;
>  	int ret;
> -	u32 cs;
> -
> -	if (nchips != 1) {
> -		dev_err(dev, "Currently only one NAND chip supported\n");
> -		return -EINVAL;
> -	}
> -
> -	np_nand = of_get_next_child(np, NULL);
> -
> -	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
> -	if (nsels != 1) {
> -		dev_err(dev, "Missing/invalid reg property\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Retrieve CS id, currently only single die NAND supported */
> -	ret = of_property_read_u32(np_nand, "reg", &cs);
> -	if (ret) {
> -		dev_err(dev, "could not retrieve reg property: %d\n", ret);
> -		return ret;
> -	}
> -
> -	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
> -	if (!nand)
> -		return -ENOMEM;
> -
> -	nand->cs[0] = cs;
> -
> -	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
> -
> -	if (IS_ERR(nand->wp_gpio)) {
> -		ret = PTR_ERR(nand->wp_gpio);
> -		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
> -		return ret;
> -	}
> -
> -	chip = &nand->chip;
> -	chip->controller = &ctrl->controller;
> -
> -	mtd = nand_to_mtd(chip);
> -
> -	mtd->dev.parent = dev;
> -	mtd->owner = THIS_MODULE;
> -
> -	nand_set_flash_node(chip, np_nand);
> -
> -	if (!mtd->name)
> -		mtd->name = "tegra_nand";
> -
> -	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
> -	chip->exec_op = tegra_nand_exec_op;
> -	chip->select_chip = tegra_nand_select_chip;
> -	chip->setup_data_interface = tegra_nand_setup_data_interface;
> -
> -	ret = nand_scan_ident(mtd, 1, NULL);
> -	if (ret)
> -		return ret;
>  
>  	if (chip->bbt_options & NAND_BBT_USE_FLASH)
>  		chip->bbt_options |= NAND_BBT_NO_OOB;
> @@ -982,7 +921,8 @@ static int tegra_nand_chips_init(struct device *dev,
>  	chip->ecc.size = 512;
>  	chip->ecc.steps = mtd->writesize / chip->ecc.size;
>  	if (chip->ecc_step_ds != 512) {
> -		dev_err(dev, "Unsupported step size %d\n", chip->ecc_step_ds);
> +		dev_err(ctrl->dev, "Unsupported step size %d\n",
> +			chip->ecc_step_ds);
>  		return -EINVAL;
>  	}
>  
> @@ -1004,14 +944,15 @@ static int tegra_nand_chips_init(struct device *dev,
>  	}
>  
>  	if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048) {
> -		dev_err(dev, "BCH supports 2K or 4K page size only\n");
> +		dev_err(ctrl->dev, "BCH supports 2K or 4K page size only\n");
>  		return -EINVAL;
>  	}
>  
>  	if (!chip->ecc.strength) {
>  		ret = tegra_nand_select_strength(chip, mtd->oobsize);
>  		if (ret < 0) {
> -			dev_err(dev, "No valid strength found, minimum %d\n",
> +			dev_err(ctrl->dev,
> +				"No valid strength found, minimum %d\n",
>  				chip->ecc_strength_ds);
>  			return ret;
>  		}
> @@ -1039,7 +980,7 @@ static int tegra_nand_chips_init(struct device *dev,
>  			nand->config_ecc |= CONFIG_TVAL_8;
>  			break;
>  		default:
> -			dev_err(dev, "ECC strength %d not supported\n",
> +			dev_err(ctrl->dev, "ECC strength %d not supported\n",
>  				chip->ecc.strength);
>  			return -EINVAL;
>  		}
> @@ -1062,17 +1003,17 @@ static int tegra_nand_chips_init(struct device *dev,
>  			nand->bch_config |= BCH_TVAL_16;
>  			break;
>  		default:
> -			dev_err(dev, "ECC strength %d not supported\n",
> +			dev_err(ctrl->dev, "ECC strength %d not supported\n",
>  				chip->ecc.strength);
>  			return -EINVAL;
>  		}
>  		break;
>  	default:
> -		dev_err(dev, "ECC algorithm not supported\n");
> +		dev_err(ctrl->dev, "ECC algorithm not supported\n");
>  		return -EINVAL;
>  	}
>  
> -	dev_info(dev, "Using %s with strength %d per 512 byte step\n",
> +	dev_info(ctrl->dev, "Using %s with strength %d per 512 byte step\n",
>  		 chip->ecc.algo == NAND_ECC_BCH ? "BCH" : "RS",
>  		 chip->ecc.strength);
>  
> @@ -1095,7 +1036,8 @@ static int tegra_nand_chips_init(struct device *dev,
>  		nand->config |= CONFIG_PS_4096;
>  		break;
>  	default:
> -		dev_err(dev, "Unsupported writesize %d\n", mtd->writesize);
> +		dev_err(ctrl->dev, "Unsupported writesize %d\n",
> +			mtd->writesize);
>  		return -ENODEV;
>  	}
>  
> @@ -1106,7 +1048,78 @@ static int tegra_nand_chips_init(struct device *dev,
>  	nand->config |= CONFIG_TAG_BYTE_SIZE(mtd->oobsize - 1);
>  	writel_relaxed(nand->config, ctrl->regs + CONFIG);
>  
> -	ret = nand_scan_tail(mtd);
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops tegra_nand_controller_ops = {
> +	.attach_chip = &tegra_nand_attach_chip,
> +};
> +
> +static int tegra_nand_chips_init(struct device *dev,
> +				 struct tegra_nand_controller *ctrl)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *np_nand;
> +	int nsels, nchips = of_get_child_count(np);
> +	struct tegra_nand_chip *nand;
> +	struct mtd_info *mtd;
> +	struct nand_chip *chip;
> +	int ret;
> +	u32 cs;
> +
> +	if (nchips != 1) {
> +		dev_err(dev, "Currently only one NAND chip supported\n");
> +		return -EINVAL;
> +	}
> +
> +	np_nand = of_get_next_child(np, NULL);
> +
> +	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
> +	if (nsels != 1) {
> +		dev_err(dev, "Missing/invalid reg property\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Retrieve CS id, currently only single die NAND supported */
> +	ret = of_property_read_u32(np_nand, "reg", &cs);
> +	if (ret) {
> +		dev_err(dev, "could not retrieve reg property: %d\n", ret);
> +		return ret;
> +	}
> +
> +	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
> +	if (!nand)
> +		return -ENOMEM;
> +
> +	nand->cs[0] = cs;
> +
> +	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
> +
> +	if (IS_ERR(nand->wp_gpio)) {
> +		ret = PTR_ERR(nand->wp_gpio);
> +		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	chip = &nand->chip;
> +	chip->controller = &ctrl->controller;
> +
> +	mtd = nand_to_mtd(chip);
> +
> +	mtd->dev.parent = dev;
> +	mtd->owner = THIS_MODULE;
> +
> +	nand_set_flash_node(chip, np_nand);
> +
> +	if (!mtd->name)
> +		mtd->name = "tegra_nand";
> +
> +	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
> +	chip->exec_op = tegra_nand_exec_op;
> +	chip->select_chip = tegra_nand_select_chip;
> +	chip->setup_data_interface = tegra_nand_setup_data_interface;
> +
> +	ret = nand_scan(mtd, 1);
>  	if (ret)
>  		return ret;
>  
> @@ -1137,6 +1150,7 @@ static int tegra_nand_probe(struct platform_device *pdev)
>  
>  	ctrl->dev = &pdev->dev;
>  	nand_controller_init(&ctrl->controller);
> +	ctrl->controller.ops = &tegra_nand_controller_ops;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	ctrl->regs = devm_ioremap_resource(&pdev->dev, res);

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Stefan Agner]

On 25.07.2018 15:31, Miquel Raynal wrote:
> Two helpers have been added to the core to do all kind of controller
> side configuration/initialization between the detection phase and the
> final NAND scan. Implement these hooks so that we can convert the driver
> to just use nand_scan() instead of the nand_scan_ident() +
> nand_scan_tail() pair.
> 

While the patch looks technically correct, I wonder whether the driver
now does what we expect it from attach logically...

E.g. shouldn't we get the wp_gpio in attach?

--
Stefan

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/tegra_nand.c | 162 +++++++++++++++++++++-----------------
>  1 file changed, 88 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> b/drivers/mtd/nand/raw/tegra_nand.c
> index 31c0d9ca9d23..79da1efc88d1 100644
> --- a/drivers/mtd/nand/raw/tegra_nand.c
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -906,74 +906,13 @@ static int tegra_nand_select_strength(struct
> nand_chip *chip, int oobsize)
>  				       bits_per_step, oobsize);
>  }
>  
> -static int tegra_nand_chips_init(struct device *dev,
> -				 struct tegra_nand_controller *ctrl)
> +static int tegra_nand_attach_chip(struct nand_chip *chip)
>  {
> -	struct device_node *np = dev->of_node;
> -	struct device_node *np_nand;
> -	int nsels, nchips = of_get_child_count(np);
> -	struct tegra_nand_chip *nand;
> -	struct mtd_info *mtd;
> -	struct nand_chip *chip;
> +	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
> +	struct tegra_nand_chip *nand = to_tegra_chip(chip);
> +	struct mtd_info *mtd = nand_to_mtd(chip);
>  	int bits_per_step;
>  	int ret;
> -	u32 cs;
> -
> -	if (nchips != 1) {
> -		dev_err(dev, "Currently only one NAND chip supported\n");
> -		return -EINVAL;
> -	}
> -
> -	np_nand = of_get_next_child(np, NULL);
> -
> -	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
> -	if (nsels != 1) {
> -		dev_err(dev, "Missing/invalid reg property\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Retrieve CS id, currently only single die NAND supported */
> -	ret = of_property_read_u32(np_nand, "reg", &cs);
> -	if (ret) {
> -		dev_err(dev, "could not retrieve reg property: %d\n", ret);
> -		return ret;
> -	}
> -
> -	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
> -	if (!nand)
> -		return -ENOMEM;
> -
> -	nand->cs[0] = cs;
> -
> -	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
> -
> -	if (IS_ERR(nand->wp_gpio)) {
> -		ret = PTR_ERR(nand->wp_gpio);
> -		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
> -		return ret;
> -	}
> -
> -	chip = &nand->chip;
> -	chip->controller = &ctrl->controller;
> -
> -	mtd = nand_to_mtd(chip);
> -
> -	mtd->dev.parent = dev;
> -	mtd->owner = THIS_MODULE;
> -
> -	nand_set_flash_node(chip, np_nand);
> -
> -	if (!mtd->name)
> -		mtd->name = "tegra_nand";
> -
> -	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
> -	chip->exec_op = tegra_nand_exec_op;
> -	chip->select_chip = tegra_nand_select_chip;
> -	chip->setup_data_interface = tegra_nand_setup_data_interface;
> -
> -	ret = nand_scan_ident(mtd, 1, NULL);
> -	if (ret)
> -		return ret;
>  
>  	if (chip->bbt_options & NAND_BBT_USE_FLASH)
>  		chip->bbt_options |= NAND_BBT_NO_OOB;
> @@ -982,7 +921,8 @@ static int tegra_nand_chips_init(struct device *dev,
>  	chip->ecc.size = 512;
>  	chip->ecc.steps = mtd->writesize / chip->ecc.size;
>  	if (chip->ecc_step_ds != 512) {
> -		dev_err(dev, "Unsupported step size %d\n", chip->ecc_step_ds);
> +		dev_err(ctrl->dev, "Unsupported step size %d\n",
> +			chip->ecc_step_ds);
>  		return -EINVAL;
>  	}
>  
> @@ -1004,14 +944,15 @@ static int tegra_nand_chips_init(struct device *dev,
>  	}
>  
>  	if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048) {
> -		dev_err(dev, "BCH supports 2K or 4K page size only\n");
> +		dev_err(ctrl->dev, "BCH supports 2K or 4K page size only\n");
>  		return -EINVAL;
>  	}
>  
>  	if (!chip->ecc.strength) {
>  		ret = tegra_nand_select_strength(chip, mtd->oobsize);
>  		if (ret < 0) {
> -			dev_err(dev, "No valid strength found, minimum %d\n",
> +			dev_err(ctrl->dev,
> +				"No valid strength found, minimum %d\n",
>  				chip->ecc_strength_ds);
>  			return ret;
>  		}
> @@ -1039,7 +980,7 @@ static int tegra_nand_chips_init(struct device *dev,
>  			nand->config_ecc |= CONFIG_TVAL_8;
>  			break;
>  		default:
> -			dev_err(dev, "ECC strength %d not supported\n",
> +			dev_err(ctrl->dev, "ECC strength %d not supported\n",
>  				chip->ecc.strength);
>  			return -EINVAL;
>  		}
> @@ -1062,17 +1003,17 @@ static int tegra_nand_chips_init(struct device *dev,
>  			nand->bch_config |= BCH_TVAL_16;
>  			break;
>  		default:
> -			dev_err(dev, "ECC strength %d not supported\n",
> +			dev_err(ctrl->dev, "ECC strength %d not supported\n",
>  				chip->ecc.strength);
>  			return -EINVAL;
>  		}
>  		break;
>  	default:
> -		dev_err(dev, "ECC algorithm not supported\n");
> +		dev_err(ctrl->dev, "ECC algorithm not supported\n");
>  		return -EINVAL;
>  	}
>  
> -	dev_info(dev, "Using %s with strength %d per 512 byte step\n",
> +	dev_info(ctrl->dev, "Using %s with strength %d per 512 byte step\n",
>  		 chip->ecc.algo == NAND_ECC_BCH ? "BCH" : "RS",
>  		 chip->ecc.strength);
>  
> @@ -1095,7 +1036,8 @@ static int tegra_nand_chips_init(struct device *dev,
>  		nand->config |= CONFIG_PS_4096;
>  		break;
>  	default:
> -		dev_err(dev, "Unsupported writesize %d\n", mtd->writesize);
> +		dev_err(ctrl->dev, "Unsupported writesize %d\n",
> +			mtd->writesize);
>  		return -ENODEV;
>  	}
>  
> @@ -1106,7 +1048,78 @@ static int tegra_nand_chips_init(struct device *dev,
>  	nand->config |= CONFIG_TAG_BYTE_SIZE(mtd->oobsize - 1);
>  	writel_relaxed(nand->config, ctrl->regs + CONFIG);
>  
> -	ret = nand_scan_tail(mtd);
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops tegra_nand_controller_ops = {
> +	.attach_chip = &tegra_nand_attach_chip,
> +};
> +
> +static int tegra_nand_chips_init(struct device *dev,
> +				 struct tegra_nand_controller *ctrl)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *np_nand;
> +	int nsels, nchips = of_get_child_count(np);
> +	struct tegra_nand_chip *nand;
> +	struct mtd_info *mtd;
> +	struct nand_chip *chip;
> +	int ret;
> +	u32 cs;
> +
> +	if (nchips != 1) {
> +		dev_err(dev, "Currently only one NAND chip supported\n");
> +		return -EINVAL;
> +	}
> +
> +	np_nand = of_get_next_child(np, NULL);
> +
> +	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
> +	if (nsels != 1) {
> +		dev_err(dev, "Missing/invalid reg property\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Retrieve CS id, currently only single die NAND supported */
> +	ret = of_property_read_u32(np_nand, "reg", &cs);
> +	if (ret) {
> +		dev_err(dev, "could not retrieve reg property: %d\n", ret);
> +		return ret;
> +	}
> +
> +	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
> +	if (!nand)
> +		return -ENOMEM;
> +
> +	nand->cs[0] = cs;
> +
> +	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
> +
> +	if (IS_ERR(nand->wp_gpio)) {
> +		ret = PTR_ERR(nand->wp_gpio);
> +		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	chip = &nand->chip;
> +	chip->controller = &ctrl->controller;
> +
> +	mtd = nand_to_mtd(chip);
> +
> +	mtd->dev.parent = dev;
> +	mtd->owner = THIS_MODULE;
> +
> +	nand_set_flash_node(chip, np_nand);
> +
> +	if (!mtd->name)
> +		mtd->name = "tegra_nand";
> +
> +	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
> +	chip->exec_op = tegra_nand_exec_op;
> +	chip->select_chip = tegra_nand_select_chip;
> +	chip->setup_data_interface = tegra_nand_setup_data_interface;
> +
> +	ret = nand_scan(mtd, 1);
>  	if (ret)
>  		return ret;
>  
> @@ -1137,6 +1150,7 @@ static int tegra_nand_probe(struct platform_device *pdev)
>  
>  	ctrl->dev = &pdev->dev;
>  	nand_controller_init(&ctrl->controller);
> +	ctrl->controller.ops = &tegra_nand_controller_ops;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	ctrl->regs = devm_ioremap_resource(&pdev->dev, res);

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Boris Brezillon]

On Thu, 26 Jul 2018 18:29:36 +0200
Stefan Agner <stefan@agner.ch> wrote:

> On 25.07.2018 15:31, Miquel Raynal wrote:
> > Two helpers have been added to the core to do all kind of controller
> > side configuration/initialization between the detection phase and the
> > final NAND scan. Implement these hooks so that we can convert the driver
> > to just use nand_scan() instead of the nand_scan_ident() +
> > nand_scan_tail() pair.
> >   
> 
> While the patch looks technically correct, I wonder whether the driver
> now does what we expect it from attach logically...
> 
> E.g. shouldn't we get the wp_gpio in attach?

Well, this series does things mechanically to avoid breaking drivers (we
just move all the code between ident and tail into the attach hook),
but any resource that is not needed for the identification phase and is
tied to the NAND chip could/should be requested in the attach hook (the
WP pin is such a resource).

Feel free to send a patch to change that.

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Stefan Agner]

On 26.07.2018 20:01, Boris Brezillon wrote:
> On Thu, 26 Jul 2018 18:29:36 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
>> On 25.07.2018 15:31, Miquel Raynal wrote:
>> > Two helpers have been added to the core to do all kind of controller
>> > side configuration/initialization between the detection phase and the
>> > final NAND scan. Implement these hooks so that we can convert the driver
>> > to just use nand_scan() instead of the nand_scan_ident() +
>> > nand_scan_tail() pair.
>> >
>>
>> While the patch looks technically correct, I wonder whether the driver
>> now does what we expect it from attach logically...
>>
>> E.g. shouldn't we get the wp_gpio in attach?
> 
> Well, this series does things mechanically to avoid breaking drivers (we
> just move all the code between ident and tail into the attach hook),
> but any resource that is not needed for the identification phase and is
> tied to the NAND chip could/should be requested in the attach hook (the
> WP pin is such a resource).

Ok, that makes completely sense and I agree with that approach! However,
it is not obvious when looking at the series.

Can we mention that fact in the commit log, e.g. something like:

"To avoid breaking this patch converts the driver mechanically by just
moving all the code between ...ident and ..tail into the attach hook.
Ideally driver should request all resources tied to the NAND chip in the
attach hook."

--
Stefan

> 
> Feel free to send a patch to change that.

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Miquel Raynal]

Hi Stefan,

Stefan Agner <stefan@agner.ch> wrote on Fri, 27 Jul 2018 09:13:09 +0200:

> On 26.07.2018 20:01, Boris Brezillon wrote:
> > On Thu, 26 Jul 2018 18:29:36 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> >   
> >> On 25.07.2018 15:31, Miquel Raynal wrote:  
> >> > Two helpers have been added to the core to do all kind of controller
> >> > side configuration/initialization between the detection phase and the
> >> > final NAND scan. Implement these hooks so that we can convert the driver
> >> > to just use nand_scan() instead of the nand_scan_ident() +
> >> > nand_scan_tail() pair.
> >> >  
> >>
> >> While the patch looks technically correct, I wonder whether the driver
> >> now does what we expect it from attach logically...
> >>
> >> E.g. shouldn't we get the wp_gpio in attach?  
> > 
> > Well, this series does things mechanically to avoid breaking drivers (we
> > just move all the code between ident and tail into the attach hook),
> > but any resource that is not needed for the identification phase and is
> > tied to the NAND chip could/should be requested in the attach hook (the
> > WP pin is such a resource).  
> 
> Ok, that makes completely sense and I agree with that approach! However,
> it is not obvious when looking at the series.
> 
> Can we mention that fact in the commit log, e.g. something like:
> 
> "To avoid breaking this patch converts the driver mechanically by just
> moving all the code between ...ident and ..tail into the attach hook.
> Ideally driver should request all resources tied to the NAND chip in the
> attach hook."

As Boris said, the whole change was somehow mechanic, I already had a
hard time understanding what each driver wanted to achieve between
those two functions, I did not dig any further as it was already very
time consuming. A cleanup of many drivers would be appreciated though.

As this would apply to the 30 patches of the series and because I
already merged all of them, I think I'll pass for this one, even if I
completely agree with the request.

Thanks,
Miquèl

Re: [PATCH v5 12/17] mtd: rawnand: tegra: convert driver to nand_scan()

[Stefan Agner]

On 27.07.2018 10:06, Miquel Raynal wrote:
> Hi Stefan,
> 
> Stefan Agner <stefan@agner.ch> wrote on Fri, 27 Jul 2018 09:13:09 +0200:
> 
>> On 26.07.2018 20:01, Boris Brezillon wrote:
>> > On Thu, 26 Jul 2018 18:29:36 +0200
>> > Stefan Agner <stefan@agner.ch> wrote:
>> >
>> >> On 25.07.2018 15:31, Miquel Raynal wrote:
>> >> > Two helpers have been added to the core to do all kind of controller
>> >> > side configuration/initialization between the detection phase and the
>> >> > final NAND scan. Implement these hooks so that we can convert the driver
>> >> > to just use nand_scan() instead of the nand_scan_ident() +
>> >> > nand_scan_tail() pair.
>> >> >
>> >>
>> >> While the patch looks technically correct, I wonder whether the driver
>> >> now does what we expect it from attach logically...
>> >>
>> >> E.g. shouldn't we get the wp_gpio in attach?
>> >
>> > Well, this series does things mechanically to avoid breaking drivers (we
>> > just move all the code between ident and tail into the attach hook),
>> > but any resource that is not needed for the identification phase and is
>> > tied to the NAND chip could/should be requested in the attach hook (the
>> > WP pin is such a resource).
>>
>> Ok, that makes completely sense and I agree with that approach! However,
>> it is not obvious when looking at the series.
>>
>> Can we mention that fact in the commit log, e.g. something like:
>>
>> "To avoid breaking this patch converts the driver mechanically by just
>> moving all the code between ...ident and ..tail into the attach hook.
>> Ideally driver should request all resources tied to the NAND chip in the
>> attach hook."
> 
> As Boris said, the whole change was somehow mechanic, I already had a
> hard time understanding what each driver wanted to achieve between
> those two functions, I did not dig any further as it was already very
> time consuming. A cleanup of many drivers would be appreciated though.

Yeah sure, and your work is very much appreciated!

Until closer look and Boris' confirmation it really wasn't clear to me
whether things are on purpose the way they are or because this patch is
seen as a first (mechanical) step...

> 
> As this would apply to the 30 patches of the series and because I
> already merged all of them, I think I'll pass for this one, even if I
> completely agree with the request.

Ok, at least it is documented on the ML now :-)

--
Stefan

[PATCH v5 13/17] mtd: rawnand: txx9ndfmc: clarify ECC parameters assignation

[Miquel Raynal]

A comment in the probe declares that values are assigned to ecc.size
and ecc.bytes, but these values will be overwritten. This is not
entirely right as they are overwritten only if
mtd->writesize >= 512. Let's clarify this by moving these assignations
to txx9ndfmc_nand_scan().

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/txx9ndfmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c
index 9019022774f7..9808b18b15e2 100644
--- a/drivers/mtd/nand/raw/txx9ndfmc.c
+++ b/drivers/mtd/nand/raw/txx9ndfmc.c
@@ -262,10 +262,13 @@ static int txx9ndfmc_nand_scan(struct mtd_info *mtd)
 	ret = nand_scan_ident(mtd, 1, NULL);
 	if (!ret) {
 		if (mtd->writesize >= 512) {
-			/* Hardware ECC 6 byte ECC per 512 Byte data */
 			chip->ecc.size = 512;
 			chip->ecc.bytes = 6;
+		} else {
+			chip->ecc.size = 256;
+			chip->ecc.bytes = 3;
 		}
+
 		ret = nand_scan_tail(mtd);
 	}
 	return ret;
@@ -332,9 +335,6 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 		chip->ecc.correct = txx9ndfmc_correct_data;
 		chip->ecc.hwctl = txx9ndfmc_enable_hwecc;
 		chip->ecc.mode = NAND_ECC_HW;
-		/* txx9ndfmc_nand_scan will overwrite ecc.size and ecc.bytes */
-		chip->ecc.size = 256;
-		chip->ecc.bytes = 3;
 		chip->ecc.strength = 1;
 		chip->chip_delay = 100;
 		chip->controller = &drvdata->controller;
-- 
2.14.1

Re: [PATCH v5 13/17] mtd: rawnand: txx9ndfmc: clarify ECC parameters assignation

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:48 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> A comment in the probe declares that values are assigned to ecc.size
> and ecc.bytes, but these values will be overwritten. This is not
> entirely right as they are overwritten only if
> mtd->writesize >= 512. Let's clarify this by moving these assignations
> to txx9ndfmc_nand_scan().
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/txx9ndfmc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c
> index 9019022774f7..9808b18b15e2 100644
> --- a/drivers/mtd/nand/raw/txx9ndfmc.c
> +++ b/drivers/mtd/nand/raw/txx9ndfmc.c
> @@ -262,10 +262,13 @@ static int txx9ndfmc_nand_scan(struct mtd_info *mtd)
>  	ret = nand_scan_ident(mtd, 1, NULL);
>  	if (!ret) {
>  		if (mtd->writesize >= 512) {
> -			/* Hardware ECC 6 byte ECC per 512 Byte data */
>  			chip->ecc.size = 512;
>  			chip->ecc.bytes = 6;
> +		} else {
> +			chip->ecc.size = 256;
> +			chip->ecc.bytes = 3;
>  		}
> +
>  		ret = nand_scan_tail(mtd);
>  	}
>  	return ret;
> @@ -332,9 +335,6 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
>  		chip->ecc.correct = txx9ndfmc_correct_data;
>  		chip->ecc.hwctl = txx9ndfmc_enable_hwecc;
>  		chip->ecc.mode = NAND_ECC_HW;
> -		/* txx9ndfmc_nand_scan will overwrite ecc.size and ecc.bytes */
> -		chip->ecc.size = 256;
> -		chip->ecc.bytes = 3;
>  		chip->ecc.strength = 1;
>  		chip->chip_delay = 100;
>  		chip->controller = &drvdata->controller;

[PATCH v5 14/17] mtd: rawnand: txx9ndfmc: convert driver to nand_scan()

[Miquel Raynal]

Two helpers have been added to the core to do all kind of controller
side configuration/initialization between the detection phase and the
final NAND scan. Implement these hooks so that we can convert the driver
to just use nand_scan() instead of the nand_scan_ident() +
nand_scan_tail() pair.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/nand/raw/txx9ndfmc.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c
index 9808b18b15e2..4d61a14fcb65 100644
--- a/drivers/mtd/nand/raw/txx9ndfmc.c
+++ b/drivers/mtd/nand/raw/txx9ndfmc.c
@@ -254,26 +254,25 @@ static void txx9ndfmc_initialize(struct platform_device *dev)
 #define TXX9NDFMC_NS_TO_CYC(gbusclk, ns) \
 	DIV_ROUND_UP((ns) * DIV_ROUND_UP(gbusclk, 1000), 1000000)
 
-static int txx9ndfmc_nand_scan(struct mtd_info *mtd)
+static int txx9ndfmc_attach_chip(struct nand_chip *chip)
 {
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	int ret;
+	struct mtd_info *mtd = nand_to_mtd(chip);
 
-	ret = nand_scan_ident(mtd, 1, NULL);
-	if (!ret) {
-		if (mtd->writesize >= 512) {
-			chip->ecc.size = 512;
-			chip->ecc.bytes = 6;
-		} else {
-			chip->ecc.size = 256;
-			chip->ecc.bytes = 3;
-		}
-
-		ret = nand_scan_tail(mtd);
+	if (mtd->writesize >= 512) {
+		chip->ecc.size = 512;
+		chip->ecc.bytes = 6;
+	} else {
+		chip->ecc.size = 256;
+		chip->ecc.bytes = 3;
 	}
-	return ret;
+
+	return 0;
 }
 
+static const struct nand_controller_ops txx9ndfmc_controller_ops = {
+	.attach_chip = txx9ndfmc_attach_chip,
+};
+
 static int __init txx9ndfmc_probe(struct platform_device *dev)
 {
 	struct txx9ndfmc_platform_data *plat = dev_get_platdata(&dev->dev);
@@ -307,6 +306,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 		 (gbusclk + 500000) / 1000000, hold, spw);
 
 	nand_controller_init(&drvdata->controller);
+	drvdata->controller.ops = &txx9ndfmc_controller_ops;
 
 	platform_set_drvdata(dev, drvdata);
 	txx9ndfmc_initialize(dev);
@@ -359,7 +359,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 		if (plat->wide_mask & (1 << i))
 			chip->options |= NAND_BUSWIDTH_16;
 
-		if (txx9ndfmc_nand_scan(mtd)) {
+		if (nand_scan(mtd, 1)) {
 			kfree(txx9_priv->mtdname);
 			kfree(txx9_priv);
 			continue;
-- 
2.14.1

[PATCH v5 15/17] mtd: rawnand: do not export nand_scan_[ident|tail]() anymore

[Miquel Raynal]

Both nand_scan_ident() and nand_scan_tail() helpers used to be called
directly from controller drivers that needed to tweak some ECC-related
parameters before nand_scan_tail(). This separation prevented dynamic
allocations during the phase of NAND identification, which was
inconvenient.

All controller drivers have been moved to use nand_scan(), in
conjunction with the chip->ecc.[attach|detach]_chip() hooks that
actually do the required tweaking sequence between both ident/tail
calls, allowing programmers to use dynamic allocation as they need all
across the scanning sequence.

Declare nand_scan_[ident|tail]() statically now.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 16 +++++++++-------
 include/linux/mtd/rawnand.h      | 18 ++++++------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 42a7a934a17b..34ea44f90fd8 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5924,7 +5924,7 @@ static int nand_dt_init(struct nand_chip *chip)
 }
 
 /**
- * nand_scan_ident - [NAND Interface] Scan for the NAND device
+ * nand_scan_ident - Scan for the NAND device
  * @mtd: MTD device structure
  * @maxchips: number of chips to scan for
  * @table: alternative NAND ID table
@@ -5932,9 +5932,13 @@ static int nand_dt_init(struct nand_chip *chip)
  * This is the first phase of the normal nand_scan() function. It reads the
  * flash ID and sets up MTD fields accordingly.
  *
+ * This helper used to be called directly from controller drivers that needed
+ * to tweak some ECC-related parameters before nand_scan_tail(). This separation
+ * prevented dynamic allocations during this phase which was unconvenient and
+ * as been banned for the benefit of the ->init_ecc()/cleanup_ecc() hooks.
  */
-int nand_scan_ident(struct mtd_info *mtd, int maxchips,
-		    struct nand_flash_dev *table)
+static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
+			   struct nand_flash_dev *table)
 {
 	int i, nand_maf_id, nand_dev_id;
 	struct nand_chip *chip = mtd_to_nand(mtd);
@@ -6008,7 +6012,6 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 
 	return 0;
 }
-EXPORT_SYMBOL(nand_scan_ident);
 
 static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
 {
@@ -6385,14 +6388,14 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 }
 
 /**
- * nand_scan_tail - [NAND Interface] Scan for the NAND device
+ * nand_scan_tail - Scan for the NAND device
  * @mtd: MTD device structure
  *
  * This is the second phase of the normal nand_scan() function. It fills out
  * all the uninitialized function pointers with the defaults and scans for a
  * bad block table if appropriate.
  */
-int nand_scan_tail(struct mtd_info *mtd)
+static int nand_scan_tail(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
@@ -6716,7 +6719,6 @@ int nand_scan_tail(struct mtd_info *mtd)
 
 	return ret;
 }
-EXPORT_SYMBOL(nand_scan_tail);
 
 static int nand_attach(struct nand_chip *chip)
 {
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index a20c78e25878..eaf000b09222 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -35,17 +35,6 @@ static inline int nand_scan(struct mtd_info *mtd, int max_chips)
 	return nand_scan_with_ids(mtd, max_chips, NULL);
 }
 
-/*
- * Separate phases of nand_scan(), allowing board driver to intervene
- * and override command or ECC setup according to flash type.
- */
-int nand_scan_ident(struct mtd_info *mtd, int max_chips,
-			   struct nand_flash_dev *table);
-int nand_scan_tail(struct mtd_info *mtd);
-
-/* Unregister the MTD device and free resources held by the NAND device */
-void nand_release(struct mtd_info *mtd);
-
 /* Internal helper for board drivers which need to override command function */
 void nand_wait_ready(struct mtd_info *mtd);
 
@@ -1739,8 +1728,13 @@ int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
 int nand_write_data_op(struct nand_chip *chip, const void *buf,
 		       unsigned int len, bool force_8bit);
 
-/* Free resources held by the NAND device */
+/*
+ * Free resources held by the NAND device, must be called on error after a
+ * sucessful nand_scan().
+ */
 void nand_cleanup(struct nand_chip *chip);
+/* Unregister the MTD device and calls nand_cleanup() */
+void nand_release(struct mtd_info *mtd);
 
 /* Default extended ID decoding function */
 void nand_decode_ext_id(struct nand_chip *chip);
-- 
2.14.1

Re: [PATCH v5 15/17] mtd: rawnand: do not export nand_scan_[ident|tail]() anymore

[Boris Brezillon]

On Wed, 25 Jul 2018 15:31:50 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Both nand_scan_ident() and nand_scan_tail() helpers used to be called
> directly from controller drivers that needed to tweak some ECC-related
> parameters before nand_scan_tail(). This separation prevented dynamic
> allocations during the phase of NAND identification, which was
> inconvenient.
> 
> All controller drivers have been moved to use nand_scan(), in
> conjunction with the chip->ecc.[attach|detach]_chip() hooks that
> actually do the required tweaking sequence between both ident/tail
> calls, allowing programmers to use dynamic allocation as they need all
> across the scanning sequence.
> 
> Declare nand_scan_[ident|tail]() statically now.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/nand_base.c | 16 +++++++++-------
>  include/linux/mtd/rawnand.h      | 18 ++++++------------
>  2 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 42a7a934a17b..34ea44f90fd8 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5924,7 +5924,7 @@ static int nand_dt_init(struct nand_chip *chip)
>  }
>  
>  /**
> - * nand_scan_ident - [NAND Interface] Scan for the NAND device
> + * nand_scan_ident - Scan for the NAND device
>   * @mtd: MTD device structure
>   * @maxchips: number of chips to scan for
>   * @table: alternative NAND ID table
> @@ -5932,9 +5932,13 @@ static int nand_dt_init(struct nand_chip *chip)
>   * This is the first phase of the normal nand_scan() function. It reads the
>   * flash ID and sets up MTD fields accordingly.
>   *
> + * This helper used to be called directly from controller drivers that needed
> + * to tweak some ECC-related parameters before nand_scan_tail(). This separation
> + * prevented dynamic allocations during this phase which was unconvenient and
> + * as been banned for the benefit of the ->init_ecc()/cleanup_ecc() hooks.
>   */
> -int nand_scan_ident(struct mtd_info *mtd, int maxchips,
> -		    struct nand_flash_dev *table)
> +static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
> +			   struct nand_flash_dev *table)
>  {
>  	int i, nand_maf_id, nand_dev_id;
>  	struct nand_chip *chip = mtd_to_nand(mtd);
> @@ -6008,7 +6012,6 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(nand_scan_ident);
>  
>  static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
>  {
> @@ -6385,14 +6388,14 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
>  }
>  
>  /**
> - * nand_scan_tail - [NAND Interface] Scan for the NAND device
> + * nand_scan_tail - Scan for the NAND device
>   * @mtd: MTD device structure
>   *
>   * This is the second phase of the normal nand_scan() function. It fills out
>   * all the uninitialized function pointers with the defaults and scans for a
>   * bad block table if appropriate.
>   */
> -int nand_scan_tail(struct mtd_info *mtd)
> +static int nand_scan_tail(struct mtd_info *mtd)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct nand_ecc_ctrl *ecc = &chip->ecc;
> @@ -6716,7 +6719,6 @@ int nand_scan_tail(struct mtd_info *mtd)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(nand_scan_tail);
>  
>  static int nand_attach(struct nand_chip *chip)
>  {
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index a20c78e25878..eaf000b09222 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -35,17 +35,6 @@ static inline int nand_scan(struct mtd_info *mtd, int max_chips)
>  	return nand_scan_with_ids(mtd, max_chips, NULL);
>  }
>  
> -/*
> - * Separate phases of nand_scan(), allowing board driver to intervene
> - * and override command or ECC setup according to flash type.
> - */
> -int nand_scan_ident(struct mtd_info *mtd, int max_chips,
> -			   struct nand_flash_dev *table);
> -int nand_scan_tail(struct mtd_info *mtd);
> -
> -/* Unregister the MTD device and free resources held by the NAND device */
> -void nand_release(struct mtd_info *mtd);
> -
>  /* Internal helper for board drivers which need to override command function */
>  void nand_wait_ready(struct mtd_info *mtd);
>  
> @@ -1739,8 +1728,13 @@ int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
>  int nand_write_data_op(struct nand_chip *chip, const void *buf,
>  		       unsigned int len, bool force_8bit);
>  
> -/* Free resources held by the NAND device */
> +/*
> + * Free resources held by the NAND device, must be called on error after a

								      ^ and

> + * sucessful nand_scan().
> + */
>  void nand_cleanup(struct nand_chip *chip);
> +/* Unregister the MTD device and calls nand_cleanup() */
> +void nand_release(struct mtd_info *mtd);
>  
>  /* Default extended ID decoding function */
>  void nand_decode_ext_id(struct nand_chip *chip);

[PATCH v5 16/17] mtd: rawnand: allocate model parameter dynamically

[Miquel Raynal]

Thanks to the migration of all drivers to use nand_scan() and the
related nand_controller_ops, we can now allocate data during the
detection phase. Let's do it first for the NAND model parameter which
is allocated in nand_detect().

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 52 +++++++++++++++++++++++++++++++---------
 include/linux/mtd/rawnand.h      |  2 +-
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 34ea44f90fd8..00e80781124a 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5225,8 +5225,11 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
 	sanitize_string(p->model, sizeof(p->model));
-	strncpy(chip->parameters.model, p->model,
-		sizeof(chip->parameters.model) - 1);
+	chip->parameters.model = kstrdup(p->model, GFP_KERNEL);
+	if (!chip->parameters.model) {
+		ret = -ENOMEM;
+		goto free_onfi_param_page;
+	}
 
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
 
@@ -5356,8 +5359,11 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
 	sanitize_string(p->model, sizeof(p->model));
-	strncpy(chip->parameters.model, p->model,
-		sizeof(chip->parameters.model) - 1);
+	chip->parameters.model = kstrdup(p->model, GFP_KERNEL);
+	if (!chip->parameters.model) {
+		ret = -ENOMEM;
+		goto free_jedec_param_page;
+	}
 
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
 
@@ -5546,8 +5552,9 @@ static bool find_full_id_nand(struct nand_chip *chip,
 		chip->onfi_timing_mode_default =
 					type->onfi_timing_mode_default;
 
-		strncpy(chip->parameters.model, type->name,
-			sizeof(chip->parameters.model) - 1);
+		chip->parameters.model = kstrdup(type->name, GFP_KERNEL);
+		if (!chip->parameters.model)
+			return false;
 
 		return true;
 	}
@@ -5706,8 +5713,9 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	if (!type->name)
 		return -ENODEV;
 
-	strncpy(chip->parameters.model, type->name,
-		sizeof(chip->parameters.model) - 1);
+	chip->parameters.model = kstrdup(type->name, GFP_KERNEL);
+	if (!chip->parameters.model)
+		return -ENOMEM;
 
 	chip->chipsize = (uint64_t)type->chipsize << 20;
 
@@ -5737,7 +5745,9 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 			mtd->name);
 		pr_warn("bus width %d instead of %d bits\n", busw ? 16 : 8,
 			(chip->options & NAND_BUSWIDTH_16) ? 16 : 8);
-		return -EINVAL;
+		ret = -EINVAL;
+
+		goto free_detect_allocation;
 	}
 
 	nand_decode_bbm_options(chip);
@@ -5774,6 +5784,11 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
 		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
 	return 0;
+
+free_detect_allocation:
+	kfree(chip->parameters.model);
+
+	return ret;
 }
 
 static const char * const nand_ecc_modes[] = {
@@ -6013,6 +6028,11 @@ static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	return 0;
 }
 
+static void nand_scan_ident_cleanup(struct nand_chip *chip)
+{
+	kfree(chip->parameters.model);
+}
+
 static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
@@ -6760,11 +6780,18 @@ int nand_scan_with_ids(struct mtd_info *mtd, int maxchips,
 
 	ret = nand_attach(chip);
 	if (ret)
-		return ret;
+		goto cleanup_ident;
 
 	ret = nand_scan_tail(mtd);
 	if (ret)
-		nand_detach(chip);
+		goto detach_chip;
+
+	return 0;
+
+detach_chip:
+	nand_detach(chip);
+cleanup_ident:
+	nand_scan_ident_cleanup(chip);
 
 	return ret;
 }
@@ -6796,6 +6823,9 @@ void nand_cleanup(struct nand_chip *chip)
 
 	/* Free controller specific allocations after chip identification */
 	nand_detach(chip);
+
+	/* Free identification phase allocations */
+	nand_scan_ident_cleanup(chip);
 }
 
 EXPORT_SYMBOL_GPL(nand_cleanup);
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index eaf000b09222..4848c00f3eda 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -476,7 +476,7 @@ struct onfi_params {
  */
 struct nand_parameters {
 	/* Generic parameters */
-	char model[100];
+	const char *model;
 	bool supports_set_get_features;
 	DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
 	DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
-- 
2.14.1

[PATCH v5 17/17] mtd: rawnand: allocate dynamically ONFI parameters during detection

[Miquel Raynal]

Now that it is possible to do dynamic allocations during the
identification phase, convert the onfi_params structure (which is only
needed with ONFI compliant chips) into a pointer that will be allocated
only if needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c    | 54 +++++++++++++++++++++++--------------
 drivers/mtd/nand/raw/nand_micron.c  |  6 ++---
 drivers/mtd/nand/raw/nand_timings.c | 12 ++++-----
 include/linux/mtd/rawnand.h         |  6 ++---
 4 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 00e80781124a..d527e448ce19 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5151,6 +5151,8 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_onfi_params *p;
+	struct onfi_params *onfi;
+	int onfi_version = 0;
 	char id[4];
 	int i, ret, val;
 
@@ -5206,21 +5208,19 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	/* Check version */
 	val = le16_to_cpu(p->revision);
 	if (val & ONFI_VERSION_2_3)
-		chip->parameters.onfi.version = 23;
+		onfi_version = 23;
 	else if (val & ONFI_VERSION_2_2)
-		chip->parameters.onfi.version = 22;
+		onfi_version = 22;
 	else if (val & ONFI_VERSION_2_1)
-		chip->parameters.onfi.version = 21;
+		onfi_version = 21;
 	else if (val & ONFI_VERSION_2_0)
-		chip->parameters.onfi.version = 20;
+		onfi_version = 20;
 	else if (val & ONFI_VERSION_1_0)
-		chip->parameters.onfi.version = 10;
+		onfi_version = 10;
 
-	if (!chip->parameters.onfi.version) {
+	if (!onfi_version) {
 		pr_info("unsupported ONFI version: %d\n", val);
 		goto free_onfi_param_page;
-	} else {
-		ret = 1;
 	}
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -5257,7 +5257,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	if (p->ecc_bits != 0xff) {
 		chip->ecc_strength_ds = p->ecc_bits;
 		chip->ecc_step_ds = 512;
-	} else if (chip->parameters.onfi.version >= 21 &&
+	} else if (onfi_version >= 21 &&
 		(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
 
 		/*
@@ -5284,19 +5284,33 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 		bitmap_set(chip->parameters.set_feature_list,
 			   ONFI_FEATURE_ADDR_TIMING_MODE, 1);
 	}
-	chip->parameters.onfi.tPROG = le16_to_cpu(p->t_prog);
-	chip->parameters.onfi.tBERS = le16_to_cpu(p->t_bers);
-	chip->parameters.onfi.tR = le16_to_cpu(p->t_r);
-	chip->parameters.onfi.tCCS = le16_to_cpu(p->t_ccs);
-	chip->parameters.onfi.async_timing_mode =
-		le16_to_cpu(p->async_timing_mode);
-	chip->parameters.onfi.vendor_revision =
-		le16_to_cpu(p->vendor_revision);
-	memcpy(chip->parameters.onfi.vendor, p->vendor,
-	       sizeof(p->vendor));
 
+	onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
+	if (!onfi) {
+		ret = -ENOMEM;
+		goto free_model;
+	}
+
+	onfi->version = onfi_version;
+	onfi->tPROG = le16_to_cpu(p->t_prog);
+	onfi->tBERS = le16_to_cpu(p->t_bers);
+	onfi->tR = le16_to_cpu(p->t_r);
+	onfi->tCCS = le16_to_cpu(p->t_ccs);
+	onfi->async_timing_mode = le16_to_cpu(p->async_timing_mode);
+	onfi->vendor_revision = le16_to_cpu(p->vendor_revision);
+	memcpy(onfi->vendor, p->vendor, sizeof(p->vendor));
+	chip->parameters.onfi = onfi;
+
+	/* Identification done, free the full ONFI parameter page and exit */
+	kfree(p);
+
+	return 1;
+
+free_model:
+	kfree(chip->parameters.model);
 free_onfi_param_page:
 	kfree(p);
+
 	return ret;
 }
 
@@ -5693,7 +5707,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 		}
 	}
 
-	chip->parameters.onfi.version = 0;
 	if (!type->name || !type->pagesize) {
 		/* Check if the chip is ONFI compliant */
 		ret = nand_flash_detect_onfi(chip);
@@ -6031,6 +6044,7 @@ static int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 static void nand_scan_ident_cleanup(struct nand_chip *chip)
 {
 	kfree(chip->parameters.model);
+	kfree(chip->parameters.onfi);
 }
 
 static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 656947d91841..fa3e0f423774 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -88,9 +88,9 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
 static int micron_nand_onfi_init(struct nand_chip *chip)
 {
 	struct nand_parameters *p = &chip->parameters;
-	struct nand_onfi_vendor_micron *micron = (void *)p->onfi.vendor;
+	struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
 
-	if (chip->parameters.onfi.version && p->onfi.vendor_revision) {
+	if (p->onfi) {
 		chip->read_retries = micron->read_retry_options;
 		chip->setup_read_retry = micron_nand_setup_read_retry;
 	}
@@ -382,7 +382,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	u8 id[5];
 	int ret;
 
-	if (!chip->parameters.onfi.version)
+	if (!chip->parameters.onfi)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	if (chip->bits_per_cell != 1)
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 9bb599106a31..ebc7b5f76f77 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -294,6 +294,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 			     int timing_mode)
 {
 	struct nand_data_interface *iface = &chip->data_interface;
+	struct onfi_params *onfi = chip->parameters.onfi;
 
 	if (type != NAND_SDR_IFACE)
 		return -EINVAL;
@@ -308,17 +309,16 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 	 * tPROG, tBERS, tR and tCCS.
 	 * These information are part of the ONFI parameter page.
 	 */
-	if (chip->parameters.onfi.version) {
-		struct nand_parameters *params = &chip->parameters;
+	if (onfi) {
 		struct nand_sdr_timings *timings = &iface->timings.sdr;
 
 		/* microseconds -> picoseconds */
-		timings->tPROG_max = 1000000ULL * params->onfi.tPROG;
-		timings->tBERS_max = 1000000ULL * params->onfi.tBERS;
-		timings->tR_max = 1000000ULL * params->onfi.tR;
+		timings->tPROG_max = 1000000ULL * onfi->tPROG;
+		timings->tBERS_max = 1000000ULL * onfi->tBERS;
+		timings->tR_max = 1000000ULL * onfi->tR;
 
 		/* nanoseconds -> picoseconds */
-		timings->tCCS_min = 1000UL * params->onfi.tCCS;
+		timings->tCCS_min = 1000UL * onfi->tCCS;
 	} else {
 		struct nand_sdr_timings *timings = &iface->timings.sdr;
 		/*
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4848c00f3eda..3e79c5637b05 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -482,7 +482,7 @@ struct nand_parameters {
 	DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
 
 	/* ONFI parameters */
-	struct onfi_params onfi;
+	struct onfi_params *onfi;
 };
 
 /* The maximum expected count of bytes in the NAND ID sequence */
@@ -1612,10 +1612,10 @@ struct platform_nand_data {
 /* return the supported asynchronous timing mode. */
 static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
 {
-	if (!chip->parameters.onfi.version)
+	if (!chip->parameters.onfi)
 		return ONFI_TIMING_MODE_UNKNOWN;
 
-	return chip->parameters.onfi.async_timing_mode;
+	return chip->parameters.onfi->async_timing_mode;
 }
 
 int onfi_fill_data_interface(struct nand_chip *chip,
-- 
2.14.1