[PATCH 0/4] mtd: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[PATCH 0/4] mtd: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Huang Shijie]

[1] Why kill them?
    As time goes on, the NAND's page size and oob size become larger and larger.
    So we have changes these two macros frequently in order to support the new
    NAND.

[2] How does this patch set do?
    There are three drivers which uses these two macros, the mxc-nand, denali,
    and the cafe-nand.

    This patch will allocate a temporary buffer for the nand_scan_ident, and 
    after it have getten the right page size and oob size, it will re-allocate
    the buffer again.

[3] I do not have the boards which have the relative NAND controller.

    I hope someone can test this patch set.

    thanks.


Huang Shijie (4):
  mtd: mxc-nand: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
  mtd: denali: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
  mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for
    nand_buffers{}
  mtd: nand: remove the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

 drivers/mtd/nand/cafe_nand.c | 49 +++++++++++++++++++++++++++++++-----------
 drivers/mtd/nand/denali.c    | 51 ++++++++++++++++++++++++++------------------
 drivers/mtd/nand/denali.h    |  4 +---
 drivers/mtd/nand/mxc_nand.c  | 18 +++++++++++++---
 drivers/mtd/nand/nand_base.c | 19 +++++++++++++----
 include/linux/mtd/nand.h     | 20 ++++++-----------
 6 files changed, 103 insertions(+), 58 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] mtd: mxc-nand: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Huang Shijie]

We kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE by the following way:
 1.) Before we call the nand_scan_ident, we allocate a temporary buffer
     whose size is PAGE_SIZE.
 2.) After we finish the nand_scan_ident, we have already getten the
     page size and oob size. We will allocate the right buffer size
     again.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 drivers/mtd/nand/mxc_nand.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 567a5e5..e9a4835 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1399,12 +1399,15 @@ static int mxcnd_probe(struct platform_device *pdev)
 	int err = 0;
 
 	/* Allocate memory for MTD device structure and private data */
-	host = devm_kzalloc(&pdev->dev, sizeof(struct mxc_nand_host) +
-			NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE, GFP_KERNEL);
+	host = devm_kzalloc(&pdev->dev, sizeof(struct mxc_nand_host),
+			GFP_KERNEL);
 	if (!host)
 		return -ENOMEM;
 
-	host->data_buf = (uint8_t *)(host + 1);
+	/* allocate a temporary buffer for the nand_scan_ident() */
+	host->data_buf = devm_kzalloc(&pdev->dev, PAGE_SIZE, GFP_KERNEL);
+	if (!host->data_buf)
+		return -ENOMEM;
 
 	host->dev = &pdev->dev;
 	/* structures must be linked */
@@ -1532,6 +1535,15 @@ static int mxcnd_probe(struct platform_device *pdev)
 		goto escan;
 	}
 
+	/* allocate the right size buffer now */
+	devm_kfree(&pdev->dev, (void *)host->data_buf);
+	host->data_buf = devm_kzalloc(&pdev->dev, mtd->writesize + mtd->oobsize,
+					GFP_KERNEL);
+	if (!host->data_buf) {
+		err = -ENOMEM;
+		goto escan;
+	}
+
 	/* Call preset again, with correct writesize this time */
 	host->devtype_data->preset(mtd);
 
-- 
1.8.3.1

[PATCH 2/4] mtd: denali: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Huang Shijie]

This patch kills the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE by the following
way:
 1.) change the @buf field of nand_buf{} from an array to a pointer.
     also remove the DENALI_BUF_SIZE macro.

 2.) Before we call the nand_scan_ident, we allocate a temporary buffer
     whose size is PAGE_SIZE.

 3.) After we finish the nand_scan_ident, we have already getten the
     page size and oob size. We will allocate the right buffer size
     again.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 drivers/mtd/nand/denali.c | 51 ++++++++++++++++++++++++++++-------------------
 drivers/mtd/nand/denali.h |  4 +---
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 3a3a47f..c07cd57 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -125,7 +125,6 @@ static void reset_buf(struct denali_nand_info *denali)
 
 static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
 {
-	BUG_ON(denali->buf.tail >= sizeof(denali->buf.buf));
 	denali->buf.buf[denali->buf.tail++] = byte;
 }
 
