2005-12-12 15:20:03

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH 2.6-git 0/4] SPI core refresh

Greetings,

this message fill be followed by the following four ones:
1) updated SPI core from Dmitry Pervushin/Vitaly Wool
2) Atmel MTD dataflash driver port for this core
3) SPI controller driver for Philips SPI controller
4) dumb EEPROM driver for EEPROM chip on SPI bus

This SPI core features:
* multiple SPI controller support
* multiple devices on the same bus support
* DMA support
* synchronous and asynchronous transfers
* library for asynchronous transfers on the bus using kernel threads
* character device interface
* custom lightweight SPI message allocation mechanism

The SPI core is now closer to the other one, David Brownell's. For instance, spi_driver structure is now the same, and some API functions are mostly quite similar. Even the footprint is now similar (~2k .text on ARM) :)

However, there are some differences I'd like to summarize:
* this core doesn't support hotplug, and David's does (we'll post this support later in a separate patch)
* our core is transparent for DMA transfers (allocation of DMA-safe buffer is triggered by the message flag set by the caller) and David's one implies limitations on tx/rx buffers passed (which can be easily resolved in the controller driver though)
* our core implements thread-based asynchronous message handling (as an option!) which seems to be the best way to go for most cases. i. e. PXA SSP driver from Stephen would be a lot more lightweight and will gain more performance if it used this method instead of multiple tasklets scheduling (as of now)
* our core hides the SPI message internals and David's exposes them to the outer world. This is in way more robust since the only way to allocate the message is through the API
* our core uses uniform message structure for all transfers and "chains" such messages, and David's uses separate spi_transfer array which hold the chain and is attached to the message structure. It's arguable which one is better; at least ours allows to easily change the message inetrnals w/o affecting actual drivers and use versatile message allocation techniques
* our core allows to omit callbacks for sync transfers which is a reasonable thing IMO (if it's a sync transfer, then the caller will just wait, so having a default callback that just waits-for-completion is rational)
* our core provides more straightfoward means (leading to improved readability of the actual drivers) to express the same things, i. e instead of
> message->status = -EIO;
> if (message->complete) {
> message->complete(message->context);
> }
one is only to do
> spimsg_complete(message, -EIO);
or, instead of
> if (!message->transfers[state->index].cs_change)
> state->cs_control(PXA2XX_CS_DEASSERT);
one is to do
> if (spimsg_get_flags(cur_msg) & SPI_M_CSKEEP)
> state->cs_control(PXA2XX_CS_DEASSERT);
* other minor differences... maybe David's gonna add some here.

Vitaly


2005-12-12 15:23:37

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH 2.6-git 2/4] SPI core refresh: MTD dataflash driver


Signed-off-by: Dmitry Pervushin <[email protected]>
Signed-off-by: Vitaly Wool <[email protected]>

drivers/mtd/devices/Kconfig | 8
drivers/mtd/devices/Makefile | 1
drivers/mtd/devices/mtd_dataflash.c | 601 ++++++++++++++++++++++++++++++++++++
include/linux/spi/flash.h | 27 +
4 files changed, 637 insertions(+)

Index: linux-2.6.orig/drivers/mtd/devices/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/mtd/devices/Kconfig
+++ linux-2.6.orig/drivers/mtd/devices/Kconfig
@@ -255,5 +255,13 @@ config MTD_DOCPROBE_55AA
LinuxBIOS or if you need to recover a DiskOnChip Millennium on which
you have managed to wipe the first block.

+config MTD_DATAFLASH
+ tristate "Support for AT45xxx DataFlash"
+ depends on MTD && SPI && EXPERIMENTAL
+ help
+ This enables access to AT45xxx DataFlash chips, using SPI.
+ Sometimes DataFlash chips are packaged inside MMC-format
+ cards; at this writing, the MMC stack won't handle those.
+
endmenu

Index: linux-2.6.orig/drivers/mtd/devices/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/mtd/devices/Makefile
+++ linux-2.6.orig/drivers/mtd/devices/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_MTD_MTDRAM) += mtdram.o
obj-$(CONFIG_MTD_LART) += lart.o
obj-$(CONFIG_MTD_BLKMTD) += blkmtd.o
obj-$(CONFIG_MTD_BLOCK2MTD) += block2mtd.o
+obj-$(CONFIG_MTD_DATAFLASH) += mtd_dataflash.o
Index: linux-2.6.orig/drivers/mtd/devices/mtd_dataflash.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/mtd/devices/mtd_dataflash.c
@@ -0,0 +1,601 @@
+/*
+ * Atmel AT45xxx DataFlash MTD driver for lightweight SPI framework
+ *
+ * Largely derived from at91_dataflash.c:
+ * Copyright (C) 2003-2005 SAN People (Pty) Ltd
+ *
+ * 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 the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+*/
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/spi.h>
+#include <linux/spi/flash.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+
+
+/*
+ * DataFlash is a kind of SPI flash. Most AT45 chips have two buffers in
+ * each chip, which may be used for double buffered I/O; but this driver
+ * doesn't (yet) use these for any kind of i/o overlap or prefetching.
+ *
+ * Sometimes DataFlash is packaged in MMC-format cards. This driver does
+ * not (yet) expect to be hotplugged through MMC or SPI. That's fair,
+ * since the MMC stack can't even use SPI (yet), much less distinguish
+ * between MMC and DataFlash protocols during enumeration.
+ */
+
+#define CONFIG_DATAFLASH_WRITE_VERIFY
+
+
+/* reads can bypass the buffers */
+#define OP_READ_CONTINUOUS 0xE8
+#define OP_READ_PAGE 0xD2
+
+/* group B requests can run even while status reports "busy" */
+#define OP_READ_STATUS 0xD7 /* group B */
+
+/* move data between host and buffer */
+#define OP_READ_BUFFER1 0xD4 /* group B */
+#define OP_READ_BUFFER2 0xD6 /* group B */
+#define OP_WRITE_BUFFER1 0x84 /* group B */
+#define OP_WRITE_BUFFER2 0x87 /* group B */
+
+/* erasing flash */
+#define OP_ERASE_PAGE 0x81
+#define OP_ERASE_BLOCK 0x50
+
+/* move data between buffer and flash */
+#define OP_TRANSFER_BUF1 0x53
+#define OP_TRANSFER_BUF2 0x55
+#define OP_MREAD_BUFFER1 0xD4
+#define OP_MREAD_BUFFER2 0xD6
+#define OP_MWERASE_BUFFER1 0x83
+#define OP_MWERASE_BUFFER2 0x86
+#define OP_MWRITE_BUFFER1 0x88 /* sector must be pre-erased */
+#define OP_MWRITE_BUFFER2 0x89 /* sector must be pre-erased */
+
+/* write to buffer, then write-erase to flash */
+#define OP_PROGRAM_VIA_BUF1 0x82
+#define OP_PROGRAM_VIA_BUF2 0x85
+
+/* compare buffer to flash */
+#define OP_COMPARE_BUF1 0x60
+#define OP_COMPARE_BUF2 0x61
+
+/* read flash to buffer, then write-erase to flash */
+#define OP_REWRITE_VIA_BUF1 0x58
+#define OP_REWRITE_VIA_BUF2 0x59
+
+/* newer chips report JEDEC manufacturer and device IDs; chip
+ * serial number and OTP bits; and per-sector writeprotect.
+ */
+#define OP_READ_ID 0x9F
+#define OP_READ_SECURITY 0x77
+#define OP_WRITE_SECURITY 0x9A /* OTP bits */
+
+
+struct dataflash {
+ u8 command[4];
+ char name[24];
+
+ unsigned short page_offset; /* offset in flash address */
+ unsigned int page_size; /* of bytes per page */
+
+ struct semaphore lock;
+ struct spi_device *spi;
+
+ struct mtd_info mtd;
+};
+
+/* ......................................................................... */
+
+/*
+ * Return the status of the DataFlash device.
+ */
+static inline int dataflash_status(struct spi_device *spi)
+{
+ /* NOTE: at45db321c over 25 MHz wants to write
+ * a dummy byte after the opcode...
+ */
+ return spi_w8r8(spi, OP_READ_STATUS);
+}
+
+/*
+ * Poll the DataFlash device until it is READY.
+ * This usually takes 5-20 msec or so; more for sector erase.
+ */
+static int dataflash_waitready(struct spi_device *spi)
+{
+ int status;
+
+ for (;;) {
+ status = dataflash_status(spi);
+ if (status < 0) {
+ DEBUG(MTD_DEBUG_LEVEL1, "%s: status %d?\n",
+ spi->dev.bus_id, status);
+ status = 0;
+ }
+
+ if (status & (1 << 7)) /* RDY/nBSY */
+ return status;
+
+ msleep(3);
+ }
+}
+
+/* ......................................................................... */
+
+/*
+ * Erase pages of flash.
+ */
+static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+ struct dataflash *priv = (struct dataflash *)mtd->priv;
+ struct spi_device *spi = priv->spi;
+ u8 *command;
+
+ DEBUG(MTD_DEBUG_LEVEL2, "%s: erase addr=%x len %x\n",
+ spi->dev.bus_id,
+ instr->addr, instr->len);
+
+ /* Sanity checks */
+ if ((instr->addr + instr->len) > mtd->size
+ || (instr->len % priv->page_size) != 0
+ || (instr->addr % priv->page_size) != 0)
+ return -EINVAL;
+
+ command = priv->command;
+
+ down(&priv->lock);
+ while (instr->len > 0) {
+ unsigned int pageaddr;
+ int status;
+
+ /* Calculate flash page address */
+ // REVISIT: pageaddr == instr->addr, yes???
+ pageaddr = (instr->addr / priv->page_size) << priv->page_offset;
+
+ /* REVISIT block erase is faster, if this page is at a block
+ * boundary and the next eight pages must all be erased ...
+ */
+ command[0] = OP_ERASE_PAGE;
+ command[1] = (u8)(pageaddr >> 16);
+ command[2] = (u8)(pageaddr >> 8);
+ command[3] = 0;
+
+ DEBUG(MTD_DEBUG_LEVEL3, "ERASE: (%x) %x %x %x [%i]\n",
+ command[0], command[1], command[2], command[3],
+ pageaddr);
+
+ status = spi_write (spi, SPI_M_CS | SPI_M_CSREL, command, 4);
+ (void) dataflash_waitready(spi);
+
+ if (status < 0) {
+ printk(KERN_ERR "%s: erase %x, err %d\n",
+ spi->dev.bus_id, pageaddr, status);
+ continue;
+ }
+
+ instr->addr += priv->page_size; /* next page */
+ instr->len -= priv->page_size;
+ }
+ up(&priv->lock);
+
+ /* Inform MTD subsystem that erase is complete */
+ instr->state = MTD_ERASE_DONE;
+ mtd_erase_callback(instr);
+
+ return 0;
+}
+
+/*
+ * Read from the DataFlash device.
+ * from : Start offset in flash device
+ * len : Amount to read
+ * retlen : About of data actually read
+ * buf : Buffer containing the data
+ */
+static int dataflash_read(struct mtd_info *mtd, loff_t from, size_t len,
+ size_t *retlen, u_char *buf)
+{
+ struct dataflash *priv = (struct dataflash *)mtd->priv;
+ unsigned int addr;
+ u8 *command;
+ int status = 0;
+ struct spi_msg *msg;
+
+ DEBUG(MTD_DEBUG_LEVEL2, "%s: read %x..%x\n",
+ priv->spi->dev.bus_id, (unsigned)from, (unsigned)(from + len));
+
+ *retlen = 0;
+
+ /* Sanity checks */
+ if (!len)
+ return 0;
+ if (from + len > mtd->size)
+ return -EINVAL;
+
+ /* Calculate flash page/byte address */
+ addr = (((unsigned)from / priv->page_size) << priv->page_offset)
+ + ((unsigned)from % priv->page_size);
+
+ command = priv->command;
+
+ DEBUG(MTD_DEBUG_LEVEL3, "READ: (%x) %x %x %x\n",
+ command[0], command[1], command[2], command[3]);
+
+ down(&priv->lock);
+
+ /* Continuous read, max clock = f(car) which may be less than
+ * the peak rate available. Some chips support commands with
+ * fewer "don't care" bytes. Both buffers stay unchanged.
+ */
+ command[0] = OP_READ_CONTINUOUS;
+ command[1] = (u8)(addr >> 16);
+ command[2] = (u8)(addr >> 8);
+ command[3] = (u8)(addr >> 0);
+ /* plus 4 "don't care" bytes */
+
+ msg = spimsg_alloc(priv->spi,
+ SPI_M_RD | SPI_M_CS,
+ command,
+ 8,
+ NULL);
+ if (!msg) {
+ status = -ENOMEM;
+ goto out;
+ }
+ if (!spimsg_chain(msg,
+ SPI_M_RD | SPI_M_CSREL,
+ buf,
+ len,
+ NULL)) {
+ status = -ENOMEM;
+ goto out;
+ }
+ status = spi_transfer(msg, NULL);
+
+ spimsg_free(msg);
+
+ up(&priv->lock);
+
+ if (status >= 0) {
+ *retlen = status;
+ status = 0;
+ } else
+ DEBUG(MTD_DEBUG_LEVEL1, "%s: read %x..%x --> %d\n",
+ priv->spi->dev.bus_id,
+ (unsigned)from, (unsigned)(from + len),
+ status);
+out:
+ return status;
+}
+
+/*
+ * Write to the DataFlash device.
+ * to : Start offset in flash device
+ * len : Amount to write
+ * retlen : Amount of data actually written
+ * buf : Buffer containing the data
+ */
+static int dataflash_write(struct mtd_info *mtd, loff_t to, size_t len,
+ size_t * retlen, const u_char * buf)
+{
+ struct dataflash *priv = (struct dataflash *)mtd->priv;
+ struct spi_device *spi = priv->spi;
+ unsigned int pageaddr, addr, offset, writelen;
+ size_t remaining = len;
+ u_char *writebuf = (u_char *) buf;
+ int status = -EINVAL;
+ u8 *command = priv->command;
+ struct spi_msg *msg;
+
+ DEBUG(MTD_DEBUG_LEVEL2, "%s: write %x..%x\n",
+ spi->dev.bus_id, (unsigned)to, (unsigned)(to + len));
+
+ *retlen = 0;
+
+ /* Sanity checks */
+ if (!len)
+ return 0;
+ if ((to + len) > mtd->size)
+ return -EINVAL;
+
+ pageaddr = ((unsigned)to / priv->page_size);
+ offset = ((unsigned)to % priv->page_size);
+ if (offset + len > priv->page_size)
+ writelen = priv->page_size - offset;
+ else
+ writelen = len;
+
+ down(&priv->lock);
+ while (remaining > 0) {
+ DEBUG(MTD_DEBUG_LEVEL3, "write @ %i:%i len=%i\n",
+ pageaddr, offset, writelen);
+
+ /* REVISIT:
+ * (a) each page in a sector must be rewritten at least
+ * once every 10K sibling erase/program operations.
+ * (b) for pages that are already erased, we could
+ * use WRITE+MWRITE not PROGRAM for ~30% speedup.
+ * (c) WRITE to buffer could be done while waiting for
+ * a previous MWRITE/MWERASE to complete ...
+ * (d) error handling here seems to be mostly missing.
+ *
+ * Two persistent bits per page, plus a per-sector counter,
+ * could support (a) and (b) ... we might consider using
+ * the second half of sector zero, which is just one block,
+ * to track that state. (On AT91, that sector should also
+ * support boot-from-DataFlash.)
+ */
+
+ addr = pageaddr << priv->page_offset;
+
+ /* (1) Maybe transfer partial page to Buffer1 */
+ if (writelen != priv->page_size) {
+ command[0] = OP_TRANSFER_BUF1;
+ command[1] = (addr & 0x00FF0000) >> 16;
+ command[2] = (addr & 0x0000FF00) >> 8;
+ command[3] = 0;
+
+ DEBUG(MTD_DEBUG_LEVEL3, "TRANSFER: (%x) %x %x %x\n",
+ command[0], command[1], command[2], command[3]);
+
+ spi_write (spi, SPI_M_CS | SPI_M_CSREL, command, 4);
+ if (status < 0)
+ DEBUG(MTD_DEBUG_LEVEL1, "%s: xfer %u -> %d \n",
+ spi->dev.bus_id, addr, status);
+
+ (void) dataflash_waitready(priv->spi);
+ }
+
+ /* (2) Program full page via Buffer1 */
+ addr += offset;
+ command[0] = OP_PROGRAM_VIA_BUF1;
+ command[1] = (addr & 0x00FF0000) >> 16;
+ command[2] = (addr & 0x0000FF00) >> 8;
+ command[3] = (addr & 0x000000FF);
+
+ DEBUG(MTD_DEBUG_LEVEL3, "PROGRAM: (%x) %x %x %x\n",
+ command[0], command[1], command[2], command[3]);
+
+ msg = spimsg_alloc(spi,
+ SPI_M_WR | SPI_M_CS,
+ command,
+ 4,
+ NULL);
+ if (!msg)
+ return -ENOMEM;
+ if (!spimsg_chain(msg,
+ SPI_M_WR | SPI_M_CSREL,
+ writebuf,
+ writelen,
+ NULL))
+ return -ENOMEM;
+
+ status = spi_transfer(msg, NULL);
+ spimsg_free(msg);
+
+ if (status < 0)
+ DEBUG(MTD_DEBUG_LEVEL1, "%s: pgm %u/%u -> %d \n",
+ spi->dev.bus_id, addr, writelen, status);
+
+ (void) dataflash_waitready(priv->spi);
+
+#ifdef CONFIG_DATAFLASH_WRITE_VERIFY
+
+ /* (3) Compare to Buffer1 */
+ addr = pageaddr << priv->page_offset;
+ command[0] = OP_COMPARE_BUF1;
+ command[1] = (addr & 0x00FF0000) >> 16;
+ command[2] = (addr & 0x0000FF00) >> 8;
+ command[3] = 0;
+
+ DEBUG(MTD_DEBUG_LEVEL3, "COMPARE: (%x) %x %x %x\n",
+ command[0], command[1], command[2], command[3]);
+
+ status = spi_write (spi, SPI_M_CS | SPI_M_CSREL, command, 4);
+ if (status < 0)
+ DEBUG(MTD_DEBUG_LEVEL1, "%s: compare %u -> %d \n",
+ spi->dev.bus_id, addr, status);
+
+ status = dataflash_waitready(priv->spi);
+
+ /* Check result of the compare operation */
+ if ((status & (1 << 6)) == 1) {
+ printk(KERN_ERR "%s: compare page %u, err %d\n",
+ spi->dev.bus_id, pageaddr, status);
+ remaining = 0;
+ status = -EIO;
+ break;
+ } else
+ status = 0;
+
+#endif /* CONFIG_DATAFLASH_WRITE_VERIFY */
+
+ remaining = remaining - writelen;
+ pageaddr++;
+ offset = 0;
+ writebuf += writelen;
+ *retlen += writelen;
+
+ if (remaining > priv->page_size)
+ writelen = priv->page_size;
+ else
+ writelen = remaining;
+ }
+ up(&priv->lock);
+
+ return status;
+}
+
+/* ......................................................................... */
+
+/*
+ * Register DataFlash device with MTD subsystem.
+ */
+static int __init
+add_dataflash(struct spi_device *spi, char *name,
+ int nr_pages, int pagesize, int pageoffset)
+{
+ struct dataflash *priv;
+ struct mtd_info *device;
+ struct flash_platform_data *pdata = spi->dev.platform_data;
+
+ priv = (struct dataflash *) kzalloc(sizeof *priv, GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ init_MUTEX(&priv->lock);
+ priv->spi = spi;
+ priv->page_size = pagesize;
+ priv->page_offset = pageoffset;
+
+ /* name must be usable with cmdlinepart */
+ sprintf(priv->name, "mtd-%s",
+ spi->dev.parent->bus_id); /* mtd-spipnx0 */
+
+ device = &priv->mtd;
+ device->name = (pdata && pdata->name) ? pdata->name : priv->name;
+ device->size = nr_pages * pagesize;
+ device->erasesize = pagesize;
+ device->owner = THIS_MODULE;
+ device->type = MTD_DATAFLASH;
+ device->flags = MTD_CAP_NORFLASH;
+ device->erase = dataflash_erase;
+ device->read = dataflash_read;
+ device->write = dataflash_write;
+ device->priv = priv;
+
+ dev_info(&spi->dev, "%s (%d KBytes)\n", name, device->size/1024);
+ dev_set_drvdata(&spi->dev, priv);
+
+#ifdef CONFIG_MTD_PARTITIONS
+ {
+ struct mtd_partition *parts;
+ int nr_parts = 0;
+
+#ifdef CONFIG_MTD_CMDLINE_PARTS
+ static const char *part_probes[] = { "cmdlinepart", NULL, };
+
+ nr_parts = parse_mtd_partitions(device, part_probes, &parts, 0);
+#endif
+
+ if (nr_parts <= 0 && pdata && pdata->parts) {
+ parts = pdata->parts;
+ nr_parts = pdata->nr_parts;
+ }
+
+ if (nr_parts > 0)
+ return add_mtd_partitions(device, parts, nr_parts);
+ }
+#endif
+ return add_mtd_device(device);
+}
+
+/*
+ * Detect and initialize DataFlash device:
+ *
+ * Device Density ID code #Pages PageSize Offset
+ * AT45DB011B 1Mbit (128K) xx0011xx (0x0c) 512 264 9
+ * AT45DB021B 2Mbit (256K) xx0101xx (0x14) 1025 264 9
+ * AT45DB041B 4Mbit (512K) xx0111xx (0x1c) 2048 264 9
+ * AT45DB081B 8Mbit (1M) xx1001xx (0x24) 4096 264 9
+ * AT45DB0161B 16Mbit (2M) xx1011xx (0x2c) 4096 528 10
+ * AT45DB0321B 32Mbit (4M) xx1101xx (0x34) 8192 528 10
+ * AT45DB0642 64Mbit (8M) xx111xxx (0x3c) 8192 1056 11
+ * AT45DB1282 128Mbit (16M) xx0100xx (0x10) 16384 1056 11
+ */
+static int __init dataflash_probe(struct spi_device *spi)
+{
+ int status;
+
+ /* if there's a device there, assume it's dataflash.
+ * board setup should have set spi->max_speed_max to
+ * match f(car) for continuous reads, mode 0 or 3.
+ */
+ status = dataflash_status(spi);
+ if (status > 0 && status != 0xff) {
+ switch (status & 0x3c) {
+ case 0x0c: /* 0 0 1 1 x x */
+ status = add_dataflash(spi, "AT45DB011B",
+ 512, 264, 9);
+ break;
+ case 0x14: /* 0 1 0 1 x x */
+ status = add_dataflash(spi, "AT45DB021B",
+ 1025, 264, 9);
+ break;
+ case 0x1c: /* 0 1 1 1 x x */
+ status = add_dataflash(spi, "AT45DB041B",
+ 2048, 264, 9);
+ break;
+ case 0x24: /* 1 0 0 1 x x */
+ status = add_dataflash(spi, "AT45DB081B",
+ 4096, 264, 9);
+ break;
+ case 0x2c: /* 1 0 1 1 x x */
+ status = add_dataflash(spi, "AT45DB161x",
+ 4096, 528, 10);
+ break;
+ case 0x34: /* 1 1 0 1 x x */
+ status = add_dataflash(spi, "AT45DB321x",
+ 8192, 528, 10);
+ break;
+ case 0x38: /* 1 1 1 x x x */
+ case 0x3c:
+ status = add_dataflash(spi, "AT45DB642x",
+ 8192, 1056, 11);
+ break;
+ /* obsolete AT45DB1282 not (yet?) supported */
+ default:
+ printk(KERN_WARNING "%s: unsupported device (%x)\n",
+ spi->dev.bus_id, status & 0x3c);
+ status = -EINVAL;
+ }
+ }
+
+ return status;
+}
+
+static int __exit dataflash_remove(struct spi_device *spi)
+{
+ // struct dataflash *flash = dev_get_drvdata(dev);
+
+ dev_warn(&spi->dev, "implement remove!\n");
+ return -EINVAL;
+}
+
+static struct spi_driver dataflash_driver = {
+ .probe = dataflash_probe,
+ .remove = __exit_p(dataflash_remove),
+
+ /* FIXME: investigate suspend and resume... */
+};
+
+static int __init dataflash_init(void)
+{
+ return spi_driver_add(&dataflash_driver);
+}
+module_init(dataflash_init);
+
+#if 0
+static void __exit dataflash_exit(void)
+{
+ driver_unregister(&dataflash_driver);
+}
+module_exit(dataflash_exit);
+#endif
+
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Andrew Victor, David Brownell");
+MODULE_DESCRIPTION("MTD DataFlash driver");
Index: linux-2.6.orig/include/linux/spi/flash.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi/flash.h
@@ -0,0 +1,27 @@
+#ifndef LINUX_SPI_FLASH_H
+#define LINUX_SPI_FLASH_H
+
+struct mtd_partition;
+
+/**
+ * struct flash_platform_data: board-specific flash data
+ * @name: optional flash device name (eg, as used with mtdparts=)
+ * @parts: optional array of mtd_partitions for static partitioning
+ * @nr_parts: number of mtd_partitions for static partitoning
+ *
+ * Board init code (in arch/.../mach-xxx/board-yyy.c files) can
+ * provide information about SPI flash parts (such as DataFlash) to
+ * help set up the device and its appropriate default partitioning.
+ *
+ * Note that for DataFlash, sizes for pages, blocks, and sectors are
+ * rarely powers of two; and partitions should be sector-aligned.
+ */
+struct flash_platform_data {
+ char *name;
+ struct mtd_partition *parts;
+ unsigned int nr_parts;
+
+ /* we'll likely add more ... use JEDEC IDs, etc */
+};
+
+#endif

2005-12-12 15:22:23

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH 2.6-git 1/4] SPI core refresh: SPI core patch

