[PATCH V2 1/3] dt: bindings: brcmnand: Add support for flash-edu

[PATCH V2 1/3] dt: bindings: brcmnand: Add support for flash-edu

[Kamal Dasu]

Adding support for EBI DMA unit (EDU).

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 .../devicetree/bindings/mtd/brcm,brcmnand.txt          | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
index 82156dc8f304..05651a654c66 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
@@ -35,11 +35,11 @@ Required properties:
                      (optional) NAND flash cache range (if at non-standard offset)
 - reg-names        : a list of the names corresponding to the previous register
                      ranges. Should contain "nand" and (optionally)
-                     "flash-dma" and/or "nand-cache".
-- interrupts       : The NAND CTLRDY interrupt and (if Flash DMA is available)
-                     FLASH_DMA_DONE
-- interrupt-names  : May be "nand_ctlrdy" or "flash_dma_done", if broken out as
-                     individual interrupts.
+                     "flash-dma" or "flash-edu" and/or "nand-cache".
+- interrupts       : The NAND CTLRDY interrupt, (if Flash DMA is available)
+                     FLASH_DMA_DONE and if EDU is avaialble and used FLASH_EDU_DONE
+- interrupt-names  : May be "nand_ctlrdy" or "flash_dma_done" or "flash_edu_done",
+                     if broken out as individual interrupts.
                      May be "nand", if the SoC has the individual NAND
                      interrupts multiplexed behind another custom piece of
                      hardware
-- 
2.17.1


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

[PATCH V2 2/3] arch: mips: brcm: Add 7425 flash-edu support

[Kamal Dasu]

Nand controller v5.0 and v6.0 have nand edu blocks that enable
dma nand flash transfers. This allows for faster read and write
access.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 arch/mips/boot/dts/brcm/bcm7425.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/boot/dts/brcm/bcm7425.dtsi b/arch/mips/boot/dts/brcm/bcm7425.dtsi
index 410e61ebaf9e..aa0b2d39c902 100644
--- a/arch/mips/boot/dts/brcm/bcm7425.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7425.dtsi
@@ -403,8 +403,8 @@
 			compatible = "brcm,brcmnand-v5.0", "brcm,brcmnand";
 			#address-cells = <1>;
 			#size-cells = <0>;
-			reg-names = "nand";
-			reg = <0x41b800 0x400>;
+			reg-names = "nand", "flash-edu";
+			reg = <0x41b800 0x400>, <0x41bc00 0x24>;
 			interrupt-parent = <&hif_l2_intc>;
 			interrupts = <24>;
 			status = "disabled";
-- 
2.17.1


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

[PATCH V2 3/3] mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers

[Kamal Dasu]

