| Message ID | 20240508095610.2146640-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5d0c35feea339e4a3a9c9e99731e4d49ad5ee329 |
| 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
Hello Martin, On 15.05.2024 21:26:17, Martin Kaiser wrote: > 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. have you found the time to look into this? regards, Marc
Hello Marc, Thus wrote Marc Kleine-Budde (mkl@pengutronix.de): > Hello Martin, > On 15.05.2024 21:26:17, Martin Kaiser wrote: > > 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. > have you found the time to look into this? I did have another look but couldn't get DMA to work :-( I got to the point where tx seemed to work and rx failed. The tx callback function was called from the dma core and rx timed out. However, looking at the mosi, miso and clk signals, there was no clock and no data transmission on the bus. The dev_dbg prints in imx-sdma didn't show anything that looked wrong... I'll have another go some time soon. In the meantime, feel free to merge this patch and disable DMA on imx25. Best regards, Martin
On Wed, 08 May 2024 11:56:10 +0200, Uwe Kleine-König wrote: > 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 > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: imx: Don't expect DMA for i.MX{25,35,50,51,53} cspi devices commit: 5d0c35feea339e4a3a9c9e99731e4d49ad5ee329 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
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