[PATCH 00/11] spi/pxa2xx: add Intel Lynxpoint SPI controller support

[PATCH 00/11] spi/pxa2xx: add Intel Lynxpoint SPI controller support

[Mika Westerberg]

Hi all,

On Intel Lynxpoint (the PCH used with Haswell) we have two SPI controllers
that reside in the Low Power Subsystem of the PCH. The hardware is mostly
compliant with PXA2xx SPI controller except that there are few additional
registers. Those are described in patch [10/11].

The patches [1-6/11] implement the foundation:

	* allow building the driver on 64-bit kernel
	* allow passing the ssp_device from platform data
	* allow passing clock rate from platform data on arches that don't
	  support common clk framework like x86
	* separate the PXA private DMA implementation out

The following patches [7-11/11] then build on top of this foundation so
that we support the generic DMA engine API, runtime power management, and
finally the Intel Lynxpoint SPI controller that is discoverable from ACPI
namespace.

Tested with DMA and PIO modes on Haswell/Lynxpoint machine.

This shouldn't break anything but I haven't been able to test on PXA (due
to lack of hardware) so I would appreciate if someone who has the hardware
could try them out.

Note that checkpatch.pl gives few warnings about using CamelCase for
identifiers but I decided to be consistent with the original style instead.
Let me know if it is not acceptable and I'll change them.

The series is based on 3.8-rc2.

Mika Westerberg (11):
  spi/pxa2xx: allow building on a 64-bit kernel
  spi/pxa2xx: convert to the pump message infrastructure
  spi/pxa2xx-pci: switch to use pcim_* interfaces
  spi/pxa2xx: embed the ssp_device to platform data
  spi/pxa2xx: make clock rate configurable from platform data
  spi/pxa2xx: use the private DMA API only when CONFIG_ARCH_PXA is set
  spi/pxa2xx: add support for DMA engine
  spi/pxa2xx: add support for runtime PM
  spi/pxa2xx: add support for SPI_LOOP
  spi/pxa2xx: add support for Intel Low Power Subsystem SPI
  spi/pxa2xx: add support for Lynxpoint SPI controllers

 drivers/spi/Kconfig            |    4 +-
 drivers/spi/spi-pxa2xx-pci.c   |  134 +---
 drivers/spi/spi-pxa2xx.c       | 1516 +++++++++++++++++++++++++++-------------
 include/linux/pxa2xx_ssp.h     |   18 +
 include/linux/spi/pxa2xx_spi.h |  109 +--
 5 files changed, 1103 insertions(+), 678 deletions(-)

-- 
1.7.10.4

[PATCH 01/11] spi/pxa2xx: allow building on a 64-bit kernel

[Mika Westerberg]

In addition fix following warnings seen when compiling 64-bit:

drivers/spi/spi-pxa2xx.c: In function ‘map_dma_buffers’: drivers/spi/spi-pxa2xx.c:384:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
drivers/spi/spi-pxa2xx.c:384:40: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
drivers/spi/spi-pxa2xx.c: In function ‘pxa2xx_spi_probe’:
drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
drivers/spi/spi-pxa2xx.c:1572:27: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/Kconfig      |    4 ++--
 drivers/spi/spi-pxa2xx.c |    5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 2e188e1..a90393d 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -299,7 +299,7 @@ config SPI_PPC4xx
 
 config SPI_PXA2XX
 	tristate "PXA2xx SSP SPI master"
-	depends on (ARCH_PXA || (X86_32 && PCI)) && EXPERIMENTAL
+	depends on ARCH_PXA || PCI
 	select PXA_SSP if ARCH_PXA
 	help
 	  This enables using a PXA2xx or Sodaville SSP port as a SPI master
@@ -307,7 +307,7 @@ config SPI_PXA2XX
 	  additional documentation can be found a Documentation/spi/pxa2xx.
 
 config SPI_PXA2XX_PCI
-	def_bool SPI_PXA2XX && X86_32 && PCI
+	def_tristate SPI_PXA2XX && PCI
 
 config SPI_RSPI
 	tristate "Renesas RSPI controller"
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 5c8c4f5..7fac65d 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -47,7 +47,7 @@ MODULE_ALIAS("platform:pxa2xx-spi");
 
 #define DMA_INT_MASK		(DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
 #define RESET_DMA_CHANNEL	(DCSR_NODESC | DMA_INT_MASK)
-#define IS_DMA_ALIGNED(x)	((((u32)(x)) & 0x07) == 0)
+#define IS_DMA_ALIGNED(x)	IS_ALIGNED((unsigned long)x, DMA_ALIGNMENT)
 #define MAX_DMA_LEN		8191
 #define DMA_ALIGNMENT		8
 
@@ -1569,8 +1569,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	master->transfer = transfer;
 
 	drv_data->ssp_type = ssp->type;
-	drv_data->null_dma_buf = (u32 *)ALIGN((u32)(drv_data +
-						sizeof(struct driver_data)), 8);
+	drv_data->null_dma_buf = (u32 *)PTR_ALIGN(drv_data + 1, 8);
 
 	drv_data->ioaddr = ssp->mmio_base;
 	drv_data->ssdr_physical = ssp->phys_base + SSDR;
-- 
1.7.10.4

[PATCH 02/11] spi/pxa2xx: convert to the pump message infrastructure

[Mika Westerberg]

The SPI core provides infrastructure for standard message queueing so use
that instead of handling everything in the driver. This simplifies the
driver.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c |  212 +++-------------------------------------------
 1 file changed, 12 insertions(+), 200 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 7fac65d..ad842eb 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -85,9 +85,6 @@ DEFINE_SSP_REG(SSPSP, 0x2c)
 #define DONE_STATE ((void*)2)
 #define ERROR_STATE ((void*)-1)
 
-#define QUEUE_RUNNING 0
-#define QUEUE_STOPPED 1
-
 struct driver_data {
 	/* Driver model hookup */
 	struct platform_device *pdev;
@@ -117,14 +114,6 @@ struct driver_data {
 	u32 clear_sr;
 	u32 mask_sr;
 
-	/* 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;
 
@@ -173,8 +162,6 @@ struct chip_data {
 	void (*cs_control)(u32 command);
 };
 
-static void pump_messages(struct work_struct *work);
-
 static void cs_assert(struct driver_data *drv_data)
 {
 	struct chip_data *chip = drv_data->cur_chip;
@@ -444,15 +431,11 @@ static void unmap_dma_buffers(struct driver_data *drv_data)
 static void giveback(struct driver_data *drv_data)
 {
 	struct spi_transfer* last_transfer;
-	unsigned long flags;
 	struct spi_message *msg;
 
-	spin_lock_irqsave(&drv_data->lock, flags);
 	msg = drv_data->cur_msg;
 	drv_data->cur_msg = NULL;
 	drv_data->cur_transfer = NULL;
-	queue_work(drv_data->workqueue, &drv_data->pump_messages);
-	spin_unlock_irqrestore(&drv_data->lock, flags);
 
 	last_transfer = list_entry(msg->transfers.prev,
 					struct spi_transfer,
@@ -481,13 +464,7 @@ static void giveback(struct driver_data *drv_data)
 		 */
 
 		/* get a pointer to the next message, if any */
-		spin_lock_irqsave(&drv_data->lock, flags);
-		if (list_empty(&drv_data->queue))
-			next_msg = NULL;
-		else
-			next_msg = list_entry(drv_data->queue.next,
-					struct spi_message, queue);
-		spin_unlock_irqrestore(&drv_data->lock, flags);
+		next_msg = spi_get_next_queued_message(drv_data->master);
 
 		/* see if the next and current messages point
 		 * to the same chip
@@ -498,10 +475,7 @@ static void giveback(struct driver_data *drv_data)
 			cs_deassert(drv_data);
 	}
 
-	msg->state = NULL;
-	if (msg->complete)
-		msg->complete(msg->context);
-
+	spi_finalize_current_message(drv_data->master);
 	drv_data->cur_chip = NULL;
 }
 
@@ -1176,31 +1150,12 @@ static void pump_transfers(unsigned long data)
 	write_SSCR1(cr1, reg);
 }
 
-static void pump_messages(struct work_struct *work)
+static int pxa2xx_spi_transfer_one_message(struct spi_master *master,
+					   struct spi_message *msg)
 {
-	struct driver_data *drv_data =
-		container_of(work, struct driver_data, pump_messages);
-	unsigned long flags;
-
-	/* Lock queue and check for queue work */
-	spin_lock_irqsave(&drv_data->lock, flags);
-	if (list_empty(&drv_data->queue) || drv_data->run == QUEUE_STOPPED) {
-		drv_data->busy = 0;
-		spin_unlock_irqrestore(&drv_data->lock, flags);
-		return;
-	}
-
-	/* Make sure we are not already running a message */
-	if (drv_data->cur_msg) {
-		spin_unlock_irqrestore(&drv_data->lock, flags);
-		return;
-	}
-
-	/* Extract head of queue */
-	drv_data->cur_msg = list_entry(drv_data->queue.next,
-					struct spi_message, queue);
-	list_del_init(&drv_data->cur_msg->queue);
+	struct driver_data *drv_data = spi_master_get_devdata(master);
 
+	drv_data->cur_msg = msg;
 	/* Initial message state*/
 	drv_data->cur_msg->state = START_STATE;
 	drv_data->cur_transfer = list_entry(drv_data->cur_msg->transfers.next,
@@ -1213,34 +1168,6 @@ static void pump_messages(struct work_struct *work)
 
 	/* Mark as busy and launch transfers */
 	tasklet_schedule(&drv_data->pump_transfers);
-
-	drv_data->busy = 1;
-	spin_unlock_irqrestore(&drv_data->lock, flags);
-}
-
-static int transfer(struct spi_device *spi, struct spi_message *msg)
-{
-	struct driver_data *drv_data = spi_master_get_devdata(spi->master);
-	unsigned long flags;
-
-	spin_lock_irqsave(&drv_data->lock, flags);
-
-	if (drv_data->run == QUEUE_STOPPED) {
-		spin_unlock_irqrestore(&drv_data->lock, flags);
-		return -ESHUTDOWN;
-	}
-
-	msg->actual_length = 0;
-	msg->status = -EINPROGRESS;
-	msg->state = START_STATE;
-
-	list_add_tail(&msg->queue, &drv_data->queue);
-
-	if (drv_data->run == QUEUE_RUNNING && !drv_data->busy)
-		queue_work(drv_data->workqueue, &drv_data->pump_messages);
-
-	spin_unlock_irqrestore(&drv_data->lock, flags);
-
 	return 0;
 }
 
@@ -1438,94 +1365,6 @@ static void cleanup(struct spi_device *spi)
 	kfree(chip);
 }
 
-static int init_queue(struct driver_data *drv_data)
-{
-	INIT_LIST_HEAD(&drv_data->queue);
-	spin_lock_init(&drv_data->lock);
-
-	drv_data->run = QUEUE_STOPPED;
-	drv_data->busy = 0;
-
-	tasklet_init(&drv_data->pump_transfers,
-			pump_transfers,	(unsigned long)drv_data);
-
-	INIT_WORK(&drv_data->pump_messages, pump_messages);
-	drv_data->workqueue = create_singlethread_workqueue(
-				dev_name(drv_data->master->dev.parent));
-	if (drv_data->workqueue == NULL)
-		return -EBUSY;
-
-	return 0;
-}
-
-static int start_queue(struct driver_data *drv_data)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&drv_data->lock, flags);
-
-	if (drv_data->run == QUEUE_RUNNING || drv_data->busy) {
-		spin_unlock_irqrestore(&drv_data->lock, flags);
-		return -EBUSY;
-	}
-
-	drv_data->run = QUEUE_RUNNING;
-	drv_data->cur_msg = NULL;
-	drv_data->cur_transfer = NULL;
-	drv_data->cur_chip = NULL;
-	spin_unlock_irqrestore(&drv_data->lock, flags);
-
-	queue_work(drv_data->workqueue, &drv_data->pump_messages);
-
-	return 0;
-}
-
-static int stop_queue(struct driver_data *drv_data)
-{
-	unsigned long flags;
-	unsigned limit = 500;
-	int status = 0;
-
-	spin_lock_irqsave(&drv_data->lock, flags);
-
-	/* This is a bit lame, but is optimized for the common execution path.
-	 * A wait_queue on the drv_data->busy could be used, but then the common
-	 * execution path (pump_messages) would be required to call wake_up or
-	 * friends on every SPI message. Do this instead */
-	drv_data->run = QUEUE_STOPPED;
-	while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
-		spin_unlock_irqrestore(&drv_data->lock, flags);
-		msleep(10);
-		spin_lock_irqsave(&drv_data->lock, flags);
-	}
-
-	if (!list_empty(&drv_data->queue) || drv_data->busy)
-		status = -EBUSY;
-
-	spin_unlock_irqrestore(&drv_data->lock, flags);
-
-	return status;
-}
-
-static int destroy_queue(struct driver_data *drv_data)
-{
-	int status;
-
-	status = stop_queue(drv_data);
-	/* we are unloading the module or failing to load (only two calls
-	 * to this routine), and neither call can handle a return value.
-	 * However, destroy_workqueue calls flush_workqueue, and that will
-	 * block until all work is done.  If the reason that stop_queue
-	 * timed out is that the work will never finish, then it does no
-	 * good to call destroy_workqueue, so return anyway. */
-	if (status != 0)
-		return status;
-
-	destroy_workqueue(drv_data->workqueue);
-
-	return 0;
-}
-
 static int pxa2xx_spi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1566,7 +1405,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	master->dma_alignment = DMA_ALIGNMENT;
 	master->cleanup = cleanup;
 	master->setup = setup;
-	master->transfer = transfer;
+	master->transfer_one_message = pxa2xx_spi_transfer_one_message;
 
 	drv_data->ssp_type = ssp->type;
 	drv_data->null_dma_buf = (u32 *)PTR_ALIGN(drv_data + 1, 8);
@@ -1639,31 +1478,19 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 		write_SSTO(0, drv_data->ioaddr);
 	write_SSPSP(0, drv_data->ioaddr);
 
-	/* Initial and start queue */
-	status = init_queue(drv_data);
-	if (status != 0) {
-		dev_err(&pdev->dev, "problem initializing queue\n");
-		goto out_error_clock_enabled;
-	}
-	status = start_queue(drv_data);
-	if (status != 0) {
-		dev_err(&pdev->dev, "problem starting queue\n");
-		goto out_error_clock_enabled;
-	}
+	tasklet_init(&drv_data->pump_transfers, pump_transfers,
+		     (unsigned long)drv_data);
 
 	/* Register with the SPI framework */
 	platform_set_drvdata(pdev, drv_data);
 	status = spi_register_master(master);
 	if (status != 0) {
 		dev_err(&pdev->dev, "problem registering spi master\n");
-		goto out_error_queue_alloc;
+		goto out_error_clock_enabled;
 	}
 
 	return status;
 
-out_error_queue_alloc:
-	destroy_queue(drv_data);
-
 out_error_clock_enabled:
 	clk_disable(ssp->clk);
 