@@ -1429,20 +1428,12 @@ int denali_init(struct denali_nand_info *denali)
 		}
 	}
 
-	/* Is 32-bit DMA supported? */
-	ret = dma_set_mask(denali->dev, DMA_BIT_MASK(32));
-	if (ret) {
-		pr_err("Spectra: no usable DMA configuration\n");
-		return ret;
-	}
-	denali->buf.dma_buf = dma_map_single(denali->dev, denali->buf.buf,
-					     DENALI_BUF_SIZE,
-					     DMA_BIDIRECTIONAL);
+	/* allocate a temporary buffer for nand_scan_ident() */
+	denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE,
+					GFP_DMA | GFP_KERNEL);
+	if (!denali->buf.buf)
+		return -ENOMEM;
 
-	if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
-		dev_err(denali->dev, "Spectra: failed to map DMA buffer\n");
-		return -EIO;
-	}
 	denali->mtd.dev.parent = denali->dev;
 	denali_hw_init(denali);
 	denali_drv_init(denali);
@@ -1475,12 +1466,29 @@ int denali_init(struct denali_nand_info *denali)
 		goto failed_req_irq;
 	}
 
-	/* MTD supported page sizes vary by kernel. We validate our
-	 * kernel supports the device here.
-	 */
-	if (denali->mtd.writesize > NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) {
-		ret = -ENODEV;
-		pr_err("Spectra: device size not supported by this version of MTD.");
+	/* allocate the right size buffer now */
+	devm_kfree(denali->dev, denali->buf.buf);
+	denali->buf.buf = devm_kzalloc(denali->dev,
+			     denali->mtd.writesize + denali->mtd.oobsize,
+			     GFP_KERNEL);
+	if (!denali->buf.buf) {
+		ret = -ENOMEM;
+		goto failed_req_irq;
+	}
+
+	/* Is 32-bit DMA supported? */
+	ret = dma_set_mask(denali->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		pr_err("Spectra: no usable DMA configuration\n");
+		goto failed_req_irq;
+	}
+
+	denali->buf.dma_buf = dma_map_single(denali->dev, denali->buf.buf,
+			     denali->mtd.writesize + denali->mtd.oobsize,
+			     DMA_BIDIRECTIONAL);
+	if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
+		dev_err(denali->dev, "Spectra: failed to map DMA buffer\n");
+		ret = -EIO;
 		goto failed_req_irq;
 	}
 
@@ -1602,7 +1610,8 @@ EXPORT_SYMBOL(denali_init);
 void denali_remove(struct denali_nand_info *denali)
 {
 	denali_irq_cleanup(denali->irq, denali);
-	dma_unmap_single(denali->dev, denali->buf.dma_buf, DENALI_BUF_SIZE,
+	dma_unmap_single(denali->dev, denali->buf.dma_buf,
+			denali->mtd.writesize + denali->mtd.oobsize,
 			DMA_BIDIRECTIONAL);
 }
 EXPORT_SYMBOL(denali_remove);
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index cec5712..9668174 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -455,12 +455,10 @@
 
 #define ECC_SECTOR_SIZE     512
 
-#define DENALI_BUF_SIZE		(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE)
-
 struct nand_buf {
 	int head;
 	int tail;
-	uint8_t buf[DENALI_BUF_SIZE];
+	uint8_t *buf;
 	dma_addr_t dma_buf;
 };
 
-- 
1.8.3.1

[PATCH 3/4] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Huang Shijie]

The patch converts the arrays to buffer pointers for nand_buffers{}.

The cafe_nand.c is the only NAND_OWN_BUFFERS user which allocates
a nand_buffers{} itself.

This patch disables the DMA for nand_scan_ident, and restore the DMA
status after we finish the nand_scan_ident. By this way, we can get the
mtd->writesize and mtd->oobsize, and then allocates the cafe->dmabuf
with them.

