| Message ID | 20240508095610.2146640-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | spi: imx: Don't expect DMA for i.MX{25,35,50,51,53} cspi devices | expand |
Hi Uwe, Thus wrote Uwe Kleine-König (u.kleine-koenig@pengutronix.de): > While in commit 2dd33f9cec90 ("spi: imx: support DMA for imx35") it was > claimed that DMA works on i.MX25, i.MX31 and i.MX35 the respective > device trees don't add DMA channels. The Reference manuals of i.MX31 and > i.MX25 also don't mention the CSPI core being DMA capable. (I didn't > check the others.) If I'm not mistaken, the imx25 reference manual https://www.nxp.com/docs/en/reference-manual/IMX25RM.pdf does say that CSPI has DMA support. Section 18.1.1 (Features) lists DMA as one of the features. There's also DMA events (section 3) for CSPI-1/2/3 RX, TX. I'd have to dig up the hardware to test again if this is actually working... Martin
Hello Martin, On Wed, May 08, 2024 at 03:44:53PM +0200, Martin Kaiser wrote: > Thus wrote Uwe Kleine-König (u.kleine-koenig@pengutronix.de): > > While in commit 2dd33f9cec90 ("spi: imx: support DMA for imx35") it was > > claimed that DMA works on i.MX25, i.MX31 and i.MX35 the respective > > device trees don't add DMA channels. The Reference manuals of i.MX31 and > > i.MX25 also don't mention the CSPI core being DMA capable. (I didn't > > check the others.) > > If I'm not mistaken, the imx25 reference manual > > https://www.nxp.com/docs/en/reference-manual/IMX25RM.pdf > > does say that CSPI has DMA support. Section 18.1.1 (Features) lists DMA as one > of the features. There's also DMA events (section 3) for CSPI-1/2/3 RX, TX. Oh indeed. I don't know what made me claim that DMA isn't mentioned in the reference manual. Maybe I looked at the i2c chapter. I now did: diff --git a/arch/arm/boot/dts/nxp/imx/imx25.dtsi b/arch/arm/boot/dts/nxp/imx/imx25.dtsi index 4a85684deff8..710b28a41bae 100644 --- a/arch/arm/boot/dts/nxp/imx/imx25.dtsi +++ b/arch/arm/boot/dts/nxp/imx/imx25.dtsi @@ -190,6 +190,8 @@ spi1: spi@43fa4000 { reg = <0x43fa4000 0x4000>; clocks = <&clks 78>, <&clks 78>; clock-names = "ipg", "per"; + dmas = <&sdma 8 1 0>, <&sdma 9 1 0>; + dma-names = "rx", "tx"; interrupts = <14>; status = "disabled"; }; @@ -229,6 +231,8 @@ spi3: spi@50004000 { interrupts = <0>; clocks = <&clks 80>, <&clks 80>; clock-names = "ipg", "per"; + dmas = <&sdma 34 1 0>, <&sdma 35 1 0>; + dma-names = "rx", "tx"; status = "disabled"; }; @@ -257,6 +261,8 @@ spi2: spi@50010000 { reg = <0x50010000 0x4000>; clocks = <&clks 79>, <&clks 79>; clock-names = "ipg", "per"; + dmas = <&sdma 6 1 0>, <&sdma 7 1 0>; + dma-names = "rx", "tx"; interrupts = <13>; status = "disabled"; }; And spi still works. I see an issue, but I think that's orthogonal to adding DMA. Will send a formal patch when I debugged that. Thanks for your feedback, Uwe
Hello, On Fri, May 10, 2024 at 02:32:56PM +0200, Uwe Kleine-König wrote: > On Wed, May 08, 2024 at 03:44:53PM +0200, Martin Kaiser wrote: > > Thus wrote Uwe Kleine-König (u.kleine-koenig@pengutronix.de): > > > While in commit 2dd33f9cec90 ("spi: imx: support DMA for imx35") it was > > > claimed that DMA works on i.MX25, i.MX31 and i.MX35 the respective > > > device trees don't add DMA channels. The Reference manuals of i.MX31 and > > > i.MX25 also don't mention the CSPI core being DMA capable. (I didn't > > > check the others.) > > > > If I'm not mistaken, the imx25 reference manual > > > > https://www.nxp.com/docs/en/reference-manual/IMX25RM.pdf > > > > does say that CSPI has DMA support. Section 18.1.1 (Features) lists DMA as one > > of the features. There's also DMA events (section 3) for CSPI-1/2/3 RX, TX. > > Oh indeed. I don't know what made me claim that DMA isn't mentioned in > the reference manual. Maybe I looked at the i2c chapter. > > I now did: > > diff --git a/arch/arm/boot/dts/nxp/imx/imx25.dtsi b/arch/arm/boot/dts/nxp/imx/imx25.dtsi > index 4a85684deff8..710b28a41bae 100644 > --- a/arch/arm/boot/dts/nxp/imx/imx25.dtsi > +++ b/arch/arm/boot/dts/nxp/imx/imx25.dtsi > @@ -190,6 +190,8 @@ spi1: spi@43fa4000 { > reg = <0x43fa4000 0x4000>; > clocks = <&clks 78>, <&clks 78>; > clock-names = "ipg", "per"; > + dmas = <&sdma 8 1 0>, <&sdma 9 1 0>; > + dma-names = "rx", "tx"; > interrupts = <14>; > status = "disabled"; > }; > @@ -229,6 +231,8 @@ spi3: spi@50004000 { > interrupts = <0>; > clocks = <&clks 80>, <&clks 80>; > clock-names = "ipg", "per"; > + dmas = <&sdma 34 1 0>, <&sdma 35 1 0>; > + dma-names = "rx", "tx"; > status = "disabled"; > }; > > @@ -257,6 +261,8 @@ spi2: spi@50010000 { > reg = <0x50010000 0x4000>; > clocks = <&clks 79>, <&clks 79>; > clock-names = "ipg", "per"; > + dmas = <&sdma 6 1 0>, <&sdma 7 1 0>; > + dma-names = "rx", "tx"; > interrupts = <13>; > status = "disabled"; > }; Additionally to the above I now did: diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index c3e5cee18bea..74da1a965a0d 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -1428,12 +1428,9 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, if (ret) goto dma_failure_no_start; - if (!spi_imx->devtype_data->setup_wml) { - dev_err(spi_imx->dev, "No setup_wml()?\n"); - ret = -EINVAL; - goto dma_failure_no_start; + if (spi_imx->devtype_data->setup_wml) { + spi_imx->devtype_data->setup_wml(spi_imx); } - spi_imx->devtype_data->setup_wml(spi_imx); /* * The TX DMA setup starts the transfer, so make sure RX is configured because there is no .setup_wml() callback for i.MX25 in imx31_cspi_devtype_data and the DMA register is already setup in mx31_prepare_transfer(). Without this change DMA isn't used. However this breaks SPI transfers, when I try to read out an MRAM I get: root@ecu02:~ hexdump -C /dev/mtd4 [ 71.813807] spi_imx 43fa4000.spi: I/O Error in DMA TX [ 71.819173] spi-nor spi0.2: SPI transfer failed: -110 [ 71.829129] spi_master spi0: failed to transfer one message from queue [ 71.843962] spi_master spi0: noqueue transfer failed So it would indeed be interesting if you ever managed to use DMA on i.MX25. Best regards Uwe
Hello, Thus wrote Uwe Kleine-König (u.kleine-koenig@pengutronix.de): > Hello, > On Fri, May 10, 2024 at 02:32:56PM +0200, Uwe Kleine-König wrote: > > On Wed, May 08, 2024 at 03:44:53PM +0200, Martin Kaiser wrote: > > > Thus wrote Uwe Kleine-König (u.kleine-koenig@pengutronix.de): > > > > While in commit 2dd33f9cec90 ("spi: imx: support DMA for imx35") it was > > > > claimed that DMA works on i.MX25, i.MX31 and i.MX35 the respective > > > > device trees don't add DMA channels. The Reference manuals of i.MX31 and > > > > i.MX25 also don't mention the CSPI core being DMA capable. (I didn't > > > > check the others.) > > > If I'm not mistaken, the imx25 reference manual > > > https://www.nxp.com/docs/en/reference-manual/IMX25RM.pdf > > > does say that CSPI has DMA support. Section 18.1.1 (Features) lists DMA as one > > > of the features. There's also DMA events (section 3) for CSPI-1/2/3 RX, TX. > > Oh indeed. I don't know what made me claim that DMA isn't mentioned in > > the reference manual. Maybe I looked at the i2c chapter. > > I now did: > > diff --git a/arch/arm/boot/dts/nxp/imx/imx25.dtsi b/arch/arm/boot/dts/nxp/imx/imx25.dtsi > > index 4a85684deff8..710b28a41bae 100644 > > --- a/arch/arm/boot/dts/nxp/imx/imx25.dtsi > > +++ b/arch/arm/boot/dts/nxp/imx/imx25.dtsi > > @@ -190,6 +190,8 @@ spi1: spi@43fa4000 { > > reg = <0x43fa4000 0x4000>; > > clocks = <&clks 78>, <&clks 78>; > > clock-names = "ipg", "per"; > > + dmas = <&sdma 8 1 0>, <&sdma 9 1 0>; > > + dma-names = "rx", "tx"; > > interrupts = <14>; > > status = "disabled"; > > }; > > @@ -229,6 +231,8 @@ spi3: spi@50004000 { > > interrupts = <0>; > > clocks = <&clks 80>, <&clks 80>; > > clock-names = "ipg", "per"; > > + dmas = <&sdma 34 1 0>, <&sdma 35 1 0>; > > + dma-names = "rx", "tx"; > > status = "disabled"; > > }; > > @@ -257,6 +261,8 @@ spi2: spi@50010000 { > > reg = <0x50010000 0x4000>; > > clocks = <&clks 79>, <&clks 79>; > > clock-names = "ipg", "per"; > > + dmas = <&sdma 6 1 0>, <&sdma 7 1 0>; > > + dma-names = "rx", "tx"; > > interrupts = <13>; > > status = "disabled"; > > }; > Additionally to the above I now did: > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index c3e5cee18bea..74da1a965a0d 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -1428,12 +1428,9 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, > if (ret) > goto dma_failure_no_start; > - if (!spi_imx->devtype_data->setup_wml) { > - dev_err(spi_imx->dev, "No setup_wml()?\n"); > - ret = -EINVAL; > - goto dma_failure_no_start; > + if (spi_imx->devtype_data->setup_wml) { > + spi_imx->devtype_data->setup_wml(spi_imx); > } > - spi_imx->devtype_data->setup_wml(spi_imx); > /* > * The TX DMA setup starts the transfer, so make sure RX is configured > because there is no .setup_wml() callback for i.MX25 in > imx31_cspi_devtype_data and the DMA register is already setup in > mx31_prepare_transfer(). Without this change DMA isn't used. > However this breaks SPI transfers, when I try to read out an MRAM I get: > root@ecu02:~ hexdump -C /dev/mtd4 > [ 71.813807] spi_imx 43fa4000.spi: I/O Error in DMA TX > [ 71.819173] spi-nor spi0.2: SPI transfer failed: -110 > [ 71.829129] spi_master spi0: failed to transfer one message from queue > [ 71.843962] spi_master spi0: noqueue transfer failed > So it would indeed be interesting if you ever managed to use DMA on > i.MX25. I believe so. Looking into my notes from 2016 (when I last tried this), I got the -110 error when the SMC bit in CONREG wasn't set. But this should now be done in mx31_prepare_transfer if DMA is used... I'll try to set up a test some time next week. Best regards, Martin
Thus wrote Martin Kaiser (martin@kaiser.cx): > > However this breaks SPI transfers, when I try to read out an MRAM I get: > > root@ecu02:~ hexdump -C /dev/mtd4 > > [ 71.813807] spi_imx 43fa4000.spi: I/O Error in DMA TX > > [ 71.819173] spi-nor spi0.2: SPI transfer failed: -110 > > [ 71.829129] spi_master spi0: failed to transfer one message from queue > > [ 71.843962] spi_master spi0: noqueue transfer failed > > So it would indeed be interesting if you ever managed to use DMA on > > i.MX25. > I believe so. Looking into my notes from 2016 (when I last tried this), I got > the -110 error when the SMC bit in CONREG wasn't set. But this should now be > done in mx31_prepare_transfer if DMA is used... > I'll try to set up a test some time next week. I can now reproduce the dma tx error. My test board uses a dummy spidev device on cspi1 and configures loopback on the cspi1 controller (disabling loopback makes no difference). I'm relatively sure that this setup did work back in 2016/17. Commit 24bb244e02a6 ("ARM: i.MX25: globally disable supervisor protect") was required before any DMA transfers started working on imx25. As far as I can see, this code is still present. I'll look into this a bit more as time permits. Martin
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index f4006c82f867..4de5476f79b8 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -1060,7 +1060,7 @@ static struct spi_imx_devtype_data imx35_cspi_devtype_data = { .rx_available = mx31_rx_available, .reset = mx31_reset, .fifo_size = 8, - .has_dmamode = true, + .has_dmamode = false, .dynamic_burst = false, .has_targetmode = false, .devtype = IMX35_CSPI,
While in commit 2dd33f9cec90 ("spi: imx: support DMA for imx35") it was claimed that DMA works on i.MX25, i.MX31 and i.MX35 the respective device trees don't add DMA channels. The Reference manuals of i.MX31 and i.MX25 also don't mention the CSPI core being DMA capable. (I didn't check the others.) Since commit e267a5b3ec59 ("spi: spi-imx: Use dev_err_probe for failed DMA channel requests") this results in an error message spi_imx 43fa4000.spi: error -ENODEV: can't get the TX DMA channel! during boot. However that isn't fatal and the driver gets loaded just fine, just without using DMA. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/spi/spi-imx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: e7b4ef8fffaca247809337bb78daceb406659f2d