Signed-off-by: Dmitry Pervushin <[email protected]>
Signed-off-by: Vitaly Wool <[email protected]>

Documentation/spi.txt | 134 +++++++++
arch/arm/Kconfig | 2
drivers/Kconfig | 2
drivers/Makefile | 1
drivers/spi/Kconfig | 46 +++
drivers/spi/Makefile | 19 +
drivers/spi/spi-alloc.c | 101 +++++++
drivers/spi/spi-alloc.h | 54 +++
drivers/spi/spi-core.c | 659 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/spi/spi-core.h | 43 +++
drivers/spi/spi-dev.c | 209 ++++++++++++++
drivers/spi/spi-thread.c | 187 +++++++++++++
drivers/spi/spi-thread.h | 42 ++
include/linux/spi.h | 250 +++++++++++++++++
14 files changed, 1749 insertions(+)

Index: linux-2.6.orig/Documentation/spi.txt
===================================================================
--- /dev/null
+++ linux-2.6.orig/Documentation/spi.txt
@@ -0,0 +1,134 @@
+Documentation/spi.txt
+========================================================
+Table of contents
+1. Introduction -- what is SPI ?
+2. Purposes of this code
+3. SPI devices stack
+3.1 SPI outline
+3.2 How the SPI devices gets discovered and probed ?
+3.3 DMA and SPI messages
+4. SPI functions and structures reference
+5. How to contact authors
+========================================================
+
+1. What is SPI ?
+----------------
+SPI stands for "Serial Peripheral Interface", a full-duplex synchronous
+serial interface for connecting low-/medium-bandwidth external devices
+using four wires. SPI devices communicate using a master/slave relation-
+ship over two data lines and two control lines:
+- Master Out Slave In (MOSI): supplies the output data from the master
+ to the inputs of the slaves;
+- Master In Slave Out (MISO): supplies the output data from a slave to
+ the input of the master. It is important to note that there can be no
+ more than one slave that is transmitting data during any particular
+ transfer;
+- Serial Clock (SCLK): a control line driven by the master, regulating
+ the flow of data bits;
+- Slave Select (SS): a control line that allows slaves to be turned on
+ and off with hardware control.
+More information is also available at http://en.wikipedia.org/wiki/Serial_Peripheral_Interface
+
+2. Purposes of this code
+------------------------
+The supplied patch is starting point for implementing drivers for various
+SPI busses as well as devices connected to these busses. Currently, the
+SPI core supports only for MASTER mode for system running Linux.
+
+3. SPI devices stack
+--------------------
+
+3.1 The SPI outline
+
+The SPI infrastructure deals with several levels of abstraction. They are
+"SPI bus", "SPI bus driver", "SPI slave device" and "SPI device driver". The
+"SPI bus" is hardware device, which usually called "SPI adapter", and has
+"SPI slave devices" connected. From the Linux' point of view, the "SPI bus" is
+structure of type platform_device, and "SPI slave device" is structure of type
+spi_device. The "SPI bus driver" is the driver which controls the whole
+SPI bus (and, particularly, creates and destroys "SPI slave devices" on the bus),
+and "SPI device driver" is driver that controls the only device on the SPI
+bus, controlled by "SPI bus driver". "SPI device driver" can indirectly
+call "SPI bus driver" to send/receive messages using API provided by SPI
+core, and provide its own interface to the kernel and/or userland.
+So, the device stack looks as follows:
+
+ +--------------+ +---------+
+ | some_bus | | spi_bus |
+ +--------------+ +---------+
+ |..| |
+ |..|--------+ +---------------+
+ +------------+| is parent to | SPI devices |
+ | SPI busses |+-------------> | |
+ +------------+ +---------------+
+ | |
+ +----------------+ +----------------------+
+ | SPI bus driver | | SPI device driver |
+ +----------------+ +----------------------+
+
+3.2 How do the SPI devices get discovered and probed ?
+
+In general, the SPI bus driver cannot effectively discover devices
+on its bus. Fortunately, the devices on SPI bus usually implemented
+onboard, so you need to create array of structures spi_device_desc and
+pass this array to function spi_bus_populate, like this:
+ struct spi_device_desc spi_slaves[] = {
+ [0] = {
+ .name = "device1",
+ .param = device1_params,
+ },
+ [1] = {
+ .name = "device2",
+ .param = NULL,
+ }
+ [2] = {
+ NULL, NULL
+ };
+ err = spi_bus_populate( the_spi_bus, spi_slaves, callback );
+
+3.3. DMA and SPI messages
+-------------------------
+
+The core provides additional robustness when the buffer suppiled is not
+DMA-safe. If it is, the core will allocate DMA-safe buffer and copy user-
+supplied buffer to it (before operation in WRITE case, and after in READ case).
+This two situations are distinguished by specific flag SPI_M_DMAUNSAFE.
+Bus driver should use spimsg_get_buffer and spimsg_put_buffer to access buffer.
+The buffer received from spimsg_get_buffer will be always DMA-safe and suitable for
+DMA mapping.
+
+4. SPI functions are structures reference
+-----------------------------------------
+Please refer to kerneldocs for the information. To create it, use command
+ $ scripts/kernel-doc -html drivers/spi/* > spi.html
+
+5. Getting the latest sources
+-----------------------------
+The stable snapshots are available at
+ http://spi-devel.sourceforge.net/downloads
+The most recent sources can be obtained via the CVS.
+Use the following CVS setup to grab them:
+* set CVSROOT environment variable to
+ :pserver:[email protected]:/cvsroot/spi-devel, i. e.
+$ export CVSROOT=:pserver:[email protected]:/cvsroot/spi-devel
+* login into the anonymous CVS using 'cvs login'. When prompted for password,
+ just type Enter, i. e.
+$ cvs login
+Logging in to :pserver:[email protected]:2401/cvsroot/spi-devel
+CVS password:
+$
+* checkout the 'spi-core' CVS repo:
+$ cvs co cpi-core
+You'll get a set of patches to be applied to the most recent 2.6-git kernel.
+
+6. How to contact authors
+-------------------------
+Do you have any comments ? Enhancements ? Device driver ? Feel free
+to contact me:
+ [email protected]
+ [email protected]
+Visit our project page:
+ http://spi-devel.sourceforge.net
+Subscribe to mailing list:
+ [email protected]
+
Index: linux-2.6.orig/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig.orig/arch/arm/Kconfig
+++ linux-2.6.orig/arch/arm/Kconfig
@@ -753,6 +753,8 @@ source "drivers/usb/Kconfig"

source "drivers/mmc/Kconfig"

+source "drivers/spi/Kconfig"
+
endmenu

source "fs/Kconfig"
Index: linux-2.6.orig/drivers/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/Kconfig
+++ linux-2.6.orig/drivers/Kconfig
@@ -44,6 +44,8 @@ source "drivers/char/Kconfig"

source "drivers/i2c/Kconfig"

+source "drivers/spi/Kconfig"
+
source "drivers/w1/Kconfig"

source "drivers/hwmon/Kconfig"
Index: linux-2.6.orig/drivers/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/Makefile
+++ linux-2.6.orig/drivers/Makefile
@@ -69,3 +69,4 @@ obj-$(CONFIG_SGI_IOC4) += sn/
obj-y += firmware/
obj-$(CONFIG_CRYPTO) += crypto/
obj-$(CONFIG_SUPERH) += sh/
+obj-$(CONFIG_SPI) += spi/
Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -0,0 +1,46 @@
+#
+# SPI device configuration
+#
+menu "SPI support"
+
+config SPI
+ tristate "SPI (Serial Peripheral Interface) bus support"
+ default false
+ help
+ Say Y if you need to enable SPI support on your kernel.
+ Say M if you want to create the spi loadable module.
+
+config SPI_THREAD
+ bool "Threaded handling of SPI asynchronous messages"
+ default true
+ help
+ Say Y here to compile thread-based asynchronous message
+ handling for SPI. This will be a default SPI async message
+ handling method, which can be overridden by bus driver.
+ If unsure, say Y.
+
+config SPI_CUSTOM_ALLOC
+ bool "Custom (faster) SPI message allocation"
+ default false
+ help
+ Say Y here to use faster SPI message allocation.
+ If unsure, say N.
+
+config SPI_DEBUG
+ bool "SPI debug output"
+ depends on SPI
+ default false
+ help
+ Say Y there if you'd like to see debug output from SPI drivers
+ If unsure, say N
+
+config SPI_CHARDEV
+ default Y
+ bool "SPI device interface"
+ depends on SPI
+ help
+ Say Y here to use /dev/spiNN device files. They make it possible to have user-space
+ programs use the SPI bus.
+
+endmenu
+
Index: linux-2.6.orig/drivers/spi/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/Makefile
@@ -0,0 +1,19 @@
+#
+# Makefile for the kernel spi bus driver.
+#
+
+spi-y += spi-core.o
+spi-$(CONFIG_SPI_CHARDEV) += spi-dev.o
+spi-$(CONFIG_SPI_THREAD) += spi-thread.o
+spi-$(CONFIG_SPI_CUSTOM_ALLOC) += spi-alloc.o
+# bus drivers
+obj-$(CONFIG_SPI_PNX) += spipnx.o
+# ...functional drivers
+obj-$(CONFIG_SPI_PNX4008_EEPROM) += pnx4008-eeprom.o
+# ...and the common spi-dev driver
+obj-$(CONFIG_SPI) += spi.o
+
+ifeq ($(CONFIG_SPI_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
+
Index: linux-2.6.orig/drivers/spi/spi-core.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-core.c
@@ -0,0 +1,659 @@
+/*
+ * drivers/spi/spi-core.c
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi.h>
+#include <asm/atomic.h>
+
+#include "spi-thread.h"
+#include "spi-alloc.h"
+#include "spi-core.h"
+
+static int spi_device_del(struct device *dev, void *data);
+
+/**
+ * spimsg_set_clock - set message's clock
+ * @message: SPI message
+ * @clock: clock settings
+ */
+void spimsg_set_clock (struct spi_msg* message, u32 clock)
+{
+ message->clock = clock;
+}
+
+/**
+ * spimsg_get_clock - get message's clock
+ * @message: SPI message
+ */
+u32 spimsg_get_clock (struct spi_msg* message)
+{
+ return message->clock;
+}
+
+/**
+ * spimsg_get_flags - get message's flags
+ * @message: SPI message
+ */
+u32 spimsg_get_flags (struct spi_msg* message)
+{
+ return message->flags;
+}
+
+/**
+ * spimsg_get_buffer - get the buffer for I/O ops
+ * @message: SPI message
+ * @buffer: ptr to the buffer to be filled in
+ */
+u32 spimsg_get_buffer (struct spi_msg *message, void **buffer)
+{
+ if (!buffer)
+ return 0;
+ *buffer = message->buf_ptr;
+
+ if (message->flags & SPI_M_DMAUNSAFE) {
+
+ *buffer = kmalloc (message->len+sizeof(void*), SLAB_KERNEL);
+ if (!*buffer)
+ return 0;
+ if (message->flags & SPI_M_WR)
+ memcpy( *buffer, message->buf_ptr, message->len );
+ }
+ return message->len;
+}
+
+/**
+ * spimsg_put_buffer - put the buffer used by I/O ops
+ * @message: SPI message
+ * @buffer: buffer used for I/O
+ */
+void spimsg_put_buffer (struct spi_msg *message, void *buffer)
+{
+ if (message->flags & SPI_M_DMAUNSAFE) {
+ if (message->flags & SPI_M_RD)
+ memcpy (message->buf_ptr, buffer, message->len);
+ kfree(buffer);
+ }
+}
+
+/**
+ * spimsg_get_spidev - obtain the SPI device from the message
+ * @message: SPI message
+ */
+struct spi_device *spimsg_get_spidev (struct spi_msg* message)
+{
+ return message->device;
+}
+
+void spimsg_set_ctx(struct spi_msg *message, void *ctx)
+{
+ message->context = ctx;
+}
+
+void *spimsg_get_ctx(struct spi_msg *message)
+{
+ return message->context;
+}
+
+int spimsg_complete(struct spi_msg *message, int code)
+{
+ if (message->status)
+ message->status(message, code);
+ return code;
+}
+
+static inline struct spi_msg *__spimsg_alloc(struct spi_device *device,
+ struct spi_msg *link,
+ u32 flags,
+ void *buf,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code))
+{
+ struct spi_msg *msg;
+
+ if ((flags & (SPI_M_RD|SPI_M_WR)) == (SPI_M_RD|SPI_M_WR))
+ return NULL;
+ msg = spimsg_kzalloc();
+ if (!msg)
+ return NULL;
+ msg->len = len;
+ msg->status = status;
+ msg->device = device;
+ msg->flags = flags;
+ INIT_LIST_HEAD(&msg->link);
+
+ msg->buf_ptr = buf;
+
+ if (link)
+ link->next = msg;
+
+ return msg;
+}
+
+/**
+ * spimsg_alloc - allocate the spi message
+ * @device: target device
+ * @flags: SPI message flags
+ * @buf: user-supplied buffer
+ * @len: buffer's length
+ * @status: user-supplied callback function
+ */
+struct spi_msg *spimsg_alloc(struct spi_device *device,
+ u32 flags,
+ void *buf,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code))
+{
+ return __spimsg_alloc(device, NULL, flags, buf, len, status);
+}
+
+/**
+ * spimsg_chain - allocate the spi message and link it to the current
+ * @msg: current message
+ * @flags: SPI message flags
+ * @buf: user-supplied buffer
+ * @len: buffer's length
+ * @status: user-supplied callback function
+ */
+struct spi_msg *spimsg_chain(struct spi_msg *msg,
+ u32 flags,
+ void *buf,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code))
+{
+ return msg ?
+ __spimsg_alloc(msg->device, msg, flags, buf, len, status) :
+ NULL;
+}
+
+/**
+ * spimsg_free - free the message(s) allocated by spimsg_alloc
+ * @msg: head message to free
+ */
+void spimsg_free(struct spi_msg *msg)
+{
+ if (msg->next)
+ spimsg_free(msg->next);
+ spimsg_kfree(msg);
+}
+
+/**
+ * spimsg_getnext - get next message in chain
+ * @message: this message
+ */
+struct spi_msg *spimsg_getnext(struct spi_msg *message)
+{
+ return message ? message->next : ERR_PTR(-EINVAL);
+}
+
+/**
+ * spi_bus_match_name - verify that driver matches device on spi bus
+ * @dev: device that hasn't yet being assigned to any driver
+ * @drv: driver for spi device
+ * Description:
+ * match the device to driver.Drivers and devices on SPI bus
+ * are matched by name, just like the platform devices
+ */
+static int spi_bus_match_name(struct device *dev, struct device_driver *drv)
+{
+ return !strcmp(TO_SPI_DEV(dev)->name, drv->name);
+}
+
+/**
+ * spi_bus_suspend - suspend all devices on the spi bus
+ * @dev: spi device to be suspended
+ * @message: PM message
+ * Description:
+ * this function set device on SPI bus to suspended state, just
+ * like platform_bus does
+ */
+static int spi_bus_suspend(struct device * dev, pm_message_t message)
+{
+ int ret = 0;
+
+ if (dev && dev->driver && TO_SPI_DRIVER(dev->driver)->suspend ) {
+ ret = TO_SPI_DRIVER(dev->driver)->suspend( TO_SPI_DEV(dev), message);
+ if (ret == 0 )
+ dev->power.power_state = message;
+ }
+ return ret;
+}
+
+/**
+ * spi_bus_resume - resume functioning of all devices on spi bus
+ * @dev: device to resume
+ * Description:
+ * This function resumes device on SPI bus, just like platform_bus does
+ */
+static int spi_bus_resume(struct device * dev)
+{
+ int ret = 0;
+
+ if (dev && dev->driver && TO_SPI_DRIVER(dev->driver)->suspend ) {
+ ret = TO_SPI_DRIVER(dev->driver)->resume(TO_SPI_DEV(dev));
+ if (ret == 0)
+ dev->power.power_state = PMSG_ON;
+ }
+ return ret;
+}
+
+struct bus_type spi_bus = {
+ .name = "spi",
+ .match = spi_bus_match_name,
+ .suspend = spi_bus_suspend,
+ .resume = spi_bus_resume,
+};
+
+/**
+ * spi_bus_driver_init - init internal bus driver structures
+ * @bus: registered spi_bus_driver structure
+ * @dev: device that represents spi controller
+ * Description:
+ * Once registered by spi_bus_register, the bus driver needs
+ * initialization, that includes starting thread, initializing
+ * internal structures.. The best place where the spi_bus_driver_init
+ * is in the `probe' function, when we already sure that passed
+ * device object is SPI master controller.
+ */
+int spi_bus_driver_init(struct spi_bus_driver *bus, struct device *dev)
+{
+ struct spi_bus_data *pd =
+ kmalloc(sizeof(struct spi_bus_data), SLAB_KERNEL);
+ int err = 0;
+
+ if (!pd) {
+ err = -ENOMEM;
+ goto init_failed;
+ }
+
+ pd->bus = bus;
+ init_MUTEX(&pd->lock);
+ INIT_LIST_HEAD(&pd->msgs);
+ pd->id = dev->bus_id;
+
+ if (!bus->start_async && !bus->stop_async) {
+ bus->start_async = spi_start_async;
+ bus->stop_async = spi_stop_async;
+ if (!bus->queue)
+ bus->queue = spi_queue;
+ }
+
+ dev->platform_data = pd;
+
+ pd->async_data = bus->start_async ? bus->start_async(dev) : NULL;
+
+ return 0;
+
+init_failed:
+ return err;
+}
+
+/**
+ * spi_bus_driver_cleanup - cleanup internal driver structures
+ *
+ * @bus: pointer to spi_bus_driver structure to deinitialize
+ * @dev: bus device object
+ *
+ * Description: in order to roll back initialization of bus driver,
+ * which is made by spi_bus_driver_init, this function should be used
+ **/
+int spi_bus_driver_cleanup (struct spi_bus_driver *bus_drv, struct device *dev)
+{
+ struct spi_bus_data *pd = dev->platform_data;
+
+ if (!pd) {
+ if(bus_drv->stop_async)
+ bus_drv->stop_async(dev, pd->async_data);
+ kfree (pd);
+ dev->platform_data = NULL;
+ device_for_each_child(dev, NULL, spi_device_del);
+ }
+ return 0;
+}
+/**
+ * __spi_bus_free -- unregister all children of the spi bus
+ * @dev: the spi bus `device' object
+ * @context: not used, will be NULL
+ * Description:
+ * This is an internal function that is called when unregistering
+ * bus driver. Responsibility of this function is freeing the
+ * resources that were requested by spi_bus_driver_init
+ */
+static int __spi_bus_free(struct device *dev, void *context)
+{
+ return spi_bus_driver_cleanup(TO_SPI_BUS_DRIVER(dev->driver), dev);
+}
+
+/**
+ * spi_bus_driver_unregister - unregister SPI bus controller from the system
+ * @bus_driver: driver registered by call to spi_bus_driver_register
+ * Description:
+ * This routine unregisters the SPI bus from the system. Before
+ * unregistering, it deletes each SPI device on the bus using call
+ * to __spi_device_free
+ */
+void spi_bus_driver_unregister(struct spi_bus_driver *bus_driver)
+{
+ if (bus_driver) {
+ driver_for_each_device(&bus_driver->driver, NULL, NULL, __spi_bus_free);
+ driver_unregister(&bus_driver->driver);
+ }
+}
+
+/**
+ * spi_device_release - release the spi device structure
+ * @dev: spi_device to be released
+ * Description:
+ * Pointer to this function will be put to dev->release place
+ * This fus called as a part of device removing
+ */
+void spi_device_release(struct device *dev)
+{
+ struct spi_device* sdev = TO_SPI_DEV(dev);
+
+ kfree(sdev);
+}
+
+/**
+ * spi_device_add - add the new (discovered) SPI device to the bus. Mostly used by bus drivers
+ * @parent: the bus device object
+ * @name: name of device (non-null!)
+ * @bus_data: bus data to be assigned to device
+ * Description:
+ * SPI device usually cannot be discovered by SPI bus driver, so it
+ * needs to take the configuration somewhere from hardcoded structures,
+ * and prepare bus_data for its devices
+ */
+struct spi_device* spi_device_add(struct device *parent, char *name, void *bus_data)
+{
+ struct spi_device* dev;
+ static int minor = 0;
+
+ if (!name)
+ goto dev_add_out;
+
+ dev = kzalloc(sizeof(struct spi_device), SLAB_KERNEL);
+ if(!dev)
+ goto dev_add_out;
+
+ dev->dev.parent = parent;
+ dev->dev.bus = &spi_bus;
+ strncpy(dev->name, name, sizeof(dev->name));
+ strncpy(dev->dev.bus_id, name, sizeof(dev->dev.bus_id));
+ dev->dev.release = spi_device_release;
+ dev->dev.platform_data = bus_data;
+
+ if (device_register(&dev->dev)<0) {
+ dev_dbg(parent, "device '%s' cannot be added\n", name);
+ goto dev_add_out_2;
+ }
+ dev->cdev = spi_class_device_create(minor, &dev->dev);
+ dev->minor = minor++;
+ return dev;
+
+dev_add_out_2:
+ kfree(dev);
+dev_add_out:
+ return NULL;
+}
+
+/**
+ * spi_device_del - delete the SPI device
+ * @dev: device to delete
+ * @data: data associated with the device
+ */
+static int spi_device_del(struct device *dev, void *data)
+{
+ struct spi_device *spidev = TO_SPI_DEV(dev);
+ if (spidev->cdev) {
+ spi_class_device_destroy(spidev->cdev);
+ spidev->cdev = NULL;
+ }
+ device_unregister(&spidev->dev);
+ return 0;
+}
+/**
+ * __spi_transfer_callback - callback to process synchronous messages
+ * @msg: message that is about to complete
+ * @code: message status
+ * Description:
+ * callback for synchronously processed message. If spi_transfer
+ * determines that there is no callback provided neither by
+ * msg->status nor callback parameter, the __spi_transfer_callback
+ * will be used, and spi_transfer does not return until transfer
+ * is finished
+ */
+static void __spi_transfer_callback(struct spi_msg *msg, int code)
+{
+ complete(&msg->sync);
+}
+
+/*
+ * spi_transfer - transfer the message either in sync or async way
+ * @msg: message to process
+ * @callback: user-supplied callback
+ * @return:
+ * if both msg->status and callback are set, the error code of -EINVAL
+ * will be returned
+ */
+int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg *, int))
+{
+ int err = -EINVAL;
+ struct device *bus = msg->device->dev.parent;
+
+ if (msg && TO_SPI_BUS_DRIVER(bus->driver)->queue)
+ {
+ if (callback && !msg->status) {
+ msg->status = callback;
+ callback = NULL;
+ }
+
+ if (!callback) {
+ if (!msg->status) {
+ init_completion (&msg->sync);
+ msg->status = __spi_transfer_callback;
+ err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
+ wait_for_completion(&msg->sync);
+ err = 0;
+ } else {
+ err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
+ }
+ }
+ }
+
+ return err;
+}
+
+/**
+ * spi_write - send data to a device on an SPI bus
+ * @dev: the target device
+ * @flags: additional flags for message
+ * @buf: buffer to be sent
+ * @len: buffer's length
+ * @return: Returns the number of bytes transferred, or negative error code.
+ */
+int spi_write(struct spi_device *dev, u32 flags, char *buf, size_t len)
+{
+ struct spi_msg *msg = spimsg_alloc(dev, SPI_M_WR | SPI_M_DMAUNSAFE | flags, buf, len, NULL);
+ int ret;
+
+ ret = spi_transfer(msg, NULL);
+ return ret == 1 ? len : ret;
+}
+
+/**
+ * spi_read - receive data from a device on an SPI bus
+ * @dev: the target device
+ * @flags: additional flags for message
+ * @buf: buffer to be sent
+ * @len: buffer's length
+ * @return: Returns the number of bytes transferred, or negative error code.
+ */
+int spi_read(struct spi_device *dev, u32 flags, char *buf, size_t len)
+{
+ int ret;
+ struct spi_msg *msg = spimsg_alloc(dev, SPI_M_RD | SPI_M_DMAUNSAFE | flags, buf, len, NULL);
+
+ ret = spi_transfer(msg, NULL);
+ return ret == 1 ? len : ret;
+}
+
+/**
+ * spi_bus_populate - populate the bus
+ * @parent: the SPI bus device object
+ * @devices_s: array of structures that represents bus population
+ * @callback: optional pointer to function that called on each device's add
+ * Description:
+ * This function is intended to populate the SPI bus corresponding to
+ * device passed as 1st parameter. If some device cannot be added
+ * because of spi_device_add fail, the function will not try to parse
+ * the rest of list
+ */
+int spi_bus_populate(struct device *parent,
+ struct spi_device_desc* devices_s,
+ void (*callback) (struct device* bus,
+ struct spi_device *new_dev,
+ void* params))
+{
+ struct spi_device *new_device;
+ int count = 0;
+
+ while (devices_s->name) {
+ dev_dbg(parent, " discovered new SPI device, name '%s'\n",
+ devices_s->name);
+ if ((new_device = spi_device_add(parent, devices_s->name, devices_s->params)) == NULL)
+ break;
+ if (callback)
+ callback(parent, new_device, devices_s->params);
+ devices_s++;
+ count++;
+ }
+ return count;
+}
+
+/**
+ * spi_bus_reset - reset the spi bus
+ * @bus: device object that represents the SPI bus
+ * @context: u32 value to be passed to reset method of bus
+ * Description:
+ * This is simple wrapper for bus' `reset' method
+ */
+void spi_bus_reset (struct device *bus, u32 context)
+{
+ if (bus && bus->driver && TO_SPI_BUS_DRIVER(bus->driver)->reset)
+ TO_SPI_BUS_DRIVER(bus->driver)->reset(bus, context);
+}
+
+/*
+ * The functions below are wrappers for corresponding device_driver's methods
+ */
+static int spi_driver_probe (struct device *dev)
+{
+ struct spi_driver *sdrv = TO_SPI_DRIVER(dev->driver);
+ struct spi_device *sdev = TO_SPI_DEV(dev);
+
+ return sdrv->probe ? sdrv->probe(sdev) : -EFAULT;
+}
+
+static int spi_driver_remove (struct device *dev)
+{
+ struct spi_driver *sdrv = TO_SPI_DRIVER(dev->driver);
+ struct spi_device *sdev = TO_SPI_DEV(dev);
+
+ return sdrv->remove ? sdrv->remove(sdev) : -EFAULT;
+}
+
+static void spi_driver_shutdown (struct device *dev)
+{
+ struct spi_driver *sdrv = TO_SPI_DRIVER(dev->driver);
+ struct spi_device *sdev = TO_SPI_DEV(dev);
+
+ if (dev->driver && sdrv->shutdown)
+ sdrv->shutdown(sdev);
+}
+
+static int __init spi_core_init(void)
+{
+ int ret = spidev_init();
+
+ if (ret < 0)
+ goto out;
+
+ ret = spimsg_init();
+ if (ret < 0)
+ goto out;
+
+ ret = bus_register(&spi_bus);
+
+out:
+ return ret;
+}
+
+int spi_driver_add(struct spi_driver *drv)
+{
+ drv->driver.bus = &spi_bus;
+ drv->driver.probe = spi_driver_probe;
+ drv->driver.remove = spi_driver_remove;
+ drv->driver.shutdown = spi_driver_shutdown;
+ return driver_register(&drv->driver);
+}
+
+static void __exit spi_core_exit(void)
+{
+ bus_unregister(&spi_bus);
+ spimsg_exit();
+ spidev_cleanup();
+}
+
+subsys_initcall(spi_core_init);
+module_exit(spi_core_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("dmitry pervushin <[email protected]>");
+
+EXPORT_SYMBOL_GPL(spi_bus_reset);
+EXPORT_SYMBOL_GPL(spi_device_add);
+EXPORT_SYMBOL_GPL(spi_driver_add);
+EXPORT_SYMBOL_GPL(spi_bus_driver_unregister);
+EXPORT_SYMBOL_GPL(spi_bus_populate);
+EXPORT_SYMBOL_GPL(spi_transfer);
+EXPORT_SYMBOL_GPL(spi_write);
+EXPORT_SYMBOL_GPL(spi_read);
+EXPORT_SYMBOL_GPL(spi_bus);
+EXPORT_SYMBOL_GPL(spi_bus_driver_init);
+EXPORT_SYMBOL_GPL(spi_bus_driver_cleanup);
+
+EXPORT_SYMBOL_GPL(spimsg_alloc);
+EXPORT_SYMBOL_GPL(spimsg_chain);
+EXPORT_SYMBOL_GPL(spimsg_free);
+EXPORT_SYMBOL_GPL(spimsg_put_buffer);
+EXPORT_SYMBOL_GPL(spimsg_get_flags);
+EXPORT_SYMBOL_GPL(spimsg_get_buffer);
+EXPORT_SYMBOL_GPL(spimsg_get_clock);
+EXPORT_SYMBOL_GPL(spimsg_set_clock);
+EXPORT_SYMBOL_GPL(spimsg_getnext);
+EXPORT_SYMBOL_GPL(spimsg_get_spidev);
+EXPORT_SYMBOL_GPL(spimsg_set_ctx);
+EXPORT_SYMBOL_GPL(spimsg_get_ctx);
+EXPORT_SYMBOL_GPL(spimsg_complete);
+
Index: linux-2.6.orig/drivers/spi/spi-core.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-core.h
@@ -0,0 +1,43 @@
+/*
+ * linux/drivers/spi/spi-core.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_CORE_H
+#define __SPI_CORE_H
+
+struct spi_msg {
+ u32 flags;
+#define SPI_M_RD 0x00000001
+#define SPI_M_WR 0x00000002 /**< Write mode flag */
+#define SPI_M_CSREL 0x00000004 /**< CS release level at end of the frame */
+#define SPI_M_CS 0x00000008 /**< CS active level at begining of frame */
+#define SPI_M_CPOL 0x00000010 /**< Clock polarity */
+#define SPI_M_CPHA 0x00000020 /**< Clock Phase */
+#define SPI_M_EXTBUF 0x80000000 /** externally allocated buffers */
+#define SPI_M_ASYNC_CB 0x40000000 /** use workqueue to deliver callbacks */
+#define SPI_M_DNA 0x20000000 /** do not allocate buffers */
+#define SPI_M_DMAUNSAFE 0x10000000 /** buffer is dma-unsafe */
+
+ u16 len; /* msg length */
+ u32 clock;
+ struct spi_device *device;
+ void *context;
+
+ struct completion sync;
+
+ struct spi_msg *next;
+
+ struct list_head link;
+
+ void (*status) (struct spi_msg * msg, int code);
+
+ void *buf_ptr;
+};
+
+#endif /* __SPI_CORE_H */
Index: linux-2.6.orig/drivers/spi/spi-dev.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-dev.c
@@ -0,0 +1,209 @@
+/*
+ * drivers/spi/spi-dev.c
+ *
+ * Character device interface for SPI
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#include <linux/init.h>
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/version.h>
+#include <linux/smp_lock.h>
+
+#include <linux/init.h>
+#include <asm/uaccess.h>
+#include <linux/spi.h>
+
+#define SPI_TRANSFER_MAX 65535L
+
+static struct class *spidev_class;
+
+static ssize_t spidev_read(struct file *file, char *buf, size_t count,
+ loff_t * offset);
+static ssize_t spidev_write(struct file *file, const char *buf, size_t count,
+ loff_t * offset);
+
+static int spidev_open(struct inode *inode, struct file *file);
+static int spidev_release(struct inode *inode, struct file *file);
+
+/**
+ * spi_class_device_create - wrapper for class_device_create
+ * @minor: sequental minor number of SPI slave device
+ * @device: pointer to struct device embedded to spi_device
+ */
+struct class_device *spi_class_device_create(int minor, struct device *device)
+{
+ return class_device_create(spidev_class, NULL, MKDEV(SPI_MAJOR, minor), device, "spi%d", minor);
+}
+
+/**
+ * spi_class_device_destroy - wrapper for class_device_destroy
+ * @cdev: class device created by spi_class_device_create
+ */
+void spi_class_device_destroy(struct class_device *cdev)
+{
+ class_device_destroy(spidev_class, cdev->devt);
+}
+
+static struct file_operations spidev_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .read = spidev_read,
+ .write = spidev_write,
+ .open = spidev_open,
+ .release = spidev_release,
+};
+
+/**
+ * spidev_read - read from the SPI device
+ * @file: device file
+ * @buf: buffer to read into
+ * @count: number of bytes to read
+ * @offset: offset to read from
+ */
+static ssize_t spidev_read(struct file *file, char __user *buf, size_t count,
+ loff_t * offset)
+{
+ int rc = 0;
+ char *kbuf = kmalloc(count, GFP_DMA);
+ struct spi_device *dev = (struct spi_device *)file->private_data;
+
+ if (!kbuf)
+ rc = -ENOMEM;
+ else {
+ rc = spi_read(dev, SPI_M_DNA, kbuf, count);
+ if (rc < 0 || copy_to_user(buf, kbuf, count))
+ rc = -EFAULT;
+ kfree(kbuf);
+ }
+ return rc;
+}
+
+/**
+ * spidev_write - write to the SPI device
+ * @file: device file
+ * @buf: buffer to write the data from
+ * @count: number of bytes to write
+ * @offset: start offset to write to
+ */
+static ssize_t spidev_write(struct file *file, const char __user *buf, size_t count,
+ loff_t * offset)
+{
+ int rc = 0;
+ char *kbuf = kmalloc(count, GFP_DMA);
+ struct spi_device *dev = (struct spi_device *)file->private_data;
+
+ if (!kbuf)
+ rc = -ENOMEM;
+ else {
+ if (!copy_from_user(kbuf, buf, count))
+ rc = spi_write(dev, SPI_M_DNA, kbuf, count);
+ else
+ rc = -EFAULT;
+ kfree(kbuf);
+ }
+ return rc;
+}
+
+struct spidev_openclose {
+ unsigned int minor;
+ struct file *file;
+};
+
+/**
+ * spidev_do_open - open the SPI device
+ * @the_device: device structure
+ * @context: context of SPI device
+ */
+static int spidev_do_open(struct device *the_dev, void *context)
+{
+ struct spidev_openclose *o = (struct spidev_openclose *)context;
+ struct spi_device *dev = TO_SPI_DEV(the_dev);
+
+ pr_debug("device->minor = %d vs %d\n", dev->minor, o->minor);
+ if (dev->minor == o->minor) {
+ get_device(&dev->dev);
+ o->file->private_data = dev;
+ return 1;
+ }
+
+ return 0;
+}
+
+/**
+ * spidev_open - open the SPI bus device
+ * @inode: device inode
+ * @file: device file
+ */
+int spidev_open(struct inode *inode, struct file *file)
+{
+ struct spidev_openclose o;
+ int status;
+
+ o.minor = iminor(inode);
+ o.file = file;
+ status = bus_for_each_dev(&spi_bus, NULL, &o, spidev_do_open);
+ if (status == 0) {
+ status = -ENODEV;
+ }
+ return status < 0 ? status : 0;
+}
+
+/**
+ * spidev_release - release the SPI bus device
+ * @inode: device inode
+ * @file: device file
+ */
+static int spidev_release(struct inode *inode, struct file *file)
+{
+ struct spi_device *dev = file->private_data;
+
+ if (dev)
+ put_device(&dev->dev);
+ file->private_data = NULL;
+
+ return 0;
+}
+
+int __init spidev_init(void)
+{
+ int res;
+
+ if ((res = register_chrdev(SPI_MAJOR, "spi", &spidev_fops)) != 0) {
+ goto out;
+ }
+
+ spidev_class = class_create(THIS_MODULE, "spi");
+ if (IS_ERR(spidev_class)) {
+ printk(KERN_ERR "%s: error creating class\n", __FUNCTION__);
+ res = -EINVAL;
+ goto out_unreg;
+ }
+
+ return 0;
+
+out_unreg:
+ unregister_chrdev(SPI_MAJOR, "spi");
+out:
+ return res;
+}
+
+void __exit spidev_cleanup(void)
+{
+ class_destroy(spidev_class);
+ unregister_chrdev(SPI_MAJOR, "spi");
+}
+
+MODULE_AUTHOR("dmitry pervushin <[email protected]>");
+MODULE_DESCRIPTION("SPI class device driver");
+MODULE_LICENSE("GPL");
Index: linux-2.6.orig/drivers/spi/spi-thread.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.c
@@ -0,0 +1,187 @@
+/*
+ * drivers/spi/spi-thread.c
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi.h>
+#include <asm/atomic.h>
+#include "spi-core.h"
+
+static int spi_thread(void *context);
+
+struct threaded_async_data {
+ atomic_t exiting;
+ struct device *dev;
+ struct task_struct *thread;
+ wait_queue_head_t wq;
+};
+
+/**
+ * __spi_start_async - start the thread
+ * @dev: device which the thread is related to
+ * @return: abstract pointer to the thread context
+ */
+void *__spi_start_async (struct device *dev)
+{
+ struct threaded_async_data *td = kmalloc (sizeof (struct threaded_async_data), GFP_KERNEL);
+
+ if (!td)
+ return NULL;
+
+ td->dev = dev;
+ atomic_set(&td->exiting, 0);
+ td->thread = kthread_run(spi_thread, td, "%s-work", dev->bus_id);
+ init_waitqueue_head(&td->wq);
+ return td;
+}
+
+/**
+ * __spi_stop_async - stop the thread
+ * @dev: device which the thread is related to
+ * @ctx: abstract pointer to the thread context
+ */
+void __spi_stop_async (struct device *dev, void *ctx)
+{
+ struct threaded_async_data *td = ctx;
+
+ if (ctx) {
+ atomic_inc (&td->exiting);
+ kthread_stop(td->thread);
+ kfree(td);
+ }
+}
+
+/**
+ * spi_thread_awake - function that called to determine if thread needs to process any messages
+ * @td: pointer to struct threaded_async_data
+ * Description:
+ * Thread wakes up if there is signal to exit (bd->exiting is set)
+ * or there are any messages in bus' queue.
+ */
+static int spi_thread_awake(struct threaded_async_data *td)
+{
+ int ret = -EINVAL;
+ struct spi_bus_data *bd = td->dev->platform_data;
+
+ if (atomic_read(&td->exiting)) {
+ return 1;
+ }
+
+ if (bd) {
+ down(&bd->lock);
+ ret = !list_empty(&bd->msgs);
+ up(&bd->lock);
+ }
+ return ret;
+}
+
+/**
+ * spi_bus_next_msg - retrieve the next message
+ * @this: spi_bus_driver that needs to retrieve next message from queue
+ * @data: pointer to spi_bus_data structure associated with spi_bus_driver
+ */
+static struct spi_msg *spi_bus_next_msg(struct spi_bus_driver *this, struct spi_bus_data *data)
+{
+ return list_entry(data->msgs.next, struct spi_msg, link);
+}
+
+/**
+ * spi_thread - the thread that calls bus functions to perform actual transfers
+ * @context: pointer to struct spi_bus_data with bus-specific data
+ * Description:
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ **/
+static int spi_thread(void *context)
+{
+ struct threaded_async_data *td = context;
+ struct spi_bus_data *bd = td->dev->platform_data;
+ struct spi_msg *cmsg = NULL;
+ int xfer_status;
+
+ while (!kthread_should_stop()) {
+
+ wait_event_interruptible(td->wq, spi_thread_awake(td));
+
+ if (atomic_read(&td->exiting))
+ goto thr_exit;
+
+ down(&bd->lock);
+ while (!list_empty(&bd->msgs)) {
+ /*
+ * this part is locked by bus_data->lock,
+ * to protect spi_msg extraction
+ */
+ cmsg = spi_bus_next_msg(bd->bus, bd);
+ list_del (&cmsg->link);
+
+ up(&bd->lock);
+
+ /*
+ * and this part is locked by device's lock;
+ * spi_queue will be able to queue new
+ * messages
+ *
+ * note that bd->selected_device is locked,
+ * not msg->device
+ * they are the same, but msg can be freed in
+ * msg->status function
+ */
+ bd->selected_device = spimsg_get_spidev(cmsg);
+ spi_device_lock(bd->selected_device);
+ if (bd->bus->set_clock && cmsg->clock)
+ bd->bus->set_clock(cmsg->device->dev.parent,
+ cmsg->clock);
+ xfer_status = bd->bus->xfer(cmsg);
+ if (cmsg->status)
+ cmsg->status(cmsg, xfer_status);
+
+ spi_device_unlock(bd->selected_device);
+
+ /* lock the bus_data again... */
+ down(&bd->lock);
+ }
+ up(&bd->lock);
+ }
+
+thr_exit:
+ return 0;
+}
+
+/**
+ * __spi_queue - (internal) queue the message to be processed asynchronously
+ * @msg: message to be sent
+ * Description:
+ * This function queues the message to spi bus driver's queue.
+ */
+int __spi_queue(struct spi_msg *msg)
+{
+ struct device *dev = &msg->device->dev;
+ struct spi_bus_data *pd = dev->parent->platform_data;
+ struct threaded_async_data *td = pd->async_data;
+
+ down(&pd->lock);
+ list_add_tail(&msg->link, &pd->msgs);
+ dev_dbg(dev->parent, "message has been queued\n");
+ up(&pd->lock);
+ wake_up_interruptible(&td->wq);
+ return 0;
+}
+
+
Index: linux-2.6.orig/drivers/spi/spi-thread.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.h
@@ -0,0 +1,42 @@
+/*
+ * linux/drivers/spi/spi-thread.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_THREAD_H
+#define __SPI_THREAD_H
+
+
+static inline void *spi_start_async (struct device *dev)
+{
+#ifdef CONFIG_SPI_THREAD
+ extern void *__spi_start_async (struct device *dev);
+ return __spi_start_async(dev);
+#else
+ return 0;
+#endif
+}
+
+static inline void spi_stop_async (struct device *dev, void *t)
+{
+#ifdef CONFIG_SPI_THREAD
+ extern void __spi_stop_async (struct device *dev, void *t);
+ __spi_stop_async (dev, t);
+#endif
+}
+
+static inline int spi_queue (struct spi_msg *msg)
+{
+#ifdef CONFIG_SPI_THREAD
+ extern int __spi_queue (struct spi_msg *msg);
+ return __spi_queue(msg);
+#else
+ return -EINVAL;
+#endif
+}
+#endif /* __SPI_THREAD_H */
Index: linux-2.6.orig/include/linux/spi.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi.h
@@ -0,0 +1,250 @@
+/*
+ * linux/include/linux/spi/spi.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * Derived from l3.h by Jamey Hicks
+ */
+#ifndef SPI_H
+#define SPI_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+
+struct spi_device;
+struct spi_driver;
+struct spi_msg;
+struct spi_bus_driver;
+
+extern struct bus_type spi_bus;
+
+struct spi_bus_data {
+ struct semaphore lock;
+ struct list_head msgs;
+ void *async_data;
+ wait_queue_head_t queue;
+ struct spi_device *selected_device;
+ struct spi_bus_driver *bus;
+ char *id;
+};
+
+#define TO_SPI_BUS_DRIVER(drv) container_of(drv, struct spi_bus_driver, driver)
+struct spi_bus_driver {
+
+ int (*xfer) (struct spi_msg * msg);
+ void (*set_clock) (struct device * bus_device, u32 clock_hz);
+ void (*reset) (struct device *bus_device, u32 context);
+
+ int (*queue) (struct spi_msg *msg);
+ void *(*start_async)( struct device *bus);
+ void (*stop_async)( struct device *bus, void *async);
+
+ struct device_driver driver;
+};
+
+#define TO_SPI_DEV(device) container_of(device, struct spi_device, dev)
+struct spi_device {
+ char name[BUS_ID_SIZE];
+ int minor;
+ struct class_device *cdev;
+ struct device dev;
+};
+
+static inline void *spi_device_get_busdata(struct spi_device *dev)
+{
+ return dev->dev.platform_data;
+}
+
+#define TO_SPI_DRIVER(drv) container_of(drv, struct spi_driver, driver)
+struct spi_driver {
+
+ int (*probe) (struct spi_device * dev);
+ int (*remove) (struct spi_device * dev);
+ void (*shutdown) (struct spi_device * dev);
+ int (*suspend) (struct spi_device * dev, pm_message_t pm);
+ int (*resume) (struct spi_device * dev);
+
+ struct device_driver driver;
+};
+
+#define SPI_DEV_DRV(device) TO_SPI_BUS_DRIVER((device)->dev.parent->driver)
+
+#define spi_device_lock(spi_dev) down(&(spi_dev)->dev.sem)
+#define spi_device_unlock(spi_dev) up(&(spi_dev)->dev.sem)
+
+#define SPI_M_RD 0x00000001
+#define SPI_M_WR 0x00000002 /**< Write mode flag */
+#define SPI_M_CSREL 0x00000004 /**< CS release level at end of the frame */
+#define SPI_M_CS 0x00000008 /**< CS active level at begining of frame */
+#define SPI_M_CSKEEP 0x00000010 /**< Don't change CS */
+#define SPI_M_CPOL 0x00000100 /**< Clock polarity */
+#define SPI_M_CPHA 0x00000200 /**< Clock Phase */
+#define SPI_M_DMAUNSAFE 0x10000000 /** buffer is dma-unsafe */
+
+void spimsg_set_ctx (struct spi_msg *message, void *ctx);
+void *spimsg_get_ctx (struct spi_msg *message);
+int spimsg_complete (struct spi_msg *message, int code);
+void spimsg_set_clock (struct spi_msg* message, u32 clock);
+u32 spimsg_get_clock (struct spi_msg* message);
+struct spi_device *spimsg_get_spidev (struct spi_msg* message);
+u32 spimsg_get_flags (struct spi_msg* message);
+u32 spimsg_get_buffer (struct spi_msg *message, void **buffer);
+void spimsg_put_buffer (struct spi_msg *message, void *buffer);
+struct spi_msg *spimsg_alloc(struct spi_device *device,
+ u32 flags,
+ void *buf,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code));
+struct spi_msg *spimsg_chain(struct spi_msg *msg,
+ u32 flags,
+ void *buf,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code));
+
+void spimsg_free (struct spi_msg *message);
+struct spi_msg *spimsg_getnext(struct spi_msg *message);
+
+#if defined (CONFIG_SPI_CHARDEV)
+extern struct class_device *spi_class_device_create(int minor, struct device *device);
+extern void spi_class_device_destroy(struct class_device *cdev);
+#else
+static inline struct class_device *spi_class_device_create(int minor, struct device *device)
+{
+ return NULL;
+}
+static inline void spi_class_device_destroy(struct class_device *cdev)
+{
+}
+#endif
+
+enum {
+ SPIMSG_OK = 0,
+ SPIMSG_FAILED = -1,
+};
+
+#define SPI_MAJOR 153
+
+struct spi_driver;
+struct spi_device;
+
+#if defined (CONFIG_SPI_CHARDEV)
+extern int __init spidev_init(void);
+extern void __exit spidev_cleanup(void);
+#else
+static inline int spidev_init(void)
+{
+ return 0;
+}
+static inline void spidev_cleanup(void)
+{
+}
+#endif
+
+static inline int spi_bus_driver_register (struct spi_bus_driver *bus_driver)
+{
+ return driver_register (&bus_driver->driver);
+}
+
+void spi_bus_driver_unregister(struct spi_bus_driver *);
+int spi_bus_driver_init(struct spi_bus_driver *driver, struct device *dev);
+int spi_bus_driver_cleanup(struct spi_bus_driver *driver, struct device *dev);
+struct spi_device* spi_device_add(struct device *parent, char *name, void *private);
+
+extern int spi_driver_add(struct spi_driver *drv);
+
+static inline void spi_driver_del(struct spi_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+
+extern void spi_bus_reset(struct device* bus, u32 context);
+extern int spi_write(struct spi_device *dev, u32 flags, char *buf, size_t len);
+extern int spi_read(struct spi_device *dev, u32 flags, char *buf, size_t len);
+
+extern int spi_transfer(struct spi_msg *message,
+ void (*status) (struct spi_msg *, int));
+struct spi_device_desc {
+ char* name;
+ void* params;
+};
+extern int spi_bus_populate(struct device *parent,
+ struct spi_device_desc *devices,
+ void (*assign) (struct device *parent,
+ struct spi_device *,
+ void *));
+
+static inline int spi_w8r8 (struct spi_device *dev, u8 wr)
+{
+ u8 byte;
+ int rc = -ENOMEM;
+ struct spi_msg *msg;
+
+ msg = spimsg_alloc(dev,
+ SPI_M_WR | SPI_M_CS | SPI_M_DMAUNSAFE,
+ &wr,
+ 1,
+ NULL);
+ if (!msg)
+ goto out;
+ msg = spimsg_chain(msg,
+ SPI_M_RD | SPI_M_CSREL | SPI_M_DMAUNSAFE,
+ &byte,
+ 1,
+ NULL);
+
+ if (!msg)
+ goto out;
+
+ rc = spi_transfer(msg, NULL);
+ spimsg_free(msg);
+
+out:
+ return rc < 0 ? rc : byte;
+}
+
+static inline int spi_w8r16 (struct spi_device *dev, u8 wr)
+{
+ u16 word;
+ int rc = -ENOMEM;
+ struct spi_msg *msg;
+
+ msg = spimsg_alloc(dev,
+ SPI_M_WR | SPI_M_CS | SPI_M_DMAUNSAFE,
+ &wr,
+ 1,
+ NULL);
+ if (!msg)
+ goto out;
+ msg = spimsg_chain(msg,
+ SPI_M_RD | SPI_M_CSREL | SPI_M_DMAUNSAFE,
+ &word,
+ 2,
+ NULL);
+
+ if (!msg)
+ goto out;
+
+ rc = spi_transfer(msg, NULL);
+ spimsg_free(msg);
+
+out:
+ return rc < 0 ? rc : word;
+}
+
+static inline int spi_sync(struct spi_msg *message)
+{
+ return spi_transfer(message, NULL);
+}
+
+static inline int spi_async(struct spi_msg *message, void (*status) (struct spi_msg *, int))
+{
+ return spi_transfer(message, status);
+}
+
+#endif /* SPI_H */
Index: linux-2.6.orig/drivers/spi/spi-alloc.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-alloc.c
@@ -0,0 +1,101 @@
+/*
+ * linux/drivers/spi/spi-alloc.c
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/spi.h>
+#include <asm/atomic.h>
+#include "spi-core.h"
+
+#define SPIMSG_POOL_SIZE 0x10000
+
+static struct spi_msg_pool {
+ void *vaddr;
+ void *cur;
+ int count;
+} spimsg_pool;
+
+static spinlock_t spimsg_lock = SPIN_LOCK_UNLOCKED;
+
+struct spi_msg *__spimsg_kzalloc(void)
+{
+ struct spi_msg *msg = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&spimsg_lock, flags);
+ if (spimsg_pool.count > 4) {
+ msg = *(struct spi_msg **) spimsg_pool.cur;
+ *(void **)spimsg_pool.cur = **(void ***)spimsg_pool.cur;
+ memset(msg, 0, sizeof(*msg));
+ spimsg_pool.count--;
+ }
+ spin_unlock_irqrestore(&spimsg_lock, flags);
+
+ return msg;
+}
+
+void __spimsg_kfree(struct spi_msg *msg)
+{
+ unsigned long flags;
+
+ if (msg) {
+ if ((unsigned long)((long)msg - (long)spimsg_pool.vaddr) >
+ SPIMSG_POOL_SIZE) {
+ printk(KERN_ERR "Trying to free entry not from the SPI pool\n");
+ BUG();
+ }
+
+ spin_lock_irqsave(&spimsg_lock, flags);
+ *(long *)msg = *(long *)spimsg_pool.cur;
+ *(long *)spimsg_pool.cur = (long)msg;
+ spimsg_pool.count++;
+ spin_unlock_irqrestore(&spimsg_lock, flags);
+ }
+}
+
+int __spimsg_init(void)
+{
+ int ret = 0, i;
+
+ int size = ((sizeof(struct spi_msg) + 3) >> 2) << 2;
+
+ spimsg_pool.cur = spimsg_pool.vaddr =
+ kzalloc(SPIMSG_POOL_SIZE, SLAB_KERNEL);
+
+ if (!spimsg_pool.cur) {
+ printk(KERN_ERR "Couldn't allocate large buffer for SPI\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ spimsg_pool.count = SPIMSG_POOL_SIZE / size;
+ for (i = 0; i < spimsg_pool.count - 1; i++) {
+ void **addr = spimsg_pool.vaddr + i*size;
+ *addr = (void *)addr + size;
+ }
+ *(long *)(spimsg_pool.vaddr + (spimsg_pool.count - 1) * size) =
+ (long)spimsg_pool.vaddr;
+
+out:
+ return ret;
+}
+
+void __spimsg_exit(void)
+{
+ if (spimsg_pool.vaddr)
+ kfree(spimsg_pool.vaddr);
+}
Index: linux-2.6.orig/drivers/spi/spi-alloc.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-alloc.h
@@ -0,0 +1,54 @@
+/*
+ * linux/linux/drivers/spi/spi-alloc.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_ALLOC_H
+#define __SPI_ALLOC_H
+
+#include <linux/slab.h>
+
+static inline struct spi_msg *spimsg_kzalloc(void)
+{
+#ifdef CONFIG_SPI_CUSTOM_ALLOC
+ extern struct spi_msg *__spimsg_kzalloc(void);
+ return __spimsg_kzalloc();
+#else
+ return kzalloc(sizeof(struct spi_msg), SLAB_KERNEL);
+#endif
+}
+
+static inline void spimsg_kfree(struct spi_msg *msg)
+{
+#ifdef CONFIG_SPI_CUSTOM_ALLOC
+ extern void __spimsg_kfree (struct spi_msg *msg);
+ __spimsg_kfree(msg);
+#else
+ kfree(msg);
+#endif
+}
+
+static inline int spimsg_init(void)
+{
+#ifdef CONFIG_SPI_CUSTOM_ALLOC
+ extern int __spimsg_init(void);
+ return __spimsg_init();
+#else
+ return 0;
+#endif
+}
+
+static inline void spimsg_exit(void)
+{
+#ifdef CONFIG_SPI_CUSTOM_ALLOC
+ extern void __spimsg_exit(void);
+ __spimsg_exit();
+#endif
+}
+
+#endif /* __SPI_ALLOC_H */

2005-12-12 15:25:33

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH 2.6-git 3/4] SPI core refresh: SPI/PNX controller

