[PATCH] ASoC: rt5514: Use the local variable to get the data from SPI device

[PATCH] ASoC: rt5514: Use the local variable to get the data from SPI device

[Oder Chiou]

The patch uses the local variable to get the data from SPI device to
prevent to get all zero data.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
---
 sound/soc/codecs/rt5514-spi.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
index 950d1ff..e693204 100644
--- a/sound/soc/codecs/rt5514-spi.c
+++ b/sound/soc/codecs/rt5514-spi.c
@@ -289,7 +289,7 @@ int rt5514_spi_burst_read(unsigned int addr, u8 *rxbuf, size_t len)
 {
 	u8 spi_cmd = RT5514_SPI_CMD_BURST_READ;
 	int status;
-	u8 write_buf[8];
+	u8 write_buf[8], read_buf[RT5514_SPI_BUF_LEN];
 	unsigned int i, end, offset = 0;
 
 	struct spi_message message;
@@ -319,7 +319,7 @@ int rt5514_spi_burst_read(unsigned int addr, u8 *rxbuf, size_t len)
 		spi_message_add_tail(&x[1], &message);
 
 		x[2].len = end;
-		x[2].rx_buf = rxbuf + offset;
+		x[2].rx_buf = read_buf;
 		spi_message_add_tail(&x[2], &message);
 
 		status = spi_sync(rt5514_spi, &message);
@@ -327,6 +327,8 @@ int rt5514_spi_burst_read(unsigned int addr, u8 *rxbuf, size_t len)
 		if (status)
 			return false;
 
+		memcpy(rxbuf + offset, read_buf, end);
+
 		offset += RT5514_SPI_BUF_LEN;
 	}
 
@@ -365,14 +367,9 @@ int rt5514_spi_burst_read(unsigned int addr, u8 *rxbuf, size_t len)
 int rt5514_spi_burst_write(u32 addr, const u8 *txbuf, size_t len)
 {
 	u8 spi_cmd = RT5514_SPI_CMD_BURST_WRITE;
-	u8 *write_buf;
+	u8 write_buf[RT5514_SPI_BUF_LEN + 6];
 	unsigned int i, end, offset = 0;
 
-	write_buf = kmalloc(RT5514_SPI_BUF_LEN + 6, GFP_KERNEL);
-
-	if (write_buf == NULL)
-		return -ENOMEM;
-
 	while (offset < len) {
 		if (offset + RT5514_SPI_BUF_LEN <= len)
 			end = RT5514_SPI_BUF_LEN;
@@ -403,8 +400,6 @@ int rt5514_spi_burst_write(u32 addr, const u8 *txbuf, size_t len)
 		offset += RT5514_SPI_BUF_LEN;
 	}
 
-	kfree(write_buf);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt5514_spi_burst_write);
-- 
2.7.4

Re: [PATCH] ASoC: rt5514: Use the local variable to get the data from SPI device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 235 bytes --]

On Fri, Jul 21, 2017 at 04:00:47PM +0800, Oder Chiou wrote:
> The patch uses the local variable to get the data from SPI device to
> prevent to get all zero data.

How do you end up with all zero data from reading directly into rxbuf?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: rt5514: Use the local variable to get the data from SPI device

[Hsin-yu Chao]

The original cause seems to be in spi driver.
I tested setting enable_dma flag to false in spi-pxa2xx driver and that
fixes the problem.
Will investigate in spi driver side. Thanks!

Hsin-yu



On Mon, Jul 24, 2017 at 11:06 AM, Oder Chiou <oder_chiou@realtek.com> wrote:

> > On Fri, Jul 21, 2017 at 04:00:47PM +0800, Oder Chiou wrote:
> > > The patch uses the local variable to get the data from SPI device to
> > > prevent to get all zero data.
> >
> > How do you end up with all zero data from reading directly into rxbuf?
> >
> While the "runtime->dma_area" put into the rxbuf directly on the Intel
> platform, it seemed that had the limitation to let the memory cannot be
> written the data using the driver of the SPI controller. We verified the
> original code on the platforms of Nvidia and Rockchip, and these are
> working fine.
>

Re: [PATCH] ASoC: rt5514: Use the local variable to get the data from SPI device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 863 bytes --]

On Mon, Jul 24, 2017 at 03:06:36AM +0000, Oder Chiou wrote:
> > On Fri, Jul 21, 2017 at 04:00:47PM +0800, Oder Chiou wrote:
> > > The patch uses the local variable to get the data from SPI device to
> > > prevent to get all zero data.

> > How do you end up with all zero data from reading directly into rxbuf?

> While the "runtime->dma_area" put into the rxbuf directly on the Intel
> platform, it seemed that had the limitation to let the memory cannot be
> written the data using the driver of the SPI controller. We verified the
> original code on the platforms of Nvidia and Rockchip, and these are
> working fine.

Do we understand why that is?  Note that IIRC you're not really supposed
to DMA to or from the stack, there are some platforms with restrictions
there so it's possible you're just moving the problem around here if
rxbuf can be on the stack.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]