[RFC] SPI core -- revisited

[RFC] SPI core -- revisited

[dmitry pervushin]

Hello guys,

we finally decided to rework the SPI core and now it its ready for your comments.. 
Here we have several boards equipped with SPI bus, and use this spi core with these boards; 
Drivers for them are available by request (...and if community approve this patch)

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

 drivers/Kconfig         |    2
 drivers/Makefile        |    1
 drivers/spi/Kconfig     |   31 ++++
 drivers/spi/Makefile    |    9 +
 drivers/spi/helpers.c   |   45 ++++++
 drivers/spi/spi-core.c  |  244 ++++++++++++++++++++++++++++++++++
 drivers/spi/spi-dev.c   |  337 ++++++++++++++++++++++++++++++++++++++++++++++++ 
 drivers/spi/spi.h       |   35 ++++
 include/linux/spi/spi.h |  205 +++++++++++++++++++++++++++++
 9 files changed, 909 insertions(+)

diff -uNr -X dontdiff linux-2.6.12/drivers/Kconfig linux/drivers/Kconfig
--- linux-2.6.12/drivers/Kconfig	2005-06-23 13:14:18.000000000 +0400
+++ linux/drivers/Kconfig	2005-06-23 15:38:08.633516840 +0400
@@ -42,6 +42,8 @@
 
 source "drivers/i2c/Kconfig"
 
+source "drivers/spi/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/misc/Kconfig"
diff -uNr -X dontdiff linux-2.6.12/drivers/Makefile linux/drivers/Makefile
--- linux-2.6.12/drivers/Makefile	2005-06-23 13:14:18.000000000 +0400
+++ linux/drivers/Makefile	2005-06-23 15:36:55.733688821 +0400
@@ -64,3 +64,4 @@
 obj-$(CONFIG_SGI_IOC4)		+= sn/
 obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
