[PATCH] mmc: spi: disable dma mapping code, as it does not work

[PATCH] mmc: spi: disable dma mapping code, as it does not work

[Phil Elwell]

since:
	commit 0589342c27944e50ebd7a54f5215002b6598b748
	Author: Rob Herring <rob.herring@calxeda.com>
	Date:   Tue Oct 29 23:36:46 2013 -0500

	     of: set dma_mask to point to coherent_dma_mask

	     Platform devices created by DT code don't initialize dma_mask pointer to
	     anything. Set it to coherent_dma_mask by default if the architecture
	     code has not set it.

the mmc-spi driver no longer detects the sd-cards any longer.

This patch disables the offending section in mmc_spi
so that the driver works again and detects the sd card normally.

There is no real negative impact as the spi-framework
does no longer rely on driver-provided DMA mappings but
maps the transfers transparently.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Submitted-by: Phil Elwell <phil@raspberrypi.org>
---
 drivers/mmc/host/mmc_spi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index ae19d83bb9de0..16946e2513afa 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1387,6 +1387,16 @@ static int mmc_spi_probe(struct spi_device *spi)
 	if (!host->data)
 		goto fail_nobuf1;
 
+	/* for some reason mmc_spi does not work with this enabled
+	 * it is not needed anyway, as the SPI framework does DMA
+	 * transfers now automatically
+	 * the patch that introduced this was:
+	 * commit 0589342c27944e50ebd7a54f5215002b6598b748
+	 * Author: Rob Herring <rob.herring@calxeda.com>
+	 * Date:   Tue Oct 29 23:36:46 2013 -0500
+	 *      of: set dma_mask to point to coherent_dma_mask
+	 */
+#if 0
 	if (spi->master->dev.parent->dma_mask) {
 		struct device	*dev = spi->master->dev.parent;
 
@@ -1402,6 +1412,7 @@ static int mmc_spi_probe(struct spi_device *spi)
 				host->data_dma, sizeof(*host->data),
 				DMA_BIDIRECTIONAL);
 	}
+#endif
 
 	/* setup message for status/busy readback */
 	spi_message_init(&host->readback);

Re: [PATCH] mmc: spi: disable dma mapping code, as it does not work

[Ulf Hansson]

On 22 May 2017 at 11:09, Phil Elwell <phil@raspberrypi.org> wrote:
> since:
>         commit 0589342c27944e50ebd7a54f5215002b6598b748
>         Author: Rob Herring <rob.herring@calxeda.com>
>         Date:   Tue Oct 29 23:36:46 2013 -0500
>
>              of: set dma_mask to point to coherent_dma_mask
>
>              Platform devices created by DT code don't initialize dma_mask pointer to
>              anything. Set it to coherent_dma_mask by default if the architecture
>              code has not set it.
>
> the mmc-spi driver no longer detects the sd-cards any longer.
>
> This patch disables the offending section in mmc_spi
> so that the driver works again and detects the sd card normally.
>
> There is no real negative impact as the spi-framework
> does no longer rely on driver-provided DMA mappings but
> maps the transfers transparently.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Submitted-by: Phil Elwell <phil@raspberrypi.org>
> ---
>  drivers/mmc/host/mmc_spi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> index ae19d83bb9de0..16946e2513afa 100644
> --- a/drivers/mmc/host/mmc_spi.c
> +++ b/drivers/mmc/host/mmc_spi.c
> @@ -1387,6 +1387,16 @@ static int mmc_spi_probe(struct spi_device *spi)
>         if (!host->data)
>                 goto fail_nobuf1;
>
> +       /* for some reason mmc_spi does not work with this enabled
> +        * it is not needed anyway, as the SPI framework does DMA
> +        * transfers now automatically
> +        * the patch that introduced this was:
> +        * commit 0589342c27944e50ebd7a54f5215002b6598b748
> +        * Author: Rob Herring <rob.herring@calxeda.com>
> +        * Date:   Tue Oct 29 23:36:46 2013 -0500
> +        *      of: set dma_mask to point to coherent_dma_mask
> +        */
> +#if 0
>         if (spi->master->dev.parent->dma_mask) {
>                 struct device   *dev = spi->master->dev.parent;
>
> @@ -1402,6 +1412,7 @@ static int mmc_spi_probe(struct spi_device *spi)
>                                 host->data_dma, sizeof(*host->data),
>                                 DMA_BIDIRECTIONAL);
>         }
> +#endif
>
>         /* setup message for status/busy readback */
>         spi_message_init(&host->readback);