Legacy mips soc platforms that have controller v5.0 and 6.0 use
flash-edu block for dma transfers. This change adds support for
nand dma transfers using the EDU block.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 296 ++++++++++++++++++++++-
 1 file changed, 290 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 1a66b1cd51c0..61347607f1da 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -102,6 +102,45 @@ struct brcm_nand_dma_desc {
 #define NAND_CTRL_RDY			(INTFC_CTLR_READY | INTFC_FLASH_READY)
 #define NAND_POLL_STATUS_TIMEOUT_MS	100
 
+#define EDU_CMD_WRITE          0x00
+#define EDU_CMD_READ           0x01
+#define EDU_STATUS_ACTIVE      BIT(0)
+#define EDU_ERR_STATUS_ERRACK  BIT(0)
+#define EDU_DONE_MASK		GENMASK(1, 0)
+
+#define EDU_CONFIG_MODE_NAND   BIT(0)
+#define EDU_CONFIG_SWAP_BYTE   BIT(1)
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define EDU_CONFIG_SWAP_CFG     EDU_CONFIG_SWAP_BYTE
+#else
+#define EDU_CONFIG_SWAP_CFG     0
+#endif
+
+/* edu registers */
+enum edu_reg {
+	EDU_CONFIG = 0,
+	EDU_DRAM_ADDR,
+	EDU_EXT_ADDR,
+	EDU_LENGTH,
+	EDU_CMD,
+	EDU_STOP,
+	EDU_STATUS,
+	EDU_DONE,
+	EDU_ERR_STATUS,
+};
+
+static const u16  edu_regs[] = {
+	[EDU_CONFIG] = 0x00,
+	[EDU_DRAM_ADDR] = 0x04,
+	[EDU_EXT_ADDR] = 0x08,
+	[EDU_LENGTH] = 0x0c,
+	[EDU_CMD] = 0x10,
+	[EDU_STOP] = 0x14,
+	[EDU_STATUS] = 0x18,
+	[EDU_DONE] = 0x1c,
+	[EDU_ERR_STATUS] = 0x20,
+};
+
 /* flash_dma registers */
 enum flash_dma_reg {
 	FLASH_DMA_REVISION = 0,
@@ -167,6 +206,8 @@ enum {
 	BRCMNAND_HAS_WP				= BIT(3),
 };
 
+struct brcmnand_host;
+
 struct brcmnand_controller {
 	struct device		*dev;
 	struct nand_controller	controller;
@@ -185,17 +226,32 @@ struct brcmnand_controller {
 
 	int			cmd_pending;
 	bool			dma_pending;
+	bool                    edu_pending;
 	struct completion	done;
 	struct completion	dma_done;
+	struct completion       edu_done;
 
 	/* List of NAND hosts (one for each chip-select) */
 	struct list_head host_list;
 
+	/* EDU info, per-transaction */
+	const u16               *edu_offsets;
+	void __iomem            *edu_base;
+	unsigned int            edu_irq;
+	int                     edu_count;
+	u64                     edu_dram_addr;
+	u32                     edu_ext_addr;
+	u32                     edu_cmd;
+	u32                     edu_config;
+
 	/* flash_dma reg */
 	const u16		*flash_dma_offsets;
 	struct brcm_nand_dma_desc *dma_desc;
 	dma_addr_t		dma_pa;
 
+	int (*dma_trans)(struct brcmnand_host *host, u64 addr, u32 *buf,
+			 u32 len, u8 dma_cmd);
+
 	/* in-memory cache of the FLASH_CACHE, used only for some commands */
 	u8			flash_cache[FC_BYTES];
 
@@ -216,6 +272,7 @@ struct brcmnand_controller {
 	u32			nand_cs_nand_xor;
 	u32			corr_stat_threshold;
 	u32			flash_dma_mode;
+	u32                     flash_edu_mode;
 	bool			pio_poll_mode;
 };
 
@@ -657,6 +714,22 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
 	__raw_writel(val, ctrl->nand_fc + word * 4);
 }
 
+static inline void edu_writel(struct brcmnand_controller *ctrl,
+			      enum edu_reg reg, u32 val)
+{
+	u16 offs = ctrl->edu_offsets[reg];
+
+	brcmnand_writel(val, ctrl->edu_base + offs);
+}
+
+static inline u32 edu_readl(struct brcmnand_controller *ctrl,
+			    enum edu_reg reg)
+{
+	u16 offs = ctrl->edu_offsets[reg];
+
+	return brcmnand_readl(ctrl->edu_base + offs);
+}
+
 static void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
 {
 
@@ -926,6 +999,16 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
 	return ctrl->flash_dma_base;
 }
 
+static inline bool has_edu(struct brcmnand_controller *ctrl)
+{
+	return ctrl->edu_base;
+}
+
+static inline bool use_dma(struct brcmnand_controller *ctrl)
+{
+	return has_flash_dma(ctrl) || has_edu(ctrl);
+}
+
 static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl)
 {
 	if (ctrl->pio_poll_mode)
@@ -1299,6 +1382,52 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
 	return tbytes;
 }
 
+static void brcmnand_edu_init(struct brcmnand_controller *ctrl)
+{
+	/* initialize edu */
+	edu_writel(ctrl, EDU_ERR_STATUS, 0);
+	edu_readl(ctrl, EDU_ERR_STATUS);
+	edu_writel(ctrl, EDU_DONE, 0);
+	edu_writel(ctrl, EDU_DONE, 0);
+	edu_writel(ctrl, EDU_DONE, 0);
+	edu_writel(ctrl, EDU_DONE, 0);
+	edu_readl(ctrl, EDU_DONE);
+}
+
+/* edu irq */
+static irqreturn_t brcmnand_edu_irq(int irq, void *data)
+{
+	struct brcmnand_controller *ctrl = data;
+
+	if (ctrl->edu_count) {
+		ctrl->edu_count--;
+		while (!(edu_readl(ctrl, EDU_DONE) & EDU_DONE_MASK))
+			udelay(1);
+		edu_writel(ctrl, EDU_DONE, 0);
+		edu_readl(ctrl, EDU_DONE);
+	}
+
+	if (ctrl->edu_count) {
+		ctrl->edu_dram_addr += FC_BYTES;
+		ctrl->edu_ext_addr += FC_BYTES;
+
+		edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
+		edu_readl(ctrl, EDU_DRAM_ADDR);
+		edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
+		edu_readl(ctrl, EDU_EXT_ADDR);
+
+		mb(); /* flush previous writes */
+		edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
+		edu_readl(ctrl, EDU_CMD);
+
+		return IRQ_HANDLED;
+	}
+
+	complete(&ctrl->edu_done);
+
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
 {
 	struct brcmnand_controller *ctrl = data;
@@ -1307,6 +1436,16 @@ static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
 	if (ctrl->dma_pending)
 		return IRQ_HANDLED;
 
+	/* check if you need to piggy back on the ctrlrdy irq */
+	if (ctrl->edu_pending) {
+		if (irq == ctrl->irq && ((int)ctrl->edu_irq >= 0))
+	/* Discard interrupts while using dedicated edu irq */
+			return IRQ_HANDLED;
+
+	/* no registered edu irq, call handler */
+		return brcmnand_edu_irq(irq, data);
+	}
+
 	complete(&ctrl->done);
 	return IRQ_HANDLED;
 }
@@ -1644,6 +1783,83 @@ static void brcmnand_write_buf(struct nand_chip *chip, const uint8_t *buf,
 	}
 }
 
+/**
+ *  Kick EDU engine
+ */
+static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf,
+			      u32 len, u8 cmd)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	unsigned long timeo = msecs_to_jiffies(200);
+	int ret = 0;
+	int dir = (cmd == CMD_PAGE_READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	u8 edu_cmd = (cmd == CMD_PAGE_READ ? EDU_CMD_READ : EDU_CMD_WRITE);
+	unsigned int trans = len >> FC_SHIFT;
+	dma_addr_t pa;
+
+	pa = dma_map_single(ctrl->dev, buf, len, dir);
+	if (dma_mapping_error(ctrl->dev, pa)) {
+		dev_err(ctrl->dev, "unable to map buffer for EDU DMA\n");
+		return -ENOMEM;
+	}
+
+	ctrl->edu_pending = true;
+	mb(); /* flush previous writes */
+
+	ctrl->edu_dram_addr = pa;
+	ctrl->edu_ext_addr = addr;
+	ctrl->edu_cmd = edu_cmd;
+	ctrl->edu_count = trans;
+
+	edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
+	edu_readl(ctrl,  EDU_DRAM_ADDR);
+	edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
+	edu_readl(ctrl, EDU_EXT_ADDR);
+	edu_writel(ctrl, EDU_LENGTH, FC_BYTES);
+	edu_readl(ctrl, EDU_LENGTH);
+
+	/* Start edu engine */
+	mb(); /* flush previous writes */
+	edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
+	edu_readl(ctrl, EDU_CMD);
+
+	if (wait_for_completion_timeout(&ctrl->edu_done, timeo) <= 0) {
+		dev_err(ctrl->dev,
+			"timeout waiting for EDU; status %#x, error status %#x\n",
+			edu_readl(ctrl, EDU_STATUS),
+			edu_readl(ctrl, EDU_ERR_STATUS));
+	}
+
+	dma_unmap_single(ctrl->dev, pa, len, dir);
+
+	/* for program page check NAND status */
+	if (((brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) &
+	      INTFC_FLASH_STATUS) & NAND_STATUS_FAIL) &&
+	    edu_cmd == EDU_CMD_WRITE) {
+		dev_info(ctrl->dev, "program failed at %llx\n",
+			 (unsigned long long)addr);
+		ret = -EIO;
+	}
+
+	/* Make sure the EDU status is clean */
+	if (edu_readl(ctrl, EDU_STATUS) & EDU_STATUS_ACTIVE)
+		dev_warn(ctrl->dev, "EDU still active: %#x\n",
+			 edu_readl(ctrl, EDU_STATUS));
+
+	if (unlikely(edu_readl(ctrl, EDU_ERR_STATUS) & EDU_ERR_STATUS_ERRACK)) {
+		dev_warn(ctrl->dev, "EDU RBUS error at addr %llx\n",
+			 (unsigned long long)addr);
+		ret = -EIO;
+	}
+
+	ctrl->edu_pending = false;
+	brcmnand_edu_init(ctrl);
+	edu_writel(ctrl, EDU_STOP, 0); /* force stop */
+	edu_readl(ctrl, EDU_STOP);
+
+	return ret;
+}
+
 /**
  * Construct a FLASH_DMA descriptor as part of a linked list. You must know the
  * following ahead of time:
@@ -1850,9 +2066,11 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
 try_dmaread:
 	brcmnand_clear_ecc_addr(ctrl);
 
-	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
-		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
-					     CMD_PAGE_READ);
+	if (ctrl->dma_trans && !oob && flash_dma_buf_ok(buf)) {
+		err = ctrl->dma_trans(host, addr, buf,
+				      trans * FC_BYTES,
+				      CMD_PAGE_READ);
+
 		if (err) {
 			if (mtd_is_bitflip_or_eccerr(err))
 				err_addr = addr;
@@ -1988,10 +2206,12 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
 	for (i = 0; i < ctrl->max_oob; i += 4)
 		oob_reg_write(ctrl, i, 0xffffffff);
 
-	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
-		if (brcmnand_dma_trans(host, addr, (u32 *)buf,
-					mtd->writesize, CMD_PROGRAM_PAGE))
+	if (use_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
+		if (ctrl->dma_trans(host, addr, (u32 *)buf, mtd->writesize,
+				    CMD_PROGRAM_PAGE))
+
 			ret = -EIO;
+
 		goto out;
 	}
 
@@ -2494,6 +2714,8 @@ static int brcmnand_suspend(struct device *dev)
 
 	if (has_flash_dma(ctrl))
 		ctrl->flash_dma_mode = flash_dma_readl(ctrl, FLASH_DMA_MODE);
+	else if (has_edu(ctrl))
+		ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
 
 	return 0;
 }
@@ -2508,6 +2730,14 @@ static int brcmnand_resume(struct device *dev)
 		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
 	}
 
+	if (has_edu(ctrl))
+		ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
+	else {
+		edu_writel(ctrl, EDU_CONFIG, ctrl->edu_config);
+		edu_readl(ctrl, EDU_CONFIG);
+		brcmnand_edu_init(ctrl);
+	}
+
 	brcmnand_write_reg(ctrl, BRCMNAND_CS_SELECT, ctrl->nand_cs_nand_select);
 	brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
 	brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
@@ -2553,6 +2783,52 @@ MODULE_DEVICE_TABLE(of, brcmnand_of_match);
 /***********************************************************************
  * Platform driver setup (per controller)
  ***********************************************************************/
