[PATCH 0/5] spi : sirf: some chip-select, DMA and checkpatch fixes

[PATCH 0/5] spi : sirf: some chip-select, DMA and checkpatch fixes

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

this patchset fixes some IO transferring issues and fixes checkpatch issues.

Qipan Li (5):
  spi: sirf: correct TXFIFO empty interrupt status bit
  spi: sirf: set SPI controller in RISC IO chipselect mode
  spi: sirf: make GPIO chipselect function work well
  spi: sirf: fix line over 80 characters style issue
  spi: sirf: fix spi full-duplex DMA transferring issue

 drivers/spi/spi-sirf.c |   74 +++++++++++++++++++++++++++++++++--------------
 1 files changed, 52 insertions(+), 22 deletions(-)

-- 
1.7.5.4

[PATCH 1/5] spi: sirf: correct TXFIFO empty interrupt status bit

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

the old code uses wrong marco - SIRFSOC_SPI_FIFO_FULL is not for
FIFO interrupt status, it is for FIFO status. here in the ISR,
SIRFSOC_SPI_TXFIFO_EMPTY is the right bit for SPI TXFIFO interrupt
status.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/spi-sirf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 1a77ad5..51d7c98 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -287,8 +287,8 @@ static irqreturn_t spi_sirfsoc_irq(int irq, void *dev_id)
 				sspi->left_rx_word)
 			sspi->rx_word(sspi);
 
-	if (spi_stat & (SIRFSOC_SPI_FIFO_EMPTY
-			| SIRFSOC_SPI_TXFIFO_THD_REACH))
+	if (spi_stat & (SIRFSOC_SPI_TXFIFO_EMPTY |
+			SIRFSOC_SPI_TXFIFO_THD_REACH))
 		while (!((readl(sspi->base + SIRFSOC_SPI_TXFIFO_STATUS)
 				& SIRFSOC_SPI_FIFO_FULL)) &&
 				sspi->left_tx_word)
-- 
1.7.5.4

[PATCH 1/5] spi: sirf: correct TXFIFO empty interrupt status bit

[Mark Brown]

On Mon, Apr 14, 2014 at 02:29:57PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> the old code uses wrong marco - SIRFSOC_SPI_FIFO_FULL is not for
> FIFO interrupt status, it is for FIFO status. here in the ISR,
> SIRFSOC_SPI_TXFIFO_EMPTY is the right bit for SPI TXFIFO interrupt
> status.

Applied, 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/20140414/b3b4ad33/attachment.sig>

[PATCH 2/5] spi: sirf: set SPI controller in RISC IO chipselect mode

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

SPI bitbang supply "chipselect" interface for change chip-select line
, in the SiRFSoC SPI controller, we need to enable "SPI_CS_IO_MODE",
otherwise, spi_sirfsoc_chipselect() has no effect.
now the driver is working is because SPI controller will control CS
automatically without SPI_CS_IO_MODE. this patch makes the CS controller
really controlled by software.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/spi-sirf.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 51d7c98..9b30743 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -559,6 +559,11 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 		regval &= ~SIRFSOC_SPI_CMD_MODE;
 		sspi->tx_by_cmd = false;
 	}
+	/*
+	 * set spi controller in RISC chipselect mode, we are controlling CS by
+	 * software BITBANG_CS_ACTIVE and BITBANG_CS_INACTIVE.
+	 */
+	regval |= SIRFSOC_SPI_CS_IO_MODE;
 	writel(regval, sspi->base + SIRFSOC_SPI_CTRL);
 
 	if (IS_DMA_VALID(t)) {
-- 
1.7.5.4

[PATCH 2/5] spi: sirf: set SPI controller in RISC IO chipselect mode

[Mark Brown]

On Mon, Apr 14, 2014 at 02:29:58PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> SPI bitbang supply "chipselect" interface for change chip-select line
> , in the SiRFSoC SPI controller, we need to enable "SPI_CS_IO_MODE",

Applied, 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/20140414/a5ace7ef/attachment.sig>

[PATCH 3/5] spi: sirf: make GPIO chipselect function work well

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

orignal GPIO chipslect is not standard because it don't take care to the
chipselect signal: BITBANG_CS_ACTIVE and BITBANG_CS_INACTIVE.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/spi-sirf.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 9b30743..67d8909 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -470,7 +470,16 @@ static void spi_sirfsoc_chipselect(struct spi_device *spi, int value)
 		writel(regval, sspi->base + SIRFSOC_SPI_CTRL);
 	} else {
 		int gpio = sspi->chipselect[spi->chip_select];
-		gpio_direction_output(gpio, spi->mode & SPI_CS_HIGH ? 0 : 1);
+		switch (value) {
+		case BITBANG_CS_ACTIVE:
+			gpio_direction_output(gpio,
+					spi->mode & SPI_CS_HIGH ? 1 : 0);
+			break;
+		case BITBANG_CS_INACTIVE:
+			gpio_direction_output(gpio,
+					spi->mode & SPI_CS_HIGH ? 0 : 1);
+			break;
+		}
 	}
 }
 
