dw_spi: add DMA support

PATCH In review Alan Cox 2 participants 3 messages Oct 22, 2010 – Oct 24, 2010 1 message not in the archive SPI

lore

Share & tools

lore

Alan Cox

author

2010-10-22 19:11 UTC #0 lore raw

PATCH 1/1 dw_spi: add DMA support

From: Feng Tang <[email protected]>

dw_spi driver in upstream only supports PIO mode, and this patch will support it to cowork with the Designware DMA controller used on Intel Moorestown platform

It has been tested with a Option GTM501L 3G modem, to use DMA mode, DMA controller 2 of Moorestown has to be enabled

Signed-off-by Feng Tang <[email protected]>

[Typo fix and renames to match intel_mid_dma renaming]

Signed-off-by Vinod Koul <[email protected]>

[Clean up]

Signed-off-by Feng Tang <[email protected]>

[Fix timing delay, add cpu_relax]

Signed-off-by Arjan van de Ven <arjan-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Alan Cox <alan-VuQAYsv1563Yd54FQh9/[email protected]>

---

 drivers/spi/Kconfig        |    4 +
 drivers/spi/Makefile       |    3 -
 drivers/spi/dw_spi.c       |   48 +++++----
 drivers/spi/dw_spi_mid.c   |  243 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/dw_spi_pci.c   |   14 ++-
 include/linux/spi/dw_spi.h |   15 +++
 6 files changed, 302 insertions(+), 25 deletions(-)
 create mode 100644 drivers/spi/dw_spi_mid.c

drivers/spi/Kconfig +4 -0

index 82e45cf..6e6a638 100644
@@ -404,6 +404,10 @@ config SPI_DW_PCI
 	tristate "PCI interface driver for DW SPI core"
 	depends on SPI_DESIGNWARE && PCI
 
+config SPI_DW_MID_DMA
+	bool "DMA support for DW SPI controller on Intel Moorestown platform"
+	depends on SPI_DW_PCI && INTEL_MID_DMAC
+
 config SPI_DW_MMIO
 	tristate "Memory-mapped io interface driver for DW SPI core"
 	depends on SPI_DESIGNWARE && HAVE_CLK

drivers/spi/Makefile +2 -1

index b427c2e..4949370 100644
@@ -17,7 +17,8 @@ obj-$(CONFIG_SPI_BUTTERFLY)		+= spi_butterfly.o
 obj-$(CONFIG_SPI_COLDFIRE_QSPI)		+= coldfire_qspi.o
 obj-$(CONFIG_SPI_DAVINCI)		+= davinci_spi.o
 obj-$(CONFIG_SPI_DESIGNWARE)		+= dw_spi.o
-obj-$(CONFIG_SPI_DW_PCI)		+= dw_spi_pci.o
+obj-$(CONFIG_SPI_DW_PCI)		+= dw_spi_midpci.o
+dw_spi_midpci-objs			:= dw_spi_pci.o dw_spi_mid.o
 obj-$(CONFIG_SPI_DW_MMIO)		+= dw_spi_mmio.o
 obj-$(CONFIG_SPI_EP93XX)		+= ep93xx_spi.o
 obj-$(CONFIG_SPI_GPIO)			+= spi_gpio.o

drivers/spi/dw_spi.c +29 ~~-19~~

146 lines not on this page · raw message

drivers/spi/dw_spi_mid.c new file +243 -0

243 lines not on this page · raw message

drivers/spi/dw_spi_pci.c +9 -5