+static int brcmnand_edu_setup(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
+	struct resource *res;
+	int ret;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-edu");
+	if (res) {
+		ctrl->edu_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ctrl->edu_base))
+			return PTR_ERR(ctrl->edu_base);
+
+		ctrl->edu_offsets = edu_regs;
+
+		edu_writel(ctrl, EDU_CONFIG, EDU_CONFIG_MODE_NAND |
+			   EDU_CONFIG_SWAP_CFG);
+		edu_readl(ctrl, EDU_CONFIG);
+
+		/* initialize edu */
+		brcmnand_edu_init(ctrl);
+
+		ctrl->edu_irq = platform_get_irq_optional(pdev, 1);
+		if ((int)ctrl->edu_irq < 0) {
+			dev_warn(dev,
+				 "FLASH EDU enabled, using ctlrdy irq\n");
+		} else {
+			ret = devm_request_irq(dev, ctrl->edu_irq,
+					       brcmnand_edu_irq, 0,
+					       "brcmnand-edu", ctrl);
+			if (ret < 0) {
+				dev_err(ctrl->dev, "can't allocate IRQ %d: error %d\n",
+					ctrl->edu_irq, ret);
+				return ret;
+			}
+
+			dev_info(dev, "FLASH EDU enabled using irq %u\n",
+				 ctrl->edu_irq);
+		}
+
+		/* set the appropriate edu transfer function to call */
+		ctrl->dma_trans = brcmnand_edu_trans;
+	}
+
+	return 0;
+}
 
 int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 {
@@ -2578,6 +2854,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 
 	init_completion(&ctrl->done);
 	init_completion(&ctrl->dma_done);
+	init_completion(&ctrl->edu_done);
 	nand_controller_init(&ctrl->controller);
 	ctrl->controller.ops = &brcmnand_controller_ops;
 	INIT_LIST_HEAD(&ctrl->host_list);
@@ -2623,6 +2900,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 				ctrl->reg_offsets[BRCMNAND_FC_BASE];
 	}
 
+	ctrl->dma_trans = NULL;
 	/* FLASH_DMA */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
 	if (res) {
@@ -2665,6 +2943,12 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 		}
 
 		dev_info(dev, "enabling FLASH_DMA\n");
+		/* set the appropriate flash dma transfer function to call */
+		ctrl->dma_trans = brcmnand_dma_trans;
+	} else	{
+		ret = brcmnand_edu_setup(pdev);
+		if (ret < 0)
+			goto err;
 	}
 
 	/* Disable automatic device ID config, direct addressing */
-- 
2.17.1


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

Re: [PATCH V2 2/3] arch: mips: brcm: Add 7425 flash-edu support

[Florian Fainelli]

On 1/21/20 12:00 PM, Kamal Dasu wrote:
> Nand controller v5.0 and v6.0 have nand edu blocks that enable
> dma nand flash transfers. This allows for faster read and write
> access.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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

Re: [PATCH V2 2/3] arch: mips: brcm: Add 7425 flash-edu support

[Paul Burton]

Hi Kamal,

On Tue, Jan 21, 2020 at 03:00:07PM -0500, Kamal Dasu wrote:
> Nand controller v5.0 and v6.0 have nand edu blocks that enable
> dma nand flash transfers. This allows for faster read and write
> access.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  arch/mips/boot/dts/brcm/bcm7425.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/brcm/bcm7425.dtsi b/arch/mips/boot/dts/brcm/bcm7425.dtsi
> index 410e61ebaf9e..aa0b2d39c902 100644
> --- a/arch/mips/boot/dts/brcm/bcm7425.dtsi
> +++ b/arch/mips/boot/dts/brcm/bcm7425.dtsi
> @@ -403,8 +403,8 @@
>  			compatible = "brcm,brcmnand-v5.0", "brcm,brcmnand";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -			reg-names = "nand";
> -			reg = <0x41b800 0x400>;
> +			reg-names = "nand", "flash-edu";
> +			reg = <0x41b800 0x400>, <0x41bc00 0x24>;
>  			interrupt-parent = <&hif_l2_intc>;
>  			interrupts = <24>;
>  			status = "disabled";

I wasn't copied on the rest of the series, but presuming patch 1
documents flash-edu in the binding documentation at
Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt:

    Acked-by: Paul Burton <paulburton@kernel.org>

Thanks,
    Paul

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

Re: [PATCH V2 3/3] mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers

[Miquel Raynal]

Hi Kamal,

I'm fine with the patch, nitpicking below :)

Kamal Dasu <kdasu.kdev@gmail.com> wrote on Tue, 21 Jan 2020 15:00:08
-0500:

