linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma()
@ 2013-08-11  0:33 Tomasz Figa
  2013-08-11  0:33 ` [PATCH 2/3] spi: s3c64xx: Do not request CS GPIO on subsequent calls to .setup() Tomasz Figa
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Tomasz Figa @ 2013-08-11  0:33 UTC (permalink / raw)
  To: linux-arm-kernel

Not all fields of dma_slave_config struct are being initialized by
prepare_dma() function, leaving those which are not in undefined state,
which can confuse DMA drivers using them.

This patch adds call to memset() to zero the struct before initializing
a subset of its fields.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/spi/spi-s3c64xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 62f32c4..1be162c 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -389,6 +389,8 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 	struct scatterlist sg;
 	struct dma_async_tx_descriptor *desc;
 
+	memset(&config, 0, sizeof(config));
+
 	if (dma->direction == DMA_DEV_TO_MEM) {
 		sdd = container_of((void *)dma,
 			struct s3c64xx_spi_driver_data, rx_dma);
-- 
1.8.3.2

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

* [PATCH 2/3] spi: s3c64xx: Do not request CS GPIO on subsequent calls to .setup()
  2013-08-11  0:33 [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Tomasz Figa
@ 2013-08-11  0:33 ` Tomasz Figa
  2013-08-11  0:33 ` [PATCH 3/3] spi: s3c64xx: Use dmaengine_prep_slave_single() to prepare DMA transfers Tomasz Figa
  2013-08-11 13:07 ` [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Tomasz Figa @ 2013-08-11  0:33 UTC (permalink / raw)
  To: linux-arm-kernel

Comments in linux/spi/spi.h and observed behavior show that .setup()
callback can be called multiple times without corresponding calls to
.cleanup(), what was incorrectly assumed by spi-s3c64xx driver, leading
to failures trying to request CS GPIO multiple times.

This patch modifies the behavior of spi-s3c64xx driver to request CS
GPIO only on first call to .setup() after last .cleanup().

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/spi/spi-s3c64xx.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 1be162c..597dc2f 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1068,20 +1068,21 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	/* Request gpio only if cs line is asserted by gpio pins */
-	if (sdd->cs_gpio) {
-		err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
-				       dev_name(&spi->dev));
-		if (err) {
-			dev_err(&spi->dev,
-				"Failed to get /CS gpio [%d]: %d\n",
-				cs->line, err);
-			goto err_gpio_req;
+	if (!spi_get_ctldata(spi)) {
+		/* Request gpio only if cs line is asserted by gpio pins */
+		if (sdd->cs_gpio) {
+			err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
+					dev_name(&spi->dev));
+			if (err) {
+				dev_err(&spi->dev,
+					"Failed to get /CS gpio [%d]: %d\n",
+					cs->line, err);
+				goto err_gpio_req;
+			}
 		}
-	}
 
-	if (!spi_get_ctldata(spi))
 		spi_set_ctldata(spi, cs);
+	}
 
 	sci = sdd->cntrlr_info;
 
-- 
1.8.3.2

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

* [PATCH 3/3] spi: s3c64xx: Use dmaengine_prep_slave_single() to prepare DMA transfers
  2013-08-11  0:33 [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Tomasz Figa
  2013-08-11  0:33 ` [PATCH 2/3] spi: s3c64xx: Do not request CS GPIO on subsequent calls to .setup() Tomasz Figa
@ 2013-08-11  0:33 ` Tomasz Figa
  2013-08-11 13:07 ` [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Tomasz Figa @ 2013-08-11  0:33 UTC (permalink / raw)
  To: linux-arm-kernel

Since the driver supports only contiguous buffers, there is no need to
manually construct a scatterlist with just a single entry, when there is
a dedicated helper for this purpose.

This patch modifies prepare_dma() function to use available helper instead
of manually creating a scatterlist.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/spi/spi-s3c64xx.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 597dc2f..9b4e54d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -386,7 +386,6 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 {
 	struct s3c64xx_spi_driver_data *sdd;
 	struct dma_slave_config config;
-	struct scatterlist sg;
 	struct dma_async_tx_descriptor *desc;
 
 	memset(&config, 0, sizeof(config));
@@ -409,14 +408,8 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 		dmaengine_slave_config(dma->ch, &config);
 	}
 
-	sg_init_table(&sg, 1);
-	sg_dma_len(&sg) = len;
-	sg_set_page(&sg, pfn_to_page(PFN_DOWN(buf)),
-		    len, offset_in_page(buf));
-	sg_dma_address(&sg) = buf;
-
-	desc = dmaengine_prep_slave_sg(dma->ch,
-		&sg, 1, dma->direction, DMA_PREP_INTERRUPT);
+	desc = dmaengine_prep_slave_single(dma->ch, buf, len,
+					dma->direction, DMA_PREP_INTERRUPT);
 
 	desc->callback = s3c64xx_spi_dmacb;
 	desc->callback_param = dma;
-- 
1.8.3.2

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

* [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma()
  2013-08-11  0:33 [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Tomasz Figa
  2013-08-11  0:33 ` [PATCH 2/3] spi: s3c64xx: Do not request CS GPIO on subsequent calls to .setup() Tomasz Figa
  2013-08-11  0:33 ` [PATCH 3/3] spi: s3c64xx: Use dmaengine_prep_slave_single() to prepare DMA transfers Tomasz Figa
@ 2013-08-11 13:07 ` Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2013-08-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Aug 11, 2013 at 02:33:28AM +0200, Tomasz Figa wrote:
> Not all fields of dma_slave_config struct are being initialized by
> prepare_dma() function, leaving those which are not in undefined state,
> which can confuse DMA drivers using them.

Applied all, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130811/106d0ab1/attachment.sig>

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

end of thread, other threads:[~2013-08-11 13:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-11  0:33 [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Tomasz Figa
2013-08-11  0:33 ` [PATCH 2/3] spi: s3c64xx: Do not request CS GPIO on subsequent calls to .setup() Tomasz Figa
2013-08-11  0:33 ` [PATCH 3/3] spi: s3c64xx: Use dmaengine_prep_slave_single() to prepare DMA transfers Tomasz Figa
2013-08-11 13:07 ` [PATCH 1/3] spi: s3c64xx: Zero dma_slave_config struct in prepare_dma() Mark Brown

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).