-- 
1.7.5.4

[PATCH 3/5] spi: sirf: make GPIO chipselect function work well

[Mark Brown]

On Mon, Apr 14, 2014 at 02:29:59PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> orignal GPIO chipslect is not standard because it don't take care to the
> chipselect signal: BITBANG_CS_ACTIVE and BITBANG_CS_INACTIVE.

Applied, 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/20140414/5273b9ee/attachment.sig>

[PATCH 4/5] spi: sirf: fix line over 80 characters style issue

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

fix a lot of "line over 80 characters" checkpatch issues, on which
the users of the driver, key customers care about this very much.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/spi-sirf.c |   51 ++++++++++++++++++++++++++++++-----------------
 1 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 67d8909..3c12f39 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -382,14 +382,16 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 	if (IS_DMA_VALID(t)) {
 		struct dma_async_tx_descriptor *rx_desc, *tx_desc;
 
-		sspi->dst_start = dma_map_single(&spi->dev, sspi->rx, t->len, DMA_FROM_DEVICE);
+		sspi->dst_start = dma_map_single(&spi->dev,
+				sspi->rx, t->len, DMA_FROM_DEVICE);
 		rx_desc = dmaengine_prep_slave_single(sspi->rx_chan,
 			sspi->dst_start, t->len, DMA_DEV_TO_MEM,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 		rx_desc->callback = spi_sirfsoc_dma_fini_callback;
 		rx_desc->callback_param = &sspi->rx_done;
 
-		sspi->src_start = dma_map_single(&spi->dev, (void *)sspi->tx, t->len, DMA_TO_DEVICE);
+		sspi->src_start = dma_map_single(&spi->dev,
+				(void *)sspi->tx, t->len, DMA_TO_DEVICE);
 		tx_desc = dmaengine_prep_slave_single(sspi->tx_chan,
 			sspi->src_start, t->len, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -404,13 +406,18 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 		/* Send the first word to trigger the whole tx/rx process */
 		sspi->tx_word(sspi);
 
-		writel(SIRFSOC_SPI_RX_OFLOW_INT_EN | SIRFSOC_SPI_TX_UFLOW_INT_EN |
-			SIRFSOC_SPI_RXFIFO_THD_INT_EN | SIRFSOC_SPI_TXFIFO_THD_INT_EN |
-			SIRFSOC_SPI_FRM_END_INT_EN | SIRFSOC_SPI_RXFIFO_FULL_INT_EN |
-			SIRFSOC_SPI_TXFIFO_EMPTY_INT_EN, sspi->base + SIRFSOC_SPI_INT_EN);
+		writel(SIRFSOC_SPI_RX_OFLOW_INT_EN |
+			SIRFSOC_SPI_TX_UFLOW_INT_EN |
+			SIRFSOC_SPI_RXFIFO_THD_INT_EN |
+			SIRFSOC_SPI_TXFIFO_THD_INT_EN |
+			SIRFSOC_SPI_FRM_END_INT_EN |
+			SIRFSOC_SPI_RXFIFO_FULL_INT_EN |
+			SIRFSOC_SPI_TXFIFO_EMPTY_INT_EN,
+			sspi->base + SIRFSOC_SPI_INT_EN);
 	}
 
-	writel(SIRFSOC_SPI_RX_EN | SIRFSOC_SPI_TX_EN, sspi->base + SIRFSOC_SPI_TX_RX_EN);
+	writel(SIRFSOC_SPI_RX_EN | SIRFSOC_SPI_TX_EN,
+			sspi->base + SIRFSOC_SPI_TX_RX_EN);
 
 	if (!IS_DMA_VALID(t)) { /* for PIO */
 		if (wait_for_completion_timeout(&sspi->rx_done, timeout) == 0)
@@ -434,8 +441,10 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 	}
 
 	if (IS_DMA_VALID(t)) {
-		dma_unmap_single(&spi->dev, sspi->src_start, t->len, DMA_TO_DEVICE);
-		dma_unmap_single(&spi->dev, sspi->dst_start, t->len, DMA_FROM_DEVICE);
+		dma_unmap_single(&spi->dev,
+				sspi->src_start, t->len, DMA_TO_DEVICE);
+		dma_unmap_single(&spi->dev,
+				sspi->dst_start, t->len, DMA_FROM_DEVICE);
 	}
 
 	/* TX, RX FIFO stop */
@@ -512,7 +521,8 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 		break;
 	case 12:
 	case 16:
-		regval |= (bits_per_word ==  12) ? SIRFSOC_SPI_TRAN_DAT_FORMAT_12 :
+		regval |= (bits_per_word ==  12) ?
+			SIRFSOC_SPI_TRAN_DAT_FORMAT_12 :
 			SIRFSOC_SPI_TRAN_DAT_FORMAT_16;
 		sspi->rx_word = spi_sirfsoc_rx_word_u16;
 		sspi->tx_word = spi_sirfsoc_tx_word_u16;
@@ -540,8 +550,8 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 		regval |= SIRFSOC_SPI_CLK_IDLE_STAT;
 
 	/*
-	 * Data should be driven at least 1/2 cycle before the fetch edge to make
-	 * sure that data gets stable at the fetch edge.
+	 * Data should be driven at least 1/2 cycle before the fetch edge
+	 * to make sure that data gets stable at the fetch edge.
 	 */
 	if (((spi->mode & SPI_CPOL) && (spi->mode & SPI_CPHA)) ||
 	    (!(spi->mode & SPI_CPOL) && !(spi->mode & SPI_CPHA)))
@@ -578,11 +588,14 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 	if (IS_DMA_VALID(t)) {
 		/* Enable DMA mode for RX, TX */
 		writel(0, sspi->base + SIRFSOC_SPI_TX_DMA_IO_CTRL);
-		writel(SIRFSOC_SPI_RX_DMA_FLUSH, sspi->base + SIRFSOC_SPI_RX_DMA_IO_CTRL);
+		writel(SIRFSOC_SPI_RX_DMA_FLUSH,
+			sspi->base + SIRFSOC_SPI_RX_DMA_IO_CTRL);
 	} else {
 		/* Enable IO mode for RX, TX */
-		writel(SIRFSOC_SPI_IO_MODE_SEL, sspi->base + SIRFSOC_SPI_TX_DMA_IO_CTRL);
-		writel(SIRFSOC_SPI_IO_MODE_SEL, sspi->base + SIRFSOC_SPI_RX_DMA_IO_CTRL);
+		writel(SIRFSOC_SPI_IO_MODE_SEL,
+			sspi->base + SIRFSOC_SPI_TX_DMA_IO_CTRL);
+		writel(SIRFSOC_SPI_IO_MODE_SEL,
+			sspi->base + SIRFSOC_SPI_RX_DMA_IO_CTRL);
 	}
 
 	return 0;
@@ -612,7 +625,8 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
 		goto err_cs;
 	}
 
-	master = spi_alloc_master(&pdev->dev, sizeof(*sspi) + sizeof(int) * num_cs);
+	master = spi_alloc_master(&pdev->dev,
+			sizeof(*sspi) + sizeof(int) * num_cs);
 	if (!master) {
 		dev_err(&pdev->dev, "Unable to allocate SPI master\n");
 		return -ENOMEM;
@@ -808,8 +822,7 @@ static struct platform_driver spi_sirfsoc_driver = {
 	.remove = spi_sirfsoc_remove,
 };
 module_platform_driver(spi_sirfsoc_driver);
-
 MODULE_DESCRIPTION("SiRF SoC SPI master driver");
-MODULE_AUTHOR("Zhiwu Song <Zhiwu.Song@csr.com>, "
-		"Barry Song <Baohua.Song@csr.com>");
+MODULE_AUTHOR("Zhiwu Song <Zhiwu.Song@csr.com>");
+MODULE_AUTHOR("Barry Song <Baohua.Song@csr.com>");
 MODULE_LICENSE("GPL v2");
-- 
1.7.5.4

[PATCH 4/5] spi: sirf: fix line over 80 characters style issue

[Mark Brown]

On Mon, Apr 14, 2014 at 02:30:00PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> fix a lot of "line over 80 characters" checkpatch issues, on which
> the users of the driver, key customers care about this very much.

Applied, 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/20140414/bcad6e8b/attachment.sig>

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

sometimes t->tx can be equal with t->rx. for example, spidev will make
tx and rx point to spidev->buffer at the same time. currently, for this
case, we map the buffer BIDIRECTION to fix the cache consistency.

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/spi/spi-sirf.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 3c12f39..0c039d4 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -383,7 +383,8 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 		struct dma_async_tx_descriptor *rx_desc, *tx_desc;
 
 		sspi->dst_start = dma_map_single(&spi->dev,
-				sspi->rx, t->len, DMA_FROM_DEVICE);
+				sspi->rx, t->len, (t->tx_buf != t->rx_buf) ?
+				DMA_FROM_DEVICE : DMA_BIDIRECTIONAL);
 		rx_desc = dmaengine_prep_slave_single(sspi->rx_chan,
 			sspi->dst_start, t->len, DMA_DEV_TO_MEM,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -391,7 +392,9 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 		rx_desc->callback_param = &sspi->rx_done;
 
 		sspi->src_start = dma_map_single(&spi->dev,
-				(void *)sspi->tx, t->len, DMA_TO_DEVICE);
+				(void *)sspi->tx, t->len,
+				(t->tx_buf != t->rx_buf) ?
+				DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 		tx_desc = dmaengine_prep_slave_single(sspi->tx_chan,
 			sspi->src_start, t->len, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-- 
1.7.5.4

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Mark Brown]

On Mon, Apr 14, 2014 at 02:30:01PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> sometimes t->tx can be equal with t->rx. for example, spidev will make
> tx and rx point to spidev->buffer at the same time. currently, for this
> case, we map the buffer BIDIRECTION to fix the cache consistency.

I've applied this but such usage is out of spec - do we have any drivers
doing this in mainline?
-------------- 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/20140414/45208439/attachment.sig>

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Barry Song]

2014-04-15 4:06 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Mon, Apr 14, 2014 at 02:30:01PM +0800, Barry Song wrote:
>> From: Qipan Li <Qipan.Li@csr.com>
>>
>> sometimes t->tx can be equal with t->rx. for example, spidev will make
>> tx and rx point to spidev->buffer at the same time. currently, for this
>> case, we map the buffer BIDIRECTION to fix the cache consistency.
>
> I've applied this but such usage is out of spec - do we have any drivers
> doing this in mainline?

i felt strange too at the first look from internal gerrit. qipan told
me there is one:
drivers/spi/spidev.c

static int spidev_message(struct spidev_data *spidev,
                struct spi_ioc_transfer *u_xfers, unsigned n_xfers)
{
buf = spidev->buffer;
..
k_tmp->rx_buf = buf;
...
k_tmp->tx_buf = buf;
...
spi_message_add_tail(k_tmp, &msg);

spidev_sync(spidev, &msg);
}

-barry

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Mark Brown]

