All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode
@ 2014-02-12 16:30 Barry Song
       [not found] ` <1392222620-17273-1-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-02-12 16:30 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: workgroup.linux-kQvG35nSl+M, Qipan Li, Barry Song

From: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>

there are many SPI clients which use the following protocal:
step 1: send command bytes to clients(rx buffer is empty)
step 2: send data bytes to clients or receive data bytes from
clients.
SiRFprimaII provides a shortcut for this kind of SPI transfer.
when tx buf is less or equal than 4 bytes and rx buf is null
in a transfer, we think it as 'command' data and use hardware
command register for the transfer.
here we can save some CPU loading than doing both tx and rx
for a normal transfer.

Signed-off-by: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>
Signed-off-by: Barry Song <Baohua.Song-kQvG35nSl+M@public.gmane.org>
---
 drivers/spi/spi-sirf.c |   39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index e430689..c400ab7 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -132,6 +132,7 @@
 #define IS_DMA_VALID(x) (x && ALIGNED(x->tx_buf) && ALIGNED(x->rx_buf) && \
 	ALIGNED(x->len) && (x->len < 2 * PAGE_SIZE))
 
+#define SIRFSOC_MAX_CMD_BYTES	4
 struct sirfsoc_spi {
 	struct spi_bitbang bitbang;
 	struct completion rx_done;
@@ -161,7 +162,7 @@ struct sirfsoc_spi {
 	dma_addr_t dst_start;
 	void *dummypage;
 	int word_width; /* in bytes */
-
+	bool	tx_by_cmd;
 	int chipselect[0];
 };
 
@@ -260,6 +261,11 @@ static irqreturn_t spi_sirfsoc_irq(int irq, void *dev_id)
 
 	writel(spi_stat, sspi->base + SIRFSOC_SPI_INT_STATUS);
 
+	if (sspi->tx_by_cmd && (spi_stat & SIRFSOC_SPI_FRM_END)) {
+		complete(&sspi->tx_done);
+		writel(0x0, sspi->base + SIRFSOC_SPI_INT_EN);
+		return IRQ_HANDLED;
+	}
 	/* Error Conditions */
 	if (spi_stat & SIRFSOC_SPI_RX_OFLOW ||
 			spi_stat & SIRFSOC_SPI_TX_UFLOW) {
@@ -310,6 +316,32 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t)
 
 	writel(SIRFSOC_SPI_INT_MASK_ALL, sspi->base + SIRFSOC_SPI_INT_STATUS);
 
+	if (t && t->tx_buf && !t->rx_buf && (t->len <= SIRFSOC_MAX_CMD_BYTES)) {
+		const u32 *cmd_ptr;
+		u32 cmd;
+		sspi->tx_by_cmd = true;
+		cmd_ptr = sspi->tx;
+		cmd = *cmd_ptr;
+		if (sspi->word_width == 1 && !(spi->mode & SPI_LSB_FIRST))
+			cmd = cpu_to_be32(cmd) >>
+				((SIRFSOC_MAX_CMD_BYTES - t->len) * 8);
+		if (sspi->word_width == 2 && t->len == 4 &&
+				(!(spi->mode & SPI_LSB_FIRST)))
+			cmd = ((cmd & 0xffff) << 16) | (cmd >> 16);
+		writel(cmd, sspi->base + SIRFSOC_SPI_CMD);
+
+		writel(SIRFSOC_SPI_FRM_END_INT_EN,
+				sspi->base + SIRFSOC_SPI_INT_EN);
+		writel(SIRFSOC_SPI_CMD_TX_EN,
+				sspi->base + SIRFSOC_SPI_TX_RX_EN);
+
+		if (wait_for_completion_timeout(&sspi->tx_done, timeout) == 0) {
+			dev_err(&spi->dev, "transfer timeout\n");
+			return 0;
+		}
+		return t->len;
+	} else
+		sspi->tx_by_cmd = false;
 	if (sspi->left_tx_word == 1) {
 		writel(readl(sspi->base + SIRFSOC_SPI_CTRL) |
 			SIRFSOC_SPI_ENA_AUTO_CLR,
@@ -519,6 +551,11 @@ spi_sirfsoc_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 	writel(txfifo_ctrl, sspi->base + SIRFSOC_SPI_TXFIFO_CTRL);
 	writel(rxfifo_ctrl, sspi->base + SIRFSOC_SPI_RXFIFO_CTRL);
 
+	if (t && t->tx_buf && !t->rx_buf && (t->len <= SIRFSOC_MAX_CMD_BYTES))
+		regval |= (SIRFSOC_SPI_CMD_BYTE_NUM((t->len - 1)) |
+				SIRFSOC_SPI_CMD_MODE);
+	else
+		regval &= ~SIRFSOC_SPI_CMD_MODE;
 	writel(regval, sspi->base + SIRFSOC_SPI_CTRL);
 
 	if (IS_DMA_VALID(t)) {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 2/3] spi: sirf: move to use generic dma dt-binding
       [not found] ` <1392222620-17273-1-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-02-12 16:30   ` Barry Song
       [not found]     ` <1392222620-17273-2-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2014-02-12 16:30   ` [PATCH 3/3] spi: sirf: use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries Barry Song
  2014-02-24  2:25   ` [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode Mark Brown
  2 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-02-12 16:30 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: workgroup.linux-kQvG35nSl+M, Barry Song

From: Barry Song <Baohua.Song-kQvG35nSl+M@public.gmane.org>

sirf-dma driver enabled generic dt binding for dma channels.
see here we remove self-defined dma channel prop and move to
use generic dma_request_slave_channel.
related changes in dts is something like:
    dmas = <&dmac1 9>,
    <&dmac1 4>;
    dma-names = "rx", "tx";

Signed-off-by: Barry Song <Baohua.Song-kQvG35nSl+M@public.gmane.org>
---
 drivers/spi/spi-sirf.c |   26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index c400ab7..a213e87 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -22,7 +22,6 @@
 #include <linux/dmaengine.h>
 #include <linux/dma-direction.h>
 #include <linux/dma-mapping.h>
-#include <linux/sirfsoc_dma.h>
 
 #define DRIVER_NAME "sirfsoc_spi"
 
@@ -585,8 +584,6 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct resource *mem_res;
 	int num_cs, cs_gpio, irq;
-	u32 rx_dma_ch, tx_dma_ch;
-	dma_cap_mask_t dma_cap_mask;
 	int i;
 	int ret;
 
@@ -597,20 +594,6 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
 		goto err_cs;
 	}
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-			"sirf,spi-dma-rx-channel", &rx_dma_ch);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Unable to get rx dma channel\n");
-		goto err_cs;
-	}
-
-	ret = of_property_read_u32(pdev->dev.of_node,
-			"sirf,spi-dma-tx-channel", &tx_dma_ch);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Unable to get tx dma channel\n");
-		goto err_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");
@@ -674,18 +657,13 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
 	sspi->bitbang.master->dev.of_node = pdev->dev.of_node;
 
 	/* request DMA channels */
