linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* SPI
@ 2005-09-26 11:12 dmitry pervushin
  2005-09-26 12:31 ` SPI Eric Piel
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: dmitry pervushin @ 2005-09-26 11:12 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: spi-devel-general

Hello guys,

I am attaching the next incarnation of SPI core; feel free to comment it.

 Documentation/spi.txt  |  351 +++++++++++++++++++++++++++++++++++++
 arch/arm/Kconfig       |    2
 drivers/Kconfig        |    2
 drivers/Makefile       |    1
 drivers/spi/Kconfig    |   33 +++
 drivers/spi/Makefile   |   11 +
 drivers/spi/spi-core.c |  456 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-dev.c  |  236 +++++++++++++++++++++++++
 include/linux/spi.h    |  214 ++++++++++++++++++++++
 9 files changed, 1306 insertions(+)

Index: linux-2.6.10/arch/arm/Kconfig
===================================================================
--- linux-2.6.10.orig/arch/arm/Kconfig
+++ linux-2.6.10/arch/arm/Kconfig
@@ -834,6 +834,8 @@ source "drivers/ssi/Kconfig"
 
 source "drivers/mmc/Kconfig"
 
+source "drivers/spi/Kconfig"
+
 endmenu
 
 source "ktools/Kconfig"
Index: linux-2.6.10/drivers/Kconfig
===================================================================
--- linux-2.6.10.orig/drivers/Kconfig
+++ linux-2.6.10/drivers/Kconfig
@@ -42,6 +42,8 @@ source "drivers/char/Kconfig"
 
 source "drivers/i2c/Kconfig"
 
+source "drivers/spi/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/misc/Kconfig"
Index: linux-2.6.10/drivers/Makefile
===================================================================
--- linux-2.6.10.orig/drivers/Makefile
+++ linux-2.6.10/drivers/Makefile
@@ -67,3 +67,4 @@ obj-$(CONFIG_DPM)		+= dpm/
 obj-$(CONFIG_MMC)		+= mmc/
 obj-y				+= firmware/
 obj-$(CONFIG_EVENT_BROKER)	+= evb/
+obj-$(CONFIG_SPI)		+= spi/
Index: linux-2.6.10/drivers/spi/Kconfig
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/Kconfig
@@ -0,0 +1,33 @@
+#
+# SPI device configuration
+#
+menu "SPI support"
+
+config SPI
+	default Y
+	tristate "SPI support"
+        default false
+	help
+	  Say Y if you need to enable SPI support on your kernel
+
+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
+	tristate "SPI device interface"
+	depends on SPI
+	help
+	  Say Y here to use spi-* device files, usually found in the /dev
+	  directory on your system.  They make it possible to have user-space
+	  programs use the SPI bus. 
+	  This support is also available as a module.  If so, the module 
+	  will be called spi-dev.
+
+endmenu
+
Index: linux-2.6.10/drivers/spi/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/Makefile
@@ -0,0 +1,11 @@
+#
+# Makefile for the kernel spi bus driver.
+#
+
+obj-$(CONFIG_SPI) += spi-core.o 
+obj-$(CONFIG_SPI_CHARDEV) += spi-dev.o
+
+ifeq ($(CONFIG_SPI_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
+
Index: linux-2.6.10/drivers/spi/spi-core.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-core.c
@@ -0,0 +1,456 @@
+/*
+ *  drivers/spi/spi-core.c
+ *
+ *  Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
+ *
+ * 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>
+
+static int spi_thread(void *context);
+
+/*
+ * spi_bus_match_name
+ * 
+ * Drivers and devices on SPI bus are matched by name, just like the
+ * platform devices, with exception of SPI_DEV_CHAR. Driver with this name
+ * will be matched against any device
+ */
+static int spi_bus_match_name(struct device *dev, struct device_driver *drv)
+{
+	return  !strcmp (drv->name, SPI_DEV_CHAR) || 
+		!strcmp(TO_SPI_DEV(dev)->name, drv->name);
+}
+
+struct bus_type spi_bus = {
+	.name = "spi",
+	.match = spi_bus_match_name,
+};
+
+/*
+ * spi_bus_driver_init
+ *
+ * This function initializes the spi_bus_data structure for the 
+ * bus. Functions has to be called when bus driver gets probed
+ *
+ * Parameters:
+ * 	spi_bus_driver* 	pointer to bus driver structure
+ * 	device*			platform device to be attached to
+ * Return value:
+ * 	0 on success, error code otherwise
+ */
+int spi_bus_driver_init(struct spi_bus_driver *bus, struct device *dev)
+{
+	struct spi_bus_data *pd =
+	    kmalloc(sizeof(struct spi_bus_data), GFP_KERNEL);
+	int err = 0;
+	
+	if (!pd) {
+		err = -ENOMEM;
+		goto init_failed_1;
+	}
+	atomic_set(&pd->exiting, 0);
+	pd->bus = bus;
+	init_MUTEX(&pd->lock);
+	INIT_LIST_HEAD(&pd->msgs);
+	init_waitqueue_head(&pd->queue);
+	pd->thread = kthread_run(spi_thread, pd, "%s-work", dev->bus_id);
+	if (IS_ERR(pd->thread)) {
+		err = PTR_ERR(pd->thread);
+		goto init_failed_2;
+	}
+	dev->platform_data = pd;
+	return 0;
+	
+init_failed_2:
+	kfree(pd);	
+init_failed_1:	
+	return err;
+}
+
+/*
+ * __spi_bus_free
+ *
+ * This function called as part of unregistering bus device driver. It
+ * calls spi_device_del for each child (SPI) device on the bus
+ *
+ * Parameters:
+ * 	struct device* dev	the 'bus' device
+ * 	void* context		not used. Will be NULL
+ */
+int __spi_bus_free(struct device *dev, void *context)
+{
+	struct spi_bus_data *pd = dev->platform_data;
+
+	atomic_inc(&pd->exiting);
+	kthread_stop(pd->thread);
+	kfree(pd);
+
+	dev_dbg( dev, "unregistering children\n" );
+	/* 
+	 * NOTE: the loop below might needs redesign. Currently
+	 *       we delete devices from the head of children list
+	 *       until the list is empty; that's because the function
+	 *       device_for_each_child will hold the semaphore needed 
+	 *       for deletion of device
+	 */
+	while( !list_empty( &dev->children ) ) {
+		struct device* child = list_entry ( dev->children.next, struct device, node );
+	    	spi_device_del (TO_SPI_DEV (child) );
+	}
+	return 0;
+}
+
+/*
+ * spi_bus_driver_unregister
+ *
+ * unregisters the SPI bus from the system. Before unregistering, it deletes
+ * each SPI device on the bus using call to __spi_device_free
+ *
+ * Parameters:
+ *  	struct spi_bus_driver* bus_driver	the bus driver
+ * Return value:
+ *  	void
+ */
+void spi_bus_driver_unregister(struct spi_bus_driver *bus_driver)
+{
+	if (bus_driver) {
+		driver_for_each_dev( &bus_driver->driver, NULL, __spi_bus_free);
+		driver_unregister(&bus_driver->driver);
+	}
+}
+
+/*
+ * spi_device_release
+ * 
+ * Pointer to this function will be put to dev->release place
+ * This function gets called as a part of device removing
+ * 
+ * Parameters:
+ * 	struct device* dev
+ * Return value:
+ * 	none
+ */
+void spi_device_release( struct device* dev )
+{
+/* just a placeholder */
+}
+
+/*
+ * spi_device_add
+ *
+ * Add the new (discovered) SPI device to the bus. Mostly used by bus drivers
+ *
+ * Parameters:
+ * 	struct device* parent		the 'bus' device
+ * 	struct spi_device* dev		new device to be added
+ * 	char* name			name of device. Should not be NULL
+ * Return value:
+ * 	error code, or 0 on success
+ */
+int spi_device_add(struct device *parent, struct spi_device *dev, char *name)
+{
+	if (!name || !dev) 
+	    return -EINVAL;
+	    
+	memset(&dev->dev, 0, sizeof(dev->dev));
+	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;
+
+	return device_register(&dev->dev);
+}
+
+/*
+ * spi_queue
+ *
+ * Queue the message to be processed asynchronously
+ *
+ * Parameters:
+ *  	struct spi_msg* msg            message to be sent
+ * Return value:
+ *  	0 on no errors, negative error code otherwise
+ */
+int spi_queue( struct spi_msg *msg)
+{
+	struct device* dev = &msg->device->dev;
+	struct spi_bus_data *pd = dev->parent->platform_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(&pd->queue);
+	return 0;
+}
+
+/*
+ * __spi_transfer_callback
+ *
+ * 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
+ *
+ * Parameters:
+ * 	struct spimsg* msg	message that is being processed now
+ * 	int code		status of processing
+ */
+static void __spi_transfer_callback( struct spi_msg* msg, int code )
+{
+	if( code & (SPIMSG_OK|SPIMSG_FAILED) ) 
+		complete( (struct completion*)msg->context );
+}
+
+/*
+ * spi_transfer
+ *
+ * Process the SPI message, by queuing it to the driver and either
+ * immediately return or waiting till the end-of-processing
+ *
+ * Parameters:
+ * 	struct spi_msg* msg	message to process
+ * 	callback		user-supplied callback. If both msg->status and
+ * 				callback are set, the error code of -EINVAL
+ * 				will be returned
+ * Return value:
+ * 	0 on success, error code otherwise. This code does not reflect
+ * 	status of message, just status of queueing
+ */
+int spi_transfer( struct spi_msg* msg, void (*callback)( struct spi_msg*, int ) )
+{
+	struct completion msg_done;
+	int err = -EINVAL;
+
+	if( callback && !msg->status ) {
+		msg->status = callback;
+		callback = NULL;
+	}
+	
+	if( !callback ) {
+		if( !msg->status ) {
+			init_completion( &msg_done );
+			msg->context = &msg_done;
+			msg->status = __spi_transfer_callback;
+			spi_queue( msg );
+			wait_for_completion( &msg_done );
+			err = 0;
+		} else {
+			err = spi_queue( msg );
+		}
+	}
+		
+	return err;
+}
+/*
+ * spi_thread
+ * 
+ * This function is started as separate thread to perform actual 
+ * transfers on SPI bus
+ *
+ * Parameters:
+ *	void* context 		pointer to struct spi_bus_data 
+ */
+static int spi_thread_awake(struct spi_bus_data *bd)
+{
+	int ret;
+
+	if (atomic_read(&bd->exiting)) {
+		return 1;
+	}
+	down(&bd->lock);
+	ret = !list_empty(&bd->msgs);
+	up(&bd->lock);
+	return ret;
+}
+
+static int spi_thread(void *context)
+{
+	struct spi_bus_data *bd = context;
+	struct spi_msg *msg;
+	int xfer_status;
+	int found;
+
+	while (!kthread_should_stop()) {
+
+		wait_event_interruptible(bd->queue, spi_thread_awake(bd));
+
+		if (atomic_read(&bd->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
+			 */
+			found = 0;
+			list_for_each_entry(msg, &bd->msgs, link) {
+				if (!bd->selected_device) {
+					bd->selected_device = msg->device;
+					if (bd->bus->select)
+						bd->bus->select(bd->
+								selected_device);
+					found = 1;
+					break;
+				}
+				if (msg->device == bd->selected_device) {
+					found = 1;
+					break;
+				}
+			}
+			if (!found) {
+				/* 
+				 * all messages for current selected_device 
+				 * are processed. 
+				 * let's switch to another device 
+				 */
+				msg =
+				    list_entry(bd->msgs.next, struct spi_msg,
+					       link);
+				if (bd->bus->deselect)
+					bd->bus->deselect(bd->selected_device);
+				bd->selected_device = msg->device;
+				if (bd->bus->select)
+					bd->bus->select(bd->selected_device);
+			}
+			list_del(&msg->link);
+			up(&bd->lock);
+
+			/* 
+			 * and this part is locked by device's lock; 
+			 * spi_queue will be able to queue new 
+			 * messages
+			 */
+			spi_device_lock(&msg->device);
+			if (msg->status)
+				msg->status(msg, SPIMSG_STARTED);
+			if( bd->bus->set_clock && msg->clock )
+				bd->bus->set_clock( 
+					msg->device->dev.parent, msg->clock );
+			xfer_status = bd->bus->xfer( msg );
+			if (msg->status) {
+				msg->status(msg, SPIMSG_DONE);
+				msg->status(msg,
+					    xfer_status ? SPIMSG_OK :
+					    SPIMSG_FAILED);
+			}
+			spi_device_unlock(&msg->device);
+
+			/* lock the bus_data again... */
+			down(&bd->lock);
+		}
+		if (bd->bus->deselect)
+			bd->bus->deselect(bd->selected_device);
+		bd->selected_device = NULL;
+		/* device has been just deselected, unlocking the bus */
+		up(&bd->lock);
+	}
+thr_exit:
+	return 0;
+}
+
+/*
+ * spi_write 
+ * 	send data to a device on an SPI bus
+ * Parameters:
+ * 	spi_device* dev		the target device
+ *	char* buf		buffer to be sent
+ *	int len			buffer length
+ * Return:
+ * 	the number of bytes transferred, or negative error code.
+ */
+int spi_write(struct spi_device *dev, const char *buf, int len)
+{
+	struct spi_msg *msg = spimsg_alloc(dev, SPI_M_WR, len, NULL);
+	int ret;
+
+	memcpy(spimsg_buffer_wr(msg), buf, len);
+	ret = spi_transfer( msg, NULL );
+	return ret == 1 ? len : ret;
+}
+
+/*
+ * spi_write 
+ * 	receive data from a device on an SPI bus
+ * Parameters:
+ * 	spi_device* dev		the target device
+ *	char* buf		buffer to be sent
+ *	int len			number of bytes to receive
+ * Return:
+ * 	 the number of bytes transferred, or negative error code.
+ */
+int spi_read(struct spi_device *dev, char *buf, int len)
+{
+	int ret;
+	struct spimsg *msg = spimsg_alloc(dev, SPI_M_RD, len, NULL);
+
+	ret = spi_transfer( msg, NULL );
+	memcpy(buf, spimsg_buffer_rd(msg), len);
+	return ret == 1 ? len : ret;
+}
+
+int spi_bus_populate(struct device *parent,
+		     char *devices,
+		     void (*callback) (struct device * bus,
+				       struct spi_device * new_dev))
+{
+	struct spi_device *new_device;
+	int count = 0;
+
+	while (devices[0]) {
+		dev_dbg(parent, "discovered new SPI device, name '%s'\n",
+			devices);
+		new_device = kmalloc(sizeof(struct spi_device), GFP_KERNEL);
+		if (!new_device) {
+			break;
+		}
+		if (spi_device_add(parent, new_device, devices)) {
+			break;
+		}
+		if (callback) {
+			callback(parent, new_device);
+		}
+		devices += (strlen(devices) + 1);
+		count++;
+	}
+	return count;
+}
+
+int __init spi_core_init( void )
+{
+	return bus_register(&spi_bus);
+}
+
+subsys_initcall(spi_core_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("dmitry pervushin <dpervushin@ru.mvista.com>");
+
+EXPORT_SYMBOL_GPL(spi_queue);
+EXPORT_SYMBOL_GPL(spi_device_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);
Index: linux-2.6.10/drivers/spi/spi-dev.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-dev.c
@@ -0,0 +1,236 @@
+/*
+    spi-dev.c - spi-bus driver, char device interface  
+
+    Copyright (C) 1995-97 Simon G. Vogl
+    Copyright (C) 1998-99 Frodo Looijaard <frodol@dds.nl>
+    Copyright (C) 2002 Compaq Computer Corporation
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+/* Adapted from i2c-dev module by Jamey Hicks <jamey.hicks@compaq.com> */
+
+/* Note that this is a complete rewrite of Simon Vogl's i2c-dev module.
+   But I have used so much of his original code and ideas that it seems
+   only fair to recognize him as co-author -- Frodo */
+
+/* The devfs code is contributed by Philipp Matthias Hahn 
+   <pmhahn@titan.lahn.de> */
+
+/* Modifications to allow work with current spi-core by 
+   Andrey Ivolgin <aivolgin@ru.mvista.com>, Sep 2004
+ */
+
+/* devfs code corrected to support automatic device addition/deletion
+   by Vitaly Wool <vwool@ru.mvista.com> (C) 2004 MontaVista Software, Inc. 
+ */
+#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
+
+struct spidev_driver_data
+{
+	int minor;
+};
+
+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);
+static int __init spidev_init(void);
+
+static void spidev_cleanup(void);
+
+static int spidev_probe(struct device *dev);
+static int spidev_remove(struct device *dev);
+
+static struct file_operations spidev_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.read = spidev_read,
+	.write = spidev_write,
+	.open = spidev_open,
+	.release = spidev_release,
+};
+
+static struct class_simple *spidev_class;
+
+static struct spi_driver spidev_driver = {
+	.driver = {
+		   .name = SPI_DEV_CHAR,
+		   .probe = spidev_probe,
+		   .remove = spidev_remove,
+		   },
+};
+
+static int spidev_minor;
+
+static int spidev_probe(struct device *dev)
+{
+	struct spidev_driver_data *drvdata;
+
+	drvdata = kmalloc(sizeof(struct spidev_driver_data), GFP_KERNEL);
+	if ( !drvdata) {
+		dev_dbg( dev, "allocating drvdata failed\n" );
+		return -ENOMEM;
+	}
+
+	drvdata->minor = spidev_minor++;
+	dev_dbg( dev, "setting device's(%p) minor to %d\n",
+		 dev, drvdata->minor);
+	dev_set_drvdata(dev, drvdata);
+
+	class_simple_device_add(spidev_class,
+				MKDEV(SPI_MAJOR, drvdata->minor),
+				NULL, "spi%d", drvdata->minor);
+	dev_dbg( dev, " added\n" );
+	return 0;
+}
+
+static int spidev_remove(struct device *dev)
+{
+	struct spidev_driver_data *drvdata;
+
+	drvdata = (struct spidev_driver_data *)dev_get_drvdata(dev);
+	class_simple_device_remove(MKDEV(SPI_MAJOR, drvdata->minor));
+	kfree(drvdata);
+	dev_dbg( dev, " removed\n" );
+	return 0;
+}
+
+static ssize_t spidev_read(struct file *file, char *buf, size_t count,
+			   loff_t * offset)
+{
+	struct spi_device *dev = (struct spi_device *)file->private_data;
+	if( count > SPI_TRANSFER_MAX ) count = SPI_TRANSFER_MAX;
+	return spi_read(dev, buf, count );
+}
+
+static ssize_t spidev_write(struct file *file, const char *buf, size_t count,
+			    loff_t * offset)
+{
+	struct spi_device *dev = (struct spi_device *)file->private_data;
+	if( count > SPI_TRANSFER_MAX ) count = SPI_TRANSFER_MAX;
+	return spi_write( dev, buf, count );
+}
+
+struct spidev_openclose {
+	unsigned int minor;
+	struct file *file;
+};
+
+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);
+	struct spidev_driver_data *drvdata;
+
+	drvdata = (struct spidev_driver_data *)dev_get_drvdata(the_dev);
+	if (NULL == drvdata) {
+		pr_debug("%s: oops, drvdata is NULL !\n", __FUNCTION__);
+		return 0;
+	}
+
+	pr_debug("drvdata->minor = %d vs %d\n", drvdata->minor, o->minor);
+	if (drvdata->minor == o->minor) {
+		get_device(&dev->dev);
+		o->file->private_data = dev;
+		return 1;
+	}
+	return 0;
+}
+
+int spidev_open(struct inode *inode, struct file *file)
+{
+	struct spidev_openclose o;
+	int status;
+
+	o.minor = iminor(inode);
+	o.file = file;
+	status = driver_for_each_dev(&spidev_driver.driver, &o, spidev_do_open);
+	if (status == 0) {
+		status = -ENODEV;
+	}
+	return status < 0 ? status : 0;
+}
+
+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;
+}
+
+static int __init spidev_init(void)
+{
+	int res;
+
+	if (0 != (res = register_chrdev(SPI_MAJOR, "spi", &spidev_fops))) {
+		goto out;
+	}
+
+	spidev_class = class_simple_create(THIS_MODULE, "spi");
+	if (IS_ERR(spidev_class)) {
+		printk(KERN_ERR "%s: error creating class\n", __FUNCTION__);
+		res = -EINVAL;
+		goto out_unreg;
+	}
+
+	if (0 != (res = spi_driver_add(&spidev_driver))) {
+		goto out_unreg;
+	}
+	printk("SPI /dev entries driver.\n");
+	return 0;
+
+out_unreg:
+	unregister_chrdev(SPI_MAJOR, "spi");
+out:
+	printk(KERN_ERR "%s: Driver initialization failed\n", __FILE__);
+	return res;
+}
+
+static void spidev_cleanup(void)
+{
+	spi_driver_del(&spidev_driver);
+	class_simple_destroy(spidev_class);
+	unregister_chrdev(SPI_MAJOR, "spi");
+}
+
+MODULE_AUTHOR("dmitry pervushin <dpervushin@ru.mvista.com>");
+MODULE_DESCRIPTION("SPI /dev entries driver");
+MODULE_LICENSE("GPL");
+
+module_init(spidev_init);
+module_exit(spidev_cleanup);
Index: linux-2.6.10/include/linux/spi.h
===================================================================
--- /dev/null
+++ linux-2.6.10/include/linux/spi.h
@@ -0,0 +1,214 @@
+/*
+ *  linux/include/linux/spi/spi.h
+ *
+ *  Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
+ *
+ * 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>
+
+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;
+	atomic_t exiting;
+	struct task_struct* thread;
+	wait_queue_head_t queue; 
+	struct spi_device* selected_device;
+	struct spi_bus_driver* bus;
+};
+
+#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 (*select)( struct spi_device* dev );
+	void (*deselect)( struct spi_device* dev );
+	void (*set_clock)( struct device* bus_device, u32 clock_hz );
+	struct device_driver driver;
+};
+
+#define TO_SPI_DEV(device) container_of( device, struct spi_device, dev )
+struct spi_device 
+{
+	char name[ BUS_ID_SIZE ];
+	struct device dev;
+};
+
+#define TO_SPI_DRIVER(drv) container_of( drv, struct spi_driver, driver )
+struct spi_driver {
+    	void* (*alloc)( size_t, int );
+    	void  (*free)( const void* );
+    	unsigned char* (*get_buffer)( struct spi_device*, void* );
+    	void (*release_buffer)( struct spi_device*, unsigned char*);
+    	void (*control)( struct spi_device*, int mode, u32 ctl );
+	struct device_driver driver;
+};
+
+#define SPI_DEV_DRV( device )  TO_SPI_DRIVER( (device)->dev.driver )
+
+#define spi_device_lock( dev ) /* down( dev->dev.sem ) */
+#define spi_device_unlock( dev ) /* up( dev->dev.sem ) */
+
+/*
+ * struct spi_msg
+ *
+ * This structure represent the SPI message internally. You should never use fields of this structure directly
+ * Please use corresponding functions to create/destroy/access fields
+ *
+ */
+struct spi_msg {
+	unsigned char flags;
+#define SPI_M_RD	0x01
+#define SPI_M_WR	0x02	/**< Write mode flag */
+#define SPI_M_CSREL	0x04	/**< CS release level at end of the frame  */
+#define SPI_M_CS	0x08	/**< CS active level at begining of frame ( default low ) */
+#define SPI_M_CPOL	0x10	/**< Clock polarity */
+#define SPI_M_CPHA	0x20	/**< Clock Phase */
+	unsigned short len;	/* msg length           */
+	unsigned long clock;
+	struct spi_device* device;
+	void	      *context;
+	struct list_head link; 
+	void (*status)( struct spi_msg* msg, int code );
+	void *devbuf_rd, *devbuf_wr;
+	u8 *databuf_rd, *databuf_wr;
+};
+
+static inline struct spi_msg* spimsg_alloc( struct spi_device* device, 
+					    unsigned flags,
+					    unsigned short len, 
+					    void (*status)( struct spi_msg*, int code ) )
+{
+    struct spi_msg* msg;
+    struct spi_driver* drv = SPI_DEV_DRV( device );
+    
+    msg = kmalloc( sizeof( struct spi_msg ), GFP_KERNEL );
+    if( !msg )
+	return NULL;
+    memset( msg, 0, sizeof( struct spi_msg ) );
+    msg->len = len;
+    msg->status = status;
+    msg->device = device;
+    msg->flags = flags;
+    INIT_LIST_HEAD( &msg->link );
+    if( flags & SPI_M_RD ) {
+        msg->devbuf_rd =  drv->alloc ? 
+	    drv->alloc( len, GFP_KERNEL ):kmalloc( len, GFP_KERNEL);
+	msg->databuf_rd = drv->get_buffer ? 
+	    drv->get_buffer( device, msg->devbuf_rd ) : msg->devbuf_rd;
+    }
+    if( flags & SPI_M_WR ) {
+        msg->devbuf_wr =  drv->alloc ? 
+	    drv->alloc( len, GFP_KERNEL ):kmalloc( len, GFP_KERNEL);
+	msg->databuf_wr = drv->get_buffer ? 
+	    drv->get_buffer( device, msg->devbuf_wr ) : msg->devbuf_wr;
+    }
+    pr_debug( "%s: msg = %p, RD=(%p,%p) WR=(%p,%p). Actual flags = %s+%s\n",
+		   __FUNCTION__,
+		  msg, 
+		  msg->devbuf_rd, msg->databuf_rd,
+		  msg->devbuf_wr, msg->databuf_wr, 
+		  msg->flags & SPI_M_RD ? "RD" : "~rd",
+		  msg->flags & SPI_M_WR ? "WR" : "~wr" );
+    return msg;
+}
+
+static inline void spimsg_free( struct spi_msg * msg )
+{
+    void (*do_free)( const void* ) = kfree;
+    struct spi_driver* drv = SPI_DEV_DRV( msg->device );
+    
+    if( msg ) {
+	    if( drv->free ) 
+		do_free = drv->free;
+	    if( drv->release_buffer ) {
+		if( msg->databuf_rd) 
+		    drv->release_buffer( msg->device, msg->databuf_rd );
+    		if( msg->databuf_wr) 
+		    drv->release_buffer( msg->device, msg->databuf_wr );
+	}
+	if( msg->devbuf_rd ) 
+	    do_free( msg->devbuf_rd );
+	if( msg->devbuf_wr)
+	    do_free( msg->devbuf_wr );
+	kfree( msg );
+    }
+}
+
+static inline u8* spimsg_buffer_rd( struct spi_msg* msg ) 
+{
+    return msg ? msg->databuf_rd : NULL;
+}
+
+static inline u8* spimsg_buffer_wr( struct spi_msg* msg ) 
+{
+    return msg ? msg->databuf_wr : NULL;
+}
+
+static inline u8* spimsg_buffer( struct spi_msg* msg ) 
+{
+    if( !msg ) return NULL;
+    if( ( msg->flags & (SPI_M_RD|SPI_M_WR) ) == (SPI_M_RD|SPI_M_WR) ) {
+	printk( KERN_ERR"%s: what buffer do you really want ?\n", __FUNCTION__ );
+	return NULL;
+    }
+    if( msg->flags & SPI_M_RD) return msg->databuf_rd;
+    if( msg->flags & SPI_M_WR) return msg->databuf_wr;
+}
+
+#define SPIMSG_OK 	0x01
+#define SPIMSG_FAILED 	0x80
+#define SPIMSG_STARTED  0x02 
+#define SPIMSG_DONE	0x04
+
+#define SPI_MAJOR	98
+
+struct spi_driver;
+struct spi_device;
+
+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_device_add( struct device* parent, struct spi_device*, char* name );
+static inline void spi_device_del( struct spi_device* dev )
+{
+	device_unregister( &dev->dev );
+}
+static inline int spi_driver_add( struct spi_driver* drv ) 
+{
+	return driver_register( &drv->driver );
+}
+static inline void spi_driver_del( struct spi_driver* drv ) 
+{
+	driver_unregister( &drv->driver );
+}
+
+#define SPI_DEV_CHAR "spi-char"
+
+extern int spi_write(struct spi_device *dev, const char *buf, int len);
+extern int spi_read(struct spi_device *dev, char *buf, int len);
+
+extern int spi_queue( struct spi_msg* message );
+extern int spi_transfer( struct spi_msg* message, void (*status)( struct spi_msg*, int ) );
+extern int spi_bus_populate( struct device* parent, char* device, void (*assign)( struct device* parent, struct spi_device* ) );
+
+#endif				/* SPI_H */
Index: linux-2.6.10/Documentation/spi.txt
===================================================================
--- /dev/null
+++ linux-2.6.10/Documentation/spi.txt
@@ -0,0 +1,351 @@
+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 device" and "SPI device driver". The 
+"SPI bus" is hardware device, which usually called "SPI adapter", and has 
+"SPI devices" connected. From the Linux' point of view, the "SPI bus" is
+structure of type platform_device, and "SPI 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 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:
+
+  +--------------+                    +---------+
+  | platform_bus |                    | spi_bus |
+  +--------------+                    +---------+
+       |..|                                |
+       |..|--------+               +---------------+   
+     +------------+| is parent to  |  SPI devices  | 
+     | SPI busses |+-------------> |               |
+     +------------+                +---------------+
+           |                               |
+     +----------------+          +----------------------+
+     | SPI bus driver |          |    SPI device driver |
+     +----------------+          +----------------------+
+
+3.2 How do the SPI devices gets discovered and probed ?
+
+In general, the SPI bus driver cannot effective discover devices
+on its bus. Fortunately, the devices on SPI bus usually implemented
+onboard, so the following method has been chosen: the SPI bus driver
+calls the function named spi_bus_populate and passed the `topology
+string' to it. The function will parse the string and call the callback
+for each device, just before registering it. This allows bus driver
+to determine parameters like CS# for each device, retrieve them from
+string and store somewhere like spi_device->platform_data. An example:
+	err = spi_bus_populate( the_spi_bus,
+			"Dev1 0 1 2\0" "Dev2 2 1 0\0",
+			extract_name )
+In this example, function like extract_name would put the '\0' on the 
+1st space of device's name, so names will become just "Dev1", "Dev2", 
+and the rest of string will become parameters of device.
+
+3.3. DMA and SPI messages
+-------------------------
+
+To handle DMA transfers on SPI bus, any device driver might provide special
+callbacks to allocate/free/get access to buffer. These callbacks are defined 
+in subsection iii of section 4.
+To send data using DMA, the buffers should be allocated using 
+dma_alloc_coherent function. Usually buffers are allocated statically or
+using kmalloc function. 
+To allow drivers to allocate buffers in non-standard 
+When one allocates the structure for spi message, it needs to provide target
+device. If its driver wants to allocate buffer in driver-specific way, it may
+provide its own allocation/free methods: alloc and free. If driver does not
+provide these methods, kmalloc and kfree will be used.
+After allocation, the buffer must be accessed to copy the buffer to be send 
+or retrieve buffer that has been just received from device. If buffer was
+allocated using driver's alloc method, it(buffer) will be accessed using 
+get_buffer. Driver should provide accessible buffer that corresponds buffer
+allocated by driver's alloc method. If there is no get_buffer method, 
+the result of alloc will be used.
+After reading/writing from/to buffer, it will be released by call to driver's
+release_buffer method.
+
+ 
+4. SPI functions are structures reference
+-----------------------------------------
+This section describes structures and functions that listed
+in include/linux/spi.h
+
+i. struct spi_msg 
+~~~~~~~~~~~~~~~~~
+
+struct spi_msg {
+        unsigned char flags;
+        unsigned short len;     
+        unsigned long clock;
+        struct spi_device* device;
+        void          *context;
+        struct list_head link;
+        void (*status)( struct spi_msg* msg, int code );
+        void *devbuf_rd, *devbuf_wr;
+        u8 *databuf_rd, *databuf_wr;
+};
+This structure represents the message that SPI device driver sends to the
+SPI bus driver to handle.
+Fields:
+	flags 	combination of message flags
+		SPI_M_RD	"read" operation (from device to host)
+		SPI_M_WR	"write" operation (from host to device)
+		SPI_M_CS	assert the CS signal before sending the message
+		SPI_M_CSREL	clear the CS signal after sending the message
+		SPI_M_CSPOL	set clock polarity to high
+		SPI_M_CPHA	set clock phase to high
+	len	length, in bytes, of allocated buffer
+	clock	reserved, set to zero
+	device	the target device of the message
+	context	user-defined field; to associate any user data with the message
+	link	used by bus driver to queue messages
+	status	user-provided callback function to inform about message flow
+	devbuf_rd, devbuf_wr
+		so-called "device buffers". These buffers allocated by the
+		device driver, if device driver provides approproate callback.
+		Otherwise, the kmalloc API will be used.
+	databuf_rd, databuf_wr
+		pointers to access content of device buffers. They are acquired
+		using get_buffer callback, if device driver provides one.
+		Otherwise, they are just pointers to corresponding
+		device buffers
+
+struct spi_msg* spimsg_alloc( struct spi_device* device,
+                              unsigned flags,
+                              unsigned short len,
+                              void (*status)( struct spi_msg*, int code ) )
+This functions is called to allocate the spi_msg structure and set the 
+corresponding fields in structure. If device->platform_data provides callbacks
+to handle buffers, alloc/get_buffer are to be used. Returns NULL on errors.
+
+struct void spimsg_free( struct spi_msg* msg )
+Deallocate spi_msg as well as internal buffers. If msg->device->platform_data
+provides callbacks to handle buffers, release_buffer and free are to be used.
+
+u8* spimsg_buffer_rd( struct spi_msg* msg )
+u8* spimsg_buffer_wr( struct spi_msg* msg )
+u8* spimsg_buffer( struct spi_msg* )
+Return the corresponding data buffer, which can be directly modified by driver.
+spimsg_buffer checks flags and return either databuf_rd or databuf_wr basing on
+value of `flags' in spi_msg structure.
+
+ii. struct spi_device
+~~~~~~~~~~~~~~~~~~~~~
+
+struct spi_device
+{
+        char name[ BUS_ID_SIZE ];
+        struct device dev;
+};
+This structure represents the physical device on SPI bus. The SPI bus driver 
+will create and register this structure for you.
+	name	the name of the device. It should match to the SPI device
+		driver name
+	dev	field used to be registered with core
+
+int spi_device_add( struct device* parent, 
+                    struct spi_device* dev, 
+                    char* name )
+This function registers the device `dev' on the spi bus, and set its parent
+to `parent', which represents the SPI bus. The device name will be set to name,
+that should be non-empty, non-NULL string. Returns 0 on no error, error code 
+otherwise.
+
+void spi_device_del( struct spi_device* dev )
+Unregister the SPI device. Return value is ignored
+
+iii. struct spi_driver
+~~~~~~~~~~~~~~~~~~~~~~
+
+struct spi_driver {
+    	void* (*alloc)( size_t, int );
+    	void  (*free)( const void* );
+    	unsigned char* (*get_buffer)( struct spi_device*, void* );
+    	void (*release_buffer)( struct spi_device*, unsigned char*);
+    	void (*control)( struct spi_device*, int mode, u32 ctl );
+        struct device_driver driver;
+};
+This structure represents the SPI device driver object. Before registering,
+all fields of driver sub-structure should be properly filled, e.g., the 
+`bus_type' should be set to spi_bus. Otherwise, the driver will be incorrectly 
+registered and its callbacks might never been called. An example of will-
+formed spi_driver structure:
+	extern struct bus_type spi_bus;
+	static struct spi_driver pnx4008_eeprom_driver = {
+        	.driver = {
+                   	.bus = &spi_bus,
+                   	.name = "pnx4008-eeprom",
+                   	.probe = pnx4008_spiee_probe,
+                   	.remove = pnx4008_spiee_remove,
+                   	.suspend = pnx4008_spiee_suspend,
+                   	.resume = pnx4008_spiee_resume,
+               	},
+};
+The method control gets called during the processing of SPI message.
+For detailed description of malloc/free/get_buffer/release_buffer, please
+look to section 3.3, "DMA and SPI messages"
+
+
+int spi_driver_add( struct spi_driver* driver )
+Register the SPI device driver with core; returns 0 on no errors, error code
+otherwise.
+
+void spi_driver_del( struct spi_driver* driver )
+Unregisters the SPI device driver; return value ignored.
+
+iv. struct spi_bus_driver
+~~~~~~~~~~~~~~~~~~~~~~~~~
+To handle transactions over the SPI bus, the spi_bus_driver structure must 
+be prepared and registered with core. Any transactions, either synchronous 
+or asynchronous, go through spi_bus_driver->xfer function. 
+
+struct spi_bus_driver
+{
+        int (*xfer)( struct spi_msg* msgs );
+        void (*select) ( struct spi_device* arg );
+        void (*deselect)( struct spi_device* arg );
+
+        struct device_driver driver;
+};
+
+Fields:
+	xfer	pointer to function to execute actual transaction on SPI bus
+		msg	message to handle
+	select	pointer to function that gets called when bus needs to 
+		select another device to be target of transfers
+	deselect
+		pointer to function that gets called before another device
+		is selected to be the target of transfers
+
+
+spi_bus_driver_register( struct spi_bus_driver* )
+
+Register the SPI bus driver with the system. The driver sub-structure should
+be properly filled before using this function, otherwise you may get unpredi-
+ctable results when trying to exchange data. An example of correctly prepared
+spi_bus_driver structure:
+	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,
+};
+The driver and corresponding platform device are matched by name, so, in 
+order the example abive to work, the platform_device named "spipnx" should
+be registered somewhere.
+
+void spi_bus_driver_unregister( struct spi_bus_driver* )
+
+Unregister the SPI bus driver registered by call to spi_buys_driver_register
+function; returns void.
+
+void spi_bus_populate( struct device* parent, 
+			      char* devices,
+			      void (*callback)( struct device* parent, struct spi_device* new_one ) )
+This function usually called by SPI bus drivers in order to populate the SPI
+bus (see also section 3.2, "How the SPI devices gets discovered and probed ?").
+After creating the spi_device, the spi_bus_populate calls the `callback' 
+function to allow to modify spi_device's fields before registering it with core.
+	parent	pointer to SPI bus
+	devices	string representing the current topology of SPI bus. It should
+		be formed like
+		"dev-1_and_its_info\0dev-2_and_its_info\0another_device\0\0"
+		the spi_bus_populate delimits this string by '\0' characters,
+		creates spi_device and after calling the callback registers the
+		spi_device
+	callback
+		pointer to function which could modify spi_device fields just
+		before registering them with core
+
+v. spi_transfer and spi_queue
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The driver that uses SPI core can initiate transfers either by calling 
+spi_transfer function (that will wait till the transfer is funished) or
+queueing the message using spi_queue function (you need to provide function
+that will be called during message is processed). In any way, you need to
+prepare the spimsg structure and know the target device to your message to 
+be sent.
+
+int spi_transfer( struct spi_msg msgs, 
+                  void (*callback)( struct spi_msg* msg, int ) )
+If callback is zero, start synchronous transfer. Otherwise, queue 
+the message.
+	msg		message to be handled
+	callback	the callback function to be called during
+			message processing. If NULL, the function
+			will wait until end of processing.
+
+int spi_queue( struct spi_msg* msg )
+
+Queue the only message to the device. Returns status of queueing. To obtain
+status of message processing, you have to provide `status' callback in message
+and examine its parameters
+	msg	message to be queued
+
+vi. the spi_bus variable
+~~~~~~~~~~~~~~~~~~~~~~~~
+This variable is created during initialization of spi core, and has to be 
+specified as `bus' on any SPI device driver (look to section iii, "struct
+spi_driver" ). If you do not specify spi_bus, your driver will be never
+matched to spi_device and never be probed with hardware. Note that 
+spi_bus.match points to function that matches drivers and devices by name,
+so SPI devices and their drivers should have the same name.
+
+5. How to contact authors
+-------------------------
+Do you have any comments ? Enhancements ? Device driver ? Feel free
+to contact me: 
+	dpervushin@gmail.com
+	dimka@pervushin.msk.ru
+Visit our project page:
+	http://spi-devel.sourceforge.net
+Subscribe to mailing list:
+	spi-devel-general@lists.sourceforge.net
+



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: SPI
  2005-09-26 11:12 SPI dmitry pervushin
@ 2005-09-26 12:31 ` Eric Piel
  2005-09-26 12:37   ` [spi-devel-general] SPI dmitry pervushin
  2005-09-26 16:20 ` SPI Grant Likely
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Eric Piel @ 2005-09-26 12:31 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

