[PATCH 0/3] Sun8i NAND DMA support

[PATCH 0/3] Sun8i NAND DMA support

[Miquel Raynal]

Hello,

This series improves the NAND throughput on sun8i platforms by adding
DMA support. This increase comes from the ECC pipelining feature.

Because NAND DMA handling has changed between A10+ and A33 SoCs, we
must introduce a new compatible. DT is updated with this new
compatible in addition with the usual DMA properties.

Tested on an A33-Olinuxino.

Thanks,
Miquèl


Miquel Raynal (3):
  dt-bindings: mtd: sunxi: Add new compatible
  mtd: rawnand: sunxi: Add DMA support for sun8i
  ARM: dts: sunxi: Improve sun8i NAND transfers by using DMA

 .../devicetree/bindings/mtd/sunxi-nand.txt    |  7 +-
 arch/arm/boot/dts/sun8i-a23-a33.dtsi          |  4 +-
 drivers/mtd/nand/raw/sunxi_nand.c             | 75 +++++++++++++++++--
 3 files changed, 77 insertions(+), 9 deletions(-)

-- 
2.19.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/3] dt-bindings: mtd: sunxi: Add new compatible

[Miquel Raynal]

The A33 NAND controller is slightly different than the A10+ ones,
eg. DMA handling is a bit different and a few register offsets
changed.

Introduce a new compatible to represent this version of the IP.

Also append '-controller' to the new compatible (which is required for
new compatibles) as this is describing a NAND controller and not a
NAND chip.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
index dcd5a5d80dc0..6128d41d8c59 100644
--- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
@@ -1,7 +1,12 @@
 Allwinner NAND Flash Controller (NFC)
 
 Required properties:
-- compatible : "allwinner,sun4i-a10-nand".
+- compatible : Must be one of:
+	       - "allwinner,sun4i-a10-nand"
+	       - "allwinner,sun8i-a33-nand-controller"
+	       The former may be used by all IPs, however sun8i family
+	       will need the second one in order to make use of the
+	       internal DMA capabilities.
 - reg : shall contain registers location and length for data and reg.
 - interrupts : shall define the nand controller interrupt.
 - #address-cells: shall be set to 1. Encode the nand CS.
-- 
2.19.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] dt-bindings: mtd: sunxi: Add new compatible

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1667 bytes --]

Hi,

On Thu, Apr 04, 2019 at 06:21:09PM +0200, Miquel Raynal wrote:
> The A33 NAND controller is slightly different than the A10+ ones,
> eg. DMA handling is a bit different and a few register offsets
> changed.
>
> Introduce a new compatible to represent this version of the IP.
>
> Also append '-controller' to the new compatible (which is required for
> new compatibles) as this is describing a NAND controller and not a
> NAND chip.

Out of curiosity, why are you requiring that suffix now?

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> index dcd5a5d80dc0..6128d41d8c59 100644
> --- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> @@ -1,7 +1,12 @@
>  Allwinner NAND Flash Controller (NFC)
>
>  Required properties:
> -- compatible : "allwinner,sun4i-a10-nand".
> +- compatible : Must be one of:
> +	       - "allwinner,sun4i-a10-nand"
> +	       - "allwinner,sun8i-a33-nand-controller"
> +	       The former may be used by all IPs, however sun8i family
> +	       will need the second one in order to make use of the
> +	       internal DMA capabilities.

I'm not sure we should have that statement. We have no idea whether or
not this can be used by all IPs *today*, and we surely don't know
about the one that are going to come out.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] dt-bindings: mtd: sunxi: Add new compatible

[Miquel Raynal]

[-- Attachment #1.1: Type: text/plain, Size: 2163 bytes --]

Hi Maxime,

Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
11:13:02 +0200:

> Hi,
> 
> On Thu, Apr 04, 2019 at 06:21:09PM +0200, Miquel Raynal wrote:
> > The A33 NAND controller is slightly different than the A10+ ones,
> > eg. DMA handling is a bit different and a few register offsets
> > changed.
> >
> > Introduce a new compatible to represent this version of the IP.
> >
> > Also append '-controller' to the new compatible (which is required for
> > new compatibles) as this is describing a NAND controller and not a
> > NAND chip.  
> 
> Out of curiosity, why are you requiring that suffix now?

Because people are confused with the terminology and we see people
mixing all the terms very often: NAND controller, NAND bus, NAND
chip, ECC engine, etc. This node only describes a NAND controller, so
let be more precise and stop naming everything just "NAND".

> 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > index dcd5a5d80dc0..6128d41d8c59 100644
> > --- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > @@ -1,7 +1,12 @@
> >  Allwinner NAND Flash Controller (NFC)
> >
> >  Required properties:
> > -- compatible : "allwinner,sun4i-a10-nand".
> > +- compatible : Must be one of:
> > +	       - "allwinner,sun4i-a10-nand"
> > +	       - "allwinner,sun8i-a33-nand-controller"
> > +	       The former may be used by all IPs, however sun8i family
> > +	       will need the second one in order to make use of the
> > +	       internal DMA capabilities.  
> 
> I'm not sure we should have that statement. We have no idea whether or
> not this can be used by all IPs *today*, and we surely don't know
> about the one that are going to come out.

Shall I just drop the whole comment ("The former" ... "capabilities.")?


Thanks,
Miquèl

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] dt-bindings: mtd: sunxi: Add new compatible

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2371 bytes --]

