diff mbox

[v3,7/7] spi: imx: defer spi initialization, if DMA engine is pending

Message ID 1446388901-6073-8-git-send-email-anton.bondarenko.sama@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Anton Bondarenko Nov. 1, 2015, 2:41 p.m. UTC
From: Anton Bondarenko <anton_bondarenko@mentor.com>

If SPI device supports DMA mode, but DMA controller is not yet
available due to e.g. a delay in the corresponding kernel module
initialization, retry to initialize SPI driver later on instead of
falling back into PIO only mode.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
---
 drivers/spi/spi-imx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Sascha Hauer Nov. 5, 2015, 8:59 a.m. UTC | #1
On Sun, Nov 01, 2015 at 03:41:41PM +0100, Anton Bondarenko wrote:
> From: Anton Bondarenko <anton_bondarenko@mentor.com>
> 
> If SPI device supports DMA mode, but DMA controller is not yet
> available due to e.g. a delay in the corresponding kernel module
> initialization, retry to initialize SPI driver later on instead of
> falling back into PIO only mode.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>

Ok, so 6/7 is only part of the deferral story. Maybe squash the two
patches into one?

Sascha

> ---
>  drivers/spi/spi-imx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 6035ddd..d2ea731 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1354,9 +1354,15 @@ static int spi_imx_probe(struct platform_device *pdev)
>  	 * Only validated on i.mx6 now, can remove the constrain if validated on
>  	 * other chips.
>  	 */
> -	if (is_imx51_ecspi(spi_imx) &&
> -	    spi_imx_sdma_init(&pdev->dev, spi_imx, master))
> -		dev_err(&pdev->dev, "dma setup error,use pio instead\n");
> +	if (is_imx51_ecspi(spi_imx)) {
> +		ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master);
> +		if (ret == -EPROBE_DEFER)
> +			goto out_clk_put;
> +
> +		if (ret < 0)
> +			dev_err(&pdev->dev, "dma setup error %d, use pio\n",
> +				ret);
> +	}
>  
>  	spi_imx->devtype_data->reset(spi_imx);
>  
> -- 
> 2.6.2
> 
>
Anton Bondarenko Nov. 5, 2015, 4:18 p.m. UTC | #2
On 05.11.2015 09:59, Sascha Hauer wrote:
> On Sun, Nov 01, 2015 at 03:41:41PM +0100, Anton Bondarenko wrote:
>> From: Anton Bondarenko <anton_bondarenko@mentor.com>
>>
>> If SPI device supports DMA mode, but DMA controller is not yet
>> available due to e.g. a delay in the corresponding kernel module
>> initialization, retry to initialize SPI driver later on instead of
>> falling back into PIO only mode.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>> Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
>
> Ok, so 6/7 is only part of the deferral story. Maybe squash the two
> patches into one?
>
> Sascha

I would like to keep these two changes separately since I have a small 
concern about this exact change.

Let's assume such scenario:
- ECSPI configured to use DMA in device tree and DMA controller 
description specified. But there is no driver for this controller in kernel.
In this case dmaengine will always return -EDEFER and driver postpone 
spi-imx initialization. So finally there will be no SPI support even in 
PIO mode.
Or for example spi-imx will be compiled into kernel, but DMA controller 
driver will be on SPI-NOR based rootfs.

So the question is "Should we allow user to use incorrect combination of 
kernel configuration and DTS and work at least with some limitation?" Or 
should we try to guarantee device tree file requirement to use DMA.

Any thoughts?

Regards, Anton
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anton Bondarenko Nov. 14, 2015, 10:03 a.m. UTC | #3
On 05.11.2015 17:18, Anton Bondarenko wrote:
> On 05.11.2015 09:59, Sascha Hauer wrote:
>> On Sun, Nov 01, 2015 at 03:41:41PM +0100, Anton Bondarenko wrote:
>>> From: Anton Bondarenko <anton_bondarenko@mentor.com>
>>>
>>> If SPI device supports DMA mode, but DMA controller is not yet
>>> available due to e.g. a delay in the corresponding kernel module
>>> initialization, retry to initialize SPI driver later on instead of
>>> falling back into PIO only mode.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>>> Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
>>
>> Ok, so 6/7 is only part of the deferral story. Maybe squash the two
>> patches into one?
>>
>> Sascha
>
> I would like to keep these two changes separately since I have a small
> concern about this exact change.
>
> Let's assume such scenario:
> - ECSPI configured to use DMA in device tree and DMA controller
> description specified. But there is no driver for this controller in
> kernel.
> In this case dmaengine will always return -EDEFER and driver postpone
> spi-imx initialization. So finally there will be no SPI support even in
> PIO mode.
> Or for example spi-imx will be compiled into kernel, but DMA controller
> driver will be on SPI-NOR based rootfs.
>
> So the question is "Should we allow user to use incorrect combination of
> kernel configuration and DTS and work at least with some limitation?" Or
> should we try to guarantee device tree file requirement to use DMA.
>
> Any thoughts?
>
> Regards, Anton

Does anyone has other comments regarding this commit?

Regards, Anton
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 6035ddd..d2ea731 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1354,9 +1354,15 @@  static int spi_imx_probe(struct platform_device *pdev)
 	 * Only validated on i.mx6 now, can remove the constrain if validated on
 	 * other chips.
 	 */
-	if (is_imx51_ecspi(spi_imx) &&
-	    spi_imx_sdma_init(&pdev->dev, spi_imx, master))
-		dev_err(&pdev->dev, "dma setup error,use pio instead\n");
+	if (is_imx51_ecspi(spi_imx)) {
+		ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master);
+		if (ret == -EPROBE_DEFER)
+			goto out_clk_put;
+
+		if (ret < 0)
+			dev_err(&pdev->dev, "dma setup error %d, use pio\n",
+				ret);
+	}
 
 	spi_imx->devtype_data->reset(spi_imx);