+obj-$(CONFIG_SPI)               += spi/
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/helpers.c linux/drivers/spi/helpers.c
--- linux-2.6.12/drivers/spi/helpers.c	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/helpers.c	2005-06-23 13:28:23.000000000 +0400
@@ -0,0 +1,45 @@
+/*
+ * drivers/spi/helper.c
+ *
+ * Helper functions.
+ *
+ * Author: dmitry pervushin <dpervushin@ru.mvista.com>
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+
+typedef struct {
+	int (*callback) (struct device * dev, void *data);
+	struct device_driver *drv;
+	void *data;
+} _find_t;
+
+static int dfed_callback(struct device *device, void *data)
+{
+	_find_t *local_data = (_find_t *) data;
+
+	return (device->driver != local_data->drv) ? 0 : 
+	        (*local_data->callback) (device, local_data->data);
+}
+
+int driver_for_each_dev(struct device_driver *drv, void *data,
+			int (*callback) (struct device * dev, void *data))
+{
+	_find_t local_data;
+
+	local_data.drv = drv;
+	local_data.data = data;
+	local_data.callback = callback;
+	return bus_for_each_dev(drv->bus, NULL, &local_data, dfed_callback);
+}
+
+EXPORT_SYMBOL(driver_for_each_dev);
+
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/Kconfig linux/drivers/spi/Kconfig
--- linux-2.6.12/drivers/spi/Kconfig	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/Kconfig	2005-06-23 15:42:34.318785056 +0400
@@ -0,0 +1,31 @@
+#
+# SPI device configuration
+#
+menu "SPI support"
+
+config SPI
+	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
+	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
+
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/Makefile linux/drivers/spi/Makefile
--- linux-2.6.12/drivers/spi/Makefile	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/Makefile	2005-06-23 15:10:08.000000000 +0400
@@ -0,0 +1,9 @@
+#
+# Makefile for the kernel spi bus driver.
+#
+
+# Init order: core, chardev, bit adapters, pcf adapters
+
+obj-$(CONFIG_SPI) += spi-core.o helpers.o
+obj-$(CONFIG_SPI_CHARDEV) += spi-dev.o
+
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/spi-core.c linux/drivers/spi/spi-core.c
--- linux-2.6.12/drivers/spi/spi-core.c	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/spi-core.c	2005-06-23 13:25:51.000000000 +0400
@@ -0,0 +1,244 @@
+/*
+ *  linux/drivers/spi/spi-core.c
+ *
+ *  Copyright (C) 2001 Russell King
+ *  Copyright (C) 2002 Compaq Computer Corporation
+ *
+ *  Adapted from l3-core.c by Jamey Hicks <jamey.hicks@compaq.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.
+ *
+ */
+
+#ifdef CONFIG_SPI_DEBUG
+#define DEBUG
+#endif				/* CONFIG_SPI_DEBUG */
+
+#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/spi/spi.h>
+
+#include "spi.h"
+
+static int spi_match(struct device *dev, struct device_driver *driver)
+{
+	struct spi_driver *spidrv = SPI_DRV(driver);
+	struct spi_adapter *spidev = SPI_ADAP(dev);
+	char **id;
+	int found = 0;
+
+	if (NULL == dev || NULL == driver) {
+		printk(KERN_ERR
+		       "%s: error - both dev and driver should not be NULL !\n",
+		       __FUNCTION__);
+		found = 0;
+		goto spi_match_done;
+	}
+	if (NULL == spidev->name) {
+		printk("%s: device has no name, so it cannot be attached\n",
+		       __FUNCTION__);
+		found = 0;
+		goto spi_match_done;
+	}
+
+	if (NULL == spidrv->supported_ids) {
+		printk
+		    ("%s: driver has no ids of devices to support, assuming ALL\n",
+		     __FUNCTION__);
+		found = 1;
+		goto spi_match_done;
+	}
+
+	id = *spidrv->supported_ids;
+	while (*id) {
+		pr_debug
+		    ("Verifying driver's supported id of '%s' against '%s'\n",
+		     *id, spidev->name);
+		if (0 == strncmp(*id, SPI_ID_ANY, strlen(SPI_ID_ANY))) {
+			pr_debug
+			    ("The driver (%p) can be attached to any device (%p)\n",
+			     driver, dev);
+			found = 1;
+			goto spi_match_done;
+		}
+		if (0 == strcmp(*id, spidev->name)) {
+			pr_debug("Done, driver (%p) match the device '%p'\n",
+				 driver, dev);
+			found = 1;
+			goto spi_match_done;
+		}
+		++id;
+	}
+
+	pr_debug("%s: no match\n ", __FUNCTION__);
+	found = 0;
+spi_match_done:	
+	return found;
+}
+
+/**
+ * spi_add_adapter - register a new SPI bus adapter
+ * @adap: spi_adapter structure for the registering adapter
+ *
+ * Make the adapter available for use by clients using name adap->name.
+ * The adap->adapters list is initialised by this function.
+ *
+ * Returns error code ( 0 on success ) ;
+ */
+
+int spi_add_adapter(struct spi_adapter *adap)
+{
+	int err;
+
+	memset(&adap->dev, 0, sizeof(adap->dev));
+
+	adap->dev.parent = NULL;
+	strncpy(adap->dev.bus_id, adap->name, sizeof(adap->dev.bus_id) - 1);
+	adap->dev.bus = &spi_bus_type;
+	sema_init(&adap->lock, 1);
+
+	err = device_register(&adap->dev);
+	pr_debug("device_register (%p) status = %d\n", &adap->dev, err);
+	return err;
+}
+
+/**
+ * spi_del_adapter - unregister a SPI bus adapter
+ * @adap: spi_adapter structure to unregister
+ *
+ * Remove an adapter from the list of available SPI Bus adapters.
+ *
+ * Returns error code (0 on success);
+ */
+
+void spi_del_adapter(struct spi_adapter *adap)
+{
+	device_unregister(&adap->dev);
+}
+
+/**
+ * spi_transfer - transfer information on an SPI bus
+ * @adap: adapter structure to perform transfer on
+ * @msgs: array of spi_msg structures describing transfer
+ * @num: number of spi_msg structures
+ *
+ * Transfer the specified messages to/from a device on the SPI bus.
+ *
+ * Returns number of messages successfully transferred, otherwise negative
+ * error code.
+ */
+int spi_transfer(struct spi_adapter *adap, struct spi_msg msgs[], int num)
+{
+	int ret = -ENOSYS;
+
+	if (adap->algo->xfer) {
+		down(&adap->lock);
+		if (adap->select) {
+			adap->select(adap, 1);
+		}
+		ret = adap->algo->xfer(adap, msgs, num);
+		if (adap->select) {
+			adap->select(adap, 0);
+		}
+		up(&adap->lock);
+	}
+	return ret;
+}
+
+/**
+ * spi_write - send data to a device on an SPI bus
+ * @client: registered client structure
+ * @addr: SPI bus address
+ * @buf: buffer for bytes to send
+ * @len: number of bytes to send
+ *
+ * Send len bytes pointed to by buf to device address addr on the SPI bus
+ * described by client.
+ *
+ * Returns the number of bytes transferred, or negative error code.
+ */
+int spi_write(struct spi_adapter *adap, int addr, const char *buf, int len)
+{
+	struct spi_msg msg;
+	int ret;
+
+	msg.addr = addr;
+	msg.flags = 0;
+	msg.buf = (char *)buf;
+	msg.len = len;
+
+	ret = spi_transfer(adap, &msg, 1);
+	return ret == 1 ? len : ret;
+}
+
+/**
+ * spi_read - receive data from a device on an SPI bus
+ * @client: registered client structure
+ * @addr: SPI bus address
+ * @buf: buffer for bytes to receive
+ * @len: number of bytes to receive
+ *
+ * Receive len bytes from device address addr on the SPI bus described by
+ * client to a buffer pointed to by buf.
+ *
+ * Returns the number of bytes transferred, or negative error code.
+ */
+int spi_read(struct spi_adapter *adap, int addr, char *buf, int len)
+{
+	struct spi_msg msg;
+	int ret;
+
+	msg.addr = addr;
+	msg.flags = SPI_M_RD;
+	msg.buf = buf;
+	msg.len = len;
+
+	ret = spi_transfer(adap, &msg, 1);
+	return ret == 1 ? len : ret;
+}
+
+/*
+ * Bus declaration, init/exit functions 
+ */
+struct bus_type spi_bus_type = {
+	.name = "spi",
+	.match = spi_match,
+};
+
+int __init spibus_init(void)
+{
+	int status;
+
+	status = bus_register(&spi_bus_type);
+	pr_debug("SPI: bus_register status = %d\n", status);
+	printk("SPI bus driver loaded.\n");
+	return status;
+}
+
+void __exit spibus_exit(void)
+{
+	bus_unregister(&spi_bus_type);
+	pr_debug("SPI: leaving\n");
+}
+
+subsys_initcall(spibus_init);
+module_exit(spibus_exit);
+
+MODULE_LICENSE( "GPL" );
+MODULE_AUTHOR( "dmitry pervushin <dpervushin@ru.mvista.com>" );
+
+EXPORT_SYMBOL_GPL(spi_add_adapter);
+EXPORT_SYMBOL_GPL(spi_del_adapter);
+EXPORT_SYMBOL_GPL(spi_transfer);
+EXPORT_SYMBOL_GPL(spi_write);
+EXPORT_SYMBOL_GPL(spi_read);
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/spi-dev.c linux/drivers/spi/spi-dev.c
--- linux-2.6.12/drivers/spi/spi-dev.c	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/spi-dev.c	2005-06-23 15:28:32.000000000 +0400
@@ -0,0 +1,337 @@
+/*
+    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. 
+ */
+
+/* $Id: cee_lsp-philips-melody.patch,v 1.1.4.8 2005/02/25 10:20:15 wool Exp $ */
+
+#ifdef CONFIG_SPI_DEBUG
+#define DEBUG
+#endif				/* CONFIG_SPI_DEBUG */
+
+#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/devfs_fs_kernel.h>
+
+#include <linux/init.h>
+#include <asm/uaccess.h>
+#include <linux/spi/spi.h>
+#include "spi.h"
+
+#define SPI_TRANSFER_MAX	65535
+#define SPI_ADAP_MAX		32
+
+/* struct file_operations changed too often in the 2.1 series for nice code */
+
+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 spidev_ioctl(struct inode *inode, struct file *file,
+			unsigned int cmd, unsigned long arg);
+static int __init spi_dev_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,
+};
+
+SPI_IDS_TABLE_BEGIN
+    SPI_ID_ANY 
+SPI_IDS_TABLE_END 
+
+static struct spi_driver spidev_driver = {
+	.owner = THIS_MODULE,
+	.driver = {
+		   .name = "generic spi",
+		   .bus = &spi_bus_type,
+		   .probe = spidev_probe,
+		   .remove = spidev_remove,
+		   },
+	.supported_ids = SPI_IDS,
+	.minor = 0,
+};
+
+static int spidev_probe(struct device *dev)
+{
+	spidev_driver_data_t *drvdata;
+	int err = 0;
+
+	if (NULL == dev) {
+		printk(KERN_ERR "%s: probing the NULL device!\n", __FUNCTION__);
+		err = -EFAULT;
+		goto probe_out;
+	}
+
+	drvdata = (spidev_driver_data_t *) kmalloc(sizeof(spidev_driver_data_t),
+						   GFP_KERNEL);
+	if (NULL == drvdata) {
+		pr_debug("%s: allocating drvdata failed\n", __FUNCTION__);
+		err = -ENOMEM;
+		goto probe_out;
+	}
+
+	drvdata->minor = spidev_driver.minor++;
+	pr_debug("%s: setting device's(%p) minor to %d\n",
+		 __FUNCTION__, dev, drvdata->minor);
+	dev_set_drvdata(dev, drvdata);
+#ifdef CONFIG_DEVFS_FS
+	devfs_mk_cdev(MKDEV(SPI_MAJOR, drvdata->minor),
+		      S_IFCHR | S_IRUSR | S_IWUSR, "spi/%d", drvdata->minor);
+#endif
+	pr_debug("%s: Registered as minor %d\n", __FUNCTION__, drvdata->minor);
+probe_out:	
+	return err;
+}
+
+static int spidev_remove(struct device *dev)
+{
+	spidev_driver_data_t *drvdata;
+	int err = 0;
+
+	if (NULL == dev) {
+		printk(KERN_ERR "%s: removing the NULL device\n", __FUNCTION__);
+	}
+
+	drvdata = (spidev_driver_data_t *) dev_get_drvdata(dev);
+	if (NULL == drvdata) {
+		pr_debug("%s: oops, drvdata is NULL !\n", __FUNCTION__);
+		err = -ENODEV;
+		goto remove_out;
+	}
+#ifdef CONFIG_DEVFS_FS
+	devfs_remove("spi/%d", drvdata->minor);
+#endif
+	kfree(drvdata);
+	pr_debug("%s: device removed\n", __FUNCTION__);
+remove_out:	
+	return err;
+}
+
+static ssize_t spidev_read(struct file *file, char *buf, size_t count,
+			   loff_t * offset)
+{
+	char *tmp;
+	int ret = 0;
+#ifdef DEBUG
+	struct inode *inode = file->f_dentry->d_inode;
+#endif
+	struct spi_adapter *adap = (struct spi_adapter *)file->private_data;
+	unsigned long (*cpy_to_user) (void *to_user, const void *from,
+				      unsigned long len);
+	void *(*alloc) (size_t, int);
+	void (*free) (const void *);
+	if (count > SPI_TRANSFER_MAX)
+		count = SPI_TRANSFER_MAX;
+
+	cpy_to_user = adap->copy_to_user ? adap->copy_to_user : copy_to_user;
+	alloc = adap->alloc ? adap->alloc : kmalloc;
+	free = adap->free ? adap->free : kfree;
+
+	/* copy user space data to kernel space. */
+	tmp = alloc(count, GFP_KERNEL);
+	if (tmp == NULL) {
+		ret = -ENOMEM;
+		goto read_out;
+	}
+
+	pr_debug("spi-%d reading %d bytes.\n", MINOR(inode->i_rdev), count);
+
+	ret = spi_read(adap, 0, tmp, count);
+	if (ret >= 0)
+		ret = cpy_to_user(buf, tmp, count) ? -EFAULT : ret;
+	free(tmp);
+read_out:	
+	return ret;
+}
+
+static ssize_t spidev_write(struct file *file, const char *buf, size_t count,
+			    loff_t * offset)
+{
+	int ret = 0;
+	char *tmp;
+	struct spi_adapter *adap = (struct spi_adapter *)file->private_data;
+#ifdef DEBUG
+	struct inode *inode = file->f_dentry->d_inode;
+#endif
+	unsigned long (*cpy_from_user) (void *to, const void *from_user,
+					unsigned long len);
+	void *(*alloc) (size_t, int);
+	void (*free) (const void *);
+
+	if (count > SPI_TRANSFER_MAX)
+		count = SPI_TRANSFER_MAX;
+
+	cpy_from_user =
+	    adap->copy_from_user ? adap->copy_from_user : copy_from_user;
+	alloc = adap->alloc ? adap->alloc : kmalloc;
+	free = adap->free ? adap->free : kfree;
+
+	/* copy user space data to kernel space. */
+	tmp = alloc(count, GFP_KERNEL);
+	if (tmp == NULL) {
+		ret =  -ENOMEM;
+		goto write_out_1;
+	}
+
+	if (cpy_from_user(tmp, buf, count)) {
+		ret = -EFAULT;
+		goto write_out_2;
+	}
+
+	pr_debug("spi-%d writing %d bytes.\n", MINOR(inode->i_rdev), count);
+	ret = spi_write(adap, 0, tmp, count);
+write_out_2:	
+	free(tmp);
+write_out_1:	
+	return ret;
+}
+
+typedef struct {
+	unsigned int minor;
+	struct file *file;
+} spidev_openclose_t;
+
+static int spidev_do_open(struct device *dev, void *context)
+{
+	spidev_openclose_t *o = (spidev_openclose_t *) context;
+	struct spi_adapter *adap = SPI_ADAP(dev);
+	spidev_driver_data_t *drvdata;
+	int ret = 0;
+
+	drvdata = (spidev_driver_data_t *) dev_get_drvdata(dev);
+	if (NULL == drvdata) {
+		pr_debug("%s: oops, drvdata is NULL !\n", __FUNCTION__);
+		goto do_open_out;
+	}
+
+	pr_debug("drvdata->minor = %d vs %d\n", drvdata->minor, o->minor);
+	if (drvdata->minor == o->minor) {
+		get_device(&adap->dev);
+		o->file->private_data = adap;
+		ret = 1;
+	}
+do_open_out:
+	return ret;
+}
+
+int spidev_open(struct inode *inode, struct file *file)
+{
+	spidev_openclose_t 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_adapter *adapter = file->private_data;
+
+	if (adapter) {
+		put_device(&adapter->dev);
+	}
+	file->private_data = NULL;
+
+	return 0;
+}
+
+static int __init spi_dev_init(void)
+{
+	int res;
+
+	printk(KERN_INFO "spi /dev entries driver\n");
+
+	if (0 != (res = register_chrdev(SPI_MAJOR, "spi", &spidev_fops))) {
+		goto out;
+	}
+
+	if (0 != (res = spi_add_driver(&spidev_driver))) {
+		goto out_unreg;
+	}
+#ifdef CONFIG_DEVFS_FS
+	devfs_mk_dir("spi");
+#endif
+	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_del_driver(&spidev_driver);
+#ifdef CONFIG_DEVFS_FS
+	devfs_remove("spi");
+#endif
+	unregister_chrdev(SPI_MAJOR, "spi");
+}
+
+MODULE_AUTHOR("Jamey Hicks <jamey.hicks@compaq.com> Frodo Looijaard <frodol@dds.nl> and Simon G. Vogl <simon@tk.uni-linz.ac.at>");
+MODULE_DESCRIPTION("SPI /dev entries driver");
+MODULE_LICENSE("GPL");
+
+module_init(spi_dev_init);
+module_exit(spidev_cleanup);
diff -uNr -X dontdiff linux-2.6.12/drivers/spi/spi.h linux/drivers/spi/spi.h
--- linux-2.6.12/drivers/spi/spi.h	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/spi/spi.h	2005-06-23 15:03:48.000000000 +0400
@@ -0,0 +1,35 @@
+/*
+ *  linux/drivers/spi/spi.h
+ *
+ *  Copyright (C) 2005 MontaVista
+ *
+ *  Local structures and macros
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License.
+ *
+ */
+#ifndef __SPI_LOCAL_H
+#define __SPI_LOCAL_H
+
+#define SPI_DRV( n ) container_of( n, struct spi_driver, driver )
+#define SPI_ADAP( n ) container_of( n, struct spi_adapter, dev )
+
+typedef struct {
+	const void *id;
+	void *found;
+} spi_srch_context_t;
+
+#define SPI_STUFF_FOUND -ECANCELED
+
+/* helpers.c */
+int driver_for_each_dev(struct device_driver *drv, void *data,
+			int (*callback) (struct device * dev, void *data));
+
+// #undef pr_debug
+//#define pr_debug printk
+#define ENTER() pr_debug( "%s: ENTERed\n", __FUNCTION__ )
+#define LEAVE() pr_debug( "%s: LEFT OUT\n", __FUNCTION__ )
+
+#endif				/* __SPI_LOCAL_H */
diff -uNr -X dontdiff linux-2.6.12/include/linux/spi/spi.h linux/include/linux/spi/spi.h
--- linux-2.6.12/include/linux/spi/spi.h	1970-01-01 03:00:00.000000000 +0300
+++ linux/include/linux/spi/spi.h	2005-06-23 13:55:06.000000000 +0400
@@ -0,0 +1,205 @@
+/*
+ *  linux/include/linux/spi/spi.h
+ *
+ *  Copyright (C) 2001 Russell King, All Rights Reserved.
+ *  Copyright (C) 2002 Compaq Computer Corporation, All Rights Reserved.
+ *
+ * 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_msg {
+	unsigned char addr;	/* slave address        */
+	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 */
+#define SPI_M_NOADDR	0x80
+
+	unsigned short len;	/* msg length           */
+	unsigned char *buf;	/* pointer to msg data  */
+	unsigned long clock;
+};
+
+
+#define SPI_MAJOR	98
+
+extern struct bus_type spi_bus_type;
+
+/*
+ * A driver is capable of handling one or more physical devices present on
+ * SPI adapters.
+ */
+struct spi_driver;
+
+struct spi_ops {
+	int (*open) (struct spi_driver *);
+	int (*command) (struct spi_driver *, int cmd, void *arg);
+	void (*close) (struct spi_driver *);
+};
+
+typedef char *spi_ids_t[];
+#define SPI_ID_ANY     "* ANY *"
+#define SPI_IDS_TABLE_BEGIN static spi_ids_t spi_devices_supported = {
+#define SPI_IDS_TABLE_END ,NULL };
+#define SPI_IDS &spi_devices_supported
+
+struct spi_driver {
+	struct spi_ops *ops;
+	struct module *owner;
+	struct device_driver driver;
+	unsigned int minor;
+	spi_ids_t *supported_ids;
+};
+
+struct spi_adapter;
+
+#define SELECT   	0x01
+#define UNSELECT 	0x02
+#define BEFORE_READ 	0x03
+#define BEFORE_WRITE	0x04
+#define AFTER_READ	0x05
+#define AFTER_WRITE	0x06
+
+struct spi_algorithm {
+	/* textual description */
+	char name[32];
+
+	/* perform bus transactions */
+	int (*xfer) (struct spi_adapter *, struct spi_msg msgs[], int num);
+	int (*chip_cs) (int, void *context);
+};
+
+/*
+ * spi_adapter is the structure used to identify a physical SPI device along
+ * with the access algorithms necessary to access it.
+ */
+struct spi_adapter {
+	/*
+	 * This name is used to uniquely identify the adapter.
+	 * It should be the same as the module name.
+	 */
+	char name[32];
+
+	/* the algorithm to access the device */
+	struct spi_algorithm *algo;
+
+	/* algorithm-specific data            */
+	void *algo_data;
+
+	/* private data                       */
+	void *private_data;
+
+	/*  This may be NULL, or should point to the module structure */
+	struct module *owner;
+
+	struct semaphore lock;
+
+	void (*select) (struct spi_adapter * this, int onoff);
+
+	/* The alternative way to allocate/free memory. */
+	void *(*alloc) (size_t, int);
+	void (*free) (const void *);
+
+	/*
+	 * Copy data from/to user in case the alternative
+	 * alloc/free functions are used.
+	 */
+	unsigned long (*copy_from_user) (void *to, const void *from_user,
+					 unsigned long len);
+	unsigned long (*copy_to_user) (void *to_user, const void *from,
+				       unsigned long len);
+
+	struct device dev;
+};
+
+extern int spi_add_adapter(struct spi_adapter *);
+extern void spi_del_adapter(struct spi_adapter *);
+
+extern int spi_transfer(struct spi_adapter *, struct spi_msg msgs[], int);
+extern int spi_write(struct spi_adapter *, int, const char *, int);
+extern int spi_read(struct spi_adapter *, int, char *, int);
+
+static inline int spi_add_driver(struct spi_driver *driver)
+{
+	return driver_register(&driver->driver);
+}
+
+static inline void spi_del_driver(struct spi_driver *driver)
+{
+	driver_unregister(&driver->driver);
+}
+
+typedef struct {
+	unsigned int minor;
+	void *private_data;
+} spidev_driver_data_t;
+
+static inline void *spi_get_adapdata(struct spi_adapter *dev)
+{
+	spidev_driver_data_t *dd;
+
+	dd = (spidev_driver_data_t *) dev_get_drvdata(&dev->dev);
+	return dd ? dd->private_data : NULL;
+}
+
+static inline void spi_set_adapdata(struct spi_adapter *dev, void *data)
+{
+	spidev_driver_data_t *dd;
+
+	dd = (spidev_driver_data_t *) dev_get_drvdata(&dev->dev);
+	if (dd) {
+		dd->private_data = data;
+	}
+}
+
+/**
+ * spi_command - send a command to a SPI device driver
+ * @client: registered client structure
+ * @cmd: device driver command
+ * @arg: device driver arguments
+ *
+ * Ask the SPI device driver to perform some function.  Further information
+ * should be sought from the device driver in question.
+ *
+ * Returns negative error code on failure.
+ */
+static inline int spi_command(struct spi_driver *clnt, int cmd, void *arg)
+{
+	struct spi_ops *ops = clnt->ops;
+	int ret = -EINVAL;
+
+	if (ops && ops->command)
+		ret = ops->command(clnt, cmd, arg);
+
+	return ret;
+}
+
+static inline int spi_open(struct spi_driver *clnt)
+{
+	struct spi_ops *ops = clnt->ops;
+	int ret = 0;
+
+	if (ops && ops->open)
+		ret = ops->open(clnt);
+	return ret;
+}
+
+static inline void spi_close(struct spi_driver *clnt)
+{
+	struct spi_ops *ops = clnt->ops;
+	if (ops && ops->close)
+		ops->close(clnt);
+}
+
+#endif				/* SPI_H */