On Fri, Apr 05, 2019 at 11:28:13AM +0200, Miquel Raynal wrote:
> Hi Maxime,
>
> Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
> 11:13:02 +0200:
>
> > Hi,
> >
> > On Thu, Apr 04, 2019 at 06:21:09PM +0200, Miquel Raynal wrote:
> > > The A33 NAND controller is slightly different than the A10+ ones,
> > > eg. DMA handling is a bit different and a few register offsets
> > > changed.
> > >
> > > Introduce a new compatible to represent this version of the IP.
> > >
> > > Also append '-controller' to the new compatible (which is required for
> > > new compatibles) as this is describing a NAND controller and not a
> > > NAND chip.
> >
> > Out of curiosity, why are you requiring that suffix now?
>
> Because people are confused with the terminology and we see people
> mixing all the terms very often: NAND controller, NAND bus, NAND
> chip, ECC engine, etc. This node only describes a NAND controller, so
> let be more precise and stop naming everything just "NAND".

Sounds reasonable :)

> >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > > index dcd5a5d80dc0..6128d41d8c59 100644
> > > --- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > > +++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
> > > @@ -1,7 +1,12 @@
> > >  Allwinner NAND Flash Controller (NFC)
> > >
> > >  Required properties:
> > > -- compatible : "allwinner,sun4i-a10-nand".
> > > +- compatible : Must be one of:
> > > +	       - "allwinner,sun4i-a10-nand"
> > > +	       - "allwinner,sun8i-a33-nand-controller"
> > > +	       The former may be used by all IPs, however sun8i family
> > > +	       will need the second one in order to make use of the
> > > +	       internal DMA capabilities.
> >
> > I'm not sure we should have that statement. We have no idea whether or
> > not this can be used by all IPs *today*, and we surely don't know
> > about the one that are going to come out.
>
> Shall I just drop the whole comment ("The former" ... "capabilities.")?

Yep.

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Miquel Raynal]

Allwinner NAND controllers can make use of DMA to enhance the I/O
throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
is a bit different than with the older SoCs, hence the introduction of
a new compatible to handle:
* the differences between register offsets,
* the burst length change from 4 to minimum 8,
* drive SRAM accesses through the AHB bus instead of the MBUS.

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

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 4282bc477761..49cd5067adaa 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -42,7 +42,8 @@
 #define NFC_REG_CMD		0x0024
 #define NFC_REG_RCMD_SET	0x0028
 #define NFC_REG_WCMD_SET	0x002C
-#define NFC_REG_IO_DATA		0x0030
+#define NFC_REG_A10_IO_DATA	0x0030
+#define NFC_REG_A33_IO_DATA	0x0300
 #define NFC_REG_ECC_CTL		0x0034
 #define NFC_REG_ECC_ST		0x0038
 #define NFC_REG_DEBUG		0x003C
@@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
 	return container_of(nand, struct sunxi_nand_chip, nand);
 }
 
+/*
+ * NAND Controller capabilities structure: stores NAND controller capabilities
+ * for distinction between compatible strings.
+ *
+ * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
+ *                      instead of MBUS (less configuration). A10+ use the MBUS
+ *                      but no extra configuration is needed.
+ * @reg_io_data:	I/O data register
+ * @dma_maxburst:	DMA maxburst
+ */
+struct sunxi_nfc_caps {
+	bool sram_through_ahb;
+	unsigned int reg_io_data;
+	unsigned int dma_maxburst;
+};
+
 /**
  * struct sunxi_nfc - stores sunxi NAND controller information
  *
@@ -228,6 +245,7 @@ struct sunxi_nfc {
 	struct list_head chips;
 	struct completion complete;
 	struct dma_chan *dmac;
+	const struct sunxi_nfc_caps *caps;
 };
 
 static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl)
@@ -350,10 +368,29 @@ static int sunxi_nfc_dma_op_prepare(struct sunxi_nfc *nfc, const void *buf,
 		goto err_unmap_buf;
 	}
 
-	writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
-	       nfc->regs + NFC_REG_CTL);
+	/*
+	 * On A33, we suppose the "internal RAM" (p.12 of the user manual)
+	 * refers to the NAND controller's internal SRAM. This memory is mapped
+	 * and so is accessible from the AHB. It seems that it can also be
+	 * accessed by the MBUS. MBUS accesses are mandatory when using the
+	 * internal DMA instead of the external DMA engine.
+	 *
+	 * During DMA I/O operation, either we access this memory from the AHB
+	 * by clearing the NFC_RAM_METHOD bit, or we set the bit and use the
+	 * MBUS. In this case, we should also configure the MBUS DMA length
+	 * NFC_REG_MDMA_CNT(0xC4) to be chunksize * nchunks. NAND I/O over MBUS
+	 * are also limited to 32kiB pages.
+	 */
+	if (nfc->caps->sram_through_ahb)
+		writel(readl(nfc->regs + NFC_REG_CTL) & ~NFC_RAM_METHOD,
+		       nfc->regs + NFC_REG_CTL);
+	else
+		writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
+		       nfc->regs + NFC_REG_CTL);
+
 	writel(nchunks, nfc->regs + NFC_REG_SECTOR_NUM);
 	writel(chunksize, nfc->regs + NFC_REG_CNT);