Since the cafe_nand.c uses the NAND_ECC_HW_SYNDROME ECC mode, we do not
allocate the buffers for @ecccalc and @ecccode.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 drivers/mtd/nand/cafe_nand.c | 49 ++++++++++++++++++++++++++++++++------------
 drivers/mtd/nand/nand_base.c | 19 +++++++++++++----
 include/linux/mtd/nand.h     | 12 +++++------
 3 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index c34985a..e9186e7 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -627,6 +627,8 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	struct cafe_priv *cafe;
 	uint32_t ctrl;
 	int err = 0;
+	int old_dma;
+	struct nand_buffers *nbuf;
 
 	/* Very old versions shared the same PCI ident for all three
 	   functions on the chip. Verify the class too... */
@@ -657,13 +659,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 		err = -ENOMEM;
 		goto out_free_mtd;
 	}
-	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112 + sizeof(struct nand_buffers),
-					  &cafe->dmaaddr, GFP_KERNEL);
-	if (!cafe->dmabuf) {
-		err = -ENOMEM;
-		goto out_ior;
-	}
-	cafe->nand.buffers = (void *)cafe->dmabuf + 2112;
 
 	cafe->rs = init_rs_non_canonical(12, &cafe_mul, 0, 1, 8);
 	if (!cafe->rs) {
@@ -723,7 +718,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 			  "CAFE NAND", mtd);
 	if (err) {
 		dev_warn(&pdev->dev, "Could not register IRQ %d\n", pdev->irq);
-		goto out_free_dma;
+		goto out_ior;
 	}
 
 	/* Disable master reset, enable NAND clock */
@@ -753,12 +748,34 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	cafe_dev_dbg(&cafe->pdev->dev, "Control %x, IRQ mask %x\n",
 		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;
+
 	/* Scan to find existence of the device */
 	if (nand_scan_ident(mtd, 2, NULL)) {
 		err = -ENXIO;
 		goto out_irq;
 	}
 
+	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev,
+				2112 + sizeof(struct nand_buffers) +
+				mtd->writesize + mtd->oobsize,
+				&cafe->dmaaddr, GFP_KERNEL);
+	if (!cafe->dmabuf) {
+		err = -ENOMEM;
+		goto out_irq;
+	}
+	cafe->nand.buffers = nbuf = (void *)cafe->dmabuf + 2112;
+
+	/* this driver does not need the @ecccalc and @ecccode */
+	nbuf->ecccalc = NULL;
+	nbuf->ecccode = NULL;
+	nbuf->databuf = (uint8_t *)(nbuf + 1);
+
+	/* 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 */
@@ -775,7 +792,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	} else {
 		printk(KERN_WARNING "Unexpected NAND flash writesize %d. Aborting\n",
 		       mtd->writesize);
-		goto out_irq;
+		goto out_free_dma;
 	}
 	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
 	cafe->nand.ecc.size = mtd->writesize;
@@ -792,7 +809,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 
 	err = nand_scan_tail(mtd);
 	if (err)
-		goto out_irq;
+		goto out_free_dma;
 
 	pci_set_drvdata(pdev, mtd);
 
@@ -801,12 +818,15 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 
 	goto out;
 
+ out_free_dma:
+	dma_free_coherent(&cafe->pdev->dev,
+			2112 + sizeof(struct nand_buffers) +
+			mtd->writesize + mtd->oobsize,
+			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);
 	free_irq(pdev->irq, mtd);
- out_free_dma:
-	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
  out_ior:
 	pci_iounmap(pdev, cafe->mmio);
  out_free_mtd:
@@ -826,7 +846,10 @@ static void cafe_nand_remove(struct pci_dev *pdev)
 	nand_release(mtd);
 	free_rs(cafe->rs);
 	pci_iounmap(pdev, cafe->mmio);