@@ -1686,26 +1513,11 @@ static int pxa2xx_spi_remove(struct platform_device *pdev)
 {
 	struct driver_data *drv_data = platform_get_drvdata(pdev);
 	struct ssp_device *ssp;
-	int status = 0;
 
 	if (!drv_data)
 		return 0;
 	ssp = drv_data->ssp;
 
-	/* Remove the queue */
-	status = destroy_queue(drv_data);
-	if (status != 0)
-		/* the kernel does not check the return status of this
-		 * this routine (mod->exit, within the kernel).  Therefore
-		 * nothing is gained by returning from here, the module is
-		 * going away regardless, and we should not leave any more
-		 * resources allocated than necessary.  We cannot free the
-		 * message memory in drv_data->queue, but we can release the
-		 * resources below.  I think the kernel should honor -EBUSY
-		 * returns but... */
-		dev_err(&pdev->dev, "pxa2xx_spi_remove: workqueue will not "
-			"complete, message memory not freed\n");
-
 	/* Disable the SSP at the peripheral and SOC level */
 	write_SSCR0(0, drv_data->ioaddr);
 	clk_disable(ssp->clk);
@@ -1748,7 +1560,7 @@ static int pxa2xx_spi_suspend(struct device *dev)
 	struct ssp_device *ssp = drv_data->ssp;
 	int status = 0;
 
-	status = stop_queue(drv_data);
+	status = spi_master_suspend(drv_data->master);
 	if (status != 0)
 		return status;
 	write_SSCR0(0, drv_data->ioaddr);
@@ -1774,7 +1586,7 @@ static int pxa2xx_spi_resume(struct device *dev)
 	clk_enable(ssp->clk);
 
 	/* Start the queue running */
-	status = start_queue(drv_data);
+	status = spi_master_resume(drv_data->master);
 	if (status != 0) {
 		dev_err(dev, "problem starting queue (%d)\n", status);
 		return status;
-- 
1.7.10.4

[PATCH 03/11] spi/pxa2xx-pci: switch to use pcim_* interfaces

[Mika Westerberg]

Instead of open-coding all the error management in the driver we can take
advantage of the pcim_* interfaces that release the resources automatically.

We also use platform_device_register_full() to register the platform device
because it allows us to create and register the platform device at one go,
simplifying the error management.

This a preparatory step for getting rid of pxa_ssp_request()/free() which
we will do in the next patch.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-pci.c |   86 +++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 56 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index cf95587..ece979e 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -54,101 +54,75 @@ EXPORT_SYMBOL_GPL(pxa_ssp_free);
 static int ce4100_spi_probe(struct pci_dev *dev,
 		const struct pci_device_id *ent)
 {
+	struct platform_device_info pi;
 	int ret;
-	resource_size_t phys_beg;
-	resource_size_t phys_len;
 	struct ce4100_info *spi_info;
 	struct platform_device *pdev;
 	struct pxa2xx_spi_master spi_pdata;
 	struct ssp_device *ssp;
 
-	ret = pci_enable_device(dev);
+	ret = pcim_enable_device(dev);
 	if (ret)
 		return ret;
 
-	phys_beg = pci_resource_start(dev, 0);
-	phys_len = pci_resource_len(dev, 0);
-
-	if (!request_mem_region(phys_beg, phys_len,
-				"CE4100 SPI")) {
-		dev_err(&dev->dev, "Can't request register space.\n");
-		ret = -EBUSY;
+	ret = pcim_iomap_regions(dev, 1 << 0, "PXA2xx SPI");
+	if (!ret)
 		return ret;
-	}
 
-	pdev = platform_device_alloc("pxa2xx-spi", dev->devfn);
-	spi_info = kzalloc(sizeof(*spi_info), GFP_KERNEL);
-	if (!pdev || !spi_info ) {
-		ret = -ENOMEM;
-		goto err_nomem;
-	}
+	spi_info = devm_kzalloc(&dev->dev, sizeof(*spi_info), GFP_KERNEL);
+	if (!spi_info)
+		return -ENOMEM;
+
 	memset(&spi_pdata, 0, sizeof(spi_pdata));
 	spi_pdata.num_chipselect = dev->devfn;
 
-	ret = platform_device_add_data(pdev, &spi_pdata, sizeof(spi_pdata));
-	if (ret)
-		goto err_nomem;
-
-	pdev->dev.parent = &dev->dev;
-	pdev->dev.of_node = dev->dev.of_node;
 	ssp = &spi_info->ssp;
 	ssp->phys_base = pci_resource_start(dev, 0);
-	ssp->mmio_base = ioremap(phys_beg, phys_len);
+	ssp->mmio_base = pcim_iomap_table(dev)[0];
 	if (!ssp->mmio_base) {
-		dev_err(&pdev->dev, "failed to ioremap() registers\n");
-		ret = -EIO;
-		goto err_nomem;
+		dev_err(&dev->dev, "failed to ioremap() registers\n");
+		return -EIO;
 	}
 	ssp->irq = dev->irq;
-	ssp->port_id = pdev->id;
+	ssp->port_id = dev->devfn;
 	ssp->type = PXA25x_SSP;
 
 	mutex_lock(&ssp_lock);
 	list_add(&ssp->node, &ssp_list);
 	mutex_unlock(&ssp_lock);
 
-	pci_set_drvdata(dev, spi_info);
+	memset(&pi, 0, sizeof(pi));
+	pi.parent = &dev->dev;
+	pi.name = "pxa2xx-spi";
+	pi.id = ssp->port_id;
+	pi.data = &spi_pdata;
+	pi.size_data = sizeof(spi_pdata);
 
-	ret = platform_device_add(pdev);
-	if (ret)
-		goto err_dev_add;
+	pdev = platform_device_register_full(&pi);
+	if (!pdev) {
+		mutex_lock(&ssp_lock);
+		list_del(&ssp->node);
+		mutex_unlock(&ssp_lock);
 
-	return ret;
+		return -ENOMEM;
+	}
 
-err_dev_add:
-	pci_set_drvdata(dev, NULL);
-	mutex_lock(&ssp_lock);
-	list_del(&ssp->node);
-	mutex_unlock(&ssp_lock);
-	iounmap(ssp->mmio_base);
+	spi_info->spi_pdev = pdev;
+	pci_set_drvdata(dev, spi_info);
 
-err_nomem:
-	release_mem_region(phys_beg, phys_len);
-	platform_device_put(pdev);
-	kfree(spi_info);
-	return ret;
+	return 0;
 }
 
 static void ce4100_spi_remove(struct pci_dev *dev)
 {
-	struct ce4100_info *spi_info;
-	struct ssp_device *ssp;
+	struct ce4100_info *spi_info = pci_get_drvdata(dev);
+	struct ssp_device *ssp = &spi_info->ssp;
 
-	spi_info = pci_get_drvdata(dev);
-	ssp = &spi_info->ssp;
 	platform_device_unregister(spi_info->spi_pdev);
 
-	iounmap(ssp->mmio_base);
-	release_mem_region(pci_resource_start(dev, 0),
-			pci_resource_len(dev, 0));
-
 	mutex_lock(&ssp_lock);
 	list_del(&ssp->node);
 	mutex_unlock(&ssp_lock);
-
-	pci_set_drvdata(dev, NULL);
-	pci_disable_device(dev);
-	kfree(spi_info);
 }
 
 static DEFINE_PCI_DEVICE_TABLE(ce4100_spi_devices) = {
-- 
1.7.10.4

[PATCH 04/11] spi/pxa2xx: embed the ssp_device to platform data

[Mika Westerberg]

The spi-pxa2xx-pci glue driver had to implement pxa_ssp_request()/free() in
order to support the spi-pxa2xx platform driver. Since the ACPI enabled
platforms can use the same platform driver we would need to implement
pxa_ssp_request()/free() in some central place that can be shared by the
ACPI and PCI glue code.

Instead of doing that we can make pxa_ssp_request()/free() to be available
only when CONFIG_ARCH_PXA is set. On other arches these are being stubbed
out in preference to passing the ssp_device from the platform data
directly.

We also change the SPI bus number to be taken from ssp->port_id instead of
platform device id. This way the supporting code that passes the ssp can
decide the number (or it can set it to the same as pdev->id).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-pci.c   |   73 +++-------------------------------------
 drivers/spi/spi-pxa2xx.c       |   15 ++++++---
 include/linux/pxa2xx_ssp.h     |    9 +++++
 include/linux/spi/pxa2xx_spi.h |    3 ++
 4 files changed, 28 insertions(+), 72 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index ece979e..364964d 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -8,55 +8,11 @@
 #include <linux/module.h>
 #include <linux/spi/pxa2xx_spi.h>
 
-struct ce4100_info {
-	struct ssp_device ssp;
-	struct platform_device *spi_pdev;
-};
-
-static DEFINE_MUTEX(ssp_lock);
-static LIST_HEAD(ssp_list);
-
-struct ssp_device *pxa_ssp_request(int port, const char *label)
-{
-	struct ssp_device *ssp = NULL;
-
-	mutex_lock(&ssp_lock);
-
-	list_for_each_entry(ssp, &ssp_list, node) {
-		if (ssp->port_id == port && ssp->use_count == 0) {
-			ssp->use_count++;
-			ssp->label = label;
-			break;
-		}
-	}
-
-	mutex_unlock(&ssp_lock);
-
-	if (&ssp->node == &ssp_list)
-		return NULL;
-
-	return ssp;
-}
-EXPORT_SYMBOL_GPL(pxa_ssp_request);
-
-void pxa_ssp_free(struct ssp_device *ssp)
-{
-	mutex_lock(&ssp_lock);
-	if (ssp->use_count) {
-		ssp->use_count--;
-		ssp->label = NULL;
-	} else
-		dev_err(&ssp->pdev->dev, "device already free\n");
-	mutex_unlock(&ssp_lock);
-}
-EXPORT_SYMBOL_GPL(pxa_ssp_free);
-
 static int ce4100_spi_probe(struct pci_dev *dev,
 		const struct pci_device_id *ent)
 {
 	struct platform_device_info pi;
 	int ret;
-	struct ce4100_info *spi_info;
 	struct platform_device *pdev;
 	struct pxa2xx_spi_master spi_pdata;
 	struct ssp_device *ssp;
@@ -69,14 +25,10 @@ static int ce4100_spi_probe(struct pci_dev *dev,
 	if (!ret)
 		return ret;
 
-	spi_info = devm_kzalloc(&dev->dev, sizeof(*spi_info), GFP_KERNEL);
-	if (!spi_info)
-		return -ENOMEM;
-
 	memset(&spi_pdata, 0, sizeof(spi_pdata));
 	spi_pdata.num_chipselect = dev->devfn;
 
-	ssp = &spi_info->ssp;
+	ssp = &spi_pdata.ssp;
 	ssp->phys_base = pci_resource_start(dev, 0);
 	ssp->mmio_base = pcim_iomap_table(dev)[0];
 	if (!ssp->mmio_base) {
@@ -87,10 +39,6 @@ static int ce4100_spi_probe(struct pci_dev *dev,
 	ssp->port_id = dev->devfn;
 	ssp->type = PXA25x_SSP;
 
-	mutex_lock(&ssp_lock);
-	list_add(&ssp->node, &ssp_list);
-	mutex_unlock(&ssp_lock);
-
 	memset(&pi, 0, sizeof(pi));
 	pi.parent = &dev->dev;
 	pi.name = "pxa2xx-spi";
@@ -99,30 +47,19 @@ static int ce4100_spi_probe(struct pci_dev *dev,
 	pi.size_data = sizeof(spi_pdata);
 
 	pdev = platform_device_register_full(&pi);
-	if (!pdev) {
-		mutex_lock(&ssp_lock);
-		list_del(&ssp->node);
-		mutex_unlock(&ssp_lock);
-
+	if (!pdev)
 		return -ENOMEM;
-	}
 
-	spi_info->spi_pdev = pdev;
-	pci_set_drvdata(dev, spi_info);
+	pci_set_drvdata(dev, pdev);
 
 	return 0;
 }
 
 static void ce4100_spi_remove(struct pci_dev *dev)
 {
-	struct ce4100_info *spi_info = pci_get_drvdata(dev);
-	struct ssp_device *ssp = &spi_info->ssp;
-
-	platform_device_unregister(spi_info->spi_pdev);
+	struct platform_device *pdev = pci_get_drvdata(dev);
 
-	mutex_lock(&ssp_lock);
-	list_del(&ssp->node);
-	mutex_unlock(&ssp_lock);
+	platform_device_unregister(pdev);
 }
 
 static DEFINE_PCI_DEVICE_TABLE(ce4100_spi_devices) = {
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index ad842eb..17bd136 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1374,11 +1374,18 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	struct ssp_device *ssp;
 	int status;
 
-	platform_info = dev->platform_data;
+	platform_info = dev_get_platdata(dev);
+	if (!platform_info) {
+		dev_err(&pdev->dev, "missing platform data\n");
+		return -ENODEV;
+	}
 
 	ssp = pxa_ssp_request(pdev->id, pdev->name);
-	if (ssp == NULL) {
-		dev_err(&pdev->dev, "failed to request SSP%d\n", pdev->id);
+	if (!ssp)
+		ssp = &platform_info->ssp;
+
+	if (!ssp->mmio_base) {
+		dev_err(&pdev->dev, "failed to get ssp\n");
 		return -ENODEV;
 	}
 
@@ -1400,7 +1407,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 
-	master->bus_num = pdev->id;
+	master->bus_num = ssp->port_id;
 	master->num_chipselect = platform_info->num_chipselect;
 	master->dma_alignment = DMA_ALIGNMENT;
 	master->cleanup = cleanup;
diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index f366320..065e7f6 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -206,6 +206,15 @@ static inline u32 pxa_ssp_read_reg(struct ssp_device *dev, u32 reg)
 	return __raw_readl(dev->mmio_base + reg);
 }
 
+#ifdef CONFIG_ARCH_PXA
 struct ssp_device *pxa_ssp_request(int port, const char *label);
 void pxa_ssp_free(struct ssp_device *);
+#else
+static inline struct ssp_device *pxa_ssp_request(int port, const char *label)
+{
+	return NULL;
+}
+static inline void pxa_ssp_free(struct ssp_device *ssp) {}
+#endif
+
 #endif
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index c73d144..6b99f09 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -28,6 +28,9 @@ struct pxa2xx_spi_master {
 	u32 clock_enable;
 	u16 num_chipselect;
 	u8 enable_dma;
+
+	/* For non-PXA arches */
+	struct ssp_device ssp;
 };
 
 /* spi_board_info.controller_data for SPI slave devices,
-- 
1.7.10.4

[PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

If the architecture doesn't support clk framework (like x86) we need a way to
pass the SSP clock rate to the driver. This patch adds a field in the platform
data 'fixed_clk_rate' that allows passing the rate.

Also include clk.h to make sure that the clk_* functions are properly stubbed
out on architectures where clk framework is not available.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-pci.c   |    1 +
 drivers/spi/spi-pxa2xx.c       |   28 +++++++++++++++++++---------
 include/linux/spi/pxa2xx_spi.h |   19 +------------------
 3 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 364964d..4de98f4 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -27,6 +27,7 @@ static int ce4100_spi_probe(struct pci_dev *dev,
 
 	memset(&spi_pdata, 0, sizeof(spi_pdata));
 	spi_pdata.num_chipselect = dev->devfn;
+	spi_pdata.fixed_clk_rate = 3686400;
 
 	ssp = &spi_pdata.ssp;
 	ssp->phys_base = pci_resource_start(dev, 0);
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 17bd136..3dedebd 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -30,6 +30,7 @@
 #include <linux/delay.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -891,11 +892,22 @@ static int set_dma_burst_and_threshold(struct chip_data *chip,
 	return retval;
 }
 
-static unsigned int ssp_get_clk_div(struct ssp_device *ssp, int rate)
+static unsigned long pxa2xx_spi_clk_rate(struct driver_data *drv_data)
 {
-	unsigned long ssp_clk = clk_get_rate(ssp->clk);
+	const struct pxa2xx_spi_master *pdata = drv_data->master_info;
 
-	if (ssp->type == PXA25x_SSP || ssp->type == CE4100_SSP)
+	return pdata->fixed_clk_rate ? pdata->fixed_clk_rate
+				     : clk_get_rate(drv_data->ssp->clk);
+}
+
+static unsigned int ssp_get_clk_div(struct driver_data *drv_data, unsigned rate)
+{
+	unsigned long ssp_clk = pxa2xx_spi_clk_rate(drv_data);
+
+	rate = min_t(unsigned, ssp_clk, rate);
+
+	if (drv_data->ssp->type == PXA25x_SSP ||
+	    drv_data->ssp->type == CE4100_SSP)
 		return ((ssp_clk / (2 * rate) - 1) & 0xff) << 8;
 	else
 		return ((ssp_clk / rate - 1) & 0xfff) << 8;
@@ -908,7 +920,6 @@ static void pump_transfers(unsigned long data)
 	struct spi_transfer *transfer = NULL;
 	struct spi_transfer *previous = NULL;
 	struct chip_data *chip = NULL;
-	struct ssp_device *ssp = drv_data->ssp;
 	void __iomem *reg = drv_data->ioaddr;
 	u32 clk_div = 0;
 	u8 bits = 0;
@@ -1005,7 +1016,7 @@ static void pump_transfers(unsigned long data)
 		if (transfer->bits_per_word)
 			bits = transfer->bits_per_word;
 
-		clk_div = ssp_get_clk_div(ssp, speed);
+		clk_div = ssp_get_clk_div(drv_data, speed);
 
 		if (bits <= 8) {
 			drv_data->n_bytes = 1;
@@ -1214,7 +1225,6 @@ static int setup(struct spi_device *spi)
 	struct pxa2xx_spi_chip *chip_info = NULL;
 	struct chip_data *chip;
 	struct driver_data *drv_data = spi_master_get_devdata(spi->master);
-	struct ssp_device *ssp = drv_data->ssp;
 	unsigned int clk_div;
 	uint tx_thres = TX_THRESH_DFLT;
 	uint rx_thres = RX_THRESH_DFLT;
@@ -1296,7 +1306,7 @@ static int setup(struct spi_device *spi)
 		}
 	}
 
-	clk_div = ssp_get_clk_div(ssp, spi->max_speed_hz);
+	clk_div = ssp_get_clk_div(drv_data, spi->max_speed_hz);
 	chip->speed_hz = spi->max_speed_hz;
 
 	chip->cr0 = clk_div
@@ -1312,12 +1322,12 @@ static int setup(struct spi_device *spi)
 	/* NOTE:  PXA25x_SSP _could_ use external clocking ... */
 	if (!pxa25x_ssp_comp(drv_data))
 		dev_dbg(&spi->dev, "%ld Hz actual, %s\n",
-			clk_get_rate(ssp->clk)
+			pxa2xx_spi_clk_rate(drv_data)
 				/ (1 + ((chip->cr0 & SSCR0_SCR(0xfff)) >> 8)),
 			chip->enable_dma ? "DMA" : "PIO");
 	else
 		dev_dbg(&spi->dev, "%ld Hz actual, %s\n",
-			clk_get_rate(ssp->clk) / 2
+			pxa2xx_spi_clk_rate(drv_data) / 2
 				/ (1 + ((chip->cr0 & SSCR0_SCR(0x0ff)) >> 8)),
 			chip->enable_dma ? "DMA" : "PIO");
 
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index 6b99f09..83b73f5 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -31,6 +31,7 @@ struct pxa2xx_spi_master {
 
 	/* For non-PXA arches */
 	struct ssp_device ssp;
+	unsigned long fixed_clk_rate;
 };
 
 /* spi_board_info.controller_data for SPI slave devices,
@@ -133,23 +134,5 @@ static inline void pxa_free_dma(int dma_ch)
 {
 }
 
-/*
- * The CE4100 does not have the clk framework implemented and SPI clock can
- * not be switched on/off or the divider changed.
- */
-static inline void clk_disable(struct clk *clk)
-{
-}
-
-static inline int clk_enable(struct clk *clk)
-{
-	return 0;
-}
-
-static inline unsigned long clk_get_rate(struct clk *clk)
-{
-	return 3686400;
-}
-
 #endif
 #endif
-- 
1.7.10.4

[PATCH 06/11] spi/pxa2xx: use the private DMA API only when CONFIG_ARCH_PXA is set

[Mika Westerberg]

The PXA SPI driver uses PXA platform specific private DMA implementation
which does not work on non-PXA platforms. In order to use this driver on
other platforms we need to move the private DMA implementation into a
separate functions that get stubbed out when !CONFIG_ARCH_PXA.

While we are there we can kill the dummy DMA bits in pxa2xx_spi.h as they
are not needed anymore for CE4100.

Once this is done we can add the generic DMA engine support to the driver
that allows usage of any DMA controller that implements DMA engine API.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c       |  612 +++++++++++++++++++++++-----------------
 include/linux/spi/pxa2xx_spi.h |   80 ------
 2 files changed, 349 insertions(+), 343 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 3dedebd..2e17679 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -46,8 +46,7 @@ MODULE_ALIAS("platform:pxa2xx-spi");
 
 #define TIMOUT_DFLT		1000
 
-#define DMA_INT_MASK		(DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
-#define RESET_DMA_CHANNEL	(DCSR_NODESC | DMA_INT_MASK)
+/* For PXA private DMA */
 #define IS_DMA_ALIGNED(x)	IS_ALIGNED((unsigned long)x, DMA_ALIGNMENT)
 #define MAX_DMA_LEN		8191
 #define DMA_ALIGNMENT		8
@@ -100,7 +99,7 @@ struct driver_data {
 	/* PXA hookup */
 	struct pxa2xx_spi_master *master_info;
 
-	/* DMA setup stuff */
+	/* PXA private DMA setup stuff */
 	int rx_channel;
 	int tx_channel;
 	u32 *null_dma_buf;
@@ -133,7 +132,6 @@ struct driver_data {
 	size_t rx_map_len;
 	size_t tx_map_len;
 	u8 n_bytes;
-	u32 dma_width;
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
 	irqreturn_t (*transfer_handler)(struct driver_data *drv_data);
@@ -146,7 +144,6 @@ struct chip_data {
 	u32 psp;
 	u32 timeout;
 	u8 n_bytes;
-	u32 dma_width;
 	u32 dma_burst_size;
 	u32 threshold;
 	u32 dma_threshold;
@@ -358,6 +355,80 @@ static void *next_transfer(struct driver_data *drv_data)
 		return DONE_STATE;
 }
 
+/* caller already set message->status; dma and pio irqs are blocked */
+static void giveback(struct driver_data *drv_data)
+{
+	struct spi_transfer* last_transfer;
+	struct spi_message *msg;
+
+	msg = drv_data->cur_msg;
+	drv_data->cur_msg = NULL;
+	drv_data->cur_transfer = NULL;
+
+	last_transfer = list_entry(msg->transfers.prev,
+					struct spi_transfer,
+					transfer_list);
+
+	/* Delay if requested before any change in chip select */
+	if (last_transfer->delay_usecs)
+		udelay(last_transfer->delay_usecs);
+
+	/* Drop chip select UNLESS cs_change is true or we are returning
+	 * a message with an error, or next message is for another chip
+	 */
+	if (!last_transfer->cs_change)
+		cs_deassert(drv_data);
+	else {
+		struct spi_message *next_msg;
+
+		/* Holding of cs was hinted, but we need to make sure
+		 * the next message is for the same chip.  Don't waste
+		 * time with the following tests unless this was hinted.
+		 *
+		 * We cannot postpone this until pump_messages, because
+		 * after calling msg->complete (below) the driver that
+		 * sent the current message could be unloaded, which
+		 * could invalidate the cs_control() callback...
+		 */
+
+		/* get a pointer to the next message, if any */
+		next_msg = spi_get_next_queued_message(drv_data->master);
+
+		/* see if the next and current messages point
+		 * to the same chip
+		 */
+		if (next_msg && next_msg->spi != msg->spi)
+			next_msg = NULL;
+		if (!next_msg || msg->state == ERROR_STATE)
+			cs_deassert(drv_data);
+	}
+
+	spi_finalize_current_message(drv_data->master);
+	drv_data->cur_chip = NULL;
+}
+
+#ifdef CONFIG_ARCH_PXA
+#define DMA_INT_MASK		(DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
+#define RESET_DMA_CHANNEL	(DCSR_NODESC | DMA_INT_MASK)
+
+static bool dma_is_possible(size_t len)
+{
+	/* Try to map dma buffer and do a dma transfer if successful, but
+	 * only if the length is non-zero and less than MAX_DMA_LEN.
+	 *
+	 * Zero-length non-descriptor DMA is illegal on PXA2xx; force use
+	 * of PIO instead.  Care is needed above because the transfer may
+	 * have have been passed with buffers that are already dma mapped.
+	 * A zero-length transfer in PIO mode will not try to write/read
+	 * to/from the buffers
+	 *
+	 * REVISIT large transfers are exactly where we most want to be
+	 * using DMA.  If this happens much, split those transfers into
+	 * multiple DMA segments rather than forcing PIO.
+	 */
+	return len > 0 && len <= MAX_DMA_LEN;
+}
+
 static int map_dma_buffers(struct driver_data *drv_data)
 {
 	struct spi_message *msg = drv_data->cur_msg;
@@ -428,58 +499,6 @@ static void unmap_dma_buffers(struct driver_data *drv_data)
 	drv_data->dma_mapped = 0;
 }
 
-/* caller already set message->status; dma and pio irqs are blocked */
-static void giveback(struct driver_data *drv_data)
-{
-	struct spi_transfer* last_transfer;
-	struct spi_message *msg;
-
-	msg = drv_data->cur_msg;
-	drv_data->cur_msg = NULL;
-	drv_data->cur_transfer = NULL;
-
-	last_transfer = list_entry(msg->transfers.prev,
-					struct spi_transfer,
-					transfer_list);
-
-	/* Delay if requested before any change in chip select */
-	if (last_transfer->delay_usecs)
-		udelay(last_transfer->delay_usecs);
-
-	/* Drop chip select UNLESS cs_change is true or we are returning
-	 * a message with an error, or next message is for another chip
-	 */
-	if (!last_transfer->cs_change)
-		cs_deassert(drv_data);
-	else {
-		struct spi_message *next_msg;
-
-		/* Holding of cs was hinted, but we need to make sure
-		 * the next message is for the same chip.  Don't waste
-		 * time with the following tests unless this was hinted.
-		 *
-		 * We cannot postpone this until pump_messages, because
-		 * after calling msg->complete (below) the driver that
-		 * sent the current message could be unloaded, which
-		 * could invalidate the cs_control() callback...
-		 */
-
-		/* get a pointer to the next message, if any */
-		next_msg = spi_get_next_queued_message(drv_data->master);
-
-		/* see if the next and current messages point
-		 * to the same chip
-		 */
-		if (next_msg && next_msg->spi != msg->spi)
-			next_msg = NULL;
-		if (!next_msg || msg->state == ERROR_STATE)
-			cs_deassert(drv_data);
-	}
-
-	spi_finalize_current_message(drv_data->master);
-	drv_data->cur_chip = NULL;
-}
-
 static int wait_ssp_rx_stall(void const __iomem *ioaddr)
 {
 	unsigned long limit = loops_per_jiffy << 1;
@@ -635,6 +654,264 @@ static irqreturn_t dma_transfer(struct driver_data *drv_data)
 	return IRQ_NONE;
 }
 
+static int dma_prepare(struct driver_data *drv_data, u32 dma_burst)
+{
+	u32 dma_width;
+
+	switch (drv_data->n_bytes) {
+	case 1:
+		dma_width = DCMD_WIDTH1;
+		break;
+	case 2:
+		dma_width = DCMD_WIDTH2;
+		break;
+	default:
+		dma_width = DCMD_WIDTH4;
+		break;
+	}
+
+	/* Setup rx DMA Channel */
+	DCSR(drv_data->rx_channel) = RESET_DMA_CHANNEL;
+	DSADR(drv_data->rx_channel) = drv_data->ssdr_physical;
+	DTADR(drv_data->rx_channel) = drv_data->rx_dma;
+	if (drv_data->rx == drv_data->null_dma_buf)
+		/* No target address increment */
+		DCMD(drv_data->rx_channel) = DCMD_FLOWSRC
+						| dma_width
+						| dma_burst
+						| drv_data->len;
+	else
+		DCMD(drv_data->rx_channel) = DCMD_INCTRGADDR
+						| DCMD_FLOWSRC
+						| dma_width
+						| dma_burst
+						| drv_data->len;
+
+	/* Setup tx DMA Channel */
+	DCSR(drv_data->tx_channel) = RESET_DMA_CHANNEL;
+	DSADR(drv_data->tx_channel) = drv_data->tx_dma;
+	DTADR(drv_data->tx_channel) = drv_data->ssdr_physical;
+	if (drv_data->tx == drv_data->null_dma_buf)
+		/* No source address increment */
+		DCMD(drv_data->tx_channel) = DCMD_FLOWTRG
+						| dma_width
+						| dma_burst
+						| drv_data->len;
+	else
+		DCMD(drv_data->tx_channel) = DCMD_INCSRCADDR
+						| DCMD_FLOWTRG
+						| dma_width
+						| dma_burst
+						| drv_data->len;
+
+	/* Enable dma end irqs on SSP to detect end of transfer */
+	if (drv_data->ssp_type == PXA25x_SSP)
+		DCMD(drv_data->tx_channel) |= DCMD_ENDIRQEN;
+
+	return 0;
+}
+
+static void dma_start(struct driver_data *drv_data)
+{
+	DCSR(drv_data->rx_channel) |= DCSR_RUN;
+	DCSR(drv_data->tx_channel) |= DCSR_RUN;
+}
+
+static int dma_setup(struct driver_data *drv_data)
+{
+	struct device *dev = &drv_data->pdev->dev;
+	struct ssp_device *ssp = drv_data->ssp;
+
+	/* Get two DMA channels	(rx and tx) */
+	drv_data->rx_channel = pxa_request_dma("pxa2xx_spi_ssp_rx",
+						DMA_PRIO_HIGH,
+						dma_handler,
+						drv_data);
+	if (drv_data->rx_channel < 0) {
+		dev_err(dev, "problem (%d) requesting rx channel\n",
+			drv_data->rx_channel);
+		return -ENODEV;
+	}
+	drv_data->tx_channel = pxa_request_dma("pxa2xx_spi_ssp_tx",
+						DMA_PRIO_MEDIUM,
+						dma_handler,
+						drv_data);
+	if (drv_data->tx_channel < 0) {
+		dev_err(dev, "problem (%d) requesting tx channel\n",
+			drv_data->tx_channel);
+		pxa_free_dma(drv_data->rx_channel);
+		return -ENODEV;
+	}
+
+	DRCMR(ssp->drcmr_rx) = DRCMR_MAPVLD | drv_data->rx_channel;
+	DRCMR(ssp->drcmr_tx) = DRCMR_MAPVLD | drv_data->tx_channel;
+
+	return 0;
+}
+
+static void dma_release(struct driver_data *drv_data)
+{
+	struct ssp_device *ssp = drv_data->ssp;
+
+	DRCMR(ssp->drcmr_rx) = 0;
+	DRCMR(ssp->drcmr_tx) = 0;
+
+	if (drv_data->tx_channel != 0)
+		pxa_free_dma(drv_data->tx_channel);
+	if (drv_data->rx_channel != 0)
+		pxa_free_dma(drv_data->rx_channel);
+}
+
+static void dma_resume(struct driver_data *drv_data)
+{
+	if (drv_data->rx_channel != -1)
+		DRCMR(drv_data->ssp->drcmr_rx) =
+			DRCMR_MAPVLD | drv_data->rx_channel;
+	if (drv_data->tx_channel != -1)
+		DRCMR(drv_data->ssp->drcmr_tx) =
+			DRCMR_MAPVLD | drv_data->tx_channel;
+}
+
+static int set_dma_burst_and_threshold(struct chip_data *chip,
+				struct spi_device *spi,
+				u8 bits_per_word, u32 *burst_code,
+				u32 *threshold)
+{
+	struct pxa2xx_spi_chip *chip_info =
+			(struct pxa2xx_spi_chip *)spi->controller_data;
+	int bytes_per_word;
+	int burst_bytes;
+	int thresh_words;
+	int req_burst_size;
+	int retval = 0;
+
+	/* Set the threshold (in registers) to equal the same amount of data
+	 * as represented by burst size (in bytes).  The computation below
+	 * is (burst_size rounded up to nearest 8 byte, word or long word)
+	 * divided by (bytes/register); the tx threshold is the inverse of
+	 * the rx, so that there will always be enough data in the rx fifo
+	 * to satisfy a burst, and there will always be enough space in the
+	 * tx fifo to accept a burst (a tx burst will overwrite the fifo if
+	 * there is not enough space), there must always remain enough empty
+	 * space in the rx fifo for any data loaded to the tx fifo.
+	 * Whenever burst_size (in bytes) equals bits/word, the fifo threshold
+	 * will be 8, or half the fifo;
+	 * The threshold can only be set to 2, 4 or 8, but not 16, because
+	 * to burst 16 to the tx fifo, the fifo would have to be empty;
+	 * however, the minimum fifo trigger level is 1, and the tx will
+	 * request service when the fifo is at this level, with only 15 spaces.
+	 */
+
+	/* find bytes/word */
+	if (bits_per_word <= 8)
+		bytes_per_word = 1;
+	else if (bits_per_word <= 16)
+		bytes_per_word = 2;
+	else
+		bytes_per_word = 4;
+
+	/* use struct pxa2xx_spi_chip->dma_burst_size if available */
+	if (chip_info)
+		req_burst_size = chip_info->dma_burst_size;
+	else {
+		switch (chip->dma_burst_size) {
+		default:
+			/* if the default burst size is not set,
+			 * do it now */
+			chip->dma_burst_size = DCMD_BURST8;
+		case DCMD_BURST8:
+			req_burst_size = 8;
+			break;
+		case DCMD_BURST16:
+			req_burst_size = 16;
+			break;
+		case DCMD_BURST32:
+			req_burst_size = 32;
+			break;
+		}
+	}
+	if (req_burst_size <= 8) {
+		*burst_code = DCMD_BURST8;
+		burst_bytes = 8;
+	} else if (req_burst_size <= 16) {
+		if (bytes_per_word == 1) {
+			/* don't burst more than 1/2 the fifo */
+			*burst_code = DCMD_BURST8;
+			burst_bytes = 8;
+			retval = 1;
+		} else {
+			*burst_code = DCMD_BURST16;
+			burst_bytes = 16;
+		}
+	} else {
+		if (bytes_per_word == 1) {
+			/* don't burst more than 1/2 the fifo */
+			*burst_code = DCMD_BURST8;
+			burst_bytes = 8;
+			retval = 1;
+		} else if (bytes_per_word == 2) {
+			/* don't burst more than 1/2 the fifo */
+			*burst_code = DCMD_BURST16;
+			burst_bytes = 16;
+			retval = 1;
+		} else {
+			*burst_code = DCMD_BURST32;
+			burst_bytes = 32;
+		}
+	}
+
+	thresh_words = burst_bytes / bytes_per_word;
+
+	/* thresh_words will be between 2 and 8 */
+	*threshold = (SSCR1_RxTresh(thresh_words) & SSCR1_RFT)
+			| (SSCR1_TxTresh(16-thresh_words) & SSCR1_TFT);
+
+	return retval;
+}
+#else
+static bool dma_is_possible(size_t len)
+{
+	return false;
+}
+
+static int map_dma_buffers(struct driver_data *drv_data)
+{
+	return 0;
+}
+
+static irqreturn_t dma_transfer(struct driver_data *drv_data)
+{
+	return IRQ_NONE;
+}
+
+static void dma_prepare(struct driver_data *drv_data, u32 dma_burst)
+{
+}
+
+static void dma_start(struct driver_data *drv_data)
+{
+}
+
+static int dma_setup(struct driver_data *drv_data)
+{
+	return -ENODEV;
+}
+
+static void dma_release(struct driver_data *drv_data)
+{
+}
+
+static inline void dma_resume(struct driver_data *drv_data) {}
+
+static int set_dma_burst_and_threshold(struct chip_data *chip,
+				struct spi_device *spi,
+				u8 bits_per_word, u32 *burst_code,
+				u32 *threshold)
+{
+	return -ENODEV;
+}
+#endif
+
 static void reset_sccr1(struct driver_data *drv_data)
 {
 	void __iomem *reg = drv_data->ioaddr;
@@ -795,103 +1072,6 @@ static irqreturn_t ssp_int(int irq, void *dev_id)
 	return drv_data->transfer_handler(drv_data);
 }
 
-static int set_dma_burst_and_threshold(struct chip_data *chip,
-				struct spi_device *spi,
-				u8 bits_per_word, u32 *burst_code,
-				u32 *threshold)
-{
-	struct pxa2xx_spi_chip *chip_info =
-			(struct pxa2xx_spi_chip *)spi->controller_data;
-	int bytes_per_word;
-	int burst_bytes;
-	int thresh_words;
-	int req_burst_size;
-	int retval = 0;
-
-	/* Set the threshold (in registers) to equal the same amount of data
-	 * as represented by burst size (in bytes).  The computation below
-	 * is (burst_size rounded up to nearest 8 byte, word or long word)
-	 * divided by (bytes/register); the tx threshold is the inverse of
-	 * the rx, so that there will always be enough data in the rx fifo
-	 * to satisfy a burst, and there will always be enough space in the
-	 * tx fifo to accept a burst (a tx burst will overwrite the fifo if
-	 * there is not enough space), there must always remain enough empty
-	 * space in the rx fifo for any data loaded to the tx fifo.
-	 * Whenever burst_size (in bytes) equals bits/word, the fifo threshold
-	 * will be 8, or half the fifo;
-	 * The threshold can only be set to 2, 4 or 8, but not 16, because
-	 * to burst 16 to the tx fifo, the fifo would have to be empty;
-	 * however, the minimum fifo trigger level is 1, and the tx will
-	 * request service when the fifo is at this level, with only 15 spaces.
-	 */
-
-	/* find bytes/word */
-	if (bits_per_word <= 8)
-		bytes_per_word = 1;
-	else if (bits_per_word <= 16)
-		bytes_per_word = 2;
-	else
-		bytes_per_word = 4;
-
-	/* use struct pxa2xx_spi_chip->dma_burst_size if available */
-	if (chip_info)
-		req_burst_size = chip_info->dma_burst_size;
-	else {
-		switch (chip->dma_burst_size) {
-		default:
-			/* if the default burst size is not set,
-			 * do it now */
-			chip->dma_burst_size = DCMD_BURST8;
-		case DCMD_BURST8:
-			req_burst_size = 8;
-			break;
-		case DCMD_BURST16:
-			req_burst_size = 16;
-			break;
-		case DCMD_BURST32:
-			req_burst_size = 32;
-			break;
-		}
-	}
-	if (req_burst_size <= 8) {
-		*burst_code = DCMD_BURST8;
-		burst_bytes = 8;
-	} else if (req_burst_size <= 16) {
-		if (bytes_per_word == 1) {
-			/* don't burst more than 1/2 the fifo */
-			*burst_code = DCMD_BURST8;
-			burst_bytes = 8;
-			retval = 1;
-		} else {
-			*burst_code = DCMD_BURST16;
-			burst_bytes = 16;
-		}
-	} else {
-		if (bytes_per_word == 1) {
-			/* don't burst more than 1/2 the fifo */
-			*burst_code = DCMD_BURST8;
-			burst_bytes = 8;
-			retval = 1;
-		} else if (bytes_per_word == 2) {
-			/* don't burst more than 1/2 the fifo */
-			*burst_code = DCMD_BURST16;
-			burst_bytes = 16;
-			retval = 1;
-		} else {
-			*burst_code = DCMD_BURST32;
-			burst_bytes = 32;
-		}
-	}
-
-	thresh_words = burst_bytes / bytes_per_word;
-
-	/* thresh_words will be between 2 and 8 */
-	*threshold = (SSCR1_RxTresh(thresh_words) & SSCR1_RFT)
-			| (SSCR1_TxTresh(16-thresh_words) & SSCR1_TFT);
-
-	return retval;
-}
-
 static unsigned long pxa2xx_spi_clk_rate(struct driver_data *drv_data)
 {
 	const struct pxa2xx_spi_master *pdata = drv_data->master_info;
@@ -961,8 +1141,8 @@ static void pump_transfers(unsigned long data)
 			cs_deassert(drv_data);
 	}
 
-	/* Check for transfers that need multiple DMA segments */
-	if (transfer->len > MAX_DMA_LEN && chip->enable_dma) {
+	/* Check if we can DMA this transfer */
+	if (!dma_is_possible(transfer->len) && chip->enable_dma) {
 
 		/* reject already-mapped transfers; PIO won't always work */
 		if (message->is_dma_mapped
@@ -992,14 +1172,13 @@ static void pump_transfers(unsigned long data)
 		return;
 	}
 	drv_data->n_bytes = chip->n_bytes;
-	drv_data->dma_width = chip->dma_width;
 	drv_data->tx = (void *)transfer->tx_buf;
 	drv_data->tx_end = drv_data->tx + transfer->len;
 	drv_data->rx = transfer->rx_buf;
 	drv_data->rx_end = drv_data->rx + transfer->len;
 	drv_data->rx_dma = transfer->rx_dma;
 	drv_data->tx_dma = transfer->tx_dma;
-	drv_data->len = transfer->len & DCMD_LENGTH;
+	drv_data->len = transfer->len;
 	drv_data->write = drv_data->tx ? chip->write : null_writer;
 	drv_data->read = drv_data->rx ? chip->read : null_reader;
 
@@ -1020,21 +1199,18 @@ static void pump_transfers(unsigned long data)
 
 		if (bits <= 8) {
 			drv_data->n_bytes = 1;
-			drv_data->dma_width = DCMD_WIDTH1;
 			drv_data->read = drv_data->read != null_reader ?
 						u8_reader : null_reader;
 			drv_data->write = drv_data->write != null_writer ?
 						u8_writer : null_writer;
 		} else if (bits <= 16) {
 			drv_data->n_bytes = 2;
-			drv_data->dma_width = DCMD_WIDTH2;
 			drv_data->read = drv_data->read != null_reader ?
 						u16_reader : null_reader;
 			drv_data->write = drv_data->write != null_writer ?
 						u16_writer : null_writer;
 		} else if (bits <= 32) {
 			drv_data->n_bytes = 4;
-			drv_data->dma_width = DCMD_WIDTH4;
 			drv_data->read = drv_data->read != null_reader ?
 						u32_reader : null_reader;
 			drv_data->write = drv_data->write != null_writer ?
@@ -1062,70 +1238,21 @@ static void pump_transfers(unsigned long data)
 
 	message->state = RUNNING_STATE;
 
-	/* Try to map dma buffer and do a dma transfer if successful, but
-	 * only if the length is non-zero and less than MAX_DMA_LEN.
-	 *
-	 * Zero-length non-descriptor DMA is illegal on PXA2xx; force use
-	 * of PIO instead.  Care is needed above because the transfer may
-	 * have have been passed with buffers that are already dma mapped.
-	 * A zero-length transfer in PIO mode will not try to write/read
-	 * to/from the buffers
-	 *
-	 * REVISIT large transfers are exactly where we most want to be
-	 * using DMA.  If this happens much, split those transfers into
-	 * multiple DMA segments rather than forcing PIO.
-	 */
 	drv_data->dma_mapped = 0;
-	if (drv_data->len > 0 && drv_data->len <= MAX_DMA_LEN)
+	if (dma_is_possible(drv_data->len))
 		drv_data->dma_mapped = map_dma_buffers(drv_data);
 	if (drv_data->dma_mapped) {
 
 		/* Ensure we have the correct interrupt handler */
 		drv_data->transfer_handler = dma_transfer;
 
-		/* Setup rx DMA Channel */
-		DCSR(drv_data->rx_channel) = RESET_DMA_CHANNEL;
-		DSADR(drv_data->rx_channel) = drv_data->ssdr_physical;
-		DTADR(drv_data->rx_channel) = drv_data->rx_dma;
-		if (drv_data->rx == drv_data->null_dma_buf)
-			/* No target address increment */
-			DCMD(drv_data->rx_channel) = DCMD_FLOWSRC
-							| drv_data->dma_width
-							| dma_burst
-							| drv_data->len;
-		else
-			DCMD(drv_data->rx_channel) = DCMD_INCTRGADDR
-							| DCMD_FLOWSRC
-							| drv_data->dma_width
-							| dma_burst
-							| drv_data->len;
-
-		/* Setup tx DMA Channel */
-		DCSR(drv_data->tx_channel) = RESET_DMA_CHANNEL;
-		DSADR(drv_data->tx_channel) = drv_data->tx_dma;
-		DTADR(drv_data->tx_channel) = drv_data->ssdr_physical;
-		if (drv_data->tx == drv_data->null_dma_buf)
-			/* No source address increment */
-			DCMD(drv_data->tx_channel) = DCMD_FLOWTRG
-							| drv_data->dma_width
-							| dma_burst
-							| drv_data->len;
-		else
-			DCMD(drv_data->tx_channel) = DCMD_INCSRCADDR
-							| DCMD_FLOWTRG
-							| drv_data->dma_width
-							| dma_burst
-							| drv_data->len;
-
-		/* Enable dma end irqs on SSP to detect end of transfer */
-		if (drv_data->ssp_type == PXA25x_SSP)
-			DCMD(drv_data->tx_channel) |= DCMD_ENDIRQEN;
+		dma_prepare(drv_data, dma_burst);
 
 		/* Clear status and start DMA engine */
 		cr1 = chip->cr1 | dma_thresh | drv_data->dma_cr1;
 		write_SSSR(drv_data->clear_sr, reg);
-		DCSR(drv_data->rx_channel) |= DCSR_RUN;
-		DCSR(drv_data->tx_channel) |= DCSR_RUN;
+
+		dma_start(drv_data);
 	} else {
 		/* Ensure we have the correct interrupt handler	*/
 		drv_data->transfer_handler = interrupt_transfer;
@@ -1267,8 +1394,6 @@ static int setup(struct spi_device *spi)
 			chip->gpio_cs = -1;
 		chip->enable_dma = 0;
 		chip->timeout = TIMOUT_DFLT;
-		chip->dma_burst_size = drv_data->master_info->enable_dma ?
-					DCMD_BURST8 : 0;
 	}
 
 	/* protocol drivers may change the chip settings, so...
@@ -1333,18 +1458,15 @@ static int setup(struct spi_device *spi)
 
 	if (spi->bits_per_word <= 8) {
 		chip->n_bytes = 1;
-		chip->dma_width = DCMD_WIDTH1;
 		chip->read = u8_reader;
 		chip->write = u8_writer;
 	} else if (spi->bits_per_word <= 16) {
 		chip->n_bytes = 2;
-		chip->dma_width = DCMD_WIDTH2;
 		chip->read = u16_reader;
 		chip->write = u16_writer;
 	} else if (spi->bits_per_word <= 32) {
 		chip->cr0 |= SSCR0_EDSS;
 		chip->n_bytes = 4;
-		chip->dma_width = DCMD_WIDTH4;
 		chip->read = u32_reader;
 		chip->write = u32_writer;
 	} else {
@@ -1452,31 +1574,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	drv_data->tx_channel = -1;
 	drv_data->rx_channel = -1;
 	if (platform_info->enable_dma) {
-
-		/* Get two DMA channels	(rx and tx) */
-		drv_data->rx_channel = pxa_request_dma("pxa2xx_spi_ssp_rx",
-							DMA_PRIO_HIGH,
-							dma_handler,
-							drv_data);
-		if (drv_data->rx_channel < 0) {
-			dev_err(dev, "problem (%d) requesting rx channel\n",
-				drv_data->rx_channel);
-			status = -ENODEV;
-			goto out_error_irq_alloc;
-		}
-		drv_data->tx_channel = pxa_request_dma("pxa2xx_spi_ssp_tx",
-							DMA_PRIO_MEDIUM,
-							dma_handler,
-							drv_data);
-		if (drv_data->tx_channel < 0) {
-			dev_err(dev, "problem (%d) requesting tx channel\n",
-				drv_data->tx_channel);
-			status = -ENODEV;
-			goto out_error_dma_alloc;
+		status = dma_setup(drv_data);
+		if (status) {
+			dev_warn(dev, "failed to setup DMA, using PIO\n");
+			platform_info->enable_dma = false;
 		}
-
-		DRCMR(ssp->drcmr_rx) = DRCMR_MAPVLD | drv_data->rx_channel;
-		DRCMR(ssp->drcmr_tx) = DRCMR_MAPVLD | drv_data->tx_channel;
 	}
 
 	/* Enable SOC clock */
@@ -1510,14 +1612,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 
 out_error_clock_enabled:
 	clk_disable(ssp->clk);
-
-out_error_dma_alloc:
-	if (drv_data->tx_channel != -1)
-		pxa_free_dma(drv_data->tx_channel);
-	if (drv_data->rx_channel != -1)
-		pxa_free_dma(drv_data->rx_channel);
-
-out_error_irq_alloc:
+	dma_release(drv_data);
 	free_irq(ssp->irq, drv_data);
 
 out_error_master_alloc:
@@ -1540,12 +1635,8 @@ static int pxa2xx_spi_remove(struct platform_device *pdev)
 	clk_disable(ssp->clk);
 
 	/* Release DMA */
-	if (drv_data->master_info->enable_dma) {
-		DRCMR(ssp->drcmr_rx) = 0;
-		DRCMR(ssp->drcmr_tx) = 0;
-		pxa_free_dma(drv_data->tx_channel);
-		pxa_free_dma(drv_data->rx_channel);
-	}
+	if (drv_data->master_info->enable_dma)
+		dma_release(drv_data);
 
 	/* Release IRQ */
 	free_irq(ssp->irq, drv_data);
@@ -1592,12 +1683,7 @@ static int pxa2xx_spi_resume(struct device *dev)
 	struct ssp_device *ssp = drv_data->ssp;
 	int status = 0;
 
-	if (drv_data->rx_channel != -1)
-		DRCMR(drv_data->ssp->drcmr_rx) =
-			DRCMR_MAPVLD | drv_data->rx_channel;
-	if (drv_data->tx_channel != -1)
-		DRCMR(drv_data->ssp->drcmr_tx) =
-			DRCMR_MAPVLD | drv_data->tx_channel;
+	dma_resume(drv_data);
 
 	/* Enable the SSP clock */
 	clk_enable(ssp->clk);
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index 83b73f5..9f8cd03 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -54,85 +54,5 @@ struct pxa2xx_spi_chip {
 
 extern void pxa2xx_set_spi_info(unsigned id, struct pxa2xx_spi_master *info);
 
-#else
-/*
- * This is the implemtation for CE4100 on x86. ARM defines them in mach/ or
- * plat/ include path.
- * The CE4100 does not provide DMA support. This bits are here to let the driver
- * compile and will never be used. Maybe we get DMA support at a later point in
- * time.
- */
-
-#define DCSR(n)         (n)
-#define DSADR(n)        (n)
-#define DTADR(n)        (n)
-#define DCMD(n)         (n)
-#define DRCMR(n)        (n)
-
-#define DCSR_RUN	(1 << 31)	/* Run Bit */
-#define DCSR_NODESC	(1 << 30)	/* No-Descriptor Fetch */
-#define DCSR_STOPIRQEN	(1 << 29)	/* Stop Interrupt Enable */
-#define DCSR_REQPEND	(1 << 8)	/* Request Pending (read-only) */
-#define DCSR_STOPSTATE	(1 << 3)	/* Stop State (read-only) */
-#define DCSR_ENDINTR	(1 << 2)	/* End Interrupt */
-#define DCSR_STARTINTR	(1 << 1)	/* Start Interrupt */
-#define DCSR_BUSERR	(1 << 0)	/* Bus Error Interrupt */
-
-#define DCSR_EORIRQEN	(1 << 28)	/* End of Receive Interrupt Enable */
-#define DCSR_EORJMPEN	(1 << 27)	/* Jump to next descriptor on EOR */
-#define DCSR_EORSTOPEN	(1 << 26)	/* STOP on an EOR */
-#define DCSR_SETCMPST	(1 << 25)	/* Set Descriptor Compare Status */
-#define DCSR_CLRCMPST	(1 << 24)	/* Clear Descriptor Compare Status */
-#define DCSR_CMPST	(1 << 10)	/* The Descriptor Compare Status */
-#define DCSR_EORINTR	(1 << 9)	/* The end of Receive */
-
-#define DRCMR_MAPVLD	(1 << 7)	/* Map Valid */
-#define DRCMR_CHLNUM	0x1f		/* mask for Channel Number */
-
-#define DDADR_DESCADDR	0xfffffff0	/* Address of next descriptor */
-#define DDADR_STOP	(1 << 0)	/* Stop */
-
-#define DCMD_INCSRCADDR	(1 << 31)	/* Source Address Increment Setting. */
-#define DCMD_INCTRGADDR	(1 << 30)	/* Target Address Increment Setting. */
-#define DCMD_FLOWSRC	(1 << 29)	/* Flow Control by the source. */
-#define DCMD_FLOWTRG	(1 << 28)	/* Flow Control by the target. */
-#define DCMD_STARTIRQEN	(1 << 22)	/* Start Interrupt Enable */
-#define DCMD_ENDIRQEN	(1 << 21)	/* End Interrupt Enable */
-#define DCMD_ENDIAN	(1 << 18)	/* Device Endian-ness. */
-#define DCMD_BURST8	(1 << 16)	/* 8 byte burst */
-#define DCMD_BURST16	(2 << 16)	/* 16 byte burst */
-#define DCMD_BURST32	(3 << 16)	/* 32 byte burst */
-#define DCMD_WIDTH1	(1 << 14)	/* 1 byte width */
-#define DCMD_WIDTH2	(2 << 14)	/* 2 byte width (HalfWord) */
-#define DCMD_WIDTH4	(3 << 14)	/* 4 byte width (Word) */
-#define DCMD_LENGTH	0x01fff		/* length mask (max = 8K - 1) */
-
-/*
- * Descriptor structure for PXA's DMA engine
- * Note: this structure must always be aligned to a 16-byte boundary.
- */
-
-typedef enum {
-	DMA_PRIO_HIGH = 0,
-	DMA_PRIO_MEDIUM = 1,
-	DMA_PRIO_LOW = 2
-} pxa_dma_prio;
-
-/*
- * DMA registration
- */
-
-static inline int pxa_request_dma(char *name,
-		pxa_dma_prio prio,
-		void (*irq_handler)(int, void *),
-		void *data)
-{
-	return -ENODEV;
-}
-
-static inline void pxa_free_dma(int dma_ch)
-{
-}
-
 #endif
 #endif
-- 
1.7.10.4

[PATCH 07/11] spi/pxa2xx: add support for DMA engine

[Mika Westerberg]

In order to use DMA with this driver on non-PXA platforms we implement support
for the generic DMA engine API. This allows to use different DMA engines with
little or no modification to the driver.

Request lines and channel numbers can be passed to the driver from the
platform specific data.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c       |  351 +++++++++++++++++++++++++++++++++++++++-
 include/linux/spi/pxa2xx_spi.h |    6 +
 2 files changed, 350 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 2e17679..9b438ad 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <linux/atomic.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/device.h>
@@ -25,6 +26,9 @@
 #include <linux/platform_device.h>
 #include <linux/spi/pxa2xx_spi.h>
 #include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/scatterlist.h>
+#include <linux/sizes.h>
 #include <linux/spi/spi.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>
@@ -48,7 +52,6 @@ MODULE_ALIAS("platform:pxa2xx-spi");
 
 /* For PXA private DMA */
 #define IS_DMA_ALIGNED(x)	IS_ALIGNED((unsigned long)x, DMA_ALIGNMENT)
-#define MAX_DMA_LEN		8191
 #define DMA_ALIGNMENT		8
 
 /*
@@ -117,6 +120,16 @@ struct driver_data {
 	/* Message Transfer pump */
 	struct tasklet_struct pump_transfers;
 
+	/* DMA engine support */
+	struct dma_chan *rx_chan;
+	struct dma_chan *tx_chan;
+	struct sg_table rx_sgt;
+	struct sg_table tx_sgt;
+	int rx_nents;
+	int tx_nents;
+	void *dummy;
+	atomic_t dma_running;
+
 	/* Current message transfer state info */
 	struct spi_message* cur_msg;
 	struct spi_transfer* cur_transfer;
@@ -410,6 +423,8 @@ static void giveback(struct driver_data *drv_data)
 #ifdef CONFIG_ARCH_PXA
 #define DMA_INT_MASK		(DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
 #define RESET_DMA_CHANNEL	(DCSR_NODESC | DMA_INT_MASK)
+#define MAX_DMA_LEN		8191
+#define DEFAULT_DMA_CR1		(SSCR1_TSRE | SSCR1_RSRE | SSCR1_TINTE)
 
 static bool dma_is_possible(size_t len)
 {
@@ -869,36 +884,347 @@ static int set_dma_burst_and_threshold(struct chip_data *chip,
 	return retval;
 }
 #else
+/* An arbitrary limit so that the DMA channel doesn't run out of descriptors */
+#define MAX_DMA_LEN	SZ_64K
+#define DEFAULT_DMA_CR1	(SSCR1_TSRE | SSCR1_RSRE | SSCR1_TRAIL)
+
 static bool dma_is_possible(size_t len)
 {
-	return false;
+	return len <= MAX_DMA_LEN;
+}
+
+static int map_dma_buffer(struct driver_data *drv_data,
+			  enum dma_data_direction dir)
+{
+	int i, nents, len = drv_data->len;
+	struct scatterlist *sg;
+	struct device *dmadev;
+	struct sg_table *sgt;
+	void *buf, *pbuf;
+
+	/*
+	 * Some DMA controllers have problems transferring buffers that are
+	 * not multiple of 4 bytes. So we truncate the transfer so that it
+	 * is suitable for such controllers, and handle the trailing bytes
+	 * manually after the DMA completes.
+	 */
+	len = ALIGN(drv_data->len, 4);
+
+	if (dir == DMA_TO_DEVICE) {
+		dmadev = drv_data->tx_chan->device->dev;
+		sgt = &drv_data->tx_sgt;
+		buf = drv_data->tx;
+		drv_data->tx_map_len = len;
+	} else {
+		dmadev = drv_data->rx_chan->device->dev;
+		sgt = &drv_data->rx_sgt;
+		buf = drv_data->rx;
+		drv_data->rx_map_len = len;
+	}
+
+	nents = DIV_ROUND_UP(len, SZ_2K);
+	if (nents != sgt->nents) {
+		int ret;
+
+		sg_free_table(sgt);
+		ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
+		if (ret)
+			return ret;
+	}
+
+	pbuf = buf;
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		size_t bytes = min_t(size_t, len, SZ_2K);
+
+		if (buf)
+			sg_set_buf(sg, pbuf, bytes);
+		else
+			sg_set_buf(sg, drv_data->dummy, bytes);
+
+		pbuf += bytes;
+		len -= bytes;
+	}
+
+	nents = dma_map_sg(dmadev, sgt->sgl, sgt->nents, dir);
+	if (!nents)
+		return -ENOMEM;
+
+	return nents;
+}
+
+static void unmap_dma_buffer(struct driver_data *drv_data,
+			     enum dma_data_direction dir)
+{
+	struct device *dmadev;
+	struct sg_table *sgt;
+
+	if (dir == DMA_TO_DEVICE) {
+		dmadev = drv_data->tx_chan->device->dev;
+		sgt = &drv_data->tx_sgt;
+	} else {
+		dmadev = drv_data->rx_chan->device->dev;
+		sgt = &drv_data->rx_sgt;
+	}
+
+	dma_unmap_sg(dmadev, sgt->sgl, sgt->nents, dir);
 }
 
 static int map_dma_buffers(struct driver_data *drv_data)
 {
-	return 0;
+	const struct chip_data *chip = drv_data->cur_chip;
+	int ret;
+
+	if (!chip->enable_dma)
+		return 0;
+
+	/* Don't bother with DMA if we can't do even a single burst */
+	if (drv_data->len < chip->dma_burst_size)
+		return 0;
+
+	ret = map_dma_buffer(drv_data, DMA_TO_DEVICE);
+	if (ret <= 0) {
+		dev_warn(&drv_data->pdev->dev, "failed to DMA map TX\n");
+		return 0;
+	}
+
+	drv_data->tx_nents = ret;
+
+	ret = map_dma_buffer(drv_data, DMA_FROM_DEVICE);
+	if (ret <= 0) {
+		unmap_dma_buffer(drv_data, DMA_TO_DEVICE);
+		dev_warn(&drv_data->pdev->dev, "failed to DMA map RX\n");
+		return 0;
+	}
+
+	drv_data->rx_nents = ret;
+	return 1;
+}
+
+static void unmap_dma_buffers(struct driver_data *drv_data)
+{
+	if (!drv_data->dma_mapped)
+		return;
+
+	unmap_dma_buffer(drv_data, DMA_FROM_DEVICE);
+	unmap_dma_buffer(drv_data, DMA_TO_DEVICE);
+
+	drv_data->dma_mapped = 0;
+}
+
+static void dma_transfer_complete(struct driver_data *drv_data, bool error)
+{
+	struct spi_message *msg = drv_data->cur_msg;
+
+	/*
+	 * It is possible that one CPU is handling ROR interrupt and other
+	 * just gets DMA completion. Calling pump_transfers() twice for the
+	 * same transfer leads to problems thus we prevent concurrent calls
+	 * by using ->dma_running.
+	 */
+	if (atomic_dec_and_test(&drv_data->dma_running)) {
+		void __iomem *reg = drv_data->ioaddr;
+
+		/*
+		 * If the other CPU is still handling the ROR interrupt we
+		 * might not know about the error yet. So we re-check the
+		 * ROR bit here before we clear the status register.
+		 */
+		if (!error) {
+			u32 status = read_SSSR(reg) & drv_data->mask_sr;
+			error = status & SSSR_ROR;
+		}
+
+		/* Clear status & disable interrupts */
+		write_SSCR1(read_SSCR1(reg) & ~drv_data->dma_cr1, reg);
+		write_SSSR_CS(drv_data, drv_data->clear_sr);
+		if (!pxa25x_ssp_comp(drv_data))
+			write_SSTO(0, reg);
+
+		if (!error) {
+			unmap_dma_buffers(drv_data);
+
+			/* Handle the last bytes of unaligned transfer */
+			drv_data->tx += drv_data->tx_map_len;
+			drv_data->write(drv_data);
+
+			drv_data->rx += drv_data->rx_map_len;
+			drv_data->read(drv_data);
+
+			msg->actual_length += drv_data->len;
+			msg->state = next_transfer(drv_data);
+		} else {
+			/* In case we got an error we disable the SSP now */
+			write_SSCR0(read_SSCR0(reg) & ~SSCR0_SSE, reg);
+
+			msg->state = ERROR_STATE;
+		}
+
+		tasklet_schedule(&drv_data->pump_transfers);
+	}
 }
 
 static irqreturn_t dma_transfer(struct driver_data *drv_data)
 {
+	u32 status;
+
+	status = read_SSSR(drv_data->ioaddr) & drv_data->mask_sr;
+	if (status & SSSR_ROR) {
+		dev_err(&drv_data->pdev->dev, "FIFO overrun\n");
+
+		dmaengine_terminate_all(drv_data->rx_chan);
+		dmaengine_terminate_all(drv_data->tx_chan);
+
+		dma_transfer_complete(drv_data, true);
+		return IRQ_HANDLED;
+	}
+
 	return IRQ_NONE;
 }
 
-static void dma_prepare(struct driver_data *drv_data, u32 dma_burst)
+static void dma_callback(void *data)
+{
+	dma_transfer_complete(data, false);
+}
+
+static struct dma_async_tx_descriptor *
+dma_prepare_one(struct driver_data *drv_data, enum dma_transfer_direction dir)
+{
+	struct pxa2xx_spi_master *pdata = drv_data->master_info;
+	struct chip_data *chip = drv_data->cur_chip;
+	enum dma_slave_buswidth width;
+	struct dma_slave_config cfg;
+	struct dma_chan *chan;
+	struct sg_table *sgt;
+	int nents, ret;
+
+	switch (drv_data->n_bytes) {
+	case 1:
+		width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		break;
+	case 2:
+		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		break;
+	default:
+		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
+	}
+
+	memset(&cfg, 0, sizeof(cfg));
+	cfg.direction = dir;
+
+	if (dir == DMA_MEM_TO_DEV) {
+		cfg.dst_addr = drv_data->ssdr_physical;
+		cfg.dst_addr_width = width;
+		cfg.dst_maxburst = chip->dma_burst_size;
+		cfg.slave_id = pdata->tx_slave_id;
+
+		sgt = &drv_data->tx_sgt;
+		nents = drv_data->tx_nents;
+		chan = drv_data->tx_chan;
+	} else {
+		cfg.src_addr = drv_data->ssdr_physical;
+		cfg.src_addr_width = width;
+		cfg.src_maxburst = chip->dma_burst_size;
+		cfg.slave_id = pdata->rx_slave_id;
+
+		sgt = &drv_data->rx_sgt;
+		nents = drv_data->rx_nents;
+		chan = drv_data->rx_chan;
+	}
+
+	ret = dmaengine_slave_config(chan, &cfg);
+	if (ret) {
+		dev_warn(&drv_data->pdev->dev, "DMA slave config failed\n");
+		return NULL;
+	}
+
+	return dmaengine_prep_slave_sg(chan, sgt->sgl, nents, dir,
+				       DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+}
+
+static int dma_prepare(struct driver_data *drv_data, u32 dma_burst)
 {
+	struct dma_async_tx_descriptor *tx_desc, *rx_desc;
+
+	tx_desc = dma_prepare_one(drv_data, DMA_MEM_TO_DEV);
+	if (!tx_desc) {
+		dev_err(&drv_data->pdev->dev,
+			"failed to get DMA TX descriptor\n");
+		return -EBUSY;
+	}
+
+	rx_desc = dma_prepare_one(drv_data, DMA_DEV_TO_MEM);
+	if (!rx_desc) {
+		dev_err(&drv_data->pdev->dev,
+			"failed to get DMA RX descriptor\n");
+		return -EBUSY;
+	}
+
+	/* We are ready when RX completes */
+	rx_desc->callback = dma_callback;
+	rx_desc->callback_param = drv_data;
+
+	dmaengine_submit(rx_desc);
+	dmaengine_submit(tx_desc);
+	return 0;
 }
 
 static void dma_start(struct driver_data *drv_data)
 {
+	dma_async_issue_pending(drv_data->rx_chan);
+	dma_async_issue_pending(drv_data->tx_chan);
+
+	atomic_set(&drv_data->dma_running, 1);
+}
+
+static bool dma_filter(struct dma_chan *chan, void *param)
+{
+	const struct pxa2xx_spi_master *pdata = param;
+
+	return chan->chan_id == pdata->tx_chan_id ||
+	       chan->chan_id == pdata->rx_chan_id;
 }
 
 static int dma_setup(struct driver_data *drv_data)
 {
-	return -ENODEV;
+	struct pxa2xx_spi_master *pdata = drv_data->master_info;
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	drv_data->dummy = devm_kzalloc(&drv_data->pdev->dev, SZ_2K, GFP_KERNEL);
+	if (!drv_data->dummy)
+		return -ENOMEM;
+
+	drv_data->tx_chan = dma_request_channel(mask, dma_filter, pdata);
+	if (!drv_data->tx_chan)
+		return -ENODEV;
+
+	drv_data->rx_chan = dma_request_channel(mask, dma_filter, pdata);
+	if (!drv_data->rx_chan) {
+		dma_release_channel(drv_data->tx_chan);
+		drv_data->tx_chan = NULL;
+		return -ENODEV;
+	}
+
+	return 0;
 }
 
 static void dma_release(struct driver_data *drv_data)
 {
+	if (drv_data->rx_chan) {
+		dmaengine_terminate_all(drv_data->rx_chan);
+		dma_release_channel(drv_data->rx_chan);
+		sg_free_table(&drv_data->rx_sgt);
+		drv_data->rx_chan = NULL;
+	}
+	if (drv_data->tx_chan) {
+		dmaengine_terminate_all(drv_data->tx_chan);
+		dma_release_channel(drv_data->tx_chan);
+		sg_free_table(&drv_data->tx_sgt);
+		drv_data->tx_chan = NULL;
+	}
 }
 
 static inline void dma_resume(struct driver_data *drv_data) {}
@@ -908,7 +1234,18 @@ static int set_dma_burst_and_threshold(struct chip_data *chip,
 				u8 bits_per_word, u32 *burst_code,
 				u32 *threshold)
 {
-	return -ENODEV;
+	struct pxa2xx_spi_chip *chip_info = spi->controller_data;
+
+	/*
+	 * If the DMA burst size is given in chip_info we use that,
+	 * otherwise we use the default. Also we use the default FIFO
+	 * thresholds for now.
+	 */
+	*burst_code = chip_info ? chip_info->dma_burst_size : 16;
+	*threshold = SSCR1_RxTresh(RX_THRESH_DFLT)
+		   | SSCR1_TxTresh(TX_THRESH_DFLT);
+
+	return 0;
 }
 #endif
 
@@ -1558,7 +1895,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 		drv_data->mask_sr = SSSR_RFS | SSSR_TFS | SSSR_ROR;
 	} else {
 		drv_data->int_cr1 = SSCR1_TIE | SSCR1_RIE | SSCR1_TINTE;
-		drv_data->dma_cr1 = SSCR1_TSRE | SSCR1_RSRE | SSCR1_TINTE;
+		drv_data->dma_cr1 = DEFAULT_DMA_CR1;
 		drv_data->clear_sr = SSSR_ROR | SSSR_TINT;
 		drv_data->mask_sr = SSSR_TINT | SSSR_RFS | SSSR_TFS | SSSR_ROR;
 	}
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index 9f8cd03..6621a06 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -29,6 +29,12 @@ struct pxa2xx_spi_master {
 	u16 num_chipselect;
 	u8 enable_dma;
 
+	/* DMA engine specific config */
+	int rx_chan_id;
+	int tx_chan_id;
+	int rx_slave_id;
+	int tx_slave_id;
+
 	/* For non-PXA arches */
 	struct ssp_device ssp;
 	unsigned long fixed_clk_rate;
-- 
1.7.10.4

[PATCH 08/11] spi/pxa2xx: add support for runtime PM

[Mika Westerberg]

Drivers should put the device into low power states proactively whenever the
device is not in use. Thus implement support for runtime PM and use the
autosuspend feature to make sure that we can still perform well in case we see
lots of SPI traffic within short period of time.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c |   73 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 9b438ad..f36d7c3 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -35,6 +35,7 @@
 #include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -1378,10 +1379,20 @@ static irqreturn_t ssp_int(int irq, void *dev_id)
 {
 	struct driver_data *drv_data = dev_id;
 	void __iomem *reg = drv_data->ioaddr;
-	u32 sccr1_reg = read_SSCR1(reg);
+	u32 sccr1_reg;
 	u32 mask = drv_data->mask_sr;
 	u32 status;
 
+	/*
+	 * The IRQ might be shared with other peripherals so we must first
+	 * check that are we RPM suspended or not. If we are we assume that
+	 * the IRQ was not for us (we shouldn't be RPM suspended when the
+	 * interrupt is enabled).
+	 */
+	if (pm_runtime_suspended(&drv_data->pdev->dev))
+		return IRQ_NONE;
+
+	sccr1_reg = read_SSCR1(reg);
 	status = read_SSSR(reg);
 
 	/* Ignore possible writes if we don't need to write */
@@ -1646,6 +1657,27 @@ static int pxa2xx_spi_transfer_one_message(struct spi_master *master,
 	return 0;
 }
 
+static int pxa2xx_spi_prepare_transfer(struct spi_master *master)
+{
+	struct driver_data *drv_data = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(&drv_data->pdev->dev);
+	return 0;
+}
+
+static int pxa2xx_spi_unprepare_transfer(struct spi_master *master)
+{
+	struct driver_data *drv_data = spi_master_get_devdata(master);
+
+	/* Disable the SSP now */
+	write_SSCR0(read_SSCR0(drv_data->ioaddr) & ~SSCR0_SSE,
+		    drv_data->ioaddr);
+
+	pm_runtime_mark_last_busy(&drv_data->pdev->dev);
+	pm_runtime_put_autosuspend(&drv_data->pdev->dev);
+	return 0;
+}
+
 static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 		    struct pxa2xx_spi_chip *chip_info)
 {
@@ -1882,6 +1914,8 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	master->cleanup = cleanup;
 	master->setup = setup;
 	master->transfer_one_message = pxa2xx_spi_transfer_one_message;
+	master->prepare_transfer_hardware = pxa2xx_spi_prepare_transfer;
+	master->unprepare_transfer_hardware = pxa2xx_spi_unprepare_transfer;
 
 	drv_data->ssp_type = ssp->type;
 	drv_data->null_dma_buf = (u32 *)PTR_ALIGN(drv_data + 1, 8);
@@ -1945,6 +1979,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 		goto out_error_clock_enabled;
 	}
 
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	return status;
 
 out_error_clock_enabled:
@@ -1967,6 +2006,8 @@ static int pxa2xx_spi_remove(struct platform_device *pdev)
 		return 0;
 	ssp = drv_data->ssp;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	/* Disable the SSP at the peripheral and SOC level */
 	write_SSCR0(0, drv_data->ioaddr);
 	clk_disable(ssp->clk);
@@ -1975,6 +2016,9 @@ static int pxa2xx_spi_remove(struct platform_device *pdev)
 	if (drv_data->master_info->enable_dma)
 		dma_release(drv_data);
 
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/* Release IRQ */
 	free_irq(ssp->irq, drv_data);
 
@@ -2034,20 +2078,37 @@ static int pxa2xx_spi_resume(struct device *dev)
 
 	return 0;
 }
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int pxa2xx_spi_runtime_suspend(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+
+	clk_disable(drv_data->ssp->clk);
+	return 0;
+}
+
+static int pxa2xx_spi_runtime_resume(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+
+	clk_enable(drv_data->ssp->clk);
+	return 0;
+}
+#endif
 
 static const struct dev_pm_ops pxa2xx_spi_pm_ops = {
-	.suspend	= pxa2xx_spi_suspend,
-	.resume		= pxa2xx_spi_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(pxa2xx_spi_suspend, pxa2xx_spi_resume)
+	SET_RUNTIME_PM_OPS(pxa2xx_spi_runtime_suspend,
+			   pxa2xx_spi_runtime_resume, NULL)
 };
-#endif
 
 static struct platform_driver driver = {
 	.driver = {
 		.name	= "pxa2xx-spi",
 		.owner	= THIS_MODULE,
-#ifdef CONFIG_PM
 		.pm	= &pxa2xx_spi_pm_ops,
-#endif
 	},
 	.probe = pxa2xx_spi_probe,
 	.remove = pxa2xx_spi_remove,
-- 
1.7.10.4

[PATCH 09/11] spi/pxa2xx: add support for SPI_LOOP

[Mika Westerberg]

This is useful when testing the functionality of the controller from userspace
and there aren't any real SPI slave devices connected to the bus.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index f36d7c3..cfc4444 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1813,6 +1813,9 @@ static int setup(struct spi_device *spi)
 	chip->cr1 |= (((spi->mode & SPI_CPHA) != 0) ? SSCR1_SPH : 0)
 			| (((spi->mode & SPI_CPOL) != 0) ? SSCR1_SPO : 0);
 
+	if (spi->mode & SPI_LOOP)
+		chip->cr1 |= SSCR1_LBM;
+
 	/* NOTE:  PXA25x_SSP _could_ use external clocking ... */
 	if (!pxa25x_ssp_comp(drv_data))
 		dev_dbg(&spi->dev, "%ld Hz actual, %s\n",
@@ -1906,7 +1909,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	master->dev.parent = &pdev->dev;
 	master->dev.of_node = pdev->dev.of_node;
 	/* the spi->mode bits understood by this driver: */
-	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LOOP;
 
 	master->bus_num = ssp->port_id;
 	master->num_chipselect = platform_info->num_chipselect;
-- 
1.7.10.4

[PATCH 10/11] spi/pxa2xx: add support for Intel Low Power Subsystem SPI

[Mika Westerberg]

Intel LPSS SPI is pretty much the same as the PXA27xx SPI except that it
has few additional features over the original:

	o FIFO depth is 256 entries
	o RX FIFO has one watermark
	o TX FIFO has two watermarks, low and high
	o chip select can be controlled by writing to a register

The new FIFO registers follow immediately the PXA27xx registers but then there
are some additional LPSS private registers at offset 1k or 2k from the base
address. For these private registers we add new accessors that take advantage
of drv_data->lpss_base once it is resolved.

We add a new type LPSS_SSP that can be used to distinguish the LPSS devices
from others.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c       |  148 ++++++++++++++++++++++++++++++++++++++--
 include/linux/pxa2xx_ssp.h     |    9 +++
 include/linux/spi/pxa2xx_spi.h |    1 +
 3 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index cfc4444..d6295c0 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2005 Stephen Street / StreetFire Sound Labs
+ * Copyright (C) 2013, Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -83,6 +84,8 @@ DEFINE_SSP_REG(SSITR, 0x0c)
 DEFINE_SSP_REG(SSDR, 0x10)
 DEFINE_SSP_REG(SSTO, 0x28)
 DEFINE_SSP_REG(SSPSP, 0x2c)
+DEFINE_SSP_REG(SSITF, SSITF)
+DEFINE_SSP_REG(SSIRF, SSIRF)
 
 #define START_STATE ((void*)0)
 #define RUNNING_STATE ((void*)1)
@@ -150,6 +153,9 @@ struct driver_data {
 	int (*read)(struct driver_data *drv_data);
 	irqreturn_t (*transfer_handler)(struct driver_data *drv_data);
 	void (*cs_control)(u32 command);
+
+	/* LPSS private registers */
+	void __iomem *lpss_base;
 };
 
 struct chip_data {
@@ -161,6 +167,8 @@ struct chip_data {
 	u32 dma_burst_size;
 	u32 threshold;
 	u32 dma_threshold;
+	u16 lpss_tx_threshold;
+	u16 lpss_rx_threshold;
 	u8 enable_dma;
 	u8 bits_per_word;
 	u32 speed_hz;
@@ -174,6 +182,106 @@ struct chip_data {
 	void (*cs_control)(u32 command);
 };
 
+#define LPSS_RX_THRESH_DFLT	64
+#define LPSS_TX_LOTHRESH_DFLT	160
+#define LPSS_TX_HITHRESH_DFLT	224
+
+/* Offset from drv_data->lpss_base */
+#define PRV_CLK_PARAMS		0x00
+#define PRV_CLK_PARAMS_CLK_EN	BIT(0)
+
+#define SPI_CS_CONTROL		0x18
+#define SPI_CS_CONTROL_SW_MODE	BIT(0)
+#define SPI_CS_CONTROL_CS_HIGH	BIT(1)
+
+static bool is_lpss_ssp(const struct driver_data *drv_data)
+{
+	return drv_data->ssp_type == LPSS_SSP;
+}
+
+/*
+ * Read and write LPSS SSP private registers. Caller must first check that
+ * is_lpss_ssp() returns true before these can be called.
+ */
+static u32 __lpss_ssp_read_priv(struct driver_data *drv_data, unsigned offset)
+{
+	WARN_ON(!drv_data->lpss_base);
+	return readl(drv_data->lpss_base + offset);
+}
+
+static void __lpss_ssp_write_priv(struct driver_data *drv_data,
+				  unsigned offset, u32 value)
+{
+	WARN_ON(!drv_data->lpss_base);
+	writel(value, drv_data->lpss_base + offset);
+}
+
+/*
+ * lpss_ssp_setup - perform LPSS SSP specific setup
+ * @drv_data: pointer to the driver private data
+ *
+ * Perform LPSS SSP specific setup. This function must be called first if
+ * one is going to use LPSS SSP private registers.
+ */
+static void lpss_ssp_setup(struct driver_data *drv_data)
+{
+	unsigned offset = 0x400;
+	u32 value, orig;
+
+	if (!is_lpss_ssp(drv_data))
+		return;
+
+	/*
+	 * Perform auto-detection of the LPSS SSP private registers. They
+	 * can be either at 1k or 2k offset from the base address.
+	 */
+	orig = readl(drv_data->ioaddr + offset + SPI_CS_CONTROL);
+
+	value = orig | SPI_CS_CONTROL_SW_MODE;
+	writel(value, drv_data->ioaddr + offset + SPI_CS_CONTROL);
+	value = readl(drv_data->ioaddr + offset + SPI_CS_CONTROL);
+	if (value != (orig | SPI_CS_CONTROL_SW_MODE)) {
+		offset = 0x800;
+		goto detection_done;
+	}
+
+	value &= ~SPI_CS_CONTROL_SW_MODE;
+	writel(value, drv_data->ioaddr + offset + SPI_CS_CONTROL);
+	value = readl(drv_data->ioaddr + offset + SPI_CS_CONTROL);
+	if (value != orig) {
+		offset = 0x800;
+		goto detection_done;
+	}
+
+detection_done:
+	/* Now set the LPSS base */
+	drv_data->lpss_base = drv_data->ioaddr + offset;
+
+	/* Enable the functional clock */
+	value = __lpss_ssp_read_priv(drv_data, PRV_CLK_PARAMS);
+	value |= PRV_CLK_PARAMS_CLK_EN;
+	__lpss_ssp_write_priv(drv_data, PRV_CLK_PARAMS, value);
+
+	/* Enable software chip select control */
+	value = SPI_CS_CONTROL_SW_MODE | SPI_CS_CONTROL_CS_HIGH;
+	__lpss_ssp_write_priv(drv_data, SPI_CS_CONTROL, value);
+}
+
+static void lpss_ssp_cs_control(struct driver_data *drv_data, bool enable)
+{
+	u32 value;
+
+	if (!is_lpss_ssp(drv_data))
+		return;
+
+	value = __lpss_ssp_read_priv(drv_data, SPI_CS_CONTROL);
+	if (enable)
+		value &= ~SPI_CS_CONTROL_CS_HIGH;
+	else
+		value |= SPI_CS_CONTROL_CS_HIGH;
+	__lpss_ssp_write_priv(drv_data, SPI_CS_CONTROL, value);
+}
+
 static void cs_assert(struct driver_data *drv_data)
 {
 	struct chip_data *chip = drv_data->cur_chip;
@@ -188,8 +296,12 @@ static void cs_assert(struct driver_data *drv_data)
 		return;
 	}
 
-	if (gpio_is_valid(chip->gpio_cs))
+	if (gpio_is_valid(chip->gpio_cs)) {
 		gpio_set_value(chip->gpio_cs, chip->gpio_cs_inverted);
+		return;
+	}
+
+	lpss_ssp_cs_control(drv_data, true);
 }
 
 static void cs_deassert(struct driver_data *drv_data)
@@ -204,8 +316,12 @@ static void cs_deassert(struct driver_data *drv_data)
 		return;
 	}
 
-	if (gpio_is_valid(chip->gpio_cs))
+	if (gpio_is_valid(chip->gpio_cs)) {
 		gpio_set_value(chip->gpio_cs, !chip->gpio_cs_inverted);
+		return;
+	}
+
+	lpss_ssp_cs_control(drv_data, false);
 }
 
 static void write_SSSR_CS(struct driver_data *drv_data, u32 val)
@@ -1610,6 +1726,13 @@ static void pump_transfers(unsigned long data)
 		write_SSSR_CS(drv_data, drv_data->clear_sr);
 	}
 
+	if (is_lpss_ssp(drv_data)) {
+		if ((read_SSIRF(reg) & 0xff) != chip->lpss_rx_threshold)
+			write_SSIRF(chip->lpss_rx_threshold, reg);
+		if ((read_SSITF(reg) & 0xffff) != chip->lpss_tx_threshold)
+			write_SSITF(chip->lpss_tx_threshold, reg);
+	}
+
 	/* see if we need to reload the config registers */
 	if ((read_SSCR0(reg) != cr0)
 		|| (read_SSCR1(reg) & SSCR1_CHANGE_MASK) !=
@@ -1722,8 +1845,17 @@ static int setup(struct spi_device *spi)
 	struct chip_data *chip;
 	struct driver_data *drv_data = spi_master_get_devdata(spi->master);
 	unsigned int clk_div;
-	uint tx_thres = TX_THRESH_DFLT;
-	uint rx_thres = RX_THRESH_DFLT;
+	uint tx_thres, tx_hi_thres, rx_thres;
+
+	if (is_lpss_ssp(drv_data)) {
+		tx_thres = LPSS_TX_LOTHRESH_DFLT;
+		tx_hi_thres = LPSS_TX_HITHRESH_DFLT;
+		rx_thres = LPSS_RX_THRESH_DFLT;
+	} else {
+		tx_thres = TX_THRESH_DFLT;
+		tx_hi_thres = 0;
+		rx_thres = RX_THRESH_DFLT;
+	}
 
 	if (!pxa25x_ssp_comp(drv_data)
 		&& (spi->bits_per_word < 4 || spi->bits_per_word > 32)) {
@@ -1776,6 +1908,8 @@ static int setup(struct spi_device *spi)
 			chip->timeout = chip_info->timeout;
 		if (chip_info->tx_threshold)
 			tx_thres = chip_info->tx_threshold;
+		if (chip_info->tx_hi_threshold)
+			tx_hi_thres = chip_info->tx_hi_threshold;
 		if (chip_info->rx_threshold)
 			rx_thres = chip_info->rx_threshold;
 		chip->enable_dma = drv_data->master_info->enable_dma;
@@ -1787,6 +1921,10 @@ static int setup(struct spi_device *spi)
 	chip->threshold = (SSCR1_RxTresh(rx_thres) & SSCR1_RFT) |
 			(SSCR1_TxTresh(tx_thres) & SSCR1_TFT);
 
+	chip->lpss_rx_threshold = SSIRF_RxThresh(rx_thres);
+	chip->lpss_tx_threshold = SSITF_TxLoThresh(tx_thres)
+				| SSITF_TxHiThresh(tx_hi_thres);
+
 	/* set dma burst and threshold outside of chip_info path so that if
 	 * chip_info goes away after setting chip->enable_dma, the
 	 * burst and threshold can still respond to changes in bits_per_word */
@@ -1971,6 +2109,8 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 		write_SSTO(0, drv_data->ioaddr);
 	write_SSPSP(0, drv_data->ioaddr);
 
+	lpss_ssp_setup(drv_data);
+
 	tasklet_init(&drv_data->pump_transfers, pump_transfers,
 		     (unsigned long)drv_data);
 
diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index 065e7f6..467cc63 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -155,6 +155,14 @@
 #define SSACD_ACDS(x)		((x) << 0)	/* Audio clock divider select */
 #define SSACD_SCDX8		(1 << 7)	/* SYSCLK division ratio select */
 
+/* LPSS SSP */
+#define SSITF			0x44		/* TX FIFO trigger level */
+#define SSITF_TxLoThresh(x)	(((x) - 1) << 8)
+#define SSITF_TxHiThresh(x)	((x) - 1)
+
+#define SSIRF			0x48		/* RX FIFO trigger level */
+#define SSIRF_RxThresh(x)	((x) - 1)
+
 enum pxa_ssp_type {
 	SSP_UNDEFINED = 0,
 	PXA25x_SSP,  /* pxa 210, 250, 255, 26x */
@@ -164,6 +172,7 @@ enum pxa_ssp_type {
 	PXA168_SSP,
 	PXA910_SSP,
 	CE4100_SSP,
+	LPSS_SSP,
 };
 
 struct ssp_device {
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index 6621a06..46b0ac4 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -45,6 +45,7 @@ struct pxa2xx_spi_master {
  */
 struct pxa2xx_spi_chip {
 	u8 tx_threshold;
+	u8 tx_hi_threshold;
 	u8 rx_threshold;
 	u8 dma_burst_size;
 	u32 timeout;
-- 
1.7.10.4

[PATCH 11/11] spi/pxa2xx: add support for Lynxpoint SPI controllers

[Mika Westerberg]

Intel Lynxpoint PCH Low Power Subsystem has two general purpose SPI
controllers that are LPSS_SSP compatible. These controllers are enumerated
from ACPI namespace with ACPI IDs INT33C0 and INT33C1.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/Kconfig      |    2 +-
 drivers/spi/spi-pxa2xx.c |  131 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 130 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index a90393d..bf1d3b5 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -299,7 +299,7 @@ config SPI_PPC4xx
 
 config SPI_PXA2XX
 	tristate "PXA2xx SSP SPI master"
-	depends on ARCH_PXA || PCI
+	depends on ARCH_PXA || PCI || ACPI
 	select PXA_SSP if ARCH_PXA
 	help
 	  This enables using a PXA2xx or Sodaville SSP port as a SPI master
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index d6295c0..9ebbfcf 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
+#include <linux/acpi.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -1916,6 +1917,13 @@ static int setup(struct spi_device *spi)
 		chip->dma_threshold = 0;
 		if (chip_info->enable_loopback)
 			chip->cr1 = SSCR1_LBM;
+	} else if (ACPI_HANDLE(&spi->dev)) {
+		/*
+		 * Slave devices enumerated from ACPI namespace don't
+		 * usually have chip_info but we still might want to use
+		 * DMA with them.
+		 */
+		chip->enable_dma = drv_data->master_info->enable_dma;
 	}
 
 	chip->threshold = (SSCR1_RxTresh(rx_thres) & SSCR1_RFT) |
@@ -2007,6 +2015,120 @@ static void cleanup(struct spi_device *spi)
 	kfree(chip);
 }
 
+#ifdef CONFIG_ACPI
+struct pxa2xx_spi_acpi_config {
+	enum pxa_ssp_type type;
+	unsigned long clk_rate;
+};
+
+/* Lynxpoint SPI controllers */
+static const struct pxa2xx_spi_acpi_config lpt_acpi_config = {
+	.type = LPSS_SSP,
+	.clk_rate = 100000000,
+};
+
+static struct acpi_device_id pxa2xx_spi_acpi_match[] = {
+	{ "INT33C0", (kernel_ulong_t)&lpt_acpi_config },
+	{ "INT33C1", (kernel_ulong_t)&lpt_acpi_config },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);
+
+static int pxa2xx_spi_acpi_add_dma(struct acpi_resource *res, void *data)
+{
+	struct pxa2xx_spi_master *pdata = data;
+
+	if (res->type == ACPI_RESOURCE_TYPE_FIXED_DMA) {
+		const struct acpi_resource_fixed_dma *dma;
+
+		dma = &res->data.fixed_dma;
+		if (pdata->tx_slave_id < 0) {
+			pdata->tx_slave_id = dma->request_lines;
+			pdata->tx_chan_id = dma->channels;
+		} else if (pdata->rx_slave_id < 0) {
+			pdata->rx_slave_id = dma->request_lines;
+			pdata->rx_chan_id = dma->channels;
+		}
+	}
+
+	/* Tell the ACPI core to skip this resource */
+	return 1;
+}
+
+static struct pxa2xx_spi_master *
+pxa2xx_spi_acpi_get_pdata(struct platform_device *pdev)
+{
+	const struct pxa2xx_spi_acpi_config *conf;
+	struct pxa2xx_spi_master *pdata;
+	const struct acpi_device_id *id;
+	struct list_head resource_list;
+	struct acpi_device *adev;
+	struct ssp_device *ssp;
+	struct resource *res;
+	int devid;
+
+	if (!ACPI_HANDLE(&pdev->dev) ||
+	    acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return NULL;
+
+	id = acpi_match_device(pxa2xx_spi_acpi_match, &pdev->dev);
+	if (!id)
+		return NULL;
+
+	conf = (const struct pxa2xx_spi_acpi_config *)id->driver_data;
+	if (!conf)
+		return NULL;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(&pdev->dev,
+			"failed to allocate memory for platform data\n");
+		return NULL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return NULL;
+
+	ssp = &pdata->ssp;
+
+	ssp->phys_base = res->start;
+	ssp->mmio_base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!ssp->mmio_base) {
+		dev_err(&pdev->dev, "failed to ioremap mmio_base\n");
+		return NULL;
+	}
+
+	ssp->irq = platform_get_irq(pdev, 0);
+	ssp->type = conf->type;
+	ssp->pdev = pdev;
+
+	ssp->port_id = -1;
+	if (adev->pnp.unique_id && !kstrtoint(adev->pnp.unique_id, 0, &devid))
+		ssp->port_id = devid;
+
+	pdata->num_chipselect = 1;
+	pdata->rx_slave_id = -1;
+	pdata->tx_slave_id = -1;
+	pdata->fixed_clk_rate = conf->clk_rate;
+
+	INIT_LIST_HEAD(&resource_list);
+	acpi_dev_get_resources(adev, &resource_list, pxa2xx_spi_acpi_add_dma,
+			       pdata);
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->enable_dma = pdata->rx_slave_id >= 0 && pdata->tx_slave_id >= 0;
+
+	return pdata;
+}
+#else
+static inline struct pxa2xx_spi_master *
+pxa2xx_spi_acpi_get_pdata(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
 static int pxa2xx_spi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -2018,8 +2140,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 
 	platform_info = dev_get_platdata(dev);
 	if (!platform_info) {
-		dev_err(&pdev->dev, "missing platform data\n");
-		return -ENODEV;
+		platform_info = pxa2xx_spi_acpi_get_pdata(pdev);
+		if (!platform_info) {
+			dev_err(&pdev->dev, "missing platform data\n");
+			return -ENODEV;
+		}
 	}
 
 	ssp = pxa_ssp_request(pdev->id, pdev->name);
@@ -2046,6 +2171,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 
 	master->dev.parent = &pdev->dev;
 	master->dev.of_node = pdev->dev.of_node;
+	ACPI_HANDLE_SET(&master->dev, ACPI_HANDLE(&pdev->dev));
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LOOP;
 
@@ -2252,6 +2378,7 @@ static struct platform_driver driver = {
 		.name	= "pxa2xx-spi",
 		.owner	= THIS_MODULE,
 		.pm	= &pxa2xx_spi_pm_ops,
+		.acpi_match_table = ACPI_PTR(pxa2xx_spi_acpi_match),
 	},
 	.probe = pxa2xx_spi_probe,
 	.remove = pxa2xx_spi_remove,
-- 
1.7.10.4

Re: [PATCH 01/11] spi/pxa2xx: allow building on a 64-bit kernel

[Eric Miao]

On Mon, Jan 7, 2013 at 6:44 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> In addition fix following warnings seen when compiling 64-bit:
>
> drivers/spi/spi-pxa2xx.c: In function ‘map_dma_buffers’: drivers/spi/spi-pxa2xx.c:384:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> drivers/spi/spi-pxa2xx.c:384:40: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> drivers/spi/spi-pxa2xx.c: In function ‘pxa2xx_spi_probe’:
> drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> drivers/spi/spi-pxa2xx.c:1572:27: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/spi/Kconfig      |    4 ++--
>  drivers/spi/spi-pxa2xx.c |    5 ++---
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 2e188e1..a90393d 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -299,7 +299,7 @@ config SPI_PPC4xx
>
>  config SPI_PXA2XX
>         tristate "PXA2xx SSP SPI master"
> -       depends on (ARCH_PXA || (X86_32 && PCI)) && EXPERIMENTAL
> +       depends on ARCH_PXA || PCI
>         select PXA_SSP if ARCH_PXA
>         help
>           This enables using a PXA2xx or Sodaville SSP port as a SPI master
> @@ -307,7 +307,7 @@ config SPI_PXA2XX
>           additional documentation can be found a Documentation/spi/pxa2xx.
>
>  config SPI_PXA2XX_PCI
> -       def_bool SPI_PXA2XX && X86_32 && PCI
> +       def_tristate SPI_PXA2XX && PCI
>

Generally looks good to me, I think we could split the changes to

* Kconfig (adding 64-bit support or removing restrictions of X86_32
for the driver)
* and to spi-pxa2xx.c (mostly to handle the alignment warnings)

>  config SPI_RSPI
>         tristate "Renesas RSPI controller"
> diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> index 5c8c4f5..7fac65d 100644
> --- a/drivers/spi/spi-pxa2xx.c
> +++ b/drivers/spi/spi-pxa2xx.c
> @@ -47,7 +47,7 @@ MODULE_ALIAS("platform:pxa2xx-spi");
>
>  #define DMA_INT_MASK           (DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
>  #define RESET_DMA_CHANNEL      (DCSR_NODESC | DMA_INT_MASK)
> -#define IS_DMA_ALIGNED(x)      ((((u32)(x)) & 0x07) == 0)
> +#define IS_DMA_ALIGNED(x)      IS_ALIGNED((unsigned long)x, DMA_ALIGNMENT)

OK.

>  #define MAX_DMA_LEN            8191
>  #define DMA_ALIGNMENT          8
>
> @@ -1569,8 +1569,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
>         master->transfer = transfer;
>
>         drv_data->ssp_type = ssp->type;
> -       drv_data->null_dma_buf = (u32 *)ALIGN((u32)(drv_data +
> -                                               sizeof(struct driver_data)), 8);
> +       drv_data->null_dma_buf = (u32 *)PTR_ALIGN(drv_data + 1, 8);

Hmm... the original code seems to have big problem and interestingly no
one has reported the issue, possibly due to null_dma_buf being seldomly
used.

However, it's still a bit obscure to have 'drv_data + 1', 'drv_data[1]' might
be a bit better for readability.

And it'll be better to have DMA_ALIGNTMENT here instead of a hard-coded
constant '8'.
    u

>
>         drv_data->ioaddr = ssp->mmio_base;
>         drv_data->ssdr_physical = ssp->phys_base + SSDR;
> --
> 1.7.10.4
>

Re: [PATCH 01/11] spi/pxa2xx: allow building on a 64-bit kernel

[Mika Westerberg]

On Tue, Jan 08, 2013 at 11:27:45AM +0800, Eric Miao wrote:
> On Mon, Jan 7, 2013 at 6:44 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > In addition fix following warnings seen when compiling 64-bit:
> >
> > drivers/spi/spi-pxa2xx.c: In function ‘map_dma_buffers’: drivers/spi/spi-pxa2xx.c:384:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > drivers/spi/spi-pxa2xx.c:384:40: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > drivers/spi/spi-pxa2xx.c: In function ‘pxa2xx_spi_probe’:
> > drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > drivers/spi/spi-pxa2xx.c:1572:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > drivers/spi/spi-pxa2xx.c:1572:27: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/spi/Kconfig      |    4 ++--
> >  drivers/spi/spi-pxa2xx.c |    5 ++---
> >  2 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index 2e188e1..a90393d 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -299,7 +299,7 @@ config SPI_PPC4xx
> >
> >  config SPI_PXA2XX
> >         tristate "PXA2xx SSP SPI master"
> > -       depends on (ARCH_PXA || (X86_32 && PCI)) && EXPERIMENTAL
> > +       depends on ARCH_PXA || PCI
> >         select PXA_SSP if ARCH_PXA
> >         help
> >           This enables using a PXA2xx or Sodaville SSP port as a SPI master
> > @@ -307,7 +307,7 @@ config SPI_PXA2XX
> >           additional documentation can be found a Documentation/spi/pxa2xx.
> >
> >  config SPI_PXA2XX_PCI
> > -       def_bool SPI_PXA2XX && X86_32 && PCI
> > +       def_tristate SPI_PXA2XX && PCI
> >
> 
> Generally looks good to me, I think we could split the changes to
> 
> * Kconfig (adding 64-bit support or removing restrictions of X86_32
> for the driver)
> * and to spi-pxa2xx.c (mostly to handle the alignment warnings)

OK.

> 
> >  config SPI_RSPI
> >         tristate "Renesas RSPI controller"
> > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> > index 5c8c4f5..7fac65d 100644
> > --- a/drivers/spi/spi-pxa2xx.c
> > +++ b/drivers/spi/spi-pxa2xx.c
> > @@ -47,7 +47,7 @@ MODULE_ALIAS("platform:pxa2xx-spi");
> >
> >  #define DMA_INT_MASK           (DCSR_ENDINTR | DCSR_STARTINTR | DCSR_BUSERR)
> >  #define RESET_DMA_CHANNEL      (DCSR_NODESC | DMA_INT_MASK)
> > -#define IS_DMA_ALIGNED(x)      ((((u32)(x)) & 0x07) == 0)
> > +#define IS_DMA_ALIGNED(x)      IS_ALIGNED((unsigned long)x, DMA_ALIGNMENT)
> 
> OK.
> 
> >  #define MAX_DMA_LEN            8191
> >  #define DMA_ALIGNMENT          8
> >
> > @@ -1569,8 +1569,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
> >         master->transfer = transfer;
> >
> >         drv_data->ssp_type = ssp->type;
> > -       drv_data->null_dma_buf = (u32 *)ALIGN((u32)(drv_data +
> > -                                               sizeof(struct driver_data)), 8);
> > +       drv_data->null_dma_buf = (u32 *)PTR_ALIGN(drv_data + 1, 8);
> 
> Hmm... the original code seems to have big problem and interestingly no
> one has reported the issue, possibly due to null_dma_buf being seldomly
> used.
> 
> However, it's still a bit obscure to have 'drv_data + 1', 'drv_data[1]' might
> be a bit better for readability.
> 
> And it'll be better to have DMA_ALIGNTMENT here instead of a hard-coded
> constant '8'.
>     u

Sure, I'll change that.

Thanks for your comments.

Re: [PATCH 03/11] spi/pxa2xx-pci: switch to use pcim_* interfaces

[Mark Brown]

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

On Mon, Jan 07, 2013 at 12:44:32PM +0200, Mika Westerberg wrote:
> Instead of open-coding all the error management in the driver we can take
> advantage of the pcim_* interfaces that release the resources automatically.

Applied, thanks.

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Mon, Jan 07, 2013 at 12:44:34PM +0200, Mika Westerberg wrote:
> If the architecture doesn't support clk framework (like x86) we need a way to
> pass the SSP clock rate to the driver. This patch adds a field in the platform
> data 'fixed_clk_rate' that allows passing the rate.

No, the way to do this is to fix x86 to enable the clock API there.  The
x86 maintainers couldn't be bothered when I submitted a patch and
getting anyone to take a patch to make it available by default appears
to be unreasonably hard but perhaps if someone from Intel tries the x86
maintainers might take a patch...

We shouldn't be adding special case code to every driver that might need
a clock that gets used on an Intel system.

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Tue, Jan 08, 2013 at 11:02:28AM +0000, Mark Brown wrote:
> On Mon, Jan 07, 2013 at 12:44:34PM +0200, Mika Westerberg wrote:
> > If the architecture doesn't support clk framework (like x86) we need a way to
> > pass the SSP clock rate to the driver. This patch adds a field in the platform
> > data 'fixed_clk_rate' that allows passing the rate.
> 
> No, the way to do this is to fix x86 to enable the clock API there.  The
> x86 maintainers couldn't be bothered when I submitted a patch and
> getting anyone to take a patch to make it available by default appears
> to be unreasonably hard but perhaps if someone from Intel tries the x86
> maintainers might take a patch...

Do you mean enabling CONFIG_COMMON_CLK on x86?

> We shouldn't be adding special case code to every driver that might need
> a clock that gets used on an Intel system.

I agree and it is cleaner to have the same API for all arches. However, I'm
not sure how do we create the fixed clocks then? There is nothing in ACPI
namespace (or in the ACPI 5.0 spec) that allows you to describe clocks and
their relationships.

So then we end up either:

	1. Creating a custom board file for Lynxpoint which creates the
	   clocks. This is something I think x86 maintainers don't want to
	   see.

	2. Creating the clock in the driver itself in its ACPI enumeration
	   implementation. This might work but it litters the drivers with
	   clock details.

Or is there something I'm missing?

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:
> On Tue, Jan 08, 2013 at 11:02:28AM +0000, Mark Brown wrote:

> > No, the way to do this is to fix x86 to enable the clock API there.  The
> > x86 maintainers couldn't be bothered when I submitted a patch and
> > getting anyone to take a patch to make it available by default appears
> > to be unreasonably hard but perhaps if someone from Intel tries the x86
> > maintainers might take a patch...

> Do you mean enabling CONFIG_COMMON_CLK on x86?

Yes.

> > We shouldn't be adding special case code to every driver that might need
> > a clock that gets used on an Intel system.

> I agree and it is cleaner to have the same API for all arches. However, I'm
> not sure how do we create the fixed clocks then? There is nothing in ACPI
> namespace (or in the ACPI 5.0 spec) that allows you to describe clocks and
> their relationships.

I'm sure it's not beyond the bounds of possibility that we could solve
this problem...

> So then we end up either:

> 	1. Creating a custom board file for Lynxpoint which creates the
> 	   clocks. This is something I think x86 maintainers don't want to
> 	   see.

> 	2. Creating the clock in the driver itself in its ACPI enumeration
> 	   implementation. This might work but it litters the drivers with
> 	   clock details.

> Or is there something I'm missing?

Well, it seems fairly clear to me that the SoC wiring ought to be done
outside the driver.

It is something that needs to be resolved for your smartphone SoCs for
this and for other things like platform data for external chips, what's
actually happening in practical systems here is that people are just
hacking the arch code as there's no mechanism for providing
configuration at present.

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On 1/8/2013 2:10 PM, Mark Brown wrote:
> On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:
>> On Tue, Jan 08, 2013 at 11:02:28AM +0000, Mark Brown wrote:
>>> No, the way to do this is to fix x86 to enable the clock API there.  The
>>> x86 maintainers couldn't be bothered when I submitted a patch and
>>> getting anyone to take a patch to make it available by default appears
>>> to be unreasonably hard but perhaps if someone from Intel tries the x86
>>> maintainers might take a patch...
>> Do you mean enabling CONFIG_COMMON_CLK on x86?
> Yes.

Why so?  x86 doesn't have a notion of direct clock control, at least not 
on the ACPI systems.

>>> We shouldn't be adding special case code to every driver that might need
>>> a clock that gets used on an Intel system.
>> I agree and it is cleaner to have the same API for all arches. However, I'm
>> not sure how do we create the fixed clocks then? There is nothing in ACPI
>> namespace (or in the ACPI 5.0 spec) that allows you to describe clocks and
>> their relationships.
> I'm sure it's not beyond the bounds of possibility that we could solve
> this problem...

No, it isn't.  Any suggestions?

>> So then we end up either:
>> 	1. Creating a custom board file for Lynxpoint which creates the
>> 	   clocks. This is something I think x86 maintainers don't want to
>> 	   see.
>> 	2. Creating the clock in the driver itself in its ACPI enumeration
>> 	   implementation. This might work but it litters the drivers with
>> 	   clock details.
>> Or is there something I'm missing?
> Well, it seems fairly clear to me that the SoC wiring ought to be done
> outside the driver.
>
> It is something that needs to be resolved for your smartphone SoCs for

We're not talking about smartphones and even not about SoCs here, sorry.

This is a laptop CPU that happens to have an SPI controller integrated.

> this and for other things like platform data for external chips, what's
> actually happening in practical systems here is that people are just
> hacking the arch code as there's no mechanism for providing
> configuration at present.

I'm not sure what you're referring to, but here we have ACPI as a 
configuration mechanism.

Which doesn't allow us to control clocks directly.

Thanks,
Rafael

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On 1/8/2013 12:02 PM, Mark Brown wrote:
> On Mon, Jan 07, 2013 at 12:44:34PM +0200, Mika Westerberg wrote:
>> If the architecture doesn't support clk framework (like x86) we need a way to
>> pass the SSP clock rate to the driver. This patch adds a field in the platform
>> data 'fixed_clk_rate' that allows passing the rate.
> No, the way to do this is to fix x86 to enable the clock API there.  The
> x86 maintainers couldn't be bothered when I submitted a patch and
> getting anyone to take a patch to make it available by default appears
> to be unreasonably hard but perhaps if someone from Intel tries the x86
> maintainers might take a patch...

OK

Please explain what you'd like to do.

I haven't seen the original patch.

> We shouldn't be adding special case code to every driver that might need
> a clock that gets used on an Intel system.

I agree with that FWIW.

Thanks,
Rafael

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Tue, Jan 08, 2013 at 10:33:55PM +0100, Rafael J. Wysocki wrote:
> On 1/8/2013 2:10 PM, Mark Brown wrote:
> >On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:
> >>On Tue, Jan 08, 2013 at 11:02:28AM +0000, Mark Brown wrote:
> >>>No, the way to do this is to fix x86 to enable the clock API there.  The
> >>>x86 maintainers couldn't be bothered when I submitted a patch and
> >>>getting anyone to take a patch to make it available by default appears
> >>>to be unreasonably hard but perhaps if someone from Intel tries the x86
> >>>maintainers might take a patch...
> >>Do you mean enabling CONFIG_COMMON_CLK on x86?
> >Yes.
> 
> Why so?  x86 doesn't have a notion of direct clock control, at least
> not on the ACPI systems.
> 
> >>>We shouldn't be adding special case code to every driver that might need
> >>>a clock that gets used on an Intel system.
> >>I agree and it is cleaner to have the same API for all arches. However, I'm
> >>not sure how do we create the fixed clocks then? There is nothing in ACPI
> >>namespace (or in the ACPI 5.0 spec) that allows you to describe clocks and
> >>their relationships.
> >I'm sure it's not beyond the bounds of possibility that we could solve
> >this problem...
> 
> No, it isn't.  Any suggestions?

I have one suggestion.

Since even on x86 we are now starting to see peripherals that exists
normally on traditional SoCs, like the SPI controller, and the drivers
expect to have clock for these.

What if we make drivers/clk/clk-x86.c that initializes necessary clocks
like the LPSS 100MHz fixed clock and registers this to all the LPSS
devices? Something along the lines of:

        clk = clk_register_fixed_rate(NULL, "lpss_iclk", NULL, CLK_IS_ROOT,
				      100000000);

	...

	clk_register_clkdev(clk, NULL, "INT33C0:00");

These are clocks that you really can't control (enable/disable) but they
allow one to get the clock rate using the standard clk API.

One problem (among other things) with this is that now we have these clocks
created and registered on every single x86 system. Other problem is that
this setup needs manual maintenance as we can't get the configuration from
ACPI.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Tue, Jan 08, 2013 at 10:33:55PM +0100, Rafael J. Wysocki wrote:
> On 1/8/2013 2:10 PM, Mark Brown wrote:
> >On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:

> >>Do you mean enabling CONFIG_COMMON_CLK on x86?

> >Yes.

> Why so?  x86 doesn't have a notion of direct clock control, at least
> not on the ACPI systems.

So, a couple of things here.  One is that your SFI systems *do* have
user controllable clocks so they really should be using the clock API.

The other is that even if the CPU doesn't want to use clocks off-CPU
devices may wish to - for example, a PCI card using off the shelf
components or in this case a device on the SPI bus on the laptop which
requires a clock and wants to manage it at runtime if possible.  We
should be able to arrange things so that drivers can work with clocks
using the standard clock API.  I'm really hopinng that having the clock
API will eventually enable us to work with clocks in off-SoC devices but
right now the only option is open coding.

> >I'm sure it's not beyond the bounds of possibility that we could solve
> >this problem...

> No, it isn't.  Any suggestions?

Well, the most obvious solution would be to just hard code this
information in the kernel and set it up based on what you do know.  For
example if this is a SoC-internal clock then set it up based on knowing
the SoC (this seems to be basically what Mika is suggesting).

> >It is something that needs to be resolved for your smartphone SoCs for

> We're not talking about smartphones and even not about SoCs here, sorry.

> This is a laptop CPU that happens to have an SPI controller integrated.

These are still x86 systems so are part of why it seems like a good idea
to make the clock API available on x86.

> >this and for other things like platform data for external chips, what's
> >actually happening in practical systems here is that people are just
> >hacking the arch code as there's no mechanism for providing
> >configuration at present.

> I'm not sure what you're referring to, but here we have ACPI as a
> configuration mechanism.

> Which doesn't allow us to control clocks directly.

Right, and because the BIOS guys don't provide any mechanism for
handling this stuff using the BIOS information what people actually
deploying systems are doing is just hacking the arch code which gets us
the worst of both worlds - we have to put things into the BIOS but we
then have to put extra information into the kernel to actually make
things functional.

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Wednesday, January 09, 2013 12:51:07 PM Mika Westerberg wrote:
> On Tue, Jan 08, 2013 at 10:33:55PM +0100, Rafael J. Wysocki wrote:
> > On 1/8/2013 2:10 PM, Mark Brown wrote:
> > >On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:
> > >>On Tue, Jan 08, 2013 at 11:02:28AM +0000, Mark Brown wrote:
> > >>>No, the way to do this is to fix x86 to enable the clock API there.  The
> > >>>x86 maintainers couldn't be bothered when I submitted a patch and
> > >>>getting anyone to take a patch to make it available by default appears
> > >>>to be unreasonably hard but perhaps if someone from Intel tries the x86
> > >>>maintainers might take a patch...
> > >>Do you mean enabling CONFIG_COMMON_CLK on x86?
> > >Yes.
> > 
> > Why so?  x86 doesn't have a notion of direct clock control, at least
> > not on the ACPI systems.
> > 
> > >>>We shouldn't be adding special case code to every driver that might need
> > >>>a clock that gets used on an Intel system.
> > >>I agree and it is cleaner to have the same API for all arches. However, I'm
> > >>not sure how do we create the fixed clocks then? There is nothing in ACPI
> > >>namespace (or in the ACPI 5.0 spec) that allows you to describe clocks and
> > >>their relationships.
> > >I'm sure it's not beyond the bounds of possibility that we could solve
> > >this problem...
> > 
> > No, it isn't.  Any suggestions?
> 
> I have one suggestion.
> 
> Since even on x86 we are now starting to see peripherals that exists
> normally on traditional SoCs, like the SPI controller, and the drivers
> expect to have clock for these.
> 
> What if we make drivers/clk/clk-x86.c that initializes necessary clocks
> like the LPSS 100MHz fixed clock and registers this to all the LPSS
> devices? Something along the lines of:
> 
>         clk = clk_register_fixed_rate(NULL, "lpss_iclk", NULL, CLK_IS_ROOT,
> 				      100000000);
> 
> 	...
> 
> 	clk_register_clkdev(clk, NULL, "INT33C0:00");
> 
> These are clocks that you really can't control (enable/disable) but they
> allow one to get the clock rate using the standard clk API.
> 
> One problem (among other things) with this is that now we have these clocks
> created and registered on every single x86 system.

Well, we can make them depend on some .config options.

> Other problem is that this setup needs manual maintenance as we can't get
> the configuration from ACPI.

So we just happen to know what the rate is supposed to be?  What's the source
of that information?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Wednesday, January 09, 2013 12:25:50 PM Mark Brown wrote:
> On Tue, Jan 08, 2013 at 10:33:55PM +0100, Rafael J. Wysocki wrote:
> > On 1/8/2013 2:10 PM, Mark Brown wrote:
> > >On Tue, Jan 08, 2013 at 02:41:53PM +0200, Mika Westerberg wrote:
> 
> > >>Do you mean enabling CONFIG_COMMON_CLK on x86?
> 
> > >Yes.
> 
> > Why so?  x86 doesn't have a notion of direct clock control, at least
> > not on the ACPI systems.
> 
> So, a couple of things here.  One is that your SFI systems *do* have
> user controllable clocks so they really should be using the clock API.

Well, fair enough.

> The other is that even if the CPU doesn't want to use clocks off-CPU
> devices may wish to - for example, a PCI card using off the shelf
> components or in this case a device on the SPI bus on the laptop which
> requires a clock and wants to manage it at runtime if possible.  We
> should be able to arrange things so that drivers can work with clocks
> using the standard clock API.  I'm really hopinng that having the clock
> API will eventually enable us to work with clocks in off-SoC devices but
> right now the only option is open coding.
> 
> > >I'm sure it's not beyond the bounds of possibility that we could solve
> > >this problem...
> 
> > No, it isn't.  Any suggestions?
> 
> Well, the most obvious solution would be to just hard code this
> information in the kernel and set it up based on what you do know.  For
> example if this is a SoC-internal clock then set it up based on knowing
> the SoC (this seems to be basically what Mika is suggesting).

Yes, it seems that we just happen to know the right value.  Sigh.

> > >It is something that needs to be resolved for your smartphone SoCs for
> 
> > We're not talking about smartphones and even not about SoCs here, sorry.
> 
> > This is a laptop CPU that happens to have an SPI controller integrated.
> 
> These are still x86 systems so are part of why it seems like a good idea
> to make the clock API available on x86.
> 
> > >this and for other things like platform data for external chips, what's
> > >actually happening in practical systems here is that people are just
> > >hacking the arch code as there's no mechanism for providing
> > >configuration at present.
> 
> > I'm not sure what you're referring to, but here we have ACPI as a
> > configuration mechanism.
> 
> > Which doesn't allow us to control clocks directly.
> 
> Right, and because the BIOS guys don't provide any mechanism for
> handling this stuff using the BIOS information what people actually
> deploying systems are doing is just hacking the arch code which gets us
> the worst of both worlds - we have to put things into the BIOS but we
> then have to put extra information into the kernel to actually make
> things functional.

We hardcode different kinds of information into the kernel anyway, but it
would be good to be able to use some criteria to decide whether or not we need
that information on the given system.

In this particular case, we'll only need the fixed clock rate for the SPI
which is not present on other x86 systems to date.

Mika, do you have any idea what checks to apply to figure out whether or not
the SPI clock will be necessary?

Perhaps we can add a check into acpi_create_platform_device()?  Something like
"if the device ID is present in table X, then run function Y for the device"
where Y will configure the clock if the SPI is matched?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Wed, Jan 09, 2013 at 11:07:26PM +0100, Rafael J. Wysocki wrote:
> We hardcode different kinds of information into the kernel anyway, but it
> would be good to be able to use some criteria to decide whether or not we need
> that information on the given system.
> 
> In this particular case, we'll only need the fixed clock rate for the SPI
> which is not present on other x86 systems to date.
> 
> Mika, do you have any idea what checks to apply to figure out whether or not
> the SPI clock will be necessary?

I've tried to figure out them but so far I have no ideas :-/

If we put that behind some config option, it 1) confuses users and 2)
distro makers will select it anyway, so I don't see this as an option.

> Perhaps we can add a check into acpi_create_platform_device()?  Something like
> "if the device ID is present in table X, then run function Y for the device"
> where Y will configure the clock if the SPI is matched?

That would work but then we must also make sure that if the device was
enumerated from PCI, it gets this clock via some other means.

The LPSS devices can be configured to either appear on PCI or ACPI and of
course ACPI has the advantage because then we can enumerate the slave
devices behind the bus as well. However, it might be possible that we need
to support PCI enumeration at some point.

Furthermore, the SPI controller has an existing PCI glue driver
(spi-pxa2xx-pci.c) that needs to pass this clock as well. This is for Intel
CE4100 platform and we must make sure that it still works after our change.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Wed, Jan 09, 2013 at 10:52:12PM +0100, Rafael J. Wysocki wrote:
> 
> > Other problem is that this setup needs manual maintenance as we can't get
> > the configuration from ACPI.
> 
> So we just happen to know what the rate is supposed to be?  What's the source
> of that information?

We get it from the LPSS specs. On lynxpoint the LPSS clock is 100MHz.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Thu, Jan 10, 2013 at 11:58:03AM +0200, Mika Westerberg wrote:
> On Wed, Jan 09, 2013 at 11:07:26PM +0100, Rafael J. Wysocki wrote:
> > We hardcode different kinds of information into the kernel anyway, but it
> > would be good to be able to use some criteria to decide whether or not we need
> > that information on the given system.
> > 
> > In this particular case, we'll only need the fixed clock rate for the SPI
> > which is not present on other x86 systems to date.
> > 
> > Mika, do you have any idea what checks to apply to figure out whether or not
> > the SPI clock will be necessary?
> 
> I've tried to figure out them but so far I have no ideas :-/
> 
> If we put that behind some config option, it 1) confuses users and 2)
> distro makers will select it anyway, so I don't see this as an option.
> 
> > Perhaps we can add a check into acpi_create_platform_device()?  Something like
> > "if the device ID is present in table X, then run function Y for the device"
> > where Y will configure the clock if the SPI is matched?
> 
> That would work but then we must also make sure that if the device was
> enumerated from PCI, it gets this clock via some other means.
> 
> The LPSS devices can be configured to either appear on PCI or ACPI and of
> course ACPI has the advantage because then we can enumerate the slave
> devices behind the bus as well. However, it might be possible that we need
> to support PCI enumeration at some point.
> 
> Furthermore, the SPI controller has an existing PCI glue driver
> (spi-pxa2xx-pci.c) that needs to pass this clock as well. This is for Intel
> CE4100 platform and we must make sure that it still works after our change.

Here's one more proposal based on the discussion so far.

	1. Enable CONFIG_COMMON_CLK on x86.

	2. Create Lynxpoint specific clock driver and place it under
	   drivers/clk/x86/clk-lpt.c. This is a platform driver for
	   platform device named 'clk-lpt'.

	3. We make the acpi_create_platform_device() match on, lets say
	   "INT33C" (a partial match), and in such case it assumes that we are
           running on Lynxpoint. It will then create platform device for 'clk-lpt'.

	4. Now the clk-lpt driver creates the clocks.

	5. The SPI driver gets the clock it wants.

In case of CE4100 we make drivers/clk/x86/clk-ce4100.c that exposes
ce4100_clk_init(). This gets called from arch/x86/platform/ce4100/ce4100.c.

I'm not sure about the LPSS PCI case. Maybe we can forget that for now
(except the CE4100 case which obviously must work)? Or add similar quirk to
the PCI side that creates the platform device for clocks.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:

> > 	3. We make the acpi_create_platform_device() match on, lets say
> > 	   "INT33C" (a partial match), and in such case it assumes that we are
> >            running on Lynxpoint. It will then create platform device for 'clk-lpt'.

> > 	4. Now the clk-lpt driver creates the clocks.

> > 	5. The SPI driver gets the clock it wants.

> That sounds reasonable to me.  Mark, what do you think?

Sounds sensible, yes - about what I'd expect.  Is it possible to match
on CPUID or similar information (given that this is all in the SoC)
instead of ACPI, that might be more robust I guess?

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:
> On Thu, Jan 10, 2013 at 11:58:03AM +0200, Mika Westerberg wrote:
> > On Wed, Jan 09, 2013 at 11:07:26PM +0100, Rafael J. Wysocki wrote:
> > > We hardcode different kinds of information into the kernel anyway, but it
> > > would be good to be able to use some criteria to decide whether or not we need
> > > that information on the given system.
> > > 
> > > In this particular case, we'll only need the fixed clock rate for the SPI
> > > which is not present on other x86 systems to date.
> > > 
> > > Mika, do you have any idea what checks to apply to figure out whether or not
> > > the SPI clock will be necessary?
> > 
> > I've tried to figure out them but so far I have no ideas :-/
> > 
> > If we put that behind some config option, it 1) confuses users and 2)
> > distro makers will select it anyway, so I don't see this as an option.
> > 
> > > Perhaps we can add a check into acpi_create_platform_device()?  Something like
> > > "if the device ID is present in table X, then run function Y for the device"
> > > where Y will configure the clock if the SPI is matched?
> > 
> > That would work but then we must also make sure that if the device was
> > enumerated from PCI, it gets this clock via some other means.
> > 
> > The LPSS devices can be configured to either appear on PCI or ACPI and of
> > course ACPI has the advantage because then we can enumerate the slave
> > devices behind the bus as well. However, it might be possible that we need
> > to support PCI enumeration at some point.
> > 
> > Furthermore, the SPI controller has an existing PCI glue driver
> > (spi-pxa2xx-pci.c) that needs to pass this clock as well. This is for Intel
> > CE4100 platform and we must make sure that it still works after our change.
> 
> Here's one more proposal based on the discussion so far.
> 
> 	1. Enable CONFIG_COMMON_CLK on x86.
> 
> 	2. Create Lynxpoint specific clock driver and place it under
> 	   drivers/clk/x86/clk-lpt.c. This is a platform driver for
> 	   platform device named 'clk-lpt'.
> 
> 	3. We make the acpi_create_platform_device() match on, lets say
> 	   "INT33C" (a partial match), and in such case it assumes that we are
>            running on Lynxpoint. It will then create platform device for 'clk-lpt'.
> 
> 	4. Now the clk-lpt driver creates the clocks.
> 
> 	5. The SPI driver gets the clock it wants.

That sounds reasonable to me.  Mark, what do you think?

> In case of CE4100 we make drivers/clk/x86/clk-ce4100.c that exposes
> ce4100_clk_init(). This gets called from arch/x86/platform/ce4100/ce4100.c.
> 
> I'm not sure about the LPSS PCI case. Maybe we can forget that for now
> (except the CE4100 case which obviously must work)? Or add similar quirk to
> the PCI side that creates the platform device for clocks.

I would wait for now until we have evidence that the LPSS PCI will be used in
practice at all.  Then we can add a PCI quirk along the above lines, I think.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Thu, Jan 10, 2013 at 12:51:59PM +0000, Mark Brown wrote:
> On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:
> 
> > > 	3. We make the acpi_create_platform_device() match on, lets say
> > > 	   "INT33C" (a partial match), and in such case it assumes that we are
> > >            running on Lynxpoint. It will then create platform device for 'clk-lpt'.
> 
> > > 	4. Now the clk-lpt driver creates the clocks.
> 
> > > 	5. The SPI driver gets the clock it wants.
> 
> > That sounds reasonable to me.  Mark, what do you think?
> 
> Sounds sensible, yes - about what I'd expect.  Is it possible to match
> on CPUID or similar information (given that this is all in the SoC)
> instead of ACPI, that might be more robust I guess?

I can look into that but I'm not sure whether there are any other way to
detect are we running on Lynxpoint or not, except the device IDs (and even
that is not 100% guaranteed because of ACPI _CIDs).

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> > 
> > I'm not sure about the LPSS PCI case. Maybe we can forget that for now
> > (except the CE4100 case which obviously must work)? Or add similar quirk to
> > the PCI side that creates the platform device for clocks.
> 
> I would wait for now until we have evidence that the LPSS PCI will be used in
> practice at all.  Then we can add a PCI quirk along the above lines, I think.

Sounds good to me. Thanks!

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Thursday, January 10, 2013 12:51:59 PM Mark Brown wrote:
> On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:
> 
> > > 	3. We make the acpi_create_platform_device() match on, lets say
> > > 	   "INT33C" (a partial match), and in such case it assumes that we are
> > >            running on Lynxpoint. It will then create platform device for 'clk-lpt'.
> 
> > > 	4. Now the clk-lpt driver creates the clocks.
> 
> > > 	5. The SPI driver gets the clock it wants.
> 
> > That sounds reasonable to me.  Mark, what do you think?
> 
> Sounds sensible, yes - about what I'd expect.  Is it possible to match
> on CPUID or similar information (given that this is all in the SoC)
> instead of ACPI, that might be more robust I guess?

This particular part may be present in different SoCs.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Thursday, January 10, 2013 03:07:40 PM Mika Westerberg wrote:
> On Thu, Jan 10, 2013 at 12:51:59PM +0000, Mark Brown wrote:
> > On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:
> > 
> > > > 	3. We make the acpi_create_platform_device() match on, lets say
> > > > 	   "INT33C" (a partial match), and in such case it assumes that we are
> > > >            running on Lynxpoint. It will then create platform device for 'clk-lpt'.
> > 
> > > > 	4. Now the clk-lpt driver creates the clocks.
> > 
> > > > 	5. The SPI driver gets the clock it wants.
> > 
> > > That sounds reasonable to me.  Mark, what do you think?
> > 
> > Sounds sensible, yes - about what I'd expect.  Is it possible to match
> > on CPUID or similar information (given that this is all in the SoC)
> > instead of ACPI, that might be more robust I guess?
> 
> I can look into that but I'm not sure whether there are any other way to
> detect are we running on Lynxpoint or not, except the device IDs (and even
> that is not 100% guaranteed because of ACPI _CIDs).

Well, we only need the clock when the SPI controller is going to be used,
so even if we have a reliable way to detect Lynxpoint, that may be not enough
(the BIOS may not expose the SPI to us, for example, in which case it will be
pointless to create the clock for it).

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Thu, Jan 10, 2013 at 02:18:08PM +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 12:51:59 PM Mark Brown wrote:

> > Sounds sensible, yes - about what I'd expect.  Is it possible to match
> > on CPUID or similar information (given that this is all in the SoC)
> > instead of ACPI, that might be more robust I guess?

> This particular part may be present in different SoCs.

Right, but I'd expect you could enumerate the SoCs?  Someone might
decide to change the clock configuration for future SoCs anyway.

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

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Thu, Jan 10, 2013 at 02:23:25PM +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 03:07:40 PM Mika Westerberg wrote:
> > On Thu, Jan 10, 2013 at 12:51:59PM +0000, Mark Brown wrote:
> > > On Thu, Jan 10, 2013 at 01:54:41PM +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, January 10, 2013 02:38:37 PM Mika Westerberg wrote:
> > > 
> > > > > 	3. We make the acpi_create_platform_device() match on, lets say
> > > > > 	   "INT33C" (a partial match), and in such case it assumes that we are
> > > > >            running on Lynxpoint. It will then create platform device for 'clk-lpt'.
> > > 
> > > > > 	4. Now the clk-lpt driver creates the clocks.
> > > 
> > > > > 	5. The SPI driver gets the clock it wants.
> > > 
> > > > That sounds reasonable to me.  Mark, what do you think?
> > > 
> > > Sounds sensible, yes - about what I'd expect.  Is it possible to match
> > > on CPUID or similar information (given that this is all in the SoC)
> > > instead of ACPI, that might be more robust I guess?
> > 
> > I can look into that but I'm not sure whether there are any other way to
> > detect are we running on Lynxpoint or not, except the device IDs (and even
> > that is not 100% guaranteed because of ACPI _CIDs).
> 
> Well, we only need the clock when the SPI controller is going to be used,
> so even if we have a reliable way to detect Lynxpoint, that may be not enough
> (the BIOS may not expose the SPI to us, for example, in which case it will be
> pointless to create the clock for it).

Good point. I'll do the checking in acpi_create_platform_device() based on
ACPI IDs so that we can be sure that the SPI controller is really there.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mika Westerberg]

On Thu, Jan 10, 2013 at 01:33:19PM +0000, Mark Brown wrote:
> On Thu, Jan 10, 2013 at 02:18:08PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 10, 2013 12:51:59 PM Mark Brown wrote:
> 
> > > Sounds sensible, yes - about what I'd expect.  Is it possible to match
> > > on CPUID or similar information (given that this is all in the SoC)
> > > instead of ACPI, that might be more robust I guess?
> 
> > This particular part may be present in different SoCs.
> 
> Right, but I'd expect you could enumerate the SoCs?  Someone might
> decide to change the clock configuration for future SoCs anyway.

Well, they can use the same LPSS block with a different CPU but then we
expect the ACPI IDs to change as well (so we can then make another set of
clocks for those).

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Rafael J. Wysocki]

On Thursday, January 10, 2013 03:58:52 PM Mika Westerberg wrote:
> On Thu, Jan 10, 2013 at 01:33:19PM +0000, Mark Brown wrote:
> > On Thu, Jan 10, 2013 at 02:18:08PM +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 12:51:59 PM Mark Brown wrote:
> > 
> > > > Sounds sensible, yes - about what I'd expect.  Is it possible to match
> > > > on CPUID or similar information (given that this is all in the SoC)
> > > > instead of ACPI, that might be more robust I guess?
> > 
> > > This particular part may be present in different SoCs.
> > 
> > Right, but I'd expect you could enumerate the SoCs?  Someone might
> > decide to change the clock configuration for future SoCs anyway.
> 
> Well, they can use the same LPSS block with a different CPU but then we
> expect the ACPI IDs to change as well (so we can then make another set of
> clocks for those).

Also, as I said in another message, even if the LPSS block is used, the SPI
may not be exposed to us by the BIOS, so SoC enumeration is not sufficient
in general.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 05/11] spi/pxa2xx: make clock rate configurable from platform data

[Mark Brown]

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

On Thu, Jan 10, 2013 at 10:56:36PM +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 03:58:52 PM Mika Westerberg wrote:

> > Well, they can use the same LPSS block with a different CPU but then we
> > expect the ACPI IDs to change as well (so we can then make another set of
> > clocks for those).

> Also, as I said in another message, even if the LPSS block is used, the SPI
> may not be exposed to us by the BIOS, so SoC enumeration is not sufficient
> in general.

The clock is going to be there in the SoC - given that you're just
exposing a fixed rate clock it's not going to do anything except use a
little memory if its' there.

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

Re: [PATCH 02/11] spi/pxa2xx: convert to the pump message infrastructure

[Linus Walleij]

On Mon, Jan 7, 2013 at 11:44 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> The SPI core provides infrastructure for standard message queueing so use
> that instead of handling everything in the driver. This simplifies the
> driver.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Wohoo!
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 06/11] spi/pxa2xx: use the private DMA API only when CONFIG_ARCH_PXA is set

[Linus Walleij]

On Mon, Jan 7, 2013 at 11:44 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> The PXA SPI driver uses PXA platform specific private DMA implementation
> which does not work on non-PXA platforms. In order to use this driver on
> other platforms we need to move the private DMA implementation into a
> separate functions that get stubbed out when !CONFIG_ARCH_PXA.
>
> While we are there we can kill the dummy DMA bits in pxa2xx_spi.h as they
> are not needed anymore for CE4100.
>
> Once this is done we can add the generic DMA engine support to the driver
> that allows usage of any DMA controller that implements DMA engine API.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/spi/spi-pxa2xx.c       |  612 +++++++++++++++++++++++-----------------
>  include/linux/spi/pxa2xx_spi.h |   80 ------

Can you even break this out to its own file?

Like drivers/spi/spi-pxa2xx-pxadma.c/.h
with stubs in the header file or something so we need
no #ifdefs in the main driver file?

The kernel looks better after this patch anyway, so
Acked-by: Linus Walleij <linus.walleij@linaro.org>
in any case.

Yours,
Linus Walleij

Re: [PATCH 07/11] spi/pxa2xx: add support for DMA engine

[Linus Walleij]

On Mon, Jan 7, 2013 at 11:44 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> In order to use DMA with this driver on non-PXA platforms we implement support
> for the generic DMA engine API. This allows to use different DMA engines with
> little or no modification to the driver.
>
> Request lines and channel numbers can be passed to the driver from the
> platform specific data.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/spi/spi-pxa2xx.c       |  351 +++++++++++++++++++++++++++++++++++++++-
>  include/linux/spi/pxa2xx_spi.h |    6 +

So if the other DMA implementation could go into spi-pxa2xx-pxadma.c
this could be spi-pxa2xx-dma.c and they could use the common header
file spi-pxa2xx-dma.h and then Kconfig select which one to compile.

Just a suggestion to get the driver more readable.

> +       /*
> +        * Some DMA controllers have problems transferring buffers that are
> +        * not multiple of 4 bytes. So we truncate the transfer so that it
> +        * is suitable for such controllers, and handle the trailing bytes
> +        * manually after the DMA completes.
> +        */
> +       len = ALIGN(drv_data->len, 4);

This is actually a property of the DMA controller.

struct dma_device already has this:

 * @copy_align: alignment shift for memcpy operations
 * @xor_align: alignment shift for xor operations
 * @pq_align: alignment shift for pq operations
 * @fill_align: alignment shift for memset operations
(...)
        u8 copy_align;
        u8 xor_align;
        u8 pq_align;
        u8 fill_align;

To align memcpy's on 4 bytes you can e.g. set .copy_align
to 2.

So the syntax is number of shifts 1 << 2 = 4.

If slave transfers can expose the same type of property we should
maybe introdyce .slave_align in the same manner so you don't have
to assume the worst in the driver.

Vinod/Dan? What do you say about this?

Yours,
Linus Walleij

Re: [PATCH 06/11] spi/pxa2xx: use the private DMA API only when CONFIG_ARCH_PXA is set

[Mika Westerberg]

On Thu, Jan 17, 2013 at 10:36:19AM +0100, Linus Walleij wrote:
> On Mon, Jan 7, 2013 at 11:44 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > The PXA SPI driver uses PXA platform specific private DMA implementation
> > which does not work on non-PXA platforms. In order to use this driver on
> > other platforms we need to move the private DMA implementation into a
> > separate functions that get stubbed out when !CONFIG_ARCH_PXA.
> >
> > While we are there we can kill the dummy DMA bits in pxa2xx_spi.h as they
> > are not needed anymore for CE4100.
> >
> > Once this is done we can add the generic DMA engine support to the driver
> > that allows usage of any DMA controller that implements DMA engine API.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/spi/spi-pxa2xx.c       |  612 +++++++++++++++++++++++-----------------
> >  include/linux/spi/pxa2xx_spi.h |   80 ------
> 
> Can you even break this out to its own file?
> 
> Like drivers/spi/spi-pxa2xx-pxadma.c/.h
> with stubs in the header file or something so we need
> no #ifdefs in the main driver file?

Good point. I need to refresh the series anyway because of the x86 common
clock changes (which I'm hoping the x86 maintainers accept), so I can do
the file split as well.

> The kernel looks better after this patch anyway, so
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> in any case.

Thanks!

Re: [PATCH 07/11] spi/pxa2xx: add support for DMA engine

[Mika Westerberg]

On Thu, Jan 17, 2013 at 10:48:15AM +0100, Linus Walleij wrote:
> On Mon, Jan 7, 2013 at 11:44 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > In order to use DMA with this driver on non-PXA platforms we implement support
> > for the generic DMA engine API. This allows to use different DMA engines with
> > little or no modification to the driver.
> >
> > Request lines and channel numbers can be passed to the driver from the
> > platform specific data.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/spi/spi-pxa2xx.c       |  351 +++++++++++++++++++++++++++++++++++++++-
> >  include/linux/spi/pxa2xx_spi.h |    6 +
> 
> So if the other DMA implementation could go into spi-pxa2xx-pxadma.c
> this could be spi-pxa2xx-dma.c and they could use the common header
> file spi-pxa2xx-dma.h and then Kconfig select which one to compile.
> 
> Just a suggestion to get the driver more readable.

Yes, it makes perfect sense. I'll do that in the next version.

> > +       /*
> > +        * Some DMA controllers have problems transferring buffers that are
> > +        * not multiple of 4 bytes. So we truncate the transfer so that it
> > +        * is suitable for such controllers, and handle the trailing bytes
> > +        * manually after the DMA completes.
> > +        */
> > +       len = ALIGN(drv_data->len, 4);
> 
> This is actually a property of the DMA controller.

Right and this is not the proper place for this but I wasn't sure where to
put this information...

> struct dma_device already has this:
> 
>  * @copy_align: alignment shift for memcpy operations
>  * @xor_align: alignment shift for xor operations
>  * @pq_align: alignment shift for pq operations
>  * @fill_align: alignment shift for memset operations
> (...)
>         u8 copy_align;
>         u8 xor_align;
>         u8 pq_align;
>         u8 fill_align;
> 
> To align memcpy's on 4 bytes you can e.g. set .copy_align
> to 2.
> 
> So the syntax is number of shifts 1 << 2 = 4.
> 
> If slave transfers can expose the same type of property we should
> maybe introdyce .slave_align in the same manner so you don't have
> to assume the worst in the driver.

... so this sounds good to me.