+
 	dmat = dmaengine_submit(dmad);
 
 	ret = dma_submit_error(dmat);
@@ -2088,6 +2125,12 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
 		goto out_mod_clk_unprepare;
 	}
 
+	nfc->caps = of_device_get_match_data(&pdev->dev);
+	if (!nfc->caps) {
+		ret = -EINVAL;
+		goto out_ahb_reset_reassert;
+	}
+
 	ret = sunxi_nfc_rst(nfc);
 	if (ret)
 		goto out_ahb_reset_reassert;
@@ -2102,12 +2145,12 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
 	if (nfc->dmac) {
 		struct dma_slave_config dmac_cfg = { };
 
-		dmac_cfg.src_addr = r->start + NFC_REG_IO_DATA;
+		dmac_cfg.src_addr = r->start + nfc->caps->reg_io_data;
 		dmac_cfg.dst_addr = dmac_cfg.src_addr;
 		dmac_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 		dmac_cfg.dst_addr_width = dmac_cfg.src_addr_width;
-		dmac_cfg.src_maxburst = 4;
-		dmac_cfg.dst_maxburst = 4;
+		dmac_cfg.src_maxburst = nfc->caps->dma_maxburst;
+		dmac_cfg.dst_maxburst = nfc->caps->dma_maxburst;
 		dmaengine_slave_config(nfc->dmac, &dmac_cfg);
 	} else {
 		dev_warn(dev, "failed to request rxtx DMA channel\n");
@@ -2152,8 +2195,26 @@ static int sunxi_nfc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
+	.reg_io_data = NFC_REG_A10_IO_DATA,
+	.dma_maxburst = 4,
+};
+
+static const struct sunxi_nfc_caps sunxi_nfc_a33_caps = {
+	.sram_through_ahb = true,
+	.reg_io_data = NFC_REG_A33_IO_DATA,
+	.dma_maxburst = 8,
+};
+
 static const struct of_device_id sunxi_nfc_ids[] = {
-	{ .compatible = "allwinner,sun4i-a10-nand" },
+	{
+		.compatible = "allwinner,sun4i-a10-nand",
+		.data = &sunxi_nfc_a10_caps,
+	},
+	{
+		.compatible = "allwinner,sun8i-a33-nand-controller",
+		.data = &sunxi_nfc_a33_caps,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sunxi_nfc_ids);
-- 
2.19.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2884 bytes --]

On Thu, Apr 04, 2019 at 06:21:10PM +0200, Miquel Raynal wrote:
> Allwinner NAND controllers can make use of DMA to enhance the I/O
> throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> is a bit different than with the older SoCs, hence the introduction of
> a new compatible to handle:
> * the differences between register offsets,
> * the burst length change from 4 to minimum 8,
> * drive SRAM accesses through the AHB bus instead of the MBUS.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
>  1 file changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 4282bc477761..49cd5067adaa 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -42,7 +42,8 @@
>  #define NFC_REG_CMD		0x0024
>  #define NFC_REG_RCMD_SET	0x0028
>  #define NFC_REG_WCMD_SET	0x002C
> -#define NFC_REG_IO_DATA		0x0030
> +#define NFC_REG_A10_IO_DATA	0x0030
> +#define NFC_REG_A33_IO_DATA	0x0300
>  #define NFC_REG_ECC_CTL		0x0034
>  #define NFC_REG_ECC_ST		0x0038
>  #define NFC_REG_DEBUG		0x003C
> @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
>  	return container_of(nand, struct sunxi_nand_chip, nand);
>  }
>
> +/*
> + * NAND Controller capabilities structure: stores NAND controller capabilities
> + * for distinction between compatible strings.
> + *
> + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> + *                      instead of MBUS (less configuration). A10+ use the MBUS

What do you mean by A10+ ?

> + *                      but no extra configuration is needed.
> + * @reg_io_data:	I/O data register
> + * @dma_maxburst:	DMA maxburst
> + */
> +struct sunxi_nfc_caps {
> +	bool sram_through_ahb;
> +	unsigned int reg_io_data;
> +	unsigned int dma_maxburst;
> +};

Ideally, the introduction of that structure and the introduction of
the A33 support should be separate patches.