Phil, I am not sure how to go with this.

Your change drops the dma implementation in the mmc_spi driver as
host->dma_dev now never gets assigned. I do think that makes sense as
the dma support should be implemented in the spi master driver and not
in the spi client driver, which currently is the case for mmc_spi.

Then perhaps a more proper change would be to remove the dma
implementation in mmc_spi altogether?

And one more thing, next time also cc the spi-list and Mark Brown, to
try to some broader input.

Kind regards
Uffe

Re: [PATCH] mmc: spi: disable dma mapping code, as it does not work

[Phil Elwell]

On 23/05/2017 08:58, Ulf Hansson wrote:
> On 22 May 2017 at 11:09, Phil Elwell <phil@raspberrypi.org> wrote:
>> since:
>>         commit 0589342c27944e50ebd7a54f5215002b6598b748
>>         Author: Rob Herring <rob.herring@calxeda.com>
>>         Date:   Tue Oct 29 23:36:46 2013 -0500
>>
>>              of: set dma_mask to point to coherent_dma_mask
>>
>>              Platform devices created by DT code don't initialize dma_mask pointer to
>>              anything. Set it to coherent_dma_mask by default if the architecture
>>              code has not set it.
>>
>> the mmc-spi driver no longer detects the sd-cards any longer.
>>
>> This patch disables the offending section in mmc_spi
>> so that the driver works again and detects the sd card normally.
>>
>> There is no real negative impact as the spi-framework
>> does no longer rely on driver-provided DMA mappings but
>> maps the transfers transparently.
>>
>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>> Submitted-by: Phil Elwell <phil@raspberrypi.org>
>> ---
>>  drivers/mmc/host/mmc_spi.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
>> index ae19d83bb9de0..16946e2513afa 100644
>> --- a/drivers/mmc/host/mmc_spi.c
>> +++ b/drivers/mmc/host/mmc_spi.c
>> @@ -1387,6 +1387,16 @@ static int mmc_spi_probe(struct spi_device *spi)
>>         if (!host->data)
>>                 goto fail_nobuf1;
>>
>> +       /* for some reason mmc_spi does not work with this enabled
>> +        * it is not needed anyway, as the SPI framework does DMA
>> +        * transfers now automatically
>> +        * the patch that introduced this was:
>> +        * commit 0589342c27944e50ebd7a54f5215002b6598b748
>> +        * Author: Rob Herring <rob.herring@calxeda.com>
>> +        * Date:   Tue Oct 29 23:36:46 2013 -0500
>> +        *      of: set dma_mask to point to coherent_dma_mask
>> +        */
>> +#if 0
>>         if (spi->master->dev.parent->dma_mask) {
>>                 struct device   *dev = spi->master->dev.parent;
>>
>> @@ -1402,6 +1412,7 @@ static int mmc_spi_probe(struct spi_device *spi)
>>                                 host->data_dma, sizeof(*host->data),
>>                                 DMA_BIDIRECTIONAL);
>>         }
>> +#endif
>>
>>         /* setup message for status/busy readback */
>>         spi_message_init(&host->readback);
> 
> Phil, I am not sure how to go with this.
> 
> Your change drops the dma implementation in the mmc_spi driver as
> host->dma_dev now never gets assigned. I do think that makes sense as
> the dma support should be implemented in the spi master driver and not
> in the spi client driver, which currently is the case for mmc_spi.
> 
> Then perhaps a more proper change would be to remove the dma
> implementation in mmc_spi altogether?

