| Message ID | 20190404162111.22618-3-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Sun8i NAND DMA support | expand |
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
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
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
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
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
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);
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
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.
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
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);
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(-)