09/26/2005 01:12 PM, dmitry pervushin wrote/a écrit:
> Hello guys,
> 
> I am attaching the next incarnation of SPI core; feel free to comment it.
Hello,

Very little comments...


> Index: linux-2.6.10/drivers/spi/Kconfig
> ===================================================================
> --- /dev/null
> +++ linux-2.6.10/drivers/spi/Kconfig
> @@ -0,0 +1,33 @@
> +#
> +# SPI device configuration
> +#
> +menu "SPI support"
> +
> +config SPI
> +	default Y
> +	tristate "SPI support"
> +        default false
> +	help
> +	  Say Y if you need to enable SPI support on your kernel
SPI is far from being well know, please put more help. At least define 
SPI as "Serial Peripheral Interface" and suggest the user to have a look 
at Documentation/spi.txt . IMHO, it's also convenient if you give the 
name of the module that will be created (spi?).


> Index: linux-2.6.10/Documentation/spi.txt
> ===================================================================
> --- /dev/null
> +++ linux-2.6.10/Documentation/spi.txt
:
:
> +
> +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/ .
Broken link, it is 
http://en.wikipedia.org/wiki/Serial_Peripheral_Interface (no trailing /)


Cheers,
Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-26 12:31 ` SPI Eric Piel
@ 2005-09-26 12:37   ` dmitry pervushin
  0 siblings, 0 replies; 19+ messages in thread