Re: [RFC] SPI core -- revisited

[Jamey Hicks]

dmitry pervushin wrote:

>Hello guys,
>
>we finally decided to rework the SPI core and now it its ready for your comments.. 
>Here we have several boards equipped with SPI bus, and use this spi core with these boards; 
>Drivers for them are available by request (...and if community approve this patch)
>
>  
>
I'm glad to see that work is progressing on SPI core.  I've worked on 
drivers on both ARM linux and Blackfin uclinux that use SPI and would 
prefer that they not be platform specific.

What I've found in my Blackfin work is that I need asynchronous SPI 
support.  The driver starts an SPI transaction and receives a callback 
when the SPI transaction has completed.  The operations are similar to 
what you've suggested, except that the message structure includes a 
pointer to a callback and a void *data pointer. 

The driver I'm working on is for the Chipcon CC2420 802.15.4 radio, 
which uses SPI to access its registers and transmit and receive FIFOs.  
In my CC2420, it queues SPI requests and initiates the next one when an 
SPI transaction completes.

Jamey

Re: [RFC] SPI core -- revisited

[Russell King]

On Thu, Jun 23, 2005 at 11:50:26AM -0400, Jamey Hicks wrote:
> dmitry pervushin wrote:
> >we finally decided to rework the SPI core and now it its ready for your comments.. 
> >Here we have several boards equipped with SPI bus, and use this spi core with these boards; 
> >Drivers for them are available by request (...and if community approve this patch)
> 
> I'm glad to see that work is progressing on SPI core.  I've worked on 
> drivers on both ARM linux and Blackfin uclinux that use SPI and would 
> prefer that they not be platform specific.