-	dma_cap_zero(dma_cap_mask);
-	dma_cap_set(DMA_INTERLEAVE, dma_cap_mask);
-
-	sspi->rx_chan = dma_request_channel(dma_cap_mask, (dma_filter_fn)sirfsoc_dma_filter_id,
-		(void *)rx_dma_ch);
+	sspi->rx_chan = dma_request_slave_channel(&pdev->dev, "rx");
 	if (!sspi->rx_chan) {
 		dev_err(&pdev->dev, "can not allocate rx dma channel\n");
 		ret = -ENODEV;
 		goto free_master;
 	}
-	sspi->tx_chan = dma_request_channel(dma_cap_mask, (dma_filter_fn)sirfsoc_dma_filter_id,
-		(void *)tx_dma_ch);
+	sspi->tx_chan = dma_request_slave_channel(&pdev->dev, "tx");
 	if (!sspi->tx_chan) {
 		dev_err(&pdev->dev, "can not allocate tx dma channel\n");
 		ret = -ENODEV;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/3] spi: sirf: use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries
       [not found] ` <1392222620-17273-1-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2014-02-12 16:30   ` [PATCH 2/3] spi: sirf: move to use generic dma dt-binding Barry Song
@ 2014-02-12 16:30   ` Barry Song
       [not found]     ` <1392222620-17273-3-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2014-02-24  2:25   ` [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode Mark Brown
  2 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-02-12 16:30 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: workgroup.linux-kQvG35nSl+M, Qipan Li, Barry Song

From: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>

use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries, this makes the codes
clean and also enable the ability of hibernation support for sirf SPI.

Signed-off-by: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>
Signed-off-by: Barry Song <Baohua.Song-kQvG35nSl+M@public.gmane.org>
---
 drivers/spi/spi-sirf.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index a213e87..5bffa5f 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -739,7 +739,7 @@ static int  spi_sirfsoc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int spi_sirfsoc_suspend(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
@@ -762,12 +762,11 @@ static int spi_sirfsoc_resume(struct device *dev)
 
 	return 0;
 }
+#endif
 
 static const struct dev_pm_ops spi_sirfsoc_pm_ops = {
-	.suspend = spi_sirfsoc_suspend,
-	.resume = spi_sirfsoc_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(spi_sirfsoc_suspend, spi_sirfsoc_resume)
 };
-#endif
 
 static const struct of_device_id spi_sirfsoc_of_match[] = {
 	{ .compatible = "sirf,prima2-spi", },
@@ -780,9 +779,7 @@ static struct platform_driver spi_sirfsoc_driver = {
 	.driver = {
 		.name = DRIVER_NAME,
 		.owner = THIS_MODULE,
-#ifdef CONFIG_PM
 		.pm     = &spi_sirfsoc_pm_ops,
-#endif
 		.of_match_table = spi_sirfsoc_of_match,
 	},
 	.probe = spi_sirfsoc_probe,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 2/3] spi: sirf: move to use generic dma dt-binding
       [not found]     ` <1392222620-17273-2-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-02-24  2:01       ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-02-24  2:01 UTC (permalink / raw)
  To: Barry Song
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, workgroup.linux-kQvG35nSl+M,
	Barry Song

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