-	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
+	dma_free_coherent(&cafe->pdev->dev,
+			2112 + sizeof(struct nand_buffers) +
+			mtd->writesize + mtd->oobsize,
+			cafe->dmabuf, cafe->dmaaddr);
 	kfree(mtd);
 }
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 9b3bb3c..02d6707 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3559,15 +3559,26 @@ int nand_scan_tail(struct mtd_info *mtd)
 	int i;
 	struct nand_chip *chip = mtd->priv;
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_buffers *nbuf;
 
 	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
 	BUG_ON((chip->bbt_options & NAND_BBT_NO_OOB_BBM) &&
 			!(chip->bbt_options & NAND_BBT_USE_FLASH));
 
-	if (!(chip->options & NAND_OWN_BUFFERS))
-		chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
-	if (!chip->buffers)
-		return -ENOMEM;
+	if (!(chip->options & NAND_OWN_BUFFERS)) {
+		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+				+ mtd->oobsize * 3, GFP_KERNEL);
+		if (!nbuf)
+			return -ENOMEM;
+		nbuf->ecccalc = (uint8_t *)(nbuf + 1);
+		nbuf->ecccode = nbuf->ecccalc + mtd->oobsize;
+		nbuf->databuf = nbuf->ecccode + mtd->oobsize;
+
+		chip->buffers = nbuf;
+	} else {
+		if (!chip->buffers)
+			return -ENOMEM;
+	}
 
 	/* Set the internal oob buffer location, just after the page data */
 	chip->oob_poi = chip->buffers->databuf + mtd->writesize;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f3ea8da..4bbcaba 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -411,17 +411,17 @@ struct nand_ecc_ctrl {
 
 /**
  * struct nand_buffers - buffer structure for read/write
- * @ecccalc:	buffer for calculated ECC
- * @ecccode:	buffer for ECC read from flash
- * @databuf:	buffer for data - dynamically sized
+ * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
+ * @ecccode:	buffer pointer for ECC read from flash, size is oobsize.
+ * @databuf:	buffer pointer for data, size is (page size + oobsize).
  *
  * Do not change the order of buffers. databuf and oobrbuf must be in
  * consecutive order.
  */
 struct nand_buffers {
-	uint8_t	ecccalc[NAND_MAX_OOBSIZE];
-	uint8_t	ecccode[NAND_MAX_OOBSIZE];
-	uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+	uint8_t	*ecccalc;
+	uint8_t	*ecccode;
+	uint8_t *databuf;
 };
 
 /**
-- 
1.8.3.1

[PATCH 3/4] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Brian Norris]

On Sat, Dec 21, 2013 at 12:02:29AM +0800, Huang Shijie wrote:
> The patch converts the arrays to buffer pointers for nand_buffers{}.
> 
> The cafe_nand.c is the only NAND_OWN_BUFFERS user which allocates
> a nand_buffers{} itself.
> 
> This patch disables the DMA for nand_scan_ident, and restore the DMA
> status after we finish the nand_scan_ident. By this way, we can get the
> mtd->writesize and mtd->oobsize, and then allocates the cafe->dmabuf
> with them.
> 
> Since the cafe_nand.c uses the NAND_ECC_HW_SYNDROME ECC mode, we do not
> allocate the buffers for @ecccalc and @ecccode.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>
> ---
>  drivers/mtd/nand/cafe_nand.c | 49 ++++++++++++++++++++++++++++++++------------
>  drivers/mtd/nand/nand_base.c | 19 +++++++++++++----
>  include/linux/mtd/nand.h     | 12 +++++------
>  3 files changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
> index c34985a..e9186e7 100644
> --- a/drivers/mtd/nand/cafe_nand.c
> +++ b/drivers/mtd/nand/cafe_nand.c
> @@ -627,6 +627,8 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  	struct cafe_priv *cafe;
>  	uint32_t ctrl;
>  	int err = 0;
> +	int old_dma;
> +	struct nand_buffers *nbuf;
>  
>  	/* Very old versions shared the same PCI ident for all three
>  	   functions on the chip. Verify the class too... */
> @@ -657,13 +659,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  		err = -ENOMEM;
>  		goto out_free_mtd;
>  	}
> -	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112 + sizeof(struct nand_buffers),
> -					  &cafe->dmaaddr, GFP_KERNEL);
> -	if (!cafe->dmabuf) {
> -		err = -ENOMEM;
> -		goto out_ior;
> -	}
> -	cafe->nand.buffers = (void *)cafe->dmabuf + 2112;