I worry about SPI at the moment because I can't see how it's being used
from just this code.

The worry I have is that it appears to contain an algorithm layer.  Would
this be better as a library for drivers to use, or something like that?

The reason I bring up this point is that my L3 layer is over-complex
for what it does (despite being about 378 lines) because it tried far
too hard to look like the I2C layer - soo much so I'm not happy with
it for mainline.

(I also have some concerns with the amount of NULL pointer checking in
the SPI code...)

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

Re: [RFC] SPI core -- revisited

[Jamey Hicks]

Russell King wrote:

>On Thu, Jun 23, 2005 at 11:50:26AM -0400, Jamey Hicks wrote:
>  
>
>>dmitry pervushin wrote:
>>    
>>
>>>we finally decided to rework the SPI core and now it its ready for your comments.. 
>>>Here we have several boards equipped with SPI bus, and use this spi core with these boards; 
>>>Drivers for them are available by request (...and if community approve this patch)
>>>      
>>>
>>I'm glad to see that work is progressing on SPI core.  I've worked on 
>>drivers on both ARM linux and Blackfin uclinux that use SPI and would 
>>prefer that they not be platform specific.
>>    
>>
>
>I worry about SPI at the moment because I can't see how it's being used
>from just this code.
>
>The worry I have is that it appears to contain an algorithm layer.  Would
>this be better as a library for drivers to use, or something like that?
>
>  
>
That's a good point.  I think the only algorithm that really gets shared 
is the bitbanging one, which could be a library, and otherwise it is 
controller specific and might as well be in the adapter.

Jamey

Re: [RFC] SPI core -- revisited

[Jean Delvare]

Hi Dmitry,

> we finally decided to rework the SPI core and now it its ready for
> your comments..  Here we have several boards equipped with SPI bus,
> and use this spi core with these boards;  Drivers for them are
> available by request (...and if community approve this patch)

I'm not exactly the best qualified person to comment on your work, but
I'd have a couple remarks nevertheless:

> --- linux-2.6.12/drivers/spi/helpers.c	1970-01-01 03:00:00.000000000 +0300
> +++ linux/drivers/spi/helpers.c	2005-06-23 13:28:23.000000000 +0400
> @@ -0,0 +1,45 @@
> +/*
> + * drivers/spi/helper.c
> + *
> + * Helper functions.
> + *
> + * Author: dmitry pervushin <dpervushin@ru.mvista.com>
> + *
> + * 2004 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/config.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +
> +typedef struct {
> +	int (*callback) (struct device * dev, void *data);
> +	struct device_driver *drv;
> +	void *data;
> +} _find_t;
> +
> +static int dfed_callback(struct device *device, void *data)
> +{
> +	_find_t *local_data = (_find_t *) data;
> +
> +	return (device->driver != local_data->drv) ? 0 : 
> +	        (*local_data->callback) (device, local_data->data);
> +}
> +
> +int driver_for_each_dev(struct device_driver *drv, void *data,
> +			int (*callback) (struct device * dev, void *data))
> +{
> +	_find_t local_data;
> +
> +	local_data.drv = drv;
> +	local_data.data = data;
> +	local_data.callback = callback;
> +	return bus_for_each_dev(drv->bus, NULL, &local_data, dfed_callback);
> +}
> +
> +EXPORT_SYMBOL(driver_for_each_dev);

This stuff doesn't seem to be spi-specific at all. It would rather be
driver core material, right?

At any rate, I don't think anybody would enjoy a kernel module named
"helpers", whether spi-specific or not.

> +MODULE_AUTHOR("Jamey Hicks <jamey.hicks@compaq.com> Frodo Looijaard
> <frodol@dds.nl> and Simon G. Vogl <simon@tk.uni-linz.ac.at>");

Please leave Frodo and Simon out of this. Crediting them in the headers
is fine if this work is derived from theirs, but they definitely are not
the authors of these modules.

> --- linux-2.6.12/drivers/spi/spi.h	1970-01-01 03:00:00.000000000 +0300
> +++ linux/drivers/spi/spi.h	2005-06-23 15:03:48.000000000 +0400
> (...)
> --- linux-2.6.12/include/linux/spi/spi.h	1970-01-01 03:00:00.000000000 +0300
> +++ linux/include/linux/spi/spi.h	2005-06-23 13:55:06.000000000 +0400

I don't think it's very smart to have two different files with the same
name in the kernel tree. More likely than not it will create confusion
at some point in time.

> +#define SELECT   	0x01
> +#define UNSELECT 	0x02
> +#define BEFORE_READ 	0x03
> +#define BEFORE_WRITE	0x04
> +#define AFTER_READ	0x05
> +#define AFTER_WRITE	0x06

This doesn't seem to belong there. You don't seem to use these values
anywhere in the spi core, and if you did these constants would need to
be prefixed with SPI_.

Thanks,
-- 
Jean Delvare

Re: [RFC] SPI core -- revisited

[Greg KH]

On Thu, Jun 23, 2005 at 04:18:54PM +0400, dmitry pervushin wrote:
> +int driver_for_each_dev(struct device_driver *drv, void *data,
> +			int (*callback) (struct device * dev, void *data))
> +{
> +	_find_t local_data;
> +
> +	local_data.drv = drv;
> +	local_data.data = data;
> +	local_data.callback = callback;
> +	return bus_for_each_dev(drv->bus, NULL, &local_data, dfed_callback);
> +}
> +
> +EXPORT_SYMBOL(driver_for_each_dev);

Use the built-in kernel functions, don't create a new one and go around
the _GPL export.

> +#ifdef CONFIG_SPI_DEBUG
> +#define DEBUG
> +#endif				/* CONFIG_SPI_DEBUG */

Have the Makefile define this for you, it's not needed in the .c file

> +#include <linux/spi/spi.h>

Why a separate subdirectory?  It should just go in include/linux/ like
everything else.

