[PATCH 1/2] spi: omap2: disable DMA requests before complete()

[PATCH 1/2] spi: omap2: disable DMA requests before complete()

[Felipe Balbi]

No actual errors have been found for completing
before disabling DMA request lines, but it just
looks more semantically correct that on our DMA
callback we quiesce the whole thing before stating
transfer is finished.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/spi/spi-omap2-mcspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index b610f52..68446db 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -298,10 +298,10 @@ static void omap2_mcspi_rx_callback(void *data)
 	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
 	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
 
-	complete(&mcspi_dma->dma_rx_completion);
-
 	/* We must disable the DMA RX request */
 	omap2_mcspi_set_dma_req(spi, 1, 0);
+
+	complete(&mcspi_dma->dma_rx_completion);
 }
 
 static void omap2_mcspi_tx_callback(void *data)
@@ -310,10 +310,10 @@ static void omap2_mcspi_tx_callback(void *data)
 	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
 	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
 
-	complete(&mcspi_dma->dma_tx_completion);
-
 	/* We must disable the DMA TX request */
 	omap2_mcspi_set_dma_req(spi, 0, 0);
+
+	complete(&mcspi_dma->dma_tx_completion);
 }
 
 static void omap2_mcspi_tx_dma(struct spi_device *spi,
-- 
1.8.1.rc1.5.g7e0651a

[PATCH 2/2] spi: devicetree: add support for loopback mode

[Felipe Balbi]

there are a few spi master drivers which make
use of that flag but there is no way to pass it
through devicetree.

This patch just creates a way to pass SPI_LOOP
via devicetree.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 Documentation/devicetree/bindings/spi/spi-bus.txt | 2 ++
 drivers/spi/spi.c                                 | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 296015e..1949586 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -55,6 +55,8 @@ contain the following properties.
     		chip select active high
 - spi-3wire       - (optional) Empty property indicating device requires
     		    3-wire mode.
+- spi-loopback    - (optional) Empty property indicating device requires
+    		    loopback mode.
 
 If a gpio chipselect is used for the SPI slave the gpio number will be passed
 via the cs_gpio
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3f1b9ee..6bcdc03 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -868,6 +868,8 @@ static void of_register_spi_devices(struct spi_master *master)
 			spi->mode |= SPI_CS_HIGH;
 		if (of_find_property(nc, "spi-3wire", NULL))
 			spi->mode |= SPI_3WIRE;
+		if (of_find_property(nc, "spi-loopback", NULL))
+			spi->mode |= SPI_LOOP;
 
 		/* Device speed */
 		prop = of_get_property(nc, "spi-max-frequency", &len);
-- 
1.8.1.rc1.5.g7e0651a

Re: [PATCH 2/2] spi: devicetree: add support for loopback mode

[Grant Likely]

On Wed, 12 Dec 2012 10:46:00 +0200, Felipe Balbi <balbi@ti.com> wrote:
> there are a few spi master drivers which make
> use of that flag but there is no way to pass it
> through devicetree.
> 
> This patch just creates a way to pass SPI_LOOP
> via devicetree.

I don't understand how this would be useful since loopback mode is
really just a test feature. Is there any reason to do loopback for
something other than test?

I think it would be better to add a sysfs or debugfs property to
manipulate the SPI_LOOP flag from userspace. What do you think?

g.

> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt | 2 ++
>  drivers/spi/spi.c                                 | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 296015e..1949586 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -55,6 +55,8 @@ contain the following properties.
>      		chip select active high
>  - spi-3wire       - (optional) Empty property indicating device requires
>      		    3-wire mode.
> +- spi-loopback    - (optional) Empty property indicating device requires
> +    		    loopback mode.
>  
>  If a gpio chipselect is used for the SPI slave the gpio number will be passed
>  via the cs_gpio
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 3f1b9ee..6bcdc03 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -868,6 +868,8 @@ static void of_register_spi_devices(struct spi_master *master)
>  			spi->mode |= SPI_CS_HIGH;
>  		if (of_find_property(nc, "spi-3wire", NULL))
>  			spi->mode |= SPI_3WIRE;
> +		if (of_find_property(nc, "spi-loopback", NULL))
> +			spi->mode |= SPI_LOOP;
>  
>  		/* Device speed */
>  		prop = of_get_property(nc, "spi-max-frequency", &len);
> -- 
> 1.8.1.rc1.5.g7e0651a
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 2/2] spi: devicetree: add support for loopback mode