I don't have enough in-depth SPI knowledge to make that decision - I'm happy to go
with what the SPI experts recommend.

> And one more thing, next time also cc the spi-list and Mark Brown, to
> try to some broader input.

Thanks for the tip, Ulf - I just went with what scripts/get_maintainer.pl suggested.
Perhaps that list needs to be updated?

Phil

Re: [PATCH] mmc: spi: disable dma mapping code, as it does not work

[Ulf Hansson]

On 23 May 2017 at 12:11, Phil Elwell <phil@raspberrypi.org> wrote:
> On 23/05/2017 08:58, Ulf Hansson wrote:
>> On 22 May 2017 at 11:09, Phil Elwell <phil@raspberrypi.org> wrote:
>>> since:
>>>         commit 0589342c27944e50ebd7a54f5215002b6598b748
>>>         Author: Rob Herring <rob.herring@calxeda.com>
>>>         Date:   Tue Oct 29 23:36:46 2013 -0500
>>>
>>>              of: set dma_mask to point to coherent_dma_mask
>>>
>>>              Platform devices created by DT code don't initialize dma_mask pointer to
>>>              anything. Set it to coherent_dma_mask by default if the architecture
>>>              code has not set it.
>>>
>>> the mmc-spi driver no longer detects the sd-cards any longer.
>>>
>>> This patch disables the offending section in mmc_spi
>>> so that the driver works again and detects the sd card normally.
>>>
>>> There is no real negative impact as the spi-framework
>>> does no longer rely on driver-provided DMA mappings but
>>> maps the transfers transparently.
>>>
>>> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>>> Submitted-by: Phil Elwell <phil@raspberrypi.org>
>>> ---
>>>  drivers/mmc/host/mmc_spi.c | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
>>> index ae19d83bb9de0..16946e2513afa 100644
>>> --- a/drivers/mmc/host/mmc_spi.c
>>> +++ b/drivers/mmc/host/mmc_spi.c
>>> @@ -1387,6 +1387,16 @@ static int mmc_spi_probe(struct spi_device *spi)
>>>         if (!host->data)
>>>                 goto fail_nobuf1;
>>>
>>> +       /* for some reason mmc_spi does not work with this enabled
>>> +        * it is not needed anyway, as the SPI framework does DMA
>>> +        * transfers now automatically
>>> +        * the patch that introduced this was:
>>> +        * commit 0589342c27944e50ebd7a54f5215002b6598b748
>>> +        * Author: Rob Herring <rob.herring@calxeda.com>
>>> +        * Date:   Tue Oct 29 23:36:46 2013 -0500
>>> +        *      of: set dma_mask to point to coherent_dma_mask
>>> +        */
>>> +#if 0
>>>         if (spi->master->dev.parent->dma_mask) {
>>>                 struct device   *dev = spi->master->dev.parent;
>>>
>>> @@ -1402,6 +1412,7 @@ static int mmc_spi_probe(struct spi_device *spi)
>>>                                 host->data_dma, sizeof(*host->data),
>>>                                 DMA_BIDIRECTIONAL);
>>>         }
>>> +#endif
>>>
>>>         /* setup message for status/busy readback */
>>>         spi_message_init(&host->readback);
>>
>> Phil, I am not sure how to go with this.
>>
>> Your change drops the dma implementation in the mmc_spi driver as
>> host->dma_dev now never gets assigned. I do think that makes sense as
>> the dma support should be implemented in the spi master driver and not
>> in the spi client driver, which currently is the case for mmc_spi.
>>
>> Then perhaps a more proper change would be to remove the dma
>> implementation in mmc_spi altogether?
>
> I don't have enough in-depth SPI knowledge to make that decision - I'm happy to go
> with what the SPI experts recommend.

Okay. Let's see.

>
>> And one more thing, next time also cc the spi-list and Mark Brown, to
>> try to some broader input.
>
> Thanks for the tip, Ulf - I just went with what scripts/get_maintainer.pl suggested.
> Perhaps that list needs to be updated?

No, this is more a special case I would say.

Kind regards
Uffe