When you move this after nand_scan_ident(), you forgot to move all the code
that uses dmabuf and dmabuf:

        /* Set up DMA address */
        cafe_writel(cafe, cafe->dmaaddr & 0xffffffff, NAND_DMA_ADDR0);
        if (sizeof(cafe->dmaaddr) > 4)
                /* Shift in two parts to shut the compiler up */
                cafe_writel(cafe, (cafe->dmaaddr >> 16) >> 16, NAND_DMA_ADDR1);
        else
                cafe_writel(cafe, 0, 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);

This code needs to stay after the point where you actually allocate the buffer.

It could also use some testing on real Cafe NAND hardware, since I don't
know what kind of use the !DMA case was getting.

>  
>  	cafe->rs = init_rs_non_canonical(12, &cafe_mul, 0, 1, 8);
>  	if (!cafe->rs) {

Brian

[PATCH 3/4] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Russell King - ARM Linux]

On Sat, Jan 11, 2014 at 03:39:37PM -0800, Brian Norris wrote:
> When you move this after nand_scan_ident(), you forgot to move all the code
> that uses dmabuf and dmabuf:
> 
>         /* Set up DMA address */
>         cafe_writel(cafe, cafe->dmaaddr & 0xffffffff, NAND_DMA_ADDR0);
>         if (sizeof(cafe->dmaaddr) > 4)
>                 /* Shift in two parts to shut the compiler up */
>                 cafe_writel(cafe, (cafe->dmaaddr >> 16) >> 16, NAND_DMA_ADDR1);
>         else
>                 cafe_writel(cafe, 0, 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);
> 
> This code needs to stay after the point where you actually allocate the buffer.

You may wish to evaluate generated code differences between the above and
implementing it like this:

	u64 dmaaddr = cafe->dmaaddr;

	cafe_writel(cafe, dmaaddr, NAND_DMA_ADDR0);
	cafe_writel(cafe, dmaaddr >> 32, NAMD_DMA_ADDR1);

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[PATCH 3/4] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Brian Norris]

On Sat, Jan 11, 2014 at 11:43:53PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 11, 2014 at 03:39:37PM -0800, Brian Norris wrote:
> > When you move this after nand_scan_ident(), you forgot to move all the code
> > that uses dmabuf and dmabuf:
> > 
> >         /* Set up DMA address */
> >         cafe_writel(cafe, cafe->dmaaddr & 0xffffffff, NAND_DMA_ADDR0);
> >         if (sizeof(cafe->dmaaddr) > 4)
> >                 /* Shift in two parts to shut the compiler up */
> >                 cafe_writel(cafe, (cafe->dmaaddr >> 16) >> 16, NAND_DMA_ADDR1);
> >         else
> >                 cafe_writel(cafe, 0, 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);
> > 
> > This code needs to stay after the point where you actually allocate the buffer.
> 
> You may wish to evaluate generated code differences between the above and
> implementing it like this:
> 
> 	u64 dmaaddr = cafe->dmaaddr;
> 
> 	cafe_writel(cafe, dmaaddr, NAND_DMA_ADDR0);
> 	cafe_writel(cafe, dmaaddr >> 32, NAMD_DMA_ADDR1);

Well, that's not the primary concern of this patch, but that looks good.
Thanks for the idea. I'll take a look sometime.

Brian

[PATCH fix] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Huang Shijie]

The patch converts the arrays to buffer pointers for nand_buffers{}.

The cafe_nand.c is the only NAND_OWN_BUFFERS user which allocates
a nand_buffers{} itself.

This patch disables the DMA for nand_scan_ident, and restore the DMA
status after we finish the nand_scan_ident. By this way, we can get the
mtd->writesize and mtd->oobsize, and then allocates the cafe->dmabuf
with them.

