[PATCH RFC 0/4] spi: dw: device tree and generic queue support

[PATCH RFC 0/4] spi: dw: device tree and generic queue support

[Baruch Siach]

Unfortunately, I probably have no chance access to thest this series on 
hardware, I's only compile tested. So I'm sending this as RFC for code review. 
If this series doesn't get merged, it may be used at least as a starting point 
for someone with hardware access.

These patches add the following:

   * Device tree support for the platform driver (dw-mmio)

   * GPIO controlled chip-select implementing the generic SPI GPIO CS binding

   * Generic queue migration

Comments are welcome.

Baruch Siach (4):
  spi: dw-mmio: allow multiple instances
  spi: dw-mmio: add device tree support
  spi: dw: add support for gpio controlled chip select
  spi: dw: migrate to generic queue infrastructure

 drivers/spi/spi-dw-mmio.c |  19 +++-
 drivers/spi/spi-dw.c      | 214 +++++++++-------------------------------------
 drivers/spi/spi-dw.h      |  14 ++-
 3 files changed, 63 insertions(+), 184 deletions(-)

-- 
1.8.5.2

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

[PATCH RFC 1/4] spi: dw-mmio: allow multiple instances

[Baruch Siach]

Use the platform_device id to uniquely identify each SPI master instance.

Cc: Jean-Hugues Deschenes <jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/spi/spi-dw-mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 9af56cdf1540..1492f5ee9aaa 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -66,7 +66,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	dws->bus_num = 0;
+	dws->bus_num = pdev->id;
 	dws->num_cs = 4;
 	dws->max_freq = clk_get_rate(dwsmmio->clk);
 
-- 
1.8.5.2

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

Re: [PATCH RFC 1/4] spi: dw-mmio: allow multiple instances

[Mark Brown]

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

On Thu, Jan 23, 2014 at 03:05:56PM +0200, Baruch Siach wrote:
> Use the platform_device id to uniquely identify each SPI master instance.

This is OK but it'll be annoying if anything is using it for a stable
dev_name() since it'll probably renumber.  Not sure there's much we can
do without platform data though.

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

[PATCH RFC 2/4] spi: dw-mmio: add device tree support

[Baruch Siach]

Cc: Jean-Hugues Deschenes <jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/spi/spi-dw-mmio.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 1492f5ee9aaa..b694f3f9b901 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -16,6 +16,7 @@
 #include <linux/spi/spi.h>
 #include <linux/scatterlist.h>
 #include <linux/module.h>
+#include <linux/of.h>
 
 #include "spi-dw.h"
 
@@ -32,6 +33,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	struct dw_spi *dws;
 	struct resource *mem;
 	int ret;
+	u32 tmp;
 
 	dwsmmio = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mmio),
 			GFP_KERNEL);
@@ -67,7 +69,11 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 		return ret;
 
 	dws->bus_num = pdev->id;
-	dws->num_cs = 4;
+	ret = of_property_read_u32(pdev->dev.of_node, "num-cs", &tmp);
+	if (ret == 0)
+		dws->num_cs = tmp;
+	else
+		dws->num_cs = 4;
 	dws->max_freq = clk_get_rate(dwsmmio->clk);
 
 	ret = dw_spi_add_host(&pdev->dev, dws);
@@ -92,12 +98,21 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id dw_spi_dt_ids[] = {
+	{ .compatible = "snps,designware-spi" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dw_spi_dt_ids);
+#endif
+
 static struct platform_driver dw_spi_mmio_driver = {
 	.probe		= dw_spi_mmio_probe,
 	.remove		= dw_spi_mmio_remove,
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(dw_spi_dt_ids),
 	},
 };
 module_platform_driver(dw_spi_mmio_driver);
-- 
1.8.5.2

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

Re: [PATCH RFC 2/4] spi: dw-mmio: add device tree support

[Mark Brown]

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

On Thu, Jan 23, 2014 at 03:05:57PM +0200, Baruch Siach wrote:
> Cc: Jean-Hugues Deschenes <jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
>  drivers/spi/spi-dw-mmio.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

DT bindings need a binding document.

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

[PATCH RFC 3/4] spi: dw: add support for gpio controlled chip select