>  /**
>   * struct sunxi_nfc - stores sunxi NAND controller information
>   *
> @@ -228,6 +245,7 @@ struct sunxi_nfc {
>  	struct list_head chips;
>  	struct completion complete;
>  	struct dma_chan *dmac;
> +	const struct sunxi_nfc_caps *caps;
>  };
>
>  static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl)
> @@ -350,10 +368,29 @@ static int sunxi_nfc_dma_op_prepare(struct sunxi_nfc *nfc, const void *buf,
>  		goto err_unmap_buf;
>  	}
>
> -	writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
> -	       nfc->regs + NFC_REG_CTL);
> +	/*
> +	 * On A33, we suppose the "internal RAM" (p.12 of the user manual)

Which user manual? It certainly isn't the A33 user manual :)

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Miquel Raynal]

[-- Attachment #1.1: Type: text/plain, Size: 3411 bytes --]

Hi Maxime,

Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
11:16:07 +0200:

> On Thu, Apr 04, 2019 at 06:21:10PM +0200, Miquel Raynal wrote:
> > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > is a bit different than with the older SoCs, hence the introduction of
> > a new compatible to handle:
> > * the differences between register offsets,
> > * the burst length change from 4 to minimum 8,
> > * drive SRAM accesses through the AHB bus instead of the MBUS.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
> >  1 file changed, 68 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > index 4282bc477761..49cd5067adaa 100644
> > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > @@ -42,7 +42,8 @@
> >  #define NFC_REG_CMD		0x0024
> >  #define NFC_REG_RCMD_SET	0x0028
> >  #define NFC_REG_WCMD_SET	0x002C
> > -#define NFC_REG_IO_DATA		0x0030
> > +#define NFC_REG_A10_IO_DATA	0x0030
> > +#define NFC_REG_A33_IO_DATA	0x0300
> >  #define NFC_REG_ECC_CTL		0x0034
> >  #define NFC_REG_ECC_ST		0x0038
> >  #define NFC_REG_DEBUG		0x003C
> > @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
> >  	return container_of(nand, struct sunxi_nand_chip, nand);
> >  }
> >
> > +/*
> > + * NAND Controller capabilities structure: stores NAND controller capabilities
> > + * for distinction between compatible strings.
> > + *
> > + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> > + *                      instead of MBUS (less configuration). A10+ use the MBUS  
> 
> What do you mean by A10+ ?

I meant A1x, A2x SoCs. Not sure it matches a product line for you, so
please suggest something to mean "SoCs which are not A33" (so far I
think all worked without this).

> 
> > + *                      but no extra configuration is needed.
> > + * @reg_io_data:	I/O data register
> > + * @dma_maxburst:	DMA maxburst
> > + */
> > +struct sunxi_nfc_caps {
> > +	bool sram_through_ahb;
> > +	unsigned int reg_io_data;
> > +	unsigned int dma_maxburst;
> > +};  
> 
> Ideally, the introduction of that structure and the introduction of
> the A33 support should be separate patches.

Sure, I can split it up.

