[v5,11/15] dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
diff mbox series

Message ID 20190610081753.11422-12-yibin.gong@nxp.com
State New, archived
Headers show
Series
  • add ecspi ERR009165 for i.mx6/7 soc family
Related show

Commit Message

Robin Gong June 10, 2019, 8:17 a.m. UTC
From: Robin Gong <yibin.gong@nxp.com>

Because the number of ecspi1 rx event on i.mx8mm is 0, the condition
check ignore such special case without dma channel enabled, which caused
ecspi1 rx works failed. Actually, no need to check event_id0/event_id1
and replace checking 'event_id1' with 'DMA_DEV_TO_DEV', so that configure
event_id1 only in case DEV_TO_DEV.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/dma/imx-sdma.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Comments

Schrempf Frieder Sept. 25, 2019, 2:53 p.m. UTC | #1
On 25.09.19 13:26, Robin Gong wrote:
> On 2019-9-24 21:28 Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
>>
>> Hi Robin,
>>
>>> From: Robin Gong <yibin.gong at nxp.com>
>>>
>>> Because the number of ecspi1 rx event on i.mx8mm is 0, the condition
>>> check ignore such special case without dma channel enabled, which
>>> caused
>>> ecspi1 rx works failed. Actually, no need to check event_id0/event_id1
>>> and replace checking 'event_id1' with 'DMA_DEV_TO_DEV', so that
>>> configure
>>> event_id1 only in case DEV_TO_DEV.
>>>
>>> Signed-off-by: Robin Gong <yibin.gong at nxp.com>
>>> Acked-by: Vinod Koul <vkoul at kernel.org>
>>
>> I have a custom board with i.MX8MM and SPI flash on ecspi1. I'm currently
>> testing with v5.3 and as SPI didn't work, I tried two different things:
>>
>> 1. Removing 'dmas' and 'dma-names' from the ecspi1 node in imx8mm.dtsi,
>>      to use PIO instead of DMA. This works as expected and the driver
>>      boots with the following messages:
>>
>>          spi_imx 30820000.spi: dma setup error -19, use pio
>>          m25p80 spi0.0: mx25v8035f (1024 Kbytes)
>>          spi_imx 30820000.spi: probed
>>
>> 2. Applying your patchset and use DMA. In this case, the flash also
>>      works fine, but there are some error messages printed while booting:
>>
>>          spi_master spi0: I/O Error in DMA RX
>>          m25p80 spi0.0: SPI transfer failed: -110
>>          spi_master spi0: failed to transfer one message from queue
>>          m25p80 spi0.0: mx25v8035f (1024 Kbytes)
>>          spi_imx 30820000.spi: probed
>>
>> It would be great to get your patches merged and fix SPI + DMA, but for
>> i.MX8MM, we need to get rid of the error messages. Do you have an idea,
>> what's wrong?

> Could you check if the length of spi message is bigger than fifo_size during
> spi_nor probe? If yes, at that time maybe sdma firmware not loaded.
> if (transfer->len < spi_imx->devtype_data->fifo_size)

Indeed, most of the transfers triggered by the SPI NOR dirver are below 
fifo_size and work fine, but some are bigger. The transfers therefore 
try to use DMA, but the firmware is not loaded yet.

How is this supposed to work? Shouldn't all transfers use PIO as long as 
the SDMA firmware is not loaded yet?

