linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] spi: spi-cadence-quadspi: Fix mapping of buffers for DMA reads
@ 2020-08-28  7:47 Vignesh Raghavendra
  2020-08-28  8:26 ` Jan Kiszka
  0 siblings, 1 reply; 3+ messages in thread
From: Vignesh Raghavendra @ 2020-08-28  7:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: Vignesh Raghavendra, Jan Kiszka, Pratyush Yadav, linux-spi, linux-kernel

Buffers need to mapped to DMA channel's device pointer instead of SPI
controller's device pointer as its system DMA that actually does data
transfer.
Data inconsistencies have been reported when reading from flash
without this fix.

Fixes: 31fb632b5d43c ("spi: Move cadence-quadspi driver to drivers/spi/")
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---

This issue was present in the original driver under SPI NOR framework as well.
But only got exposed as driver started handling probe deferral for DMA channel
request and thus uses DMA almost always unlike before.

 drivers/spi/spi-cadence-quadspi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 508b219eabf8..c6795c684b16 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -907,14 +907,16 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
 	struct dma_async_tx_descriptor *tx;
 	dma_cookie_t cookie;
 	dma_addr_t dma_dst;
+	struct device *ddev;
 
 	if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
 		memcpy_fromio(buf, cqspi->ahb_base + from, len);
 		return 0;
 	}
 
-	dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
-	if (dma_mapping_error(dev, dma_dst)) {
+	ddev = cqspi->rx_chan->device->dev;
+	dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
+	if (dma_mapping_error(ddev, dma_dst)) {
 		dev_err(dev, "dma mapping failed\n");
 		return -ENOMEM;
 	}
@@ -948,7 +950,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
 	}
 
 err_unmap:
-	dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
+	dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
 
 	return ret;
 }
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] spi: spi-cadence-quadspi: Fix mapping of buffers for DMA reads
  2020-08-28  7:47 [PATCH] spi: spi-cadence-quadspi: Fix mapping of buffers for DMA reads Vignesh Raghavendra
@ 2020-08-28  8:26 ` Jan Kiszka
  2020-08-31 12:58   ` Vignesh Raghavendra
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2020-08-28  8:26 UTC (permalink / raw)
  To: Vignesh Raghavendra, Mark Brown; +Cc: Pratyush Yadav, linux-spi, linux-kernel

On 28.08.20 09:47, Vignesh Raghavendra wrote:
> Buffers need to mapped to DMA channel's device pointer instead of SPI
> controller's device pointer as its system DMA that actually does data
> transfer.
> Data inconsistencies have been reported when reading from flash
> without this fix.
> 
> Fixes: 31fb632b5d43c ("spi: Move cadence-quadspi driver to drivers/spi/")

This looks wrong, ...

> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> 
> This issue was present in the original driver under SPI NOR framework as well.
> But only got exposed as driver started handling probe deferral for DMA channel
> request and thus uses DMA almost always unlike before.

...you rather want 935da5e5100f57d843cac4781b21f1c235059aa0 then.

Other than that:

Tested-by: Jan Kiszka <jan.kiszka@siemens.com>

Thanks!
Jan

> 
>  drivers/spi/spi-cadence-quadspi.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 508b219eabf8..c6795c684b16 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -907,14 +907,16 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>  	struct dma_async_tx_descriptor *tx;
>  	dma_cookie_t cookie;
>  	dma_addr_t dma_dst;
> +	struct device *ddev;
>  
>  	if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
>  		memcpy_fromio(buf, cqspi->ahb_base + from, len);
>  		return 0;
>  	}
>  
> -	dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
> -	if (dma_mapping_error(dev, dma_dst)) {
> +	ddev = cqspi->rx_chan->device->dev;
> +	dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(ddev, dma_dst)) {
>  		dev_err(dev, "dma mapping failed\n");
>  		return -ENOMEM;
>  	}
> @@ -948,7 +950,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>  	}
>  
>  err_unmap:
> -	dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
> +	dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>  
>  	return ret;
>  }
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] spi: spi-cadence-quadspi: Fix mapping of buffers for DMA reads
  2020-08-28  8:26 ` Jan Kiszka
@ 2020-08-31 12:58   ` Vignesh Raghavendra
  0 siblings, 0 replies; 3+ messages in thread
From: Vignesh Raghavendra @ 2020-08-31 12:58 UTC (permalink / raw)
  To: Jan Kiszka, Mark Brown; +Cc: Pratyush Yadav, linux-spi, linux-kernel



On 8/28/20 1:56 PM, Jan Kiszka wrote:
> On 28.08.20 09:47, Vignesh Raghavendra wrote:
>> Buffers need to mapped to DMA channel's device pointer instead of SPI
>> controller's device pointer as its system DMA that actually does data
>> transfer.
>> Data inconsistencies have been reported when reading from flash
>> without this fix.
>>
>> Fixes: 31fb632b5d43c ("spi: Move cadence-quadspi driver to drivers/spi/")
> 
> This looks wrong, ...
> 
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>>
>> This issue was present in the original driver under SPI NOR framework as well.
>> But only got exposed as driver started handling probe deferral for DMA channel
>> request and thus uses DMA almost always unlike before.
> 
> ...you rather want 935da5e5100f57d843cac4781b21f1c235059aa0 then.
> 

Actually, the offending code is actually from 
commit ffa639e069fb ("mtd: spi-nor: cadence-quadspi: Add DMA support for direct mode reads)
which adds dma_{un}map_single() calls.

Will send v2 with Fixes tag updated to point to above commit 


> Other than that:
> 
> Tested-by: Jan Kiszka <jan.kiszka@siemens.com>

Thanks!

Regards
Vignesh

> 
> Thanks!
> Jan
> 
>>
>>  drivers/spi/spi-cadence-quadspi.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>> index 508b219eabf8..c6795c684b16 100644
>> --- a/drivers/spi/spi-cadence-quadspi.c
>> +++ b/drivers/spi/spi-cadence-quadspi.c
>> @@ -907,14 +907,16 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>>  	struct dma_async_tx_descriptor *tx;
>>  	dma_cookie_t cookie;
>>  	dma_addr_t dma_dst;
>> +	struct device *ddev;
>>  
>>  	if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
>>  		memcpy_fromio(buf, cqspi->ahb_base + from, len);
>>  		return 0;
>>  	}
>>  
>> -	dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
>> -	if (dma_mapping_error(dev, dma_dst)) {
>> +	ddev = cqspi->rx_chan->device->dev;
>> +	dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
>> +	if (dma_mapping_error(ddev, dma_dst)) {
>>  		dev_err(dev, "dma mapping failed\n");
>>  		return -ENOMEM;
>>  	}
>> @@ -948,7 +950,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>>  	}
>>  
>>  err_unmap:
>> -	dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
>> +	dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>>  
>>  	return ret;
>>  }
>>
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-08-31 12:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28  7:47 [PATCH] spi: spi-cadence-quadspi: Fix mapping of buffers for DMA reads Vignesh Raghavendra
2020-08-28  8:26 ` Jan Kiszka
2020-08-31 12:58   ` Vignesh Raghavendra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).