From: dmitry pervushin @ 2005-09-26 12:37 UTC (permalink / raw)
  To: Eric Piel; +Cc: Linux Kernel Mailing List, spi-devel-general

On Mon, 2005-09-26 at 14:31 +0200, Eric Piel wrote:
> > +	  Say Y if you need to enable SPI support on your kernel
> SPI is far from being well know, please put more help. At least define 
> SPI as "Serial Peripheral Interface" and suggest the user to have a look 
> at Documentation/spi.txt . IMHO, it's also convenient if you give the 
> name of the module that will be created (spi?).
The module name is spi-core. If one who configures the kernel does not
know about the SPI, it seems that it is better to keep the option
unchanged... However, I edit the text in Kconfig.
> Broken link, it is 
> http://en.wikipedia.org/wiki/Serial_Peripheral_Interface (no trailing /)
Fixed.

Thank you for comments,
dmitry 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: SPI
  2005-09-26 11:12 SPI dmitry pervushin
  2005-09-26 12:31 ` SPI Eric Piel
@ 2005-09-26 16:20 ` Grant Likely
  2005-09-27  7:39   ` [spi-devel-general] SPI dmitry pervushin
  2005-09-26 16:25 ` SPI Valdis.Kletnieks
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Grant Likely @ 2005-09-26 16:20 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

resend; sorry.  Forgot to cc: the list.

On 9/26/05, dmitry pervushin <dpervushin@gmail.com> wrote:
> Hello guys,
>
> I am attaching the next incarnation of SPI core; feel free to comment it.

===================================================================
> --- /dev/null
> +++ linux-2.6.10/Documentation/spi.txt
> @@ -0,0 +1,351 @@
> +Documentation/spi.txt

> +3.1 The SPI outline
> +
> +The SPI infrastructure deals with several levels of abstraction. They are
> +"SPI bus", "SPI bus driver", "SPI device" and "SPI device driver". The
Would "SPI slave" or "SPI slave device" be better terminology than
"SPI device"?  That way the terminology matches how SPI hardware docs
are usually written.  (not a big deal, just thought I'd ask)

> +"SPI bus" is hardware device, which usually called "SPI adapter", and has
> +"SPI devices" connected. From the Linux' point of view, the "SPI bus" is
> +structure of type platform_device, and "SPI 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 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:
> +
> +  +--------------+                    +---------+
> +  | platform_bus |                    | spi_bus |
> +  +--------------+                    +---------+
> +       |..|                                |
> +       |..|--------+               +---------------+
> +     +------------+| is parent to  |  SPI devices  |
> +     | SPI busses |+-------------> |               |
> +     +------------+                +---------------+
> +           |                               |
> +     +----------------+          +----------------------+
> +     | SPI bus driver |          |    SPI device driver |
> +     +----------------+          +----------------------+
Helpful diagram.  :)