On Tue, Apr 15, 2014 at 09:43:24AM +0800, Barry Song wrote:

> i felt strange too at the first look from internal gerrit. qipan told
> me there is one:
> drivers/spi/spidev.c

OK, that needs fixing.  That code predates anyone doing DMA.
-------------- 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/20140415/6f4fee84/attachment.sig>

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Barry Song]

2014-04-15 17:26 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Tue, Apr 15, 2014 at 09:43:24AM +0800, Barry Song wrote:
>
>> i felt strange too at the first look from internal gerrit. qipan told
>> me there is one:
>> drivers/spi/spidev.c
>
> OK, that needs fixing.  That code predates anyone doing DMA.

if it is out of specification, it seems fixing spidev.c is not enough
as spi transfer actually needs an precondition of tx!=rx now.

-barry

[PATCH 5/5] spi: sirf: fix spi full-duplex DMA transferring issue

[Mark Brown]

On Tue, Apr 15, 2014 at 05:57:39PM +0800, Barry Song wrote:
> 2014-04-15 17:26 GMT+08:00 Mark Brown <broonie@kernel.org>:

> > OK, that needs fixing.  That code predates anyone doing DMA.

> if it is out of specification, it seems fixing spidev.c is not enough
> as spi transfer actually needs an precondition of tx!=rx now.

Well, it's enough to avoid bugs.  Adding the validation to make sure
nobody introduces the problem again is nice too but isn't as urgent.
-------------- 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/20140415/1439cfb3/attachment.sig>