Since the cafe_nand.c uses the NAND_ECC_HW_SYNDROME ECC mode, we do not
allocate the buffers for @ecccalc and @ecccode.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---

fix: Setup the DMA address after we have allocated the DMA buffer.

---
 drivers/mtd/nand/cafe_nand.c |   68 ++++++++++++++++++++++++++++-------------
 drivers/mtd/nand/nand_base.c |   19 +++++++++--
 include/linux/mtd/nand.h     |   12 ++++----
 3 files changed, 67 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index f2f64ad..4e66726 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -627,6 +627,8 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	struct cafe_priv *cafe;
 	uint32_t ctrl;
 	int err = 0;
+	int old_dma;
+	struct nand_buffers *nbuf;
 
 	/* Very old versions shared the same PCI ident for all three
 	   functions on the chip. Verify the class too... */
@@ -655,13 +657,6 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 		err = -ENOMEM;
 		goto out_free_mtd;
 	}
-	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112 + sizeof(struct nand_buffers),
-					  &cafe->dmaaddr, GFP_KERNEL);
-	if (!cafe->dmabuf) {
-		err = -ENOMEM;
-		goto out_ior;
-	}
-	cafe->nand.buffers = (void *)cafe->dmabuf + 2112;
 
 	cafe->rs = init_rs_non_canonical(12, &cafe_mul, 0, 1, 8);
 	if (!cafe->rs) {
@@ -721,7 +716,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 			  "CAFE NAND", mtd);
 	if (err) {
 		dev_warn(&pdev->dev, "Could not register IRQ %d\n", pdev->irq);
-		goto out_free_dma;
+		goto out_ior;
 	}
 
 	/* Disable master reset, enable NAND clock */
@@ -735,6 +730,32 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	cafe_writel(cafe, 0x7006, GLOBAL_CTRL);
 	cafe_writel(cafe, 0x700a, GLOBAL_CTRL);
 
+	/* Enable NAND IRQ in global IRQ mask register */
+	cafe_writel(cafe, 0x80000007, GLOBAL_IRQ_MASK);
+	cafe_dev_dbg(&cafe->pdev->dev, "Control %x, IRQ mask %x\n",
+		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;
+
+	/* Scan to find existence of the device */
+	if (nand_scan_ident(mtd, 2, NULL)) {
+		err = -ENXIO;
+		goto out_irq;
+	}
+
+	cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev,
+				2112 + sizeof(struct nand_buffers) +
+				mtd->writesize + mtd->oobsize,
+				&cafe->dmaaddr, GFP_KERNEL);
+	if (!cafe->dmabuf) {
+		err = -ENOMEM;
+		goto out_irq;
+	}
+	cafe->nand.buffers = nbuf = (void *)cafe->dmabuf + 2112;
+
 	/* Set up DMA address */
 	cafe_writel(cafe, cafe->dmaaddr & 0xffffffff, NAND_DMA_ADDR0);
 	if (sizeof(cafe->dmaaddr) > 4)
@@ -746,16 +767,13 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	cafe_dev_dbg(&cafe->pdev->dev, "Set DMA address to %x (virt %p)\n",
 		cafe_readl(cafe, NAND_DMA_ADDR0), cafe->dmabuf);
 
-	/* Enable NAND IRQ in global IRQ mask register */
-	cafe_writel(cafe, 0x80000007, GLOBAL_IRQ_MASK);
-	cafe_dev_dbg(&cafe->pdev->dev, "Control %x, IRQ mask %x\n",
-		cafe_readl(cafe, GLOBAL_CTRL), cafe_readl(cafe, GLOBAL_IRQ_MASK));
+	/* this driver does not need the @ecccalc and @ecccode */
+	nbuf->ecccalc = NULL;
+	nbuf->ecccode = NULL;
+	nbuf->databuf = (uint8_t *)(nbuf + 1);
 
-	/* Scan to find existence of the device */
-	if (nand_scan_ident(mtd, 2, NULL)) {
-		err = -ENXIO;
-		goto out_irq;
-	}
+	/* Restore the DMA flag */
+	usedma = old_dma;
 
 	cafe->ctl2 = 1<<27; /* Reed-Solomon ECC */
 	if (mtd->writesize == 2048)