> +	if (NULL == dev || NULL == driver) {

dev == NULL instead of the other way around (yes, I know why you do
this, but the rest of the kernel is the other way, and gcc will warn you
if you forget a '=').

> +SPI_IDS_TABLE_BEGIN
> +    SPI_ID_ANY 
> +SPI_IDS_TABLE_END 

Ick, that's a horrible way to define things.  Please make it look like
real .c code.

> +static struct spi_driver spidev_driver = {
> +	.owner = THIS_MODULE,
> +	.driver = {
> +		   .name = "generic spi",

No spaces in driver names please.

> +	drvdata = (spidev_driver_data_t *) kmalloc(sizeof(spidev_driver_data_t),
> +						   GFP_KERNEL);

Cast is unnecessary.

Also, get rid of all typedefs, they are not acceptable.

> +#ifdef CONFIG_DEVFS_FS
> +	devfs_mk_cdev(MKDEV(SPI_MAJOR, drvdata->minor),
> +		      S_IFCHR | S_IRUSR | S_IWUSR, "spi/%d", drvdata->minor);
> +#endif

Don't put #ifdef in code where it is not needed at all.

> +	if (NULL == dev) {
> +		printk(KERN_ERR "%s: removing the NULL device\n", __FUNCTION__);
> +	}

Extra { } are not needed.

What is this checking for anyway?

Use dev_err() for these things.

> +static ssize_t spidev_read(struct file *file, char *buf, size_t count,
> +			   loff_t * offset)
> +{
> +	char *tmp;
> +	int ret = 0;
> +#ifdef DEBUG
> +	struct inode *inode = file->f_dentry->d_inode;
> +#endif

#ifdef not nice to have here, it's not needed.

> +#define SPI_STUFF_FOUND -ECANCELED

Just use the raw error number, don't redefine it.

> +struct spi_adapter {
> +	/*
> +	 * This name is used to uniquely identify the adapter.
> +	 * It should be the same as the module name.
> +	 */
> +	char name[32];

Why not use the device.bus_id instead?

I too would like to see how this code is used, example drivers please.

thanks,

greg k-h

[RFC] SPI core -- revisited

[dpervushin]

 

Here is the sample of usage of our SPI core; this is the driver of atmel
chip connected to the pnx spi bus; 

Index: linux-2.6.10/drivers/spi/Kconfig
===================================================================
--- linux-2.6.10.orig/drivers/spi/Kconfig
+++ linux-2.6.10/drivers/spi/Kconfig
@@ -27,5 +27,14 @@
 	  This support is also available as a module.  If so, the module 
 	  will be called spi-dev.
 
+config SPI_PNX010X
+	tristate "PNX010x SPI bus support"
+	depends on SPI
+
+config SPI_PNX010X_ATMEL
+	tristate "Atmel Flash chip on PNX010x SPI support"
+	depends on SPI_PNX010X
+
+
 endmenu
 
Index: linux-2.6.10/drivers/spi/Makefile
===================================================================
--- linux-2.6.10.orig/drivers/spi/Makefile
+++ linux-2.6.10/drivers/spi/Makefile
@@ -6,4 +6,6 @@
 
 obj-$(CONFIG_SPI) += spi-core.o helpers.o
 obj-$(CONFIG_SPI_CHARDEV) += spi-dev.o
+obj-$(CONFIG_SPI_PNX010X)     += spi-pnx010x.o
+obj-$(CONFIG_SPI_PNX010X_ATMEL)       += spi-pnx010x_atmel.o
 
Index: linux-2.6.10/drivers/spi/spi-pnx010x.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-pnx010x.c
@@ -0,0 +1,385 @@
+/*
+ * drivers/spi/spi-pnx010x.c
+ *
+ * SPI support for PNX0105.
+ *
+ * Author: Dennis Kovalev <dkovalev@ru.mvista.com>
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether 
+express
+ * or implied.
+ */
+
+/*  Uncomment the following line for debugging info  */
+/*
+#define SPIPNX_DEBUG
+*/
+#ifdef SPIPNX_DEBUG
+#define DEBUG 1
+#endif
+
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/spi/spi.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/miscdevice.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+#include <asm/hardware.h>
+#include <asm/irq.h>
+#include <asm/uaccess.h>
+#include <asm/arch/sdma.h>
+
+#define DBG(args...)	pr_debug(args)
+#include <linux/spi/spi-pnx010x.h>
+
+static struct completion threshold;
+static struct completion op_complete;
+static void		*phys_spi_data_reg = 0;
+static unsigned int	 spi_sdma_channel = 0;
+static char		 sdma_ch_name[] = "SDMA channel";
+static sdma_setup_t	 sdma_setup;
+static sdma_config_t	 sdma_config;
+static int		 sdma_use_count = 0;
+
+static void default_spipnx_cs (int type, void *data);
+
+static irqreturn_t spipnx_interrupt (int irq, void *dev_id, struct 
+pt_regs *regs) {
+	unsigned int i_stat = spi_regs.stat;
+	
+	if ( (i_stat & VH_BLASSPI_STAT_REG_SPI_EOT_MSK) && 
+	     (spi_regs.ier & VH_BLASSPI_IER_REG_SPI_INTEOT_MSK) ) {
+		spi_regs.ier &= ~VH_BLASSPI_IER_REG_SPI_INTEOT_MSK;
+		complete (&op_complete);
+	}
+	if ( (i_stat & VH_BLASSPI_STAT_REG_SPI_THR_MSK) && 
+	     (spi_regs.ier & VH_BLASSPI_IER_REG_SPI_INTTHR_MSK) ) {
+		spi_regs.ier &= ~VH_BLASSPI_IER_REG_SPI_INTTHR_MSK;
+		complete (&threshold);
+	}
+	spi_regs.stat |= (1 << VH_BLASSPI_STAT_REG_SPI_INTCLR_POS);    /*
clear interrupt   */
+	spi_regs.timer_status = 0L;
+
+	return IRQ_HANDLED;
+}
+
+void spipnx_select_chip (void)
+{
+	unsigned reg = gpio_read_reg (PADMUX1_MODE0);
+	gpio_write_reg ((reg & ~GPIO_PIN_SPI_CE), PADMUX1_MODE0); }
+
+void spipnx_unselect_chip (void)
+{
+	unsigned reg = gpio_read_reg (PADMUX1_MODE0);
+	gpio_write_reg (reg | GPIO_PIN_SPI_CE, PADMUX1_MODE0); }
+
+static void spipnx_spi_init (void *algo_data) {
+	spi_pnx0105_algo_data_t *alg_data;
+	alg_data = (spi_pnx0105_algo_data_t *) algo_data;
+
+	spipnx_select_chip();
+	spi_regs.global = (1 << VH_BLASSPI_GLOBAL_REG_BLRES_SPI_POS) | 
+			  (1 << VH_BLASSPI_GLOBAL_REG_SPI_ON_POS);
+
+	mdelay (5);
+    
+	spi_regs.global = 1 << VH_BLASSPI_GLOBAL_REG_SPI_ON_POS;
/*  enable device once more             */
+	spi_regs.con =  (alg_data->mode << 16) | 
+			(1 << VH_BLASSPI_CON_REG_MS_POS) | 
+			SPI_CLOCK;
+	spi_regs.con |= (1 << VH_BLASSPI_CON_REG_SPI_BIDIR_POS) | (7 << 9);
/*  internal MUX setup(!) and set 8 bit transfers   */
+
+	spipnx_unselect_chip();
+}
+
+static void default_spipnx_cs (int type, void *data) {
+	switch (type) {
+		case SELECT:
+			spipnx_select_chip();
+			break;
+		case UNSELECT:
+			spipnx_unselect_chip();
+			break;
+		default:
+			break;
+	}
+}
+int spipnx_register (spi_pnx0105_algo_data_t *algo_data) {
+	int status;
+
+	if (!algo_data->sdma_mode)
+		return 0;
+
+	if (!sdma_use_count) {
+		status = sdma_request_channel (sdma_ch_name, NULL, NULL);
+		if (status < 0) {
+			printk ("No SDMA channel available\n");
+			return -EBUSY;
+		} else {
+			spi_sdma_channel = (unsigned int)status;
+			DBG ("acquired SDMA channel #%d\n",
spi_sdma_channel);
+		}
+	}
+
+	sdma_use_count++;
+	DBG("%s: sdma_use_count = %d\n", __FUNCTION__, sdma_use_count);
+
+	return 0;
+}
+
+void spipnx_unregister (spi_pnx0105_algo_data_t *algo_data) {
+	int status;
+
+	if (!algo_data->sdma_mode)
+		return;
+
+	if (sdma_use_count == 1) {
+		status = sdma_release_channel(spi_sdma_channel);
+		if (status < 0)
+			DBG ("Unable to release SDMA channel #%d\n",
spi_sdma_channel);
+	}
+
+	sdma_use_count--;
+	DBG("%s: sdma_use_count = %d\n", __FUNCTION__, sdma_use_count); }
+
+int spipnx_xfer_nonsdma (struct spi_adapter *adap, struct spi_msg 
+msgs[], int num) {
+	struct spi_msg *pmsg;
+	int i, j, bptr;
+	spinlock_t lock;
+	spi_pnx0105_algo_data_t *alg_data;
+	void (*chip_cs)(int type, void *data);
+
+	alg_data = (spi_pnx0105_algo_data_t *) adap->algo_data;
+	chip_cs = alg_data->chip_cs_callback? alg_data->chip_cs_callback :
default_spipnx_cs;
+	DBG ("Non-SDMA processing %d messages ...\n", num);
+
+	chip_cs(SELECT, alg_data->cb_data);
+	spi_regs.con |= (1 << VH_BLASSPI_CON_REG_THR_POS);
+
+	for (i=0; i<num; i++) {
+		pmsg = &msgs[i];
+		if ( pmsg->flags & SPI_M_RD ) {                      /*
here we have to read data         */
+			chip_cs(BEFORE_READ, alg_data->cb_data);
+			bptr = 0;
+			init_completion (&op_complete);
+			spi_regs.frm = 0x0000FFFF & (pmsg->len);     /*
tell the SPI how much we need     */
+			spi_regs.con &= ~VH_BLASSPI_CON_REG_RXTX_MSK;
/*  set mode to RX                    */
+			spi_regs.con |= (1 <<
VH_BLASSPI_CON_REG_SHIFT_OFF_POS);           /*  this could affect, but how?
*/
+			spi_regs.ier = (1 <<
VH_BLASSPI_IER_REG_SPI_INTEOT_POS);           /*  enable end of transfer
interrupt  */
+			spi_regs.dat = 0x00;                         /*
dummy write to start transfer     */
+			
+			while (bptr < pmsg->len) {
+				if ((pmsg->len)-bptr > FIFO_CHUNK_SIZE) {
/*  if there's data left for another  */
+					init_completion (&threshold);
/*  init wait queue for another chunk */
+					spin_lock_irq(&lock);
+					spi_regs.ier |= (1 <<
VH_BLASSPI_IER_REG_SPI_INTTHR_POS);  /*  chunk, then enable THR interrupt
*/
+					spin_unlock_irq(&lock);
+					wait_for_completion (&threshold);
+				} else
+					wait_for_completion (&op_complete);
+				for
(j=bptr;j<(((pmsg->len)-bptr<FIFO_CHUNK_SIZE)?(pmsg->len):(bptr+FIFO_CHUNK_S
IZE));j++)
+					pmsg->buf[j] = spi_regs.dat;
+				bptr += ((pmsg->len)-bptr
<FIFO_CHUNK_SIZE)?(pmsg->len - bptr):FIFO_CHUNK_SIZE;
+			}
+			DBG ("[i]:SPI_M_RD: %x\n", *pmsg->buf);
+			chip_cs(AFTER_READ, alg_data->cb_data);
+		} else {                                           /*  now
we have to transmit data        */
+			chip_cs(BEFORE_WRITE, alg_data->cb_data);
+			spi_regs.con |= (1 << VH_BLASSPI_CON_REG_RXTX_POS);
+			spi_regs.frm = 0x0000FFFF & (pmsg->len);
+			bptr = 0;
+			init_completion (&op_complete);
+			spi_regs.ier = (1 <<
VH_BLASSPI_IER_REG_SPI_INTEOT_POS);
+
+			while (bptr < pmsg->len) {
+				for
(j=bptr;j<(((pmsg->len)-bptr<FIFO_CHUNK_SIZE)?(pmsg->len):(bptr+FIFO_CHUNK_S
IZE));j++) {
+					spi_regs.dat = pmsg->buf[j];
+				}
+				if ((pmsg->len)-bptr > FIFO_CHUNK_SIZE) {
+					init_completion (&threshold);
+					spin_lock_irq(&lock);
+					spi_regs.ier |= (1 <<
VH_BLASSPI_IER_REG_SPI_INTTHR_POS);
+					spin_unlock_irq(&lock);
+					wait_for_completion (&threshold);
+				}
+				bptr += ((pmsg->len)-bptr
<FIFO_CHUNK_SIZE)?(pmsg->len - bptr):FIFO_CHUNK_SIZE;
+			}
+			wait_for_completion (&op_complete);
+			DBG ("[i]:SPI_M_WR: %x\n", *pmsg->buf);
+			chip_cs(AFTER_WRITE, alg_data->cb_data);
+		}
+	}
+	chip_cs(UNSELECT, alg_data->cb_data);
+	spi_regs.ier &= ~VH_BLASSPI_IER_REG_SPI_INTEOT_MSK;
+	
+	return num;
+}
+
+int spipnx_xfer_sdma (struct spi_adapter *adap, struct spi_msg msgs[], 
+int num) {
+	struct spi_msg *pmsg;
+	int i;
+	spi_pnx0105_algo_data_t *alg_data;
+	spi_pnx_msg_buff_t *buff;
+ 	void (*chip_cs)(int type, void *data);
+
+	alg_data = (spi_pnx0105_algo_data_t *) adap->algo_data;
+	chip_cs = alg_data->chip_cs_callback? alg_data->chip_cs_callback :
default_spipnx_cs;
+	DBG ("SDMA processing %d messages ...\n", num);
+	
+	chip_cs(SELECT, alg_data->cb_data);
+	
+	for (i=0; i<num; i++) {
+		pmsg = &msgs[i];
+		buff = (spi_pnx_msg_buff_t *) pmsg->buf;
+		DBG ("SDMA processing [%d] message ...\n", i);
+		
+		if ( pmsg->flags & SPI_M_RD ) {
+			chip_cs(BEFORE_READ, alg_data->cb_data);
+
+			init_completion (&op_complete);
+			spi_regs.con &= ~VH_BLASSPI_CON_REG_RXTX_MSK;
+			spi_regs.con |= (1 <<
VH_BLASSPI_CON_REG_SHIFT_OFF_POS);    /*  disable generating clock while
reading DAT reg  */
+			spi_regs.frm = 0x0000FFFF & (pmsg->len);
+			spi_regs.ier = (1 <<
VH_BLASSPI_IER_REG_SPI_INTEOT_POS);           /*  enable end of transfer
interrupt  */
+
+			spi_regs.dat = 0;
+			while (!(spi_regs.stat &
VH_BLASSPI_STAT_REG_SPI_THR_MSK))
+				cpu_relax();
+
+			sdma_setup.src_address      = (unsigned
int)phys_spi_data_reg;
+			sdma_setup.dest_address     = (unsigned
int)buff->dma_buffer;
+			sdma_setup.trans_length     = pmsg->len;
+			sdma_config.write_slave_nr  = MEMORY_DMA_SLAVE_NR;
+			sdma_config.read_slave_nr   = BLAS_SPI_DMA_SLAVE_NR;
+			sdma_pack_config(&sdma_config,
&sdma_setup.packed_config);
+			sdma_prog_channel (spi_sdma_channel, &sdma_setup);
+			
+			sdma_start_channel (spi_sdma_channel);
+			wait_for_completion (&op_complete);
+			sdma_stop_channel(spi_sdma_channel);
+			DBG ("[i]:SPI_M_RD: %x\n", *buff->io_buffer);
+			chip_cs(AFTER_READ, alg_data->cb_data);
+		} else {
+			chip_cs(BEFORE_WRITE, alg_data->cb_data);
+			spi_regs.con |= (1 << VH_BLASSPI_CON_REG_RXTX_POS);
+			spi_regs.frm = 0x0000FFFF & (pmsg->len);
+			spi_regs.con &= ~VH_BLASSPI_CON_REG_SHIFT_OFF_MSK;
+			
+			sdma_setup.src_address      = (unsigned
int)buff->dma_buffer;
+			sdma_setup.dest_address     = (unsigned
int)phys_spi_data_reg;
+			sdma_setup.trans_length     = pmsg->len;
+			sdma_config.write_slave_nr  = MEMORY_DMA_SLAVE_NR;
+			sdma_config.read_slave_nr   = BLAS_SPI_DMA_SLAVE_NR;
+			sdma_pack_config(&sdma_config,
&sdma_setup.packed_config);
+			sdma_prog_channel (spi_sdma_channel, &sdma_setup);
+
+			spi_regs.ier = (1 <<
VH_BLASSPI_IER_REG_SPI_INTEOT_POS);           /*  enable end of transfer
interrupt  */
+			init_completion (&op_complete);
+
+			sdma_start_channel (spi_sdma_channel);
+			wait_for_completion (&op_complete);
+			sdma_stop_channel(spi_sdma_channel);
+			DBG ("[i]:SPI_M_WR: %x\n", *buff->io_buffer);
+			chip_cs(AFTER_WRITE, alg_data->cb_data);
+		}
+	}
+	chip_cs(UNSELECT, alg_data->cb_data);
+	
+	return num;
+}
+
+int spipnx_xfer (struct spi_adapter *adap, struct spi_msg msgs[], int 
+num) {
+	int retval, i;
+	struct spi_msg *pmsg;
+	spi_pnx0105_algo_data_t *alg_data;
+
+	alg_data = (spi_pnx0105_algo_data_t *) adap->algo_data;
+	
+	DBG ("processing %d messages ...\n", num);
+	for (i=0; i<num; i++) {
+		pmsg = &msgs[i];
+		if ((pmsg->len >= 0xFFFF) || (!pmsg->buf))
+			return -EINVAL;
+		DBG ("%d) %c - %d bytes\n", i, pmsg->flags?'R':'W',
pmsg->len);
+	}
+	if (alg_data->sdma_mode) {
+		sdma_config.transfer_size     = SDMA_TRANSFER_BYTE;
+		sdma_config.invert_endian     = SDMA_INVERT_ENDIAN_NO;
+		sdma_config.companion_channel = 0;
+		sdma_config.companion_enable  = SDMA_COMPANION_DISABLE;
+		sdma_config.circular_buffer   = SDMA_CIRC_BUF_DISABLE;
+	}
+	if (alg_data->chip_cs_callback)
+		alg_data->chip_cs_callback(INIT, alg_data->cb_data);
+	else
+		spipnx_spi_init (alg_data);
+
+	if (alg_data->sdma_mode) 
+		retval = spipnx_xfer_sdma (adap, msgs, num);
+	else
+		retval = spipnx_xfer_nonsdma (adap, msgs, num);
+	return retval;
+}
+
+static void __exit spipnx_cleanup(void) {
+	free_irq(VH_INTC_INT_NUM_BLAS_SPI_INT, NULL);
+
+	printk ("SPI module for PNX0105 unloaded.\n"); }
+
+
+static int __init spipnx_init(void)
+{
+	if (request_irq(VH_INTC_INT_NUM_BLAS_SPI_INT, spipnx_interrupt,
SA_INTERRUPT, "SPI", NULL) ) {
+		DBG ("Unable to register IRQ\n");
+		return -EBUSY;
+	}
+
+	phys_spi_data_reg = &(((vhblas_spiregs *)BLAS_SPI_BASE)->dat);
+
+	memset(&sdma_config, 0, sizeof(sdma_config));
+	memset(&sdma_setup, 0, sizeof(sdma_setup));
+
+	printk ("SPI module for PNX0105 loaded.\n");
+	return 0;
+}
+
+
+EXPORT_SYMBOL(spipnx_xfer);
+EXPORT_SYMBOL(spipnx_select_chip);
+EXPORT_SYMBOL(spipnx_unselect_chip);
+EXPORT_SYMBOL(spipnx_register);
+EXPORT_SYMBOL(spipnx_unregister);
+
+
+MODULE_AUTHOR("Dennis Kovalev <dkovalev@ru.mvista.com>"); 
+MODULE_DESCRIPTION("SPI driver for PNX0105."); MODULE_LICENSE("GPL");
+
+module_init(spipnx_init);
+module_exit(spipnx_cleanup);
Index: linux-2.6.10/drivers/spi/spi-pnx010x_atmel.c
===================================================================
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-pnx010x_atmel.c
@@ -0,0 +1,199 @@
+/*
+ * drivers/spi/spi-pnx010x_atmel.c
+ *
+ * Provides Atmel SPI chip support for PNX0105.
+ *
+ * Author: Dennis Kovalev <dkovalev@ru.mvista.com>
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether 
+express
+ * or implied.
+ */
+
+/*  Uncomment the following line for debugging info  */
+/*
+#define SPIPNX_DEBUG
+*/
+#ifdef SPIPNX_DEBUG
+#define DEBUG 1
+#endif
+
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/spi/spi.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/miscdevice.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <asm/hardware.h>
+#include <asm/io.h>
+#include <asm/irq.h>
+#include <asm/uaccess.h>
+#include <asm/arch/sdma.h>
+
+#include <linux/spi/spi-pnx010x.h>
+
+
+#define DBG(args...)	pr_debug(args)
+
+static int		 mode = 0;
+static int		 sdma_mode = 0;
+static int		 spipnx_initialized;
+
+static struct spi_algorithm spipnx_algorithm = {
+	name:		ALGORITHM_NAME,
+	xfer:		spipnx_xfer,
+};
+
+static spi_pnx0105_algo_data_t spi_pnx_algo_data = {
+	cb_data:		NULL,
+	chip_cs_callback:	NULL,
+};
+
+static struct spi_adapter spipnx_adapter = {
+	name:		ADAPTER_NAME,
+	algo:		&spipnx_algorithm,
+	algo_data:	(void *) &spi_pnx_algo_data,
+	owner:		THIS_MODULE,
+	alloc:		NULL,
+	free:		NULL,
+	copy_from_user:	NULL,
+	copy_to_user:	NULL,
+};
+
+void *pnx_memory_alloc(size_t size, int base) {
+	spi_pnx_msg_buff_t *buff;
+
+	buff = (spi_pnx_msg_buff_t *) kmalloc(sizeof(spi_pnx_msg_buff_t),
base);
+	buff->size = size;
+	buff->io_buffer = dma_alloc_coherent (NULL, size, &buff->dma_buffer,

+base);
+	
+	DBG("%s:allocated memory(%x) for io_buffer = %x dma_buffer = %x\n",
+		__FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );
+
+	return (void *) buff;
+}
+
+void pnx_memory_free(const void *data)
+{
+	spi_pnx_msg_buff_t *buff;
+
+	buff = (spi_pnx_msg_buff_t *) data;
+	DBG("%s:deleted memory(%x) for io_buffer = %x dma_buffer = %x\n",
+		__FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );
+
+	dma_free_coherent (NULL, buff->size, buff->io_buffer,
buff->dma_buffer);
+	kfree(buff);
+}
+
+unsigned long pnx_copy_from_user(void *to, const void *from_user, 
+unsigned long len) {
+	spi_pnx_msg_buff_t *buff;
+	int ret;
+
+	buff = (spi_pnx_msg_buff_t *) to;
+	ret = copy_from_user(buff->io_buffer, from_user,len);
+
+	return ret;
+}
+
+unsigned long pnx_copy_to_user(void *to_user, const void *from, 
+unsigned long len) {
+	spi_pnx_msg_buff_t *buff;
+	int ret;
+
+	buff = (spi_pnx_msg_buff_t *) from;
+	ret = copy_to_user(to_user, buff->io_buffer, len);
+
+	return ret;
+}
+
+int spi_pnx_set_cs_callback (void (*cs_callback)(int, void*), void 
+*algo_data) {
+	spi_pnx0105_algo_data_t *alg_data;
+  
+	alg_data = (spi_pnx0105_algo_data_t *) algo_data;
+	alg_data->chip_cs_callback = cs_callback;
+
+	pr_debug ("%s: callback installed\n", __FUNCTION__);
+	return 0;
+}
+
+void spi_pnx_set_cs_callback_data (void *cb_data, void *algo_data) {
+	spi_pnx0105_algo_data_t *alg_data;
+  
+	alg_data = (spi_pnx0105_algo_data_t *) algo_data;
+	alg_data->cb_data = cb_data;
+}
+
+static int __init spi_sv_init (void)
+{
+ 	unsigned int reg;
+	int ret;
+	spipnx_initialized = 0;
+
+	if ((mode < 0) || (mode > 3)) {
+		DBG ("Warning: value of mode = %d is out of range. Mode is
set to default: mode = 0", mode);
+		spi_pnx_algo_data.mode = 0;
+	} else
+		spi_pnx_algo_data.mode = mode;
+	spi_pnx_algo_data.sdma_mode = sdma_mode;
+	
+	reg = gpio_read_reg (PADMUX1_MODE1);                    /*  Select
mode 1 on SPI_CE pin  */
+	gpio_write_reg (reg | GPIO_PIN_SPI_CE, PADMUX1_MODE1);
+	
+	if (spi_pnx_algo_data.sdma_mode) {
+		spipnx_adapter.alloc = pnx_memory_alloc;
+		spipnx_adapter.free = pnx_memory_free;
+		spipnx_adapter.copy_from_user = pnx_copy_from_user;
+		spipnx_adapter.copy_to_user = pnx_copy_to_user;
+	}
+
+	if (spi_add_adapter(&spipnx_adapter)) {
+		DBG ("adding adapter '%s' failed\n", ADAPTER_NAME);
+		return -EFAULT;
+	}
+
+	ret = spipnx_register(&spi_pnx_algo_data);
+	if (ret < 0)
+		return ret;
+
+	DBG ("Loaded spi-pnx010x.o in %s mode, MODE%d, SPI clock =
PCLK/%d\n",\
+		spi_pnx_algo_data.sdma_mode?"SMDA":"non-SDMA", 
+spi_pnx_algo_data.mode, (SPI_CLOCK+1)*2);
+
+	return 0;
+}
+
+static void __exit spi_sv_cleanup (void) {
+
+	spi_regs.global = 0L;     /* disable SPI periph   */
+
+	spipnx_unregister(&spi_pnx_algo_data);	
+	spi_del_adapter(&spipnx_adapter);
+
+	DBG ("module removed\n");
+}
+
+MODULE_AUTHOR("Dennis Kovalev <dkovalev@ru.mvista.com>"); 
+MODULE_DESCRIPTION("SPI driver high level for PNX0105."); 
+MODULE_LICENSE("GPL");
+
+
+MODULE_PARM(mode, "i");
+MODULE_PARM(sdma_mode, "i");
+module_init (spi_sv_init);
+module_exit (spi_sv_cleanup);
Index: linux-2.6.10/include/linux/spi/spi-pnx010x.h
===================================================================
--- /dev/null
+++ linux-2.6.10/include/linux/spi/spi-pnx010x.h
@@ -0,0 +1,100 @@
+/*
+ * SPI support for PNX0105.
+ *
+ * Author: Dennis Kovalev <dkovalev@ru.mvista.com>
+ *
+ * 2004 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether 
+express
+ * or implied.
+ */
+
+
+#ifndef CONFIG_ARCH_PNX010X
+#error This driver cannot be run on non-PNX0105 #endif
+
+#ifndef _SPI_PNX0105_DRIVER
+#define _SPI_PNX0105_DRIVER
+
+#include <asm/arch/platform.h>
+#include <vhal/spi_pnx0105.h>
+
+#define ADAPTER_NAME	"SWIFT SPI"
+#define ALGORITHM_NAME	"PNX0105 SPI"
+
+#define SPI_CLOCK		0x10
+#define FIFO_CHUNK_SIZE		56
+
+/*  GPIO related definitions  */
+#include <vhal/ioconf.h>
+#define PADMUX1_PINS		(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_PINS)
+#define PADMUX1_MODE0		(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE0)
+#define PADMUX1_MODE0SET	(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE0_SET)
+#define PADMUX1_MODE0RESET	(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE0_RESET)
+#define PADMUX1_MODE1		(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE1)
+#define PADMUX1_MODE1SET	(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE1_SET)
+#define PADMUX1_MODE1RESET	(IOCONF_PNX0105_PADMUX0 +
VH_IOCONF_REG_MODE1_RESET)
+
+#define GPIO_PIN_SPI_CE
(1<<VH_IOCONF_PNX0105_PADMUX1_MSPI_CE_POS)
+#define PADMUX1_BASE_ADDR		IO_ADDRESS(IOCONF_BASE)
+
+#define GPIO_PIN_SPI_CE
(1<<VH_IOCONF_PNX0105_PADMUX1_MSPI_CE_POS)
+#define PADMUX1_BASE_ADDR		IO_ADDRESS(IOCONF_BASE)
+
+#define gpio_write_reg(val,reg)		writel (val,
PADMUX1_BASE_ADDR + reg)
+#define gpio_read_reg(reg)		readl (PADMUX1_BASE_ADDR + reg)
+
+#if 0
+typedef enum {
+	SELECT,
+	UNSELECT,
+	INIT,
+	BEFORE_READ,
+	AFTER_READ,
+	BEFORE_WRITE,
+	AFTER_WRITE,
+} spi_pnx0105_cb_type_t;
+#endif
+
+#define spi_pnx0105_cb_type_t int
+
+typedef struct {
+int		 mode;
+int		 sdma_mode;
+void		*cb_data;
+void		(*chip_cs_callback)(int, void*);
+} spi_pnx0105_algo_data_t;
+
+typedef struct {
+	char		*io_buffer;
+	dma_addr_t	dma_buffer;
+	size_t		size;
+} spi_pnx_msg_buff_t;
+
+
+#define SPI_BASE_ADDR		IO_ADDRESS(BLAS_SPI_BASE)
+
+#define spi_regs		(*((vhblas_spiregs *)SPI_BASE_ADDR))
+
+#define SPI_INTERRUPT		IRQ_SPI
+
+
+/*******************************************************************/
+/* ----- Types and defines:  -----     */
+
+#define SPI_RECEIVE                        0
+#define SPI_TRANSMIT                       1
+
+#define SPI_ENDIAN_SWAP_NO                 0
+#define SPI_ENDIAN_SWAP_YES                1
+
+extern int spi_pnx_set_cs_callback (void (*cs_callback)(int, void *), 
+void *algo_data); extern void spi_pnx_set_cs_callback_data (void 
+*cb_data, void *algo_data); extern int spipnx_xfer (struct spi_adapter 
+*adap, struct spi_msg msgs[], int num); extern int spipnx_register 
+(spi_pnx0105_algo_data_t *); extern void spipnx_unregister 
+(spi_pnx0105_algo_data_t *); extern void spipnx_select_chip (void); 
+extern void spipnx_unselect_chip (void);
+
+#endif

Re: [RFC] SPI core -- revisited

[Alexey Dobriyan]

On Sunday 26 June 2005 23:36, dpervushin@ru.mvista.com wrote:
> Here is the sample of usage of our SPI core; this is the driver of atmel
> chip connected to the pnx spi bus; 

> --- /dev/null
> +++ linux-2.6.10/drivers/spi/spi-pnx010x.c

> +#define DBG(args...)	pr_debug(args)

Just use pr_debug()

> +static void		*phys_spi_data_reg = 0;

NULL for pointers.

> +void spipnx_select_chip (void)
> +{
> +	unsigned reg = gpio_read_reg (PADMUX1_MODE0);
> +	gpio_write_reg ((reg & ~GPIO_PIN_SPI_CE), PADMUX1_MODE0); }

Closing brackets on the separate line, _please_.

> +static void spipnx_spi_init (void *algo_data) {

As well as opening ones.

> --- /dev/null
> +++ linux-2.6.10/drivers/spi/spi-pnx010x_atmel.c

> +#define DBG(args...)	pr_debug(args)

> +static struct spi_algorithm spipnx_algorithm = {
> +	name:		ALGORITHM_NAME,
> +	xfer:		spipnx_xfer,

	.name = ALGORITHM_NAME,
	.xfer = ...

> +static spi_pnx0105_algo_data_t spi_pnx_algo_data = {
> +	cb_data:		NULL,
> +	chip_cs_callback:	NULL,
> +};

> +static struct spi_adapter spipnx_adapter = {
> +	name:		ADAPTER_NAME,
> +	algo:		&spipnx_algorithm,
> +	algo_data:	(void *) &spi_pnx_algo_data,
> +	owner:		THIS_MODULE,
> +	alloc:		NULL,
> +	free:		NULL,
> +	copy_from_user:	NULL,
> +	copy_to_user:	NULL,
> +};

> +void *pnx_memory_alloc(size_t size, int base) {
> +	spi_pnx_msg_buff_t *buff;
> +
> +	buff = (spi_pnx_msg_buff_t *) kmalloc(sizeof(spi_pnx_msg_buff_t),

Useless cast.

> base);
> +	buff->size = size;
> +	buff->io_buffer = dma_alloc_coherent (NULL, size, &buff->dma_buffer,
> 
> +base);
> +	
> +	DBG("%s:allocated memory(%x) for io_buffer = %x dma_buffer = %x\n",
> +		__FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );
> +
> +	return (void *) buff;
> +}

No error path. You're an optimist.

> +void pnx_memory_free(const void *data)
> +{
> +	spi_pnx_msg_buff_t *buff;
> +
> +	buff = (spi_pnx_msg_buff_t *) data;
> +	DBG("%s:deleted memory(%x) for io_buffer = %x dma_buffer = %x\n",
> +		__FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );

%p was invented for pointers.

> +unsigned long pnx_copy_from_user(void *to, const void *from_user, 

const void __user *from, I believe.

> +unsigned long pnx_copy_to_user(void *to_user, const void *from,

const void __user *to.

Re: [RFC] SPI core -- revisited

[Arjan van de Ven]

> %p was invented for pointers.
> 
> > +unsigned long pnx_copy_from_user(void *to, const void *from_user, 
> 
> const void __user *from, I believe.
> 
> > +unsigned long pnx_copy_to_user(void *to_user, const void *from,
> 

> const void __user *to.
> -


... or just kill the wrappers entirely!

Re: [RFC] SPI core -- revisited

[Vitaly Wool]

Arjan van de Ven wrote:

<snip>

>
> ... or just kill the wrappers entirely!
>
>  
>
Nothing's gonna work in DMA case if he kills the wrappers.

Re: [RFC] SPI core -- revisited

[Arjan van de Ven]

On Mon, 2005-06-27 at 07:37 +0400, Vitaly Wool wrote:
> Arjan van de Ven wrote:
> 
> <snip>
> 
> >
> > ... or just kill the wrappers entirely!
> >
> >  
> >
> Nothing's gonna work in DMA case if he kills the wrappers.

how is that??

Re: [RFC] SPI core -- revisited

[Vitaly Wool]

Arjan van de Ven wrote:

>>Nothing's gonna work in DMA case if he kills the wrappers.
>>    
>>
>
>how is that??
>  
>
These functions are not exactly *wrappers*, there's some little 
additional logic inside.
spi-pnx0105_atmel.c uses spi_pnx_msg_buff_t structure to embed physical 
and virtual address and length of the memory area allocated by 
consistent_alloc, so if we just get rid of the alloc/free functions, 
we'll copy wrong data from the userspace and nothing'll work.

Let's look at it from another point. When a read request comes from the 
userspace to spi-dev, spi-dev should allocate  memory and copy the user 
data in there. The problem is it is not (and shouldn't be) aware whether 
the transfer is gonna be DMA or not so spi-dev can't choose 
theappropriate method of memory allocation. Therefore it's reasonable to 
let algorithm provide routines to do that.

Re: [RFC] SPI core -- revisited

[Russell King]

On Mon, Jun 27, 2005 at 01:13:44PM +0400, Vitaly Wool wrote:
> Arjan van de Ven wrote:
> >how is that??
>
> These functions are not exactly *wrappers*, there's some little 
> additional logic inside.
> spi-pnx0105_atmel.c uses spi_pnx_msg_buff_t structure to embed physical 
> and virtual address and length of the memory area allocated by 
> consistent_alloc, so if we just get rid of the alloc/free functions, 
> we'll copy wrong data from the userspace and nothing'll work.
> 
> Let's look at it from another point. When a read request comes from the 
> userspace to spi-dev, spi-dev should allocate  memory and copy the user 
> data in there. The problem is it is not (and shouldn't be) aware whether 
> the transfer is gonna be DMA or not so spi-dev can't choose 
> theappropriate method of memory allocation. Therefore it's reasonable to 
> let algorithm provide routines to do that.

Sorry, this isn't making sense.  Are you implying that you want to use
DMA to copy data from user to kernel space?  Or something else?

In any case, I do hope that you are aware that copy_to_user/copy_from_user
have rather important side effects other than just copying data?  They
cause page faults which ensure that the user space pages are paged in
and/or copied if written to as appropriate.

In another post, you mention that this patch was not provided for review.
However, it _is_ effectively provided for review because it's an
illustration of _how_ the interfaces are used - and we're reviewing
the interfaces provided by the SPI core.

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

Re: [RFC] SPI core -- revisited

[Vitaly Wool]

Russell King wrote:

>On Mon, Jun 27, 2005 at 01:13:44PM +0400, Vitaly Wool wrote:
>  
>
>>Arjan van de Ven wrote:
>>    
>>
>>>how is that??
>>>      
>>>
>>These functions are not exactly *wrappers*, there's some little 
>>additional logic inside.
>>spi-pnx0105_atmel.c uses spi_pnx_msg_buff_t structure to embed physical 
>>and virtual address and length of the memory area allocated by 
>>consistent_alloc, so if we just get rid of the alloc/free functions, 
>>we'll copy wrong data from the userspace and nothing'll work.
>>
>>Let's look at it from another point. When a read request comes from the 
>>userspace to spi-dev, spi-dev should allocate  memory and copy the user 
>>data in there. The problem is it is not (and shouldn't be) aware whether 
>>the transfer is gonna be DMA or not so spi-dev can't choose 
>>theappropriate method of memory allocation. Therefore it's reasonable to 
>>let algorithm provide routines to do that.
>>    
>>
>
>Sorry, this isn't making sense.  Are you implying that you want to use
>DMA to copy data from user to kernel space?  Or something else?
>
>In any case, I do hope that you are aware that copy_to_user/copy_from_user
>have rather important side effects other than just copying data?  They
>cause page faults which ensure that the user space pages are paged in
>and/or copied if written to as appropriate.
>  
>
Looks like you've misunderstood me. I was talking only about mem-to-dev 
and dev-to-mem DMA transfers, where dev is actually an SPI device. The 
thing I was talking about was that the 'wrappers' are necessary to call 
*real* copy_from_user/copy_to_user with the appropriate parameters. I 
hope this can also be derived by looking in the code.

>In another post, you mention that this patch was not provided for review.
>However, it _is_ effectively provided for review because it's an
>illustration of _how_ the interfaces are used - and we're reviewing
>the interfaces provided by the SPI core.
>
>  
>
Ok, readily agreeing to that.
But -- does the interfaces review imply checking wthether the figure 
brackets are complying to K&R? ;)

Re: [RFC] SPI core -- revisited

[Greg KH]

On Mon, Jun 27, 2005 at 02:20:26PM +0400, Vitaly Wool wrote:
> But -- does the interfaces review imply checking wthether the figure 
> brackets are complying to K&R? ;)

Yes.  If you get the coding style wrong, it's harder for all of us to
review the code.  See my old OLS paper about coding style for the
research that backs this up.  

Basically, if the format is wrong, your brain has a much harder time to
see the actual logic of your code.  And you want us to review the logic
:)

thanks,

greg k-h

Re: [RFC] SPI core -- revisited

[Vitaly Wool]

Alexey Dobriyan wrote:

>> +static void        *phys_spi_data_reg = 0;
>>   
>
>
> NULL for pointers.
>
>  
>
>> +void spipnx_select_chip (void)
>> +{
>> +    unsigned reg = gpio_read_reg (PADMUX1_MODE0);
>> +    gpio_write_reg ((reg & ~GPIO_PIN_SPI_CE), PADMUX1_MODE0); }
>>   
>
>
> Closing brackets on the separate line, _please_.
>
>  
>
>> +static void spipnx_spi_init (void *algo_data) {
>>   
>
>
> As well as opening ones.
>
>  
>
>> --- /dev/null
>> +++ linux-2.6.10/drivers/spi/spi-pnx010x_atmel.c
>>   
>
>
>  
>
Hm, as far as I understand, this patch is not intended to become a part 
of kernel; neither it is posted to go through a formal review. It's just 
an example usage of the SPI core, isn't it?

What I would like to see, however, is dealing with the scatter-gather 
lists. I guess this should be in algorithm, isn't it?