Signed-off-by: Dmitry Pervushin <[email protected]>
Signed-off-by: Vitaly Wool <[email protected]>

Kconfig | 8
pnx4008-eeprom.c | 121 +++++++++++++
spipnx-init.h | 9 +
spipnx.c | 488 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
spipnx.h | 58 ++++++
5 files changed, 684 insertions(+)

Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Kconfig
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -42,5 +42,13 @@ config SPI_CHARDEV
Say Y here to use /dev/spiNN device files. They make it possible to have user-space
programs use the SPI bus.

+config SPI_PNX
+ tristate "PNX SPI bus support"
+ depends on ARCH_PNX4008 && SPI
+
+config SPI_PNX4008_EEPROM
+ tristate "Dummy EEPROM driver"
+ depends on SPI && SPI_PNX && ARCH_PNX4008
+
endmenu

Index: linux-2.6.orig/drivers/spi/pnx4008-eeprom.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/pnx4008-eeprom.c
@@ -0,0 +1,121 @@
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/device.h>
+#include <linux/spi.h>
+#include <linux/proc_fs.h>
+#include <linux/ctype.h>
+
+#include "spipnx.h"
+
+#define EEPROM_SIZE 256
+#define DRIVER_NAME "EEPROM"
+#define READ_BUFF_SIZE 160
+
+static int __init spi_eeprom_init(void);
+static void __exit spi_eeprom_cleanup(void);
+
+static int spiee_read_block (struct device *d, void *block)
+{
+ struct spi_device *device = TO_SPI_DEV (d);
+ char cmd[2];
+ struct spi_msg *msg = spimsg_alloc(device,
+ SPI_M_CS|SPI_M_CSREL,
+ NULL,
+ 0,
+ NULL);
+ struct spi_msg *msg_cmd = spimsg_chain(msg, SPI_M_CS|SPI_M_WR|SPI_M_DMAUNSAFE, cmd, 2, NULL);
+ spimsg_chain(msg_cmd, SPI_M_RD|SPI_M_CSREL, block, 256, NULL);
+
+ cmd[ 0 ] = 0x03;
+ cmd[ 1 ] = 0x00;
+
+ spimsg_set_clock(msg, 2000000); /* 2 MHz */
+ spi_transfer(msg, NULL);
+ spimsg_free(msg);
+ return 256;
+}
+static ssize_t blk_show (struct device *d, struct device_attribute *attr, char *text )
+{
+ char *rdbuff = kmalloc (256, SLAB_KERNEL);
+ char line1[80],line2[80];
+ char item1[5], item2[5];
+ int bytes, i, x, blen;
+
+ blen = spiee_read_block (d, rdbuff);
+
+ bytes = 0;
+
+ strcpy(text, "");
+ for (i = 0; i < blen; i += 8) {
+ strcpy(line1, "");
+ strcpy(line2, "" );
+ for (x = i; x < i + 8; x++) {
+ if (x > blen) {
+ sprintf(item1, " ");
+ sprintf(item2, " " );
+ } else {
+ sprintf(item1, "%02x ", rdbuff[x]);
+ if (isprint(rdbuff[x])) {
+ sprintf(item2, "%c", rdbuff[x]);
+ } else {
+ sprintf(item2, ".");
+ }
+ }
+ strcat(line1, item1);
+ strcat(line2, item2);
+ }
+
+ strcat(text, line1);
+ strcat(text, "| " );
+ strcat(text, line2);
+ strcat(text, "\n" );
+
+ bytes += (strlen (line1 ) + strlen(line2) + 4);
+ }
+
+ kfree (rdbuff);
+
+ return bytes + 1;
+}
+
+static DEVICE_ATTR(blk, S_IRUGO, blk_show, NULL );
+
+
+static int spiee_probe(struct spi_device *this_dev)
+{
+ device_create_file(&this_dev->dev, &dev_attr_blk);
+ return 0;
+}
+
+static int spiee_remove(struct spi_device *this_dev)
+{
+ device_remove_file(&this_dev->dev, &dev_attr_blk);
+ return 0;
+}
+
+static struct spi_driver eeprom_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ },
+ .probe = spiee_probe,
+ .remove = spiee_remove,
+};
+
+static int __init spi_eeprom_init(void)
+{
+ return spi_driver_add(&eeprom_driver);
+}
+static void __exit spi_eeprom_cleanup(void)
+{
+ spi_driver_del(&eeprom_driver);
+}
+
+module_init(spi_eeprom_init);
+module_exit(spi_eeprom_cleanup);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("dmitry pervushin <[email protected]>");
+MODULE_DESCRIPTION("SPI EEPROM driver");
Index: linux-2.6.orig/drivers/spi/spipnx-init.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spipnx-init.h
@@ -0,0 +1,9 @@
+#include <linux/kernel.h>
+#include <asm/arch/spi.h>
+
+#ifdef CONFIG_ARCH_PNX4008
+struct spipnx_wifi_params spipnx_wifi_params = {
+ .gpio_cs_line = GPIO_03,
+};
+#endif
+
Index: linux-2.6.orig/drivers/spi/spipnx.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spipnx.c
@@ -0,0 +1,488 @@
+/*
+ * drivers/spi/spi-pnx.c
+ *
+ * SPI support for PNX 010x/4008 boards.
+ *
+ * Author: dmitry pervushin <[email protected]>
+ * Based on Dennis Kovalev's <[email protected]> bus driver for pnx010x
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/platform.h>
+#include <linux/spi.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/miscdevice.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/dma-mapping.h>
+#include <asm/io.h>
+#include <asm/hardware.h>
+#include <asm/irq.h>
+#include <asm/uaccess.h>
+#include <asm/dma.h>
+#include <asm/dma-mapping.h>
+#include <asm/hardware/clock.h>
+
+#include "spipnx.h"
+#include "spipnx-init.h"
+
+#define lock_device( dev ) /* down( &dev->sem ); */
+#define unlock_device( dev ) /* up( &dev->sem ); */
+
+#define spipnx_dump_xchg(xxx...) pr_debug( xxx )
+
+
+static int spipnx_spi_init(struct device *bus_device);
+static int spipnx_xfer(struct spi_msg *msg);
+static int spipnx_probe(struct device *bus_device);
+static int spipnx_remove(struct device *bus_device);
+static int spipnx_suspend(struct device *bus_device, pm_message_t);
+static int spipnx_resume(struct device *bus_device);
+static void spipnx_control(struct spi_device *device, int code, u32 ctl);
+static int spipnx_request_hardware(struct spipnx_driver_data *dd,
+ struct platform_device *dev);
+static void spipnx_free_hardware(struct spipnx_driver_data *dd,
+ struct platform_device *dev);
+static void spipnx_set_clock(struct device *bus_device, u32 clock);
+static void spipnx_reset (struct device *bus_device, u32 context );
+
+static struct spi_bus_driver spipnx_driver = {
+ .driver = {
+ .bus = &platform_bus_type,
+ .name = "spipnx",
+ .probe = spipnx_probe,
+ .remove = spipnx_remove,
+ .suspend = spipnx_suspend,
+ .resume = spipnx_resume,
+ },
+ .xfer = spipnx_xfer,
+ .set_clock = spipnx_set_clock,
+ .reset = spipnx_reset,
+};
+
+void spipnx_set_mode(struct device *bus_device, int mode)
+{
+ struct spipnx_driver_data *dd = dev_get_drvdata(bus_device);
+
+ lock_device(bus_device);
+ dd->dma_mode = mode;
+ unlock_device(bus_device);
+}
+
+static void spipnx_set_clock(struct device *bus_device, u32 clock)
+{
+ struct spipnx_driver_data *dd = dev_get_drvdata(bus_device);
+
+ dd->saved_clock = clock;
+ dev_dbg (bus_device, "setting clock speed to %d\n", dd->saved_clock );
+}
+
+static void spipnx_reset (struct device *bus_device, u32 context )
+{
+ struct spipnx_driver_data *dd = dev_get_drvdata(bus_device);
+
+ dd->spi_regs->con = context;
+ dev_dbg (bus_device, "CON set to 0x%08X\n", context );
+}
+
+static int spipnx_probe(struct device *device)
+{
+ struct spipnx_driver_data *dd;
+ int err;
+
+ dd = kzalloc(sizeof(struct spipnx_driver_data), GFP_KERNEL);
+ if (!dd) {
+ err = -ENOMEM;
+ goto probe_fail_1;
+ }
+ /*
+ * these special values are needed because 0 is the valid
+ * assignment to IRQ# and DMA channel as well as DMA slave#
+ */
+ dd->dma_channel = -1;
+ dd->slave_nr = -1;
+ dd->irq = NO_IRQ;
+
+ err = spipnx_request_hardware(dd, to_platform_device(device));
+ if (err)
+ goto probe_fail_2;
+
+ spi_bus_driver_init(&spipnx_driver, device);
+
+ dd->state = SPIPNX_STATE_READY;
+
+ dev_set_drvdata(device, dd);
+
+ spipnx_arch_add_devices( device, to_platform_device(device)->id );
+
+ return 0;
+
+probe_fail_2:
+ spipnx_remove(device);
+probe_fail_1:
+ return err;
+}
+
+int spipnx_remove(struct device *device)
+{
+ struct spipnx_driver_data *dd = dev_get_drvdata(device);
+
+ spi_bus_driver_cleanup(TO_SPI_BUS_DRIVER(device->driver), device);
+ spipnx_free_hardware(dd, to_platform_device(device));
+ kfree(dd);
+ return 0;
+}
+
+static void spipnx_free_hardware(struct spipnx_driver_data *dd,
+ struct platform_device *dev)
+{
+ lock_device(&dev->dev);
+
+ spipnx_clk_stop(dd->clk);
+
+ if (dd->io_flags & IORESOURCE_IRQ) {
+ free_irq(dd->irq, dev->dev.bus_id);
+ dd->io_flags &= ~IORESOURCE_IRQ;
+ }
+ if (dd->io_flags & IORESOURCE_MEM) {
+ /*
+ release_mem_region((unsigned long)dd->iostart,
+ dd->ioend - dd->iostart + 1);
+ */
+ dd->io_flags &= ~IORESOURCE_MEM;
+ }
+ if (dd->io_flags & IORESOURCE_DMA) {
+ spipnx_release_dma(&dd->dma_channel);
+ dd->io_flags &= ~IORESOURCE_DMA;
+ }
+
+ spipnx_clk_free(dd->clk);
+
+#ifdef DEBUG
+ if (dd->io_flags)
+ printk(KERN_ERR
+ "Oops, dd_ioflags for driver data %p is still 0x%x!\n",
+ dd, dd->io_flags);
+#endif
+ unlock_device(&dev->dev);
+}
+
+void spipnx_dma_handler (int channel, int cause, void* context, struct pt_regs* pt_regs )
+{
+#ifdef CONFIG_ARCH_PNX4008
+ struct spipnx_driver_data *dd = context;
+
+ if (cause & DMA_TC_INT )
+ complete( &dd->threshold );
+#endif
+}
+
+int spipnx_request_hardware(struct spipnx_driver_data *dd,
+ struct platform_device *dev)
+{
+ int err = 0;
+ struct resource *rsrc;
+
+ lock_device(&dev->dev);
+ rsrc = platform_get_resource(dev, IORESOURCE_MEM, 0);
+ if (rsrc) {
+ dd->iostart = rsrc->start;
+ dd->ioend = rsrc->end;
+ dd->spi_regs = ioremap(dd->iostart, SZ_4K);
+ /*
+ if (!request_mem_region
+ (dd->iostart, dd->ioend - dd->iostart + 1, dev->dev.bus_id))
+ err = -ENODEV;
+ else
+ */
+ dd->io_flags |= IORESOURCE_MEM;
+ }
+ dd->clk = spipnx_clk_init(dev);
+ if (IS_ERR(dd->clk)) {
+ err = PTR_ERR(dd->clk);
+ goto out;
+ }
+
+ spipnx_clk_start(dd->clk);
+
+ dd->irq = platform_get_irq(dev, 0);
+ if (dd->irq != NO_IRQ) {
+ dd->spi_regs->ier = 0;
+ dd->spi_regs->stat = 0;
+ }
+
+ rsrc = platform_get_resource(dev, IORESOURCE_DMA, 0);
+ if (rsrc) {
+ dd->slave_nr = rsrc->start;
+ err = spipnx_request_dma(&dd->dma_channel, dev->dev.bus_id, spipnx_dma_handler, dd);
+ if (!err)
+ dd->io_flags |= IORESOURCE_DMA;
+ }
+out:
+ unlock_device(&dev.dev);
+ return err;
+}
+
+int __init spipnx_init(void)
+{
+ printk("PNX/SPI bus driver\n");
+ return spi_bus_driver_register(&spipnx_driver);
+}
+
+void __exit spipnx_cleanup(void)
+{
+ spi_bus_driver_unregister(&spipnx_driver);
+}
+
+static int spipnx_spi_init(struct device *bus_device)
+{
+ struct spipnx_driver_data *dd = dev_get_drvdata(bus_device);
+ int timeout;
+ u32 config;
+
+ config = dd->spi_regs->con;
+
+ dd->spi_regs->global = SPIPNX_GLOBAL_RESET_SPI | SPIPNX_GLOBAL_SPI_ON;
+ udelay(10);
+ dd->spi_regs->global = SPIPNX_GLOBAL_SPI_ON;
+ for (timeout = 10000; timeout >= 0; --timeout)
+ if (dd->spi_regs->global & SPIPNX_GLOBAL_SPI_ON)
+ break;
+ if (timeout <= 0)
+ return -1;
+
+ config |= (7<<9);
+ config &= ~(SPIPNX_CON_RXTX);
+ dd->spi_regs->con = config;
+ spipnx_arch_init(dd->saved_clock / 1000, dd->spi_regs);
+ dd->spi_regs->stat |= 0x100;
+ return 0;
+}
+
+void spipnx_control(struct spi_device *device, int type, u32 ctl)
+{
+ spipnx_arch_control(device, type, ctl);
+}
+
+static inline int
+spipnx_xfer_buggy (struct spi_msg *msg)
+{
+ u8* dat;
+ void *buf;
+ int len;
+ struct spi_msg *pmsg;
+ struct spi_device *spidev;
+ struct device *dev;
+ struct spipnx_driver_data *dd;
+ vhblas_spiregs* regs;
+ int i, rc = 0;
+ u32 stat, fc;
+ struct spipnx_dma_transfer_t params;
+ u32 flags = 0;
+
+ if (!msg) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ spidev = spimsg_get_spidev(msg);
+ dev = &spidev->dev;
+ dd = dev_get_drvdata(dev->parent);
+ regs = dd->spi_regs;
+
+ for (pmsg = msg; pmsg; pmsg = spimsg_getnext(pmsg)) {
+ flags = spimsg_get_flags(pmsg);
+ len = spimsg_get_buffer(pmsg, &buf);
+ dat = buf;
+
+ spipnx_control(spidev, MESSAGE_START, flags & SPI_M_CS);
+ if (flags & SPI_M_WR) {
+ regs->con |= SPIPNX_CON_RXTX;
+ regs->ier |= SPIPNX_IER_EOT;
+ regs->con &= ~SPIPNX_CON_SHIFT_OFF;
+ regs->frm = len;
+
+ if (dd->dma_mode && len >= FIFO_CHUNK_SIZE) {
+ params.dma_buffer = dma_map_single(dev->parent, dat, len, DMA_TO_DEVICE);
+ params.saved_ll = NULL;
+ params.saved_ll_dma = 0;
+
+ spipnx_setup_dma(dd->iostart, dd->slave_nr, dd->dma_channel,
+ DMA_MODE_WRITE, dat, len);
+ init_completion (&dd->threshold);
+ spipnx_start_dma(dd->dma_channel);
+ wait_for_completion (&dd->threshold);
+ spipnx_stop_dma(dd->dma_channel, dat);
+ dma_unmap_single(dev->parent, params.dma_buffer, len, DMA_TO_DEVICE);
+ } else {
+ regs->con &= ~SPIPNX_CON_SHIFT_OFF;
+ for (i = 0; i < len ; i ++ ) {
+ while (regs->stat & 0x04)
+ continue;
+ regs->dat = *dat;
+ dat ++;
+ }
+ }
+
+ while ((regs->stat & SPIPNX_STAT_EOT) == 0)
+ continue;
+ }
+
+ if (flags & SPI_M_RD) {
+ int stopped;
+ u8 c;
+
+ regs->con &= ~SPIPNX_CON_RXTX;
+ regs->ier |= SPIPNX_IER_EOT;
+ regs->con &= ~SPIPNX_CON_SHIFT_OFF;
+ regs->frm = len;
+
+ regs->dat = 0; /* kick off read */
+
+ if (dd->dma_mode && len > FIFO_CHUNK_SIZE - 1) {
+ params.dma_buffer = dma_map_single(dev->parent, dat, len - (FIFO_CHUNK_SIZE - 1), DMA_FROM_DEVICE);
+ params.saved_ll = NULL;
+ params.saved_ll_dma = 0;
+
+ spipnx_setup_dma(dd->iostart, dd->slave_nr, dd->dma_channel,
+ DMA_MODE_READ, dat, len - (FIFO_CHUNK_SIZE - 1));
+ init_completion (&dd->threshold);
+ spipnx_start_dma(dd->dma_channel);
+ wait_for_completion (&dd->threshold);
+ spipnx_stop_dma(dd->dma_channel, dat);
+ dma_unmap_single(dev->parent, params.dma_buffer, len - (FIFO_CHUNK_SIZE - 1), DMA_FROM_DEVICE);
+ dat = dat + len - (FIFO_CHUNK_SIZE - 1);
+ len = FIFO_CHUNK_SIZE - 1;
+ }
+
+ stopped = 0;
+ stat = 0;
+ for(i = 0; i < len; ) {
+ unsigned long irq_flags;
+ local_irq_save(irq_flags);
+
+ stat = dd->spi_regs->stat;;
+ fc = dd->spi_regs->frm;
+ /* has hardware finished ? */
+ if(fc == 0) {
+ regs->con |= SPIPNX_CON_SHIFT_OFF;
+ stopped = 1;
+ }
+ /* FIFO not empty and hardware not about to finish */
+ if((!(stat & SPI_STAT_SPI_BE)) && ((fc > 4) || (stopped ))) {
+ *dat++= c = dd->spi_regs->dat;
+ i++;
+ }
+ if((stat & SPI_STAT_SPI_BF) && (!stopped)) {
+ *dat++= c = dd->spi_regs->dat;
+ i++;
+ }
+ local_irq_restore(irq_flags);
+ }
+ }
+ regs->stat |= SPIPNX_STAT_CLRINT;
+ spipnx_control(spidev, MESSAGE_END, flags & SPI_M_CSREL);
+ spimsg_put_buffer(pmsg, buf);
+ }
+out:
+ return rc;
+}
+
+static inline char *spimsg_flags_str(unsigned flags)
+{
+ static char s[4] = "";
+
+ s[0] = 0;
+ if (flags & SPI_M_RD)
+ strcat(s, "R");
+ if (flags & SPI_M_WR)
+ strcat(s, "W");
+ if (flags & ~(SPI_M_WR | SPI_M_RD))
+ strcat(s, "*");
+ return s;
+}
+
+int spipnx_xfer(struct spi_msg *msg)
+{
+ struct spi_device *dev = spimsg_get_spidev(msg);
+ struct spipnx_driver_data *dd = dev_get_drvdata(dev->dev.parent);
+ int status;
+
+ lock_device(dev->parent);
+ if (!SPIPNX_IS_READY(dd)) {
+ status = -EIO;
+ goto unlock_and_out;
+ }
+
+ dev_dbg(&dev->dev, " message (%s)\n",
+ spimsg_flags_str(spimsg_get_flags(msg)));
+
+ if (spipnx_spi_init(dev->dev.parent) < 0) {
+ dev_dbg(&dev->dev, "spi_init failed, skipping the transfer\n");
+ status = -EFAULT;
+ goto unlock_and_out;
+ }
+
+ dd->progress = 1;
+ status = spipnx_xfer_buggy(msg);
+ dd->progress = 0;
+unlock_and_out:
+ unlock_device(dev->parent);
+ return status;
+}
+
+static int spipnx_suspend(struct device *dev, pm_message_t level)
+{
+ int err = 0;
+#ifdef CONFIG_PM
+ struct spipnx_driver_data *dd = dev_get_drvdata(dev);
+
+ lock_device(dev);
+ spipnx_release_dma(&dd->dma_channel); /* any sanity checks, eg using DMA,
+ will be done here */
+ spipnx_clk_stop(dd->clk);
+ dev->power.power_state = level;
+ unlock_device(dev);
+#endif
+ return err;
+}
+
+static int spipnx_resume(struct device *dev)
+{
+ int err = 0;
+#ifdef CONFIG_PM
+ struct spipnx_driver_data *dd = dev_get_drvdata(dev);
+
+ lock_device(dev);
+ spipnx_clk_start(dd->clk);
+ if (dd->slave_nr >= 0)
+ spipnx_request_dma(&dd->dma_channel, dev->bus_id, spipnx_dma_handler, dd);
+ dev->power.power_state = PMSG_ON;
+ unlock_device(dev);
+#endif /* CONFIG_PM */
+ return err;
+}
+
+EXPORT_SYMBOL(spipnx_set_mode);
+MODULE_AUTHOR("dmitry pervushin <[email protected]>");
+MODULE_DESCRIPTION("SPI driver for Philips' PNX boards");
+MODULE_LICENSE("GPL");
+module_init(spipnx_init);
+module_exit(spipnx_cleanup);
Index: linux-2.6.orig/drivers/spi/spipnx.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spipnx.h
@@ -0,0 +1,58 @@
+/*
+ * SPI support for pnx010x.
+ *
+ * Author: Dennis Kovalev <[email protected]>
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef _SPI_PNX_BUS_DRIVER
+#define _SPI_PNX_BUS_DRIVER
+
+#include <asm/arch/platform.h>
+#include <asm/arch/dma.h>
+#include <asm/dma.h>
+#include <asm/arch/spi.h>
+
+/* structures */
+struct spipnx_driver_data {
+ unsigned io_flags;
+
+ int irq;
+
+ int dma_mode;
+ int dma_channel;
+ int slave_nr;
+
+ u32 iostart, ioend;
+ vhblas_spiregs *spi_regs;
+
+ struct clk *clk;
+ int clk_id[4];
+
+ int state;
+ u32 saved_clock;
+
+ struct completion op_complete;
+ struct completion threshold;
+
+ int progress;
+
+ void (*control) (struct spi_device * dev, int code, u32 ctl);
+};
+
+#define SPIPNX_STATE_UNINITIALIZED 0
+#define SPIPNX_STATE_READY 1
+#define SPIPNX_STATE_SUSPENDED 2
+
+#define SPIPNX_IS_READY( bus ) ( (bus)->state == SPIPNX_STATE_READY )
+
+#define FIFO_CHUNK_SIZE 56
+
+#define SPI_ENDIAN_SWAP_NO 0
+#define SPI_ENDIAN_SWAP_YES 1
+
+#endif // __SPI_PNX_BUS_DRIVER