On Thu, Feb 13, 2014 at 12:30:19AM +0800, Barry Song wrote:
> From: Barry Song <Baohua.Song-kQvG35nSl+M@public.gmane.org>
> 
> sirf-dma driver enabled generic dt binding for dma channels.
> see here we remove self-defined dma channel prop and move to
> use generic dma_request_slave_channel.
> related changes in dts is something like:

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 3/3] spi: sirf: use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries
       [not found]     ` <1392222620-17273-3-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-02-24  2:01       ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-02-24  2:01 UTC (permalink / raw)
  To: Barry Song
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, workgroup.linux-kQvG35nSl+M,
	Qipan Li, Barry Song

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Thu, Feb 13, 2014 at 12:30:20AM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>
> 
> use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries, this makes the codes
> clean and also enable the ability of hibernation support for sirf SPI.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode
       [not found] ` <1392222620-17273-1-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2014-02-12 16:30   ` [PATCH 2/3] spi: sirf: move to use generic dma dt-binding Barry Song
  2014-02-12 16:30   ` [PATCH 3/3] spi: sirf: use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries Barry Song
@ 2014-02-24  2:25   ` Mark Brown
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-02-24  2:25 UTC (permalink / raw)
  To: Barry Song
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, workgroup.linux-kQvG35nSl+M,
	Qipan Li, Barry Song

[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]

On Thu, Feb 13, 2014 at 12:30:18AM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li-kQvG35nSl+M@public.gmane.org>
> 
> there are many SPI clients which use the following protocal:
> step 1: send command bytes to clients(rx buffer is empty)
> step 2: send data bytes to clients or receive data bytes from
> clients.

The code here is a bit impenetrable, it could probably benefit from some
comments or possibly refactoring to split out the command based
transfers into a separate function so you end up with this:

> +	if (t && t->tx_buf && !t->rx_buf && (t->len <= SIRFSOC_MAX_CMD_BYTES)) {

		do_cmd_xfer();
	else
		do_normal_xfer();

> +		const u32 *cmd_ptr;
> +		u32 cmd;
> +		sspi->tx_by_cmd = true;
> +		cmd_ptr = sspi->tx;
> +		cmd = *cmd_ptr;
> +		if (sspi->word_width == 1 && !(spi->mode & SPI_LSB_FIRST))
> +			cmd = cpu_to_be32(cmd) >>
> +				((SIRFSOC_MAX_CMD_BYTES - t->len) * 8);
> +		if (sspi->word_width == 2 && t->len == 4 &&
> +				(!(spi->mode & SPI_LSB_FIRST)))
> +			cmd = ((cmd & 0xffff) << 16) | (cmd >> 16);

The above dererferences cmd_ptr no matter what the actual length is.  If
it's 4 bytes that's fine but if it's less this means the code is reading
beyond the end of the buffer.  Most likely this is going to be safe but
it's out of spec and could result in doing something that messes with
DMA mappings.  A memcpy() would be safer.

This is also a bit unclear since the code is so densely written, more
whitespace would help, as would avoiding splitting the set of
assignments you use to avoid casts (but like I say they seem unsafe
anyway).

> +		return t->len;
> +	} else
> +		sspi->tx_by_cmd = false;

You should have { } on either both sides or only one side of the if.

> +	if (t && t->tx_buf && !t->rx_buf && (t->len <= SIRFSOC_MAX_CMD_BYTES))
> +		regval |= (SIRFSOC_SPI_CMD_BYTE_NUM((t->len - 1)) |
> +				SIRFSOC_SPI_CMD_MODE);
> +	else
> +		regval &= ~SIRFSOC_SPI_CMD_MODE;

This check is being repeated in two places which doesn't feel 100% safe,
why not use tx_by_cmd?

What the code is doing is basically fine but it's quite hard to read.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

end of thread, other threads:[~2014-02-24  2:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-12 16:30 [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode Barry Song
     [not found] ` <1392222620-17273-1-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-12 16:30   ` [PATCH 2/3] spi: sirf: move to use generic dma dt-binding Barry Song
     [not found]     ` <1392222620-17273-2-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-24  2:01       ` Mark Brown
2014-02-12 16:30   ` [PATCH 3/3] spi: sirf: use SET_SYSTEM_SLEEP_PM_OPS to initialize PM entries Barry Song
     [not found]     ` <1392222620-17273-3-git-send-email-21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-24  2:01       ` Mark Brown
2014-02-24  2:25   ` [PATCH 1/3] spi: sirf: provide a shortcut for spi command-data mode Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.