Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel

From: Jan Kiszka <jan.kiszka@web.de>
To: Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Mark Brown <broonie@kernel.org>
Cc: Boris Brezillon <bbrezillon@kernel.org>,
	Ramuthevar Vadivel Murugan 
	<vadivel.muruganx.ramuthevar@linux.intel.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-spi@vger.kernel.org, simon.k.r.goldschmidt@gmail.com,
	dinguyen@kernel.org, marex@denx.de
Subject: Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
Date: Sat, 22 Aug 2020 20:05:38 +0200	[thread overview]
Message-ID: <6c8d9bff-3a67-0e6c-d4d1-36b7ed5007b9@web.de> (raw)
In-Reply-To: <20200601070444.16923-6-vigneshr@ti.com>

On 01.06.20 09:04, Vignesh Raghavendra wrote:
> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
> is not yet probed. Currently driver just falls back to using PIO mode
> (which is less efficient) in this case. Instead return probe deferral
> error as is so that driver will be re probed once DMA provider is
> available.
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../mtd/spi-nor/controllers/cadence-quadspi.c  | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> index 608ca657ff7f5..0570ebca135a9 100644
> --- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> @@ -1169,7 +1169,7 @@ static void cqspi_controller_init(struct cqspi_st *cqspi)
>  	cqspi_controller_enable(cqspi, 1);
>  }
>
> -static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
> +static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>  {
>  	dma_cap_mask_t mask;
>
> @@ -1178,11 +1178,16 @@ static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>
>  	cqspi->rx_chan = dma_request_chan_by_mask(&mask);
>  	if (IS_ERR(cqspi->rx_chan)) {
> -		dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
> +		int ret = PTR_ERR(cqspi->rx_chan);
> +
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
>  		cqspi->rx_chan = NULL;
> -		return;
> +		return ret;
>  	}
>  	init_completion(&cqspi->rx_dma_complete);
> +
> +	return 0;
>  }
>
>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>  				mtd->name);
>
> -			if (!cqspi->rx_chan)
> -				cqspi_request_mmap_dma(cqspi);
> +			if (!cqspi->rx_chan) {
> +				ret = cqspi_request_mmap_dma(cqspi);
> +				if (ret == -EPROBE_DEFER)
> +					goto err;
> +			}
>  		}
>  	}
>
>

This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
the eval board yet).

Without that commit, read happens via PIO, and that works. With the
commit, the pattern

with open("out.bin", "wb") as out:
    pos = 0
    while pos < 2:
        with open("/dev/mtd0", "rb") as mtd:
           mtd.seek(pos * 0x10000)
           out.write(mtd.read(0x10000))
        pos += 1

gives the wrong result for the second block while

with open("out2.bin", "wb") as out:
    with open("/dev/mtd0", "rb") as mtd:
        out.write(mtd.read(0x20000))

(or "mtd_debug read") is fine.

What could be the reason? Our DTBs and k3-am654-base-board.dtb had some
deviations /wrt the ospi node, but aligning ours to the base board made
no difference.

Jan

[1] https://github.com/siemens/linux/commits/jan/iot2050