2005-12-12 15:27:23

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH 2.6-git 4/4] SPI core refresh: dumb EEPROM driver

Signed-off-by: Dmitry Pervushin <[email protected]>
Signed-off-by: Vitaly Wool <[email protected]>

pnx4008-eeprom.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 121 insertions(+)

Index: linux-2.6.orig/drivers/spi/pnx4008-eeprom.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/pnx4008-eeprom.c
@@ -0,0 +1,121 @@
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/device.h>
+#include <linux/spi.h>
+#include <linux/proc_fs.h>
+#include <linux/ctype.h>
+
+#include "spipnx.h"
+
+#define EEPROM_SIZE 256
+#define DRIVER_NAME "EEPROM"
+#define READ_BUFF_SIZE 160
+
+static int __init spi_eeprom_init(void);
+static void __exit spi_eeprom_cleanup(void);
+
+static int spiee_read_block (struct device *d, void *block)
+{
+ struct spi_device *device = TO_SPI_DEV (d);
+ char cmd[2];
+ struct spi_msg *msg = spimsg_alloc(device,
+ SPI_M_CS|SPI_M_CSREL,
+ NULL,
+ 0,
+ NULL);
+ struct spi_msg *msg_cmd = spimsg_chain(msg, SPI_M_CS|SPI_M_WR|SPI_M_DMAUNSAFE, cmd, 2, NULL);
+ spimsg_chain(msg_cmd, SPI_M_RD|SPI_M_CSREL, block, 256, NULL);
+
+ cmd[ 0 ] = 0x03;
+ cmd[ 1 ] = 0x00;
+
+ spimsg_set_clock(msg, 2000000); /* 2 MHz */
+ spi_transfer(msg, NULL);
+ spimsg_free(msg);
+ return 256;
+}
+static ssize_t blk_show (struct device *d, struct device_attribute *attr, char *text )
+{
+ char *rdbuff = kmalloc (256, SLAB_KERNEL);
+ char line1[80],line2[80];
+ char item1[5], item2[5];
+ int bytes, i, x, blen;
+
+ blen = spiee_read_block (d, rdbuff);
+
+ bytes = 0;
+
+ strcpy(text, "");
+ for (i = 0; i < blen; i += 8) {
+ strcpy(line1, "");
+ strcpy(line2, "" );
+ for (x = i; x < i + 8; x++) {
+ if (x > blen) {
+ sprintf(item1, " ");
+ sprintf(item2, " " );
+ } else {
+ sprintf(item1, "%02x ", rdbuff[x]);
+ if (isprint(rdbuff[x])) {
+ sprintf(item2, "%c", rdbuff[x]);
+ } else {
+ sprintf(item2, ".");
+ }
+ }
+ strcat(line1, item1);
+ strcat(line2, item2);
+ }
+
+ strcat(text, line1);
+ strcat(text, "| " );
+ strcat(text, line2);
+ strcat(text, "\n" );
+
+ bytes += (strlen (line1 ) + strlen(line2) + 4);
+ }
+
+ kfree (rdbuff);
+
+ return bytes + 1;
+}
+
+static DEVICE_ATTR(blk, S_IRUGO, blk_show, NULL );
+
+
+static int spiee_probe(struct spi_device *this_dev)
+{
+ device_create_file(&this_dev->dev, &dev_attr_blk);
+ return 0;
+}
+
+static int spiee_remove(struct spi_device *this_dev)
+{
+ device_remove_file(&this_dev->dev, &dev_attr_blk);
+ return 0;
+}
+
+static struct spi_driver eeprom_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ },
+ .probe = spiee_probe,
+ .remove = spiee_remove,
+};
+
+static int __init spi_eeprom_init(void)
+{
+ return spi_driver_add(&eeprom_driver);
+}
+static void __exit spi_eeprom_cleanup(void)
+{
+ spi_driver_del(&eeprom_driver);
+}
+
+module_init(spi_eeprom_init);
+module_exit(spi_eeprom_cleanup);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("dmitry pervushin <[email protected]>");
+MODULE_DESCRIPTION("SPI EEPROM driver");