@@ -773,7 +791,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	} else {
 		printk(KERN_WARNING "Unexpected NAND flash writesize %d. Aborting\n",
 		       mtd->writesize);
-		goto out_irq;
+		goto out_free_dma;
 	}
 	cafe->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
 	cafe->nand.ecc.size = mtd->writesize;
@@ -790,7 +808,7 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 
 	err = nand_scan_tail(mtd);
 	if (err)
-		goto out_irq;
+		goto out_free_dma;
 
 	pci_set_drvdata(pdev, mtd);
 
@@ -799,12 +817,15 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 
 	goto out;
 
+ out_free_dma:
+	dma_free_coherent(&cafe->pdev->dev,
+			2112 + sizeof(struct nand_buffers) +
+			mtd->writesize + mtd->oobsize,
+			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);
 	free_irq(pdev->irq, mtd);
- out_free_dma:
-	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
  out_ior:
 	pci_iounmap(pdev, cafe->mmio);
  out_free_mtd:
@@ -824,7 +845,10 @@ static void cafe_nand_remove(struct pci_dev *pdev)
 	nand_release(mtd);
 	free_rs(cafe->rs);
 	pci_iounmap(pdev, cafe->mmio);
-	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
+	dma_free_coherent(&cafe->pdev->dev,
+			2112 + sizeof(struct nand_buffers) +
+			mtd->writesize + mtd->oobsize,
+			cafe->dmabuf, cafe->dmaaddr);
 	kfree(mtd);
 }
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index dd22cc4..21e1746 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3733,15 +3733,26 @@ int nand_scan_tail(struct mtd_info *mtd)
 	int i;
 	struct nand_chip *chip = mtd->priv;
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_buffers *nbuf;
 
 	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
 	BUG_ON((chip->bbt_options & NAND_BBT_NO_OOB_BBM) &&
 			!(chip->bbt_options & NAND_BBT_USE_FLASH));
 
