diff mbox series

spi: imx: Don't expect DMA for i.MX{25,35,50,51,53} cspi devices

Message ID 20240508095610.2146640-2-u.kleine-koenig@pengutronix.de
State New
Headers show
Series spi: imx: Don't expect DMA for i.MX{25,35,50,51,53} cspi devices | expand

Commit Message

Uwe Kleine-König May 8, 2024, 9:56 a.m. UTC
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

Comments

Uwe Kleine-König May 10, 2024, 12:32 p.m. UTC | #1
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
Uwe Kleine-König May 10, 2024, 1:40 p.m. UTC | #2
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
diff mbox series

Patch

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,