> +
> +3.2 How do the SPI devices gets discovered and probed ?
> +
> +In general, the SPI bus driver cannot effective discover devices
> +on its bus. Fortunately, the devices on SPI bus usually implemented
> +onboard, so the following method has been chosen: the SPI bus driver
> +calls the function named spi_bus_populate and passed the `topology
> +string' to it. The function will parse the string and call the callback
> +for each device, just before registering it. This allows bus driver
> +to determine parameters like CS# for each device, retrieve them from
> +string and store somewhere like spi_device->platform_data. An example:
> +       err = spi_bus_populate( the_spi_bus,
> +                       "Dev1 0 1 2\0" "Dev2 2 1 0\0",
> +                       extract_name )
In my mind, this is not ideal.  For example, the MPC5200 has 4 PSC
ports which can be in SPI mode.  The SPI bus driver should/will not
know what devices are attached to it.  It should be the responsibility
of the board setup code to populate the bus.... on the other hand,
perhaps the bus driver should look to it's platform_device structure
to find a table of attached devices.  Generic platform_device parsing
code could be used by all SPI bus drivers.

Along the same lines, an SPI bus driver may not know the board
specific way SS lines are driven.  If GPIO is used as SS lines then
each SPI bus will need a different SS routine.  However, it looks like
this is not an issue for your infrastructure.  The individual SPI bus
driver can be handed a SS callback by the board setup code for each
SPI bus.

> +In this example, function like extract_name would put the '\0' on the
> +1st space of device's name, so names will become just "Dev1", "Dev2",
> +and the rest of string will become parameters of device.
I don't think it's wise to use '\0' as a delimiter.  Sure it makes
parsing really simple when the string passed in is formed correctly,
but if someone misses the last '\0' you have no way to know where the
string ends.  It also makes it difficult support passing a device
string from the kernel command line.

> +4. SPI functions are structures reference
> +-----------------------------------------
> +This section describes structures and functions that listed
> +in include/linux/spi.h
I would like to see this function and structure reference in the spi.h
file itself rather than here.  Better chance of it being kept up to
date that way.

> +
> +i. struct spi_msg
> +~~~~~~~~~~~~~~~~~
> +
> +struct spi_msg {
> +        unsigned char flags;
> +        unsigned short len;
> +        unsigned long clock;
> +        struct spi_device* device;
> +        void          *context;
> +        struct list_head link;
> +        void (*status)( struct spi_msg* msg, int code );
> +        void *devbuf_rd, *devbuf_wr;
> +        u8 *databuf_rd, *databuf_wr;
> +};
> +This structure represents the message that SPI device driver sends to the
> +SPI bus driver to handle.
Is there any way for the SPI device to constrain the clock rate for a
transfer?  For example, if the devices maximum speed is lower than the
bus maximum speed.

I looked over the code and I didn't notice anything obviously
incorrect.  I greatly appreciate the large block of documentation.

Overall, I like.  It looks like it does what I need it to.  If I get a
chance this week I'll port my SPI drivers to it and try it out on my
MPC5200 board.

Thanks!
g.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: SPI
  2005-09-26 11:12 SPI dmitry pervushin
  2005-09-26 12:31 ` SPI Eric Piel
  2005-09-26 16:20 ` SPI Grant Likely
@ 2005-09-26 16:25 ` Valdis.Kletnieks
  2005-09-26 16:46   ` [spi-devel-general] SPI Vitaly Wool
  2005-09-26 20:25 ` SPI Jesper Juhl
  2005-09-27 12:43 ` SPI Greg KH
  4 siblings, 1 reply; 19+ messages in thread
From: Valdis.Kletnieks @ 2005-09-26 16:25 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

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

On Mon, 26 Sep 2005 15:12:14 +0400, dmitry pervushin said:
> Hello guys,
> 
> I am attaching the next incarnation of SPI core; feel free to comment it.

> +/* The devfs code is contributed by Philipp Matthias Hahn 
> +   <pmhahn@titan.lahn.de> */

> +/* devfs code corrected to support automatic device addition/deletion
> +   by Vitaly Wool <vwool@ru.mvista.com> (C) 2004 MontaVista Software, Inc. 
> + */

I'd like to thank Vitaly and Philipp for their work, which was probably useful
at the time, but I've always wondered - when cleaning up code, should such comments
be removed too, or left as historical reminders?  The MAINTAINERS file seems
to get cleaned most of the time, the CREDITS doesn't - which way should
in-source comments go?


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-26 16:25 ` SPI Valdis.Kletnieks
@ 2005-09-26 16:46   ` Vitaly Wool
  0 siblings, 0 replies; 19+ messages in thread
From: Vitaly Wool @ 2005-09-26 16:46 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: dmitry pervushin, Linux Kernel Mailing List, spi-devel-general

My POV is that those lines should go away.

Best regards,
   Vitaly

Valdis.Kletnieks@vt.edu wrote:

>On Mon, 26 Sep 2005 15:12:14 +0400, dmitry pervushin said:
>  
>
>>Hello guys,
>>
>>I am attaching the next incarnation of SPI core; feel free to comment it.
>>    
>>
>
>  
>
>>+/* The devfs code is contributed by Philipp Matthias Hahn 
>>+   <pmhahn@titan.lahn.de> */
>>    
>>
>
>  
>
>>+/* devfs code corrected to support automatic device addition/deletion
>>+   by Vitaly Wool <vwool@ru.mvista.com> (C) 2004 MontaVista Software, Inc. 
>>+ */
>>    
>>
>
>I'd like to thank Vitaly and Philipp for their work, which was probably useful
>at the time, but I've always wondered - when cleaning up code, should such comments
>be removed too, or left as historical reminders?  The MAINTAINERS file seems
>to get cleaned most of the time, the CREDITS doesn't - which way should
>in-source comments go?
>
>  
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: SPI
  2005-09-26 11:12 SPI dmitry pervushin
                   ` (2 preceding siblings ...)
  2005-09-26 16:25 ` SPI Valdis.Kletnieks
@ 2005-09-26 20:25 ` Jesper Juhl
  2005-09-27 12:43 ` SPI Greg KH
  4 siblings, 0 replies; 19+ messages in thread
From: Jesper Juhl @ 2005-09-26 20:25 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

On Monday 26 September 2005 13:12, dmitry pervushin wrote:
> Hello guys,
> 
> I am attaching the next incarnation of SPI core; feel free to comment it.
> 
A few small style comments below.

General notes:
	Please use only tabs for indentation.
	Please get rid of all the trailing whitespace. A small sed script 
	 like this will do:  sed -r s/"[ \t]+$"/""/
	Please use only one statement pr line.
	Please get rid of the extra whitespace after opening paren and before
	 closing paren:  not like ( this ), but like (this).
	Please use a single space after if. Like this: if (foo), not if(foo).
	For pointer variables,  "type *name" is usually prefered, 
	 not "type* name" or "type * name".

See the changes I've made below for more details (note: I may have missed some
 bits, if so, please correct what I missed as well) :-)

See Documentation/CodingStyle for yet more details and rationale.



[snip]
> + */
> +static int spi_bus_match_name(struct device *dev, struct device_driver *drv)
> +{
> +	return  !strcmp (drv->name, SPI_DEV_CHAR) || 

	return  !strcmp(drv->name, SPI_DEV_CHAR) || 


[snip]
> + * Parameters:
> + * 	struct device* dev	the 'bus' device
> + * 	void* context		not used. Will be NULL

 * 	struct device *dev	the 'bus' device
 * 	void *context		not used. Will be NULL


[snip]
> +int __spi_bus_free(struct device *dev, void *context)
> +{
> +	struct spi_bus_data *pd = dev->platform_data;
> +
> +	atomic_inc(&pd->exiting);
> +	kthread_stop(pd->thread);
> +	kfree(pd);
> +
> +	dev_dbg( dev, "unregistering children\n" );

	dev_dbg(dev, "unregistering children\n");


> +	/* 
> +	 * NOTE: the loop below might needs redesign. Currently
> +	 *       we delete devices from the head of children list
> +	 *       until the list is empty; that's because the function
> +	 *       device_for_each_child will hold the semaphore needed 
> +	 *       for deletion of device
> +	 */
> +	while( !list_empty( &dev->children ) ) {
> +		struct device* child = list_entry ( dev->children.next, struct device, node );
> +	    	spi_device_del (TO_SPI_DEV (child) );

	while(!list_empty(&dev->children)) {
		struct device *child = list_entry(dev->children.next, struct device, node);
	    	spi_device_del(TO_SPI_DEV(child));


[snip]
> + * spi_bus_driver_unregister
> + *
> + * unregisters the SPI bus from the system. Before unregistering, it deletes
> + * each SPI device on the bus using call to __spi_device_free
> + *
> + * Parameters:
> + *  	struct spi_bus_driver* bus_driver	the bus driver

 *  	struct spi_bus_driver *bus_driver	the bus driver


[snip]
> +void spi_bus_driver_unregister(struct spi_bus_driver *bus_driver)
> +{
> +	if (bus_driver) {
> +		driver_for_each_dev( &bus_driver->driver, NULL, __spi_bus_free);

		driver_for_each_dev(&bus_driver->driver, NULL, __spi_bus_free);


[snip]
> + * 	struct device* dev

 * 	struct device *dev

> + * Return value:
> + * 	none
> + */
> +void spi_device_release( struct device* dev )

void spi_device_release(struct device *dev)


[snip]
> + * 	struct device* parent		the 'bus' device
> + * 	struct spi_device* dev		new device to be added
> + * 	char* name			name of device. Should not be NULL

 * 	struct device *parent		the 'bus' device
 * 	struct spi_device *dev		new device to be added
 * 	char *name			name of device. Should not be NULL

[snip]
> +int spi_device_add(struct device *parent, struct spi_device *dev, char *name)
> +{
> +	if (!name || !dev) 
> +	    return -EINVAL;
> +	    
> +	memset(&dev->dev, 0, sizeof(dev->dev));
> +	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 ) );

	strncpy(dev->dev.bus_id, name, sizeof(dev->dev.bus_id));


[snip]
> + * spi_queue
> + *
> + * Queue the message to be processed asynchronously
> + *
> + * Parameters:
> + *  	struct spi_msg* msg            message to be sent

 *  	struct spi_msg *msg            message to be sent


> + * Return value:
> + *  	0 on no errors, negative error code otherwise
> + */
> +int spi_queue( struct spi_msg *msg)

int spi_queue(struct spi_msg *msg)


> +{
> +	struct device* dev = &msg->device->dev;

	struct device *dev = &msg->device->dev;


> +	struct spi_bus_data *pd = dev->parent->platform_data;
> +
> +	down(&pd->lock);
> +	list_add_tail(&msg->link, &pd->msgs);
> +	dev_dbg(dev->parent, "message has been queued\n" );

	dev_dbg(dev->parent, "message has been queued\n");


[snip]
> + * __spi_transfer_callback
> + *
> + * 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
> + *
> + * Parameters:
> + * 	struct spimsg* msg	message that is being processed now

 * 	struct spimsg *msg	message that is being processed now


> + * 	int code		status of processing
> + */
> +static void __spi_transfer_callback( struct spi_msg* msg, int code )

static void __spi_transfer_callback(struct spi_msg *msg, int code)



> +{
> +	if( code & (SPIMSG_OK|SPIMSG_FAILED) ) 
> +		complete( (struct completion*)msg->context );

	if (code & (SPIMSG_OK|SPIMSG_FAILED)) 
		complete((struct completion*)msg->context);

[snip]
> + * spi_transfer
> + *
> + * Process the SPI message, by queuing it to the driver and either
> + * immediately return or waiting till the end-of-processing
> + *
> + * Parameters:
> + * 	struct spi_msg* msg	message to process

 * 	struct spi_msg *msg	message to process


[snip]
> +int spi_transfer( struct spi_msg* msg, void (*callback)( struct spi_msg*, int ) )

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


> +{
> +	struct completion msg_done;
> +	int err = -EINVAL;
> +
> +	if( callback && !msg->status ) {

	if (callback && !msg->status) {


[snip]
> +	if( !callback ) {
> +		if( !msg->status ) {
> +			init_completion( &msg_done );
> +			msg->context = &msg_done;
> +			msg->status = __spi_transfer_callback;
> +			spi_queue( msg );
> +			wait_for_completion( &msg_done );
> +			err = 0;
> +		} else {
> +			err = spi_queue( msg );

	if (!callback) {
		if (!msg->status) {
			init_completion(&msg_done);
			msg->context = &msg_done;
			msg->status = __spi_transfer_callback;
			spi_queue(msg);
			wait_for_completion(&msg_done);
			err = 0;
		} else {
			err = spi_queue(msg);


[snip]
> + * spi_thread
> + * 
> + * This function is started as separate thread to perform actual 
> + * transfers on SPI bus
> + *
> + * Parameters:
> + *	void* context 		pointer to struct spi_bus_data 

 *	void *context 		pointer to struct spi_bus_data 


[snip]
> +	while (!kthread_should_stop()) {
> +
^^^^^ superfluous blank line.
> +		wait_event_interruptible(bd->queue, spi_thread_awake(bd));


[snip]
> +			if( bd->bus->set_clock && msg->clock )
> +				bd->bus->set_clock( 
> +					msg->device->dev.parent, msg->clock );
> +			xfer_status = bd->bus->xfer( msg );

			if (bd->bus->set_clock && msg->clock)
				bd->bus->set_clock(	<-- this line has trailing whitespace.
					msg->device->dev.parent, msg->clock);
			xfer_status = bd->bus->xfer(msg);



[snip]
> + * spi_write 
> + * 	send data to a device on an SPI bus
> + * Parameters:
> + * 	spi_device* dev		the target device
> + *	char* buf		buffer to be sent

 * 	spi_device *dev		the target device
 *	char *buf		buffer to be sent


[snip]
> +	ret = spi_transfer( msg, NULL );

	ret = spi_transfer(msg, NULL);


[snip]
> + * spi_write 
> + * 	receive data from a device on an SPI bus
> + * Parameters:
> + * 	spi_device* dev		the target device
> + *	char* buf		buffer to be sent

 * 	spi_device *dev		the target device
 *	char *buf		buffer to be sent


[snip]
> +int spi_read(struct spi_device *dev, char *buf, int len)
> +{
> +	int ret;
> +	struct spimsg *msg = spimsg_alloc(dev, SPI_M_RD, len, NULL);
> +
> +	ret = spi_transfer( msg, NULL );

	ret = spi_transfer(msg, NULL);


[snip]
> +int spi_bus_populate(struct device *parent,
> +		     char *devices,
> +		     void (*callback) (struct device * bus,
> +				       struct spi_device * new_dev))

int spi_bus_populate(struct device *parent,
		char *devices,
		void (*callback)(struct device *bus,
			struct spi_device *new_dev))



[snip]
> +	while (devices[0]) {
> +		dev_dbg(parent, "discovered new SPI device, name '%s'\n",
> +			devices);
> +		new_device = kmalloc(sizeof(struct spi_device), GFP_KERNEL);
> +		if (!new_device) {
> +			break;
> +		}
> +		if (spi_device_add(parent, new_device, devices)) {
> +			break;
> +		}
> +		if (callback) {
> +			callback(parent, new_device);
> +		}

		if (!new_device)
			break;
		if (spi_device_add(parent, new_device, devices))
			break;
		if (callback)
			callback(parent, new_device);

We usually don't use curly braces for if statements when the body of the if
is only a single statement.


[snip]
> +int __init spi_core_init( void )

int __init spi_core_init(void)

[snip]
> +++ linux-2.6.10/drivers/spi/spi-dev.c
[snip]
[snip]
> +#include <linux/init.h>
> +#include <asm/uaccess.h>
> +#include <linux/spi.h>

#include <linux/init.h>
#include <linux/spi.h>
#include <asm/uaccess.h>

conventionally, asm/ includes are placed last.


[snip]
> +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 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);


[snip]
> +static int spidev_probe(struct device *dev)
> +{
> +	struct spidev_driver_data *drvdata;
> +
> +	drvdata = kmalloc(sizeof(struct spidev_driver_data), GFP_KERNEL);
> +	if ( !drvdata) {
> +		dev_dbg( dev, "allocating drvdata failed\n" );

	if (!drvdata) {
		dev_dbg(dev, "allocating drvdata failed\n");


[snip]
> +	dev_dbg( dev, " added\n" );

	dev_dbg(dev, " added\n");


[snip]
> +static int spidev_remove(struct device *dev)
> +{
> +	struct spidev_driver_data *drvdata;
> +
> +	drvdata = (struct spidev_driver_data *)dev_get_drvdata(dev);
> +	class_simple_device_remove(MKDEV(SPI_MAJOR, drvdata->minor));
> +	kfree(drvdata);
> +	dev_dbg( dev, " removed\n" );

	dev_dbg(dev, " removed\n");


[snip]
> +static ssize_t spidev_read(struct file *file, char *buf, size_t count,
> +			   loff_t * offset)

static ssize_t spidev_read(struct file *file, char *buf, size_t count,
		loff_t *offset)


> +{
> +	struct spi_device *dev = (struct spi_device *)file->private_data;
> +	if( count > SPI_TRANSFER_MAX ) count = SPI_TRANSFER_MAX;
> +	return spi_read(dev, buf, count );

	if (count > SPI_TRANSFER_MAX)
		count = SPI_TRANSFER_MAX;
	return spi_read(dev, buf, count);


[snip]
> +static ssize_t spidev_write(struct file *file, const 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)


> +{
> +	struct spi_device *dev = (struct spi_device *)file->private_data;
> +	if( count > SPI_TRANSFER_MAX ) count = SPI_TRANSFER_MAX;
> +	return spi_write( dev, buf, count );

	if (count > SPI_TRANSFER_MAX)
		count = SPI_TRANSFER_MAX;
	return spi_write(dev, buf, count);


[snip]
> +	if (NULL == drvdata) {

	if (drvdata == NULL) {

debatable, but I believe the most common style is what I changed it to.


[snip]
> +++ linux-2.6.10/include/linux/spi.h
[snip]
> +/*
> + *  linux/include/linux/spi/spi.h
> + *
> + *  Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
> + *
> + * 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

Blank line between end of comment and start of code at the top of file seems
to be most common.


[snip]
> +struct spi_bus_data
> +{
> +	struct semaphore lock;
> +	struct list_head msgs;
> +	atomic_t exiting;
> +	struct task_struct* thread;
> +	wait_queue_head_t queue; 
> +	struct spi_device* selected_device;
> +	struct spi_bus_driver* bus;

	struct task_struct *thread;
	wait_queue_head_t queue; 
	struct spi_device *selected_device;
	struct spi_bus_driver *bus;


[snip]
> +#define TO_SPI_BUS_DRIVER(drv) container_of( drv, struct spi_bus_driver, driver )

#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 (*select)( struct spi_device* dev );
> +	void (*deselect)( struct spi_device* dev );
> +	void (*set_clock)( struct device* bus_device, u32 clock_hz );
> +	struct device_driver driver;

	int (*xfer)(struct spi_msg *msg);
	void (*select)(struct spi_device *dev);
	void (*deselect)(struct spi_device *dev);
	void (*set_clock)(struct device *bus_device, u32 clock_hz);


[snip]
> +#define TO_SPI_DEV(device) container_of( device, struct spi_device, dev )

#define TO_SPI_DEV(device) container_of(device, struct spi_device, dev)


> +struct spi_device 
> +{
> +	char name[ BUS_ID_SIZE ];

	char name[BUS_ID_SIZE];


[snip]
> +#define TO_SPI_DRIVER(drv) container_of( drv, struct spi_driver, driver )

#define TO_SPI_DRIVER(drv) container_of(drv, struct spi_driver, driver)


> +struct spi_driver {
> +    	void* (*alloc)( size_t, int );
> +    	void  (*free)( const void* );
> +    	unsigned char* (*get_buffer)( struct spi_device*, void* );
> +    	void (*release_buffer)( struct spi_device*, unsigned char*);
> +    	void (*control)( struct spi_device*, int mode, u32 ctl );

struct spi_driver {
    	void *(*alloc)(size_t, int);
    	void (*free)(const void *);
    	unsigned char *(*get_buffer)(struct spi_device *, void *);
    	void (*release_buffer)(struct spi_device *, unsigned char *);
    	void (*control)(struct spi_device *, int mode, u32 ctl);


[snip]
> +#define SPI_DEV_DRV( device )  TO_SPI_DRIVER( (device)->dev.driver )

#define SPI_DEV_DRV(device) TO_SPI_DRIVER((device)->dev.driver)

> +
> +#define spi_device_lock( dev ) /* down( dev->dev.sem ) */
> +#define spi_device_unlock( dev ) /* up( dev->dev.sem ) */

#define spi_device_lock(dev) /* down(dev->dev.sem) */
#define spi_device_unlock(dev) /* up(dev->dev.sem) */



> +/*
> + * struct spi_msg
> + *
> + * This structure represent the SPI message internally. You should never use fields of this structure directly
> + * Please use corresponding functions to create/destroy/access fields

 * This structure represent the SPI message internally.
 * You should never use fields of this structure directly.
 * Please use corresponding functions to create/destroy/access fields


[snip]
> +struct spi_msg {
> +	unsigned char flags;
> +#define SPI_M_RD	0x01
> +#define SPI_M_WR	0x02	/**< Write mode flag */
> +#define SPI_M_CSREL	0x04	/**< CS release level at end of the frame  */
> +#define SPI_M_CS	0x08	/**< CS active level at begining of frame ( default low ) */
> +#define SPI_M_CPOL	0x10	/**< Clock polarity */
> +#define SPI_M_CPHA	0x20	/**< Clock Phase */
> +	unsigned short len;	/* msg length           */
> +	unsigned long clock;
> +	struct spi_device* device;
> +	void	      *context;
> +	struct list_head link; 
> +	void (*status)( struct spi_msg* msg, int code );

#define SPI_M_WR	0x02	/* Write mode flag */
#define SPI_M_CSREL	0x04	/* CS release level at end of the frame */
#define SPI_M_CS	0x08	/* CS active level at begining of frame (default low) */
#define SPI_M_CPOL	0x10	/* Clock polarity */
#define SPI_M_CPHA	0x20	/* Clock Phase */
	unsigned short len;	/* msg length */
	unsigned long clock;
	struct spi_device *device;
	void *context;
	struct list_head link; 
	void (*status)(struct spi_msg *msg, int code);


[snip]
> +static inline struct spi_msg* spimsg_alloc( struct spi_device* device, 
> +					    unsigned flags,
> +					    unsigned short len, 
> +					    void (*status)( struct spi_msg*, int code ) )

static inline struct spi_msg* spimsg_alloc( struct spi_device* device, 
		unsigned flags, unsigned short len,
		void (*status)(struct spi_msg *, int code))

> +{
> +    struct spi_msg* msg;
> +    struct spi_driver* drv = SPI_DEV_DRV( device );
> +    
> +    msg = kmalloc( sizeof( struct spi_msg ), GFP_KERNEL );
> +    if( !msg )
> +	return NULL;
> +    memset( msg, 0, sizeof( struct spi_msg ) );

    struct spi_msg *msg;
    struct spi_driver *drv = SPI_DEV_DRV(device);
    
    msg = kmalloc(sizeof(struct spi_msg), GFP_KERNEL);
    if (!msg)
	return NULL;
    memset(msg, 0, sizeof(struct spi_msg));

In addition to the spacing changes you also seem to be using spaces for 
indentation here instead of tabs. Please use only tabs for indentation - this 
is true for other locations in the file as well, but I'm only mentioning it 
once here.


[snip]
> +    INIT_LIST_HEAD( &msg->link );
> +    if( flags & SPI_M_RD ) {
> +        msg->devbuf_rd =  drv->alloc ? 
> +	    drv->alloc( len, GFP_KERNEL ):kmalloc( len, GFP_KERNEL);
> +	msg->databuf_rd = drv->get_buffer ? 
> +	    drv->get_buffer( device, msg->devbuf_rd ) : msg->devbuf_rd;
> +    }

    INIT_LIST_HEAD(&msg->link);
    if (flags & SPI_M_RD) {
        msg->devbuf_rd =  drv->alloc ? 
	    drv->alloc(len, GFP_KERNEL) : kmalloc(len, GFP_KERNEL);
	msg->databuf_rd = drv->get_buffer ? 
	    drv->get_buffer(device, msg->devbuf_rd) : msg->devbuf_rd;
    }


> +    if( flags & SPI_M_WR ) {
> +        msg->devbuf_wr =  drv->alloc ? 
> +	    drv->alloc( len, GFP_KERNEL ):kmalloc( len, GFP_KERNEL);
> +	msg->databuf_wr = drv->get_buffer ? 
> +	    drv->get_buffer( device, msg->devbuf_wr ) : msg->devbuf_wr;
> +    }

	if (flags & SPI_M_WR) {
		msg->devbuf_wr =  drv->alloc ? 
			drv->alloc(len, GFP_KERNEL) :
				kmalloc(len, GFP_KERNEL);
		msg->databuf_wr = drv->get_buffer ? 
			drv->get_buffer(device, msg->devbuf_wr) :
				msg->devbuf_wr;
    }


> +    pr_debug( "%s: msg = %p, RD=(%p,%p) WR=(%p,%p). Actual flags = %s+%s\n",
> +		   __FUNCTION__,
> +		  msg, 
> +		  msg->devbuf_rd, msg->databuf_rd,
> +		  msg->devbuf_wr, msg->databuf_wr, 
> +		  msg->flags & SPI_M_RD ? "RD" : "~rd",
> +		  msg->flags & SPI_M_WR ? "WR" : "~wr" );

pr_debug("%s: msg = %p, RD=(%p,%p) WR=(%p,%p). Actual flags = %s+%s\n",
	__FUNCTION__,
	msg, 
	msg->devbuf_rd, msg->databuf_rd,
	msg->devbuf_wr, msg->databuf_wr, 
	msg->flags & SPI_M_RD ? "RD" : "~rd",
	msg->flags & SPI_M_WR ? "WR" : "~wr");


[snip]
> +static inline void spimsg_free( struct spi_msg * msg )
> +{
> +    void (*do_free)( const void* ) = kfree;
> +    struct spi_driver* drv = SPI_DEV_DRV( msg->device );

static inline void spimsg_free(struct spi_msg *msg)
{
	void (*do_free)(const void *) = kfree;
	struct spi_driver *drv = SPI_DEV_DRV(msg->device);

> +    
> +    if( msg ) {
> +	    if( drv->free ) 
> +		do_free = drv->free;
> +	    if( drv->release_buffer ) {
> +		if( msg->databuf_rd) 
> +		    drv->release_buffer( msg->device, msg->databuf_rd );
> +    		if( msg->databuf_wr) 
> +		    drv->release_buffer( msg->device, msg->databuf_wr );
> +	}
> +	if( msg->devbuf_rd ) 
> +	    do_free( msg->devbuf_rd );
> +	if( msg->devbuf_wr)
> +	    do_free( msg->devbuf_wr );
> +	kfree( msg );
> +    }

	if (msg) {
		if (drv->free) 
			do_free = drv->free;
		if (drv->release_buffer) {
			if (msg->databuf_rd) 
				drv->release_buffer(msg->device, msg->databuf_rd);
			if (msg->databuf_wr) 
				drv->release_buffer( msg->device, msg->databuf_wr);
		}
		if (msg->devbuf_rd) 
			do_free( msg->devbuf_rd);
		if (msg->devbuf_wr)
			do_free( msg->devbuf_wr);
		kfree(msg);
	}




[snip]
> +static inline u8* spimsg_buffer_rd( struct spi_msg* msg ) 

static inline u8 *spimsg_buffer_rd(struct spi_msg *msg) 


[snip]
> +static inline u8* spimsg_buffer_wr( struct spi_msg* msg ) 

static inline u8 *spimsg_buffer_wr(struct spi_msg *msg) 


[snip]
> +static inline u8* spimsg_buffer( struct spi_msg* msg ) 
> +{
> +    if( !msg ) return NULL;
> +    if( ( msg->flags & (SPI_M_RD|SPI_M_WR) ) == (SPI_M_RD|SPI_M_WR) ) {
> +	printk( KERN_ERR"%s: what buffer do you really want ?\n", __FUNCTION__ );
> +	return NULL;
> +    }
> +    if( msg->flags & SPI_M_RD) return msg->databuf_rd;
> +    if( msg->flags & SPI_M_WR) return msg->databuf_wr;
> +}

static inline u8 *spimsg_buffer(struct spi_msg* msg)
{
	if (!msg)
		return NULL;
	if ((msg->flags & (SPI_M_RD|SPI_M_WR)) == (SPI_M_RD|SPI_M_WR)) {
		printk(KERN_ERR "%s: what buffer do you really want ?\n",
			__FUNCTION__ );
		return NULL;
	}
	if (msg->flags & SPI_M_RD)
		return msg->databuf_rd;
	if (msg->flags & SPI_M_WR)
		return msg->databuf_wr;
}


> +
> +#define SPIMSG_OK 	0x01
> +#define SPIMSG_FAILED 	0x80
> +#define SPIMSG_STARTED  0x02 
> +#define SPIMSG_DONE	0x04
> +
> +#define SPI_MAJOR	98

#define SPIMSG_OK	0x01
#define SPIMSG_FAILED	0x80
#define SPIMSG_STARTED	0x02 
#define SPIMSG_DONE	0x04

#define SPI_MAJOR	98

It may not be obvious what change I made here, so I'll tell you. You were
mixing spaces and tabs between the defined named and the value, I've changed
it to only use a single tab (it still lines up nicely).

[snip]
> +static inline int spi_bus_driver_register( struct spi_bus_driver* bus_driver )
> +{
> +	return driver_register( &bus_driver->driver );
> +}

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_device_add( struct device* parent, struct spi_device*, char* name );

void spi_bus_driver_unregister(struct spi_bus_driver *);
int spi_bus_driver_init(struct spi_bus_driver * driver, struct device *dev);
int spi_device_add(struct device *parent, struct spi_device *, char *name);


> +static inline void spi_device_del( struct spi_device* dev )
> +{
> +	device_unregister( &dev->dev );
> +}

static inline void spi_device_del(struct spi_device *dev)
{
	device_unregister(&dev->dev);
}

> +static inline int spi_driver_add( struct spi_driver* drv ) 
> +{
> +	return driver_register( &drv->driver );
> +}

static inline int spi_driver_add(struct spi_driver *drv) 
{
	return driver_register(&drv->driver);
}

> +static inline void spi_driver_del( struct spi_driver* drv ) 
> +{
> +	driver_unregister( &drv->driver );
> +}

static inline void spi_driver_del(struct spi_driver *drv)
{
	driver_unregister(&drv->driver);
}


[snip]
> +extern int spi_queue( struct spi_msg* message );
> +extern int spi_transfer( struct spi_msg* message, void (*status)( struct spi_msg*, int ) );
> +extern int spi_bus_populate( struct device* parent, char* device, void (*assign)( struct device* parent, struct spi_device* ) );

extern int spi_queue(struct spi_msg* message);
extern int spi_transfer(struct spi_msg *message,
		void (*status)(struct spi_msg *, int));
extern int spi_bus_populate(struct device *parent, char *device,
		void (*assign)(struct device *parent, struct spi_device *));

> +
> +#endif				/* SPI_H */

#endif	/* SPI_H */



-- 
Jesper Juhl <jesper.juhl@gmail.com>




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-26 16:20 ` SPI Grant Likely
@ 2005-09-27  7:39   ` dmitry pervushin
  0 siblings, 0 replies; 19+ messages in thread
From: dmitry pervushin @ 2005-09-27  7:39 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linux Kernel Mailing List, spi-devel-general

On Mon, 2005-09-26 at 10:20 -0600, Grant Likely wrote:
> Would "SPI slave" or "SPI slave device" be better terminology than
> "SPI device"?  That way the terminology matches how SPI hardware docs
> are usually written.  (not a big deal, just thought I'd ask)
The term "SPI slave device" looks correct.. I am correcting the doc :)
> > +       err = spi_bus_populate( the_spi_bus,
> > +                       "Dev1 0 1 2\0" "Dev2 2 1 0\0",
> > +                       extract_name )
> In my mind, this is not ideal.  For example, the MPC5200 has 4 PSC
> ports which can be in SPI mode.  The SPI bus driver should/will not
> know what devices are attached to it.  It should be the responsibility
> of the board setup code to populate the bus.... on the other hand,
> perhaps the bus driver should look to it's platform_device structure
> to find a table of attached devices.  Generic platform_device parsing
> code could be used by all SPI bus drivers.
The spi_bus_populate is not the only way to populate the bus; the bus
driver can discover SPI devices on his own and directly call
spi_device_add, isn't it ?
> > +In this example, function like extract_name would put the '\0' on the
> > +1st space of device's name, so names will become just "Dev1", "Dev2",
> > +and the rest of string will become parameters of device.
> I don't think it's wise to use '\0' as a delimiter.  Sure it makes
> parsing really simple when the string passed in is formed correctly,
> but if someone misses the last '\0' you have no way to know where the
> string ends.  It also makes it difficult support passing a device
> string from the kernel command line.
You're right. Using spi_populate_bus is the simplest way, that may lead
to errors... From the other hand, if we used another char to delimit
device name and its parameters, there would be person who would want
this character in device name... I think that we can add another
approach to populate the bus ? 
> 
> > +4. SPI functions are structures reference
> > +-----------------------------------------
> > +This section describes structures and functions that listed
> > +in include/linux/spi.h
> I would like to see this function and structure reference in the spi.h
> file itself rather than here.  Better chance of it being kept up to
> date that way.
Yes; but I personally prefer to look to the only place instead of
spi.h/spi-core.c. I'll try to keep the things consistent :)

> > +This structure represents the message that SPI device driver sends to the
> > +SPI bus driver to handle.
> Is there any way for the SPI device to constrain the clock rate for a
> transfer?  For example, if the devices maximum speed is lower than the
> bus maximum speed.
Thank you for this comment; the `clock' field is initially intended to
do this. Device driver might set the field to maximum speed, and bus
driver would analyze the field in its xfer function and send the message
on lower speed. Moreover, there is the `set_clock' callback in
spi_bus_driver. If msg specifies its own clock value, the bus driver's
set_clock will be called just before transferring the message.
> Overall, I like.  It looks like it does what I need it to.  If I get a
> chance this week I'll port my SPI drivers to it and try it out on my
> MPC5200 board.
Thank you! If your drivers are going to open source, could you also sent
them to spi mailing list, to prepare the consolidated patch ? I hope if
there is no significant troubles, the current core will go to the
mainstream kernel :)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: SPI
  2005-09-26 11:12 SPI dmitry pervushin
                   ` (3 preceding siblings ...)
  2005-09-26 20:25 ` SPI Jesper Juhl
@ 2005-09-27 12:43 ` Greg KH
  2005-09-27 14:27   ` [spi-devel-general] SPI dmitry pervushin
  4 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2005-09-27 12:43 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

On Mon, Sep 26, 2005 at 03:12:14PM +0400, dmitry pervushin wrote:
> +/*
> + * spi_device_release
> + * 
> + * Pointer to this function will be put to dev->release place
> + * This function gets called as a part of device removing
> + * 
> + * Parameters:
> + * 	struct device* dev
> + * Return value:
> + * 	none
> + */
> +void spi_device_release( struct device* dev )
> +{
> +/* just a placeholder */
> +}

This is ALWAYS wrong, please fix your code.  See the many times I have
been over this issue in the archives.

Also, please fix your coding style to match the kernel if you wish to
have a chance to get it included. :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-27 12:43 ` SPI Greg KH
@ 2005-09-27 14:27   ` dmitry pervushin
  2005-09-27 14:35     ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: dmitry pervushin @ 2005-09-27 14:27 UTC (permalink / raw)
  To: Greg KH; +Cc: Linux Kernel Mailing List, spi-devel-general

On Tue, 2005-09-27 at 05:43 -0700, Greg KH wrote:
> This is ALWAYS wrong, please fix your code.  See the many times I have
> been over this issue in the archives.
Do you mean this comment ? The spi_device_release does nothing, just to
prevent compains from device_release function :)
> 
> Also, please fix your coding style to match the kernel if you wish to
> have a chance to get it included. :)
Hmm... running Lindent... done. Thank you once more, for your valuable
comments :)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-27 14:27   ` [spi-devel-general] SPI dmitry pervushin
@ 2005-09-27 14:35     ` Greg KH
  2005-09-27 14:49       ` dmitry pervushin
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2005-09-27 14:35 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