index 1f52755..076d1f8 100644
@@ -1,5 +1,5 @@
 /*
- * mrst_spi_pci.c - PCI interface driver for DW SPI Core
+ * dw_spi_pci.c - PCI interface driver for DW SPI Core
  *
  * Copyright (c) 2009, Intel Corporation.
  *

@@ -26,8 +26,8 @@
 #define DRIVER_NAME "dw_spi_pci"
 
 struct dw_spi_pci {
-	struct pci_dev		*pdev;
-	struct dw_spi		dws;
+	struct pci_dev	*pdev;
+	struct dw_spi	dws;
 };
 
 static int __devinit spi_pci_probe(struct pci_dev *pdev,

@@ -72,9 +72,13 @@ static int __devinit spi_pci_probe(struct pci_dev *pdev,
 	dws->parent_dev = &pdev->dev;
 	dws->bus_num = 0;
 	dws->num_cs = 4;
-	dws->max_freq = 25000000;	/* for Moorestwon */
 	dws->irq = pdev->irq;
-	dws->fifo_len = 40;		/* FIFO has 40 words buffer */
+
+	if (pdev->device == 0x0800) {
+		ret = dw_spi_mid_init(dws);
+		if (ret)
+			goto err_unmap;
+	}
 
 	ret = dw_spi_add_host(dws);
 	if (ret)

include/linux/spi/dw_spi.h +15 -0

index c91302f..b2ecf9f 100644
@@ -1,5 +1,6 @@
 #ifndef DW_SPI_HEADER_H
 #define DW_SPI_HEADER_H
+
 #include <linux/io.h>
 
 /* Bit fields in CTRLR0 */

@@ -82,6 +83,13 @@ struct dw_spi_reg {
 				though only low 16 bits matters */
 } __packed;
 