> 
> >  /**
> >   * struct sunxi_nfc - stores sunxi NAND controller information
> >   *
> > @@ -228,6 +245,7 @@ struct sunxi_nfc {
> >  	struct list_head chips;
> >  	struct completion complete;
> >  	struct dma_chan *dmac;
> > +	const struct sunxi_nfc_caps *caps;
> >  };
> >
> >  static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl)
> > @@ -350,10 +368,29 @@ static int sunxi_nfc_dma_op_prepare(struct sunxi_nfc *nfc, const void *buf,
> >  		goto err_unmap_buf;
> >  	}
> >
> > -	writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
> > -	       nfc->regs + NFC_REG_CTL);
> > +	/*
> > +	 * On A33, we suppose the "internal RAM" (p.12 of the user manual)  
> 
> Which user manual? It certainly isn't the A33 user manual :)

You are right it is the A33 NAND flash controller spec.


Thanks,
Miquèl

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2400 bytes --]

On Fri, Apr 05, 2019 at 11:37:42AM +0200, Miquel Raynal wrote:
> Hi Maxime,
>
> Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
> 11:16:07 +0200:
>
> > On Thu, Apr 04, 2019 at 06:21:10PM +0200, Miquel Raynal wrote:
> > > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > > is a bit different than with the older SoCs, hence the introduction of
> > > a new compatible to handle:
> > > * the differences between register offsets,
> > > * the burst length change from 4 to minimum 8,
> > > * drive SRAM accesses through the AHB bus instead of the MBUS.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
> > >  1 file changed, 68 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > > index 4282bc477761..49cd5067adaa 100644
> > > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > > @@ -42,7 +42,8 @@
> > >  #define NFC_REG_CMD		0x0024
> > >  #define NFC_REG_RCMD_SET	0x0028
> > >  #define NFC_REG_WCMD_SET	0x002C
> > > -#define NFC_REG_IO_DATA		0x0030
> > > +#define NFC_REG_A10_IO_DATA	0x0030
> > > +#define NFC_REG_A33_IO_DATA	0x0300
> > >  #define NFC_REG_ECC_CTL		0x0034
> > >  #define NFC_REG_ECC_ST		0x0038
> > >  #define NFC_REG_DEBUG		0x003C
> > > @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
> > >  	return container_of(nand, struct sunxi_nand_chip, nand);
> > >  }
> > >
> > > +/*
> > > + * NAND Controller capabilities structure: stores NAND controller capabilities
> > > + * for distinction between compatible strings.
> > > + *
> > > + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> > > + *                      instead of MBUS (less configuration). A10+ use the MBUS
> >
> > What do you mean by A10+ ?
>
> I meant A1x, A2x SoCs. Not sure it matches a product line for you, so
> please suggest something to mean "SoCs which are not A33" (so far I
> think all worked without this).

The list is pretty small, so we can just name them. That would be the
A10, A10s A13 and A20.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Miquel Raynal]

[-- Attachment #1.1: Type: text/plain, Size: 2679 bytes --]

Hi Maxime,

Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
12:55:59 +0200:

> On Fri, Apr 05, 2019 at 11:37:42AM +0200, Miquel Raynal wrote:
> > Hi Maxime,
> >
> > Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
> > 11:16:07 +0200:
> >  
> > > On Thu, Apr 04, 2019 at 06:21:10PM +0200, Miquel Raynal wrote:  
> > > > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > > > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > > > is a bit different than with the older SoCs, hence the introduction of
> > > > a new compatible to handle:
> > > > * the differences between register offsets,
> > > > * the burst length change from 4 to minimum 8,
> > > > * drive SRAM accesses through the AHB bus instead of the MBUS.
> > > >
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
> > > >  1 file changed, 68 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > > > index 4282bc477761..49cd5067adaa 100644
> > > > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > > > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > > > @@ -42,7 +42,8 @@
> > > >  #define NFC_REG_CMD		0x0024
> > > >  #define NFC_REG_RCMD_SET	0x0028
> > > >  #define NFC_REG_WCMD_SET	0x002C
> > > > -#define NFC_REG_IO_DATA		0x0030
> > > > +#define NFC_REG_A10_IO_DATA	0x0030
> > > > +#define NFC_REG_A33_IO_DATA	0x0300
> > > >  #define NFC_REG_ECC_CTL		0x0034
> > > >  #define NFC_REG_ECC_ST		0x0038
> > > >  #define NFC_REG_DEBUG		0x003C
> > > > @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
> > > >  	return container_of(nand, struct sunxi_nand_chip, nand);
> > > >  }
> > > >
> > > > +/*
> > > > + * NAND Controller capabilities structure: stores NAND controller capabilities
> > > > + * for distinction between compatible strings.
> > > > + *
> > > > + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> > > > + *                      instead of MBUS (less configuration). A10+ use the MBUS  
> > >
> > > What do you mean by A10+ ?  
> >
> > I meant A1x, A2x SoCs. Not sure it matches a product line for you, so
> > please suggest something to mean "SoCs which are not A33" (so far I
> > think all worked without this).  
> 
> The list is pretty small, so we can just name them. That would be the
> A10, A10s A13 and A20.

You really need a "A10+"-like acronym for these ;)

Ok, I'll add the list.


Thanks,
Miquèl

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3005 bytes --]

On Fri, Apr 05, 2019 at 02:25:54PM +0200, Miquel Raynal wrote:
> Hi Maxime,
>
> Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
> 12:55:59 +0200:
>
> > On Fri, Apr 05, 2019 at 11:37:42AM +0200, Miquel Raynal wrote:
> > > Hi Maxime,
> > >
> > > Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
> > > 11:16:07 +0200:
> > >
> > > > On Thu, Apr 04, 2019 at 06:21:10PM +0200, Miquel Raynal wrote:
> > > > > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > > > > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > > > > is a bit different than with the older SoCs, hence the introduction of
> > > > > a new compatible to handle:
> > > > > * the differences between register offsets,
> > > > > * the burst length change from 4 to minimum 8,
> > > > > * drive SRAM accesses through the AHB bus instead of the MBUS.
> > > > >
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
> > > > >  1 file changed, 68 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > > > > index 4282bc477761..49cd5067adaa 100644
> > > > > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > > > > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > > > > @@ -42,7 +42,8 @@
> > > > >  #define NFC_REG_CMD		0x0024
> > > > >  #define NFC_REG_RCMD_SET	0x0028
> > > > >  #define NFC_REG_WCMD_SET	0x002C
> > > > > -#define NFC_REG_IO_DATA		0x0030
> > > > > +#define NFC_REG_A10_IO_DATA	0x0030
> > > > > +#define NFC_REG_A33_IO_DATA	0x0300
> > > > >  #define NFC_REG_ECC_CTL		0x0034
> > > > >  #define NFC_REG_ECC_ST		0x0038
> > > > >  #define NFC_REG_DEBUG		0x003C
> > > > > @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
> > > > >  	return container_of(nand, struct sunxi_nand_chip, nand);
> > > > >  }
> > > > >
> > > > > +/*
> > > > > + * NAND Controller capabilities structure: stores NAND controller capabilities
> > > > > + * for distinction between compatible strings.
> > > > > + *
> > > > > + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> > > > > + *                      instead of MBUS (less configuration). A10+ use the MBUS
> > > >
> > > > What do you mean by A10+ ?
> > >
> > > I meant A1x, A2x SoCs. Not sure it matches a product line for you, so
> > > please suggest something to mean "SoCs which are not A33" (so far I
> > > think all worked without this).
> >
> > The list is pretty small, so we can just name them. That would be the
> > A10, A10s A13 and A20.
>
> You really need a "A10+"-like acronym for these ;)

A10, A10s, A13, A20 and GR8 but not A23, A31, A33, A64, A80, A83t, or
any SoC that is still not out yet.

A10+ doesn't look like the most appropriate fit for that list :)

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Boris Brezillon]

On Thu,  4 Apr 2019 18:21:10 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Allwinner NAND controllers can make use of DMA to enhance the I/O
> throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> is a bit different than with the older SoCs, hence the introduction of
> a new compatible to handle:
> * the differences between register offsets,
> * the burst length change from 4 to minimum 8,
> * drive SRAM accesses through the AHB bus instead of the MBUS.

Hm, now that you know MBUS accesses are working fine (IIRC, that's what
you used for the SPL DMA-based implementation), why not directly use
MBUS accesses on A33? I mean, it's likely faster than going through
the DMA engine (which is shared by several IPs), and AFAIR, the MBUS
setup is pretty simple.

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++++++++++++++++++++++++---
>  1 file changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 4282bc477761..49cd5067adaa 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -42,7 +42,8 @@
>  #define NFC_REG_CMD		0x0024
>  #define NFC_REG_RCMD_SET	0x0028
>  #define NFC_REG_WCMD_SET	0x002C
> -#define NFC_REG_IO_DATA		0x0030
> +#define NFC_REG_A10_IO_DATA	0x0030
> +#define NFC_REG_A33_IO_DATA	0x0300
>  #define NFC_REG_ECC_CTL		0x0034
>  #define NFC_REG_ECC_ST		0x0038
>  #define NFC_REG_DEBUG		0x003C
> @@ -200,6 +201,22 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
>  	return container_of(nand, struct sunxi_nand_chip, nand);
>  }
>  
> +/*
> + * NAND Controller capabilities structure: stores NAND controller capabilities
> + * for distinction between compatible strings.
> + *
> + * @sram_through_ahb:	On A33, we choose to access the internal RAM through AHB
> + *                      instead of MBUS (less configuration). A10+ use the MBUS
> + *                      but no extra configuration is needed.
> + * @reg_io_data:	I/O data register
> + * @dma_maxburst:	DMA maxburst
> + */
> +struct sunxi_nfc_caps {
> +	bool sram_through_ahb;
> +	unsigned int reg_io_data;
> +	unsigned int dma_maxburst;
> +};
> +
>  /**
>   * struct sunxi_nfc - stores sunxi NAND controller information
>   *
> @@ -228,6 +245,7 @@ struct sunxi_nfc {
>  	struct list_head chips;
>  	struct completion complete;
>  	struct dma_chan *dmac;
> +	const struct sunxi_nfc_caps *caps;
>  };
>  
>  static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl)
> @@ -350,10 +368,29 @@ static int sunxi_nfc_dma_op_prepare(struct sunxi_nfc *nfc, const void *buf,
>  		goto err_unmap_buf;
>  	}
>  
> -	writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
> -	       nfc->regs + NFC_REG_CTL);
> +	/*
> +	 * On A33, we suppose the "internal RAM" (p.12 of the user manual)
> +	 * refers to the NAND controller's internal SRAM. This memory is mapped
> +	 * and so is accessible from the AHB. It seems that it can also be
> +	 * accessed by the MBUS. MBUS accesses are mandatory when using the
> +	 * internal DMA instead of the external DMA engine.
> +	 *
> +	 * During DMA I/O operation, either we access this memory from the AHB
> +	 * by clearing the NFC_RAM_METHOD bit, or we set the bit and use the
> +	 * MBUS. In this case, we should also configure the MBUS DMA length
> +	 * NFC_REG_MDMA_CNT(0xC4) to be chunksize * nchunks. NAND I/O over MBUS
> +	 * are also limited to 32kiB pages.
> +	 */
> +	if (nfc->caps->sram_through_ahb)
> +		writel(readl(nfc->regs + NFC_REG_CTL) & ~NFC_RAM_METHOD,
> +		       nfc->regs + NFC_REG_CTL);
> +	else
> +		writel(readl(nfc->regs + NFC_REG_CTL) | NFC_RAM_METHOD,
> +		       nfc->regs + NFC_REG_CTL);
> +
>  	writel(nchunks, nfc->regs + NFC_REG_SECTOR_NUM);
>  	writel(chunksize, nfc->regs + NFC_REG_CNT);
> +
>  	dmat = dmaengine_submit(dmad);
>  
>  	ret = dma_submit_error(dmat);
> @@ -2088,6 +2125,12 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
>  		goto out_mod_clk_unprepare;
>  	}
>  
> +	nfc->caps = of_device_get_match_data(&pdev->dev);
> +	if (!nfc->caps) {
> +		ret = -EINVAL;
> +		goto out_ahb_reset_reassert;
> +	}
> +
>  	ret = sunxi_nfc_rst(nfc);
>  	if (ret)
>  		goto out_ahb_reset_reassert;
> @@ -2102,12 +2145,12 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
>  	if (nfc->dmac) {
>  		struct dma_slave_config dmac_cfg = { };
>  
> -		dmac_cfg.src_addr = r->start + NFC_REG_IO_DATA;
> +		dmac_cfg.src_addr = r->start + nfc->caps->reg_io_data;
>  		dmac_cfg.dst_addr = dmac_cfg.src_addr;
>  		dmac_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  		dmac_cfg.dst_addr_width = dmac_cfg.src_addr_width;
> -		dmac_cfg.src_maxburst = 4;
> -		dmac_cfg.dst_maxburst = 4;
> +		dmac_cfg.src_maxburst = nfc->caps->dma_maxburst;
> +		dmac_cfg.dst_maxburst = nfc->caps->dma_maxburst;
>  		dmaengine_slave_config(nfc->dmac, &dmac_cfg);
>  	} else {
>  		dev_warn(dev, "failed to request rxtx DMA channel\n");
> @@ -2152,8 +2195,26 @@ static int sunxi_nfc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
> +	.reg_io_data = NFC_REG_A10_IO_DATA,
> +	.dma_maxburst = 4,
> +};
> +
> +static const struct sunxi_nfc_caps sunxi_nfc_a33_caps = {
> +	.sram_through_ahb = true,
> +	.reg_io_data = NFC_REG_A33_IO_DATA,
> +	.dma_maxburst = 8,
> +};
> +
>  static const struct of_device_id sunxi_nfc_ids[] = {
> -	{ .compatible = "allwinner,sun4i-a10-nand" },
> +	{
> +		.compatible = "allwinner,sun4i-a10-nand",
> +		.data = &sunxi_nfc_a10_caps,
> +	},
> +	{
> +		.compatible = "allwinner,sun8i-a33-nand-controller",
> +		.data = &sunxi_nfc_a33_caps,
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sunxi_nfc_ids);


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 14 Apr
2019 11:05:49 +0200:

> On Thu,  4 Apr 2019 18:21:10 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > is a bit different than with the older SoCs, hence the introduction of
> > a new compatible to handle:
> > * the differences between register offsets,
> > * the burst length change from 4 to minimum 8,
> > * drive SRAM accesses through the AHB bus instead of the MBUS.  
> 
> Hm, now that you know MBUS accesses are working fine (IIRC, that's what
> you used for the SPL DMA-based implementation), why not directly use
> MBUS accesses on A33? I mean, it's likely faster than going through
> the DMA engine (which is shared by several IPs), and AFAIR, the MBUS
> setup is pretty simple.

Because all the driver is already in shape to use the external DMA
engine and it was very easy and quick (have a look at the diff of the
v3) to use it again.

However, the choice I am describing here is not DMA vs. MBUS (or MDMA),
it is MBUS vs. AHB, it is just about the bus that will access the SRAM
(this is what we have understood with Maxime from the datasheets and
the tests we have done). For this choice, we tested with both buses: no
throughput change so we think that it is not a bottleneck anyway.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Boris Brezillon]