> Legacy mips soc platforms that have controller v5.0 and 6.0 use
> flash-edu block for dma transfers. This change adds support for
> nand dma transfers using the EDU block.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 296 ++++++++++++++++++++++-
>  1 file changed, 290 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 1a66b1cd51c0..61347607f1da 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -102,6 +102,45 @@ struct brcm_nand_dma_desc {
>  #define NAND_CTRL_RDY			(INTFC_CTLR_READY | INTFC_FLASH_READY)
>  #define NAND_POLL_STATUS_TIMEOUT_MS	100
>  
> +#define EDU_CMD_WRITE          0x00
> +#define EDU_CMD_READ           0x01
> +#define EDU_STATUS_ACTIVE      BIT(0)
> +#define EDU_ERR_STATUS_ERRACK  BIT(0)
> +#define EDU_DONE_MASK		GENMASK(1, 0)
> +
> +#define EDU_CONFIG_MODE_NAND   BIT(0)
> +#define EDU_CONFIG_SWAP_BYTE   BIT(1)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define EDU_CONFIG_SWAP_CFG     EDU_CONFIG_SWAP_BYTE
> +#else
> +#define EDU_CONFIG_SWAP_CFG     0
> +#endif
> +
> +/* edu registers */
> +enum edu_reg {
> +	EDU_CONFIG = 0,
> +	EDU_DRAM_ADDR,
> +	EDU_EXT_ADDR,
> +	EDU_LENGTH,
> +	EDU_CMD,
> +	EDU_STOP,
> +	EDU_STATUS,
> +	EDU_DONE,
> +	EDU_ERR_STATUS,
> +};
> +
> +static const u16  edu_regs[] = {
> +	[EDU_CONFIG] = 0x00,
> +	[EDU_DRAM_ADDR] = 0x04,
> +	[EDU_EXT_ADDR] = 0x08,
> +	[EDU_LENGTH] = 0x0c,
> +	[EDU_CMD] = 0x10,
> +	[EDU_STOP] = 0x14,
> +	[EDU_STATUS] = 0x18,
> +	[EDU_DONE] = 0x1c,
> +	[EDU_ERR_STATUS] = 0x20,
> +};

Why not defining the offsets in the enum directly?

> +
>  /* flash_dma registers */
>  enum flash_dma_reg {
>  	FLASH_DMA_REVISION = 0,
> @@ -167,6 +206,8 @@ enum {
>  	BRCMNAND_HAS_WP				= BIT(3),
>  };
>  
> +struct brcmnand_host;
> +
>  struct brcmnand_controller {
>  	struct device		*dev;
>  	struct nand_controller	controller;
> @@ -185,17 +226,32 @@ struct brcmnand_controller {
>  
>  	int			cmd_pending;
>  	bool			dma_pending;
> +	bool                    edu_pending;
>  	struct completion	done;
>  	struct completion	dma_done;
> +	struct completion       edu_done;
>  
>  	/* List of NAND hosts (one for each chip-select) */
>  	struct list_head host_list;
>  
> +	/* EDU info, per-transaction */
> +	const u16               *edu_offsets;
> +	void __iomem            *edu_base;
> +	unsigned int            edu_irq;
> +	int                     edu_count;
> +	u64                     edu_dram_addr;
> +	u32                     edu_ext_addr;
> +	u32                     edu_cmd;
> +	u32                     edu_config;
> +
>  	/* flash_dma reg */
>  	const u16		*flash_dma_offsets;
>  	struct brcm_nand_dma_desc *dma_desc;
>  	dma_addr_t		dma_pa;
>  
> +	int (*dma_trans)(struct brcmnand_host *host, u64 addr, u32 *buf,
> +			 u32 len, u8 dma_cmd);
> +
>  	/* in-memory cache of the FLASH_CACHE, used only for some commands */
>  	u8			flash_cache[FC_BYTES];
>  
> @@ -216,6 +272,7 @@ struct brcmnand_controller {
>  	u32			nand_cs_nand_xor;
>  	u32			corr_stat_threshold;
>  	u32			flash_dma_mode;
> +	u32                     flash_edu_mode;
>  	bool			pio_poll_mode;
>  };
>  
> @@ -657,6 +714,22 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
>  	__raw_writel(val, ctrl->nand_fc + word * 4);
>  }
>  
> +static inline void edu_writel(struct brcmnand_controller *ctrl,
> +			      enum edu_reg reg, u32 val)
> +{
> +	u16 offs = ctrl->edu_offsets[reg];
> +
> +	brcmnand_writel(val, ctrl->edu_base + offs);
> +}
> +
> +static inline u32 edu_readl(struct brcmnand_controller *ctrl,
> +			    enum edu_reg reg)
> +{
> +	u16 offs = ctrl->edu_offsets[reg];
> +
> +	return brcmnand_readl(ctrl->edu_base + offs);
> +}
> +
>  static void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
>  {
>  
> @@ -926,6 +999,16 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
>  	return ctrl->flash_dma_base;
>  }
>  
> +static inline bool has_edu(struct brcmnand_controller *ctrl)
> +{
> +	return ctrl->edu_base;
> +}
> +
> +static inline bool use_dma(struct brcmnand_controller *ctrl)
> +{
> +	return has_flash_dma(ctrl) || has_edu(ctrl);
> +}
> +
>  static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl)
>  {
>  	if (ctrl->pio_poll_mode)
> @@ -1299,6 +1382,52 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
>  	return tbytes;
>  }
>  
> +static void brcmnand_edu_init(struct brcmnand_controller *ctrl)
> +{
> +	/* initialize edu */
> +	edu_writel(ctrl, EDU_ERR_STATUS, 0);
> +	edu_readl(ctrl, EDU_ERR_STATUS);
> +	edu_writel(ctrl, EDU_DONE, 0);
> +	edu_writel(ctrl, EDU_DONE, 0);
> +	edu_writel(ctrl, EDU_DONE, 0);
> +	edu_writel(ctrl, EDU_DONE, 0);
> +	edu_readl(ctrl, EDU_DONE);
> +}
> +
> +/* edu irq */
> +static irqreturn_t brcmnand_edu_irq(int irq, void *data)
> +{
> +	struct brcmnand_controller *ctrl = data;
> +
> +	if (ctrl->edu_count) {
> +		ctrl->edu_count--;
> +		while (!(edu_readl(ctrl, EDU_DONE) & EDU_DONE_MASK))
> +			udelay(1);
> +		edu_writel(ctrl, EDU_DONE, 0);
> +		edu_readl(ctrl, EDU_DONE);
> +	}
> +
> +	if (ctrl->edu_count) {
> +		ctrl->edu_dram_addr += FC_BYTES;
> +		ctrl->edu_ext_addr += FC_BYTES;
> +
> +		edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
> +		edu_readl(ctrl, EDU_DRAM_ADDR);
> +		edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
> +		edu_readl(ctrl, EDU_EXT_ADDR);
> +
> +		mb(); /* flush previous writes */
> +		edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
> +		edu_readl(ctrl, EDU_CMD);
> +
> +		return IRQ_HANDLED;
> +	}
> +
> +	complete(&ctrl->edu_done);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
>  {
>  	struct brcmnand_controller *ctrl = data;
> @@ -1307,6 +1436,16 @@ static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
>  	if (ctrl->dma_pending)
>  		return IRQ_HANDLED;
>  
> +	/* check if you need to piggy back on the ctrlrdy irq */
> +	if (ctrl->edu_pending) {
> +		if (irq == ctrl->irq && ((int)ctrl->edu_irq >= 0))
> +	/* Discard interrupts while using dedicated edu irq */
> +			return IRQ_HANDLED;
> +
> +	/* no registered edu irq, call handler */
> +		return brcmnand_edu_irq(irq, data);
> +	}
> +
>  	complete(&ctrl->done);
>  	return IRQ_HANDLED;
>  }
> @@ -1644,6 +1783,83 @@ static void brcmnand_write_buf(struct nand_chip *chip, const uint8_t *buf,
>  	}
>  }
>  
> +/**
> + *  Kick EDU engine
> + */
> +static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf,
> +			      u32 len, u8 cmd)
> +{
> +	struct brcmnand_controller *ctrl = host->ctrl;
> +	unsigned long timeo = msecs_to_jiffies(200);
> +	int ret = 0;
> +	int dir = (cmd == CMD_PAGE_READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
> +	u8 edu_cmd = (cmd == CMD_PAGE_READ ? EDU_CMD_READ : EDU_CMD_WRITE);
> +	unsigned int trans = len >> FC_SHIFT;
> +	dma_addr_t pa;
> +
> +	pa = dma_map_single(ctrl->dev, buf, len, dir);
> +	if (dma_mapping_error(ctrl->dev, pa)) {
> +		dev_err(ctrl->dev, "unable to map buffer for EDU DMA\n");
> +		return -ENOMEM;
> +	}
> +
> +	ctrl->edu_pending = true;
> +	mb(); /* flush previous writes */

I'd prefer to have the barriers right after the IO access. I don't
think it changes anything but that would ease understand what writes
are you concerned about. Also, maybe you can use non _relaxed accessors
as you usually writel/readl in raw, I don't think you actually need a
barrier in this case.

> +
> +	ctrl->edu_dram_addr = pa;
> +	ctrl->edu_ext_addr = addr;
> +	ctrl->edu_cmd = edu_cmd;
> +	ctrl->edu_count = trans;
> +
> +	edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
> +	edu_readl(ctrl,  EDU_DRAM_ADDR);
> +	edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
> +	edu_readl(ctrl, EDU_EXT_ADDR);
> +	edu_writel(ctrl, EDU_LENGTH, FC_BYTES);
> +	edu_readl(ctrl, EDU_LENGTH);
> +
> +	/* Start edu engine */
> +	mb(); /* flush previous writes */
> +	edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
> +	edu_readl(ctrl, EDU_CMD);
> +
> +	if (wait_for_completion_timeout(&ctrl->edu_done, timeo) <= 0) {
> +		dev_err(ctrl->dev,
> +			"timeout waiting for EDU; status %#x, error status %#x\n",
> +			edu_readl(ctrl, EDU_STATUS),
> +			edu_readl(ctrl, EDU_ERR_STATUS));
> +	}
> +
> +	dma_unmap_single(ctrl->dev, pa, len, dir);
> +
> +	/* for program page check NAND status */
> +	if (((brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) &
> +	      INTFC_FLASH_STATUS) & NAND_STATUS_FAIL) &&
> +	    edu_cmd == EDU_CMD_WRITE) {
> +		dev_info(ctrl->dev, "program failed at %llx\n",
> +			 (unsigned long long)addr);
> +		ret = -EIO;
> +	}
> +
> +	/* Make sure the EDU status is clean */
> +	if (edu_readl(ctrl, EDU_STATUS) & EDU_STATUS_ACTIVE)
> +		dev_warn(ctrl->dev, "EDU still active: %#x\n",
> +			 edu_readl(ctrl, EDU_STATUS));
> +
> +	if (unlikely(edu_readl(ctrl, EDU_ERR_STATUS) & EDU_ERR_STATUS_ERRACK)) {
> +		dev_warn(ctrl->dev, "EDU RBUS error at addr %llx\n",
> +			 (unsigned long long)addr);
> +		ret = -EIO;
> +	}
> +
> +	ctrl->edu_pending = false;
> +	brcmnand_edu_init(ctrl);
> +	edu_writel(ctrl, EDU_STOP, 0); /* force stop */
> +	edu_readl(ctrl, EDU_STOP);
> +
> +	return ret;
> +}
> +
>  /**
>   * Construct a FLASH_DMA descriptor as part of a linked list. You must know the
>   * following ahead of time:
> @@ -1850,9 +2066,11 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>  try_dmaread:
>  	brcmnand_clear_ecc_addr(ctrl);
>  
> -	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> -		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> -					     CMD_PAGE_READ);
> +	if (ctrl->dma_trans && !oob && flash_dma_buf_ok(buf)) {
> +		err = ctrl->dma_trans(host, addr, buf,
> +				      trans * FC_BYTES,
> +				      CMD_PAGE_READ);
> +
>  		if (err) {
>  			if (mtd_is_bitflip_or_eccerr(err))
>  				err_addr = addr;
> @@ -1988,10 +2206,12 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
>  	for (i = 0; i < ctrl->max_oob; i += 4)
>  		oob_reg_write(ctrl, i, 0xffffffff);
>  
> -	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> -		if (brcmnand_dma_trans(host, addr, (u32 *)buf,
> -					mtd->writesize, CMD_PROGRAM_PAGE))
> +	if (use_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> +		if (ctrl->dma_trans(host, addr, (u32 *)buf, mtd->writesize,
> +				    CMD_PROGRAM_PAGE))
> +
>  			ret = -EIO;
> +
>  		goto out;
>  	}
>  
> @@ -2494,6 +2714,8 @@ static int brcmnand_suspend(struct device *dev)
>  
>  	if (has_flash_dma(ctrl))
>  		ctrl->flash_dma_mode = flash_dma_readl(ctrl, FLASH_DMA_MODE);
> +	else if (has_edu(ctrl))
> +		ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
>  
>  	return 0;
>  }
> @@ -2508,6 +2730,14 @@ static int brcmnand_resume(struct device *dev)
>  		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
>  	}
>  
> +	if (has_edu(ctrl))
> +		ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
> +	else {
> +		edu_writel(ctrl, EDU_CONFIG, ctrl->edu_config);
> +		edu_readl(ctrl, EDU_CONFIG);
> +		brcmnand_edu_init(ctrl);
> +	}
> +
>  	brcmnand_write_reg(ctrl, BRCMNAND_CS_SELECT, ctrl->nand_cs_nand_select);
>  	brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
>  	brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
> @@ -2553,6 +2783,52 @@ MODULE_DEVICE_TABLE(of, brcmnand_of_match);
>  /***********************************************************************
>   * Platform driver setup (per controller)
>   ***********************************************************************/
> +static int brcmnand_edu_setup(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
> +	struct resource *res;
> +	int ret;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-edu");
> +	if (res) {
> +		ctrl->edu_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(ctrl->edu_base))
> +			return PTR_ERR(ctrl->edu_base);
> +
> +		ctrl->edu_offsets = edu_regs;
> +
> +		edu_writel(ctrl, EDU_CONFIG, EDU_CONFIG_MODE_NAND |
> +			   EDU_CONFIG_SWAP_CFG);
> +		edu_readl(ctrl, EDU_CONFIG);
> +
> +		/* initialize edu */
> +		brcmnand_edu_init(ctrl);
> +
> +		ctrl->edu_irq = platform_get_irq_optional(pdev, 1);
> +		if ((int)ctrl->edu_irq < 0) {

Why not declaring it as an int directly? I think it's preferred.

> +			dev_warn(dev,
> +				 "FLASH EDU enabled, using ctlrdy irq\n");
> +		} else {
> +			ret = devm_request_irq(dev, ctrl->edu_irq,
> +					       brcmnand_edu_irq, 0,
> +					       "brcmnand-edu", ctrl);
> +			if (ret < 0) {
> +				dev_err(ctrl->dev, "can't allocate IRQ %d: error %d\n",
> +					ctrl->edu_irq, ret);
> +				return ret;
> +			}
> +
> +			dev_info(dev, "FLASH EDU enabled using irq %u\n",
> +				 ctrl->edu_irq);
> +		}
> +
> +		/* set the appropriate edu transfer function to call */
> +		ctrl->dma_trans = brcmnand_edu_trans;
> +	}
> +
> +	return 0;
> +}
>  
>  int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  {
> @@ -2578,6 +2854,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  
>  	init_completion(&ctrl->done);
>  	init_completion(&ctrl->dma_done);
> +	init_completion(&ctrl->edu_done);
>  	nand_controller_init(&ctrl->controller);
>  	ctrl->controller.ops = &brcmnand_controller_ops;
>  	INIT_LIST_HEAD(&ctrl->host_list);
> @@ -2623,6 +2900,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  				ctrl->reg_offsets[BRCMNAND_FC_BASE];
>  	}
>  
> +	ctrl->dma_trans = NULL;
>  	/* FLASH_DMA */
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
>  	if (res) {
> @@ -2665,6 +2943,12 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  		}
>  
>  		dev_info(dev, "enabling FLASH_DMA\n");
> +		/* set the appropriate flash dma transfer function to call */
> +		ctrl->dma_trans = brcmnand_dma_trans;
> +	} else	{
> +		ret = brcmnand_edu_setup(pdev);
> +		if (ret < 0)
> +			goto err;

Nitpicking: you could drop the initialization of dma_trans to NULL and
assign ctrl->dma_trans in both cases of this if/else block (by moving
it out of the brcmnand_edu_setup()). I think it enhances readability.

>  	}
>  
>  	/* Disable automatic device ID config, direct addressing */