[Felipe Balbi]

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

On Sat, Dec 15, 2012 at 12:32:24AM +0000, Grant Likely wrote:
> On Wed, 12 Dec 2012 10:46:00 +0200, Felipe Balbi <balbi@ti.com> wrote:
> > there are a few spi master drivers which make
> > use of that flag but there is no way to pass it
> > through devicetree.
> > 
> > This patch just creates a way to pass SPI_LOOP
> > via devicetree.
> 
> I don't understand how this would be useful since loopback mode is
> really just a test feature. Is there any reason to do loopback for
> something other than test?
> 
> I think it would be better to add a sysfs or debugfs property to
> manipulate the SPI_LOOP flag from userspace. What do you think?

might be nicer in the long run, indeed. Want me to look into it, or do
you wanna do it yourself ?

-- 
balbi

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

Re: [PATCH 2/2] spi: devicetree: add support for loopback mode

[Grant Likely]

On Sat, 15 Dec 2012 16:55:46 +0200, Felipe Balbi <balbi@ti.com> wrote:
> On Sat, Dec 15, 2012 at 12:32:24AM +0000, Grant Likely wrote:
> > On Wed, 12 Dec 2012 10:46:00 +0200, Felipe Balbi <balbi@ti.com> wrote:
> > > there are a few spi master drivers which make
> > > use of that flag but there is no way to pass it
> > > through devicetree.
> > > 
> > > This patch just creates a way to pass SPI_LOOP
> > > via devicetree.
> > 
> > I don't understand how this would be useful since loopback mode is
> > really just a test feature. Is there any reason to do loopback for
> > something other than test?
> > 
> > I think it would be better to add a sysfs or debugfs property to
> > manipulate the SPI_LOOP flag from userspace. What do you think?
> 
> might be nicer in the long run, indeed. Want me to look into it, or do
> you wanna do it yourself ?

Yes, please look into it. After all, you're the one who needs the feature/  :-)

g.

Re: [PATCH 1/2] spi: omap2: disable DMA requests before complete()

[Grant Likely]

On Wed, 12 Dec 2012 10:45:59 +0200, Felipe Balbi <balbi@ti.com> wrote:
> No actual errors have been found for completing
> before disabling DMA request lines, but it just
> looks more semantically correct that on our DMA
> callback we quiesce the whole thing before stating
> transfer is finished.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-omap2-mcspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> index b610f52..68446db 100644
> --- a/drivers/spi/spi-omap2-mcspi.c
> +++ b/drivers/spi/spi-omap2-mcspi.c
> @@ -298,10 +298,10 @@ static void omap2_mcspi_rx_callback(void *data)
>  	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
>  	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
>  
> -	complete(&mcspi_dma->dma_rx_completion);
> -
>  	/* We must disable the DMA RX request */
>  	omap2_mcspi_set_dma_req(spi, 1, 0);
> +
> +	complete(&mcspi_dma->dma_rx_completion);
>  }
>  
>  static void omap2_mcspi_tx_callback(void *data)
> @@ -310,10 +310,10 @@ static void omap2_mcspi_tx_callback(void *data)
>  	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
>  	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
>  
> -	complete(&mcspi_dma->dma_tx_completion);
> -
>  	/* We must disable the DMA TX request */
>  	omap2_mcspi_set_dma_req(spi, 0, 0);
> +
> +	complete(&mcspi_dma->dma_tx_completion);
>  }
>  
>  static void omap2_mcspi_tx_dma(struct spi_device *spi,
> -- 
> 1.8.1.rc1.5.g7e0651a
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.