On Mon, 15 Apr 2019 08:58:59 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 14 Apr
> 2019 11:05:49 +0200:
> 
> > On Thu,  4 Apr 2019 18:21:10 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > > is a bit different than with the older SoCs, hence the introduction of
> > > a new compatible to handle:
> > > * the differences between register offsets,
> > > * the burst length change from 4 to minimum 8,
> > > * drive SRAM accesses through the AHB bus instead of the MBUS.    
> > 
> > Hm, now that you know MBUS accesses are working fine (IIRC, that's what
> > you used for the SPL DMA-based implementation), why not directly use
> > MBUS accesses on A33? I mean, it's likely faster than going through
> > the DMA engine (which is shared by several IPs), and AFAIR, the MBUS
> > setup is pretty simple.  
> 
> Because all the driver is already in shape to use the external DMA
> engine and it was very easy and quick (have a look at the diff of the
> v3) to use it again.

Yes, I see that. I might be wrong but I'd expect the MDMA version to be
just as simple as this one.

> 
> However, the choice I am describing here is not DMA vs. MBUS (or MDMA),
> it is MBUS vs. AHB, it is just about the bus that will access the SRAM
> (this is what we have understood with Maxime from the datasheets and
> the tests we have done).

Yes, sorry, I meant MDMA vs shared DMA engine, but I guess MBUS is only
used through MDMA accesses anyway, right?