[Baruch Siach]

Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/spi/spi-dw.c | 33 ++++++++++++++++++++++++++++++---
 drivers/spi/spi-dw.h |  6 +++++-
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index bf98d63d92b3..595761593492 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -24,6 +24,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <linux/gpio.h>
 
 #include "spi-dw.h"
 
@@ -265,6 +266,8 @@ static void giveback(struct dw_spi *dws)
 	struct spi_transfer *last_transfer;
 	unsigned long flags;
 	struct spi_message *msg;
+	int cs_gpio = dws->cur_msg->spi->cs_gpio;
+	u16 mode = dws->cur_msg->spi->mode;
 
 	spin_lock_irqsave(&dws->lock, flags);
 	msg = dws->cur_msg;
@@ -280,8 +283,12 @@ static void giveback(struct dw_spi *dws)
 					struct spi_transfer,
 					transfer_list);
 
-	if (!last_transfer->cs_change && dws->cs_control)
-		dws->cs_control(MRST_SPI_DEASSERT);
+	if (!last_transfer->cs_change) {
+		if (dws->cs_control)
+			dws->cs_control(MRST_SPI_DEASSERT);
+		if (gpio_is_valid(cs_gpio))
+			gpio_set_value(cs_gpio, !(mode & SPI_CS_HIGH));
+	}
 
 	msg->state = NULL;
 	if (msg->complete)
@@ -509,7 +516,8 @@ static void pump_transfers(unsigned long data)
 			dw_writew(dws, DW_SPI_CTRL0, cr0);
 
 		spi_set_clk(dws, clk_div ? clk_div : chip->clk_div);
-		spi_chip_sel(dws, spi->chip_select);
+		spi_chip_sel(dws, spi->chip_select, spi->cs_gpio,
+				spi->mode & SPI_CS_HIGH);
 
 		/* Set the interrupt mask, for poll mode just disable all int */
 		spi_mask_intr(dws, 0xff);
@@ -615,6 +623,7 @@ static int dw_spi_setup(struct spi_device *spi)
 {
 	struct dw_spi_chip *chip_info = NULL;
 	struct chip_data *chip;
+	int ret;
 
 	/* Only alloc on first setup */
 	chip = spi_get_ctldata(spi);
@@ -668,12 +677,30 @@ static int dw_spi_setup(struct spi_device *spi)
 			| (spi->mode  << SPI_MODE_OFFSET)
 			| (chip->tmode << SPI_TMOD_OFFSET);
 
+	if (gpio_is_valid(spi->cs_gpio)) {
+		ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
+		if (ret)
+			return ret;
+		ret = gpio_direction_output(spi->cs_gpio,
+				!(spi->mode & SPI_CS_HIGH));
+		if (ret)
+			goto err_gpio_free;
+	}
+
 	return 0;
+
+err_gpio_free:
+	if (gpio_is_valid(spi->cs_gpio))
+		gpio_free(spi->cs_gpio);
+
+	return ret;
 }
 
 static void dw_spi_cleanup(struct spi_device *spi)
 {
 	struct chip_data *chip = spi_get_ctldata(spi);
+	if (gpio_is_valid(spi->cs_gpio))
+		gpio_free(spi->cs_gpio);
 	kfree(chip);
 }
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 587643dae11e..4a3a6d764b48 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -3,6 +3,7 @@
 
 #include <linux/io.h>
 #include <linux/scatterlist.h>
+#include <linux/gpio.h>
 
 /* Register offsets */
 #define DW_SPI_CTRL0			0x00
@@ -186,13 +187,16 @@ static inline void spi_set_clk(struct dw_spi *dws, u16 div)
 	dw_writel(dws, DW_SPI_BAUDR, div);
 }
 
-static inline void spi_chip_sel(struct dw_spi *dws, u16 cs)
+static inline void spi_chip_sel(struct dw_spi *dws, u16 cs, int cs_gpio,
+		int gpio_active_val)
 {
 	if (cs > dws->num_cs)
 		return;
 
 	if (dws->cs_control)
 		dws->cs_control(1);
+	if (gpio_is_valid(cs_gpio))
+		gpio_set_value(cs_gpio, gpio_active_val);
 
 	dw_writel(dws, DW_SPI_SER, 1 << cs);
 }
-- 
1.8.5.2

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