On Tue, Sep 27, 2005 at 06:27:16PM +0400, dmitry pervushin wrote:
> On Tue, 2005-09-27 at 05:43 -0700, Greg KH wrote:
> > This is ALWAYS wrong, please fix your code.  See the many times I have
> > been over this issue in the archives.
> Do you mean this comment ? The spi_device_release does nothing, just to
> prevent compains from device_release function :)

Think about why the kernel complains about a non-existant release
function.  Just replacing that complaint with a function that does
nothing does NOT solve the issue, it only makes the warning go away.

Please read up on how the lifetime rules work for devices, and what
needs to happen in the release function (hint, take a look at other
busses, like USB and PCI for examples of what needs to be done.)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-27 14:35     ` Greg KH
@ 2005-09-27 14:49       ` dmitry pervushin
  2005-09-27 14:54         ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: dmitry pervushin @ 2005-09-27 14:49 UTC (permalink / raw)
  To: Greg KH; +Cc: Linux Kernel Mailing List, spi-devel-general

On Tue, 2005-09-27 at 07:35 -0700, Greg KH wrote:
> Please read up on how the lifetime rules work for devices, and what
> needs to happen in the release function (hint, take a look at other
> busses, like USB and PCI for examples of what needs to be done.)
As far as I can see, pci_release_device deletes the pci_dev using kfree.
But here we have statically allocated spi_device structures --
spi_device_add does not allocate spi_device, but uses caller-allocated
one.
> 
> thanks,
> 
> greg k-h
> 
> 
> -------------------------------------------------------
> SF.Net email is sponsored by:
> Tame your development challenges with Apache's Geronimo App Server. Download
> it for free - -and be entered to win a 42" plasma tv or your very own
> Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-27 14:49       ` dmitry pervushin
@ 2005-09-27 14:54         ` Greg KH
  2005-09-27 15:19           ` dmitry pervushin
  2005-09-28 13:14           ` [PATCH] SPI dmitry pervushin
  0 siblings, 2 replies; 19+ messages in thread
From: Greg KH @ 2005-09-27 14:54 UTC (permalink / raw)
  To: dmitry pervushin; +Cc: Linux Kernel Mailing List, spi-devel-general

On Tue, Sep 27, 2005 at 06:49:57PM +0400, dmitry pervushin wrote:
> On Tue, 2005-09-27 at 07:35 -0700, Greg KH wrote:
> > Please read up on how the lifetime rules work for devices, and what
> > needs to happen in the release function (hint, take a look at other
> > busses, like USB and PCI for examples of what needs to be done.)
> As far as I can see, pci_release_device deletes the pci_dev using kfree.

Yes.

> But here we have statically allocated spi_device structures --
> spi_device_add does not allocate spi_device, but uses caller-allocated
> one.

Not good, reference counted structures almost always should be
dynamically created.  Please change this to also be true for SPI,
otherwise you will have a lot of nasty issues with devices that can be
removed at any point in time.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-09-27 14:54         ` Greg KH
@ 2005-09-27 15:19           ` dmitry pervushin
  2005-09-28 13:14           ` [PATCH] SPI dmitry pervushin
  1 sibling, 0 replies; 19+ messages in thread
From: dmitry pervushin @ 2005-09-27 15:19 UTC (permalink / raw)
  To: Greg KH; +Cc: Linux Kernel Mailing List, spi-devel-general

On Tue, 2005-09-27 at 07:54 -0700, Greg KH wrote:
> Not good, reference counted structures almost always should be
> dynamically created.  Please change this to also be true for SPI,
> otherwise you will have a lot of nasty issues with devices that can be
> removed at any point in time.
Hmm. I thought a bit about this and it seems reasonable. I'll change
this to dynamic allocation. 

> thanks,
> 
> greg k-h


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] SPI
  2005-09-27 14:54         ` Greg KH
  2005-09-27 15:19           ` dmitry pervushin
@ 2005-09-28 13:14           ` dmitry pervushin
  1 sibling, 0 replies; 19+ messages in thread
From: dmitry pervushin @ 2005-09-28 13:14 UTC (permalink / raw)
  To: Greg KH, akpm; +Cc: Linux Kernel Mailing List, spi-devel-general

Hello people,

Here is the revised SPI-core patch. Changes are as follows:
- spi_device_add now allocates and returns struct spi_device* instead of
taking caller-allocated spi_device* as parameter;
- spi_device_release changed to deallocate spi_device* allocated by call
to spi_device_add;
- new function spi_bus_populate2; it populates bus in the same manner as
spi_bus_populate does, but parameter is array of struct spi_device_desc
instead of string delimited by '\0's;
- code style fixes;
- documentation fixes.

-----------------8<- cut here ------------------------------------
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 systems
running Linux.

 Documentation/spi.txt  |  374 ++++++++++++++++++++++++++++++++++++
 arch/arm/Kconfig       |    2
 drivers/Kconfig        |    2
 drivers/Makefile       |    1
 drivers/spi/Kconfig    |   33 +++
 drivers/spi/Makefile   |   14 +
 drivers/spi/spi-core.c |  506 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-dev.c  |  219 +++++++++++++++++++++
 include/linux/spi.h    |  232 ++++++++++++++++++++++

Signed-off-by: dmitry pervushin <dpervushin@ru.mvista.com>

PATCH FOLLOWS
Index: linux-2.6.10/arch/arm/Kconfig
===================================================================
--- linux-2.6.10.orig/arch/arm/Kconfig
+++ linux-2.6.10/arch/arm/Kconfig
@@ -834,6 +834,8 @@ source "drivers/ssi/Kconfig"
 
 source "drivers/mmc/Kconfig"
 
+source "drivers/spi/Kconfig"
+
 endmenu
 
 source "ktools/Kconfig"
Index: linux-2.6.10/drivers/Kconfig
===================================================================
--- linux-2.6.10.orig/drivers/Kconfig
+++ linux-2.6.10/drivers/Kconfig
@@ -42,6 +42,8 @@ source "drivers/char/Kconfig"
 
 source "drivers/i2c/Kconfig"
 
+source "drivers/spi/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/misc/Kconfig"
Index: linux-2.6.10/drivers/Makefile
===================================================================
--- linux-2.6.10.orig/drivers/Makefile
+++ linux-2.6.10/drivers/Makefile
@@ -67,3 +67,4 @@ obj-$(CONFIG_DPM)		+= dpm/
 obj-$(CONFIG_MMC)		+= mmc/
 obj-y				+= firmware/
 obj-$(CONFIG_EVENT_BROKER)	+= evb/