Thanks,
Miquèl

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

Re: [PATCH V2 3/3] mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers

[Kamal Dasu]

On Wed, Jan 22, 2020 at 3:50 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Kamal,
>
> I'm fine with the patch, nitpicking below :)
>
> Kamal Dasu <kdasu.kdev@gmail.com> wrote on Tue, 21 Jan 2020 15:00:08
> -0500:
>
> > Legacy mips soc platforms that have controller v5.0 and 6.0 use
> > flash-edu block for dma transfers. This change adds support for
> > nand dma transfers using the EDU block.
> >
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 296 ++++++++++++++++++++++-
> >  1 file changed, 290 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index 1a66b1cd51c0..61347607f1da 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -102,6 +102,45 @@ struct brcm_nand_dma_desc {
> >  #define NAND_CTRL_RDY                        (INTFC_CTLR_READY | INTFC_FLASH_READY)
> >  #define NAND_POLL_STATUS_TIMEOUT_MS  100
> >
> > +#define EDU_CMD_WRITE          0x00
> > +#define EDU_CMD_READ           0x01
> > +#define EDU_STATUS_ACTIVE      BIT(0)
> > +#define EDU_ERR_STATUS_ERRACK  BIT(0)
> > +#define EDU_DONE_MASK                GENMASK(1, 0)
> > +
> > +#define EDU_CONFIG_MODE_NAND   BIT(0)
> > +#define EDU_CONFIG_SWAP_BYTE   BIT(1)
> > +#ifdef CONFIG_CPU_BIG_ENDIAN
> > +#define EDU_CONFIG_SWAP_CFG     EDU_CONFIG_SWAP_BYTE
> > +#else
> > +#define EDU_CONFIG_SWAP_CFG     0
> > +#endif
> > +
> > +/* edu registers */
> > +enum edu_reg {
> > +     EDU_CONFIG = 0,
> > +     EDU_DRAM_ADDR,
> > +     EDU_EXT_ADDR,
> > +     EDU_LENGTH,
> > +     EDU_CMD,
> > +     EDU_STOP,
> > +     EDU_STATUS,
> > +     EDU_DONE,
> > +     EDU_ERR_STATUS,
> > +};
> > +
> > +static const u16  edu_regs[] = {
> > +     [EDU_CONFIG] = 0x00,
> > +     [EDU_DRAM_ADDR] = 0x04,
> > +     [EDU_EXT_ADDR] = 0x08,
> > +     [EDU_LENGTH] = 0x0c,
> > +     [EDU_CMD] = 0x10,
> > +     [EDU_STOP] = 0x14,
> > +     [EDU_STATUS] = 0x18,
> > +     [EDU_DONE] = 0x1c,
> > +     [EDU_ERR_STATUS] = 0x20,
> > +};
>
> Why not defining the offsets in the enum directly?
>

EDU is separate block and following the convention used fr the rest of
the driver. Would prefer to keep it this way.

> > +
> >  /* flash_dma registers */
> >  enum flash_dma_reg {
> >       FLASH_DMA_REVISION = 0,
> > @@ -167,6 +206,8 @@ enum {
> >       BRCMNAND_HAS_WP                         = BIT(3),
> >  };
> >
> > +struct brcmnand_host;
> > +
> >  struct brcmnand_controller {
> >       struct device           *dev;
> >       struct nand_controller  controller;
> > @@ -185,17 +226,32 @@ struct brcmnand_controller {
> >
> >       int                     cmd_pending;
> >       bool                    dma_pending;
> > +     bool                    edu_pending;
> >       struct completion       done;
> >       struct completion       dma_done;
> > +     struct completion       edu_done;
> >
> >       /* List of NAND hosts (one for each chip-select) */
> >       struct list_head host_list;
> >
> > +     /* EDU info, per-transaction */
> > +     const u16               *edu_offsets;
> > +     void __iomem            *edu_base;
> > +     unsigned int            edu_irq;
> > +     int                     edu_count;
> > +     u64                     edu_dram_addr;
> > +     u32                     edu_ext_addr;
> > +     u32                     edu_cmd;
> > +     u32                     edu_config;
> > +
> >       /* flash_dma reg */
> >       const u16               *flash_dma_offsets;
> >       struct brcm_nand_dma_desc *dma_desc;
> >       dma_addr_t              dma_pa;
> >
> > +     int (*dma_trans)(struct brcmnand_host *host, u64 addr, u32 *buf,
> > +                      u32 len, u8 dma_cmd);
> > +
> >       /* in-memory cache of the FLASH_CACHE, used only for some commands */
> >       u8                      flash_cache[FC_BYTES];
> >
> > @@ -216,6 +272,7 @@ struct brcmnand_controller {
> >       u32                     nand_cs_nand_xor;
> >       u32                     corr_stat_threshold;
> >       u32                     flash_dma_mode;
> > +     u32                     flash_edu_mode;
> >       bool                    pio_poll_mode;
> >  };
> >
> > @@ -657,6 +714,22 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
> >       __raw_writel(val, ctrl->nand_fc + word * 4);
> >  }
> >
> > +static inline void edu_writel(struct brcmnand_controller *ctrl,
> > +                           enum edu_reg reg, u32 val)
> > +{
> > +     u16 offs = ctrl->edu_offsets[reg];
> > +
> > +     brcmnand_writel(val, ctrl->edu_base + offs);
> > +}
> > +
> > +static inline u32 edu_readl(struct brcmnand_controller *ctrl,
> > +                         enum edu_reg reg)
> > +{
> > +     u16 offs = ctrl->edu_offsets[reg];
> > +
> > +     return brcmnand_readl(ctrl->edu_base + offs);
> > +}
> > +
> >  static void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
> >  {
> >
> > @@ -926,6 +999,16 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
> >       return ctrl->flash_dma_base;
> >  }
> >
> > +static inline bool has_edu(struct brcmnand_controller *ctrl)
> > +{
> > +     return ctrl->edu_base;
> > +}
> > +
> > +static inline bool use_dma(struct brcmnand_controller *ctrl)
> > +{
> > +     return has_flash_dma(ctrl) || has_edu(ctrl);
> > +}
> > +
> >  static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl)
> >  {
> >       if (ctrl->pio_poll_mode)
> > @@ -1299,6 +1382,52 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
> >       return tbytes;
> >  }
> >
> > +static void brcmnand_edu_init(struct brcmnand_controller *ctrl)
> > +{
> > +     /* initialize edu */
> > +     edu_writel(ctrl, EDU_ERR_STATUS, 0);
> > +     edu_readl(ctrl, EDU_ERR_STATUS);
> > +     edu_writel(ctrl, EDU_DONE, 0);
> > +     edu_writel(ctrl, EDU_DONE, 0);
> > +     edu_writel(ctrl, EDU_DONE, 0);
> > +     edu_writel(ctrl, EDU_DONE, 0);
> > +     edu_readl(ctrl, EDU_DONE);
> > +}
> > +
> > +/* edu irq */
> > +static irqreturn_t brcmnand_edu_irq(int irq, void *data)
> > +{
> > +     struct brcmnand_controller *ctrl = data;
> > +
> > +     if (ctrl->edu_count) {
> > +             ctrl->edu_count--;
> > +             while (!(edu_readl(ctrl, EDU_DONE) & EDU_DONE_MASK))
> > +                     udelay(1);
> > +             edu_writel(ctrl, EDU_DONE, 0);
> > +             edu_readl(ctrl, EDU_DONE);
> > +     }
> > +
> > +     if (ctrl->edu_count) {
> > +             ctrl->edu_dram_addr += FC_BYTES;
> > +             ctrl->edu_ext_addr += FC_BYTES;
> > +
> > +             edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
> > +             edu_readl(ctrl, EDU_DRAM_ADDR);
> > +             edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
> > +             edu_readl(ctrl, EDU_EXT_ADDR);
> > +
> > +             mb(); /* flush previous writes */
> > +             edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
> > +             edu_readl(ctrl, EDU_CMD);
> > +
> > +             return IRQ_HANDLED;
> > +     }
> > +
> > +     complete(&ctrl->edu_done);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> >  static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
> >  {
> >       struct brcmnand_controller *ctrl = data;
> > @@ -1307,6 +1436,16 @@ static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
> >       if (ctrl->dma_pending)
> >               return IRQ_HANDLED;
> >
> > +     /* check if you need to piggy back on the ctrlrdy irq */
> > +     if (ctrl->edu_pending) {
> > +             if (irq == ctrl->irq && ((int)ctrl->edu_irq >= 0))
> > +     /* Discard interrupts while using dedicated edu irq */
> > +                     return IRQ_HANDLED;
> > +
> > +     /* no registered edu irq, call handler */
> > +             return brcmnand_edu_irq(irq, data);
> > +     }
> > +
> >       complete(&ctrl->done);
> >       return IRQ_HANDLED;
> >  }
> > @@ -1644,6 +1783,83 @@ static void brcmnand_write_buf(struct nand_chip *chip, const uint8_t *buf,
> >       }
> >  }
> >
> > +/**
> > + *  Kick EDU engine
> > + */
> > +static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf,
> > +                           u32 len, u8 cmd)
> > +{
> > +     struct brcmnand_controller *ctrl = host->ctrl;
> > +     unsigned long timeo = msecs_to_jiffies(200);
> > +     int ret = 0;
> > +     int dir = (cmd == CMD_PAGE_READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
> > +     u8 edu_cmd = (cmd == CMD_PAGE_READ ? EDU_CMD_READ : EDU_CMD_WRITE);
> > +     unsigned int trans = len >> FC_SHIFT;
> > +     dma_addr_t pa;
> > +
> > +     pa = dma_map_single(ctrl->dev, buf, len, dir);
> > +     if (dma_mapping_error(ctrl->dev, pa)) {
> > +             dev_err(ctrl->dev, "unable to map buffer for EDU DMA\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     ctrl->edu_pending = true;
> > +     mb(); /* flush previous writes */
>
> I'd prefer to have the barriers right after the IO access. I don't
> think it changes anything but that would ease understand what writes
> are you concerned about. Also, maybe you can use non _relaxed accessors
> as you usually writel/readl in raw, I don't think you actually need a
> barrier in this case.
>
Agree will remove the barrier.

> > +
> > +     ctrl->edu_dram_addr = pa;
> > +     ctrl->edu_ext_addr = addr;
> > +     ctrl->edu_cmd = edu_cmd;
> > +     ctrl->edu_count = trans;
> > +
> > +     edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr);
> > +     edu_readl(ctrl,  EDU_DRAM_ADDR);
> > +     edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
> > +     edu_readl(ctrl, EDU_EXT_ADDR);
> > +     edu_writel(ctrl, EDU_LENGTH, FC_BYTES);
> > +     edu_readl(ctrl, EDU_LENGTH);
> > +
> > +     /* Start edu engine */
> > +     mb(); /* flush previous writes */
> > +     edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd);
> > +     edu_readl(ctrl, EDU_CMD);
> > +
> > +     if (wait_for_completion_timeout(&ctrl->edu_done, timeo) <= 0) {
> > +             dev_err(ctrl->dev,
> > +                     "timeout waiting for EDU; status %#x, error status %#x\n",
> > +                     edu_readl(ctrl, EDU_STATUS),
> > +                     edu_readl(ctrl, EDU_ERR_STATUS));
> > +     }
> > +
> > +     dma_unmap_single(ctrl->dev, pa, len, dir);
> > +
> > +     /* for program page check NAND status */
> > +     if (((brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) &
> > +           INTFC_FLASH_STATUS) & NAND_STATUS_FAIL) &&
> > +         edu_cmd == EDU_CMD_WRITE) {
> > +             dev_info(ctrl->dev, "program failed at %llx\n",
> > +                      (unsigned long long)addr);
> > +             ret = -EIO;
> > +     }
> > +
> > +     /* Make sure the EDU status is clean */
> > +     if (edu_readl(ctrl, EDU_STATUS) & EDU_STATUS_ACTIVE)
> > +             dev_warn(ctrl->dev, "EDU still active: %#x\n",
> > +                      edu_readl(ctrl, EDU_STATUS));
> > +
> > +     if (unlikely(edu_readl(ctrl, EDU_ERR_STATUS) & EDU_ERR_STATUS_ERRACK)) {
> > +             dev_warn(ctrl->dev, "EDU RBUS error at addr %llx\n",
> > +                      (unsigned long long)addr);
> > +             ret = -EIO;
> > +     }
> > +
> > +     ctrl->edu_pending = false;
> > +     brcmnand_edu_init(ctrl);
> > +     edu_writel(ctrl, EDU_STOP, 0); /* force stop */
> > +     edu_readl(ctrl, EDU_STOP);
> > +
> > +     return ret;
> > +}
> > +
> >  /**
> >   * Construct a FLASH_DMA descriptor as part of a linked list. You must know the
> >   * following ahead of time:
> > @@ -1850,9 +2066,11 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> >  try_dmaread:
> >       brcmnand_clear_ecc_addr(ctrl);
> >
> > -     if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> > -             err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> > -                                          CMD_PAGE_READ);
> > +     if (ctrl->dma_trans && !oob && flash_dma_buf_ok(buf)) {
> > +             err = ctrl->dma_trans(host, addr, buf,
> > +                                   trans * FC_BYTES,
> > +                                   CMD_PAGE_READ);
> > +
> >               if (err) {
> >                       if (mtd_is_bitflip_or_eccerr(err))
> >                               err_addr = addr;
> > @@ -1988,10 +2206,12 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
> >       for (i = 0; i < ctrl->max_oob; i += 4)
> >               oob_reg_write(ctrl, i, 0xffffffff);
> >
> > -     if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> > -             if (brcmnand_dma_trans(host, addr, (u32 *)buf,
> > -                                     mtd->writesize, CMD_PROGRAM_PAGE))
> > +     if (use_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> > +             if (ctrl->dma_trans(host, addr, (u32 *)buf, mtd->writesize,
> > +                                 CMD_PROGRAM_PAGE))
> > +
> >                       ret = -EIO;
> > +
> >               goto out;
> >       }
> >
> > @@ -2494,6 +2714,8 @@ static int brcmnand_suspend(struct device *dev)
> >
> >       if (has_flash_dma(ctrl))
> >               ctrl->flash_dma_mode = flash_dma_readl(ctrl, FLASH_DMA_MODE);
> > +     else if (has_edu(ctrl))
> > +             ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
> >
> >       return 0;
> >  }
> > @@ -2508,6 +2730,14 @@ static int brcmnand_resume(struct device *dev)
> >               flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
> >       }
> >
> > +     if (has_edu(ctrl))
> > +             ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG);
> > +     else {
> > +             edu_writel(ctrl, EDU_CONFIG, ctrl->edu_config);
> > +             edu_readl(ctrl, EDU_CONFIG);
> > +             brcmnand_edu_init(ctrl);
> > +     }
> > +
> >       brcmnand_write_reg(ctrl, BRCMNAND_CS_SELECT, ctrl->nand_cs_nand_select);
> >       brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
> >       brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
> > @@ -2553,6 +2783,52 @@ MODULE_DEVICE_TABLE(of, brcmnand_of_match);
> >  /***********************************************************************
> >   * Platform driver setup (per controller)
> >   ***********************************************************************/
> > +static int brcmnand_edu_setup(struct platform_device *pdev)
> > +{
> > +     struct device *dev = &pdev->dev;
> > +     struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
> > +     struct resource *res;
> > +     int ret;
> > +
> > +     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-edu");
> > +     if (res) {
> > +             ctrl->edu_base = devm_ioremap_resource(dev, res);
> > +             if (IS_ERR(ctrl->edu_base))
> > +                     return PTR_ERR(ctrl->edu_base);
> > +
> > +             ctrl->edu_offsets = edu_regs;
> > +
> > +             edu_writel(ctrl, EDU_CONFIG, EDU_CONFIG_MODE_NAND |
> > +                        EDU_CONFIG_SWAP_CFG);
> > +             edu_readl(ctrl, EDU_CONFIG);
> > +
> > +             /* initialize edu */
> > +             brcmnand_edu_init(ctrl);
> > +
> > +             ctrl->edu_irq = platform_get_irq_optional(pdev, 1);
> > +             if ((int)ctrl->edu_irq < 0) {
>
> Why not declaring it as an int directly? I think it's preferred.
>

Ok will make this change

> > +                     dev_warn(dev,
> > +                              "FLASH EDU enabled, using ctlrdy irq\n");
> > +             } else {
> > +                     ret = devm_request_irq(dev, ctrl->edu_irq,
> > +                                            brcmnand_edu_irq, 0,
> > +                                            "brcmnand-edu", ctrl);
> > +                     if (ret < 0) {
> > +                             dev_err(ctrl->dev, "can't allocate IRQ %d: error %d\n",
> > +                                     ctrl->edu_irq, ret);
> > +                             return ret;
> > +                     }
> > +
> > +                     dev_info(dev, "FLASH EDU enabled using irq %u\n",
> > +                              ctrl->edu_irq);
> > +             }
> > +
> > +             /* set the appropriate edu transfer function to call */
> > +             ctrl->dma_trans = brcmnand_edu_trans;
> > +     }
> > +
> > +     return 0;
> > +}
> >
> >  int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
> >  {
> > @@ -2578,6 +2854,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
> >
> >       init_completion(&ctrl->done);
> >       init_completion(&ctrl->dma_done);
> > +     init_completion(&ctrl->edu_done);
> >       nand_controller_init(&ctrl->controller);
> >       ctrl->controller.ops = &brcmnand_controller_ops;
> >       INIT_LIST_HEAD(&ctrl->host_list);
> > @@ -2623,6 +2900,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
> >                               ctrl->reg_offsets[BRCMNAND_FC_BASE];
> >       }
> >
> > +     ctrl->dma_trans = NULL;
> >       /* FLASH_DMA */
> >       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
> >       if (res) {
> > @@ -2665,6 +2943,12 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
> >               }
> >
> >               dev_info(dev, "enabling FLASH_DMA\n");
> > +             /* set the appropriate flash dma transfer function to call */
> > +             ctrl->dma_trans = brcmnand_dma_trans;
> > +     } else  {
> > +             ret = brcmnand_edu_setup(pdev);
> > +             if (ret < 0)
> > +                     goto err;
>
> Nitpicking: you could drop the initialization of dma_trans to NULL and
> assign ctrl->dma_trans in both cases of this if/else block (by moving
> it out of the brcmnand_edu_setup()). I think it enhances readability.
>

Will make this change as well.

> >       }
> >
> >       /* Disable automatic device ID config, direct addressing */
>
>
> Thanks,
> Miquèl


Will send a V3 patch with the changes.

Thanks
Kamal

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

Re: [PATCH V2 1/3] dt: bindings: brcmnand: Add support for flash-edu

[Rob Herring]

On Tue, 21 Jan 2020 15:00:06 -0500, Kamal Dasu wrote:
> Adding support for EBI DMA unit (EDU).
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  .../devicetree/bindings/mtd/brcm,brcmnand.txt          | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

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