Re: [PATCH RFC 3/4] spi: dw: add support for gpio controlled chip select

[Mark Brown]

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

On Thu, Jan 23, 2014 at 03:05:58PM +0200, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>

If you convert to the generic queue then there's generic support for
managing a GPIO based /CS if you set cs_gpio in the spi_device.  We
should extend that for GPIO descriptors too.

> +	if (gpio_is_valid(spi->cs_gpio)) {
> +		ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
> +		if (ret)
> +			return ret;
> +		ret = gpio_direction_output(spi->cs_gpio,
> +				!(spi->mode & SPI_CS_HIGH));

devm_gpio_request_one().

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

Re: [PATCH RFC 3/4] spi: dw: add support for gpio controlled chip select

[Baruch Siach]

Hi Mark,

On Thu, Jan 23, 2014 at 01:26:23PM +0000, Mark Brown wrote:
> On Thu, Jan 23, 2014 at 03:05:58PM +0200, Baruch Siach wrote:
> > Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> 
> If you convert to the generic queue then there's generic support for
> managing a GPIO based /CS if you set cs_gpio in the spi_device.  We
> should extend that for GPIO descriptors too.

Thank. This further simplifies things. AFAICS the generic gpio chip select 
code does not request the GPIO. Is there a reason for that?

> > +	if (gpio_is_valid(spi->cs_gpio)) {
> > +		ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
> > +		if (ret)
> > +			return ret;
> > +		ret = gpio_direction_output(spi->cs_gpio,
> > +				!(spi->mode & SPI_CS_HIGH));
> 
> devm_gpio_request_one().

Right.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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

Re: [PATCH RFC 3/4] spi: dw: add support for gpio controlled chip select

[Mark Brown]

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

On Thu, Jan 23, 2014 at 04:05:46PM +0200, Baruch Siach wrote:
> On Thu, Jan 23, 2014 at 01:26:23PM +0000, Mark Brown wrote:

> > If you convert to the generic queue then there's generic support for
> > managing a GPIO based /CS if you set cs_gpio in the spi_device.  We
> > should extend that for GPIO descriptors too.

> Thank. This further simplifies things. AFAICS the generic gpio chip select 
> code does not request the GPIO. Is there a reason for that?

Mostly because it might not interact so well with deferred probe, lots
of drivers only request/figure out the GPIOs in setup at the minute but
for deferred probe we ought to be doing it as part of the main driver
probe().  We should also be switching away to the GPIO descriptor API is
part of it, but also I want to do a pass and add standard DT bindings
for this.

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

Re: [PATCH RFC 3/4] spi: dw: add support for gpio controlled chip select

[Baruch Siach]

Hi Mark,

On Thu, Jan 23, 2014 at 01:26:23PM +0000, Mark Brown wrote:
> On Thu, Jan 23, 2014 at 03:05:58PM +0200, Baruch Siach wrote:
> > Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> 
> If you convert to the generic queue then there's generic support for
> managing a GPIO based /CS if you set cs_gpio in the spi_device.  We
> should extend that for GPIO descriptors too.

Generic queue GPIO chip-select management depends on switching from 
transfer_one_message to transfer_one as you suggested in another email. 
Unfortunately there is just too much logic going on in the pump_transfers 
workqueue, and I can't expect it to work correctly without testing on real 
hardware.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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

[PATCH RFC 4/4] spi: dw: migrate to generic queue infrastructure

[Baruch Siach]

Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/spi/spi-dw.c | 183 ++++-----------------------------------------------
 drivers/spi/spi-dw.h |   8 ---
 2 files changed, 12 insertions(+), 179 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 595761593492..39a57adaf1fd 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -37,9 +37,6 @@
 #define DONE_STATE	((void *)2)
 #define ERROR_STATE	((void *)-1)
 
-#define QUEUE_RUNNING	0
-#define QUEUE_STOPPED	1
-
 #define MRST_SPI_DEASSERT	0
 #define MRST_SPI_ASSERT		1
 
@@ -264,20 +261,16 @@ static int map_dma_buffers(struct dw_spi *dws)
 static void giveback(struct dw_spi *dws)
 {
 	struct spi_transfer *last_transfer;
-	unsigned long flags;
 	struct spi_message *msg;
 	int cs_gpio = dws->cur_msg->spi->cs_gpio;
 	u16 mode = dws->cur_msg->spi->mode;
 
-	spin_lock_irqsave(&dws->lock, flags);
 	msg = dws->cur_msg;
 	dws->cur_msg = NULL;
 	dws->cur_transfer = NULL;
 	dws->prev_chip = dws->cur_chip;
 	dws->cur_chip = NULL;
 	dws->dma_mapped = 0;
-	queue_work(dws->workqueue, &dws->pump_messages);
-	spin_unlock_irqrestore(&dws->lock, flags);
 
 	last_transfer = list_entry(msg->transfers.prev,
 					struct spi_transfer,
@@ -290,9 +283,7 @@ static void giveback(struct dw_spi *dws)
 			gpio_set_value(cs_gpio, !(mode & SPI_CS_HIGH));
 	}
 
-	msg->state = NULL;
-	if (msg->complete)
-		msg->complete(msg->context);
+	spi_finalize_current_message(dws->master);
 }
 
 static void int_error_stop(struct dw_spi *dws, const char *msg)
@@ -544,30 +535,12 @@ early_exit:
 	return;
 }
 
-static void pump_messages(struct work_struct *work)
+static int dw_spi_transfer_one_message(struct spi_master *master,
+		struct spi_message *msg)
 {
-	struct dw_spi *dws =
-		container_of(work, struct dw_spi, pump_messages);
-	unsigned long flags;
-
-	/* Lock queue and check for queue work */
-	spin_lock_irqsave(&dws->lock, flags);
-	if (list_empty(&dws->queue) || dws->run == QUEUE_STOPPED) {
-		dws->busy = 0;
-		spin_unlock_irqrestore(&dws->lock, flags);
-		return;
-	}
-
-	/* Make sure we are not already running a message */
-	if (dws->cur_msg) {
-		spin_unlock_irqrestore(&dws->lock, flags);
-		return;
-	}
-
-	/* Extract head of queue */
-	dws->cur_msg = list_entry(dws->queue.next, struct spi_message, queue);
-	list_del_init(&dws->cur_msg->queue);
+	struct dw_spi *dws = spi_master_get_devdata(master);
 
+	dws->cur_msg = msg;
 	/* Initial message state*/
 	dws->cur_msg->state = START_STATE;
 	dws->cur_transfer = list_entry(dws->cur_msg->transfers.next,
@@ -575,46 +548,9 @@ static void pump_messages(struct work_struct *work)
 						transfer_list);
 	dws->cur_chip = spi_get_ctldata(dws->cur_msg->spi);
 
-	/* Mark as busy and launch transfers */
+	/* Launch transfers */
 	tasklet_schedule(&dws->pump_transfers);
 
-	dws->busy = 1;
-	spin_unlock_irqrestore(&dws->lock, flags);
-}
-
-/* spi_device use this to queue in their spi_msg */
-static int dw_spi_transfer(struct spi_device *spi, struct spi_message *msg)
-{
-	struct dw_spi *dws = spi_master_get_devdata(spi->master);
-	unsigned long flags;
-
-	spin_lock_irqsave(&dws->lock, flags);
-
-	if (dws->run == QUEUE_STOPPED) {
-		spin_unlock_irqrestore(&dws->lock, flags);
-		return -ESHUTDOWN;
-	}
-
-	msg->actual_length = 0;
-	msg->status = -EINPROGRESS;
-	msg->state = START_STATE;
-
-	list_add_tail(&msg->queue, &dws->queue);
-
-	if (dws->run == QUEUE_RUNNING && !dws->busy) {
-
-		if (dws->cur_transfer || dws->cur_msg)
-			queue_work(dws->workqueue,
-					&dws->pump_messages);
-		else {
-			/* If no other data transaction in air, just go */
-			spin_unlock_irqrestore(&dws->lock, flags);
-			pump_messages(&dws->pump_messages);
-			return 0;
-		}
-	}
-
-	spin_unlock_irqrestore(&dws->lock, flags);
 	return 0;
 }
 
@@ -704,81 +640,6 @@ static void dw_spi_cleanup(struct spi_device *spi)
 	kfree(chip);
 }
 