+obj-$(CONFIG_SPI)		+= spi/
Index: linux-2.6.10/drivers/spi/Kconfig
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/Kconfig
@@ -0,0 +1,33 @@
+#
+# SPI device configuration
+#
+menu "SPI support"
+
+config SPI
+	default Y
+	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-core loadable module.
+
+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
+	tristate "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.
+	  This support is also available as a module.  If so, the module
+	  will be called spi-dev.
+
+endmenu
+
Index: linux-2.6.10/drivers/spi/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/Makefile
@@ -0,0 +1,14 @@
+#
+# Makefile for the kernel spi bus driver.
+#
+
+obj-$(CONFIG_SPI) += spi-core.o
+# bus drivers
+# ...functional drivers
+# ...and the common spi-dev driver
+obj-$(CONFIG_SPI_CHARDEV) += spi-dev.o
+
+ifeq ($(CONFIG_SPI_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
+
Index: linux-2.6.10/drivers/spi/spi-core.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-core.c
@@ -0,0 +1,506 @@
+/*
+ *  drivers/spi/spi-core.c
+ *
+ *  Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
+ *
+ * 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>
+
+static int spi_thread(void *context);
+
+/*
+ * spi_bus_match_name
+ *
+ * Drivers and devices on SPI bus are matched by name, just like the
+ * platform devices, with exception of SPI_DEV_CHAR. Driver with this name
+ * will be matched against any device
+ */
+static int spi_bus_match_name(struct device *dev, struct device_driver *drv)
+{
+	return !strcmp(drv->name, SPI_DEV_CHAR) ||
+	    !strcmp(TO_SPI_DEV(dev)->name, drv->name);
+}
+
+struct bus_type spi_bus = {
+	.name = "spi",
+	.match = spi_bus_match_name,
+};
+
+/*
+ * spi_bus_driver_init
+ *
+ * This function initializes the spi_bus_data structure for the
+ * bus. Functions has to be called when bus driver gets probed
+ *
+ * Parameters:
+ * 	spi_bus_driver* 	pointer to bus driver structure
+ * 	device*			platform device to be attached to
+ * Return value:
+ * 	0 on success, error code otherwise
+ */
+int spi_bus_driver_init(struct spi_bus_driver *bus, struct device *dev)
+{
+	struct spi_bus_data *pd =
+	    kmalloc(sizeof(struct spi_bus_data), GFP_KERNEL);
+	int err = 0;
+
+	if (!pd) {
+		err = -ENOMEM;
+		goto init_failed_1;
+	}
+	atomic_set(&pd->exiting, 0);
+	pd->bus = bus;
+	init_MUTEX(&pd->lock);
+	INIT_LIST_HEAD(&pd->msgs);
+	init_waitqueue_head(&pd->queue);
+	pd->thread = kthread_run(spi_thread, pd, "%s-work", dev->bus_id);
+	if (IS_ERR(pd->thread)) {
+		err = PTR_ERR(pd->thread);
+		goto init_failed_2;
+	}
+	dev->platform_data = pd;
+	return 0;
+
+init_failed_2:
+	kfree(pd);
+      init_failed_1:
+	return err;
+}
+
+/*
+ * __spi_bus_free
+ *
+ * This function called as part of unregistering bus device driver. It
+ * calls spi_device_del for each child (SPI) device on the bus
+ *
+ * Parameters:
+ * 	struct device* dev	the 'bus' device
+ * 	void* context		not used. Will be NULL
+ */
+int __spi_bus_free(struct device *dev, void *context)
+{
+	struct spi_bus_data *pd = dev->platform_data;
+
+	atomic_inc(&pd->exiting);
+	kthread_stop(pd->thread);
+	kfree(pd);
+
+	dev_dbg(dev, "unregistering children\n");
+	/*
+	 * NOTE: the loop below might needs redesign. Currently
+	 *       we delete devices from the head of children list
+	 *       until the list is empty; that's because the function
+	 *       device_for_each_child will hold the semaphore needed
+	 *       for deletion of device
+	 */
+	while (!list_empty(&dev->children)) {
+		struct device *child =
+		    list_entry(dev->children.next, struct device, node);
+		spi_device_del(TO_SPI_DEV(child));
+	}
+	return 0;
+}
+
+/*
+ * spi_bus_driver_unregister
+ *
+ * unregisters the SPI bus from the system. Before unregistering, it deletes
+ * each SPI device on the bus using call to __spi_device_free
+ *
+ * Parameters:
+ *  	struct spi_bus_driver* bus_driver	the bus driver
+ * Return value:
+ *  	void
+ */
+void spi_bus_driver_unregister(struct spi_bus_driver *bus_driver)
+{
+	if (bus_driver) {
+		driver_for_each_dev(&bus_driver->driver, NULL, __spi_bus_free);
+		driver_unregister(&bus_driver->driver);
+	}
+}
+
+/*
+ * spi_device_release
+ *
+ * Pointer to this function will be put to dev->release place
+ * This function gets called as a part of device removing
+ *
+ * Parameters:
+ * 	struct device* dev
+ * Return value:
+ * 	none
+ */
+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
+ *
+ * Parameters:
+ * 	struct device* parent		the 'bus' device
+ * 	char* name			name of device. Should not be NULL
+ * Return value:
+ * 	pointer to allocated spi_device structure; NULL on error
+ */
+struct spi_device* spi_device_add(struct device *parent, char *name)
+{
+	struct spi_device* dev;
+
+	if (!name)
+		goto dev_add_out;
+
+	dev = kmalloc(sizeof(struct spi_device), GFP_KERNEL);
+	if( !dev )
+		goto dev_add_out;
+
+	memset(&dev->dev, 0, sizeof(dev->dev));
+	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;
+
+	if (device_register(&dev->dev)<0) {
+		dev_dbg(parent, " device '%s' cannot be added\n", name);
+		goto dev_add_out_2;
+	}
+	return dev;
+
+dev_add_out_2:
+	kfree(dev);
+dev_add_out:
+	return NULL;
+}
+
+/*
+ * spi_queue
+ *
+ * Queue the message to be processed asynchronously
+ *
+ * Parameters:
+ *  	struct spi_msg* msg            message to be sent
+ * Return value:
+ *  	0 on no errors, negative error code otherwise
+ */
+int spi_queue(struct spi_msg *msg)
+{
+	struct device *dev = &msg->device->dev;
+	struct spi_bus_data *pd = dev->parent->platform_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(&pd->queue);
+	return 0;
+}
+
+/*
+ * __spi_transfer_callback
+ *
+ * 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
+ *
+ * Parameters:
+ * 	struct spimsg* msg	message that is being processed now
+ * 	int code		status of processing
+ */
+static void __spi_transfer_callback(struct spi_msg *msg, int code)
+{
+	if (code & (SPIMSG_OK | SPIMSG_FAILED))
+		complete((struct completion *)msg->context);
+}
+
+/*
+ * spi_transfer
+ *
+ * Process the SPI message, by queuing it to the driver and either
+ * immediately return or waiting till the end-of-processing
+ *
+ * Parameters:
+ * 	struct spi_msg* msg	message to process
+ * 	callback		user-supplied callback. If both msg->status and
+ * 				callback are set, the error code of -EINVAL
+ * 				will be returned
+ * Return value:
+ * 	0 on success, error code otherwise. This code does not reflect
+ * 	status of message, just status of queueing
+ */
+int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg *, int))
+{
+	struct completion msg_done;
+	int err = -EINVAL;
+
+	if (callback && !msg->status) {
+		msg->status = callback;
+		callback = NULL;
+	}
+
+	if (!callback) {
+		if (!msg->status) {
+			init_completion(&msg_done);
+			msg->context = &msg_done;
+			msg->status = __spi_transfer_callback;
+			spi_queue(msg);
+			wait_for_completion(&msg_done);
+			err = 0;
+		} else {
+			err = spi_queue(msg);
+		}
+	}
+
+	return err;
+}
+
+/*
+ * spi_thread
+ *
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ *
+ * Parameters:
+ *	void* context 		pointer to struct spi_bus_data
+ */
+static int spi_thread_awake(struct spi_bus_data *bd)
+{
+	int ret;
+
+	if (atomic_read(&bd->exiting)) {
+		return 1;
+	}
+	down(&bd->lock);
+	ret = !list_empty(&bd->msgs);
+	up(&bd->lock);
+	return ret;
+}
+
+static int spi_thread(void *context)
+{
+	struct spi_bus_data *bd = context;
+	struct spi_msg *msg;
+	int xfer_status;
+	int found;
+
+	while (!kthread_should_stop()) {
+
+		wait_event_interruptible(bd->queue, spi_thread_awake(bd));
+
+		if (atomic_read(&bd->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
+			 */
+			found = 0;
+			list_for_each_entry(msg, &bd->msgs, link) {
+				if (!bd->selected_device) {
+					bd->selected_device = msg->device;
+					if (bd->bus->select)
+						bd->bus->select(bd->
+								selected_device);
+					found = 1;
+					break;
+				}
+				if (msg->device == bd->selected_device) {
+					found = 1;
+					break;
+				}
+			}
+			if (!found) {
+				/*
+				 * all messages for current selected_device
+				 * are processed.
+				 * let's switch to another device
+				 */
+				msg =
+				    list_entry(bd->msgs.next, struct spi_msg,
+					       link);
+				if (bd->bus->deselect)
+					bd->bus->deselect(bd->selected_device);
+				bd->selected_device = msg->device;
+				if (bd->bus->select)
+					bd->bus->select(bd->selected_device);
+			}
+			list_del(&msg->link);
+			up(&bd->lock);
+
+			/*
+			 * and this part is locked by device's lock;
+			 * spi_queue will be able to queue new
+			 * messages
+			 */
+			spi_device_lock(&msg->device);
+			if (msg->status)
+				msg->status(msg, SPIMSG_STARTED);
+			if (bd->bus->set_clock && msg->clock)
+				bd->bus->set_clock(msg->device->dev.parent,
+						   msg->clock);
+			xfer_status = bd->bus->xfer(msg);
+			if (msg->status) {
+				msg->status(msg, SPIMSG_DONE);
+				msg->status(msg,
+					    xfer_status ? SPIMSG_OK :
+					    SPIMSG_FAILED);
+			}
+			spi_device_unlock(&msg->device);
+
+			/* lock the bus_data again... */
+			down(&bd->lock);
+		}
+		if (bd->bus->deselect)
+			bd->bus->deselect(bd->selected_device);
+		bd->selected_device = NULL;
+		/* device has been just deselected, unlocking the bus */
+		up(&bd->lock);
+	}
+thr_exit:
+	return 0;
+}
+
+/*
+ * spi_write
+ * 	send data to a device on an SPI bus
+ * Parameters:
+ * 	spi_device* dev		the target device
+ *	char* buf		buffer to be sent
+ *	int len			buffer length
+ * Return:
+ * 	the number of bytes transferred, or negative error code.
+ */
+int spi_write(struct spi_device *dev, const char *buf, int len)
+{
+	struct spi_msg *msg = spimsg_alloc(dev, SPI_M_WR, len, NULL);
+	int ret;
+
+	memcpy(spimsg_buffer_wr(msg), buf, len);
+	ret = spi_transfer(msg, NULL);
+	return ret == 1 ? len : ret;
+}
+
+/*
+ * spi_write
+ * 	receive data from a device on an SPI bus
+ * Parameters:
+ * 	spi_device* dev		the target device
+ *	char* buf		buffer to be sent
+ *	int len			number of bytes to receive
+ * Return:
+ * 	 the number of bytes transferred, or negative error code.
+ */
+int spi_read(struct spi_device *dev, char *buf, int len)
+{
+	int ret;
+	struct spimsg *msg = spimsg_alloc(dev, SPI_M_RD, len, NULL);
+
+	ret = spi_transfer(msg, NULL);
+	memcpy(buf, spimsg_buffer_rd(msg), len);
+	return ret == 1 ? len : ret;
+}
+
+/*
+ * spi_bus_populate and spi_bus_populate2
+ *
+ * These two functions intended to populate the SPI bus corresponding to
+ * device passed as 1st parameter. The difference is in the way to describe
+ * new SPI slave devices: the spi_bus_populate takes the ASCII string delimited
+ * by '\0', where each section matches one SPI device name _and_ its parameters,
+ * and the spi_bus_populate2 takes the array of structures spi_device_desc.
+ *
+ * If some device cannot be added because of spi_device_add fail, the function will
+ * not try to parse the rest of list
+ *
+ * Return:
+ * 	the number of devices that were successfully added
+ */
+int spi_bus_populate(struct device *parent,
+		     char *devices,
+		     void (*callback) (struct device * bus,
+				       struct spi_device * new_dev))
+{
+	struct spi_device *new_device;
+	int count = 0;
+
+	while (devices[0]) {
+		dev_dbg(parent, " discovered new SPI device, name '%s'\n",
+			devices);
+		if ((new_device = spi_device_add(parent, devices)) == NULL)
+			break;
+		if (callback)
+			callback(parent, new_device);
+		devices += (strlen(devices) + 1);
+		count++;
+	}
+	return count;
+}
+
+int spi_bus_populate2(struct device *parent,
+			struct spi_device_desc* devices,
+			void (*callback) (struct device* bus,
+					  struct spi_device *new_dev,
+					  void* params))
+{
+	struct spi_device *new_device;
+	int count = 0;
+
+	while (devices->name) {
+		dev_dbg(parent, " discovered new SPI device, name '%s'\n",
+				devices->name );
+		if ((new_device = spi_device_add(parent, devices->name)) == NULL)
+			break;
+		if (callback)
+			callback(parent, new_device, devices->params);
+		devices++;
+		count++;
+	}
+	return count;
+}
+
+int __init spi_core_init(void)
+{
+	return bus_register(&spi_bus);
+}
+
+subsys_initcall(spi_core_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("dmitry pervushin <dpervushin@ru.mvista.com>");
+
+EXPORT_SYMBOL_GPL(spi_queue);
+EXPORT_SYMBOL_GPL(spi_device_add);
+EXPORT_SYMBOL_GPL(spi_bus_driver_unregister);
+EXPORT_SYMBOL_GPL(spi_bus_populate);
+EXPORT_SYMBOL_GPL(spi_bus_populate2);
+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);
Index: linux-2.6.10/drivers/spi/spi-dev.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-dev.c
@@ -0,0 +1,219 @@
+/*
+    spi-dev.c - spi driver, char device interface
+
+    Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
+
+    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.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#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
+
+struct spidev_driver_data {
+	int minor;
+};
+
+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);
+static int __init spidev_init(void);
+
+static void spidev_cleanup(void);
+
+static int spidev_probe(struct device *dev);
+static int spidev_remove(struct device *dev);
+
+static struct file_operations spidev_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.read = spidev_read,
+	.write = spidev_write,
+	.open = spidev_open,
+	.release = spidev_release,
+};
+
+static struct class_simple *spidev_class;
+
+static struct spi_driver spidev_driver = {
+	.driver = {
+		   .name = SPI_DEV_CHAR,
+		   .probe = spidev_probe,
+		   .remove = spidev_remove,
+		   },
+};
+
+static int spidev_minor;
+
+static int spidev_probe(struct device *dev)
+{
+	struct spidev_driver_data *drvdata;
+
+	drvdata = kmalloc(sizeof(struct spidev_driver_data), GFP_KERNEL);
+	if (!drvdata) {
+		dev_dbg(dev, "allocating drvdata failed\n");
+		return -ENOMEM;
+	}
+
+	drvdata->minor = spidev_minor++;
+	dev_dbg(dev, "setting device's(%p) minor to %d\n", dev, drvdata->minor);
+	dev_set_drvdata(dev, drvdata);
+
+	class_simple_device_add(spidev_class,
+				MKDEV(SPI_MAJOR, drvdata->minor),
+				NULL, "spi%d", drvdata->minor);
+	dev_dbg(dev, " added\n");
+	return 0;
+}
+
+static int spidev_remove(struct device *dev)
+{
+	struct spidev_driver_data *drvdata;
+
+	drvdata = (struct spidev_driver_data *)dev_get_drvdata(dev);
+	class_simple_device_remove(MKDEV(SPI_MAJOR, drvdata->minor));
+	kfree(drvdata);
+	dev_dbg(dev, " removed\n");
+	return 0;
+}
+
+static ssize_t spidev_read(struct file *file, char *buf, size_t count,
+			   loff_t * offset)
+{
+	struct spi_device *dev = (struct spi_device *)file->private_data;
+	if (count > SPI_TRANSFER_MAX)
+		count = SPI_TRANSFER_MAX;
+	return spi_read(dev, buf, count);
+}
+
+static ssize_t spidev_write(struct file *file, const char *buf, size_t count,
+			    loff_t * offset)
+{
+	struct spi_device *dev = (struct spi_device *)file->private_data;
+	if (count > SPI_TRANSFER_MAX)
+		count = SPI_TRANSFER_MAX;
+	return spi_write(dev, buf, count);
+}
+
+struct spidev_openclose {
+	unsigned int minor;
+	struct file *file;
+};
+
+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);
+	struct spidev_driver_data *drvdata;
+
+	drvdata = (struct spidev_driver_data *)dev_get_drvdata(the_dev);
+	if (!drvdata) {
+		pr_debug("%s: oops, drvdata is NULL !\n", __FUNCTION__);
+		goto do_open_fail;
+	}
+
+	pr_debug("drvdata->minor = %d vs %d\n", drvdata->minor, o->minor);
+	if (drvdata->minor == o->minor) {
+		get_device(&dev->dev);
+		o->file->private_data = dev;
+		return 1;
+	}
+
+do_open_fail:
+	return 0;
+}
+
+int spidev_open(struct inode *inode, struct file *file)
+{
+	struct spidev_openclose o;
+	int status;
+
+	o.minor = iminor(inode);
+	o.file = file;
+	status = driver_for_each_dev(&spidev_driver.driver, &o, spidev_do_open);
+	if (status == 0) {
+		status = -ENODEV;
+	}
+	return status < 0 ? status : 0;
+}
+
+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;
+}
+
+static int __init spidev_init(void)
+{
+	int res;
+
+	if ((res = register_chrdev(SPI_MAJOR, "spi", &spidev_fops)) != 0) {
+		goto out;
+	}
+
+	spidev_class = class_simple_create(THIS_MODULE, "spi");
+	if (IS_ERR(spidev_class)) {
+		printk(KERN_ERR "%s: error creating class\n", __FUNCTION__);
+		res = -EINVAL;
+		goto out_unreg;
+	}
+
+	if ((res = spi_driver_add(&spidev_driver)) != 0)
+		goto out_unreg;
+
+	printk("SPI /dev entries driver.\n");
+	return 0;
+
+out_unreg:
+	unregister_chrdev(SPI_MAJOR, "spi");
+out:
+	printk(KERN_ERR "%s: Driver initialization failed\n", __FILE__);
+	return res;
+}
+
+static void spidev_cleanup(void)
+{
+	spi_driver_del(&spidev_driver);
+	class_simple_destroy(spidev_class);
+	unregister_chrdev(SPI_MAJOR, "spi");
+}
+
+MODULE_AUTHOR("dmitry pervushin <dpervushin@ru.mvista.com>");
+MODULE_DESCRIPTION("SPI /dev entries driver");
+MODULE_LICENSE("GPL");
+
+module_init(spidev_init);
+module_exit(spidev_cleanup);
Index: linux-2.6.10/Documentation/spi.txt
===================================================================
--- /dev/null
+++ linux-2.6.10/Documentation/spi.txt
@@ -0,0 +1,374 @@
+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:
+
+  +--------------+                    +---------+
+  | platform_bus |                    | spi_bus |
+  +--------------+                    +---------+
+       |..|                                |
+       |..|--------+               +---------------+
+     +------------+| is parent to  |  SPI devices  |
+     | SPI busses |+-------------> |               |
+     +------------+                +---------------+
+           |                               |
+     +----------------+          +----------------------+
+     | SPI bus driver |          |    SPI device driver |
+     +----------------+          +----------------------+
+
+3.2 How do the SPI devices gets discovered and probed ?
+
+In general, the SPI bus driver cannot effective discover devices
+on its bus. Fortunately, the devices on SPI bus usually implemented
+onboard, so the following method has been chosen: the SPI bus driver
+calls the function named spi_bus_populate and passed the `topology
+string' to it. The function will parse the string and call the callback
+for each device, just before registering it. This allows bus driver
+to determine parameters like CS# for each device, retrieve them from
+string and store somewhere like spi_device->platform_data. An example:
+	err = spi_bus_populate( the_spi_bus,
+			"Dev1 0 1 2\0" "Dev2 2 1 0\0",
+			extract_name )
+In this example, function like extract_name would put the '\0' on the
+1st space of device's name, so names will become just "Dev1", "Dev2",
+and the rest of string will become parameters of device.
+
+The another way is to create array of structures spi_device_desc and pass
+this array to function spi_bus_populate2, like this:
+  struct spi_device_desc spi_slaves[] = {
+    [0] = {
+	.name = "device1",
+        .param = device1_params,
+    },
+    [1] = {
+        .name = "device2",
+        .param = NULL,
+    }
+    [2] = {
+	NULL, NULL
+    };
+  spi_bus_populate2( the_spi_bus, spi_slaves, callback );
+
+3.3. DMA and SPI messages
+-------------------------
+
+To handle DMA transfers on SPI bus, any device driver might provide special
+callbacks to allocate/free/get access to buffer. These callbacks are defined
+in subsection iii of section 4.
+To send data using DMA, the buffers should be allocated using
+dma_alloc_coherent function. Usually buffers are allocated statically or
+using kmalloc function.
+To allow drivers to allocate buffers in non-standard
+When one allocates the structure for spi message, it needs to provide target
+device. If its driver wants to allocate buffer in driver-specific way, it may
+provide its own allocation/free methods: alloc and free. If driver does not
+provide these methods, kmalloc and kfree will be used.
+After allocation, the buffer must be accessed to copy the buffer to be send
+or retrieve buffer that has been just received from device. If buffer was
+allocated using driver's alloc method, it(buffer) will be accessed using
+get_buffer. Driver should provide accessible buffer that corresponds buffer
+allocated by driver's alloc method. If there is no get_buffer method,
+the result of alloc will be used.
+After reading/writing from/to buffer, it will be released by call to driver's
+release_buffer method.
+
+
+4. SPI functions are structures reference
+-----------------------------------------
+This section describes structures and functions that listed
+in include/linux/spi.h
+
+i. struct spi_msg
+~~~~~~~~~~~~~~~~~
+
+struct spi_msg {
+        unsigned char flags;
+        unsigned short len;
+        unsigned long clock;
+        struct spi_device* device;
+        void          *context;
+        struct list_head link;
+        void (*status)( struct spi_msg* msg, int code );
+        void *devbuf_rd, *devbuf_wr;
+        u8 *databuf_rd, *databuf_wr;
+};
+This structure represents the message that SPI device driver sends to the
+SPI bus driver to handle.
+Fields:
+	flags 	combination of message flags
+		SPI_M_RD	"read" operation (from device to host)
+		SPI_M_WR	"write" operation (from host to device)
+		SPI_M_CS	assert the CS signal before sending the message
+		SPI_M_CSREL	clear the CS signal after sending the message
+		SPI_M_CSPOL	set clock polarity to high
+		SPI_M_CPHA	set clock phase to high
+	len	length, in bytes, of allocated buffer
+	clock	reserved, set to zero
+	device	the target device of the message
+	context	user-defined field; to associate any user data with the message
+	link	used by bus driver to queue messages
+	status	user-provided callback function to inform about message flow
+	devbuf_rd, devbuf_wr
+		so-called "device buffers". These buffers allocated by the
+		device driver, if device driver provides approproate callback.
+		Otherwise, the kmalloc API will be used.
+	databuf_rd, databuf_wr
+		pointers to access content of device buffers. They are acquired
+		using get_buffer callback, if device driver provides one.
+		Otherwise, they are just pointers to corresponding
+		device buffers
+
+struct spi_msg* spimsg_alloc( struct spi_device* device,
+                              unsigned flags,
+                              unsigned short len,
+                              void (*status)( struct spi_msg*, int code ) )
+This functions is called to allocate the spi_msg structure and set the
+corresponding fields in structure. If device->platform_data provides callbacks
+to handle buffers, alloc/get_buffer are to be used. Returns NULL on errors.
+
+struct void spimsg_free( struct spi_msg* msg )
+Deallocate spi_msg as well as internal buffers. If msg->device->platform_data
+provides callbacks to handle buffers, release_buffer and free are to be used.
+
+u8* spimsg_buffer_rd( struct spi_msg* msg )
+u8* spimsg_buffer_wr( struct spi_msg* msg )
+u8* spimsg_buffer( struct spi_msg* )
+Return the corresponding data buffer, which can be directly modified by driver.
+spimsg_buffer checks flags and return either databuf_rd or databuf_wr basing on
+value of `flags' in spi_msg structure.
+
+ii. struct spi_device
+~~~~~~~~~~~~~~~~~~~~~
+
+struct spi_device
+{
+        char name[ BUS_ID_SIZE ];
+        struct device dev;
+};
+This structure represents the physical device on SPI bus. The SPI bus driver
+will create and register this structure for you.
+	name	the name of the device. It should match to the SPI device
+		driver name
+	dev	field used to be registered with core
+
+struct spi_device* spi_device_add( struct device* parent,
+                    		   char* name )
+This function registers the device on the spi bus, and set its parent
+to `parent', which represents the SPI bus. The device name will be set to name,
+that should be non-empty, non-NULL string. Returns pointer to allocated
+spi_device structure, NULL on error.
+
+void spi_device_del( struct spi_device* dev )
+Unregister the SPI device. Return value is ignored
+
+iii. struct spi_driver
+~~~~~~~~~~~~~~~~~~~~~~
+
+struct spi_driver {
+    	void* (*alloc)( size_t, int );
+    	void  (*free)( const void* );
+    	unsigned char* (*get_buffer)( struct spi_device*, void* );
+    	void (*release_buffer)( struct spi_device*, unsigned char*);
+    	void (*control)( struct spi_device*, int mode, u32 ctl );
+        struct device_driver driver;
+};
+This structure represents the SPI device driver object. Before registering,
+all fields of driver sub-structure should be properly filled, e.g., the
+`bus_type' should be set to spi_bus. Otherwise, the driver will be incorrectly
+registered and its callbacks might never been called. An example of will-
+formed spi_driver structure:
+	extern struct bus_type spi_bus;
+	static struct spi_driver pnx4008_eeprom_driver = {
+        	.driver = {
+                   	.bus = &spi_bus,
+                   	.name = "pnx4008-eeprom",
+                   	.probe = pnx4008_spiee_probe,
+                   	.remove = pnx4008_spiee_remove,
+                   	.suspend = pnx4008_spiee_suspend,
+                   	.resume = pnx4008_spiee_resume,
+               	},
+};
+The method control gets called during the processing of SPI message.
+For detailed description of malloc/free/get_buffer/release_buffer, please
+look to section 3.3, "DMA and SPI messages"
+
+
+int spi_driver_add( struct spi_driver* driver )
+Register the SPI device driver with core; returns 0 on no errors, error code
+otherwise.
+
+void spi_driver_del( struct spi_driver* driver )
+Unregisters the SPI device driver; return value ignored.
+
+iv. struct spi_bus_driver
+~~~~~~~~~~~~~~~~~~~~~~~~~
+To handle transactions over the SPI bus, the spi_bus_driver structure must
+be prepared and registered with core. Any transactions, either synchronous
+or asynchronous, go through spi_bus_driver->xfer function.
+
+struct spi_bus_driver
+{
+        int (*xfer)( struct spi_msg* msgs );
+        void (*select) ( struct spi_device* arg );
+        void (*deselect)( struct spi_device* arg );
+
+        struct device_driver driver;
+};
+
+Fields:
+	xfer	pointer to function to execute actual transaction on SPI bus
+		msg	message to handle
+	select	pointer to function that gets called when bus needs to
+		select another device to be target of transfers
+	deselect
+		pointer to function that gets called before another device
+		is selected to be the target of transfers
+
+
+spi_bus_driver_register( struct spi_bus_driver* )
+
+Register the SPI bus driver with the system. The driver sub-structure should
+be properly filled before using this function, otherwise you may get unpredi-
+ctable results when trying to exchange data. An example of correctly prepared
+spi_bus_driver structure:
+	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,
+};
+The driver and corresponding platform device are matched by name, so, in
+order the example abive to work, the platform_device named "spipnx" should
+be registered somewhere.
+
+void spi_bus_driver_unregister( struct spi_bus_driver* )
+
+Unregister the SPI bus driver registered by call to spi_buys_driver_register
+function; returns void.
+
+int spi_bus_populate( struct device* parent,
+			      char* devices,
+			      void (*callback)( struct device* parent, struct spi_device* new_one ) )
+This function usually called by SPI bus drivers in order to populate the SPI
+bus (see also section 3.2, "How the SPI devices gets discovered and probed ?").
+After creating the spi_device, the spi_bus_populate calls the `callback'
+function to allow to modify spi_device's fields before registering it with core.
+	parent	pointer to SPI bus
+	devices	string representing the current topology of SPI bus. It should
+		be formed like
+		"dev-1_and_its_info\0dev-2_and_its_info\0another_device\0\0"
+		the spi_bus_populate delimits this string by '\0' characters,
+		creates spi_device and after calling the callback registers the
+		spi_device
+	callback
+		pointer to function which could modify spi_device fields just
+		before registering them with core
+
+int spi_bus_populate2 (struct device *parent, struct spi_device_desc *devices,
+			void (*callback) (struct device* parent, struct spi_device* new_dev,
+					  void *params ))
+This is another way to populate the bus; but instead of string with device names and
+parameters, the array of structures spi_device_desc is passed. Each item in array describes
+the SPI slave device to create.
+
+
+v. spi_transfer and spi_queue
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The driver that uses SPI core can initiate transfers either by calling
+spi_transfer function (that will wait till the transfer is funished) or
+queueing the message using spi_queue function (you need to provide function
+that will be called during message is processed). In any way, you need to
+prepare the spimsg structure and know the target device to your message to
+be sent.
+
+int spi_transfer( struct spi_msg msgs,
+                  void (*callback)( struct spi_msg* msg, int ) )
+If callback is zero, start synchronous transfer. Otherwise, queue
+the message.
+	msg		message to be handled
+	callback	the callback function to be called during
+			message processing. If NULL, the function
+			will wait until end of processing.
+
+int spi_queue( struct spi_device* device,
+               struct spi_msg* msg )
+Queue the only message to the device. Returns status of queueing. To obtain
+status of message processing, you have to provide `status' callback in message
+and examine its parameters
+	msg	message to be queued
+
+vi. the spi_bus variable
+~~~~~~~~~~~~~~~~~~~~~~~~
+This variable is created during initialization of spi core, and has to be
+specified as `bus' on any SPI device driver (look to section iii, "struct
+spi_driver" ). If you do not specify spi_bus, your driver will be never
+matched to spi_device and never be probed with hardware. Note that
+spi_bus.match points to function that matches drivers and devices by name,
+so SPI devices and their drivers should have the same name.
+
+5. How to contact authors
+-------------------------
+Do you have any comments ? Enhancements ? Device driver ? Feel free
+to contact me:
+	dpervushin@gmail.com
+	dimka@pervushin.msk.ru
+Visit our project page:
+	http://spi-devel.sourceforge.net
+Subscribe to mailing list:
+	spi-devel-general@lists.sourceforge.net
+
Index: linux-2.6.10/include/linux/spi.h
===================================================================
--- /dev/null
+++ linux-2.6.10/include/linux/spi.h
@@ -0,0 +1,232 @@
+/*
+ *  linux/include/linux/spi/spi.h
+ *
+ *  Copyright (C) 2005 MontaVista Software, Inc <sources@mvista.com>
+ *
+ * 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>
+
+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;
+	atomic_t exiting;
+	struct task_struct *thread;
+	wait_queue_head_t queue;
+	struct spi_device *selected_device;
+	struct spi_bus_driver *bus;
+};
+
+#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 	(*select) (struct spi_device * dev);
+	void 	(*deselect) (struct spi_device * dev);
+	void 	(*set_clock) (struct device * bus_device, u32 clock_hz);
+	struct device_driver driver;
+};
+
+#define TO_SPI_DEV(device) container_of( device, struct spi_device, dev )
+struct spi_device {
+	char name[BUS_ID_SIZE];
+	struct device dev;
+};
+
+#define TO_SPI_DRIVER(drv) container_of( drv, struct spi_driver, driver )
+struct spi_driver {
+	void 	       *(*alloc) (size_t, int);
+	void 	 	(*free) (const void *);
+	unsigned char  *(*get_buffer) (struct spi_device *, void *);
+	void 		(*release_buffer) (struct spi_device *, unsigned char *);
+	void 		(*control) (struct spi_device *, int mode, u32 ctl);
+	struct device_driver driver;
+};
+
+#define SPI_DEV_DRV( device )  TO_SPI_DRIVER( (device)->dev.driver )
+
+#define spi_device_lock( dev )		/* down( dev->dev.sem ) */
+#define spi_device_unlock( dev )	/* up( dev->dev.sem ) */
+
+/*
+ * struct spi_msg
+ *
+ * This structure represent the SPI message internally. You should never use fields of this structure directly
+ * Please use corresponding functions to create/destroy/access fields
+ *
+ */
+struct spi_msg {
+	unsigned char flags;
+#define SPI_M_RD	0x01
+#define SPI_M_WR	0x02	/**< Write mode flag */
+#define SPI_M_CSREL	0x04	/**< CS release level at end of the frame  */
+#define SPI_M_CS	0x08	/**< CS active level at begining of frame ( default low ) */
+#define SPI_M_CPOL	0x10	/**< Clock polarity */
+#define SPI_M_CPHA	0x20	/**< Clock Phase */
+	unsigned short len;	/* msg length           */
+	unsigned long clock;
+	struct spi_device *device;
+	void *context;
+	struct list_head link;
+	void (*status) (struct spi_msg * msg, int code);
+	void *devbuf_rd, *devbuf_wr;
+	u8 *databuf_rd, *databuf_wr;
+};
+
+static inline struct spi_msg *spimsg_alloc(struct spi_device *device,
+					   unsigned flags,
+					   unsigned short len,
+					   void (*status) (struct spi_msg *,
+							   int code))
+{
+	struct spi_msg *msg;
+	struct spi_driver *drv = SPI_DEV_DRV(device);
+
+	msg = kmalloc(sizeof(struct spi_msg), GFP_KERNEL);
+	if (!msg)
+		return NULL;
+	memset(msg, 0, sizeof(struct spi_msg));
+	msg->len = len;
+	msg->status = status;
+	msg->device = device;
+	msg->flags = flags;
+	INIT_LIST_HEAD(&msg->link);
+	if (flags & SPI_M_RD) {
+		msg->devbuf_rd = drv->alloc ?
+		    drv->alloc(len, GFP_KERNEL) : kmalloc(len, GFP_KERNEL);
+		msg->databuf_rd = drv->get_buffer ?
+		    drv->get_buffer(device, msg->devbuf_rd) : msg->devbuf_rd;
+	}
+	if (flags & SPI_M_WR) {
+		msg->devbuf_wr = drv->alloc ?
+		    drv->alloc(len, GFP_KERNEL) : kmalloc(len, GFP_KERNEL);
+		msg->databuf_wr = drv->get_buffer ?
+		    drv->get_buffer(device, msg->devbuf_wr) : msg->devbuf_wr;
+	}
+	pr_debug("%s: msg = %p, RD=(%p,%p) WR=(%p,%p). Actual flags = %s+%s\n",
+		 __FUNCTION__,
+		 msg,
+		 msg->devbuf_rd, msg->databuf_rd,
+		 msg->devbuf_wr, msg->databuf_wr,
+		 msg->flags & SPI_M_RD ? "RD" : "~rd",
+		 msg->flags & SPI_M_WR ? "WR" : "~wr");
+	return msg;
+}
+
+static inline void spimsg_free(struct spi_msg *msg)
+{
+	void (*do_free) (const void *) = kfree;
+	struct spi_driver *drv = SPI_DEV_DRV(msg->device);
+
+	if (msg) {
+		if (drv->free)
+			do_free = drv->free;
+		if (drv->release_buffer) {
+			if (msg->databuf_rd)
+				drv->release_buffer(msg->device,
+						    msg->databuf_rd);
+			if (msg->databuf_wr)
+				drv->release_buffer(msg->device,
+						    msg->databuf_wr);
+		}
+		if (msg->devbuf_rd)
+			do_free(msg->devbuf_rd);
+		if (msg->devbuf_wr)
+			do_free(msg->devbuf_wr);
+		kfree(msg);
+	}
+}
+
+static inline u8 *spimsg_buffer_rd(struct spi_msg *msg)
+{
+	return msg ? msg->databuf_rd : NULL;
+}
+
+static inline u8 *spimsg_buffer_wr(struct spi_msg *msg)
+{
+	return msg ? msg->databuf_wr : NULL;
+}
+
+static inline u8 *spimsg_buffer(struct spi_msg *msg)
+{
+	if (!msg)
+		return NULL;
+	if ((msg->flags & (SPI_M_RD | SPI_M_WR)) == (SPI_M_RD | SPI_M_WR)) {
+		printk(KERN_ERR "%s: what buffer do you really want ?\n",
+		       __FUNCTION__);
+		return NULL;
+	}
+	if (msg->flags & SPI_M_RD)
+		return msg->databuf_rd;
+	if (msg->flags & SPI_M_WR)
+		return msg->databuf_wr;
+}
+
+#define SPIMSG_OK 	0x01
+#define SPIMSG_FAILED 	0x80
+#define SPIMSG_STARTED  0x02
+#define SPIMSG_DONE	0x04
+
+#define SPI_MAJOR	153
+
+struct spi_driver;
+struct spi_device;
+
+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);
+struct spi_device* spi_device_add(struct device *parent, char *name);
+
+static inline void spi_device_del(struct spi_device *dev)
+{
+	device_unregister(&dev->dev);
+}
+static inline int spi_driver_add(struct spi_driver *drv)
+{
+	drv->driver.bus = &spi_bus;
+	return driver_register(&drv->driver);
+}
+static inline void spi_driver_del(struct spi_driver *drv)
+{
+	driver_unregister(&drv->driver);
+}
+
+#define SPI_DEV_CHAR "spi-char"
+
+extern int spi_write(struct spi_device *dev, const char *buf, int len);
+extern int spi_read(struct spi_device *dev, char *buf, int len);
+
+extern int spi_queue(struct spi_msg *message);
+extern int spi_transfer(struct spi_msg *message,
+			void (*status) (struct spi_msg *, int));
+extern int spi_bus_populate(struct device *parent, char *device,
+			    void (*assign) (struct device *parent,
+					    struct spi_device *));
+struct spi_device_desc {
+	char* name;
+	void* params;
+};
+extern int spi_bus_populate2(struct device *parent,
+			     struct spi_device_desc *devices,
+			     void (*assign) (struct device *parent,
+				             struct spi_device *,
+					     void *));
+
+#endif				/* SPI_H */




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-11-23  9:33         ` Vitaly Wool
@ 2005-11-23 19:05           ` David Brownell
  0 siblings, 0 replies; 19+ messages in thread