(+ Cc: linux-spi@vger.kernel.org)
Robin Gong Sept. 27, 2019, 1:55 a.m. UTC | #2
On 2019-9-25 22:53 Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
> On 25.09.19 13:26, Robin Gong wrote:
> > On 2019-9-24 21:28 Schrempf Frieder <frieder.schrempf@kontron.de>
> wrote:
> >>
> >> Hi Robin,
> >>
> >>> From: Robin Gong <yibin.gong at nxp.com>
> >>>
> >>> Because the number of ecspi1 rx event on i.mx8mm is 0, the condition
> >>> check ignore such special case without dma channel enabled, which
> >>> caused
> >>> ecspi1 rx works failed. Actually, no need to check
> >>> event_id0/event_id1 and replace checking 'event_id1' with
> >>> 'DMA_DEV_TO_DEV', so that configure
> >>> event_id1 only in case DEV_TO_DEV.
> >>>
> >>> Signed-off-by: Robin Gong <yibin.gong at nxp.com>
> >>> Acked-by: Vinod Koul <vkoul at kernel.org>
> >>
> >> I have a custom board with i.MX8MM and SPI flash on ecspi1. I'm
> >> currently testing with v5.3 and as SPI didn't work, I tried two different
> things:
> >>
> >> 1. Removing 'dmas' and 'dma-names' from the ecspi1 node in
> imx8mm.dtsi,
> >>      to use PIO instead of DMA. This works as expected and the driver
> >>      boots with the following messages:
> >>
> >>          spi_imx 30820000.spi: dma setup error -19, use pio
> >>          m25p80 spi0.0: mx25v8035f (1024 Kbytes)
> >>          spi_imx 30820000.spi: probed
> >>
> >> 2. Applying your patchset and use DMA. In this case, the flash also
> >>      works fine, but there are some error messages printed while
> booting:
> >>
> >>          spi_master spi0: I/O Error in DMA RX
> >>          m25p80 spi0.0: SPI transfer failed: -110
> >>          spi_master spi0: failed to transfer one message from queue
> >>          m25p80 spi0.0: mx25v8035f (1024 Kbytes)
> >>          spi_imx 30820000.spi: probed
> >>
> >> It would be great to get your patches merged and fix SPI + DMA, but
> >> for i.MX8MM, we need to get rid of the error messages. Do you have an
> >> idea, what's wrong?
> 
> > Could you check if the length of spi message is bigger than fifo_size
> > during spi_nor probe? If yes, at that time maybe sdma firmware not loaded.
> > if (transfer->len < spi_imx->devtype_data->fifo_size)
> 
> Indeed, most of the transfers triggered by the SPI NOR dirver are below
> fifo_size and work fine, but some are bigger. The transfers therefore try to
> use DMA, but the firmware is not loaded yet.
> 
> How is this supposed to work? Shouldn't all transfers use PIO as long as the
> SDMA firmware is not loaded yet?
Yes, for ecspi should work with ram script, it's better check if sdma firmware
is ready in spi_imx_dma_configure(), need modification in sdma driver too.
I'll create another patch after this patch set accepted. 
> 
> (+ Cc: linux-spi@vger.kernel.org)

Patch
diff mbox series

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 525ca89..f7c150d 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1202,7 +1202,7 @@  static int sdma_config_channel(struct dma_chan *chan)
 	if ((sdmac->peripheral_type != IMX_DMATYPE_MEMORY) &&
 			(sdmac->peripheral_type != IMX_DMATYPE_DSP)) {
 		/* Handle multiple event channels differently */
-		if (sdmac->event_id1) {
+		if (sdmac->direction == DMA_DEV_TO_DEV) {
 			if (sdmac->peripheral_type == IMX_DMATYPE_ASRC_SP ||
 			    sdmac->peripheral_type == IMX_DMATYPE_ASRC)
 				sdma_set_watermarklevel_for_p2p(sdmac);
@@ -1370,9 +1370,9 @@  static void sdma_free_chan_resources(struct dma_chan *chan)
 
 	sdma_channel_synchronize(chan);
 
-	if (sdmac->event_id0)
-		sdma_event_disable(sdmac, sdmac->event_id0);
-	if (sdmac->event_id1)
+	sdma_event_disable(sdmac, sdmac->event_id0);
+
+	if (sdmac->direction == DMA_DEV_TO_DEV)
 		sdma_event_disable(sdmac, sdmac->event_id1);
 
 	sdmac->event_id0 = 0;
@@ -1670,13 +1670,11 @@  static int sdma_config(struct dma_chan *chan,
 	memcpy(&sdmac->slave_config, dmaengine_cfg, sizeof(*dmaengine_cfg));
 
 	/* Set ENBLn earlier to make sure dma request triggered after that */
-	if (sdmac->event_id0) {
-		if (sdmac->event_id0 >= sdmac->sdma->drvdata->num_events)
-			return -EINVAL;
-		sdma_event_enable(sdmac, sdmac->event_id0);
-	}
+	if (sdmac->event_id0 >= sdmac->sdma->drvdata->num_events)
+		return -EINVAL;
+	sdma_event_enable(sdmac, sdmac->event_id0);
 
-	if (sdmac->event_id1) {
+	if (sdmac->direction == DMA_DEV_TO_DEV) {
 		if (sdmac->event_id1 >= sdmac->sdma->drvdata->num_events)
 			return -EINVAL;
 		sdma_event_enable(sdmac, sdmac->event_id1);