2005-12-12 15:49:42

by Russell King

[permalink] [raw]
Subject: Re: [PATCH 2.6-git 1/4] SPI core refresh: SPI core patch

Only two comments this time.

On Mon, Dec 12, 2005 at 06:22:49PM +0300, Vitaly Wool wrote:
> +static int spi_bus_suspend(struct device * dev, pm_message_t message)
> +{
> + int ret = 0;
> +
> + if (dev && dev->driver && TO_SPI_DRIVER(dev->driver)->suspend ) {

dev will always be non-NULL here.

> +static int spi_bus_resume(struct device * dev)
> +{
> + int ret = 0;
> +
> + if (dev && dev->driver && TO_SPI_DRIVER(dev->driver)->suspend ) {

Ditto.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-12-12 18:03:00

by Rui Sousa

[permalink] [raw]
Subject: Re: [PATCH 2.6-git 0/4] SPI core refresh

On Mon, 2005-12-12 at 18:20 +0300, Vitaly Wool wrote:
> Greetings,

Hi,

> this message fill be followed by the following four ones:
> 1) updated SPI core from Dmitry Pervushin/Vitaly Wool
> 2) Atmel MTD dataflash driver port for this core
> 3) SPI controller driver for Philips SPI controller
> 4) dumb EEPROM driver for EEPROM chip on SPI bus
>
> This SPI core features:
> * multiple SPI controller support
> * multiple devices on the same bus support
> * DMA support
> * synchronous and asynchronous transfers
> * library for asynchronous transfers on the bus using kernel threads
> * character device interface
> * custom lightweight SPI message allocation mechanism

[snipped]

One problem I still have with this implementation (well, if I remember
correctly David's has the same problem) is that it's not possible to
read/write from/to the SPI bus in interrupt context.

How do you handle IRQ's generated by a SPI device (e.g ack the
interrupt, check if it was the SPI device that generated the
interrupt, ...) if you can't read/write on the SPI bus from interrupt
context?


Rui

2005-12-13 12:18:39

by dmitry pervushin

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> How do you handle IRQ's generated by a SPI device (e.g ack the
> interrupt, check if it was the SPI device that generated the
> interrupt, ...) if you can't read/write on the SPI bus from interrupt
> context?
Hmm... what do you mean by "cannot read/write" ? Normally you can
write/read registers in interrupt context, and then set the
flag/complete the completion/what else ?
In other words, could you please share the code that causes problems ?

--
cheers, dmitry pervushin

2005-12-13 14:06:01

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH/RFC] SPI: add async message handing library to David Brownell's core

Greetings,

as a part of the convergence process between two SPI cores (one is developed by David Brownell and the other one -- by Dmitry Pervushin and me) I'd like to post here one more result of mindwork :). Despite our previous activities of that kind which were more related to porting drivers from one core to the other, this is a port of the library for handling async SPI messages using kernel threads, one per each SPI controller, from our core to David's.

This thingie hasn't been thoroughly tested yet, but it's lightweight and easy to understand so I don't think solving the problems that may arise will take long. Though I haven't actually done that yet, I'm sure that Stephen's PXA SSP driver will become easier to understand and less in footprint and will work faster when it's rewritten using this library. (Yes, I do expect performance improvement here as the current implementation schedules multiple tasklets, so scheduling penalty is high.)

This patch performes just minimal intrusion to the SPI core itself, adding a 'void *context' field to the spi_master structure which seems to be a good thing anyway.

drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 3
drivers/spi/spi-thread.c | 198 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi-thread.h | 27 +++++
include/linux/spi/spi.h | 3
5 files changed, 240 insertions(+)

Signed-off-by: Vitaly Wool <[email protected]>

Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Kconfig
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -13,6 +13,7 @@ config SPI_ARCH_HAS_MASTER
default y if ARCH_AT91
default y if ARCH_OMAP
default y if ARCH_PXA
+ default y if ARCH_PNX4008
default y if X86 # devel hack only!! (ICH7 can...)

config SPI_ARCH_HAS_SLAVE
@@ -63,6 +64,14 @@ config SPI_MASTER
controller and the protocol drivers for the SPI slave chips
that are connected.

+config SPI_THREAD
+ boolean "Library for threaded asynchronous message processing"
+ depends on SPI_MASTER
+ help
+ Choose this if you want to use thread-based asynchronous
+ message processing library. Using kernel threads for
+ asynchronous data processing is the most common option.
+
comment "SPI Master Controller Drivers"
depends on SPI_MASTER

Index: linux-2.6.orig/drivers/spi/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Makefile
+++ linux-2.6.orig/drivers/spi/Makefile
@@ -10,6 +10,9 @@ endif
# config declarations into driver model code
obj-$(CONFIG_SPI_MASTER) += spi.o

+# thread-based async message handling library
+obj-$(CONFIG_SPI_THREAD) += spi-thread.o
+
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
# ... add above this line ...
Index: linux-2.6.orig/drivers/spi/spi-thread.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.c
@@ -0,0 +1,198 @@
+/*
+ * drivers/spi/spi-thread.c
+ *
+ * Authors:
+ * Vitaly Wool <[email protected]>
+ * Dmitry Pervushin <[email protected]>
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi/spi.h>
+#include <asm/atomic.h>
+
+static int spi_thread(void *);
+static int spi_queue(struct spi_device *, struct spi_message *);
+
+struct threaded_async_data {
+ atomic_t exiting;
+ struct spi_master *master;
+ struct task_struct *thread;
+ wait_queue_head_t wq;
+ struct list_head msgs;
+ struct semaphore lock;
+ int (*xfer) (struct spi_master *, struct spi_message *);
+};
+
+/**
+ * spi_start_async - start the thread
+ * @master: SPI controller structure which the thread is related to
+ * @return: abstract pointer to the thread context
+ */
+int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ struct threaded_async_data *td = kzalloc (sizeof (struct threaded_async_data), GFP_KERNEL);
+ int rc = 0;
+
+ if (!td) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if (master->context) {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ td->master = master;
+ atomic_set(&td->exiting, 0);
+ td->thread = kthread_run(spi_thread, td, "%s-work", master->cdev.dev->bus_id);
+ init_waitqueue_head(&td->wq);
+ init_MUTEX(&td->lock);
+ INIT_LIST_HEAD(&td->msgs);
+ master->transfer = spi_queue;
+ td->xfer = xfer;
+ master->context = td;
+
+out:
+ return rc;
+}
+EXPORT_SYMBOL_GPL(spi_start_async);
+
+/**
+ * spi_stop_async - stop the thread
+ * @master: SPI controller structure which the thread is related to
+ */
+void spi_stop_async (struct spi_master *master)
+{
+ struct threaded_async_data *td = master->context;
+
+ if (td) {
+ /* TODO: free messages, if any */
+ atomic_inc (&td->exiting);
+ kthread_stop(td->thread);
+ kfree(td);
+ master->context = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(spi_stop_async);
+
+/**
+ * spi_thread_awake - function that called to determine if thread needs to process any messages
+ * @td: pointer to struct threaded_async_data
+ * Description:
+ * Thread wakes up if there is signal to exit (bd->exiting is set)
+ * or there are any messages in bus' queue.
+ */
+static int spi_thread_awake(struct threaded_async_data *td)
+{
+ int ret = -EINVAL;
+
+ if (!td || atomic_read(&td->exiting)) {
+ goto out;
+ }
+
+ down(&td->lock);
+ ret = !list_empty(&td->msgs);
+ up(&td->lock);
+out:
+ return ret;
+}
+
+/**
+ * spi_get_next_msg - retrieve the next message
+ * @data: pointer to threaded_async_data structure associated with
+ * SPI controller thread
+ */
+static inline struct spi_message *spi_get_next_msg(struct threaded_async_data *data)
+{
+ return list_entry(data->msgs.next, struct spi_message, queue);
+}
+
+/**
+ * spi_thread - the thread that calls bus functions to perform actual transfers
+ * @context: pointer to struct spi_bus_data with bus-specific data
+ * Description:
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ */
+static int spi_thread(void *context)
+{
+ struct threaded_async_data *td = context;
+ struct spi_message *message = NULL;
+
+ while (!kthread_should_stop()) {
+
+ wait_event_interruptible(td->wq, spi_thread_awake(td));
+
+ if (atomic_read(&td->exiting))
+ goto thr_exit;
+
+ down(&td->lock);
+ while (!list_empty(&td->msgs)) {
+ /*
+ * this part is locked by td->lock,
+ * to protect spi_message extraction
+ */
+ message = spi_get_next_msg(td);
+ list_del (&message->queue);
+
+ up(&td->lock);
+
+ message->status = td->xfer(td->master, message);
+ if (message->complete)
+ message->complete(context);
+
+ /* lock the data again... */
+ down(&td->lock);
+ }
+ up(&td->lock);
+ }
+
+thr_exit:
+ return 0;
+}
+
+/**
+ * spi_queue - (internal) queue the message to be processed asynchronously
+ * @spi: SPI device to perform transfer to/from
+ * @msg: message to be sent
+ * Description:
+ * This function queues the message to SPI controller's queue.
+ */
+static int spi_queue(struct spi_device *spi, struct spi_message *msg)
+{
+ struct threaded_async_data *td = spi->master->context;
+ int rc = 0;
+
+ if (!td) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ msg->spi = spi;
+ down(&td->lock);
+ list_add_tail(&msg->queue, &td->msgs);
+ dev_dbg(spi->dev.parent, "message has been queued\n");
+ up(&td->lock);
+ wake_up_interruptible(&td->wq);
+
+out:
+ return rc;
+}
+
Index: linux-2.6.orig/include/linux/spi/spi-thread.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi/spi-thread.h
@@ -0,0 +1,27 @@
+/*
+ * linux/drivers/spi/spi-thread.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_THREAD_H
+#define __SPI_THREAD_H
+
+
+#if defined (CONFIG_SPI_THREAD)
+extern int spi_start_async (struct spi_master *, int (*)(struct spi_master *, struct spi_message *));
+extern void spi_stop_async (struct spi_master *);
+#else
+static inline int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ return -EINVAL;
+}
+static inline void spi_stop_async (struct spi_master *master)
+{
+}
+#endif /* CONFIG_SPI_THREAD */
+#endif /* __SPI_THREAD_H */
Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -152,6 +152,7 @@ static inline void spi_unregister_driver
* device's SPI controller; protocol code may call this.
* @transfer: adds a message to the controller's transfer queue.
* @cleanup: frees controller-specific state
+ * @context: controller-specific data
*
* Each SPI master controller can communicate with one or more spi_device
* children. These make a small bus, sharing MOSI, MISO and SCK signals
@@ -207,6 +208,8 @@ struct spi_master {

/* called on release() to free memory provided by spi_master */
void (*cleanup)(const struct spi_device *spi);
+
+ void *context;
};

/* the spi driver core manages memory for the spi_master classdev */

2005-12-13 16:37:55

by David Brownell

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

> On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > How do you handle IRQ's generated by a SPI device (e.g ack the
> > interrupt, check if it was the SPI device that generated the
> > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > context?

I don't understand the question. The SPI controller may be busy
handling transactions for a different device, so the best you
could do from _any_ IRQ handler is to queue some messages that
will get to the IRQ-issuing device sometime later. It's not
possible to make the other transactions finish any faster, or
abort them safely mid-transfer.

The way the ads7846 driver handles it, for example, is to issue
a transaction collecting a bunch of touchscreen sensor readings.
And disable its (nonsharable) irq, since the interrupt will be
raised for some time and there's no portable way for Linux to
force the IRQ to trigger only on the relevant edge.

See the 2.6.15-rc5-mm1 patches...

- Dave

2005-12-13 16:52:52

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greetings,

this patch removes nasty buffer allocation in spi core for sync message transfers. (And thus solves the problem of possible priority inversion I've pointed out before).
The write_then_read function sets the flags singalling for buffers not being DMA-safe and it's absolutely up to the controller whatta do with that.
I. e. it will prolly allocate/copy if it performs DMA transfers or just don't do anything special if it's working in PIO. It can even use a single DMA-safe buffer as David's core did (although I wouldn't encourage anyone to do that! :))
This is of coure more flexible and also more optimal as it will *not* require _redundant_ memcpy's and semaphores protection in case of PIO transfers.
Please note that this is a key patch for us in some aspects, although it's rather small! :)

Signed-off-by: Vitaly Wool <[email protected]>
Signed-off-by: Dmitry Pervushin <[email protected]>

drivers/spi/spi.c | 35 ++++++-----------------------------
include/linux/spi/spi.h | 7 +++++++
2 files changed, 13 insertions(+), 29 deletions(-)

Index: linux-2.6.orig/drivers/spi/spi.c
===================================================================
--- linux-2.6.orig.orig/drivers/spi/spi.c
+++ linux-2.6.orig/drivers/spi/spi.c
@@ -507,10 +507,6 @@ int spi_sync(struct spi_device *spi, str
}
EXPORT_SYMBOL_GPL(spi_sync);

-#define SPI_BUFSIZ (SMP_CACHE_BYTES)
-
-static u8 *buf;
-
/**
* spi_write_then_read - SPI synchronous write followed by read
* @spi: device with which data will be exchanged
@@ -532,39 +528,28 @@ int spi_write_then_read(struct spi_devic
const u8 *txbuf, unsigned n_tx,
u8 *rxbuf, unsigned n_rx)
{
- static DECLARE_MUTEX(lock);
-
int status;
struct spi_message message;
struct spi_transfer x[2];

- /* Use preallocated DMA-safe buffer. We can't avoid copying here,
- * (as a pure convenience thing), but we can keep heap costs
- * out of the hot path.
- */
- if ((n_tx + n_rx) > SPI_BUFSIZ)
- return -EINVAL;
-
- down(&lock);
memset(x, 0, sizeof x);
+ memset(&message, 0, sizeof message);

- memcpy(buf, txbuf, n_tx);
- x[0].tx_buf = buf;
+ x[0].tx_buf = tx_buf;
x[0].len = n_tx;
+ x[0].tx_dma_unsafe = 1;

- x[1].rx_buf = buf + n_tx;
+ x[1].rx_buf = rx_buf;
x[1].len = n_rx;
+ x[1].rx_dma_unsafe = 1;

/* do the i/o */
message.transfers = x;
message.n_transfer = ARRAY_SIZE(x);
status = spi_sync(spi, &message);
- if (status == 0) {
- memcpy(rxbuf, x[1].rx_buf, n_rx);
+ if (status == 0)
status = message.status;
- }

- up(&lock);
return status;
}
EXPORT_SYMBOL_GPL(spi_write_then_read);
@@ -575,12 +560,6 @@ static int __init spi_init(void)
{
int status;

- buf = kmalloc(SPI_BUFSIZ, SLAB_KERNEL);
- if (!buf) {
- status = -ENOMEM;
- goto err0;
- }
-
status = bus_register(&spi_bus_type);
if (status < 0)
goto err1;
@@ -593,9 +572,7 @@ static int __init spi_init(void)
err2:
bus_unregister(&spi_bus_type);
err1:
- kfree(buf);
buf = NULL;
-err0:
return status;
}

Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -248,6 +248,10 @@ extern struct spi_master *spi_busnum_to_
* @rx_dma: DMA address of buffer, if spi_message.is_dma_mapped
* @len: size of rx and tx buffers (in bytes)
* @cs_change: affects chipselect after this transfer completes
+ * @tx_dma_unsafe: signals to controller driver that the buffer
+ * is not DMA-safe
+ * @rx_dma_unsafe: signals to controller driver that the buffer
+ * is not DMA-safe
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
* the next transfer or completing this spi_message.
@@ -288,6 +292,9 @@ struct spi_transfer {
dma_addr_t rx_dma;

unsigned cs_change:1;
+ unsigned rx_dma_unsafe:1;
+ unsigned tx_dma_unsafe:1;
+
u16 delay_usecs;
};