-	if (!(chip->options & NAND_OWN_BUFFERS))
-		chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
-	if (!chip->buffers)
-		return -ENOMEM;
+	if (!(chip->options & NAND_OWN_BUFFERS)) {
+		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+				+ mtd->oobsize * 3, GFP_KERNEL);
+		if (!nbuf)
+			return -ENOMEM;
+		nbuf->ecccalc = (uint8_t *)(nbuf + 1);
+		nbuf->ecccode = nbuf->ecccalc + mtd->oobsize;
+		nbuf->databuf = nbuf->ecccode + mtd->oobsize;
+
+		chip->buffers = nbuf;
+	} else {
+		if (!chip->buffers)
+			return -ENOMEM;
+	}
 
 	/* Set the internal oob buffer location, just after the page data */
 	chip->oob_poi = chip->buffers->databuf + mtd->writesize;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index d06a56a..773bd93 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -481,17 +481,17 @@ struct nand_ecc_ctrl {
 
 /**
  * struct nand_buffers - buffer structure for read/write
- * @ecccalc:	buffer for calculated ECC
- * @ecccode:	buffer for ECC read from flash
- * @databuf:	buffer for data - dynamically sized
+ * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
+ * @ecccode:	buffer pointer for ECC read from flash, size is oobsize.
+ * @databuf:	buffer pointer for data, size is (page size + oobsize).
  *
  * Do not change the order of buffers. databuf and oobrbuf must be in
  * consecutive order.
  */
 struct nand_buffers {
-	uint8_t	ecccalc[NAND_MAX_OOBSIZE];
-	uint8_t	ecccode[NAND_MAX_OOBSIZE];
-	uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+	uint8_t	*ecccalc;
+	uint8_t	*ecccode;
+	uint8_t *databuf;
 };
 
 /**
-- 
1.7.2.rc3

[PATCH fix] mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for nand_buffers{}

[Brian Norris]

On Mon, Jan 13, 2014 at 02:27:12PM +0800, Huang Shijie wrote:
> The patch converts the arrays to buffer pointers for nand_buffers{}.
> 
> The cafe_nand.c is the only NAND_OWN_BUFFERS user which allocates
> a nand_buffers{} itself.
> 
> This patch disables the DMA for nand_scan_ident, and restore the DMA
> status after we finish the nand_scan_ident. By this way, we can get the
> mtd->writesize and mtd->oobsize, and then allocates the cafe->dmabuf
> with them.
> 
> Since the cafe_nand.c uses the NAND_ECC_HW_SYNDROME ECC mode, we do not
> allocate the buffers for @ecccalc and @ecccode.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
> 
> fix: Setup the DMA address after we have allocated the DMA buffer.

Thanks, looks OK. Pushed to l2-mtd.git/next.

Brian

[PATCH 4/4] mtd: nand: remove the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Huang Shijie]

There is no reference to these two macros now.
Just remove them.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 include/linux/mtd/nand.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 4bbcaba..863609e 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -52,14 +52,6 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
 #define NAND_MAX_CHIPS		8
 
 /*
- * This constant declares the max. oobsize / page, which
- * is supported now. If you add a chip with bigger oobsize/page
- * adjust this accordingly.
- */
-#define NAND_MAX_OOBSIZE	744
-#define NAND_MAX_PAGESIZE	8192
-
-/*
  * Constants for hardware specific CLE/ALE/NCE function
  *
  * These are bits which can be or'ed to set/clear multiple
-- 
1.8.3.1

[PATCH 4/4] mtd: nand: remove the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Brian Norris]

On Sat, Dec 21, 2013 at 12:02:30AM +0800, Huang Shijie wrote:
> There is no reference to these two macros now.
> Just remove them.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>

Now that patch 3 is fixed, I've pushed this patch as well. Thanks for
cleaning this up!

Brian

[PATCH 0/4] mtd: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Josh Triplett]

On Sat, Dec 21, 2013 at 12:02:26AM +0800, Huang Shijie wrote:
> [1] Why kill them?
>     As time goes on, the NAND's page size and oob size become larger and larger.
>     So we have changes these two macros frequently in order to support the new
>     NAND.
> 
> [2] How does this patch set do?
>     There are three drivers which uses these two macros, the mxc-nand, denali,
>     and the cafe-nand.
> 
>     This patch will allocate a temporary buffer for the nand_scan_ident, and 
>     after it have getten the right page size and oob size, it will re-allocate
>     the buffer again.
> 
> [3] I do not have the boards which have the relative NAND controller.
> 
>     I hope someone can test this patch set.
> 
>     thanks.
> 
> 
> Huang Shijie (4):
>   mtd: mxc-nand: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
>   mtd: denali: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
>   mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for
>     nand_buffers{}
>   mtd: nand: remove the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

For all four patches:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

[PATCH 0/4] mtd: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

[Brian Norris]

On Sat, Dec 21, 2013 at 12:02:26AM +0800, Huang Shijie wrote:
> [1] Why kill them?
>     As time goes on, the NAND's page size and oob size become larger and larger.
>     So we have changes these two macros frequently in order to support the new
>     NAND.
> 
> [2] How does this patch set do?
>     There are three drivers which uses these two macros, the mxc-nand, denali,
>     and the cafe-nand.
> 
>     This patch will allocate a temporary buffer for the nand_scan_ident, and 
>     after it have getten the right page size and oob size, it will re-allocate
>     the buffer again.
> 
> [3] I do not have the boards which have the relative NAND controller.
> 
>     I hope someone can test this patch set.
> 
>     thanks.
> 
> 
> Huang Shijie (4):
>   mtd: mxc-nand: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
>   mtd: denali: kill the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE
>   mtd: nand: kill the the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE for
>     nand_buffers{}
>   mtd: nand: remove the NAND_MAX_PAGESIZE/NAND_MAX_OOBSIZE

Pushed only the first 2 patches. Patch 3 still has some issues.

Thanks,
Brian