-static int init_queue(struct dw_spi *dws)
-{
-	INIT_LIST_HEAD(&dws->queue);
-	spin_lock_init(&dws->lock);
-
-	dws->run = QUEUE_STOPPED;
-	dws->busy = 0;
-
-	tasklet_init(&dws->pump_transfers,
-			pump_transfers,	(unsigned long)dws);
-
-	INIT_WORK(&dws->pump_messages, pump_messages);
-	dws->workqueue = create_singlethread_workqueue(
-					dev_name(dws->master->dev.parent));
-	if (dws->workqueue == NULL)
-		return -EBUSY;
-
-	return 0;
-}
-
-static int start_queue(struct dw_spi *dws)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dws->lock, flags);
-
-	if (dws->run == QUEUE_RUNNING || dws->busy) {
-		spin_unlock_irqrestore(&dws->lock, flags);
-		return -EBUSY;
-	}
-
-	dws->run = QUEUE_RUNNING;
-	dws->cur_msg = NULL;
-	dws->cur_transfer = NULL;
-	dws->cur_chip = NULL;
-	dws->prev_chip = NULL;
-	spin_unlock_irqrestore(&dws->lock, flags);
-
-	queue_work(dws->workqueue, &dws->pump_messages);
-
-	return 0;
-}
-
-static int stop_queue(struct dw_spi *dws)
-{
-	unsigned long flags;
-	unsigned limit = 50;
-	int status = 0;
-
-	spin_lock_irqsave(&dws->lock, flags);
-	dws->run = QUEUE_STOPPED;
-	while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
-		spin_unlock_irqrestore(&dws->lock, flags);
-		msleep(10);
-		spin_lock_irqsave(&dws->lock, flags);
-	}
-
-	if (!list_empty(&dws->queue) || dws->busy)
-		status = -EBUSY;
-	spin_unlock_irqrestore(&dws->lock, flags);
-
-	return status;
-}
-
-static int destroy_queue(struct dw_spi *dws)
-{
-	int status;
-
-	status = stop_queue(dws);
-	if (status != 0)
-		return status;
-	destroy_workqueue(dws->workqueue);
-	return 0;
-}
-
 /* Restart the controller, disable all interrupts, clean rx fifo */
 static void spi_hw_init(struct dw_spi *dws)
 {
@@ -835,7 +696,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	master->num_chipselect = dws->num_cs;
 	master->cleanup = dw_spi_cleanup;
 	master->setup = dw_spi_setup;
-	master->transfer = dw_spi_transfer;
+	master->transfer_one_message = dw_spi_transfer_one_message;
 
 	/* Basic HW init */
 	spi_hw_init(dws);
@@ -848,33 +709,21 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 		}
 	}
 