From: David Brownell @ 2005-11-23 19:05 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Mark Underwood, dmitry pervushin, Andrew Morton, Greg KH,
	osdl.org, Linux Kernel Mailing List, spi-devel-general

> >By the way Vitaly ... I'm still not getting responses directly from you.
> >Are you pruning me off CC lists, or is there something between us that's
> >filtering out posts?  It's certainly hard to respond ...
> >
> No, definitely I'm not. I'm just responding with "Reply All" button in 
> my Thunderbird, if you wanna know the details.
> And I'm quite surprised to hear about the kind of problems you have as 
> you're the only one who's having them.

Well, as was pointed out offline, you aren't always CC'ing me.  In this
case you did, and I got your message.


> >Vitaly, you're really going to have to come up with some facts if you
> >keep claiming the SPI framework I've posted doesn't support DMA.
> >(I'm not going to let you pull a SCO on us here.)  For that matter,
> >you might want to consider the fact that Stephen's pxa2xx_spi
> >driver disproves your arguments (it includes DMA support).
>   
> 
> Again, a single working driver cannot disapprove anything.

It certainly can disprove assertions like the one you made...


> Evidently enough for anyone more or less familiar with logic, if one 
> claims that something is working, he should prove it's working 
> everywhere not somewhere; on the other hand, to disapprove this 
> statement it's enough to give one example.

Which is what I did.  You said that the approach I was using did NOT
support DMA.  I disproved it with several counterexamples ... the whole
USB stack for one, using that same DMA approach (in "struct urb" on the
host side, and "struct usb_request" on the peripheral side), as well as
more specifically that pxa2xx_spi driver (with "struct spi_message").


> The thing is that what we do in the core to provide DMA capabilities 
> should be implemented in controller driver in your case.
> I can agree that it might be considered an addition to the core and not 
> directly included in it, but leaving rather standard operations to the 
> controller driver degrades the added value of the core.

That's an entirely different argument, note.

And in this case, it was a conscious decision not to require any
mid-layer logic to interpose itself between layers of SPI driver.
Drivers using PIO shouldn't need to pay mapping costs ... but if
the mapping were done in a mid-layer, they would always do so.


> >Controller drivers don't deal with userland addresses.  That's the
> >responsibility of the layers on top ... e.g. the filesystem or driver
> >code that accepts them.  Most just copy to/from userspace, but some
> >pay the costs to support direct I/O.
> >  
> >
> Oh no, you've got me wrong. I was tryng to say that an upper level 
> driver which copies the data from the userspace should be copying it to 
> GFP_DMA buffer, otherwise it won't work or will force the controller 
> driver to allocate/copy.

GFP_DMA is not necessary ... plus, it's usually inappropriate.  Except
for broken hardware, any buffer returned by kmalloc is fine ... and for
that broken hardware, the dma mapping calls should be setting up the
relevant bounce buffering.  (See how ARM does it for various XScale
systems, the dmabounce_* code.)


> Basically the upper level driver shouldn't give a damn whether the 
> buffers should be DMA-able or not since it might be used on different 
> platforms w/ and w/o DMA capabilities. 

If it were any effort to provide DMA-safe buffers, I might agree.
But it isn't; most buffers come from kmalloc() already.

Remember, this is the same requirement applying to the rest of
the Linux driver stack.  Now that the requirements have been
clear for a couple years (Documentation/DMA-mapping.txt, and
yes I had something to do with finally getting those rules
written down) this is NOT a problem of any note.


> >>>- it needs to timely send some credentials (what is the case for the 
> >>>WLAN driver, for instance).
> 
> >Again, not the responsibility of the lowest level driver.   A WLAN driver
> >layered on top could, of course.  That's the way such things are done in
> >other driver stacks.
> >
> >Are you seriously suggesting that an SPI controller driver should have any
> >clue whatever about credentials??   Which of the dozens of schemes should
> >become that "special", then ... and why??
>  
> I'm afraid that you were not reading my response attentively. 

I was trying, but your words were making no sense to me.


> Credentials are stored in a staic-allocated buffer in the upper level 
> driver. Then they are passed down to the SPI core which will fail to 
> send the buffer containing the credentials if it's not copied to DMAable 
> memory somewhere in between.

So it'd be just like with any other driver stack.  In this case, the issue
would be with whatever is passing static credentials where DMA-safe buffers
are required.  (In fact, static buffers work on every Linux platform I've ever
worked with, but it's not guaranteed ... the main issue would be the same one
that shows up with dma to/from stack, where cacheline sharing causes chaos on
systems without dma-coherent cache.)

For what it's worth, usually credentials get copied into an sk_buff, which
is DMA-safe since it comes from kmalloc().  If you're talking about some
out-of-tree WLAN code, I'd expect those issues would be resolved (in the WLAN
code) before that stack gets merged.

- Dave


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
       [not found]       ` <200511221233.16634.david-b@pacbell.net>
@ 2005-11-23  9:33         ` Vitaly Wool
  2005-11-23 19:05           ` David Brownell
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Wool @ 2005-11-23  9:33 UTC (permalink / raw)
  To: David Brownell
  Cc: Mark Underwood, dmitry pervushin, Andrew Morton, Greg KH,
	osdl.org, Linux Kernel Mailing List, spi-devel-general

David Brownell wrote:

>By the way Vitaly ... I'm still not getting responses directly from you.
>
>Are you pruning me off CC lists, or is there something between us that's
>filtering out posts?  It's certainly hard to respond to comments of yours
>when I can only find them by going through some list archive I wouldn't
>normally read...
>  
>
No, definitely I'm not. I'm just responding with "Reply All" button in 
my Thunderbird, if you wanna know the details.
And I'm quite surprised to hear about the kind of problems you have as 
you're the only one who's having them.

>>>>So for example one way you know that (c) is well met is that it's the same
>>>>approach used in USB (both host and peripheral/gadget sides); that's been
>>>>working well for quite a few years now.  (Despite comments from Dmitry
>>>>and Vitaly to the contrary.)
>>>> 
>>>>
>>>>        
>>>>
>>>Lemme point you out that if somehting is "working" on a limited number 
>>>of platforms within the limited number of use cases, that's not 
>>>necessarily a correct implementation.
>>>      
>>>
>>No, but it's a good indication :).
>>    
>>
>
>And I'm not sure what a "limited number of platforms" etc is supposed
>to suggest.  In terms of platforms supporting USB on Linux, there are
>surely more of them than there are for PCI ... since every PCI platform
>can support USB, and many non-PCI platforms do also.  A better word
>for that DMA support would seem to be "extensive", not "limited".
>
>In fact, the support for non-PCI platforms was one of the things that
>led to the last DMA-related reworks for USB.  The PCI calls wouldn't
>work (of course), but USB needs things like DMA-coherent buffers (for
>transfer and queue head descriptors, used by most controllers) as well
>as bus-neutral DMA operations.  And it even works for things like
>scatterlist merging through IOMMU mappings ... that wide portability
>is **very much** a good indication that (c) is a non-issue with the
>framework I've assembled.
>
>
>Vitaly, you're really going to have to come up with some facts if you
>keep claiming the SPI framework I've posted doesn't support DMA.
>(I'm not going to let you pull a SCO on us here.)  For that matter,
>you might want to consider the fact that Stephen's pxa2xx_spi
>driver disproves your arguments (it includes DMA support).
>  
>
Again, a single working driver cannot disapprove anything.
Evidently enough for anyone more or less familiar with logic, if one 
claims that something is working, he should prove it's working 
everywhere not somewhere; on the other hand, to disapprove this 
statement it's enough to give one example.

However, your latest core should be working on the problematic target as 
opposed to the previous one. I didn't try that but I think it'll work.
The thing is that what we do in the core to provide DMA capabilities 
should be implemented in controller driver in your case.
I can agree that it might be considered an addition to the core and not 
directly included in it, but leaving rather standard operations to the 
controller driver degrades the added value of the core.

>
>  
>
>>>>>>- (A) has some assumptions on buffers that are passed down to spi
>>>>>> functions.
>>>>>>            
>>>>>>
>>>>Make that "requirements"; FWIW they're the same ones that apply to all
>>>>other kernel driver frameworks I've seen:  that buffers be DMA-safe.
>>>>It would not be helpful (IMO) to define different rules; that's also
>>>>called the "Principle of Least Astonishment".  :)
>>>>        
>>>>
>>>Yeah within this requirement it's correct. But that requirement may 
>>>really make the SPI controller driver a lot more complex if
>>>- it has to send something received from the userland
>>>      
>>>
>
>Controller drivers don't deal with userland addresses.  That's the
>responsibility of the layers on top ... e.g. the filesystem or driver
>code that accepts them.  Most just copy to/from userspace, but some
>pay the costs to support direct I/O.
>  
>
Oh no, you've got me wrong. I was tryng to say that an upper level 
driver which copies the data from the userspace should be copying it to 
GFP_DMA buffer, otherwise it won't work or will force the controller 
driver to allocate/copy.
And putting a requirement on the upper level driver to always do GFP_DMA 
kmalloc's might put too high restrictions on the memory usage.
Basically the upper level driver shouldn't give a damn whether the 
buffers should be DMA-able or not since it might be used on different 
platforms w/ and w/o DMA capabilities. Thus, it's going to be either a 
requirement for the upper level drivers that implies the SPI core not 
being transparent to the upper level, or a requirement for each 
controller driver that might be handling this situation to implement the 
same thing we did in the core. Neither of these seems reasonable to me.

>
>  
>
>>>- it needs to timely send some credentials (what is the case for the 
>>>WLAN driver, for instance).
>>>      
>>>
>
>Again, not the responsibility of the lowest level driver.   A WLAN driver
>layered on top could, of course.  That's the way such things are done in
>other driver stacks.
>
>Are you seriously suggesting that an SPI controller driver should have any
>clue whatever about credentials??   Which of the dozens of schemes should
>become that "special", then ... and why??
>
>  
>
I'm afraid that you were not reading my response attentively. 
Credentials are stored in a staic-allocated buffer in the upper level 
driver. Then they are passed down to the SPI core which will fail to 
send the buffer containing the credentials if it's not copied to DMAable 
memory somewhere in between.

Vitaly

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-11-22  6:00   ` [spi-devel-general] SPI Vitaly Wool
@ 2005-11-22 19:11     ` Mark Underwood
       [not found]       ` <200511221233.16634.david-b@pacbell.net>
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Underwood @ 2005-11-22 19:11 UTC (permalink / raw)
  To: Vitaly Wool, David Brownell
  Cc: Mark Underwood, dmitry pervushin, Andrew Morton, Greg KH,
	Linux Kernel Mailing List, spi-devel-general