> For this choice, we tested with both buses: no
> throughput change so we think that it is not a bottleneck anyway.

Well, you'd need to test with a lot of traffic going through the DMA
engine to check if that makes a difference.

Anyway, it was just a suggestion, keep it like that if you think using
MDMA is not worthwhile.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: sunxi: Add DMA support for sun8i

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 15 Apr
2019 09:14:20 +0200:

> On Mon, 15 Apr 2019 08:58:59 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 14 Apr
> > 2019 11:05:49 +0200:
> >   
> > > On Thu,  4 Apr 2019 18:21:10 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Allwinner NAND controllers can make use of DMA to enhance the I/O
> > > > throughput thanks to ECC pipelining. DMA handling with sun8i NAND IP
> > > > is a bit different than with the older SoCs, hence the introduction of
> > > > a new compatible to handle:
> > > > * the differences between register offsets,
> > > > * the burst length change from 4 to minimum 8,
> > > > * drive SRAM accesses through the AHB bus instead of the MBUS.      
> > > 
> > > Hm, now that you know MBUS accesses are working fine (IIRC, that's what
> > > you used for the SPL DMA-based implementation), why not directly use
> > > MBUS accesses on A33? I mean, it's likely faster than going through
> > > the DMA engine (which is shared by several IPs), and AFAIR, the MBUS
> > > setup is pretty simple.    
> > 
> > Because all the driver is already in shape to use the external DMA
> > engine and it was very easy and quick (have a look at the diff of the
> > v3) to use it again.  
> 
> Yes, I see that. I might be wrong but I'd expect the MDMA version to be
> just as simple as this one.
> 
> > 
> > However, the choice I am describing here is not DMA vs. MBUS (or MDMA),
> > it is MBUS vs. AHB, it is just about the bus that will access the SRAM
> > (this is what we have understood with Maxime from the datasheets and
> > the tests we have done).  
> 
> Yes, sorry, I meant MDMA vs shared DMA engine, but I guess MBUS is only
> used through MDMA accesses anyway, right?

