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

From: Miquel Raynal
Date: Thu Jan 09 2020 - 11:10:20 EST


Hi Kamal,

Kamal Dasu <kdasu.kdev@xxxxxxxxx> wrote on Wed, 20 Nov 2019 13:20:59
-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@xxxxxxxxx>

I don't have the patch 2/3 in my mailbox :-/ Can you please resend with
the right numbering or Cc myself on the 2nd?

Otherwise, minor comments below :)

> ---
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 269 ++++++++++++++++++++++-
> 1 file changed, 263 insertions(+), 6 deletions(-)

[...]

> +/* 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))
> + udelay(1);
> + edu_writel(ctrl, EDU_DONE, 0);
> + (void)edu_readl(ctrl, EDU_DONE);

Why this cast? (and all the others)

> + }
> +
> + 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);
> + (void)edu_readl(ctrl, EDU_DRAM_ADDR);
> + edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr);
> + (void)edu_readl(ctrl, EDU_EXT_ADDR);

[...]

>
> @@ -2561,6 +2767,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);
> @@ -2650,6 +2857,56 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
> dev_info(dev, "enabling FLASH_DMA\n");
> }
>
> + /* use EDU DMA only no FLASH_DMA present */
> + if (has_flash_dma(ctrl))
> + res = 0;
> + else
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "flash-edu");

Can we simplify this block?

> +
> + if (res) {

What about a dedicated helper to do the EDU configuration only?

> + 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);
> + (void)edu_readl(ctrl, EDU_CONFIG);
> +
> + /* initialize edu */
> + edu_writel(ctrl, EDU_ERR_STATUS, 0);
> + edu_writel(ctrl, EDU_DONE, 0);
> + (void)edu_readl(ctrl, EDU_DONE);
> +
> + ctrl->edu_irq = platform_get_irq(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(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 dma transfer function to call */
> + if (has_flash_dma(ctrl))
> + ctrl->dma_trans = brcmnand_dma_trans;
> + else if (has_edu(ctrl))
> + ctrl->dma_trans = brcmnand_edu_trans;
> + else
> + ctrl->dma_trans = NULL;
> +
> /* Disable automatic device ID config, direct addressing */
> brcmnand_rmw_reg(ctrl, BRCMNAND_CS_SELECT,
> CS_SELECT_AUTO_DEVICE_ID_CFG | 0xff, 0, 0);

Thanks,
MiquÃl