-	/* Initial and start queue */
-	ret = init_queue(dws);
-	if (ret) {
-		dev_err(&master->dev, "problem initializing queue\n");
-		goto err_diable_hw;
-	}
-	ret = start_queue(dws);
-	if (ret) {
-		dev_err(&master->dev, "problem starting queue\n");
-		goto err_diable_hw;
-	}
+	tasklet_init(&dws->pump_transfers, pump_transfers, (unsigned long)dws);
 
 	spi_master_set_devdata(master, dws);
 	ret = devm_spi_register_master(dev, master);
 	if (ret) {
 		dev_err(&master->dev, "problem registering spi master\n");
-		goto err_queue_alloc;
+		goto err_dma_exit;
 	}
 
 	mrst_spi_debugfs_init(dws);
 	return 0;
 
-err_queue_alloc:
-	destroy_queue(dws);
+err_dma_exit:
 	if (dws->dma_ops && dws->dma_ops->dma_exit)
 		dws->dma_ops->dma_exit(dws);
-err_diable_hw:
 	spi_enable_chip(dws, 0);
 err_free_master:
 	spi_master_put(master);
@@ -884,18 +733,10 @@ EXPORT_SYMBOL_GPL(dw_spi_add_host);
 
 void dw_spi_remove_host(struct dw_spi *dws)
 {
-	int status = 0;
-
 	if (!dws)
 		return;
 	mrst_spi_debugfs_remove(dws);
 
-	/* Remove the queue */
-	status = destroy_queue(dws);
-	if (status != 0)
-		dev_err(&dws->master->dev,
-			"dw_spi_remove: workqueue will not complete, message memory not freed\n");
-
 	if (dws->dma_ops && dws->dma_ops->dma_exit)
 		dws->dma_ops->dma_exit(dws);
 	spi_enable_chip(dws, 0);
@@ -908,7 +749,7 @@ int dw_spi_suspend_host(struct dw_spi *dws)
 {
 	int ret = 0;
 
-	ret = stop_queue(dws);
+	ret = spi_master_suspend(dws->master);
 	if (ret)
 		return ret;
 	spi_enable_chip(dws, 0);
@@ -922,7 +763,7 @@ int dw_spi_resume_host(struct dw_spi *dws)
 	int ret;
 
 	spi_hw_init(dws);
-	ret = start_queue(dws);
+	ret = spi_master_resume(dws->master);
 	if (ret)
 		dev_err(&dws->master->dev, "fail to start queue (%d)\n", ret);
 	return ret;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 4a3a6d764b48..39844f262bd1 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -105,14 +105,6 @@ struct dw_spi {
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */
 
-	/* Driver message queue */
-	struct workqueue_struct	*workqueue;
-	struct work_struct	pump_messages;
-	spinlock_t		lock;
-	struct list_head	queue;
-	int			busy;
-	int			run;
-
 	/* Message Transfer pump */
 	struct tasklet_struct	pump_transfers;
 
-- 
1.8.5.2

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

Re: [PATCH RFC 4/4] spi: dw: migrate to generic queue infrastructure

[Mark Brown]

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

On Thu, Jan 23, 2014 at 03:05:59PM +0200, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>

This looks good from a scan through, can you go futher and use
transfer_one()?

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

Re: [PATCH RFC 4/4] spi: dw: migrate to generic queue infrastructure

[Baruch Siach]

Hi Mark,

On Thu, Jan 23, 2014 at 07:51:48PM +0000, Mark Brown wrote:
> On Thu, Jan 23, 2014 at 03:05:59PM +0200, Baruch Siach wrote:
> > Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> 
> This looks good from a scan through, can you go futher and use
> transfer_one()?

I'll look into it. I had to dig in include/linux/spi/spi.h, 'git blame' and 
commit b158935f70b9c to see how it all works together. The 
Documentation/spi/spi-summary doesn't mention the transfer_one callback. The 
kerneldoc in spi.h should make it clear that transfer_one and 
transfer_one_message are mutually exclusive.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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

Re: [PATCH RFC 4/4] spi: dw: migrate to generic queue infrastructure

[Mark Brown]

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

On Thu, Jan 23, 2014 at 10:07:10PM +0200, Baruch Siach wrote:

> I'll look into it. I had to dig in include/linux/spi/spi.h, 'git blame' and 
> commit b158935f70b9c to see how it all works together. The 
> Documentation/spi/spi-summary doesn't mention the transfer_one callback. The 
> kerneldoc in spi.h should make it clear that transfer_one and 
> transfer_one_message are mutually exclusive.

Feel free to submit documentation patches...  I'd have thought that
transfer_one_message() and transfer_one() being exclusive was fairly
obvious given that the former takes an entire message.

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

Re: [PATCH RFC 4/4] spi: dw: migrate to generic queue infrastructure

[Baruch Siach]

Hi Mark,

On Thu, Jan 23, 2014 at 08:13:06PM +0000, Mark Brown wrote:
> On Thu, Jan 23, 2014 at 10:07:10PM +0200, Baruch Siach wrote:
> > I'll look into it. I had to dig in include/linux/spi/spi.h, 'git blame' 
> > and commit b158935f70b9c to see how it all works together. The 
> > Documentation/spi/spi-summary doesn't mention the transfer_one callback. 
> > The kerneldoc in spi.h should make it clear that transfer_one and 
> > transfer_one_message are mutually exclusive.
> 
> Feel free to submit documentation patches...

Done.

> I'd have thought that transfer_one_message() and transfer_one() being 
> exclusive was fairly obvious given that the former takes an entire message.

IMO, adding this small clarification would better serve newcomers. I think 
there is also a need for some additional explanation on the difference between 
these two callback, and why a driver should implement one over the other. I 
hope to look into it when I have more experience with the new API.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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