I have no proof that I actually used MBUS, but there is a bit which can
be set to use MBUS to access the SRAM even when not using MDMA. In this
case, MBUS payload length had to be filled or the operation would not
succeed. However when using AHB, there is no need for this extra
configuration. That's why I decided to use the AHB.

> 
> > For this choice, we tested with both buses: no
> > throughput change so we think that it is not a bottleneck anyway.  
> 
> Well, you'd need to test with a lot of traffic going through the DMA
> engine to check if that makes a difference.

True, I probably didn't stressed the platform enough to see the
difference.

> 
> Anyway, it was just a suggestion, keep it like that if you think using
> MDMA is not worthwhile.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/3] ARM: dts: sunxi: Improve sun8i NAND transfers by using DMA

[Miquel Raynal]

In the current state, sun8i NAND controllers use PIO during
transfers. Throughput can be increased thanks to the use of DMA
(mostly during reads, because of the ECC pipelining feature).

Besides the usual addition of DMA DT properties, because the sun8i
NAND DMA handling is different than for older SoCs, we must also
update the compatible which has recently been introduced for this
purpose.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 14a7d0288b45..f928b4bceb22 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -162,11 +162,13 @@
 		};
 
 		nfc: nand@1c03000 {
-			compatible = "allwinner,sun4i-a10-nand";
+			compatible = "allwinner,sun8i-a33-nand-controller";
 			reg = <0x01c03000 0x1000>;
 			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&ccu CLK_BUS_NAND>, <&ccu CLK_NAND>;
 			clock-names = "ahb", "mod";
+			dmas = <&dma 5>;
+			dma-names = "rxtx";
 			resets = <&ccu RST_BUS_NAND>;
 			reset-names = "ahb";
 			pinctrl-names = "default";
-- 
2.19.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/3] ARM: dts: sunxi: Improve sun8i NAND transfers by using DMA

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

On Thu, Apr 04, 2019 at 06:21:11PM +0200, Miquel Raynal wrote:
> In the current state, sun8i NAND controllers use PIO during
> transfers. Throughput can be increased thanks to the use of DMA
> (mostly during reads, because of the ECC pipelining feature).
>
> Besides the usual addition of DMA DT properties, because the sun8i

The only thing that "sun8i" means for Allwinner is that the SoC has
Cortex-A7 CPUs. You need to be more precise than that, why not just
use A33?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/3] ARM: dts: sunxi: Improve sun8i NAND transfers by using DMA

[Miquel Raynal]

[-- Attachment #1.1: Type: text/plain, Size: 676 bytes --]

Hi Maxime,

Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri, 5 Apr 2019
11:18:07 +0200:

> On Thu, Apr 04, 2019 at 06:21:11PM +0200, Miquel Raynal wrote:
> > In the current state, sun8i NAND controllers use PIO during
> > transfers. Throughput can be increased thanks to the use of DMA
> > (mostly during reads, because of the ECC pipelining feature).
> >
> > Besides the usual addition of DMA DT properties, because the sun8i  
> 
> The only thing that "sun8i" means for Allwinner is that the SoC has
> Cortex-A7 CPUs. You need to be more precise than that, why not just
> use A33?

I am always confused. I'll stick to A33 then.


Thanks,
Miquèl

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/