+struct dw_spi;
+struct dw_spi_dma_ops {
+	int (*dma_init)(struct dw_spi *dws);
+	void (*dma_exit)(struct dw_spi *dws);
+	int (*dma_transfer)(struct dw_spi *dws, int cs_change);
+};
+
 struct dw_spi {
 	struct spi_master	*master;
 	struct spi_device	*cur_dev;

@@ -143,6 +151,9 @@ struct dw_spi {
 	u64			rx_param;
 	struct device		*dma_dev;
 	dma_addr_t		dma_addr;
+	struct dw_spi_dma_ops	*dma_ops;
+	void			*dma_priv; /* platform relate info */
+	struct pci_dev		*dmac;
 
 	/* Bus interface info */
 	void			*priv;

@@ -216,4 +227,8 @@ extern int dw_spi_add_host(struct dw_spi *dws);
 extern void dw_spi_remove_host(struct dw_spi *dws);
 extern int dw_spi_suspend_host(struct dw_spi *dws);
 extern int dw_spi_resume_host(struct dw_spi *dws);
+extern void dw_spi_xfer_done(struct dw_spi *dws);
+
+/* platform related setup */
+extern int dw_spi_mid_init(struct dw_spi *dws); /* Intel MID platforms */
 #endif /* DW_SPI_HEADER_H */

------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest Create new apps & games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev

A message that is not in the archive (sent off-list or not yet received): 20101022191049.8830.51496.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

Linus Walleij

2010-10-23 16:10 UTC #2 lore raw

2010/10/22 Alan Cox <alan-qBU/[email protected]>:

Quoted 13 lines

(...)

drivers/spi/dw_spi_mid.c new file +12 -0

+static int mid_spi_dma_init(struct dw_spi *dws)
+{
+       struct mid_dma *dw_dma = dws->dma_priv;
+       struct intel_mid_dma_slave *rxs, *txs;
+       dma_cap_mask_t mask;
+
+       dws->txchan = NULL;
+       dws->rxchan = NULL;
+
+       /*get pci device for DMA*/
+       dws->dmac = pci_get_device(PCI_VENDOR_ID_INTEL, 0x813, NULL);
+

drivers/spi/dw_spi_mid.c line 71

Quoted 10 lines

drivers/spi/dw_spi_mid.c new file +10 -0

+       /* 1. Init rx channel */
+       rxs = &dw_dma->dmas_rx;
+
+       rxs->dirn = DMA_FROM_DEVICE;
+       rxs->hs_mode = LNW_DMA_HW_HS;
+       rxs->cfg_mode = LNW_DMA_PER_TO_MEM;
+       rxs->src_width = LNW_DMA_WIDTH_16BIT;
+       rxs->dst_width = LNW_DMA_WIDTH_32BIT;
+       rxs->src_msize = LNW_DMA_MSIZE_16;
+       rxs->dst_msize = LNW_DMA_MSIZE_16;

Hm, hm hm. You go configure these channel characteristics by derferencing and writing private data.

There *is* a generic runtime channel control interface as of kernel 2.6.36. Compare to this snippet from the PL022 DMA support that was merged into becoming 2.6.37 a day ago or so (behold the beauty in drivers/spi/amba-pl022.c):

        struct dma_slave_config rx_conf = {
                .src_addr = SSP_DR(pl022->phybase),
                .direction = DMA_FROM_DEVICE,
                .src_maxburst = pl022->vendor->fifodepth >> 1,
        };
       (...)
        struct dma_chan *rxchan = pl022->dma_rx_channel;

       (...)

        switch (pl022->read) {
        case READING_NULL:
                /* Use the same as for writing */
                rx_conf.src_addr_width = DMA_SLAVE_BUSWIDTH_UNDEFINED;
                break;
        case READING_U8:
                rx_conf.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
                break;
        case READING_U16:
                rx_conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
                break;
        case READING_U32:
                rx_conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
                break;
        }
        rxchan->device->device_control(rxchan, DMA_SLAVE_CONFIG,
                                       (unsigned long) &rx_conf);

I don't know if I'm particularly biased by being infatuated by my own interface, but can't you just implement the DMA_SLAVE_CONFIG command for the DW DMA controller instead of this
custom stuff?

Quoted 3 lines

+ dma_cap_zero(mask);
+ dma_cap_set(DMA_MEMCPY, mask);
+ dma_cap_set(DMA_SLAVE, mask);

What are you doing? Normally you want *either* a memcpy channel (which wouldn't require the kind of setup you do above, hell memcpy shan't be any different from the C library function memcpy()!) *or* you want a slave channel.

Most probably you should drop dma_cap_set(DMA_MEMCPY, mask), if it is needed your DMAengine driver is likely weird.

Then I think I get the answer later on:

Quoted 1 line

+ dws->rxchan = dma_request_channel(mask, mid_spi_dma_chan_filter, dws);

Aha generic DMA engine, that's cool! :)

So then again, use the DMA-engine configuration mechanism, and get the slave channel will ya?

Quoted 8 lines

(...)
+static int mid_spi_dma_transfer(struct dw_spi *dws, int cs_change) (...)
+ /* 3. start the RX dma transfer */
+ if (dws->rx_dma) {
+ rxdesc = rxchan->device->device_prep_dma_memcpy(rxchan,
+ dws->rx_dma, dws->dma_addr,
+ dws->len, flag);

So you're still using the memcpy() prepare function in order to be able to supply the endpoint address. Incidentally, that is an additional argument that you can pass to the generic channel control mechanism using .src_addr/.dst_addr respectively.

And the DW controller does *indeed* have a
device_prep_slave_sg hook.

My recommendation: put generic channel control into the DMA driver you're using and inspect amba-pl022.c for an idea on how to do this.

Since the .device_prep_slave_sg command only takes SGlists you will have to wrap your buffers in SGlists but I'm working on a simplifying wrapper for that.

Yours,
Linus Walleij

Alan Cox

author

2010-10-24 08:47 UTC #3 lore raw

Quoted 2 lines

There *is* a generic runtime channel control interface as of kernel 2.6.36. Compare to this snippet from the PL022 DMA

Not when this driver was done.

Quoted 2 lines

support that was merged into becoming 2.6.37 a day ago or so (behold the beauty in drivers/spi/amba-pl022.c):

Funnily enough I've already moved to using the proper fields in the next version of all the DMA using patches so that bit is cool, but I've not done the other stuff you then point out so I'll go through your example and tidy up.