2005-12-13 16:57:56

by Rui Sousa

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > How do you handle IRQ's generated by a SPI device (e.g ack the
> > interrupt, check if it was the SPI device that generated the
> > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > context?
> Hmm... what do you mean by "cannot read/write" ? Normally you can
> write/read registers in interrupt context

The registers I want to read are from a SPI device (a SPI slave attached
to a SPI bus). I need to use SPI bus transfers to access them.

> , and then set the
> flag/complete the completion/what else ?

If I read the API correctly reading/writing a byte from the SPI bus
(synchronously) always implies putting the task doing the read to sleep:

int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
*, int))
{

...
err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
wait_for_completion(&msg->sync);
...
}

So, how can I, from an interrupt handler, read/write a couple of bytes
from my SPI device using this API?

> In other words, could you please share the code that causes problems ?
>

Rui

2005-12-13 17:15:34

by dmitry pervushin

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

On Tue, 2005-12-13 at 16:11 +0100, Rui Sousa wrote:
> On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> > On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > > How do you handle IRQ's generated by a SPI device (e.g ack the
> > > interrupt, check if it was the SPI device that generated the
> > > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > > context?
> > Hmm... what do you mean by "cannot read/write" ? Normally you can
> > write/read registers in interrupt context
>
I expect the answer from David Brownell too; and, because both discussed
frameworks are going to be the only (and the best), please also pay
attention to his (possible) answer. But I'd comment your code :)
> If I read the API correctly reading/writing a byte from the SPI bus
> (synchronously) always implies putting the task doing the read to sleep:
>
> int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
> *, int))
> {
>
> ...
> err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> wait_for_completion(&msg->sync);
> ...
> }
>
> So, how can I, from an interrupt handler, read/write a couple of bytes
> from my SPI device using this API?
You can issue the request (in terms of core that you are using, queue
the message) in interrupt context, and perform the rest of processing in
`status' callback. It will be called when message is processed. You can
ack the irq here, and continue your processing.


2005-12-13 18:03:26

by Rui Sousa

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

On Tue, 2005-12-13 at 08:35 -0800, David Brownell wrote:
> > On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > > How do you handle IRQ's generated by a SPI device (e.g ack the
> > > interrupt, check if it was the SPI device that generated the
> > > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > > context?
>
> I don't understand the question. The SPI controller may be busy
> handling transactions for a different device,

If you provide an async API to start with... if everything is sync
then this problem goes away[1]. When I started writing the SPI layer for
my platform (before any SPI patches started circulating) I was going
to use an async API and then decided against it. For the type of device
I'm using, a SLIC device, where only control data is exchanged on the
SPI bus (read register, check some bit, write register, ...) and very
small amounts of data are transferred at a time (4 bytes maximum) there
is really no point to an async API.

Once you give the possibility to transfer data asynchronously on the SPI
bus, and even if you add a pseudo sync interface, you loose the
possibility to call the transfer function (synchronously) from hard/soft
interrupt context. One way or the other you will eventually need to
sleep.

This forces a simple code like:

u8 device_read_reg(int reg)
{
u8 buf;

buf = reg;
spi_write(dev, &buf, 1);

spi_read(dev, &buf, 1);

return buf[0];
}

to be moved to user context or to become much more complex (state
machine + async callbacks).

I guess what I'm trying to say is that for a simple device like the one
I'm using, having an async API adds more trouble that what it's worth
it. Unfortunately I don't have a better solution to put forward.

> so the best you
> could do from _any_ IRQ handler is to queue some messages that
> will get to the IRQ-issuing device sometime later.
> It's not
> possible to make the other transactions finish any faster, or
> abort them safely mid-transfer.
>
> The way the ads7846 driver handles it, for example, is to issue
> a transaction collecting a bunch of touchscreen sensor readings.
> And disable its (nonsharable) irq,

nonsharable is the magic word. I thought of this solution, leaving the
interrupt handler with the interrupt masked and do all the handling in
a workqueue... but my GPIO pin used to generate the interrupt is being
shared with another device.

> since the interrupt will be
> raised for some time and there's no portable way for Linux to
> force the IRQ to trigger only on the relevant edge.
>
> See the 2.6.15-rc5-mm1 patches...
>
> - Dave
>
>

Rui

[1] I know this is not a reasonable solution for a number of cases.


2005-12-13 19:09:51

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Tuesday 13 December 2005 8:53 am, Vitaly Wool wrote:
>
> this patch removes nasty buffer allocation in spi core for sync message transfers.

That's not "core" in the normal "everyone must use this" sense.
Even the folk that do use synchronous transfers aren't always
going to use that particular codepath.

Neither is it "remove" in the normal sense either. The hot
path never had an allocation before ... but it could easily
have one now, because that sort of bounce-buffer semantic is
what a DMA_UNSAFE flag demands from the lower levels. (That's
a key part part of the proposed change that's not included in
this patch... making the chage look much smaller.)


How much of the reason you're arguing for this is because you
have that WLAN stack that does some static allocation for I/O
buffers? Changing that to use dynamic allocation -- the more
usual style -- should be easy. But instead, you want all code
in the SPI stack to need to worry about two different kinds of
I/O memory: the normal stuff, and the DMA-unsafe kind. Not
just this WLAN code which for some reason started out using
a nonportable scheme for allocating I/O buffers.

It'd take a lot more persuasion to make me think that's a good
idea. That's the kind of subtle confusion that really promotes
hard-to-find bugs in drivers, and lots of developer confusion.
And all that to support a new less-portable I/O buffer model...

It's way better to just insist that all I/O buffers (in all
generic APIs) be DMA-safe. AFAICT that's a pretty standard
rule everywhere in Linux.

- Dave

2005-12-13 19:16:03

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote:
> It's way better to just insist that all I/O buffers (in all
> generic APIs) be DMA-safe. AFAICT that's a pretty standard
> rule everywhere in Linux.

I agree.

greg k-h

2005-12-13 21:48:13

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>Neither is it "remove" in the normal sense either. The hot
>path never had an allocation before ... but it could easily
>have one now, because that sort of bounce-buffer semantic is
>what a DMA_UNSAFE flag demands from the lower levels. (That's
>a key part part of the proposed change that's not included in
>this patch... making the chage look much smaller.)
>
>
Makes no sense to me, sorry. What 'not included' changes are you talking
about?

>
>How much of the reason you're arguing for this is because you
>have that WLAN stack that does some static allocation for I/O
>buffers? Changing that to use dynamic allocation -- the more
>usual style -- should be easy. But instead, you want all code
>in the SPI stack to need to worry about two different kinds of
>I/O memory: the normal stuff, and the DMA-unsafe kind. Not
>
>
Err, this change also allows to get rid of ugly stuff in write_then_read.
It also allows to keep track of memory allocation in SPI controller
driver withough hacky tricks.
And... it's not *all* the code; if it doesn't provide such means, it's
also fine.

>just this WLAN code which for some reason started out using
>a nonportable scheme for allocating I/O buffers.
>
>
I'm afraid I just can't understand what you mean by 'portable' here.

>It'd take a lot more persuasion to make me think that's a good
>idea. That's the kind of subtle confusion that really promotes
>hard-to-find bugs in drivers, and lots of developer confusion.
>
>
What hard-to-find bugs, I wonder?
And... I'm afraid that your core is unacceptable for us w/o the changes
proposed.

Vitaly

2005-12-13 22:15:29

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>That's not "core" in the normal "everyone must use this" sense.
>Even the folk that do use synchronous transfers aren't always
>going to use that particular codepath.
>
>
Lemme explain once more why what you have is a lot worse than what I
suggest.
Take for instance spi_w8r8 function used in lotsa places in the drivers
you and Stephen have posted.
This function has a) *implicit memcpy* inherited from
spi_write_then_read b) *implicit priority inversion* inherited from the
same place.
On the other hand, any smart enough SPI controller driver wouldn't ever
want to send and receive one byte via DMA, especially chaining messages
because the overhead will be great; memcpy's also adding overhead. It's
gonna use PIO instead.
So this is a serious reason to let controller driver decide things
related to memory allocation. It doesn't concern only static-allocated
buffers but also, say, stack-allocated (used in your core in many
places). And controller driver needs something to track whether it has
allocated a buffer for transfer for DMA safety or not. Don't you want a
linked list of allocated buffers in the controller driver?! And the
flag I'm adding within the patch does that with minimal intrusion to
your core.

Hope that helps understand my reasons.

Vitaly

2005-12-14 06:57:44

by Vitaly Wool

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

Looks like we'll have to remove explicit wait_for_completion from
spi_trasnfer to make it (potentially) work.
Rui, your turn will then be
a) not to use thread async message handling which is the default but
b) use some kind of PIO method for reading from the SPI bus.

The other solution which looks preferable to me is to have a pretty
simple interrupt handler which just wakes up a thread which in turn
reads what it wants not caring much about sleeps (as it's not gonna be
interrupt context any more).

Vitaly

Rui Sousa wrote:

>On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
>
>
>>On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
>>
>>
>>>How do you handle IRQ's generated by a SPI device (e.g ack the
>>>interrupt, check if it was the SPI device that generated the
>>>interrupt, ...) if you can't read/write on the SPI bus from interrupt
>>>context?
>>>
>>>
>>Hmm... what do you mean by "cannot read/write" ? Normally you can
>>write/read registers in interrupt context
>>
>>
>
>The registers I want to read are from a SPI device (a SPI slave attached
>to a SPI bus). I need to use SPI bus transfers to access them.
>
>
>
>>, and then set the
>>flag/complete the completion/what else ?
>>
>>
>
>If I read the API correctly reading/writing a byte from the SPI bus
>(synchronously) always implies putting the task doing the read to sleep:
>
>int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
>*, int))
>{
>
>...
> err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> wait_for_completion(&msg->sync);
>...
>}
>
>So, how can I, from an interrupt handler, read/write a couple of bytes
>from my SPI device using this API?
>
>
>
>>In other words, could you please share the code that causes problems ?
>>
>>
>>
>
>Rui
>
>
>
>

2005-12-14 13:50:41

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greg KH wrote:

>On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote:
>
>
>>It's way better to just insist that all I/O buffers (in all
>>generic APIs) be DMA-safe. AFAICT that's a pretty standard
>>rule everywhere in Linux.
>>
>>
>
>I agree.
>
>
Well, why then David doesn't insist on that in his own code?
His synchronous transfer functions are allocating transfer buffers on
stack which is not DMA-safe.
Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
Why isn't he just kmalloc'ing/kfree'ing buffers each time these
functions are called (as he proposes for upper layer drivers to do)?
That's a significant inconsistency. Is it also the thing you agree with?

And they way he does it implies redundant memcpy's and kmalloc's:
suppose we have two controller drivers working in two threads and
calling write_then_read in such a way that the one called later has to
allocate a new buffer. Suppose also that both controller drivers are
working in *PIO* mode. In this situation you have one redundant kmalloc
and two redundant memcpy's, not speaking about overhead brought up by
mutexes.

The thing is that only controller driver is aware whether DMA is needed
or not, so it's controller driver that should work it out.
Requesting all the buffers to be DMA-safe will make a significant
performance drop on all small transfers!

Vitaly

2005-12-14 14:28:47

by Rui Sousa

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH 2.6-git 0/4] SPI core refresh

On Wed, 2005-12-14 at 09:57 +0300, Vitaly Wool wrote:
> Looks like we'll have to remove explicit wait_for_completion from
> spi_trasnfer to make it (potentially) work.
> Rui, your turn will then be
> a) not to use thread async message handling which is the default but
> b) use some kind of PIO method for reading from the SPI bus.

This is what I'm doing now (sync + PIO). In any case the SPI bus
controller I'm using only supports PIO.

> The other solution which looks preferable to me is to have a pretty
> simple interrupt handler which just wakes up a thread which in turn
> reads what it wants not caring much about sleeps (as it's not gonna be
> interrupt context any more).

The main problem is that just to be able to ack the interrupt I need to
write on the SPI bus. So now I need to (1) modify my GPIO interrupt
handler so that it let's me exit before acking the interrupt on the
device (2) no longer share this GPIO with other devices interrupts...

It's possible to use the proposed API, it's just that in my case it will
increase the SPI slave driver complexity.

> Vitaly

Rui

> Rui Sousa wrote:
>
> >On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> >
> >
> >>On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> >>
> >>
> >>>How do you handle IRQ's generated by a SPI device (e.g ack the
> >>>interrupt, check if it was the SPI device that generated the
> >>>interrupt, ...) if you can't read/write on the SPI bus from interrupt
> >>>context?
> >>>
> >>>
> >>Hmm... what do you mean by "cannot read/write" ? Normally you can
> >>write/read registers in interrupt context
> >>
> >>
> >
> >The registers I want to read are from a SPI device (a SPI slave attached
> >to a SPI bus). I need to use SPI bus transfers to access them.
> >
> >
> >
> >>, and then set the
> >>flag/complete the completion/what else ?
> >>
> >>
> >
> >If I read the API correctly reading/writing a byte from the SPI bus
> >(synchronously) always implies putting the task doing the read to sleep:
> >
> >int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
> >*, int))
> >{
> >
> >...
> > err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> > wait_for_completion(&msg->sync);
> >...
> >}
> >
> >So, how can I, from an interrupt handler, read/write a couple of bytes
> >from my SPI device using this API?
> >
> >
> >
> >>In other words, could you please share the code that causes problems ?
> >>
> >>
> >>
> >
> >Rui
> >
> >
> >
> >
>
>


2005-12-14 17:04:01

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Tuesday 13 December 2005 2:15 pm, Vitaly Wool wrote:

> Take for instance spi_w8r8 function used in lotsa places in the drivers
> you and Stephen have posted.
> This function has a) *implicit memcpy* inherited from
> spi_write_then_read b) *implicit priority inversion* inherited from the
> same place.

No, (a) is explicit, along with comments not to use that family of
calls when such things matter more than the convenience of those
RPC-ish calls. And (b) was fixed a small patch, now merged.

- Dave

2005-12-14 17:24:12

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>On Tuesday 13 December 2005 2:15 pm, Vitaly Wool wrote:
>
>
>
>>Take for instance spi_w8r8 function used in lotsa places in the drivers
>>you and Stephen have posted.
>>This function has a) *implicit memcpy* inherited from
>>spi_write_then_read b) *implicit priority inversion* inherited from the
>>same place.
>>
>>
>
>No, (a) is explicit, along with comments not to use that family of
>calls when such things matter more than the convenience of those
>RPC-ish calls. And (b) was fixed a small patch, now merged.
>
>
Hmm. Why not use a) when it's convenient? You're placing an artificial
requirement then.
Yep, saw patch for b). One more kmalloc introduces which can be avoided
easily with my patch ;)

Vitaly

2005-12-14 17:24:13

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Wed, Dec 14, 2005 at 04:50:03PM +0300, Vitaly Wool wrote:
> Greg KH wrote:
>
> >On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote:
> >
> >
> >>It's way better to just insist that all I/O buffers (in all
> >>generic APIs) be DMA-safe. AFAICT that's a pretty standard
> >>rule everywhere in Linux.
> >>
> >>
> >
> >I agree.
> >
> >
> Well, why then David doesn't insist on that in his own code?

Heh, I don't know, only David can answer that :)

> His synchronous transfer functions are allocating transfer buffers on
> stack which is not DMA-safe.
> Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
> Why isn't he just kmalloc'ing/kfree'ing buffers each time these
> functions are called (as he proposes for upper layer drivers to do)?
> That's a significant inconsistency. Is it also the thing you agree with?
>
> And they way he does it implies redundant memcpy's and kmalloc's:
> suppose we have two controller drivers working in two threads and
> calling write_then_read in such a way that the one called later has to
> allocate a new buffer. Suppose also that both controller drivers are
> working in *PIO* mode. In this situation you have one redundant kmalloc
> and two redundant memcpy's, not speaking about overhead brought up by
> mutexes.
>
> The thing is that only controller driver is aware whether DMA is
> needed or not, so it's controller driver that should work it out.
> Requesting all the buffers to be DMA-safe will make a significant
> performance drop on all small transfers!

What is the speed of your SPI bus?

And what are your preformance requirements?

thanks,

greg k-h

2005-12-14 17:40:59

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Wednesday 14 December 2005 5:50 am, Vitaly Wool wrote:
> >>It's way better to just insist that all I/O buffers (in all
> >>generic APIs) be DMA-safe. AFAICT that's a pretty standard
> >>rule everywhere in Linux.
> >
> >I agree.
>
> Well, why then David doesn't insist on that in his own code?
> His synchronous transfer functions

You seem to be referring to one non-generic function that's
documented as OK to pass DMA-unsafe buffers to. There are
several other synchronous transfer calls that don't make
such guarantees.


> are allocating transfer buffers on
> stack which is not DMA-safe.

I think the very first version did that, but nothing since
has taken that shortcut. (Several months now.) It uses
a buffer that's allocated on the heap.


> Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
> Why isn't he just kmalloc'ing/kfree'ing buffers each time these
> functions are called

So that the typical case, with little SPI contention, doesn't
hit the heap? That's sure what I thought ... though I can't speak
for what other people may think I thought. You were the one that
wanted to optimize the atypical case to remove a blocking path!


> (as he proposes for upper layer drivers to do)?

No I didn't. I actually said that the upper layer drivers should
just not use DMA-unsafe areas for I/O buffers in the first place.
Places like stacks or static data. Doing that means there's never
a need for a new kmalloc buffer, unless maybe you're marshaling
things into a scratch buffer.


> The thing is that only controller driver is aware whether DMA is needed
> or not, so it's controller driver that should work it out.

Given the policy that all code avoids DMA-unsafe areas for I/O buffers,
the issue has already been worked out globally.

- Dave

2005-12-14 17:55:39

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>On Wednesday 14 December 2005 5:50 am, Vitaly Wool wrote:
>
>
>>>>It's way better to just insist that all I/O buffers (in all
>>>>generic APIs) be DMA-safe. AFAICT that's a pretty standard
>>>>rule everywhere in Linux.
>>>>
>>>>
>>>I agree.
>>>
>>>
>>Well, why then David doesn't insist on that in his own code?
>>His synchronous transfer functions
>>
>>are allocating transfer buffers on
>>stack which is not DMA-safe.
>>
>>
>
>I think the very first version did that, but nothing since
>has taken that shortcut. (Several months now.) It uses
>a buffer that's allocated on the heap.
>
>
You're wrong.
Isn't it a part of your code:

static inline ssize_t spi_w8r8(struct spi_device *spi, u8 cmd)
{
ssize_t status;
u8 result;

status = spi_write_then_read(spi, &cmd, 1, &result, 1);

/* return negative errno or unsigned value */
return (status < 0) ? status : result;
}

You're allocating u8 var on stack, then allocate a 1-byte-long buffer
and copy the data instead of letting the controller driver decide
whether this allocation/copy is necessary or not. And in 99% cases it's
not gonna be necessary as any controller driver w/o brain damage will
transfer this as PIO.
So the overhead for sending/receiving 1 byte will be _very big_
(trylock, kmalloc, memcpy). See now what I'm talking about?

>
>
>
>>Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
>>Why isn't he just kmalloc'ing/kfree'ing buffers each time these
>>functions are called
>>
>>
>
>So that the typical case, with little SPI contention, doesn't
>hit the heap? That's sure what I thought ... though I can't speak
>for what other people may think I thought. You were the one that
>wanted to optimize the atypical case to remove a blocking path!
>
>
I meant kmalloc'ing/kfree'ing buffers is spi_w8r8/spi_w8r16/etc.

Vitaly

2005-12-14 17:54:13

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greg KH wrote:

>What is the speed of your SPI bus?
>
>And what are your preformance requirements?
>
>
The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
at true 10 Mbit/sec.

Vitaly

P. S. I'm speaking not about this particular case during most part of
this conversation. Sound cards behind the SPI bus will suffer a lot more
since it's their path to use wXrY functions (lotsa small transfers)
rather than WLAN's.

2005-12-14 18:48:49

by Stephen Street

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add async message handing library to David Brownell's core

On Tue, 2005-12-13 at 17:06 +0300, Vitaly Wool wrote:
> Greetings,
>
> This thingie hasn't been thoroughly tested yet, but it's lightweight
> and easy to understand so I don't think solving the problems that
> may arise will take long. Though I haven't actually done that yet,
> I'm sure that Stephen's PXA SSP driver will become easier to understand
> and less in footprint and will work faster when it's rewritten using
> this library. (Yes, I do expect performance improvement here as the
> current implementation schedules multiple tasklets, so
> scheduling penalty is high.)

Is this really true? Is tasklet scheduling "harder" than kernal thread
scheduling? A close look at my PXA SSP SPI implementation will reveal
that my design is nearly lock-less and callable from any execution
context (i.e. interrupt context).

In general, the "pump_transfer" and "pump_message" tasklets should
mutually exclusive (anything else is a programming
mistake). I certainly could collapse the tasklets into a single one if
you think this would be better?

> + * spi_queue - (internal) queue the message to be processed asynchronously
> + * @spi: SPI device to perform transfer to/from
> + * @msg: message to be sent
> + * Description:
> + * This function queues the message to SPI controller's queue.
> + */
> +static int spi_queue(struct spi_device *spi, struct spi_message *msg)
> +{
> + struct threaded_async_data *td = spi->master->context;
> + int rc = 0;
> +
> + if (!td) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + msg->spi = spi;
> + down(&td->lock);
> + list_add_tail(&msg->queue, &td->msgs);
> + dev_dbg(spi->dev.parent, "message has been queued\n");
> + up(&td->lock);
> + wake_up_interruptible(&td->wq);
> +
> +out:
> + return rc;
> +}
> +

This can not be invoke this from "interrupt context" which is a
requirement for my SPI devices (CS8415A, CS8405A, CS4341).


2005-12-14 19:03:12

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote:

> Sound cards behind the SPI bus will suffer a lot more
> since it's their path to use wXrY functions (lotsa small transfers)
> rather than WLAN's.

No, "stupid drivers will suffer"; nothing new. Just observe
how the ads7846 touchscreen driver does small async transfers.

Remember too that sending audio data over SPI (rather than
say I2S, McBSP etc) is a different case than using it for the
mixer controls.

- Dave

2005-12-14 19:17:23

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Wed, Dec 14, 2005 at 08:53:54PM +0300, Vitaly Wool wrote:
> Greg KH wrote:
>
> >What is the speed of your SPI bus?
> >
> >And what are your preformance requirements?
> >
> >
> The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
> at true 10 Mbit/sec.

Then you should be fine with the copying data and memset stuff, based on
the workload the rest of the kernel does for other busses which have
this same requirement of DMAable buffers.

And I'm sure David will be glad to have you point out any places in his
code where it accidentally takes data off of the stack instead of
allocating it.

thanks,

greg k-h

2005-12-14 19:20:06

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote:
>
>
>
>> Sound cards behind the SPI bus will suffer a lot more
>>since it's their path to use wXrY functions (lotsa small transfers)
>>rather than WLAN's.
>>
>>
>
>No, "stupid drivers will suffer"; nothing new. Just observe
>how the ads7846 touchscreen driver does small async transfers.
>
>
So just answer please yes or no: are your spi_wXrY functions intended
for usage at all or not?
Maybe you should remove them completely?

Vitaly

2005-12-14 19:27:05

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote:
> Greg KH wrote:
>
> >What is the speed of your SPI bus?
> >
> >And what are your preformance requirements?
> >
> >
> The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
> at true 10 Mbit/sec.

Some SPI flash chips are rated at 60 MHz ... there's no "official"
standard placing such limits on SPI.

The various IEEE 802.15.4 (ZigBee) transceivers seem to take up
to 10 MHz clocks, FWIW. USB controllers, 12 MHz (surprise! they
are of course full speed, 12 MHz).

Sensors with A-to-D converters are often lower rate; only 100K
samples per second, say, at maybe 12 bits each; more like 2 MHz.

- Dave

2005-12-14 19:29:46

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

David Brownell wrote:

>On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote:
>
>
>>Greg KH wrote:
>>
>>
>>
>>>What is the speed of your SPI bus?
>>>
>>>And what are your preformance requirements?
>>>
>>>
>>>
>>>
>>The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
>>at true 10 Mbit/sec.
>>
>>
>
>Some SPI flash chips are rated at 60 MHz ... there's no "official"
>standard placing such limits on SPI.
>
>
David, can you please stop treating me as an idiot? Of course I meant
the specific SPI bus we're working with which is quite evident from the
context.
Thanks.

Vitaly

2005-12-14 19:31:09

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greg KH wrote:

>On Wed, Dec 14, 2005 at 08:53:54PM +0300, Vitaly Wool wrote:
>
>
>>Greg KH wrote:
>>
>>
>>
>>>What is the speed of your SPI bus?
>>>
>>>And what are your preformance requirements?
>>>
>>>
>>>
>>>
>>The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
>>at true 10 Mbit/sec.
>>
>>
>
>Then you should be fine with the copying data and memset stuff, based on
>the workload the rest of the kernel does for other busses which have
>this same requirement of DMAable buffers.
>
>And I'm sure David will be glad to have you point out any places in his
>code where it accidentally takes data off of the stack instead of
>allocating it.
>
>
Hmm, I recall I've already posted some pieces of code with that stuff.

Vitaly

2005-12-14 19:35:07

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote:
>
>
>
>> Sound cards behind the SPI bus will suffer a lot more
>>since it's their path to use wXrY functions (lotsa small transfers)
>>rather than WLAN's.
>>
>>
>
>No, "stupid drivers will suffer"; nothing new. Just observe
>how the ads7846 touchscreen driver does small async transfers.
>
>
Two more cents: ads7846 has a bunch of code just related to the need to
do something from the interrupt context. Without this constraint, I
don't see why w8r8 can not be used in most cases (of course, with the
changes I propose).

Vitaly

2005-12-14 19:41:57

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add async message handing library to David Brownell's core

Stephen Street wrote:

>On Tue, 2005-12-13 at 17:06 +0300, Vitaly Wool wrote:
>
>
>>Greetings,
>>
>>This thingie hasn't been thoroughly tested yet, but it's lightweight
>>and easy to understand so I don't think solving the problems that
>>may arise will take long. Though I haven't actually done that yet,
>>I'm sure that Stephen's PXA SSP driver will become easier to understand
>>and less in footprint and will work faster when it's rewritten using
>>this library. (Yes, I do expect performance improvement here as the
>>current implementation schedules multiple tasklets, so
>>scheduling penalty is high.)
>>
>>
>
>Is this really true? Is tasklet scheduling "harder" than kernal thread
>scheduling? A close look at my PXA SSP SPI implementation will reveal
>that my design is nearly lock-less and callable from any execution
>context (i.e. interrupt context).
>
>
It's harder in your case because the tasklet is created each time it's
scheduled again, as far as I see it in your impleemntation.
Each SPI controller thread is created only once so it's more lightweight
than what you do.

>>+ * spi_queue - (internal) queue the message to be processed asynchronously
>>+ * @spi: SPI device to perform transfer to/from
>>+ * @msg: message to be sent
>>+ * Description:
>>+ * This function queues the message to SPI controller's queue.
>>+ */
>>+static int spi_queue(struct spi_device *spi, struct spi_message *msg)
>>+{
>>+ struct threaded_async_data *td = spi->master->context;
>>+ int rc = 0;
>>+
>>+ if (!td) {
>>+ rc = -EINVAL;
>>+ goto out;
>>+ }
>>+
>>+ msg->spi = spi;
>>+ down(&td->lock);
>>+ list_add_tail(&msg->queue, &td->msgs);
>>+ dev_dbg(spi->dev.parent, "message has been queued\n");
>>+ up(&td->lock);
>>+ wake_up_interruptible(&td->wq);
>>+
>>+out:
>>+ return rc;
>>+}
>>+
>>
>>
>
>This can not be invoke this from "interrupt context" which is a
>requirement for my SPI devices (CS8415A, CS8405A, CS4341).
>
>
>
>
Okay, not a major issue though. Change mutexes to
spin_lock_irq/spin_unlock_irq and it's callable from an interrupt
context, right?

Vitaly

2005-12-14 19:59:31

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog


> static inline ssize_t spi_w8r8(struct spi_device *spi, u8 cmd)
> {
> ssize_t status;
> u8 result;
>
> status = spi_write_then_read(spi, &cmd, 1, &result, 1);
>
> /* return negative errno or unsigned value */
> return (status < 0) ? status : result;
> }
>
> You're allocating u8 var on stack, then allocate a 1-byte-long buffer
> and copy the data instead of letting the controller driver decide
> whether this allocation/copy is necessary or not.

Yeah, like that matters in the face of the overhead to queue
the message, get to the head of the SPI transfer queue, go
through that queue, then finally wake up the task that was
synchronously blocking in write_then_read(). Oh, and since
that's inlined, GCC may be re-using existing state...

If folk want an "it looks simple" convenient/friendly API,
there is always a price to pay. In this case, that cost is
dwarfed by the mere fact that they're using a synchronous
model to access shared resources (the SPI controller).


> >>Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
> >>Why isn't he just kmalloc'ing/kfree'ing buffers each time these
> >>functions are called
> >
> >So that the typical case, with little SPI contention, doesn't
> >hit the heap? That's sure what I thought ... though I can't speak
> >for what other people may think I thought. You were the one that
> >wanted to optimize the atypical case to remove a blocking path!
>
> I meant kmalloc'ing/kfree'ing buffers is spi_w8r8/spi_w8r16/etc.

As I said: so the _typical_ case doesn't hit the heap. There are
inherent overheads for such RPC-style calls. But there's also no
point in gratuitously increasing them.

- Dave

2005-12-14 20:11:22

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

David Brownell wrote:

>>static inline ssize_t spi_w8r8(struct spi_device *spi, u8 cmd)
>>{
>> ssize_t status;
>> u8 result;
>>
>> status = spi_write_then_read(spi, &cmd, 1, &result, 1);
>>
>> /* return negative errno or unsigned value */
>> return (status < 0) ? status : result;
>>}
>>
>>You're allocating u8 var on stack, then allocate a 1-byte-long buffer
>>and copy the data instead of letting the controller driver decide
>>whether this allocation/copy is necessary or not.
>>
>>
>
>Yeah, like that matters in the face of the overhead to queue
>the message, get to the head of the SPI transfer queue, go
>through that queue, then finally wake up the task that was
>synchronously blocking in write_then_read(). Oh, and since
>that's inlined, GCC may be re-using existing state...
>
>If folk want an "it looks simple" convenient/friendly API,
>there is always a price to pay. In this case, that cost is
>dwarfed by the mere fact that they're using a synchronous
>model to access shared resources (the SPI controller).
>
>
It's just words; the patch I'm proposing cuts this price to nothing but
you're just being too stubborn to accept that. :(

>
>
>
>>>>Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
>>>>Why isn't he just kmalloc'ing/kfree'ing buffers each time these
>>>>functions are called
>>>>
>>>>
>>>So that the typical case, with little SPI contention, doesn't
>>>hit the heap? That's sure what I thought ... though I can't speak
>>>for what other people may think I thought. You were the one that
>>>wanted to optimize the atypical case to remove a blocking path!
>>>
>>>
>>
>>I meant kmalloc'ing/kfree'ing buffers is spi_w8r8/spi_w8r16/etc.
>>
>>
>
>As I said: so the _typical_ case doesn't hit the heap. There are
>inherent overheads for such RPC-style calls. But there's also no
>point in gratuitously increasing
>
Sorry, increasing what?

Okay, lemme summarize: I've spent a day working out the minimal changes
I'd like to have added to your core and two days trying to convince
those changes are necessary. You just don't want to listen to me. So the
conclusion is that the convergence process has been deadlocked, and the
only way out for us is -- we'll continue to work on our core, as
there're things we'd like to have in SPI core and there're other people
using our core.

Vitaly

2005-12-14 20:18:55

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add async message handing library

On Tuesday 13 December 2005 6:06 am, Vitaly Wool wrote:
> Greetings,
>
> as a part of the convergence process between two SPI cores ...
> I'd like to post here one more result of mindwork :). ...
> this is a port of the library
> for handling async SPI messages using kernel threads, one per each
> SPI controller, from our core to David's.

I'll have to take some time to look at this; I had planned
to evolve the spi_bitbang framework a bit more. It's already
working (parport/pc), but needs some improvements.

Just wanted you to not expect a response Real Soon Now, since
there are other things I have to do too. :)

- Dave

2005-12-14 20:21:21

by David Brownell

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog


> So just answer please yes or no: are your spi_wXrY functions intended
> for usage at all or not?

Certainly. Not _mis_used though.

2005-12-14 21:20:06

by Stephen Street

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add async message handing library to David Brownell's core

On Wed, 2005-12-14 at 22:41 +0300, Vitaly Wool wrote:
> >Is this really true? Is tasklet scheduling "harder" than kernal thread
> >scheduling? A close look at my PXA SSP SPI implementation will reveal
> >that my design is nearly lock-less and callable from any execution
> >context (i.e. interrupt context).
> >
> >
> It's harder in your case because the tasklet is created each time it's
> scheduled again, as far as I see it in your impleemntation.
> Each SPI controller thread is created only once so it's more lightweight
> than what you do.
>
I'm not sure what you mean by "create". The tasklet structures are
created and initialized once in the driver probe function. I'm not an
expert but I looked into the implementation (softirq.c) of tasklets and
found the following design:

1) Tasklets are run by a softirq.
2) A softirq is really a kernel thread allocated on a per cpu basis.
3) A "scheduled" tasklet is simply a member of a link list maintained by
the softirq thread.

My driver implementation has the following features:

1) Uses only one kernel thread for all SPI controllers.
2) Reuses existing performance tuned kernel infrastructure (i.e.
tasklets)
3) Implements a low latency locking scheme for dispatching SPI transfers
via tasklet's serial scheduling guarantees.

IMHO, from a system load perspective, my approach is lighter and simpler
than adding a dedicated kernel thread for each SPI controller.

Stephen


2005-12-15 06:48:19

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

David Brownell wrote:

>No, "stupid drivers will suffer"; nothing new. Just observe
>how the ads7846 touchscreen driver does small async transfers.
>
>
One cannot allocate memory in interrupt context, so the way to go is
allocating it on stack, thus the buffer is not DMA-safe.
Making it DMA-safe in thread that does the very message processing is a
good way of overcoming this.
Using preallocated buffer is not a good way, since it may well be
already used by another interrupt or not yet processed by the worker
thread (or tasklet, or whatever).
The way it's done in this ads7846 driver is not quite acceptable.
Losing the transfer if the previous one is still processed is *not* the
way to go in some cases. One can not predict how many transfers are
gonna be dropped due to "previous trransfer is being processed" problem,
it depends on the system load. And though it's not a problem for
touchscreen, it *will* be a problem if it were MMC, for instance.

Vitaly

2005-12-15 10:09:35

by dmitry pervushin

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Wed, 2005-12-14 at 20:53 +0300, Vitaly Wool wrote:
> Greg KH wrote:
>
> >What is the speed of your SPI bus?
> >
> >And what are your preformance requirements?
> >
> >
> The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work
> at true 10 Mbit/sec.
My two cents: the faster is better; SPI bus itself can work on 52MHz,
and AFAIK WLAN developers limit the speed due to their firmware
reqiorements.
>
> Vitaly
>
> P. S. I'm speaking not about this particular case during most part of
> this conversation. Sound cards behind the SPI bus will suffer a lot more
> since it's their path to use wXrY functions (lotsa small transfers)
> rather than WLAN's.
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems? Stop! Download the new AJAX search engine that makes
> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> _______________________________________________
> spi-devel-general mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>
>

2005-12-15 12:19:15

by Vitaly Wool

[permalink] [raw]
Subject: [PATCH/RFC] SPI: async message handing library update

This is just an update of the library for handling async SPI messages using kernel threads, one per each SPI controller. The only change is that thread's mutex has been changed to spinlock, in order to make it possible to call transfer function from the interrupt context.

Signed-off-by: Vitaly Wool <[email protected]>

drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 3
drivers/spi/spi-thread.c | 198 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi-thread.h | 27 +++++
include/linux/spi/spi.h | 3
5 files changed, 240 insertions(+)

Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Kconfig
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -13,6 +13,7 @@ config SPI_ARCH_HAS_MASTER
default y if ARCH_AT91
default y if ARCH_OMAP
default y if ARCH_PXA
+ default y if ARCH_PNX4008
default y if X86 # devel hack only!! (ICH7 can...)

config SPI_ARCH_HAS_SLAVE
@@ -63,6 +64,14 @@ config SPI_MASTER
controller and the protocol drivers for the SPI slave chips
that are connected.

+config SPI_THREAD
+ boolean "Library for threaded asynchronous message processing"
+ depends on SPI_MASTER
+ help
+ Choose this if you want to use thread-based asynchronous
+ message processing library. Using kernel threads for
+ asynchronous data processing is the most common option.
+
comment "SPI Master Controller Drivers"
depends on SPI_MASTER

Index: linux-2.6.orig/drivers/spi/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Makefile
+++ linux-2.6.orig/drivers/spi/Makefile
@@ -10,6 +10,9 @@ endif
# config declarations into driver model code
obj-$(CONFIG_SPI_MASTER) += spi.o

+# thread-based async message handling library
+obj-$(CONFIG_SPI_THREAD) += spi-thread.o
+
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
# ... add above this line ...
Index: linux-2.6.orig/drivers/spi/spi-thread.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.c
@@ -0,0 +1,198 @@
+/*
+ * drivers/spi/spi-thread.c
+ *
+ * Authors:
+ * Vitaly Wool <[email protected]>
+ * Dmitry Pervushin <[email protected]>
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi/spi.h>
+#include <asm/atomic.h>
+
+static int spi_thread(void *);
+static int spi_queue(struct spi_device *, struct spi_message *);
+
+struct threaded_async_data {
+ atomic_t exiting;
+ struct spi_master *master;
+ struct task_struct *thread;
+ wait_queue_head_t wq;
+ struct list_head msgs;
+ spinlock_t lock;
+ int (*xfer) (struct spi_master *, struct spi_message *);
+};
+
+/**
+ * spi_start_async - start the thread
+ * @master: SPI controller structure which the thread is related to
+ * @return: abstract pointer to the thread context
+ */
+int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ struct threaded_async_data *td = kzalloc (sizeof (struct threaded_async_data), GFP_KERNEL);
+ int rc = 0;
+
+ if (!td) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if (master->context) {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ td->master = master;
+ atomic_set(&td->exiting, 0);
+ td->thread = kthread_run(spi_thread, td, "%s-work", master->cdev.dev->bus_id);
+ init_waitqueue_head(&td->wq);
+ spi_lock_init(&td->lock);
+ INIT_LIST_HEAD(&td->msgs);
+ master->transfer = spi_queue;
+ td->xfer = xfer;
+ master->context = td;
+
+out:
+ return rc;
+}
+EXPORT_SYMBOL_GPL(spi_start_async);
+
+/**
+ * spi_stop_async - stop the thread
+ * @master: SPI controller structure which the thread is related to
+ */
+void spi_stop_async (struct spi_master *master)
+{
+ struct threaded_async_data *td = master->context;
+
+ if (td) {
+ /* TODO: free messages, if any */
+ atomic_inc (&td->exiting);
+ kthread_stop(td->thread);
+ kfree(td);
+ master->context = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(spi_stop_async);
+
+/**
+ * spi_thread_awake - function that called to determine if thread needs to process any messages
+ * @td: pointer to struct threaded_async_data
+ * Description:
+ * Thread wakes up if there is signal to exit (bd->exiting is set)
+ * or there are any messages in bus' queue.
+ */
+static int spi_thread_awake(struct threaded_async_data *td)
+{
+ int ret = -EINVAL;
+
+ if (!td || atomic_read(&td->exiting)) {
+ goto out;
+ }
+
+ spin_lock_irq(&td->lock);
+ ret = !list_empty(&td->msgs);
+ spin_unlock_irq(&td->lock);
+out:
+ return ret;
+}
+
+/**
+ * spi_get_next_msg - retrieve the next message
+ * @data: pointer to threaded_async_data structure associated with
+ * SPI controller thread
+ */
+static inline struct spi_message *spi_get_next_msg(struct threaded_async_data *data)
+{
+ return list_entry(data->msgs.next, struct spi_message, queue);
+}
+
+/**
+ * spi_thread - the thread that calls bus functions to perform actual transfers
+ * @context: pointer to struct spi_bus_data with bus-specific data
+ * Description:
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ */
+static int spi_thread(void *context)
+{
+ struct threaded_async_data *td = context;
+ struct spi_message *message = NULL;
+
+ while (!kthread_should_stop()) {
+
+ wait_event_interruptible(td->wq, spi_thread_awake(td));
+
+ if (atomic_read(&td->exiting))
+ goto thr_exit;
+
+ spin_lock_irq(&td->lock);
+ while (!list_empty(&td->msgs)) {
+ /*
+ * this part is locked by td->lock,
+ * to protect spi_message extraction
+ */
+ message = spi_get_next_msg(td);
+ list_del (&message->queue);
+
+ spin_unlock_irq(&td->lock);
+
+ message->status = td->xfer(td->master, message);
+ if (message->complete)
+ message->complete(context);
+
+ /* lock the data again... */
+ spin_lock_irq(&td->lock);
+ }
+ spin_unlock_irq(&td->lock);
+ }
+
+thr_exit:
+ return 0;
+}
+
+/**
+ * spi_queue - (internal) queue the message to be processed asynchronously
+ * @spi: SPI device to perform transfer to/from
+ * @msg: message to be sent
+ * Description:
+ * This function queues the message to SPI controller's queue.
+ */
+static int spi_queue(struct spi_device *spi, struct spi_message *msg)
+{
+ struct threaded_async_data *td = spi->master->context;
+ int rc = 0;
+
+ if (!td) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ msg->spi = spi;
+ spin_lock_irq(&td->lock);
+ list_add_tail(&msg->queue, &td->msgs);
+ spin_unlock_irq(&td->lock);
+ dev_dbg(spi->dev.parent, "message has been queued\n");
+ wake_up_interruptible(&td->wq);
+
+out:
+ return rc;
+}
+
Index: linux-2.6.orig/include/linux/spi/spi-thread.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi/spi-thread.h
@@ -0,0 +1,27 @@
+/*
+ * linux/drivers/spi/spi-thread.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <[email protected]>
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_THREAD_H
+#define __SPI_THREAD_H
+
+
+#if defined (CONFIG_SPI_THREAD)
+extern int spi_start_async (struct spi_master *, int (*)(struct spi_master *, struct spi_message *));
+extern void spi_stop_async (struct spi_master *);
+#else
+static inline int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ return -EINVAL;
+}
+static inline void spi_stop_async (struct spi_master *master)
+{
+}
+#endif /* CONFIG_SPI_THREAD */
+#endif /* __SPI_THREAD_H */
Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -152,6 +152,7 @@ static inline void spi_unregister_driver
* device's SPI controller; protocol code may call this.
* @transfer: adds a message to the controller's transfer queue.
* @cleanup: frees controller-specific state
+ * @context: controller-specific data
*
* Each SPI master controller can communicate with one or more spi_device
* children. These make a small bus, sharing MOSI, MISO and SCK signals
@@ -207,6 +208,8 @@ struct spi_master {

/* called on release() to free memory provided by spi_master */
void (*cleanup)(const struct spi_device *spi);
+
+ void *context;
};

/* the spi driver core manages memory for the spi_master classdev */

2005-12-15 16:51:06

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Thu, Dec 15, 2005 at 09:47:42AM +0300, Vitaly Wool wrote:
> David Brownell wrote:
>
> >No, "stupid drivers will suffer"; nothing new. Just observe
> >how the ads7846 touchscreen driver does small async transfers.
> >
> >
> One cannot allocate memory in interrupt context, so the way to go is
> allocating it on stack, thus the buffer is not DMA-safe.
> Making it DMA-safe in thread that does the very message processing is a
> good way of overcoming this.
> Using preallocated buffer is not a good way, since it may well be
> already used by another interrupt or not yet processed by the worker
> thread (or tasklet, or whatever).

Yes it is a good way. That's the way USB currently works in the kernel,
and it works just fine. It keeps the rules simple and everyone knows
what needs to be done.

thanks,

greg k-h

2005-12-15 20:06:31

by David Brownell

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

On Wednesday 14 December 2005 10:47 pm, Vitaly Wool wrote:

> One cannot allocate memory in interrupt context, so the way to go is
> allocating it on stack, thus the buffer is not DMA-safe.

kmalloc(..., GFP_ATOMIC) is the way to allocate memory in irq context.
It's done that way throughout the kernel.


> Making it DMA-safe in thread that does the very message processing is a
> good way of overcoming this.

The rest of Linux appears to work fine without needing such mechanisms...

I really fail to see why you think SPI needs that. USB isn't the only
counterexample, but it's particularly relevant since both USB and SPI
use asynchronous message passing over serial links ... and USB has a
rather complete driver stack over it. (None of the USB based WLAN
drivers need those static buffers you worry about, by the way...)


> Using preallocated buffer is not a good way, since it may well be
> already used by another interrupt or not yet processed by the worker
> thread (or tasklet, or whatever).

We would call those "driver bugs" and expect patches. :)


> The way it's done in this ads7846 driver is not quite acceptable.
> Losing the transfer if the previous one is still processed is *not* the
> way to go in some cases.

That's not the way it works though. And you seem to have made up
a "losing the transfer" failure mode out of whole cloth; I have no
idea where that came from.


> One can not predict how many transfers are
> gonna be dropped due to "previous trransfer is being processed" problem,
> it depends on the system load. And though it's not a problem for
> touchscreen, it *will* be a problem if it were MMC, for instance.

Huh, well I've already seen one nice "mmc/sd over SPI" driver, using
a slightly earlier version of the framework than is found in mm3.
It's being used for root filesystems.

So demonstrably there is no problem for MMC/SD, either.

- Dave

2005-12-15 22:18:17

by Vitaly Wool

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

David Brownell wrote:

>On Wednesday 14 December 2005 10:47 pm, Vitaly Wool wrote:
>
>
>
>>One cannot allocate memory in interrupt context, so the way to go is
>>allocating it on stack, thus the buffer is not DMA-safe.
>>
>>
>
>kmalloc(..., GFP_ATOMIC) is the way to allocate memory in irq context.
>It's done that way throughout the kernel.
>
>
It's not applicable within the RT-related changes. kmalloc anyway takes
mutexes, so allocationg it in interrupt context is buggy.
*Legacy* kernel code does that but why produce a new code with that?

>
>
>
>>Making it DMA-safe in thread that does the very message processing is a
>>good way of overcoming this.
>>
>>
>
>The rest of Linux appears to work fine without needing such mechanisms...
>
>
The rest of Linux still has a lot of bugs. Noone I guess is ready to
argue that.

>I really fail to see why you think SPI needs that. USB isn't the only
>counterexample, but it's particularly relevant since both USB and SPI
>use asynchronous message passing over serial links ... and USB has a
>rather complete driver stack over it. (None of the USB based WLAN
>drivers need those static buffers you worry about, by the way...)
>
>
I haven't heard of USB device registers needing to be written in IRQ
context. I'm not that well familiar with USB, so if you give such an
example, that'd be fine.

>>Using preallocated buffer is not a good way, since it may well be
>>already used by another interrupt or not yet processed by the worker
>>thread (or tasklet, or whatever).
>>
>>
>
>We would call those "driver bugs" and expect patches. :)
>
>
>
Well, I know one such patch ;)

>>One can not predict how many transfers are
>>gonna be dropped due to "previous trransfer is being processed" problem,
>>it depends on the system load. And though it's not a problem for
>>touchscreen, it *will* be a problem if it were MMC, for instance.
>>
>>
>
>Huh, well I've already seen one nice "mmc/sd over SPI" driver, using
>a slightly earlier version of the framework than is found in mm3.
>It's being used for root filesystems.
>
>So demonstrably there is no problem for MMC/SD, either.
>
>
No problem... Has the driver been tested in stress conditions? If not,
then I guess you can't say this for sure :)

Vitaly

2005-12-15 22:23:44

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greg KH wrote:

>On Thu, Dec 15, 2005 at 09:47:42AM +0300, Vitaly Wool wrote:
>
>
>>David Brownell wrote:
>>
>>
>>
>>>No, "stupid drivers will suffer"; nothing new. Just observe
>>>how the ads7846 touchscreen driver does small async transfers.
>>>
>>>
>>>
>>>
>>One cannot allocate memory in interrupt context, so the way to go is
>>allocating it on stack, thus the buffer is not DMA-safe.
>>Making it DMA-safe in thread that does the very message processing is a
>>good way of overcoming this.
>>Using preallocated buffer is not a good way, since it may well be
>>already used by another interrupt or not yet processed by the worker
>>thread (or tasklet, or whatever).
>>
>>
>
>Yes it is a good way. That's the way USB currently works in the kernel,
>and it works just fine. It keeps the rules simple and everyone knows
>what needs to be done.
>
>
Looking at my usbnet stuff, I can't share that opinion :-/
Are you really ready to lower the performance and quality of service
just for approach uniformity?
And, can you please point me out the examples of devices behind USB bus
that need to write registers from an interrupt context?

Vitaly


2005-12-15 22:34:12

by Greg KH

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

On Fri, Dec 16, 2005 at 01:17:56AM +0300, Vitaly Wool wrote:
> David Brownell wrote:
>
> >On Wednesday 14 December 2005 10:47 pm, Vitaly Wool wrote:
> >
> >
> >
> >>One cannot allocate memory in interrupt context, so the way to go is
> >>allocating it on stack, thus the buffer is not DMA-safe.
> >>
> >>
> >
> >kmalloc(..., GFP_ATOMIC) is the way to allocate memory in irq context.
> >It's done that way throughout the kernel.
> >
> >
> It's not applicable within the RT-related changes. kmalloc anyway takes
> mutexes, so allocationg it in interrupt context is buggy.

What RT-related changes cause this?

> *Legacy* kernel code does that but why produce a new code with that?

In this terminoligy, you are calling 2.6.15-rc5 "legacy". Which is not
true.

> >>Making it DMA-safe in thread that does the very message processing is a
> >>good way of overcoming this.
> >>
> >>
> >
> >The rest of Linux appears to work fine without needing such mechanisms...
> >
> >
> The rest of Linux still has a lot of bugs. Noone I guess is ready to
> argue that.

Huh? Please point out these bugs in the mainline tree and we will be
glad to fix them.

> >I really fail to see why you think SPI needs that. USB isn't the only
> >counterexample, but it's particularly relevant since both USB and SPI
> >use asynchronous message passing over serial links ... and USB has a
> >rather complete driver stack over it. (None of the USB based WLAN
> >drivers need those static buffers you worry about, by the way...)
> >
> >
> I haven't heard of USB device registers needing to be written in IRQ
> context. I'm not that well familiar with USB, so if you give such an
> example, that'd be fine.

The USB host controller drivers routienly allocate memory in irq context
as they are being asked to submit a new "packet" from a driver which was
called in irq context. Lots of USB drivers also allocate buffers in irq
context too.

So, please, drop this line of argument, it will not go any further.

greg k-h

2005-12-15 23:02:54

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Fri, Dec 16, 2005 at 01:23:32AM +0300, Vitaly Wool wrote:
> Greg KH wrote:
>
> >On Thu, Dec 15, 2005 at 09:47:42AM +0300, Vitaly Wool wrote:
> >
> >
> >>David Brownell wrote:
> >>
> >>
> >>
> >>>No, "stupid drivers will suffer"; nothing new. Just observe
> >>>how the ads7846 touchscreen driver does small async transfers.
> >>>
> >>>
> >>>
> >>>
> >>One cannot allocate memory in interrupt context, so the way to go is
> >>allocating it on stack, thus the buffer is not DMA-safe.
> >>Making it DMA-safe in thread that does the very message processing is a
> >>good way of overcoming this.
> >>Using preallocated buffer is not a good way, since it may well be
> >>already used by another interrupt or not yet processed by the worker
> >>thread (or tasklet, or whatever).
> >>
> >>
> >
> >Yes it is a good way. That's the way USB currently works in the kernel,
> >and it works just fine. It keeps the rules simple and everyone knows
> >what needs to be done.
> >
> >
> Looking at my usbnet stuff, I can't share that opinion :-/
> Are you really ready to lower the performance and quality of service
> just for approach uniformity?

What performance issues? As an example, USB has this rule, and we can
saturate a 480Mbit line with a _userspace_ driver (loads of memcopy
calles involved there.)

> And, can you please point me out the examples of devices behind USB bus
> that need to write registers from an interrupt context?

usb to serial drivers need to allocate buffers for their write functions
as they can be called in irq context from a tty line dicipline, which
causes a USB packet to be dynamically created and sent to the USB core.
I also think the USB network and ATM drivers have these requirements
too, just search for GFP_ATOMIC in the drivers/usb/ directory to find
these instances.

Anyway, this is my last response about this issue. Please consider it
over.

thanks,

greg k-h

2005-12-16 03:35:01

by Andy Isaacson

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

On Thu, Dec 15, 2005 at 02:33:22PM -0800, Greg KH wrote:
> On Fri, Dec 16, 2005 at 01:17:56AM +0300, Vitaly Wool wrote:
> > I haven't heard of USB device registers needing to be written in IRQ
> > context. I'm not that well familiar with USB, so if you give such an
> > example, that'd be fine.
>
> The USB host controller drivers routienly allocate memory in irq context
> as they are being asked to submit a new "packet" from a driver which was
> called in irq context. Lots of USB drivers also allocate buffers in irq
> context too.
>
> So, please, drop this line of argument, it will not go any further.

I know almost nothing about SPI, so I could be completely wrong here,
but I think I might have an inkling about the problem Vitaly is trying
to solve. Note that I haven't actually had to make a design like this
work, so I'm not intimately familiar with the issues, but I have seen
designs that I believe would suffer from the following problem.

Suppose you have a chip (temp sensor for example) controlled via serial
bus (I2C is what I'm familiar with, from what I know this scenario would
apply to SPI too) that *also* drives an interrupt into the CPU. You
want to be able to share the interrupt, so you need to be able to
turn off the interrupt from IRQ context (when the driver decides that it
is going to handle the interrupt). But the only way to turn off the
interrupt on the peripheral chip is to send a message via the serial
bus. Now if the IRQ gets entered while the serial bus is busy servicing
another device, the device driver author is in trouble - she can't
return from the irq handler until she's acked the IRQ and the device is
no longer asserting the interrupt, but she can't ack the IRQ without
sending a command on the serial bus, which she can't do because the bus
is currently in use higher up the call stack.

Now one could argue that this design is broken (requiring a slow serial
bus access to ack an irq means that you end up with very high-latency
interrupt handlers) but it's my impression that such designs are not
unheard of in the embedded world.

You end up having to leave the interrupt masked on return from the
irq_handler routine, ack it at a higher level at some later point, and
then re-enable it.

-andy

2005-12-16 06:51:45

by Greg KH

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog

On Thu, Dec 15, 2005 at 07:34:59PM -0800, Andy Isaacson wrote:
> Now one could argue that this design is broken (requiring a slow serial
> bus access to ack an irq means that you end up with very high-latency
> interrupt handlers) but it's my impression that such designs are not
> unheard of in the embedded world.

Then you just atomically allocate a buffer like all of the current
kernel drivers do :)

Come on people, this really isn't an issue...

thanks,

greg k-h

2005-12-16 08:38:04

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

Greg KH wrote:

>On Fri, Dec 16, 2005 at 01:23:32AM +0300, Vitaly Wool wrote:
>
>
>>Greg KH wrote:
>>
>>
>>
>>>On Thu, Dec 15, 2005 at 09:47:42AM +0300, Vitaly Wool wrote:
>>>
>>>
>>>
>>>
>>>>David Brownell wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>No, "stupid drivers will suffer"; nothing new. Just observe
>>>>>how the ads7846 touchscreen driver does small async transfers.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>One cannot allocate memory in interrupt context, so the way to go is
>>>>allocating it on stack, thus the buffer is not DMA-safe.
>>>>Making it DMA-safe in thread that does the very message processing is a
>>>>good way of overcoming this.
>>>>Using preallocated buffer is not a good way, since it may well be
>>>>already used by another interrupt or not yet processed by the worker
>>>>thread (or tasklet, or whatever).
>>>>
>>>>
>>>>
>>>>
>>>Yes it is a good way. That's the way USB currently works in the kernel,
>>>and it works just fine. It keeps the rules simple and everyone knows
>>>what needs to be done.
>>>
>>>
>>>
>>>
>>Looking at my usbnet stuff, I can't share that opinion :-/
>>Are you really ready to lower the performance and quality of service
>>just for approach uniformity?
>>
>>
>
>What performance issues? As an example, USB has this rule, and we can
>saturate a 480Mbit line with a _userspace_ driver (loads of memcopy
>calles involved there.)
>
>
What CPU is used there? I guess it's not 144 MHz ARM ;-)

>
>
>>And, can you please point me out the examples of devices behind USB bus
>>that need to write registers from an interrupt context?
>>
>>
>
>usb to serial drivers need to allocate buffers for their write functions
>as they can be called in irq context from a tty line dicipline, which
>causes a USB packet to be dynamically created and sent to the USB core.
>I also think the USB network and ATM drivers have these requirements
>too, just search for GFP_ATOMIC in the drivers/usb/ directory to find
>these instances.
>
>
Oh BTW... I'm experiencing constant problems with root filesystem over
NFS over usbnet on my target
I'm getting "server not responding, still trying" error whenever the
system (208-MHz ARM926 board) is under heavy load.
I think it may well be related to the thing we discuss.

Vitaly

2005-12-16 17:44:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core

On Fri, Dec 16, 2005 at 11:37:46AM +0300, Vitaly Wool wrote:
> >What performance issues? As an example, USB has this rule, and we can
> >saturate a 480Mbit line with a _userspace_ driver (loads of memcopy
> >calles involved there.)
> >
> What CPU is used there? I guess it's not 144 MHz ARM ;-)

I don't think so either, but don't remember the exact cpu (but it was an
embedded system...)

> >>And, can you please point me out the examples of devices behind USB bus
> >>that need to write registers from an interrupt context?
> >>
> >
> >usb to serial drivers need to allocate buffers for their write functions
> >as they can be called in irq context from a tty line dicipline, which
> >causes a USB packet to be dynamically created and sent to the USB core.
> >I also think the USB network and ATM drivers have these requirements
> >too, just search for GFP_ATOMIC in the drivers/usb/ directory to find
> >these instances.
> >
> Oh BTW... I'm experiencing constant problems with root filesystem over
> NFS over usbnet on my target
> I'm getting "server not responding, still trying" error whenever the
> system (208-MHz ARM926 board) is under heavy load.
> I think it may well be related to the thing we discuss.

I really doubt it. See David's other message to you about this. And
if you have problems, please report them on the linux-usb-devel mailing
list. We have never had bug reports due to issues involving the DMA
requirements (with the exception of people finding drivers that were
trying to send data off of the stack...)

thanks,

greg k-h

2005-12-16 18:32:11

by David Brownell

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog


> > Oh BTW... I'm experiencing constant problems with root filesystem over
> > NFS over usbnet on my target

As you clarified off-line ... it's "pegasus", not "usbnet",
which is giving you problems. That's entirely different code;
although "pegasus" could be modified to run on top of the
core that "usbnet" provides.

The problems I've seen with "pegasus" have more to do with
wierd chip behaviors (and weak fault recovery logic) than
with anything DMA-related.

- Dave

2005-12-18 19:17:00

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update

OK, I made some time to look at that. As I've mentioned already,
the "spi_bitbang" code needed to morph in this direction, and
that was in fact the patch I was hoping you'd be sending. So I
finished that up instead, and have sent it separately ... but
I've also attached the C file to this response.


> +struct threaded_async_data {
> + atomic_t exiting;
> + struct spi_master *master;
> + struct task_struct *thread;
> + wait_queue_head_t wq;

I just kept the workqueue, named after its device.
It makes the code look a lot simpler!


> + struct list_head msgs;
> + spinlock_t lock;

> + int (*xfer) (struct spi_master *, struct spi_message *);
> +};
> +
> +/**
> + * spi_start_async - start the thread
> + * @master: SPI controller structure which the thread is related to
> + * @return: abstract pointer to the thread context
> + */
> +int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *,
> struct spi_message *))

I did this differently, but liked your start/stop names. :)
So:
int spi_bitbang_start(struct spi_bitbang *bitbang);
int spi_bitbang_stop(struct spi_bitbang *bitbang);

to start and stop processing the queue associated with the bitbanged
spi_master. Callbacks just get stored in the structure; simpler,
and the return value is just a normal zero-or-negative-errno.


> @@ -152,6 +152,7 @@ static inline void spi_unregister_driver
> * device's SPI controller; protocol code may call this.
> * @transfer: adds a message to the controller's transfer queue.
> * @cleanup: frees controller-specific state
> + * @context: controller-specific data
> *
> * Each SPI master controller can communicate with one or more spi_device
> * children. These make a small bus, sharing MOSI, MISO and SCK signals

I've not seen a need for that yet; the class_get_devdata() is doing
that already. And it's now wrapped up as spi_master_get_devdata().

- Dave


Attachments:
(No filename) (1.87 kB)
spi_bitbang.c (11.69 kB)
Download all attachments

2005-12-19 15:50:23

by dmitry pervushin

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: async message handing library update

On Sun, 2005-12-18 at 10:59 -0800, David Brownell wrote:
> chipselect = !t->cs_change;
> if (chipselect);
> continue;
>
> bitbang->chipselect(spi, 0);
>
> /* REVISIT do we want the udelay here instead?
> */
> msleep(1);
Ohhh. Have you intentionally put the semicolon after "if
(chipselect)" ?

2005-12-20 07:23:15

by David Brownell

[permalink] [raw]
Subject: Re: [spi-devel-general] Re: [PATCH/RFC] SPI: async message handing library update

On Monday 19 December 2005 7:40 am, dmitry pervushin wrote:
> On Sun, 2005-12-18 at 10:59 -0800, David Brownell wrote:
> > chipselect = !t->cs_change;
> > if (chipselect);
> > continue;
> >
> > bitbang->chipselect(spi, 0);
> >
> > /* REVISIT do we want the udelay here instead?
> > */
> > msleep(1);
> Ohhh. Have you intentionally put the semicolon after "if
> (chipselect)" ?

Nope. Good catch ... haven't had a chance to try that yet with that driver;
not all drivers actually need to drop chipselect like that.

Any other comments about that approach to the "let someone else manage the
queue"? This one's more clearly "implementation utilities" than what you had
seemed to be talking about before, but I think it does address a point where
I think we were agreeing: it should be easy for people to whip together a
controller driver by providing just some lower level I/O calls.

- Dave

2005-12-20 18:01:54

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update

Hi David --

just a cuple of notes here and below...

General one: how is it supposed to set SPI bus clock in this model? I
guess that the only option is to set it in txrx_*.
That is not optimal since it means setting clock for each transfer which
is not an optimal solution, better have a function (bitbang->set_clock
or whatever) )to set clock per message.

> if (!spi->max_speed_hz)
> spi->max_speed_hz = 500 * 1000;
>
> /* nsecs = max(50, (clock period)/2), be optimistic */
> cs->nsecs = (1000000000/2) / (spi->max_speed_hz);
> if (cs->nsecs < 50)
> cs->nsecs = 50;
>
>
Suggest not to hardcode values here.

> /* set up default clock polarity, and activate chip */
> if (!chipselect) {
> bitbang->chipselect(spi, 1);
> ndelay(nsecs);
>
>
Suggest special enum/define for chipselect value.

> /* protocol tweaks before next transfer */
> if (t->delay_usecs)
> udelay(t->delay_usecs);
>
>
Suggest nsecs here as well.

Generic note: haven't tested that with DMA, will have more comments
prolly...
Another one: I just feel comfortabel with using 'bitbang' term for the
variety of SPI stuff which this library suits.

Vitaly

2005-12-21 13:16:27

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update

David,

> if (t->len) {
> /* FIXME if bitbang->use_dma, dma_map_single()
> * before the transfer, and dma_unmap_single()
> * afterwards, for either or both buffers...
> */
>
>
please *please* *_please_*!!! don't do it! :)
Let the controller driver do it *only in case it's not working in PIO!*

Vitaly

2005-12-22 17:28:37

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update

On Tuesday 20 December 2005 10:02 am, Vitaly Wool wrote:
> Hi David --
>
> just a cuple of notes here and below...
>
> General one: how is it supposed to set SPI bus clock in this model? I
> guess that the only option is to set it in txrx_*.

No, as I said separately: the standard spi_master.setup() call does
this. Things are arranged so that drivers using this code can splice
in their own code for it ... true bitbangers won't, because the
delays in the I/O loops suffice. But drivers using "word-at-a-time"
controllers (hardware-assisted shift registers!) will, as will ones
that do "buffer-at-a-time" I/O (e.g. to stuff/unstuff fifos, or set
up non-queued DMA).


> > if (!spi->max_speed_hz)
> > spi->max_speed_hz = 500 * 1000;
> >
> > /* nsecs = max(50, (clock period)/2), be optimistic */
> > cs->nsecs = (1000000000/2) / (spi->max_speed_hz);
> > if (cs->nsecs < 50)
> > cs->nsecs = 50;
> >
> >
> Suggest not to hardcode values here.

I suppose it'd make sense to just fail if max_speed_hz is invalid,
and if there's some board that an bitbang at over 10MHz we should
avoid getting in its way.


> > /* set up default clock polarity, and activate chip */
> > if (!chipselect) {
> > bitbang->chipselect(spi, 1);
> > ndelay(nsecs);
> >
> >
> Suggest special enum/define for chipselect value.

Added. Instead of "1", "BITBANG_CS_ACTIVE" ... which will normally
pull the nCS line low (after setting clock to match CPOL).


> > /* protocol tweaks before next transfer */
> > if (t->delay_usecs)
> > udelay(t->delay_usecs);
> >
> >
> Suggest nsecs here as well.

The relevant chip delays seem to be specified in usecs ... I don't
like using nsecs for the clock timings, but without doing that it'd
be impractical to define rates at the levels the hardware actually
uses. There are still some "nsec" leakages out of the real-bitbang
code up to the next level, fixable over time.


> >???????????????????????????????/* FIXME if bitbang->use_dma, dma_map_single()
> >??????????????????????????????? * before the transfer, and dma_unmap_single()
> >??????????????????????????????? * afterwards, for either or both buffers...
> >??????????????????????????????? */ ?
>
> please *please* *_please_*!!! don't do it! :)
> Let the controller driver do it *only in case it's not working in PIO!*

OK. That'd be more work for the controller driver, but you're
right that a lot of the drivers using these utilities are rather
likely to be PIO-oriented. If they want DMA speedups, they can
do the mappings themselves (in cases where the driver didn't
do them already).


> Another one: I just feel comfortabel with using 'bitbang' term for the
> variety of SPI stuff which this library suits.

You _do_ feel comfortable with it? I actually feel a bit odd, since
only one of the three driver types is really bitbanging. And in fact
it still bothers me that the other two tie down a task, but that's
the price for reusing common infrastructure.

- Dave

2005-12-22 22:10:20

by Vitaly Wool

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update

David Brownell wrote:

>>> if (!spi->max_speed_hz)
>>> spi->max_speed_hz = 500 * 1000;
>>>
>>> /* nsecs = max(50, (clock period)/2), be optimistic */
>>> cs->nsecs = (1000000000/2) / (spi->max_speed_hz);
>>> if (cs->nsecs < 50)
>>> cs->nsecs = 50;
>>>
>>>
>>>
>>>
>>Suggest not to hardcode values here.
>>
>>
>
>I suppose it'd make sense to just fail if max_speed_hz is invalid,
>and if there's some board that an bitbang at over 10MHz we should
>avoid getting in its way.
>
>
Well, why do this?

>>> /* protocol tweaks before next transfer */
>>> if (t->delay_usecs)
>>> udelay(t->delay_usecs);
>>>
>>>
>>>
>>>
>>Suggest nsecs here as well.
>>
>>
>
>The relevant chip delays seem to be specified in usecs ... I don't
>like using nsecs for the clock timings, but without doing that it'd
>be impractical to define rates at the levels the hardware actually
>uses. There are still some "nsec" leakages out of the real-bitbang
>code up to the next level, fixable over time.
>
>
>
Ok.

>
>
>
>>> /* FIXME if bitbang->use_dma, dma_map_single()
>>> * before the transfer, and dma_unmap_single()
>>> * afterwards, for either or both buffers...
>>> */
>>>
>>>
>>please *please* *_please_*!!! don't do it! :)
>>Let the controller driver do it *only in case it's not working in PIO!*
>>
>>
>
>OK. That'd be more work for the controller driver, but you're
>right that a lot of the drivers using these utilities are rather
>likely to be PIO-oriented. If they want DMA speedups, they can
>do the mappings themselves (in cases where the driver didn't
>do them already).
>
>
Agreed :)

>
>
>
>>Another one: I just feel comfortabel with using 'bitbang' term for the
>>variety of SPI stuff which this library suits.
>>
>>
>
>You _do_ feel comfortable with it? I actually feel a bit odd, since
>only one of the three driver types is really bitbanging. And in fact
>it still bothers me that the other two tie down a task, but that's
>the price for reusing common infrastructure.
>
>
Oh sorry, of course I meant "I just don't feel comfortable..."

BTW: the message handling is one per-transfer basis for bitbang. But in
this case it's not possible to imlement chained DMA transfers (2
channels, one for Rx, one for Tx, basically that's your sample use case :)

Vitaly

2005-12-23 00:19:20

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH/RFC] SPI: async message handing library update


> BTW: the message handling is one per-transfer basis for bitbang. But in
> this case it's not possible to imlement chained DMA transfers (2
> channels, one for Rx, one for Tx, basically that's your sample use case :)

This library code is intended to help folk get some functional and correct
drivers quickly ... with "chained DMA" support being explicitly a non-goal.

If you want a top performing driver, you'd go about it differently ... you
would handle all the transfers directly, and not use library code like this.
Every SOC seems to have its own preferred way to do DMA chaining, so likely
the driver would just implement the three spi_master methods directly and
map most spi_message objects into single SOC-specific DMA chains.

- Dave