--- Vitaly Wool <vwool@ru.mvista.com> wrote:

> David Brownell wrote:
> 
> >>>- (A) uses more complicated structure of SPI message, that contains one
> >>>  or more atomic transfers, and (B)
> >>>  offers the only spi_msg that represents the atomic transfer on SPI bus.
> >>>  The similar approach can be implemented
> >>>  in (B), and actually is implemented. But my imp[ression is that
> >>>  such enhancement may be added later..
> >>>      
> >>>
> >>I wouldn't have said that the message structure in (A) is more complex then (B). For example,
> in
> >>(B) you have many flags which controls things like SPI mode which are not needed in every
> message.
> >>Once the SPI controller has been setup for a particular slave device you don't need to
> constantly
> >>send this information. 
> >>    
> >>
> >
> >And in fact, constantly sending it means some drivers will have to waste
> >time constantly checking it, in case it changed.  If that setup is stored
> >in controller registers, it's a lot better to just have the setup() call
> >be responsible for changing the communication parameters.  (This is the
> >approach used by both MMC and PCMCIA, for what it's worth...)
> >
> >  
> >
> I'm not aware if MMC/PCMCIA guys are happy with this approach :), but 
> anyway what you're talking about here makes sense.
> 
> >  
> >
> >>In (B) how to do you handle SPI devices which require to send several messages with out
> releasing
> >>their cs? There are/will be some devices which require this. 
> >>    
> >>
> >
> >In fact, that's why the transfer segments are grouped.  One builds SPI
> >protocol requests out of several such segments.  A very common idiom is
> >writing a command, then reading its response.  Chipselect must stay
> >active during the whole sequence.
> >
> >Adding support for such a basic mechanism "later" doesn't seem like
> >a good idea to me.
> >  
> >
> It is supported by means of flags. I'm afraid your note is pointless here.

How does this work? As I said before I can see that you can leave a cs active, but how can you
make sure the next message is for that SPI device and not for another one?

> 
> >
> >  
> >
> >>>- (A) uses workqueues to queue and handle SPI messages, and (B)
> >>>  allocates the kernel thread to the same purpose.
> >>>  Using workqueues is not very good solution in real-time environment; I
> >>>  think that allocating and starting the 
> >>>  separate thread will give us more predictable and stable results;
> >>>      
> >>>
> >>Where does (A) use a workqueue? (A) doesn't use a workqueue or thread and instead leaves it up
> to
> >>the adapter driver how to handle the messages that it gets sent (which in the case of some
> drivers
> >>will mean no thread or workqueue). (B) is _forcing_ a thread on the adapter which the adapter
> may
> >>not need. 
> >>    
> >>
> >
> >Exactly.  That's one of the things I meant when I recently listed some of the
> >top goals of the framework I did:
> >
> >(a) SPI shouldn't perpetuate the driver model botches of I2C;
> >(b) ditto I2C's "everything is synchronous" botches;
> >(c) it should work well with DMA, to support things like DataFlash;
> >(d) given the variety of SPI chips, protocol controls are needed;
> >(e) place minimal implementation constraints on controller drivers.
> >
> >So for example one way you know that (c) is well met is that it's the same
> >approach used in USB (both host and peripheral/gadget sides); that's been
> >working well for quite a few years now.  (Despite comments from Dmitry
> >and Vitaly to the contrary.)
> >  
> >
> Lemme point you out that if somehting is "working" on a limited number 
> of platforms within the limited number of use cases, that's not 
> necessarily a correct implementation.

No, but it's a good indication :).

> 
> > 
> >  
> >
> >>>- (A) has some assumptions on buffers that are passed down to spi
> >>>  functions.
> >>>      
> >>>
> >
> >Make that "requirements"; FWIW they're the same ones that apply to all
> >other kernel driver frameworks I've seen:  that buffers be DMA-safe.
> >It would not be helpful (IMO) to define different rules; that's also
> >called the "Principle of Least Astonishment".  :)
> >  
> >
> Yeah within this requirement it's correct. But that requirement may 
> really make the SPI controller driver a lot more complex if
> - it has to send something received from the userland
> - it needs to timely send some credentials (what is the case for the 
> WLAN driver, for instance).
> 
> >
> >  
> >
> >>>  If some controller driver (or bus driver 
> >>>  in terms of (B)) tries to perform DMA transfers, it must copy the
> >>>  passed buffers to some memory allocated
> >>>  using GFP_DMA flag and map it using dma_map_single.
> >>>      
> >>>
> >
> >Based on this and other comments from Dmitry/Vitaly, I suspect they
> >don't see how the Linux DMA APIs work.  The correct statement is that if
> >a controller driver wants to use DMA, it must dma_{map,unmap}_single().
> >The upper level drivers don't _need_ to worry about that.
> >  
> >
> The upper level drivers do need to worry about their buffers being 
> DMAable then which requirement adds more complexity.
> For instance, that means that the upper level drivers can't use 
> static/const for the data being sent, what might be pretty much annoying 
> when you have to send the same data multiple times.
> Our approach is definitely more robust, although it may be reasonable to 
> simplify it somehow.
> 
> >>>- (A) retrieves SPI message from the queue in sequential order (FIFO),
> >>>      
> >>>
> >
> >Only with respect to a given device.  It would make no sense to reorder the
> >queue so that writing X, then Y, then Z would morph into "X Z Y" or "Z Y X".  :)
> >
> >It's specifically _undefined_ how requests going to different devices are ordered.
> >Some hardware will be happier if things are synchronized (e.g. to a vertical
> >retrace IRQ), some systems might need to prioritize certain devices, and so on.
> >
> >I do think FIFO makes a good general policy, for boards without any of those
> >special requirements.
> >
> >
> >  
> >
> >>>  but (B) provides more flexible way by providing
> >>>  special callback to retrieve next message from queue. This callback may
> >>>  implement its own discipline of scheduling 
> >>>  SPI messages. In any way, the default is FIFO.
> >>>      
> >>>
> >>I think (A) is missing a method of adding extra spi_message(s) in callback to extend the
> current
> >>transfer on that SPI device. I can imagine a case where you will be required to read status
> >>information from a device and in this status information is the length of the data it has just
> >>received (for example if it was a network adapter). Straight after reading this information
> the
> >>device would start sending the data it has received but when the read status message was
> issued
> >>the length of the data wasn't known.
> >>    
> >>
> >
> >Do you actually have hardware which works that way?  That would be an example
> >of a system that needs some specific prioritization of transfers (see below).
> >  
> >
> Let's just agree on the _fact_ that here our approach overcame yours. 
> IMO it's pretty evident.

Neither proposal is complete.

> 
> >
> >  
> >
> >>Currently with (A) we would have to stop the transfer and 
> >>restart the whole thing again, this time using the length of the data we found form the last
> >>message. 
> >>    
> >>
> >
> >Well, each transfer segement would clearly stop, but if that segment had
> >the flag set which says "leave chipselect active", then the controller
> >driver would have the flexibility to prioritize transfers to that chip.
> >  
> >
> Hey, you've told us earlier you don't need any flags, right?
> If we start talking about flags necessity, then your *complicated* 
> approach to the spi_message doesn't make sense, sorry.

You don't need _as many_ flags as in (B)

> 
> >>>- (A) uses standartized way to provide CS information, and (B) relies on
> >>>  functional drivers callbacks, which looks more
> >>>  flexible to me.
> >>>      
> >>>
> >>I'm not sure what you mean here. You need to provide the cs numbers with SPI device in order
> for
> >>the core to create the unique addres and entry in sysfs. 
> >>    
> >>
> >
> >I'm not sure what he means either.  :)
> >
> >Stephen's PXA2xx SPI driver uses callbacks internally, but that's kind of
> >specific to that PXA hardware ... there's no chipselect handled by the
> >controller, one of the dozens of GPIOs must be chosen and that's clearly
> >a board-specific mechanism (uses controller_data as I recall).  He tells
> >me he plans to post the latest version of that -- many updates including
> >PXA255 SSP support (not just NSSP) and code shrinkage -- early next week.
> >
> >But most of the SPI controllers I've seen just have a fixed number of
> >chipselects, typically four, handled directly by the controller.  That's
> >why the "standardized way" is just to use a 0..N chipselect number.
> >  
> >
> Suppose we have a specific driver or system service (API) implemented to 
> handle chip selects. We'll have to duplicate its functionality within 
> your approach what is incorrect in architectural terms. Our approach is 
> free from this drawback.

But suppose you have a SPI controller which has cs lines in the IP block. Are you saying you will
write another driver which manipulates the registers in the SPI controller out side of the adapter
driver? Some hardware might not support that (i.e. you can't directly controll the cs pin). 
 
Every solution will always work better on some hardware then other hardware. The point is to make
the solution work on all types hardware, here I think David's solution is better.

Mark

> 
> Vitaly
> 
> 



		
___________________________________________________________ 
Does your mail provider give you FREE antivirus protection? 
Get Yahoo! Mail http://uk.mail.yahoo.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [spi-devel-general] Re: SPI
  2005-11-21 21:27 ` SPI David Brownell
@ 2005-11-22  6:00   ` Vitaly Wool
  2005-11-22 19:11     ` Mark Underwood
  0 siblings, 1 reply; 19+ messages in thread
From: Vitaly Wool @ 2005-11-22  6:00 UTC (permalink / raw)
  To: David Brownell
  Cc: Mark Underwood, dmitry pervushin, Andrew Morton, Greg KH,
	Linux Kernel Mailing List, spi-devel-general

David Brownell wrote:

>>>- (A) uses more complicated structure of SPI message, that contains one
>>>  or more atomic transfers, and (B)
>>>  offers the only spi_msg that represents the atomic transfer on SPI bus.
>>>  The similar approach can be implemented
>>>  in (B), and actually is implemented. But my imp[ression is that
>>>  such enhancement may be added later..
>>>      
>>>
>>I wouldn't have said that the message structure in (A) is more complex then (B). For example, in
>>(B) you have many flags which controls things like SPI mode which are not needed in every message.
>>Once the SPI controller has been setup for a particular slave device you don't need to constantly
>>send this information. 
>>    
>>
>
>And in fact, constantly sending it means some drivers will have to waste
>time constantly checking it, in case it changed.  If that setup is stored
>in controller registers, it's a lot better to just have the setup() call
>be responsible for changing the communication parameters.  (This is the
>approach used by both MMC and PCMCIA, for what it's worth...)
>
>  
>
I'm not aware if MMC/PCMCIA guys are happy with this approach :), but 
anyway what you're talking about here makes sense.

>  
>
>>In (B) how to do you handle SPI devices which require to send several messages with out releasing
>>their cs? There are/will be some devices which require this. 
>>    
>>
>
>In fact, that's why the transfer segments are grouped.  One builds SPI
>protocol requests out of several such segments.  A very common idiom is
>writing a command, then reading its response.  Chipselect must stay
>active during the whole sequence.
>
>Adding support for such a basic mechanism "later" doesn't seem like
>a good idea to me.
>  
>
It is supported by means of flags. I'm afraid your note is pointless here.

>
>  
>
>>>- (A) uses workqueues to queue and handle SPI messages, and (B)
>>>  allocates the kernel thread to the same purpose.
>>>  Using workqueues is not very good solution in real-time environment; I
>>>  think that allocating and starting the 
>>>  separate thread will give us more predictable and stable results;
>>>      
>>>
>>Where does (A) use a workqueue? (A) doesn't use a workqueue or thread and instead leaves it up to
>>the adapter driver how to handle the messages that it gets sent (which in the case of some drivers
>>will mean no thread or workqueue). (B) is _forcing_ a thread on the adapter which the adapter may
>>not need. 
>>    
>>
>
>Exactly.  That's one of the things I meant when I recently listed some of the
>top goals of the framework I did:
>
>(a) SPI shouldn't perpetuate the driver model botches of I2C;
>(b) ditto I2C's "everything is synchronous" botches;
>(c) it should work well with DMA, to support things like DataFlash;
>(d) given the variety of SPI chips, protocol controls are needed;
>(e) place minimal implementation constraints on controller drivers.
>
>So for example one way you know that (c) is well met is that it's the same
>approach used in USB (both host and peripheral/gadget sides); that's been
>working well for quite a few years now.  (Despite comments from Dmitry
>and Vitaly to the contrary.)
>  
>
Lemme point you out that if somehting is "working" on a limited number 
of platforms within the limited number of use cases, that's not 
necessarily a correct implementation.

> 
>  
>
>>>- (A) has some assumptions on buffers that are passed down to spi
>>>  functions.
>>>      
>>>
>
>Make that "requirements"; FWIW they're the same ones that apply to all
>other kernel driver frameworks I've seen:  that buffers be DMA-safe.
>It would not be helpful (IMO) to define different rules; that's also
>called the "Principle of Least Astonishment".  :)
>  
>
Yeah within this requirement it's correct. But that requirement may 
really make the SPI controller driver a lot more complex if
- it has to send something received from the userland
- it needs to timely send some credentials (what is the case for the 
WLAN driver, for instance).

>
>  
>
>>>  If some controller driver (or bus driver 
>>>  in terms of (B)) tries to perform DMA transfers, it must copy the
>>>  passed buffers to some memory allocated
>>>  using GFP_DMA flag and map it using dma_map_single.
>>>      
>>>
>
>Based on this and other comments from Dmitry/Vitaly, I suspect they
>don't see how the Linux DMA APIs work.  The correct statement is that if
>a controller driver wants to use DMA, it must dma_{map,unmap}_single().
>The upper level drivers don't _need_ to worry about that.
>  
>
The upper level drivers do need to worry about their buffers being 
DMAable then which requirement adds more complexity.
For instance, that means that the upper level drivers can't use 
static/const for the data being sent, what might be pretty much annoying 
when you have to send the same data multiple times.
Our approach is definitely more robust, although it may be reasonable to 
simplify it somehow.

>>>- (A) retrieves SPI message from the queue in sequential order (FIFO),
>>>      
>>>
>
>Only with respect to a given device.  It would make no sense to reorder the
>queue so that writing X, then Y, then Z would morph into "X Z Y" or "Z Y X".  :)
>
>It's specifically _undefined_ how requests going to different devices are ordered.
>Some hardware will be happier if things are synchronized (e.g. to a vertical
>retrace IRQ), some systems might need to prioritize certain devices, and so on.
>
>I do think FIFO makes a good general policy, for boards without any of those
>special requirements.
>
>
>  
>
>>>  but (B) provides more flexible way by providing
>>>  special callback to retrieve next message from queue. This callback may
>>>  implement its own discipline of scheduling 
>>>  SPI messages. In any way, the default is FIFO.
>>>      
>>>
>>I think (A) is missing a method of adding extra spi_message(s) in callback to extend the current
>>transfer on that SPI device. I can imagine a case where you will be required to read status
>>information from a device and in this status information is the length of the data it has just
>>received (for example if it was a network adapter). Straight after reading this information the
>>device would start sending the data it has received but when the read status message was issued
>>the length of the data wasn't known.
>>    
>>
>
>Do you actually have hardware which works that way?  That would be an example
>of a system that needs some specific prioritization of transfers (see below).
>  
>
Let's just agree on the _fact_ that here our approach overcame yours. 
IMO it's pretty evident.

>
>  
>
>>Currently with (A) we would have to stop the transfer and 
>>restart the whole thing again, this time using the length of the data we found form the last
>>message. 
>>    
>>
>
>Well, each transfer segement would clearly stop, but if that segment had
>the flag set which says "leave chipselect active", then the controller
>driver would have the flexibility to prioritize transfers to that chip.
>  
>
Hey, you've told us earlier you don't need any flags, right?
If we start talking about flags necessity, then your *complicated* 
approach to the spi_message doesn't make sense, sorry.

>>>- (A) uses standartized way to provide CS information, and (B) relies on
>>>  functional drivers callbacks, which looks more
>>>  flexible to me.
>>>      
>>>
>>I'm not sure what you mean here. You need to provide the cs numbers with SPI device in order for
>>the core to create the unique addres and entry in sysfs. 
>>    
>>
>
>I'm not sure what he means either.  :)
>
>Stephen's PXA2xx SPI driver uses callbacks internally, but that's kind of
>specific to that PXA hardware ... there's no chipselect handled by the
>controller, one of the dozens of GPIOs must be chosen and that's clearly
>a board-specific mechanism (uses controller_data as I recall).  He tells
>me he plans to post the latest version of that -- many updates including
>PXA255 SSP support (not just NSSP) and code shrinkage -- early next week.
>
>But most of the SPI controllers I've seen just have a fixed number of
>chipselects, typically four, handled directly by the controller.  That's
>why the "standardized way" is just to use a 0..N chipselect number.
>  
>
Suppose we have a specific driver or system service (API) implemented to 
handle chip selects. We'll have to duplicate its functionality within 
your approach what is incorrect in architectural terms. Our approach is 
free from this drawback.

Vitaly


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2005-11-23 19:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-09-26 11:12 SPI dmitry pervushin
2005-09-26 12:31 ` SPI Eric Piel
2005-09-26 12:37   ` [spi-devel-general] SPI dmitry pervushin
2005-09-26 16:20 ` SPI Grant Likely
2005-09-27  7:39   ` [spi-devel-general] SPI dmitry pervushin
2005-09-26 16:25 ` SPI Valdis.Kletnieks
2005-09-26 16:46   ` [spi-devel-general] SPI Vitaly Wool
2005-09-26 20:25 ` SPI Jesper Juhl
2005-09-27 12:43 ` SPI Greg KH
2005-09-27 14:27   ` [spi-devel-general] SPI dmitry pervushin
2005-09-27 14:35     ` Greg KH
2005-09-27 14:49       ` dmitry pervushin
2005-09-27 14:54         ` Greg KH
2005-09-27 15:19           ` dmitry pervushin
2005-09-28 13:14           ` [PATCH] SPI dmitry pervushin
2005-11-21 20:15 SPI Mark Underwood
2005-11-21 21:27 ` SPI David Brownell
2005-11-22  6:00   ` [spi-devel-general] SPI Vitaly Wool
2005-11-22 19:11     ` Mark Underwood
     [not found]       ` <200511221233.16634.david-b@pacbell.net>
2005-11-23  9:33         ` Vitaly Wool
2005-11-23 19:05           ` David Brownell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).