public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH -mm] Blackfin: blackfin i2c driver
@ 2007-03-06  6:54 Wu, Bryan
  2007-03-06  7:18 ` Andrew Morton
  2007-03-06 21:43 ` Alexey Dobriyan
  0 siblings, 2 replies; 19+ messages in thread
From: Wu, Bryan @ 2007-03-06  6:54 UTC (permalink / raw)
  To: khali, Andrew Morton, linux-kernel, mhfan

Hi folks, 

[PATCH] Blackfin: blackfin i2c driver

The i2c linux driver for blackfin architecture which supports both GPIO
i2c operation and blackfin on-chip TWI controller i2c operation.

Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
---
 drivers/i2c/busses/Kconfig         |   47 ++++
 drivers/i2c/busses/i2c-bfin-gpio.c |   98 ++++++++++
 drivers/i2c/busses/i2c-bfin-twi.c  |  530 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 675 insertions(+)
Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
--- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-05 11:20:04.000000000 +0800
+++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-06 14:46:46.000000000 +0800
@@ -5,6 +5,53 @@
 menu "I2C Hardware Bus support"
 	depends on I2C
 
+config I2C_BFIN_GPIO
+	tristate "Generic Blackfin and HHBF533/561 development board I2C support"
+	depends on I2C && EXPERIMENTAL
+	select I2C_ALGOBIT
+	help
+	--
+
+menu "BFIN I2C SDA/SCL Selection"
+	depends on I2C_BFIN_GPIO
+config BFIN_SDA
+	int "SDA is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 2 if (BF533 || BF532 || BF531) 
+
+config BFIN_SCL
+	int "SCL is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 3 
+endmenu
+
+config I2C_BFIN_GPIO_CYCLE_DELAY
+	int "Cycle Delay in usec"
+	depends on I2C_BFIN_GPIO
+	range 1 100 
+	default 40
+
+config I2C_BFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This the TWI I2C device driver for Blackfin 534/536/537.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config TWICLK_KHZ
+	int "TWI clock (kHZ)"
+	depends on I2C_BFIN_TWI
+	default 50
+	help
+		The unit of the TWI clock is kilo HZ. Please divide the clock 
+		by 1024 if you count it in HZ. The value should be less than 400.
+
 config I2C_ALI1535
 	tristate "ALI 1535"
 	depends on I2C && PCI
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c	2007-03-06 14:47:31.000000000 +0800
@@ -0,0 +1,98 @@
+/****************************************************************
+ * Description:                                                 *
+ *                                                              *
+ * Maintainer: Meihui Fan <mhfan@ustc.edu>		        *
+ *                                                              *
+ * CopyRight (c)  2004  HHTech                                  *
+ *   www.hhcn.com, www.hhcn.org                                 *
+ *   All rights reserved.                                       *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ *                                                              *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
+#include <asm/blackfin.h>
+#include <asm/gpio.h>
+
+#define	I2C_HW_B_HHBF		    0x13
+
+static void hhbf_setsda(void *data, int state)
+{
+    if (state) {
+    	gpio_direction_input(CONFIG_BFIN_SDA);
+
+    } else {
+    	gpio_direction_output(CONFIG_BFIN_SDA);
+	gpio_set_value(CONFIG_BFIN_SDA, 0);
+    }
+}
+
+static void hhbf_setscl(void *data, int state)
+{
+	gpio_set_value(CONFIG_BFIN_SCL, state);
+}
+
+static int hhbf_getsda(void *data)
+{
+      return (gpio_get_value(CONFIG_BFIN_SDA) != 0);
+}
+
+
+static struct i2c_algo_bit_data bit_hhbf_data = {
+    .setsda  = hhbf_setsda,
+    .setscl  = hhbf_setscl,
+    .getsda  = hhbf_getsda,
+    .udelay  = CONFIG_I2C_BFIN_GPIO_CYCLE_DELAY,
+    .timeout = HZ
+};
+
+static struct i2c_adapter hhbf_ops = {
+    .owner 	= THIS_MODULE,
+    .id 	= I2C_HW_B_HHBF,
+    .algo_data 	= &bit_hhbf_data,
+    .name	= "HHBF I2C driver",
+};
+
+static int __init i2c_hhbf_init(void)
+{
+
+    if(gpio_request(CONFIG_BFIN_SCL, NULL)) {
+    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SCL);
+	return -1;
+	}
+
+    if(gpio_request(CONFIG_BFIN_SDA, NULL)) {
+    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SDA);
+	return -1;
+	}
+
+
+    gpio_direction_output(CONFIG_BFIN_SCL);
+    gpio_direction_input(CONFIG_BFIN_SDA);
+    gpio_set_value(CONFIG_BFIN_SCL, 1);    
+
+    return i2c_bit_add_bus(&hhbf_ops);
+}
+
+static void __exit i2c_hhbf_exit(void)
+{
+    gpio_free(CONFIG_BFIN_SCL);
+    gpio_free(CONFIG_BFIN_SDA);    
+    i2c_bit_del_bus(&hhbf_ops);
+}
+
+MODULE_AUTHOR("Meihui Fan <mhfan@ustc.edu>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin and HHBF Boards");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_hhbf_init);
+module_exit(i2c_hhbf_exit);
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c	2007-03-06 14:46:46.000000000 +0800
@@ -0,0 +1,530 @@
+/****************************************************************
+ * Description: Driver for Blackfin Two Wire Interface          *
+ * Maintainer:  sonicz  <sonic.zhang@analog.com>                *
+ *                                                              *
+ * Copyright (c) 2005-2007 Analog Device                        *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/mm.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+
+#include <asm/blackfin.h>
+#include <asm/irq.h>
+
+#define I2C_BFIN_TWI       0x00
+#define POLL_TIMEOUT       (2 * HZ)
+#ifndef CONFIG_TWICLK_KHZ
+#define CONFIG_TWICLK_KHZ  400
+#endif
+
+/* SMBus mode*/
+#define TWI_I2C_MODE_STANDARD		0x01
+#define TWI_I2C_MODE_STANDARDSUB	0x02
+#define TWI_I2C_MODE_COMBINED		0x04
+
+struct bfin_twi_iface
+{
+	struct semaphore 	twi_lock;
+	int			irq;
+	spinlock_t		lock;
+	char			read_write;
+	u8			command;
+	u8			*transPtr;
+	int			readNum;
+	int			writeNum;
+	int			cur_mode;
+	int			manual_stop;
+	int			result;
+	int			timeout_count;
+	struct timer_list	timeout_timer;
+	struct i2c_adapter	adap;
+	struct completion	complete;
+};
+
+static struct bfin_twi_iface twi_iface;
+
+static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
+{
+	unsigned short twi_int_stat = bfin_read_TWI_INT_STAT();
+	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
+
+	if (XMTSERV & twi_int_stat) {
+		/* Transmit next data */
+		if (iface->writeNum > 0) {
+			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+			iface->writeNum--;
+		}
+		/* start receive immediately after complete sending in combine mode. */
+		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MDIR | RSTART);
+		} else if (iface->manual_stop)
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | STOP);
+		SSYNC();
+		/* Clear status */
+		bfin_write_TWI_INT_STAT(XMTSERV);
+		SSYNC();
+	}
+	if (RCVSERV & twi_int_stat) {
+		if (iface->readNum > 0) {
+			/* Receive next data */
+			*(iface->transPtr) = bfin_read_TWI_RCV_DATA8();
+			if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+				/* Change combine mode into sub mode after read first data. */
+				iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+				/* Get read number from first byte in block combine mode. */
+				if (iface->readNum == 1 && iface->manual_stop)
+					iface->readNum = *iface->transPtr+1;
+			}
+			iface->transPtr++;
+			iface->readNum--;
+		} else if (iface->manual_stop) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | STOP);
+			SSYNC();
+		}
+		/* Clear interrupt source */
+		bfin_write_TWI_INT_STAT(RCVSERV);
+		SSYNC();
+	}
+	if (MERR & twi_int_stat) {
+		bfin_write_TWI_INT_STAT(MERR);
+		bfin_write_TWI_INT_MASK(0);
+		bfin_write_TWI_MASTER_STAT(0x3e);
+		bfin_write_TWI_MASTER_CTL(0);
+		SSYNC();
+		iface->result = -1;
+		/* if both err and complete int stats are set, return proper results. */
+		if (MCOMP & twi_int_stat) {
+			bfin_write_TWI_INT_STAT(MCOMP);
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			/* If it is a quick transfer, only address bug no data, not an err, return 1. */
+			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
+				iface->result = 1;
+			/* If address not acknowledged return -1, else return 0. */
+			else if (!(mast_stat & ANAK))
+				iface->result = 0;
+		}
+		complete(&iface->complete);
+		return;
+	}
+	if (MCOMP & twi_int_stat) {
+		bfin_write_TWI_INT_STAT(MCOMP);
+		SSYNC();
+		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			if (iface->readNum == 0) {
+				/* set the read number to 1 and ask for manual stop in block combine mode */
+				iface->readNum = 1;
+				iface->manual_stop = 1;
+				bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | (0xff << 6));
+			} else {
+				/* set the readd number in other combine mode. */
+				bfin_write_TWI_MASTER_CTL((bfin_read_TWI_MASTER_CTL() & (~(0xff << 6))) | ( iface->readNum << 6));
+			}
+			/* remove restart bit and enable master receive */
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() & ~RSTART);
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | MDIR);
+			SSYNC();
+		} else {
+			iface->result = 1;
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			complete(&iface->complete);
+		}
+	}
+}
+
+/* Interrupt handler */
+static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)dev_id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	del_timer(&iface->timeout_timer);
+	bfin_twi_handle_interrupt(iface);
+	spin_unlock_irqrestore(&iface->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static void bfin_twi_timeout(unsigned long data)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	bfin_twi_handle_interrupt(iface);
+	if (iface->result == 0) {
+		iface->timeout_count--;
+		if (iface->timeout_count > 0) {
+			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+			add_timer(&iface->timeout_timer);
+		} else {
+			iface->result = -1;
+			complete(&iface->complete);
+		}
+	}
+	spin_unlock_irqrestore(&iface->lock, flags);
+}
+
+/*
+ * Generic i2c master transfer entrypoint
+ */
+static int bfin_twi_master_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	struct bfin_twi_iface* iface = (struct bfin_twi_iface*)adap->algo_data;
+	struct i2c_msg *pmsg;
+	int i, ret;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	down(&iface->twi_lock);
+
+	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		up(&iface->twi_lock);
+		schedule();
+		down(&iface->twi_lock);
+	}
+
+	ret = 0;
+	for (i = 0; rc >= 0 && i < num;) {
+		pmsg = &msgs[i++];
+		if (pmsg->flags & I2C_M_TEN) {
+			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr not supported !\n");
+			rc = -EINVAL;
+			break;
+		}
+
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		iface->manual_stop = 0;
+		iface->transPtr = pmsg->buf;
+		iface->writeNum = iface->readNum = pmsg->len;
+		iface->result = 0;
+		iface->timeout_count = 10;
+		/* Set Transmit device address */
+		bfin_write_TWI_MASTER_ADDR(pmsg->addr);
+
+		/* FIFO Initiation. Data in FIFO should be discarded before start a new operation.*/
+		bfin_write_TWI_FIFO_CTL(0x3);
+		SSYNC();
+		bfin_write_TWI_FIFO_CTL(0);
+		SSYNC();
+
+		if (pmsg->flags & I2C_M_RD)
+			iface->read_write = I2C_SMBUS_READ;
+		else {
+			iface->read_write = I2C_SMBUS_WRITE;
+			/* Transmit first data */
+			if (iface->writeNum > 0) {
+				bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+				iface->writeNum--;
+				SSYNC();
+			}
+		}
+
+		/* clear int stat */
+		bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+		/* Interrupt mask . Enable XMT, RCV interrupt */
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | ((iface->read_write == I2C_SMBUS_READ)? RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (pmsg->len > 0 && pmsg->len <= 255)
+			bfin_write_TWI_MASTER_CTL(( pmsg->len << 6 ));
+		else if (pmsg->len > 255) {
+			bfin_write_TWI_MASTER_CTL(( 0xff << 6 ));
+			iface->manual_stop = 1;
+		} else
+			break;
+
+		iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+		add_timer(&iface->timeout_timer);
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | ((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) | ((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		SSYNC();
+
+		wait_for_completion(&iface->complete);
+
+		rc = iface->result;
+		if (rc == 1)
+			ret++;
+		else if (rc == -1)
+			break;
+	}
+
+	/* Release sem */
+	up(&iface->twi_lock);
+
+	return ret;
+}
+
+/*
+ * SMBus type transfer entrypoint
+ */
+
+int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			unsigned short flags, char read_write,
+			u8 command, int size, union i2c_smbus_data * data)
+{
+	struct bfin_twi_iface* iface = (struct bfin_twi_iface*)adap->algo_data;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	down(&iface->twi_lock);
+
+	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		up(&iface->twi_lock);
+		schedule();
+		down(&iface->twi_lock);
+	}
+
+	iface->writeNum = 0;
+	iface->readNum = 0;
+
+	/* Prepare datas & select mode */
+	switch (size) {
+	case I2C_SMBUS_QUICK:
+		iface->transPtr = NULL;
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE:
+		if (data == NULL)
+			iface->transPtr = NULL;
+		else {
+			if (read_write == I2C_SMBUS_READ)
+				iface->readNum = 1;
+			else
+				iface->writeNum = 1;
+			iface->transPtr = &data->byte;
+		}
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = &data->byte;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		iface->writeNum = 2;
+		iface->readNum = 2;
+		iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 0;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = data->block[0]+1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = data->block;
+		break;
+	default:
+		return -1;
+	}
+
+	iface->result = 0;
+	iface->manual_stop = 0;
+	iface->read_write = read_write;
+	iface->command = command;
+	iface->timeout_count = 10;
+
+	/* FIFO Initiation. Data in FIFO should be discarded before start a new operation.*/
+	bfin_write_TWI_FIFO_CTL(0x3);
+	SSYNC();
+	bfin_write_TWI_FIFO_CTL(0);
+
+	/* clear int stat */
+	bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+	/* Set Transmit device address */
+	bfin_write_TWI_MASTER_ADDR(addr);
+	SSYNC();
+
+	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+	add_timer(&iface->timeout_timer);
+
+	switch (iface->cur_mode) {
+	case TWI_I2C_MODE_STANDARDSUB:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | ((iface->read_write == I2C_SMBUS_READ) ? RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (iface->writeNum + 1 <= 255)
+			bfin_write_TWI_MASTER_CTL(((iface->writeNum+1) << 6));
+		else {
+			bfin_write_TWI_MASTER_CTL(( 0xff << 6 ));
+			iface->manual_stop = 1;
+		}
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | ((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	case TWI_I2C_MODE_COMBINED:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
+		SSYNC();
+
+		if (iface->writeNum > 0)
+			bfin_write_TWI_MASTER_CTL(((iface->writeNum+1) << 6));
+		else
+			bfin_write_TWI_MASTER_CTL(( 0x1 << 6 ));
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | ((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	default:
+		bfin_write_TWI_MASTER_CTL(0);
+		if (size != I2C_SMBUS_QUICK) {
+			/* Don't access xmit data register when this is a read operation. */
+			if (iface->read_write != I2C_SMBUS_READ) {
+				if (iface->writeNum > 0) {
+					bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+					if (iface->writeNum <= 255)
+						bfin_write_TWI_MASTER_CTL(( iface->writeNum << 6 ));
+					else {
+						bfin_write_TWI_MASTER_CTL(( 0xff << 6 ));
+						iface->manual_stop = 1;
+					}
+					iface->writeNum--;
+				} else {
+					bfin_write_TWI_XMT_DATA8(iface->command);
+					bfin_write_TWI_MASTER_CTL(( 1 << 6 ));
+				}
+			} else {
+				if (iface->readNum > 0 && iface->readNum <= 255)
+					bfin_write_TWI_MASTER_CTL(( iface->readNum << 6 ));
+				else if (iface->readNum > 255) {
+					bfin_write_TWI_MASTER_CTL(( 0xff << 6 ));
+					iface->manual_stop = 1;
+				} else {
+					del_timer(&iface->timeout_timer);
+					break;
+				}
+			}
+		}
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | ((iface->read_write == I2C_SMBUS_READ) ? RCVSERV : XMTSERV));
+		SSYNC();
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | ((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0)	| ((CONFIG_TWICLK_KHZ > 100) ? FAST : 0));
+		break;
+	}
+	SSYNC();
+
+	wait_for_completion(&iface->complete);
+
+	rc = (iface->result >= 0) ? 0 : -1;
+
+	/* Release sem */
+	up(&iface->twi_lock);
+
+	return rc;
+}
+
+/*
+ * Return what the adapter supports
+ */
+static u32 bfin_twi_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
+	       I2C_FUNC_I2C;
+}
+
+
+static struct i2c_algorithm bfin_twi_algorithm = {
+	.master_xfer   = bfin_twi_master_xfer,
+	.smbus_xfer    = bfin_twi_smbus_xfer,
+	.functionality = bfin_twi_functionality,
+};
+
+static int __init i2c_bfin_twi_init(void)
+{
+	struct i2c_adapter *p_adap;
+	int rc;
+
+	init_MUTEX(&(twi_iface.twi_lock));
+	spin_lock_init(&twi_iface.lock);
+	init_completion(&twi_iface.complete);
+	twi_iface.irq = IRQ_TWI;
+
+	init_timer(&twi_iface.timeout_timer);
+	twi_iface.timeout_timer.function = bfin_twi_timeout;
+	twi_iface.timeout_timer.data = (unsigned long)&twi_iface;
+
+	p_adap = &twi_iface.adap;
+	p_adap->id = I2C_BFIN_TWI;
+	strcpy(p_adap->name, "i2c-bfin-twi");
+	p_adap->algo = &bfin_twi_algorithm;
+	p_adap->algo_data = &twi_iface;
+	p_adap->client_register = NULL;
+	p_adap->client_unregister = NULL;
+	p_adap->class = I2C_CLASS_ALL;
+
+	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry, SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
+	if (rc) {
+		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n", twi_iface.irq);
+		return -ENODEV;
+	}
+
+	/* Set TWI internal clock as 10MHz */
+	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
+
+	/* Set Twi interface clock as specified */
+	if (CONFIG_TWICLK_KHZ > 400)
+		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) | (( 5*1024 / 400 ) & 0xFF));
+	else
+		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_TWICLK_KHZ ) << 8) | (( 5*1024 / CONFIG_TWICLK_KHZ ) & 0xFF));
+
+	/* Enable TWI */
+	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
+	SSYNC();
+
+	return i2c_add_adapter(p_adap);
+}
+
+static void __exit i2c_bfin_twi_exit(void)
+{
+	i2c_del_adapter(&twi_iface.adap);
+	free_irq(twi_iface.irq, &twi_iface);
+}
+
+MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin TWI");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_bfin_twi_init);
+module_exit(i2c_bfin_twi_exit);
_

Thanks
-Bryan Wu

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-06  6:54 [PATCH -mm] Blackfin: blackfin i2c driver Wu, Bryan
@ 2007-03-06  7:18 ` Andrew Morton
  2007-03-07  5:17   ` Sonic Zhang
  2007-03-06 21:43 ` Alexey Dobriyan
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-03-06  7:18 UTC (permalink / raw)
  To: bryan.wu; +Cc: khali, linux-kernel, mhfan

On Tue, 06 Mar 2007 14:54:18 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:

> Hi folks, 
> 
> [PATCH] Blackfin: blackfin i2c driver
> 
> The i2c linux driver for blackfin architecture which supports both GPIO
> i2c operation and blackfin on-chip TWI controller i2c operation.
> 

Little things...

> +static int __init i2c_hhbf_init(void)
> +{
> +
> +    if(gpio_request(CONFIG_BFIN_SCL, NULL)) {
> +    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SCL);
> +	return -1;
> +	}
> +
> +    if(gpio_request(CONFIG_BFIN_SDA, NULL)) {
> +    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SDA);
> +	return -1;
> +	}

whitespace breakage there

> +
> +    gpio_direction_output(CONFIG_BFIN_SCL);
> +    gpio_direction_input(CONFIG_BFIN_SDA);
> +    gpio_set_value(CONFIG_BFIN_SCL, 1);    
> +
> +    return i2c_bit_add_bus(&hhbf_ops);
> +}
> +
> +#define TWI_I2C_MODE_COMBINED		0x04
> +
> +struct bfin_twi_iface
> +{

	struct bfin_twi_iface {

> +	struct semaphore 	twi_lock;

Can this be converted to a mutex?

> +	int			irq;
> +	spinlock_t		lock;
> + */
>
> ...
>
> +static int bfin_twi_master_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +{
> +	struct bfin_twi_iface* iface = (struct bfin_twi_iface*)adap->algo_data;

This code has zillions of unneeded casts of void*

> +	struct i2c_msg *pmsg;
> +	int i, ret;
> +	int rc = 0;
> +
> +	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> +		return -ENXIO;
> +
> +	down(&iface->twi_lock);
> +
> +	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> +		up(&iface->twi_lock);
> +		schedule();
> +		down(&iface->twi_lock);
> +	}

That's a busy loop until this task's timeslice has expired.  It'll work,
but it'll suck a bit.  (Repeated in several places)

> +	ret = 0;
> +	for (i = 0; rc >= 0 && i < num;) {
> +		pmsg = &msgs[i++];

Strange.  Why not do the i++ in the `for' statement?

> +		if (pmsg->flags & I2C_M_TEN) {
> +			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr not supported !\n");
> +			rc = -EINVAL;
> +			break;
> +		}
> +
>
> ...
>
> +	switch (iface->cur_mode) {
> +	case TWI_I2C_MODE_STANDARDSUB:
> +		bfin_write_TWI_XMT_DATA8(iface->command);
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR | ((iface->read_write == I2C_SMBUS_READ) ? RCVSERV : XMTSERV));

It's preferred if code is readable in an 80-col display.

> +		SSYNC();
> +
> +		if (iface->writeNum + 1 <= 255)
> +			bfin_write_TWI_MASTER_CTL(((iface->writeNum+1) << 6));


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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-06  6:54 [PATCH -mm] Blackfin: blackfin i2c driver Wu, Bryan
  2007-03-06  7:18 ` Andrew Morton
@ 2007-03-06 21:43 ` Alexey Dobriyan
  2007-03-06 23:04   ` Mike Frysinger
  2007-03-07  5:57   ` Wu, Bryan
  1 sibling, 2 replies; 19+ messages in thread
From: Alexey Dobriyan @ 2007-03-06 21:43 UTC (permalink / raw)
  To: Wu, Bryan; +Cc: khali, Andrew Morton, linux-kernel, mhfan

On Tue, Mar 06, 2007 at 02:54:18PM +0800, Wu, Bryan wrote:
> [PATCH] Blackfin: blackfin i2c driver
>
> The i2c linux driver for blackfin architecture which supports both GPIO
> i2c operation and blackfin on-chip TWI controller i2c operation.

> +config TWICLK_KHZ
> +	int "TWI clock (kHZ)"
> +	depends on I2C_BFIN_TWI
> +	default 50
> +	help
> +		The unit of the TWI clock is kilo HZ. Please divide the clock 
> +		by 1024 if you count it in HZ. The value should be less than 400.

Indentation in Kconfig files is Tab-Space-Space.

> --- /dev/null
> +++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c

> +static int __init i2c_hhbf_init(void)
> +{
> +
> +    if(gpio_request(CONFIG_BFIN_SCL, NULL)) {
> +    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SCL);

I may be first to ask this, but, please, use __func__ in all new code.
__FUNCTION__ is gcc-2.95-ism, which is no longer supported.

> +	return -1;
> +	}
> +
> +    if(gpio_request(CONFIG_BFIN_SDA, NULL)) {
> +    	printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__FUNCTION__, CONFIG_BFIN_SDA);

__func__, Do you need to gpio_free() ?

> +	return -1;
> +	}
> +
> +
> +    gpio_direction_output(CONFIG_BFIN_SCL);
> +    gpio_direction_input(CONFIG_BFIN_SDA);
> +    gpio_set_value(CONFIG_BFIN_SCL, 1);    
> +
> +    return i2c_bit_add_bus(&hhbf_ops);

Do you need unwinding with gpio_free() like below?

> +static void __exit i2c_hhbf_exit(void)
> +{
> +    gpio_free(CONFIG_BFIN_SCL);
> +    gpio_free(CONFIG_BFIN_SDA);    

trailing whitespace.

> +    i2c_bit_del_bus(&hhbf_ops);
> +}

> --- /dev/null
> +++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c

> +static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
> +{
> +	unsigned short twi_int_stat = bfin_read_TWI_INT_STAT();
> +	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
> +
> +	if (XMTSERV & twi_int_stat) {

Please, put bitmasks on the right. Much easier to read. See below for
other places.

	if (twi_int_stat & XMTSERV)

Also, I'd rename variable to twi_intr_status, to not confuse with
statistics or something.

> +		/* Transmit next data */
> +		if (iface->writeNum > 0) {
> +			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
> +			iface->writeNum--;

StudlyCaps.

> +static int bfin_twi_master_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +{
> +	struct bfin_twi_iface* iface = (struct bfin_twi_iface*)adap->algo_data;
> +	struct i2c_msg *pmsg;
> +	int i, ret;
> +	int rc = 0;
> +
> +	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> +		return -ENXIO;
> +
> +	down(&iface->twi_lock);
> +
> +	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> +		up(&iface->twi_lock);
> +		schedule();
> +		down(&iface->twi_lock);
> +	}

> +int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> +			unsigned short flags, char read_write,
> +			u8 command, int size, union i2c_smbus_data * data)
> +{
> +	struct bfin_twi_iface* iface = (struct bfin_twi_iface*)adap->algo_data;
> +	int rc = 0;
> +
> +	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> +		return -ENXIO;
> +
> +	down(&iface->twi_lock);
> +
> +	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> +		up(&iface->twi_lock);
> +		schedule();
> +		down(&iface->twi_lock);
> +	}

Smells like schedule() abuse for delays.

> +	switch (iface->cur_mode) {
> +	case TWI_I2C_MODE_STANDARDSUB:
> +		bfin_write_TWI_XMT_DATA8(iface->command);
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR | ((iface->read_write == I2C_SMBUS_READ) ? RCVSERV : XMTSERV));
> +		SSYNC();
> +
> +		if (iface->writeNum + 1 <= 255)
> +			bfin_write_TWI_MASTER_CTL(((iface->writeNum+1) << 6));

Too many ():

			bfin_write_TWI_MASTER_CTL((iface->writeNum + 1) << 6);

> +		else {
> +			bfin_write_TWI_MASTER_CTL(( 0xff << 6 ));

ditto

> +			iface->manual_stop = 1;
> +		}
> +		/* Master enable */
> +		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | ((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
> +		break;
> +	case TWI_I2C_MODE_COMBINED:
> +		bfin_write_TWI_XMT_DATA8(iface->command);
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
> +		SSYNC();
> +
> +		if (iface->writeNum > 0)
> +			bfin_write_TWI_MASTER_CTL(((iface->writeNum+1) << 6));

ditto

> +		else
> +			bfin_write_TWI_MASTER_CTL(( 0x1 << 6 ));

ditto, etc

> +static int __init i2c_bfin_twi_init(void)
> +{

> +	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry, SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
> +	if (rc) {
> +		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n", twi_iface.irq);
> +		return -ENODEV;
> +	}
> +
> +	/* Set TWI internal clock as 10MHz */
> +	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
> +
> +	/* Set Twi interface clock as specified */
> +	if (CONFIG_TWICLK_KHZ > 400)
> +		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) | (( 5*1024 / 400 ) & 0xFF));
> +	else
> +		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_TWICLK_KHZ ) << 8) | (( 5*1024 / CONFIG_TWICLK_KHZ ) & 0xFF));
> +
> +	/* Enable TWI */
> +	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
> +	SSYNC();
> +
> +	return i2c_add_adapter(p_adap);

free_irq on error?


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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-06 21:43 ` Alexey Dobriyan
@ 2007-03-06 23:04   ` Mike Frysinger
  2007-03-07  5:57   ` Wu, Bryan
  1 sibling, 0 replies; 19+ messages in thread
From: Mike Frysinger @ 2007-03-06 23:04 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Wu, Bryan, khali, Andrew Morton, linux-kernel, mhfan

On 3/6/07, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > +static int __init i2c_bfin_twi_init(void)
> > +{
> > +     rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry, SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
> > +     if (rc) {
> > +             printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n", twi_iface.irq);
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Set TWI internal clock as 10MHz */
> > +     bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
> > +
> > +     /* Set Twi interface clock as specified */
> > +     if (CONFIG_TWICLK_KHZ > 400)
> > +             bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) | (( 5*1024 / 400 ) & 0xFF));
> > +     else
> > +             bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_TWICLK_KHZ ) << 8) | (( 5*1024 / CONFIG_TWICLK_KHZ ) & 0xFF));
> > +
> > +     /* Enable TWI */
> > +     bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
> > +     SSYNC();
> > +
> > +     return i2c_add_adapter(p_adap);
>
> free_irq on error?

what error ?  do you mean i2c_add_adapter() ?
-mike

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-06  7:18 ` Andrew Morton
@ 2007-03-07  5:17   ` Sonic Zhang
  2007-03-07  6:06     ` Andrew Morton
  0 siblings, 1 reply; 19+ messages in thread
From: Sonic Zhang @ 2007-03-07  5:17 UTC (permalink / raw)
  To: Andrew Morton; +Cc: bryan.wu, khali, linux-kernel, mhfan

On 3/6/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 06 Mar 2007 14:54:18 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:
>
> > Hi folks,
> >
> > [PATCH] Blackfin: blackfin i2c driver
> >

> > +     struct i2c_msg *pmsg;
> > +     int i, ret;
> > +     int rc = 0;
> > +
> > +     if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> > +             return -ENXIO;
> > +
> > +     down(&iface->twi_lock);
> > +
> > +     while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> > +             up(&iface->twi_lock);
> > +             schedule();
> > +             down(&iface->twi_lock);
> > +     }
>
> That's a busy loop until this task's timeslice has expired.  It'll work,
> but it'll suck a bit.  (Repeated in several places)
>

OK, I change it into yield(). So, current process will be move to the
tail of the run queue. Is that OK with you?

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-06 21:43 ` Alexey Dobriyan
  2007-03-06 23:04   ` Mike Frysinger
@ 2007-03-07  5:57   ` Wu, Bryan
  2007-03-07  6:58     ` Jean Delvare
  2007-03-07  7:02     ` Andrew Morton
  1 sibling, 2 replies; 19+ messages in thread
From: Wu, Bryan @ 2007-03-07  5:57 UTC (permalink / raw)
  To: Alexey Dobriyan, Andrew Morton, Sonic Zhang, Mike Frysinger
  Cc: Wu, Bryan, khali, linux-kernel, mhfan

Dear Andrew and Alexey:

Thanks a lot for the review.

Here is the updated blackfin i2c driver.

[PATCH] Blackfin: blackfin i2c driver

The i2c linux driver for blackfin architecture which supports both GPIO
i2c operation and blackfin on-chip TWI controller i2c operation.

Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
---
 drivers/i2c/busses/Kconfig         |   47 ++++
 drivers/i2c/busses/i2c-bfin-gpio.c |   98 +++++++++
 drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 734 insertions(+)

Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
--- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 13:32:02.000000000 +0800
+++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 13:44:19.000000000 +0800
@@ -5,6 +5,53 @@
 menu "I2C Hardware Bus support"
 	depends on I2C
 
+config I2C_BFIN_GPIO
+	tristate "Generic Blackfin and HHBF533/561 development board I2C support"
+	depends on I2C && EXPERIMENTAL
+	select I2C_ALGOBIT
+	help
+	--
+
+menu "BFIN I2C SDA/SCL Selection"
+	depends on I2C_BFIN_GPIO
+config BFIN_SDA
+	int "SDA is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 2 if (BF533 || BF532 || BF531) 
+
+config BFIN_SCL
+	int "SCL is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 3 
+endmenu
+
+config I2C_BFIN_GPIO_CYCLE_DELAY
+	int "Cycle Delay in usec"
+	depends on I2C_BFIN_GPIO
+	range 1 100 
+	default 40
+
+config I2C_BFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This the TWI I2C device driver for Blackfin 534/536/537.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config TWICLK_KHZ
+	int "TWI clock (kHZ)"
+	depends on I2C_BFIN_TWI
+	default 50
+	help
+	  The unit of the TWI clock is kilo HZ. Please divide the clock 
+	  by 1024 if you count it in HZ. The value should be less than 400.
+
 config I2C_ALI1535
 	tristate "ALI 1535"
 	depends on I2C && PCI
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c	2007-03-07 13:44:19.000000000 +0800
@@ -0,0 +1,98 @@
+/****************************************************************
+ * Description:                                                 *
+ *                                                              *
+ * Maintainer: Meihui Fan <mhfan@ustc.edu>		        *
+ *                                                              *
+ * CopyRight (c)  2004  HHTech                                  *
+ *   www.hhcn.com, www.hhcn.org                                 *
+ *   All rights reserved.                                       *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ *                                                              *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
+#include <asm/blackfin.h>
+#include <asm/gpio.h>
+
+#define	I2C_HW_B_HHBF		    0x13
+
+static void hhbf_setsda(void *data, int state)
+{
+	if (state) {
+		gpio_direction_input(CONFIG_BFIN_SDA);
+
+	} else {
+		gpio_direction_output(CONFIG_BFIN_SDA);
+		gpio_set_value(CONFIG_BFIN_SDA, 0);
+	}
+}
+
+static void hhbf_setscl(void *data, int state)
+{
+	gpio_set_value(CONFIG_BFIN_SCL, state);
+}
+
+static int hhbf_getsda(void *data)
+{
+	return (gpio_get_value(CONFIG_BFIN_SDA) != 0);
+}
+
+
+static struct i2c_algo_bit_data bit_hhbf_data = {
+	.setsda  = hhbf_setsda,
+	.setscl  = hhbf_setscl,
+	.getsda  = hhbf_getsda,
+	.udelay  = CONFIG_I2C_BFIN_GPIO_CYCLE_DELAY,
+	.timeout = HZ
+};
+
+static struct i2c_adapter hhbf_ops = {
+	.owner 	= THIS_MODULE,
+	.id 	= I2C_HW_B_HHBF,
+	.algo_data 	= &bit_hhbf_data,
+	.name	= "HHBF I2C driver",
+};
+
+static int __init i2c_hhbf_init(void)
+{
+
+	if (gpio_request(CONFIG_BFIN_SCL, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__func__, CONFIG_BFIN_SCL);
+		return -1;
+	}
+
+	if (gpio_request(CONFIG_BFIN_SDA, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__func__, CONFIG_BFIN_SDA);
+		return -1;
+	}
+
+
+	gpio_direction_output(CONFIG_BFIN_SCL);
+	gpio_direction_input(CONFIG_BFIN_SDA);
+	gpio_set_value(CONFIG_BFIN_SCL, 1);
+
+	return i2c_bit_add_bus(&hhbf_ops);
+}
+
+static void __exit i2c_hhbf_exit(void)
+{
+	gpio_free(CONFIG_BFIN_SCL);
+	gpio_free(CONFIG_BFIN_SDA);
+	i2c_bit_del_bus(&hhbf_ops);
+}
+
+MODULE_AUTHOR("Meihui Fan <mhfan@ustc.edu>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin and HHBF Boards");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_hhbf_init);
+module_exit(i2c_hhbf_exit);
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c	2007-03-07 13:44:19.000000000 +0800
@@ -0,0 +1,589 @@
+/****************************************************************
+ * Description: Driver for Blackfin Two Wire Interface          *
+ * Maintainer:  sonicz  <sonic.zhang@analog.com>                *
+ *                                                              *
+ * Copyright (c) 2005-2007 Analog Device                        *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/mm.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+
+#include <asm/blackfin.h>
+#include <asm/irq.h>
+
+#define I2C_BFIN_TWI       0x00
+#define POLL_TIMEOUT       (2 * HZ)
+#ifndef CONFIG_TWICLK_KHZ
+#define CONFIG_TWICLK_KHZ  400
+#endif
+
+/* SMBus mode*/
+#define TWI_I2C_MODE_STANDARD		0x01
+#define TWI_I2C_MODE_STANDARDSUB	0x02
+#define TWI_I2C_MODE_COMBINED		0x04
+
+struct bfin_twi_iface
+{
+	struct mutex		twi_lock;
+	int			irq;
+	spinlock_t		lock;
+	char			read_write;
+	u8			command;
+	u8			*transPtr;
+	int			readNum;
+	int			writeNum;
+	int			cur_mode;
+	int			manual_stop;
+	int			result;
+	int			timeout_count;
+	struct timer_list	timeout_timer;
+	struct i2c_adapter	adap;
+	struct completion	complete;
+};
+
+static struct bfin_twi_iface twi_iface;
+
+static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
+{
+	unsigned short twi_int_status = bfin_read_TWI_INT_STAT();
+	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
+
+	if (twi_int_status & XMTSERV) {
+		/* Transmit next data */
+		if (iface->writeNum > 0) {
+			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+			iface->writeNum--;
+		}
+		/* start receive immediately after complete sending in
+		 * combine mode.
+		 */
+		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| MDIR | RSTART);
+		} else if (iface->manual_stop)
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+		SSYNC();
+		/* Clear status */
+		bfin_write_TWI_INT_STAT(XMTSERV);
+		SSYNC();
+	}
+	if (twi_int_status & RCVSERV) {
+		if (iface->readNum > 0) {
+			/* Receive next data */
+			*(iface->transPtr) = bfin_read_TWI_RCV_DATA8();
+			if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+				/* Change combine mode into sub mode after
+				 * read first data.
+				 */
+				iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+				/* Get read number from first byte in block
+				 * combine mode.
+				 */
+				if (iface->readNum == 1 && iface->manual_stop)
+					iface->readNum = *iface->transPtr+1;
+			}
+			iface->transPtr++;
+			iface->readNum--;
+		} else if (iface->manual_stop) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+			SSYNC();
+		}
+		/* Clear interrupt source */
+		bfin_write_TWI_INT_STAT(RCVSERV);
+		SSYNC();
+	}
+	if (twi_int_status & MERR) {
+		bfin_write_TWI_INT_STAT(MERR);
+		bfin_write_TWI_INT_MASK(0);
+		bfin_write_TWI_MASTER_STAT(0x3e);
+		bfin_write_TWI_MASTER_CTL(0);
+		SSYNC();
+		iface->result = -1;
+		/* if both err and complete int stats are set, return proper
+		 * results.
+		 */
+		if (twi_int_status & MCOMP) {
+			bfin_write_TWI_INT_STAT(MCOMP);
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			/* If it is a quick transfer, only address bug no data,
+			 * not an err, return 1.
+			 */
+			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
+				iface->result = 1;
+			/* If address not acknowledged return -1,
+			 * else return 0.
+			 */
+			else if (!(mast_stat & ANAK))
+				iface->result = 0;
+		}
+		complete(&iface->complete);
+		return;
+	}
+	if (twi_int_status & MCOMP) {
+		bfin_write_TWI_INT_STAT(MCOMP);
+		SSYNC();
+		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			if (iface->readNum == 0) {
+				/* set the read number to 1 and ask for manual
+				 * stop in block combine mode
+				 */
+				iface->readNum = 1;
+				iface->manual_stop = 1;
+				bfin_write_TWI_MASTER_CTL(
+					bfin_read_TWI_MASTER_CTL()
+					| (0xff << 6));
+			} else {
+				/* set the readd number in other
+				 * combine mode.
+				 */
+				bfin_write_TWI_MASTER_CTL(
+					(bfin_read_TWI_MASTER_CTL() &
+					(~(0xff << 6))) |
+					( iface->readNum << 6));
+			}
+			/* remove restart bit and enable master receive */
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() &
+				~RSTART);
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() |
+				MEN | MDIR);
+			SSYNC();
+		} else {
+			iface->result = 1;
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			complete(&iface->complete);
+		}
+	}
+}
+
+/* Interrupt handler */
+static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)dev_id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	del_timer(&iface->timeout_timer);
+	bfin_twi_handle_interrupt(iface);
+	spin_unlock_irqrestore(&iface->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static void bfin_twi_timeout(unsigned long data)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	bfin_twi_handle_interrupt(iface);
+	if (iface->result == 0) {
+		iface->timeout_count--;
+		if (iface->timeout_count > 0) {
+			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+			add_timer(&iface->timeout_timer);
+		} else {
+			iface->result = -1;
+			complete(&iface->complete);
+		}
+	}
+	spin_unlock_irqrestore(&iface->lock, flags);
+}
+
+/*
+ * Generic i2c master transfer entrypoint
+ */
+static int bfin_twi_master_xfer(struct i2c_adapter *adap,
+				struct i2c_msg msgs[], int num)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	struct i2c_msg *pmsg;
+	int i, ret;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		yield();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	ret = 0;
+	for (i = 0; rc >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+		if (pmsg->flags & I2C_M_TEN) {
+			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr \
+					not supported !\n");
+			rc = -EINVAL;
+			break;
+		}
+
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		iface->manual_stop = 0;
+		iface->transPtr = pmsg->buf;
+		iface->writeNum = iface->readNum = pmsg->len;
+		iface->result = 0;
+		iface->timeout_count = 10;
+		/* Set Transmit device address */
+		bfin_write_TWI_MASTER_ADDR(pmsg->addr);
+
+		/* FIFO Initiation. Data in FIFO should be 
+		 *  discarded before start a new operation.
+		 */
+		bfin_write_TWI_FIFO_CTL(0x3);
+		SSYNC();
+		bfin_write_TWI_FIFO_CTL(0);
+		SSYNC();
+
+		if (pmsg->flags & I2C_M_RD)
+			iface->read_write = I2C_SMBUS_READ;
+		else {
+			iface->read_write = I2C_SMBUS_WRITE;
+			/* Transmit first data */
+			if (iface->writeNum > 0) {
+				bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+				iface->writeNum--;
+				SSYNC();
+			}
+		}
+
+		/* clear int stat */
+		bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+		/* Interrupt mask . Enable XMT, RCV interrupt */
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ)?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (pmsg->len > 0 && pmsg->len <= 255)
+			bfin_write_TWI_MASTER_CTL(pmsg->len << 6);
+		else if (pmsg->len > 255) {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		} else
+			break;
+
+		iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+		add_timer(&iface->timeout_timer);
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | 
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		SSYNC();
+
+		wait_for_completion(&iface->complete);
+
+		rc = iface->result;
+		if (rc == 1)
+			ret++;
+		else if (rc == -1)
+			break;
+	}
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return ret;
+}
+
+/*
+ * SMBus type transfer entrypoint
+ */
+
+int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			unsigned short flags, char read_write,
+			u8 command, int size, union i2c_smbus_data * data)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		yield();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	iface->writeNum = 0;
+	iface->readNum = 0;
+
+	/* Prepare datas & select mode */
+	switch (size) {
+	case I2C_SMBUS_QUICK:
+		iface->transPtr = NULL;
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE:
+		if (data == NULL)
+			iface->transPtr = NULL;
+		else {
+			if (read_write == I2C_SMBUS_READ)
+				iface->readNum = 1;
+			else
+				iface->writeNum = 1;
+			iface->transPtr = &data->byte;
+		}
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = &data->byte;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		iface->writeNum = 2;
+		iface->readNum = 2;
+		iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 0;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = data->block[0]+1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = data->block;
+		break;
+	default:
+		return -1;
+	}
+
+	iface->result = 0;
+	iface->manual_stop = 0;
+	iface->read_write = read_write;
+	iface->command = command;
+	iface->timeout_count = 10;
+
+	/* FIFO Initiation. Data in FIFO should be discarded before
+	 * start a new operation.
+	 */
+	bfin_write_TWI_FIFO_CTL(0x3);
+	SSYNC();
+	bfin_write_TWI_FIFO_CTL(0);
+
+	/* clear int stat */
+	bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+	/* Set Transmit device address */
+	bfin_write_TWI_MASTER_ADDR(addr);
+	SSYNC();
+
+	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+	add_timer(&iface->timeout_timer);
+
+	switch (iface->cur_mode) {
+	case TWI_I2C_MODE_STANDARDSUB:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (iface->writeNum + 1 <= 255)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		}
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	case TWI_I2C_MODE_COMBINED:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
+		SSYNC();
+
+		if (iface->writeNum > 0)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else
+			bfin_write_TWI_MASTER_CTL(0x1 << 6);
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	default:
+		bfin_write_TWI_MASTER_CTL(0);
+		if (size != I2C_SMBUS_QUICK) {
+			/* Don't access xmit data register when this is a
+			 * read operation.
+			 */
+			if (iface->read_write != I2C_SMBUS_READ) {
+				if (iface->writeNum > 0) {
+					bfin_write_TWI_XMT_DATA8
+						(*(iface->transPtr++));
+					if (iface->writeNum <= 255)
+						bfin_write_TWI_MASTER_CTL
+							(iface->writeNum << 6);
+					else {
+						bfin_write_TWI_MASTER_CTL
+							(0xff << 6);
+						iface->manual_stop = 1;
+					}
+					iface->writeNum--;
+				} else {
+					bfin_write_TWI_XMT_DATA8
+						(iface->command);
+					bfin_write_TWI_MASTER_CTL(1 << 6);
+				}
+			} else {
+				if (iface->readNum > 0
+					&& iface->readNum <= 255)
+					bfin_write_TWI_MASTER_CTL
+						(iface->readNum << 6);
+				else if (iface->readNum > 255) {
+					bfin_write_TWI_MASTER_CTL( 0xff << 6 );
+					iface->manual_stop = 1;
+				} else {
+					del_timer(&iface->timeout_timer);
+					break;
+				}
+			}
+		}
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | 
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_TWICLK_KHZ > 100) ? FAST : 0));
+		break;
+	}
+	SSYNC();
+
+	wait_for_completion(&iface->complete);
+
+	rc = (iface->result >= 0) ? 0 : -1;
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return rc;
+}
+
+/*
+ * Return what the adapter supports
+ */
+static u32 bfin_twi_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
+	       I2C_FUNC_I2C;
+}
+
+
+static struct i2c_algorithm bfin_twi_algorithm = {
+	.master_xfer   = bfin_twi_master_xfer,
+	.smbus_xfer    = bfin_twi_smbus_xfer,
+	.functionality = bfin_twi_functionality,
+};
+
+static int __init i2c_bfin_twi_init(void)
+{
+	struct i2c_adapter *p_adap;
+	int rc;
+
+	mutex_init(&(twi_iface.twi_lock));
+	spin_lock_init(&twi_iface.lock);
+	init_completion(&twi_iface.complete);
+	twi_iface.irq = IRQ_TWI;
+
+	init_timer(&twi_iface.timeout_timer);
+	twi_iface.timeout_timer.function = bfin_twi_timeout;
+	twi_iface.timeout_timer.data = (unsigned long)&twi_iface;
+
+	p_adap = &twi_iface.adap;
+	p_adap->id = I2C_BFIN_TWI;
+	strcpy(p_adap->name, "i2c-bfin-twi");
+	p_adap->algo = &bfin_twi_algorithm;
+	p_adap->algo_data = &twi_iface;
+	p_adap->client_register = NULL;
+	p_adap->client_unregister = NULL;
+	p_adap->class = I2C_CLASS_ALL;
+
+	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry,
+		SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
+	if (rc) {
+		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n",
+			twi_iface.irq);
+		return -ENODEV;
+	}
+
+	/* Set TWI internal clock as 10MHz */
+	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
+
+	/* Set Twi interface clock as specified */
+	if (CONFIG_TWICLK_KHZ > 400)
+		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) |
+			(( 5*1024 / 400 ) & 0xFF));
+	else
+		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_TWICLK_KHZ ) << 8) |
+			(( 5*1024 / CONFIG_TWICLK_KHZ ) & 0xFF));
+
+	/* Enable TWI */
+	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
+	SSYNC();
+
+	rc = i2c_add_adapter(p_adap);
+	if(rc < 0)
+		free_irq(twi_iface.irq, &twi_iface);
+	return rc;	
+}
+
+static void __exit i2c_bfin_twi_exit(void)
+{
+	i2c_del_adapter(&twi_iface.adap);
+	free_irq(twi_iface.irq, &twi_iface);
+}
+
+MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin TWI");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_bfin_twi_init);
+module_exit(i2c_bfin_twi_exit);
_

Thanks a lot
-Bryan Wu

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  5:17   ` Sonic Zhang
@ 2007-03-07  6:06     ` Andrew Morton
  2007-03-07  6:38       ` Wu, Bryan
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-03-07  6:06 UTC (permalink / raw)
  To: Sonic Zhang; +Cc: bryan.wu, khali, linux-kernel, mhfan

On Wed, 7 Mar 2007 13:17:57 +0800 "Sonic Zhang" <sonic.adi@gmail.com> wrote:

> On 3/6/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Tue, 06 Mar 2007 14:54:18 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:
> >
> > > Hi folks,
> > >
> > > [PATCH] Blackfin: blackfin i2c driver
> > >
> 
> > > +     struct i2c_msg *pmsg;
> > > +     int i, ret;
> > > +     int rc = 0;
> > > +
> > > +     if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> > > +             return -ENXIO;
> > > +
> > > +     down(&iface->twi_lock);
> > > +
> > > +     while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> > > +             up(&iface->twi_lock);
> > > +             schedule();
> > > +             down(&iface->twi_lock);
> > > +     }
> >
> > That's a busy loop until this task's timeslice has expired.  It'll work,
> > but it'll suck a bit.  (Repeated in several places)
> >
> 
> OK, I change it into yield(). So, current process will be move to the
> tail of the run queue. Is that OK with you?

Nope, yield is terribly bad when there are busy processes running: it can
stall for a very long time indeed,

Is this hardware not capable of generating an interrupt when BUSBUSY gets
negated?

I guess not, in which case you're stuck with having to poll it - probably
use a cond_resched() in the loop, and an angry comment.


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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  6:06     ` Andrew Morton
@ 2007-03-07  6:38       ` Wu, Bryan
  0 siblings, 0 replies; 19+ messages in thread
From: Wu, Bryan @ 2007-03-07  6:38 UTC (permalink / raw)
  To: Andrew Morton, Alexey Dobriyan, Sonic Zhang, Mike Frysinger
  Cc: bryan.wu, khali, linux-kernel, mhfan

> > 
> > OK, I change it into yield(). So, current process will be move to the
> > tail of the run queue. Is that OK with you?
> 
> Nope, yield is terribly bad when there are busy processes running: it can
> stall for a very long time indeed,
> 
> Is this hardware not capable of generating an interrupt when BUSBUSY gets
> negated?
> 
> I guess not, in which case you're stuck with having to poll it - probably
> use a cond_resched() in the loop, and an angry comment.

Thanks, we fix it. please pick up this one.

[PATCH] Blackfin: blackfin i2c driver

The i2c linux driver for blackfin architecture which supports both GPIO
i2c operation and blackfin on-chip TWI controller i2c operation.

Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
---

 drivers/i2c/busses/Kconfig         |   47 ++++
 drivers/i2c/busses/i2c-bfin-gpio.c |   98 +++++++++
 drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 734 insertions(+)

Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
--- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 13:32:02.000000000 +0800
+++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 13:44:19.000000000 +0800
@@ -5,6 +5,53 @@
 menu "I2C Hardware Bus support"
 	depends on I2C
 
+config I2C_BFIN_GPIO
+	tristate "Generic Blackfin and HHBF533/561 development board I2C support"
+	depends on I2C && EXPERIMENTAL
+	select I2C_ALGOBIT
+	help
+	--
+
+menu "BFIN I2C SDA/SCL Selection"
+	depends on I2C_BFIN_GPIO
+config BFIN_SDA
+	int "SDA is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 2 if (BF533 || BF532 || BF531) 
+
+config BFIN_SCL
+	int "SCL is GPIO Number"
+	range 0 15 if (BF533 || BF532 || BF531) 
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 3 
+endmenu
+
+config I2C_BFIN_GPIO_CYCLE_DELAY
+	int "Cycle Delay in usec"
+	depends on I2C_BFIN_GPIO
+	range 1 100 
+	default 40
+
+config I2C_BFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This the TWI I2C device driver for Blackfin 534/536/537.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config TWICLK_KHZ
+	int "TWI clock (kHZ)"
+	depends on I2C_BFIN_TWI
+	default 50
+	help
+	  The unit of the TWI clock is kilo HZ. Please divide the clock 
+	  by 1024 if you count it in HZ. The value should be less than 400.
+
 config I2C_ALI1535
 	tristate "ALI 1535"
 	depends on I2C && PCI
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c	2007-03-07 13:44:19.000000000 +0800
@@ -0,0 +1,98 @@
+/****************************************************************
+ * Description:                                                 *
+ *                                                              *
+ * Maintainer: Meihui Fan <mhfan@ustc.edu>		        *
+ *                                                              *
+ * CopyRight (c)  2004  HHTech                                  *
+ *   www.hhcn.com, www.hhcn.org                                 *
+ *   All rights reserved.                                       *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ *                                                              *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
+#include <asm/blackfin.h>
+#include <asm/gpio.h>
+
+#define	I2C_HW_B_HHBF		    0x13
+
+static void hhbf_setsda(void *data, int state)
+{
+	if (state) {
+		gpio_direction_input(CONFIG_BFIN_SDA);
+
+	} else {
+		gpio_direction_output(CONFIG_BFIN_SDA);
+		gpio_set_value(CONFIG_BFIN_SDA, 0);
+	}
+}
+
+static void hhbf_setscl(void *data, int state)
+{
+	gpio_set_value(CONFIG_BFIN_SCL, state);
+}
+
+static int hhbf_getsda(void *data)
+{
+	return (gpio_get_value(CONFIG_BFIN_SDA) != 0);
+}
+
+
+static struct i2c_algo_bit_data bit_hhbf_data = {
+	.setsda  = hhbf_setsda,
+	.setscl  = hhbf_setscl,
+	.getsda  = hhbf_getsda,
+	.udelay  = CONFIG_I2C_BFIN_GPIO_CYCLE_DELAY,
+	.timeout = HZ
+};
+
+static struct i2c_adapter hhbf_ops = {
+	.owner 	= THIS_MODULE,
+	.id 	= I2C_HW_B_HHBF,
+	.algo_data 	= &bit_hhbf_data,
+	.name	= "HHBF I2C driver",
+};
+
+static int __init i2c_hhbf_init(void)
+{
+
+	if (gpio_request(CONFIG_BFIN_SCL, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__func__, CONFIG_BFIN_SCL);
+		return -1;
+	}
+
+	if (gpio_request(CONFIG_BFIN_SDA, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n",__func__, CONFIG_BFIN_SDA);
+		return -1;
+	}
+
+
+	gpio_direction_output(CONFIG_BFIN_SCL);
+	gpio_direction_input(CONFIG_BFIN_SDA);
+	gpio_set_value(CONFIG_BFIN_SCL, 1);
+
+	return i2c_bit_add_bus(&hhbf_ops);
+}
+
+static void __exit i2c_hhbf_exit(void)
+{
+	gpio_free(CONFIG_BFIN_SCL);
+	gpio_free(CONFIG_BFIN_SDA);
+	i2c_bit_del_bus(&hhbf_ops);
+}
+
+MODULE_AUTHOR("Meihui Fan <mhfan@ustc.edu>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin and HHBF Boards");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_hhbf_init);
+module_exit(i2c_hhbf_exit);
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c	2007-03-07 14:32:31.000000000 +0800
@@ -0,0 +1,589 @@
+/****************************************************************
+ * Description: Driver for Blackfin Two Wire Interface          *
+ * Maintainer:  sonicz  <sonic.zhang@analog.com>                *
+ *                                                              *
+ * Copyright (c) 2005-2007 Analog Device                        *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/mm.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+
+#include <asm/blackfin.h>
+#include <asm/irq.h>
+
+#define I2C_BFIN_TWI       0x00
+#define POLL_TIMEOUT       (2 * HZ)
+#ifndef CONFIG_TWICLK_KHZ
+#define CONFIG_TWICLK_KHZ  400
+#endif
+
+/* SMBus mode*/
+#define TWI_I2C_MODE_STANDARD		0x01
+#define TWI_I2C_MODE_STANDARDSUB	0x02
+#define TWI_I2C_MODE_COMBINED		0x04
+
+struct bfin_twi_iface
+{
+	struct mutex		twi_lock;
+	int			irq;
+	spinlock_t		lock;
+	char			read_write;
+	u8			command;
+	u8			*transPtr;
+	int			readNum;
+	int			writeNum;
+	int			cur_mode;
+	int			manual_stop;
+	int			result;
+	int			timeout_count;
+	struct timer_list	timeout_timer;
+	struct i2c_adapter	adap;
+	struct completion	complete;
+};
+
+static struct bfin_twi_iface twi_iface;
+
+static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
+{
+	unsigned short twi_int_status = bfin_read_TWI_INT_STAT();
+	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
+
+	if (twi_int_status & XMTSERV) {
+		/* Transmit next data */
+		if (iface->writeNum > 0) {
+			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+			iface->writeNum--;
+		}
+		/* start receive immediately after complete sending in
+		 * combine mode.
+		 */
+		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| MDIR | RSTART);
+		} else if (iface->manual_stop)
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+		SSYNC();
+		/* Clear status */
+		bfin_write_TWI_INT_STAT(XMTSERV);
+		SSYNC();
+	}
+	if (twi_int_status & RCVSERV) {
+		if (iface->readNum > 0) {
+			/* Receive next data */
+			*(iface->transPtr) = bfin_read_TWI_RCV_DATA8();
+			if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+				/* Change combine mode into sub mode after
+				 * read first data.
+				 */
+				iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+				/* Get read number from first byte in block
+				 * combine mode.
+				 */
+				if (iface->readNum == 1 && iface->manual_stop)
+					iface->readNum = *iface->transPtr+1;
+			}
+			iface->transPtr++;
+			iface->readNum--;
+		} else if (iface->manual_stop) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+			SSYNC();
+		}
+		/* Clear interrupt source */
+		bfin_write_TWI_INT_STAT(RCVSERV);
+		SSYNC();
+	}
+	if (twi_int_status & MERR) {
+		bfin_write_TWI_INT_STAT(MERR);
+		bfin_write_TWI_INT_MASK(0);
+		bfin_write_TWI_MASTER_STAT(0x3e);
+		bfin_write_TWI_MASTER_CTL(0);
+		SSYNC();
+		iface->result = -1;
+		/* if both err and complete int stats are set, return proper
+		 * results.
+		 */
+		if (twi_int_status & MCOMP) {
+			bfin_write_TWI_INT_STAT(MCOMP);
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			/* If it is a quick transfer, only address bug no data,
+			 * not an err, return 1.
+			 */
+			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
+				iface->result = 1;
+			/* If address not acknowledged return -1,
+			 * else return 0.
+			 */
+			else if (!(mast_stat & ANAK))
+				iface->result = 0;
+		}
+		complete(&iface->complete);
+		return;
+	}
+	if (twi_int_status & MCOMP) {
+		bfin_write_TWI_INT_STAT(MCOMP);
+		SSYNC();
+		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			if (iface->readNum == 0) {
+				/* set the read number to 1 and ask for manual
+				 * stop in block combine mode
+				 */
+				iface->readNum = 1;
+				iface->manual_stop = 1;
+				bfin_write_TWI_MASTER_CTL(
+					bfin_read_TWI_MASTER_CTL()
+					| (0xff << 6));
+			} else {
+				/* set the readd number in other
+				 * combine mode.
+				 */
+				bfin_write_TWI_MASTER_CTL(
+					(bfin_read_TWI_MASTER_CTL() &
+					(~(0xff << 6))) |
+					( iface->readNum << 6));
+			}
+			/* remove restart bit and enable master receive */
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() &
+				~RSTART);
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() |
+				MEN | MDIR);
+			SSYNC();
+		} else {
+			iface->result = 1;
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			complete(&iface->complete);
+		}
+	}
+}
+
+/* Interrupt handler */
+static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)dev_id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	del_timer(&iface->timeout_timer);
+	bfin_twi_handle_interrupt(iface);
+	spin_unlock_irqrestore(&iface->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static void bfin_twi_timeout(unsigned long data)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	bfin_twi_handle_interrupt(iface);
+	if (iface->result == 0) {
+		iface->timeout_count--;
+		if (iface->timeout_count > 0) {
+			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+			add_timer(&iface->timeout_timer);
+		} else {
+			iface->result = -1;
+			complete(&iface->complete);
+		}
+	}
+	spin_unlock_irqrestore(&iface->lock, flags);
+}
+
+/*
+ * Generic i2c master transfer entrypoint
+ */
+static int bfin_twi_master_xfer(struct i2c_adapter *adap,
+				struct i2c_msg msgs[], int num)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	struct i2c_msg *pmsg;
+	int i, ret;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		cond_resched();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	ret = 0;
+	for (i = 0; rc >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+		if (pmsg->flags & I2C_M_TEN) {
+			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr \
+					not supported !\n");
+			rc = -EINVAL;
+			break;
+		}
+
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		iface->manual_stop = 0;
+		iface->transPtr = pmsg->buf;
+		iface->writeNum = iface->readNum = pmsg->len;
+		iface->result = 0;
+		iface->timeout_count = 10;
+		/* Set Transmit device address */
+		bfin_write_TWI_MASTER_ADDR(pmsg->addr);
+
+		/* FIFO Initiation. Data in FIFO should be 
+		 *  discarded before start a new operation.
+		 */
+		bfin_write_TWI_FIFO_CTL(0x3);
+		SSYNC();
+		bfin_write_TWI_FIFO_CTL(0);
+		SSYNC();
+
+		if (pmsg->flags & I2C_M_RD)
+			iface->read_write = I2C_SMBUS_READ;
+		else {
+			iface->read_write = I2C_SMBUS_WRITE;
+			/* Transmit first data */
+			if (iface->writeNum > 0) {
+				bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+				iface->writeNum--;
+				SSYNC();
+			}
+		}
+
+		/* clear int stat */
+		bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+		/* Interrupt mask . Enable XMT, RCV interrupt */
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ)?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (pmsg->len > 0 && pmsg->len <= 255)
+			bfin_write_TWI_MASTER_CTL(pmsg->len << 6);
+		else if (pmsg->len > 255) {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		} else
+			break;
+
+		iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+		add_timer(&iface->timeout_timer);
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | 
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		SSYNC();
+
+		wait_for_completion(&iface->complete);
+
+		rc = iface->result;
+		if (rc == 1)
+			ret++;
+		else if (rc == -1)
+			break;
+	}
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return ret;
+}
+
+/*
+ * SMBus type transfer entrypoint
+ */
+
+int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			unsigned short flags, char read_write,
+			u8 command, int size, union i2c_smbus_data * data)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		cond_resched();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	iface->writeNum = 0;
+	iface->readNum = 0;
+
+	/* Prepare datas & select mode */
+	switch (size) {
+	case I2C_SMBUS_QUICK:
+		iface->transPtr = NULL;
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE:
+		if (data == NULL)
+			iface->transPtr = NULL;
+		else {
+			if (read_write == I2C_SMBUS_READ)
+				iface->readNum = 1;
+			else
+				iface->writeNum = 1;
+			iface->transPtr = &data->byte;
+		}
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = &data->byte;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		iface->writeNum = 2;
+		iface->readNum = 2;
+		iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 0;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = data->block[0]+1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = data->block;
+		break;
+	default:
+		return -1;
+	}
+
+	iface->result = 0;
+	iface->manual_stop = 0;
+	iface->read_write = read_write;
+	iface->command = command;
+	iface->timeout_count = 10;
+
+	/* FIFO Initiation. Data in FIFO should be discarded before
+	 * start a new operation.
+	 */
+	bfin_write_TWI_FIFO_CTL(0x3);
+	SSYNC();
+	bfin_write_TWI_FIFO_CTL(0);
+
+	/* clear int stat */
+	bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+	/* Set Transmit device address */
+	bfin_write_TWI_MASTER_ADDR(addr);
+	SSYNC();
+
+	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+	add_timer(&iface->timeout_timer);
+
+	switch (iface->cur_mode) {
+	case TWI_I2C_MODE_STANDARDSUB:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (iface->writeNum + 1 <= 255)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		}
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	case TWI_I2C_MODE_COMBINED:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
+		SSYNC();
+
+		if (iface->writeNum > 0)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else
+			bfin_write_TWI_MASTER_CTL(0x1 << 6);
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_TWICLK_KHZ>100) ? FAST : 0));
+		break;
+	default:
+		bfin_write_TWI_MASTER_CTL(0);
+		if (size != I2C_SMBUS_QUICK) {
+			/* Don't access xmit data register when this is a
+			 * read operation.
+			 */
+			if (iface->read_write != I2C_SMBUS_READ) {
+				if (iface->writeNum > 0) {
+					bfin_write_TWI_XMT_DATA8
+						(*(iface->transPtr++));
+					if (iface->writeNum <= 255)
+						bfin_write_TWI_MASTER_CTL
+							(iface->writeNum << 6);
+					else {
+						bfin_write_TWI_MASTER_CTL
+							(0xff << 6);
+						iface->manual_stop = 1;
+					}
+					iface->writeNum--;
+				} else {
+					bfin_write_TWI_XMT_DATA8
+						(iface->command);
+					bfin_write_TWI_MASTER_CTL(1 << 6);
+				}
+			} else {
+				if (iface->readNum > 0
+					&& iface->readNum <= 255)
+					bfin_write_TWI_MASTER_CTL
+						(iface->readNum << 6);
+				else if (iface->readNum > 255) {
+					bfin_write_TWI_MASTER_CTL( 0xff << 6 );
+					iface->manual_stop = 1;
+				} else {
+					del_timer(&iface->timeout_timer);
+					break;
+				}
+			}
+		}
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN | 
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_TWICLK_KHZ > 100) ? FAST : 0));
+		break;
+	}
+	SSYNC();
+
+	wait_for_completion(&iface->complete);
+
+	rc = (iface->result >= 0) ? 0 : -1;
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return rc;
+}
+
+/*
+ * Return what the adapter supports
+ */
+static u32 bfin_twi_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
+	       I2C_FUNC_I2C;
+}
+
+
+static struct i2c_algorithm bfin_twi_algorithm = {
+	.master_xfer   = bfin_twi_master_xfer,
+	.smbus_xfer    = bfin_twi_smbus_xfer,
+	.functionality = bfin_twi_functionality,
+};
+
+static int __init i2c_bfin_twi_init(void)
+{
+	struct i2c_adapter *p_adap;
+	int rc;
+
+	mutex_init(&(twi_iface.twi_lock));
+	spin_lock_init(&twi_iface.lock);
+	init_completion(&twi_iface.complete);
+	twi_iface.irq = IRQ_TWI;
+
+	init_timer(&twi_iface.timeout_timer);
+	twi_iface.timeout_timer.function = bfin_twi_timeout;
+	twi_iface.timeout_timer.data = (unsigned long)&twi_iface;
+
+	p_adap = &twi_iface.adap;
+	p_adap->id = I2C_BFIN_TWI;
+	strcpy(p_adap->name, "i2c-bfin-twi");
+	p_adap->algo = &bfin_twi_algorithm;
+	p_adap->algo_data = &twi_iface;
+	p_adap->client_register = NULL;
+	p_adap->client_unregister = NULL;
+	p_adap->class = I2C_CLASS_ALL;
+
+	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry,
+		SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
+	if (rc) {
+		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n",
+			twi_iface.irq);
+		return -ENODEV;
+	}
+
+	/* Set TWI internal clock as 10MHz */
+	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
+
+	/* Set Twi interface clock as specified */
+	if (CONFIG_TWICLK_KHZ > 400)
+		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) |
+			(( 5*1024 / 400 ) & 0xFF));
+	else
+		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_TWICLK_KHZ ) << 8) |
+			(( 5*1024 / CONFIG_TWICLK_KHZ ) & 0xFF));
+
+	/* Enable TWI */
+	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
+	SSYNC();
+
+	rc = i2c_add_adapter(p_adap);
+	if(rc < 0)
+		free_irq(twi_iface.irq, &twi_iface);
+	return rc;	
+}
+
+static void __exit i2c_bfin_twi_exit(void)
+{
+	i2c_del_adapter(&twi_iface.adap);
+	free_irq(twi_iface.irq, &twi_iface);
+}
+
+MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin TWI");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_bfin_twi_init);
+module_exit(i2c_bfin_twi_exit);
_

Best Regards,
-Bryan Wu

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  5:57   ` Wu, Bryan
@ 2007-03-07  6:58     ` Jean Delvare
  2007-03-07  7:14       ` Andrew Morton
  2007-03-07  9:53       ` Wu, Bryan
  2007-03-07  7:02     ` Andrew Morton
  1 sibling, 2 replies; 19+ messages in thread
From: Jean Delvare @ 2007-03-07  6:58 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Alexey Dobriyan, Andrew Morton, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

Hi Bryan,

On Wed, 07 Mar 2007 13:57:58 +0800, Wu, Bryan wrote:
> Here is the updated blackfin i2c driver.
> 
> [PATCH] Blackfin: blackfin i2c driver
> 
> The i2c linux driver for blackfin architecture which supports both GPIO
> i2c operation and blackfin on-chip TWI controller i2c operation.
> 
> Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
> ---
>  drivers/i2c/busses/Kconfig         |   47 ++++
>  drivers/i2c/busses/i2c-bfin-gpio.c |   98 +++++++++
>  drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

I'd prefer i2c-blackfin-gpio and i2c-blackfin-twi. Abreviations tend to
confuse newcomers.

>  3 files changed, 734 insertions(+)
> 
> Index: linux-2.6/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 13:32:02.000000000 +0800
> +++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 13:44:19.000000000 +0800
> @@ -5,6 +5,53 @@
>  menu "I2C Hardware Bus support"
>  	depends on I2C
>  
> +config I2C_BFIN_GPIO

I2C_BLACKFIN_GPIO

Please move the entries to the right location. The list is sorted
alphabetically if you didn't notice.

> +	tristate "Generic Blackfin and HHBF533/561 development board I2C support"

You can drop the trailing "I2C support", the user is in a menu named
"I2C hardware bus support" so it's pretty clear what we're talking
about.

> +	depends on I2C && EXPERIMENTAL
> +	select I2C_ALGOBIT
> +	help
> +	--
> +
> +menu "BFIN I2C SDA/SCL Selection"
> +	depends on I2C_BFIN_GPIO
> +config BFIN_SDA

I2C_BLACKFIN_SDA

> +	int "SDA is GPIO Number"

"SDA GPIO pin number"

> +	range 0 15 if (BF533 || BF532 || BF531) 

Trailing whitespace.

> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 2 if (BF533 || BF532 || BF531) 

Trailing whitespace.

No default for the other cases?

> +
> +config BFIN_SCL

I2C_BLACKFIN_SCL
Etc etc, all the options should start with I2C_BLACKFIN.

> +	int "SCL is GPIO Number"

"SCL GPIO pin number"

> +	range 0 15 if (BF533 || BF532 || BF531) 

Trailing whitespace, and many more after that. Please fix them all!

> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 3 
> +endmenu
> +
> +config I2C_BFIN_GPIO_CYCLE_DELAY
> +	int "Cycle Delay in usec"
> +	depends on I2C_BFIN_GPIO
> +	range 1 100 
> +	default 40

This should really not be a kernel configuration option. Please turn it
into a kernel module parameter or a sysfs attribute if you really need
it. Also note that we already have an interface to change this
value from user-space (using an ioctl on /dev/i2c-N) and that might be
sufficient for your needs.

And allowing 1 usec delay is probably not a good idea, I don't
recommend values below 6 usec with i2c-algo-bit.

> +
> +config I2C_BFIN_TWI
> +	tristate "Blackfin TWI I2C support"
> +	depends on I2C && (BF534 || BF536 || BF537)
> +	help
> +	  This the TWI I2C device driver for Blackfin 534/536/537.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-bfin-twi.
> +
> +config TWICLK_KHZ
> +	int "TWI clock (kHZ)"

kHz

> +	depends on I2C_BFIN_TWI
> +	default 50
> +	help
> +	  The unit of the TWI clock is kilo HZ. Please divide the clock 
> +	  by 1024 if you count it in HZ. The value should be less than 400.

Why don't you use "range" here too to ensure that the value is actually
less than 400? Either way, same as above, IMHO this should not be a
compilation-time decision.

A kHz is really 1000 Hz, not 1024. And everybody skilled enough to
configure a kernel should know that, I doubt it's worth reminding.

> +
>  config I2C_ALI1535
>  	tristate "ALI 1535"
>  	depends on I2C && PCI

All these options won't work really well until you also change
drivers/i2c/busses/Makefile to make something useful with them...

-- 
Jean Delvare

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  5:57   ` Wu, Bryan
  2007-03-07  6:58     ` Jean Delvare
@ 2007-03-07  7:02     ` Andrew Morton
  1 sibling, 0 replies; 19+ messages in thread
From: Andrew Morton @ 2007-03-07  7:02 UTC (permalink / raw)
  To: bryan.wu
  Cc: Alexey Dobriyan, Sonic Zhang, Mike Frysinger, khali, linux-kernel,
	mhfan, Roman Zippel

On Wed, 07 Mar 2007 13:57:58 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:

> Here is the updated blackfin i2c driver.
> 
> [PATCH] Blackfin: blackfin i2c driver
> 
> The i2c linux driver for blackfin architecture which supports both GPIO
> i2c operation and blackfin on-chip TWI controller i2c operation.
> 
> Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
> ---
>  drivers/i2c/busses/Kconfig         |   47 ++++
>  drivers/i2c/busses/i2c-bfin-gpio.c |   98 +++++++++
>  drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 734 insertions(+)
> 
> Index: linux-2.6/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 13:32:02.000000000 +0800
> +++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 13:44:19.000000000 +0800
> @@ -5,6 +5,53 @@
>  menu "I2C Hardware Bus support"
>  	depends on I2C
>  
> +config I2C_BFIN_GPIO
> +	tristate "Generic Blackfin and HHBF533/561 development board I2C support"
> +	depends on I2C && EXPERIMENTAL
> +	select I2C_ALGOBIT
> +	help
> +	--
> +
> +menu "BFIN I2C SDA/SCL Selection"
> +	depends on I2C_BFIN_GPIO
> +config BFIN_SDA
> +	int "SDA is GPIO Number"
> +	range 0 15 if (BF533 || BF532 || BF531) 
> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 2 if (BF533 || BF532 || BF531) 
> +
> +config BFIN_SCL
> +	int "SCL is GPIO Number"
> +	range 0 15 if (BF533 || BF532 || BF531) 
> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 3 
> +endmenu
> +
> +config I2C_BFIN_GPIO_CYCLE_DELAY
> +	int "Cycle Delay in usec"
> +	depends on I2C_BFIN_GPIO
> +	range 1 100 
> +	default 40
> +
> +config I2C_BFIN_TWI
> +	tristate "Blackfin TWI I2C support"
> +	depends on I2C && (BF534 || BF536 || BF537)
> +	help
> +	  This the TWI I2C device driver for Blackfin 534/536/537.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-bfin-twi.
> +
> +config TWICLK_KHZ
> +	int "TWI clock (kHZ)"
> +	depends on I2C_BFIN_TWI
> +	default 50
> +	help
> +	  The unit of the TWI clock is kilo HZ. Please divide the clock 
> +	  by 1024 if you count it in HZ. The value should be less than 400.
> +

Well that's cute.  This patch causes an i386 `make allmodconfig' to spew
these:

    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 
    SDA is GPIO Number (BFIN_SDA) [] (NEW) 

out at about 1,000,000/sec, infinitely.

I'll put a `depends on BFIN' in there to shut it up, but I think you've
tickled a Kconfig bug.


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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  6:58     ` Jean Delvare
@ 2007-03-07  7:14       ` Andrew Morton
  2007-03-07  7:39         ` Wu, Bryan
  2007-03-07  9:53       ` Wu, Bryan
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-03-07  7:14 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Bryan Wu, Alexey Dobriyan, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

On Wed, 7 Mar 2007 07:58:22 +0100 Jean Delvare <khali@linux-fr.org> wrote:

> > +config BFIN_SDA
> 
> I2C_BLACKFIN_SDA

The blackfin architecture uses "bfin" pretty much universally, so this
usage is consistent.

box:/usr/src/25> grep -i blackfin patches/blackfin*|wc -l
   1608
box:/usr/src/25> grep -i bfin patches/blackfin*|wc -l
   6198

Let's just hope nobody makes a bluefin.

> > +	range 0 15 if (BF533 || BF532 || BF531) 
> 
> Trailing whitespace.

I always remove that when merging a patch.

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  7:14       ` Andrew Morton
@ 2007-03-07  7:39         ` Wu, Bryan
  2007-03-07  7:45           ` Andrew Morton
  0 siblings, 1 reply; 19+ messages in thread
From: Wu, Bryan @ 2007-03-07  7:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jean Delvare, Bryan Wu, Alexey Dobriyan, Sonic Zhang,
	Mike Frysinger, linux-kernel, mhfan

On Tue, 2007-03-06 at 23:14 -0800, Andrew Morton wrote:
> On Wed, 7 Mar 2007 07:58:22 +0100 Jean Delvare <khali@linux-fr.org> wrote:
> 
> > > +config BFIN_SDA
> > 
> > I2C_BLACKFIN_SDA
> 
> The blackfin architecture uses "bfin" pretty much universally, so this
> usage is consistent.
> 
> box:/usr/src/25> grep -i blackfin patches/blackfin*|wc -l
>    1608
> box:/usr/src/25> grep -i bfin patches/blackfin*|wc -l
>    6198
> 

Thanks for you understanding, but now we want to move to use
CONFIG_BLACKFIN options. There is a new task in our development plan to
change things to CONFIG_BLACKFIN.

At this moment, we both provide CONFIG_BFIN and CONFIG_BLACKFIN. When
all the things relied on CONFIG_BFIN/bfin are changed to
CONFIG_BLACKFIN/blackfin, the CONFIG_BFIN will be removed.

So here I will follow Jean's comments.

> Let's just hope nobody makes a bluefin.

So it is ok  for both blackfin and bluefin. But I think Black is cooler
than Blue. -:)

> 
> > > +	range 0 15 if (BF533 || BF532 || BF531) 
> > 
> > Trailing whitespace.
> 
> I always remove that when merging a patch.

Thanks a lot, could you please give me a script just to kill this
whitespace? So I can do it before sending you patches.

Thanks Jean and Andrew.
-Bryan

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  7:39         ` Wu, Bryan
@ 2007-03-07  7:45           ` Andrew Morton
  2007-03-08  3:57             ` trailing whitespace killing (Re: [PATCH -mm] Blackfin: blackfin i2c driver) Oleg Verych
  2007-03-08  7:35             ` [PATCH -mm] Blackfin: blackfin i2c driver Jean Delvare
  0 siblings, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2007-03-07  7:45 UTC (permalink / raw)
  To: bryan.wu
  Cc: Jean Delvare, Alexey Dobriyan, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

On Wed, 07 Mar 2007 15:39:27 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:

> Thanks a lot, could you please give me a script just to kill this
> whitespace? So I can do it before sending you patches.


Is pretty simple:

#!/bin/sh
#
# Strip any trailing whitespace which a unified diff adds.
#

strip1()
{
	TMP=$(mktemp /tmp/XXXXXX)
	cp $1 $TMP
	sed -e '/^+/s/[ 	]*$//' < $TMP > $1
	rm $TMP
}

for i in $*
do
	strip1 $i
done


that'll be in
http://www.zip.com.au/~akpm/linux/patches/patch-scripts-0.20/patch-scripts-0.20.tar.gz
too

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  6:58     ` Jean Delvare
  2007-03-07  7:14       ` Andrew Morton
@ 2007-03-07  9:53       ` Wu, Bryan
  2007-03-08  9:27         ` Jean Delvare
  1 sibling, 1 reply; 19+ messages in thread
From: Wu, Bryan @ 2007-03-07  9:53 UTC (permalink / raw)
  To: Jean Delvare, Andrew Morton, Alexey Dobriyan
  Cc: Bryan Wu, Sonic Zhang, Mike Frysinger, linux-kernel, mhfan


> > Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
> > ---
> >  drivers/i2c/busses/Kconfig         |   47 ++++
> >  drivers/i2c/busses/i2c-bfin-gpio.c |   98 +++++++++
> >  drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> I'd prefer i2c-blackfin-gpio and i2c-blackfin-twi. Abreviations tend to
> confuse newcomers.
> 

There are tons of code using bfin abreviations for
functions/files/variables names. So we prefer to follow the name scheme.

> >  3 files changed, 734 insertions(+)
> > 
> > Index: linux-2.6/drivers/i2c/busses/Kconfig
> > ===================================================================
> > --- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 13:32:02.000000000 +0800
> > +++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 13:44:19.000000000 +0800
> > @@ -5,6 +5,53 @@
> >  menu "I2C Hardware Bus support"
> >  	depends on I2C
> >  
> > +config I2C_BFIN_GPIO
> 
> I2C_BLACKFIN_GPIO
> 

In the Kconfig, we are trying to using CONFIG_BLACKFIN. So, I changed
all the BFIN to BLACKFIN as you mentioned.

> Please move the entries to the right location. The list is sorted
> alphabetically if you didn't notice.
> 
> > +	tristate "Generic Blackfin and HHBF533/561 development board I2C support"
> 
> You can drop the trailing "I2C support", the user is in a menu named
> "I2C hardware bus support" so it's pretty clear what we're talking
> about.
> 
> > +	depends on I2C && EXPERIMENTAL
> > +	select I2C_ALGOBIT
> > +	help
> > +	--
> > +
> > +menu "BFIN I2C SDA/SCL Selection"
> > +	depends on I2C_BFIN_GPIO
> > +config BFIN_SDA
> 
> I2C_BLACKFIN_SDA
> 
> > +	int "SDA is GPIO Number"
> 
> "SDA GPIO pin number"
> 
> > +	range 0 15 if (BF533 || BF532 || BF531) 
> 
> Trailing whitespace.
> 
> > +	range 0 47 if (BF534 || BF536 || BF537)
> > +	range 0 47 if BF561
> > +	default 2 if (BF533 || BF532 || BF531) 
> 
> Trailing whitespace.
> 
> No default for the other cases?
> 
> > +
> > +config BFIN_SCL
> 
> I2C_BLACKFIN_SCL
> Etc etc, all the options should start with I2C_BLACKFIN.
> 
> > +	int "SCL is GPIO Number"
> 
> "SCL GPIO pin number"
> 
> > +	range 0 15 if (BF533 || BF532 || BF531) 
> 
> Trailing whitespace, and many more after that. Please fix them all!
> 

All above fixed.

> > +	range 0 47 if (BF534 || BF536 || BF537)
> > +	range 0 47 if BF561
> > +	default 3 
> > +endmenu
> > +
> > +config I2C_BFIN_GPIO_CYCLE_DELAY
> > +	int "Cycle Delay in usec"
> > +	depends on I2C_BFIN_GPIO
> > +	range 1 100 
> > +	default 40
> 
> This should really not be a kernel configuration option. Please turn it
> into a kernel module parameter or a sysfs attribute if you really need
> it. Also note that we already have an interface to change this
> value from user-space (using an ioctl on /dev/i2c-N) and that might be
> sufficient for your needs.
> 

Actually, for some customer's requirement, they just want to set the
configuration in kernel config time not in the runtime. 


> And allowing 1 usec delay is probably not a good idea, I don't
> recommend values below 6 usec with i2c-algo-bit.

I add a range here from 5 to 100, default is 40.

> > +
> > +config I2C_BFIN_TWI
> > +	tristate "Blackfin TWI I2C support"
> > +	depends on I2C && (BF534 || BF536 || BF537)
> > +	help
> > +	  This the TWI I2C device driver for Blackfin 534/536/537.
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called i2c-bfin-twi.
> > +
> > +config TWICLK_KHZ
> > +	int "TWI clock (kHZ)"
> 
> kHz
> 
> > +	depends on I2C_BFIN_TWI
> > +	default 50
> > +	help
> > +	  The unit of the TWI clock is kilo HZ. Please divide the clock 
> > +	  by 1024 if you count it in HZ. The value should be less than 400.
> 
> Why don't you use "range" here too to ensure that the value is actually
> less than 400? Either way, same as above, IMHO this should not be a
> compilation-time decision.
> 
> A kHz is really 1000 Hz, not 1024. And everybody skilled enough to
> configure a kernel should know that, I doubt it's worth reminding.
> 

All above fixed.

> > +
> >  config I2C_ALI1535
> >  	tristate "ALI 1535"
> >  	depends on I2C && PCI
> 
> All these options won't work really well until you also change
> drivers/i2c/busses/Makefile to make something useful with them...
> 

Sorry for missing the Makefile in this patch. 

Thanks for your review and this is the latest one.

Andrew, I also fixed Kconfig bug in your
blackfin-blackfin-i2c-driver-fix.patch
Please add this one to -mm tree and remove old
one/update.patch/fix.patch from -mm tree.

[PATCH] Blackfin: blackfin i2c driver

The i2c linux driver for blackfin architecture which supports both GPIO
i2c operation and blackfin on-chip TWI controller i2c operation.

Signed-off-by: Bryan Wu <bryan.wu@analog.com>
Reviewed-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Alexey Dobriyan <adobriyan@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
---

 drivers/i2c/busses/Kconfig         |   47 ++++
 drivers/i2c/busses/Makefile        |    2
 drivers/i2c/busses/i2c-bfin-gpio.c |  100 +++++++++
 drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 738 insertions(+)

Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
--- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 17:18:49.000000000 +0800
+++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 17:23:05.000000000 +0800
@@ -91,6 +91,53 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-au1550.
 
+config I2C_BLACKFIN_GPIO
+	tristate "Blackfin GPIO based I2C interface"
+	depends on I2C && BLACKFIN && EXPERIMENTAL
+	select I2C_ALGOBIT
+	help
+	  Say Y here if you have an Blackfin BF5xx based
+	  system and are using GPIO lines for an I2C bus.
+
+menu "Blackfin I2C SDA/SCL Selection"
+	depends on I2C_BLACKFIN_GPIO
+config I2C_BLACKFIN_GPIO_SDA
+	int "SDA GPIO pin number"
+	range 0 15 if (BF533 || BF532 || BF531)
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 2
+
+config I2C_BLACKFIN_GPIO_SCL
+	int "SCL GPIO pin number"
+	range 0 15 if (BF533 || BF532 || BF531)
+	range 0 47 if (BF534 || BF536 || BF537)
+	range 0 47 if BF561
+	default 3
+endmenu
+
+config I2C_BLACKFIN_GPIO_CYCLE_DELAY
+	int "Cycle Delay in uSec"
+	depends on I2C_BLACKFIN_GPIO
+	range 5 100
+	default 40
+
+config I2C_BLACKFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This is the TWI I2C device driver for Blackfin 534/536/537.
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config I2C_BLACKFIN_TWI_CLK_KHZ
+	int "Blackfin TWI I2C clock (kHz)"
+	depends on I2C_BLACKFIN_TWI
+	range 10 400
+	default 50
+	help
+	  The unit of the TWI clock is kilo HZ.
+
 config I2C_ELEKTOR
 	tristate "Elektor ISA card"
 	depends on I2C && ISA && BROKEN_ON_SMP
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c	2007-03-07 17:32:30.000000000 +0800
@@ -0,0 +1,100 @@
+/****************************************************************
+ * Description:                                                 *
+ *                                                              *
+ * Maintainer: Meihui Fan <mhfan@ustc.edu>		        *
+ *                                                              *
+ * CopyRight (c)  2004  HHTech                                  *
+ *   www.hhcn.com, www.hhcn.org                                 *
+ *   All rights reserved.                                       *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ *                                                              *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
+#include <asm/blackfin.h>
+#include <asm/gpio.h>
+
+#define	I2C_HW_B_HHBF		    0x13
+
+static void hhbf_setsda(void *data, int state)
+{
+	if (state) {
+		gpio_direction_input(CONFIG_I2C_BLACKFIN_GPIO_SDA);
+
+	} else {
+		gpio_direction_output(CONFIG_I2C_BLACKFIN_GPIO_SDA);
+		gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SDA, 0);
+	}
+}
+
+static void hhbf_setscl(void *data, int state)
+{
+	gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SCL, state);
+}
+
+static int hhbf_getsda(void *data)
+{
+	return (gpio_get_value(CONFIG_I2C_BLACKFIN_GPIO_SDA) != 0);
+}
+
+
+static struct i2c_algo_bit_data bit_hhbf_data = {
+	.setsda  = hhbf_setsda,
+	.setscl  = hhbf_setscl,
+	.getsda  = hhbf_getsda,
+	.udelay  = CONFIG_I2C_BLACKFIN_GPIO_CYCLE_DELAY,
+	.timeout = HZ
+};
+
+static struct i2c_adapter hhbf_ops = {
+	.owner 	= THIS_MODULE,
+	.id 	= I2C_HW_B_HHBF,
+	.algo_data 	= &bit_hhbf_data,
+	.name	= "Blackfin GPIO based I2C driver",
+};
+
+static int __init i2c_hhbf_init(void)
+{
+
+	if (gpio_request(CONFIG_I2C_BLACKFIN_GPIO_SCL, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n", __func__,
+				CONFIG_I2C_BLACKFIN_GPIO_SCL);
+		return -1;
+	}
+
+	if (gpio_request(CONFIG_I2C_BLACKFIN_GPIO_SDA, NULL)) {
+		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n", __func__,
+				CONFIG_I2C_BLACKFIN_GPIO_SDA);
+		return -1;
+	}
+
+
+	gpio_direction_output(CONFIG_I2C_BLACKFIN_GPIO_SCL);
+	gpio_direction_input(CONFIG_I2C_BLACKFIN_GPIO_SDA);
+	gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SCL, 1);
+
+	return i2c_bit_add_bus(&hhbf_ops);
+}
+
+static void __exit i2c_hhbf_exit(void)
+{
+	gpio_free(CONFIG_I2C_BLACKFIN_GPIO_SCL);
+	gpio_free(CONFIG_I2C_BLACKFIN_GPIO_SDA);
+	i2c_bit_del_bus(&hhbf_ops);
+}
+
+MODULE_AUTHOR("Meihui Fan <mhfan@ustc.edu>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin and HHBF Boards");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_hhbf_init);
+module_exit(i2c_hhbf_exit);
Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c	2007-03-07 17:28:57.000000000 +0800
@@ -0,0 +1,589 @@
+/****************************************************************
+ * Description: Driver for Blackfin Two Wire Interface          *
+ * Maintainer:  sonicz  <sonic.zhang@analog.com>                *
+ *                                                              *
+ * Copyright (c) 2005-2007 Analog Device                        *
+ *                                                              *
+ * This file is free software;                                  *
+ *   you are free to modify and/or redistribute it   	        *
+ *   under the terms of the GNU General Public Licence (GPL).   *
+ ****************************************************************/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/mm.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+
+#include <asm/blackfin.h>
+#include <asm/irq.h>
+
+#define I2C_BLACKFIN_TWI       0x00
+#define POLL_TIMEOUT       (2 * HZ)
+#ifndef CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ
+#define CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ  400
+#endif
+
+/* SMBus mode*/
+#define TWI_I2C_MODE_STANDARD		0x01
+#define TWI_I2C_MODE_STANDARDSUB	0x02
+#define TWI_I2C_MODE_COMBINED		0x04
+
+struct bfin_twi_iface
+{
+	struct mutex		twi_lock;
+	int			irq;
+	spinlock_t		lock;
+	char			read_write;
+	u8			command;
+	u8			*transPtr;
+	int			readNum;
+	int			writeNum;
+	int			cur_mode;
+	int			manual_stop;
+	int			result;
+	int			timeout_count;
+	struct timer_list	timeout_timer;
+	struct i2c_adapter	adap;
+	struct completion	complete;
+};
+
+static struct bfin_twi_iface twi_iface;
+
+static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
+{
+	unsigned short twi_int_status = bfin_read_TWI_INT_STAT();
+	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
+
+	if (twi_int_status & XMTSERV) {
+		/* Transmit next data */
+		if (iface->writeNum > 0) {
+			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+			iface->writeNum--;
+		}
+		/* start receive immediately after complete sending in
+		 * combine mode.
+		 */
+		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| MDIR | RSTART);
+		} else if (iface->manual_stop)
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+		SSYNC();
+		/* Clear status */
+		bfin_write_TWI_INT_STAT(XMTSERV);
+		SSYNC();
+	}
+	if (twi_int_status & RCVSERV) {
+		if (iface->readNum > 0) {
+			/* Receive next data */
+			*(iface->transPtr) = bfin_read_TWI_RCV_DATA8();
+			if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+				/* Change combine mode into sub mode after
+				 * read first data.
+				 */
+				iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+				/* Get read number from first byte in block
+				 * combine mode.
+				 */
+				if (iface->readNum == 1 && iface->manual_stop)
+					iface->readNum = *iface->transPtr+1;
+			}
+			iface->transPtr++;
+			iface->readNum--;
+		} else if (iface->manual_stop) {
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
+				| STOP);
+			SSYNC();
+		}
+		/* Clear interrupt source */
+		bfin_write_TWI_INT_STAT(RCVSERV);
+		SSYNC();
+	}
+	if (twi_int_status & MERR) {
+		bfin_write_TWI_INT_STAT(MERR);
+		bfin_write_TWI_INT_MASK(0);
+		bfin_write_TWI_MASTER_STAT(0x3e);
+		bfin_write_TWI_MASTER_CTL(0);
+		SSYNC();
+		iface->result = -1;
+		/* if both err and complete int stats are set, return proper
+		 * results.
+		 */
+		if (twi_int_status & MCOMP) {
+			bfin_write_TWI_INT_STAT(MCOMP);
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			/* If it is a quick transfer, only address bug no data,
+			 * not an err, return 1.
+			 */
+			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
+				iface->result = 1;
+			/* If address not acknowledged return -1,
+			 * else return 0.
+			 */
+			else if (!(mast_stat & ANAK))
+				iface->result = 0;
+		}
+		complete(&iface->complete);
+		return;
+	}
+	if (twi_int_status & MCOMP) {
+		bfin_write_TWI_INT_STAT(MCOMP);
+		SSYNC();
+		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
+			if (iface->readNum == 0) {
+				/* set the read number to 1 and ask for manual
+				 * stop in block combine mode
+				 */
+				iface->readNum = 1;
+				iface->manual_stop = 1;
+				bfin_write_TWI_MASTER_CTL(
+					bfin_read_TWI_MASTER_CTL()
+					| (0xff << 6));
+			} else {
+				/* set the readd number in other
+				 * combine mode.
+				 */
+				bfin_write_TWI_MASTER_CTL(
+					(bfin_read_TWI_MASTER_CTL() &
+					(~(0xff << 6))) |
+					( iface->readNum << 6));
+			}
+			/* remove restart bit and enable master receive */
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() &
+				~RSTART);
+			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() |
+				MEN | MDIR);
+			SSYNC();
+		} else {
+			iface->result = 1;
+			bfin_write_TWI_INT_MASK(0);
+			bfin_write_TWI_MASTER_CTL(0);
+			SSYNC();
+			complete(&iface->complete);
+		}
+	}
+}
+
+/* Interrupt handler */
+static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)dev_id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	del_timer(&iface->timeout_timer);
+	bfin_twi_handle_interrupt(iface);
+	spin_unlock_irqrestore(&iface->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static void bfin_twi_timeout(unsigned long data)
+{
+	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iface->lock, flags);
+	bfin_twi_handle_interrupt(iface);
+	if (iface->result == 0) {
+		iface->timeout_count--;
+		if (iface->timeout_count > 0) {
+			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+			add_timer(&iface->timeout_timer);
+		} else {
+			iface->result = -1;
+			complete(&iface->complete);
+		}
+	}
+	spin_unlock_irqrestore(&iface->lock, flags);
+}
+
+/*
+ * Generic i2c master transfer entrypoint
+ */
+static int bfin_twi_master_xfer(struct i2c_adapter *adap,
+				struct i2c_msg msgs[], int num)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	struct i2c_msg *pmsg;
+	int i, ret;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		cond_resched();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	ret = 0;
+	for (i = 0; rc >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+		if (pmsg->flags & I2C_M_TEN) {
+			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr \
+					not supported !\n");
+			rc = -EINVAL;
+			break;
+		}
+
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		iface->manual_stop = 0;
+		iface->transPtr = pmsg->buf;
+		iface->writeNum = iface->readNum = pmsg->len;
+		iface->result = 0;
+		iface->timeout_count = 10;
+		/* Set Transmit device address */
+		bfin_write_TWI_MASTER_ADDR(pmsg->addr);
+
+		/* FIFO Initiation. Data in FIFO should be
+		 *  discarded before start a new operation.
+		 */
+		bfin_write_TWI_FIFO_CTL(0x3);
+		SSYNC();
+		bfin_write_TWI_FIFO_CTL(0);
+		SSYNC();
+
+		if (pmsg->flags & I2C_M_RD)
+			iface->read_write = I2C_SMBUS_READ;
+		else {
+			iface->read_write = I2C_SMBUS_WRITE;
+			/* Transmit first data */
+			if (iface->writeNum > 0) {
+				bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
+				iface->writeNum--;
+				SSYNC();
+			}
+		}
+
+		/* clear int stat */
+		bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+		/* Interrupt mask . Enable XMT, RCV interrupt */
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ)?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (pmsg->len > 0 && pmsg->len <= 255)
+			bfin_write_TWI_MASTER_CTL(pmsg->len << 6);
+		else if (pmsg->len > 255) {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		} else
+			break;
+
+		iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+		add_timer(&iface->timeout_timer);
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
+		SSYNC();
+
+		wait_for_completion(&iface->complete);
+
+		rc = iface->result;
+		if (rc == 1)
+			ret++;
+		else if (rc == -1)
+			break;
+	}
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return ret;
+}
+
+/*
+ * SMBus type transfer entrypoint
+ */
+
+int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			unsigned short flags, char read_write,
+			u8 command, int size, union i2c_smbus_data * data)
+{
+	struct bfin_twi_iface* iface = adap->algo_data;
+	int rc = 0;
+
+	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
+		return -ENXIO;
+
+	mutex_lock(&iface->twi_lock);
+
+	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
+		mutex_unlock(&iface->twi_lock);
+		cond_resched();
+		mutex_lock(&iface->twi_lock);
+	}
+
+	iface->writeNum = 0;
+	iface->readNum = 0;
+
+	/* Prepare datas & select mode */
+	switch (size) {
+	case I2C_SMBUS_QUICK:
+		iface->transPtr = NULL;
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE:
+		if (data == NULL)
+			iface->transPtr = NULL;
+		else {
+			if (read_write == I2C_SMBUS_READ)
+				iface->readNum = 1;
+			else
+				iface->writeNum = 1;
+			iface->transPtr = &data->byte;
+		}
+		iface->cur_mode = TWI_I2C_MODE_STANDARD;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = &data->byte;
+		break;
+	case I2C_SMBUS_WORD_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = 2;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+		iface->writeNum = 2;
+		iface->readNum = 2;
+		iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		iface->transPtr = (u8 *)&data->word;
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		if (read_write == I2C_SMBUS_READ) {
+			iface->readNum = 0;
+			iface->cur_mode = TWI_I2C_MODE_COMBINED;
+		} else {
+			iface->writeNum = data->block[0]+1;
+			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
+		}
+		iface->transPtr = data->block;
+		break;
+	default:
+		return -1;
+	}
+
+	iface->result = 0;
+	iface->manual_stop = 0;
+	iface->read_write = read_write;
+	iface->command = command;
+	iface->timeout_count = 10;
+
+	/* FIFO Initiation. Data in FIFO should be discarded before
+	 * start a new operation.
+	 */
+	bfin_write_TWI_FIFO_CTL(0x3);
+	SSYNC();
+	bfin_write_TWI_FIFO_CTL(0);
+
+	/* clear int stat */
+	bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
+
+	/* Set Transmit device address */
+	bfin_write_TWI_MASTER_ADDR(addr);
+	SSYNC();
+
+	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
+	add_timer(&iface->timeout_timer);
+
+	switch (iface->cur_mode) {
+	case TWI_I2C_MODE_STANDARDSUB:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		if (iface->writeNum + 1 <= 255)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else {
+			bfin_write_TWI_MASTER_CTL(0xff << 6);
+			iface->manual_stop = 1;
+		}
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
+		break;
+	case TWI_I2C_MODE_COMBINED:
+		bfin_write_TWI_XMT_DATA8(iface->command);
+		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
+		SSYNC();
+
+		if (iface->writeNum > 0)
+			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
+		else
+			bfin_write_TWI_MASTER_CTL(0x1 << 6);
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
+		break;
+	default:
+		bfin_write_TWI_MASTER_CTL(0);
+		if (size != I2C_SMBUS_QUICK) {
+			/* Don't access xmit data register when this is a
+			 * read operation.
+			 */
+			if (iface->read_write != I2C_SMBUS_READ) {
+				if (iface->writeNum > 0) {
+					bfin_write_TWI_XMT_DATA8
+						(*(iface->transPtr++));
+					if (iface->writeNum <= 255)
+						bfin_write_TWI_MASTER_CTL
+							(iface->writeNum << 6);
+					else {
+						bfin_write_TWI_MASTER_CTL
+							(0xff << 6);
+						iface->manual_stop = 1;
+					}
+					iface->writeNum--;
+				} else {
+					bfin_write_TWI_XMT_DATA8
+						(iface->command);
+					bfin_write_TWI_MASTER_CTL(1 << 6);
+				}
+			} else {
+				if (iface->readNum > 0
+					&& iface->readNum <= 255)
+					bfin_write_TWI_MASTER_CTL
+						(iface->readNum << 6);
+				else if (iface->readNum > 255) {
+					bfin_write_TWI_MASTER_CTL( 0xff << 6 );
+					iface->manual_stop = 1;
+				} else {
+					del_timer(&iface->timeout_timer);
+					break;
+				}
+			}
+		}
+		bfin_write_TWI_INT_MASK(MCOMP | MERR |
+			((iface->read_write == I2C_SMBUS_READ) ?
+			RCVSERV : XMTSERV));
+		SSYNC();
+
+		/* Master enable */
+		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
+			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
+			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0));
+		break;
+	}
+	SSYNC();
+
+	wait_for_completion(&iface->complete);
+
+	rc = (iface->result >= 0) ? 0 : -1;
+
+	/* Release mutex */
+	mutex_unlock(&iface->twi_lock);
+
+	return rc;
+}
+
+/*
+ * Return what the adapter supports
+ */
+static u32 bfin_twi_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
+	       I2C_FUNC_I2C;
+}
+
+
+static struct i2c_algorithm bfin_twi_algorithm = {
+	.master_xfer   = bfin_twi_master_xfer,
+	.smbus_xfer    = bfin_twi_smbus_xfer,
+	.functionality = bfin_twi_functionality,
+};
+
+static int __init i2c_bfin_twi_init(void)
+{
+	struct i2c_adapter *p_adap;
+	int rc;
+
+	mutex_init(&(twi_iface.twi_lock));
+	spin_lock_init(&twi_iface.lock);
+	init_completion(&twi_iface.complete);
+	twi_iface.irq = IRQ_TWI;
+
+	init_timer(&twi_iface.timeout_timer);
+	twi_iface.timeout_timer.function = bfin_twi_timeout;
+	twi_iface.timeout_timer.data = (unsigned long)&twi_iface;
+
+	p_adap = &twi_iface.adap;
+	p_adap->id = I2C_BLACKFIN_TWI;
+	strcpy(p_adap->name, "i2c-bfin-twi");
+	p_adap->algo = &bfin_twi_algorithm;
+	p_adap->algo_data = &twi_iface;
+	p_adap->client_register = NULL;
+	p_adap->client_unregister = NULL;
+	p_adap->class = I2C_CLASS_ALL;
+
+	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry,
+		SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
+	if (rc) {
+		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n",
+			twi_iface.irq);
+		return -ENODEV;
+	}
+
+	/* Set TWI internal clock as 10MHz */
+	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
+
+	/* Set Twi interface clock as specified */
+	if (CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 400)
+		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) |
+			(( 5*1024 / 400 ) & 0xFF));
+	else
+		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ ) << 8) |
+			(( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ ) & 0xFF));
+
+	/* Enable TWI */
+	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
+	SSYNC();
+
+	rc = i2c_add_adapter(p_adap);
+	if(rc < 0)
+		free_irq(twi_iface.irq, &twi_iface);
+	return rc;
+}
+
+static void __exit i2c_bfin_twi_exit(void)
+{
+	i2c_del_adapter(&twi_iface.adap);
+	free_irq(twi_iface.irq, &twi_iface);
+}
+
+MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin TWI");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_bfin_twi_init);
+module_exit(i2c_bfin_twi_exit);
Index: linux-2.6/drivers/i2c/busses/Makefile
===================================================================
--- linux-2.6.orig/drivers/i2c/busses/Makefile	2007-03-07 17:18:49.000000000 +0800
+++ linux-2.6/drivers/i2c/busses/Makefile	2007-03-07 17:30:11.000000000 +0800
@@ -10,6 +10,8 @@
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
+obj-$(CONFIG_I2C_BLACKFIN_GPIO) += i2c-bfin-gpio.o
+obj-$(CONFIG_I2C_BLACKFIN_TWI)  += i2c-bfin-twi.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
_

Thanks again
Best Regards,
-Bryan Wu

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

* trailing whitespace killing (Re: [PATCH -mm] Blackfin: blackfin i2c driver)
  2007-03-07  7:45           ` Andrew Morton
@ 2007-03-08  3:57             ` Oleg Verych
  2007-03-08  7:35             ` [PATCH -mm] Blackfin: blackfin i2c driver Jean Delvare
  1 sibling, 0 replies; 19+ messages in thread
From: Oleg Verych @ 2007-03-08  3:57 UTC (permalink / raw)
  To: Andrew Morton, bryan.wu
  Cc: Jean Delvare, Alexey Dobriyan, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

> From: Andrew Morton
> Newsgroups: gmane.linux.kernel
> Subject: Re: [PATCH -mm] Blackfin: blackfin i2c driver
> Date: Tue, 6 Mar 2007 23:45:29 -0800
[]
> On Wed, 07 Mar 2007 15:39:27 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:
>
>> Thanks a lot, could you please give me a script just to kill this
>> whitespace? So I can do it before sending you patches.
>
>
> Is pretty simple:
>
> #!/bin/sh
> #
> # Strip any trailing whitespace which a unified diff adds.
> #
>
> strip1()
> {
> 	TMP=$(mktemp /tmp/XXXXXX)
> 	cp $1 $TMP
> 	sed -e '/^+/s/[ 	]*$//' < $TMP > $1
> 	rm $TMP
> }
>
> for i in $*
> do
> 	strip1 $i
> done
>
>
> that'll be in
> http://www.zip.com.au/~akpm/linux/patches/patch-scripts-0.20/patch-scripts-0.20.tar.gz
> too

It doesn't work for me. Maybe i can't understand what you are trying to
do, anyway.

General suggestion is can be:

   sed -e 's_[ \t]*$__'

(i.e any line on stdin with space/tab mixed tails is stripped on stdout)

You can use it as wrapper for diff, sending patch bombs, etc.
(very nice with pipes):

shell$ diff -Npu2 old new | sed -e 's_[ \t]*$__' > patch.diff
shell$ < patch-set.mbox sed -e 's_[ \t]*$__' | formail -s /usr/sbin/sendmail -bm -t 

similar in scripts; quilt (patch sets manager) notices about them.
____

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  7:45           ` Andrew Morton
  2007-03-08  3:57             ` trailing whitespace killing (Re: [PATCH -mm] Blackfin: blackfin i2c driver) Oleg Verych
@ 2007-03-08  7:35             ` Jean Delvare
  1 sibling, 0 replies; 19+ messages in thread
From: Jean Delvare @ 2007-03-08  7:35 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Bryan Wu, Alexey Dobriyan, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

On Tue, 6 Mar 2007 23:45:29 -0800, Andrew Morton wrote:
> On Wed, 07 Mar 2007 15:39:27 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:
> 
> > Thanks a lot, could you please give me a script just to kill this
> > whitespace? So I can do it before sending you patches.
> 
> 
> Is pretty simple:
> 
> #!/bin/sh
> #
> # Strip any trailing whitespace which a unified diff adds.
> #
> 
> strip1()
> {
> 	TMP=$(mktemp /tmp/XXXXXX)
> 	cp $1 $TMP
> 	sed -e '/^+/s/[ 	]*$//' < $TMP > $1
> 	rm $TMP
> }
> 
> for i in $*
> do
> 	strip1 $i
> done
> 
> 
> that'll be in
> http://www.zip.com.au/~akpm/linux/patches/patch-scripts-0.20/patch-scripts-0.20.tar.gz
> too

Alternatively, you can use quilt [1] to manage your patches and enable
the --strip-trailing-whitespace option by default.

[1] http://savannah.nongnu.org/projects/quilt/

-- 
Jean Delvare

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-07  9:53       ` Wu, Bryan
@ 2007-03-08  9:27         ` Jean Delvare
  2007-03-09  4:04           ` Sonic Zhang
  0 siblings, 1 reply; 19+ messages in thread
From: Jean Delvare @ 2007-03-08  9:27 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Andrew Morton, Alexey Dobriyan, Sonic Zhang, Mike Frysinger,
	linux-kernel, mhfan

Hi Brian,

Thanks for the quick update.

> [PATCH] Blackfin: blackfin i2c driver
> 
> The i2c linux driver for blackfin architecture which supports both GPIO
> i2c operation and blackfin on-chip TWI controller i2c operation.
> 
> Signed-off-by: Bryan Wu <bryan.wu@analog.com>
> Reviewed-by: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Alexey Dobriyan <adobriyan@gmail.com>
> Reviewed-by: Jean Delvare <khali@linux-fr.org>

Actually I had only reviewed the Kconfig part, not the driver itself.
Here's a more complete review:

> ---
> 
>  drivers/i2c/busses/Kconfig         |   47 ++++
>  drivers/i2c/busses/Makefile        |    2
>  drivers/i2c/busses/i2c-bfin-gpio.c |  100 +++++++++
>  drivers/i2c/busses/i2c-bfin-twi.c  |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 738 insertions(+)
> 
> Index: linux-2.6/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/i2c/busses/Kconfig	2007-03-07 17:18:49.000000000 +0800
> +++ linux-2.6/drivers/i2c/busses/Kconfig	2007-03-07 17:23:05.000000000 +0800
> @@ -91,6 +91,53 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-au1550.
>  
> +config I2C_BLACKFIN_GPIO
> +	tristate "Blackfin GPIO based I2C interface"
> +	depends on I2C && BLACKFIN && EXPERIMENTAL
> +	select I2C_ALGOBIT
> +	help
> +	  Say Y here if you have an Blackfin BF5xx based
> +	  system and are using GPIO lines for an I2C bus.
> +
> +menu "Blackfin I2C SDA/SCL Selection"
> +	depends on I2C_BLACKFIN_GPIO
> +config I2C_BLACKFIN_GPIO_SDA
> +	int "SDA GPIO pin number"
> +	range 0 15 if (BF533 || BF532 || BF531)
> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 2
> +
> +config I2C_BLACKFIN_GPIO_SCL
> +	int "SCL GPIO pin number"
> +	range 0 15 if (BF533 || BF532 || BF531)
> +	range 0 47 if (BF534 || BF536 || BF537)
> +	range 0 47 if BF561
> +	default 3
> +endmenu
> +
> +config I2C_BLACKFIN_GPIO_CYCLE_DELAY
> +	int "Cycle Delay in uSec"

The proper symbol is "us".

> +	depends on I2C_BLACKFIN_GPIO
> +	range 5 100
> +	default 40
> +
> +config I2C_BLACKFIN_TWI
> +	tristate "Blackfin TWI I2C support"
> +	depends on I2C && (BF534 || BF536 || BF537)
> +	help
> +	  This is the TWI I2C device driver for Blackfin 534/536/537.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-bfin-twi.
> +
> +config I2C_BLACKFIN_TWI_CLK_KHZ
> +	int "Blackfin TWI I2C clock (kHz)"
> +	depends on I2C_BLACKFIN_TWI
> +	range 10 400
> +	default 50
> +	help
> +	  The unit of the TWI clock is kilo HZ.

kHz

> +
>  config I2C_ELEKTOR
>  	tristate "Elektor ISA card"
>  	depends on I2C && ISA && BROKEN_ON_SMP
> Index: linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/drivers/i2c/busses/i2c-bfin-gpio.c	2007-03-07 17:32:30.000000000 +0800
> @@ -0,0 +1,100 @@
> +/****************************************************************
> + * Description:                                                 *
> + *                                                              *
> + * Maintainer: Meihui Fan <mhfan@ustc.edu>		        *

Indentation problem. Also, maintainer information goes in MAINTAINERS,
not individual drivers.

> + *                                                              *
> + * CopyRight (c)  2004  HHTech                                  *
> + *   www.hhcn.com, www.hhcn.org                                 *
> + *   All rights reserved.                                       *
> + *                                                              *
> + * This file is free software;                                  *
> + *   you are free to modify and/or redistribute it   	        *

Indentation problem here too.

> + *   under the terms of the GNU General Public Licence (GPL).   *
> + *                                                              *
> + ****************************************************************/
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-bit.h>
> +
> +#include <asm/blackfin.h>
> +#include <asm/gpio.h>
> +
> +#define	I2C_HW_B_HHBF		    0x13

Holy god, no! This define goes in include/linux/i2c-id.h.

> +
> +static void hhbf_setsda(void *data, int state)
> +{
> +	if (state) {
> +		gpio_direction_input(CONFIG_I2C_BLACKFIN_GPIO_SDA);
> +

Extra blank line.

> +	} else {
> +		gpio_direction_output(CONFIG_I2C_BLACKFIN_GPIO_SDA);
> +		gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SDA, 0);
> +	}
> +}
> +
> +static void hhbf_setscl(void *data, int state)
> +{
> +	gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SCL, state);
> +}
> +
> +static int hhbf_getsda(void *data)
> +{
> +	return (gpio_get_value(CONFIG_I2C_BLACKFIN_GPIO_SDA) != 0);
> +}
> +
> +
> +static struct i2c_algo_bit_data bit_hhbf_data = {
> +	.setsda  = hhbf_setsda,
> +	.setscl  = hhbf_setscl,
> +	.getsda  = hhbf_getsda,

Is the hardware not capable of SCL sensing? If it is, you really want
to implement it. Bit-banged I2C without SCL sensing doesn't completely
implement the I2C standard, it lacks the SCL stretching feature.

> +	.udelay  = CONFIG_I2C_BLACKFIN_GPIO_CYCLE_DELAY,
> +	.timeout = HZ

Always add an extra comma at the end of struct declarations.

> +};
> +
> +static struct i2c_adapter hhbf_ops = {
> +	.owner 	= THIS_MODULE,
> +	.id 	= I2C_HW_B_HHBF,
> +	.algo_data 	= &bit_hhbf_data,
> +	.name	= "Blackfin GPIO based I2C driver",

This structure describes a (virtual) device, not a driver.

> +};
> +
> +static int __init i2c_hhbf_init(void)
> +{
> +

Extra blank line.

> +	if (gpio_request(CONFIG_I2C_BLACKFIN_GPIO_SCL, NULL)) {
> +		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n", __func__,
> +				CONFIG_I2C_BLACKFIN_GPIO_SCL);

The function name doesn't matter here, what is the end user going to do
with the information? What matters is the driver name.

> +		return -1;

This -1 will be interpreted as -EPERM by insmod. Please use a proper
error code.

> +	}
> +
> +	if (gpio_request(CONFIG_I2C_BLACKFIN_GPIO_SDA, NULL)) {
> +		printk(KERN_ERR "%s: gpio_request GPIO %d failed \n", __func__,
> +				CONFIG_I2C_BLACKFIN_GPIO_SDA);
> +		return -1;

Broken error path. You've successfully requested the GPIO for SCL, but
return with an error without releasing it.

> +	}
> +
> +

Extra blank line.

> +	gpio_direction_output(CONFIG_I2C_BLACKFIN_GPIO_SCL);
> +	gpio_direction_input(CONFIG_I2C_BLACKFIN_GPIO_SDA);
> +	gpio_set_value(CONFIG_I2C_BLACKFIN_GPIO_SCL, 1);
> +

At this point, i2c-core will complain that you created an i2c_adapter
without a valid parent device. This needs to be fixed.

> +	return i2c_bit_add_bus(&hhbf_ops);
> +}
> +
> +static void __exit i2c_hhbf_exit(void)
> +{
> +	gpio_free(CONFIG_I2C_BLACKFIN_GPIO_SCL);
> +	gpio_free(CONFIG_I2C_BLACKFIN_GPIO_SDA);
> +	i2c_bit_del_bus(&hhbf_ops);

You're doing things the wrong way around here, you're freeing the
resources while there might still be someone using the adapter.

BTW, i2c_bit_del_bus no longer exists, so this won't compile. Use
i2c_del_adapter instead. You really want to test your code with a
recent kernel before submitting it again.

> +}
> +
> +MODULE_AUTHOR("Meihui Fan <mhfan@ustc.edu>");
> +MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin and HHBF Boards");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i2c_hhbf_init);
> +module_exit(i2c_hhbf_exit);
> Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c	2007-03-07 17:28:57.000000000 +0800
> @@ -0,0 +1,589 @@
> +/****************************************************************
> + * Description: Driver for Blackfin Two Wire Interface          *
> + * Maintainer:  sonicz  <sonic.zhang@analog.com>                *

Same as above, this goes to MAINTAINERS.

> + *                                                              *
> + * Copyright (c) 2005-2007 Analog Device                        *
> + *                                                              *
> + * This file is free software;                                  *
> + *   you are free to modify and/or redistribute it   	        *

Broken indentation.

> + *   under the terms of the GNU General Public Licence (GPL).   *
> + ****************************************************************/
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/mm.h>
> +#include <linux/timer.h>
> +#include <linux/spinlock.h>
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +
> +#include <asm/blackfin.h>
> +#include <asm/irq.h>
> +
> +#define I2C_BLACKFIN_TWI       0x00

No, define a valid ID in linux/i2c-id.h if you need one, otherwise
don't set an ID at all.

> +#define POLL_TIMEOUT       (2 * HZ)
> +#ifndef CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ
> +#define CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ  400
> +#endif

How could this actually not be defined?

> +
> +/* SMBus mode*/
> +#define TWI_I2C_MODE_STANDARD		0x01
> +#define TWI_I2C_MODE_STANDARDSUB	0x02
> +#define TWI_I2C_MODE_COMBINED		0x04
> +
> +struct bfin_twi_iface
> +{

The curly brace goes at the end of the previous line.

> +	struct mutex		twi_lock;
> +	int			irq;
> +	spinlock_t		lock;
> +	char			read_write;
> +	u8			command;
> +	u8			*transPtr;
> +	int			readNum;
> +	int			writeNum;
> +	int			cur_mode;
> +	int			manual_stop;
> +	int			result;
> +	int			timeout_count;
> +	struct timer_list	timeout_timer;
> +	struct i2c_adapter	adap;
> +	struct completion	complete;
> +};
> +
> +static struct bfin_twi_iface twi_iface;
> +
> +static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
> +{
> +	unsigned short twi_int_status = bfin_read_TWI_INT_STAT();
> +	unsigned short mast_stat = bfin_read_TWI_MASTER_STAT();
> +
> +	if (twi_int_status & XMTSERV) {
> +		/* Transmit next data */
> +		if (iface->writeNum > 0) {
> +			bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
> +			iface->writeNum--;
> +		}
> +		/* start receive immediately after complete sending in
> +		 * combine mode.
> +		 */
> +		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
> +			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
> +				| MDIR | RSTART);
> +		} else if (iface->manual_stop)
> +			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
> +				| STOP);
> +		SSYNC();
> +		/* Clear status */
> +		bfin_write_TWI_INT_STAT(XMTSERV);
> +		SSYNC();
> +	}
> +	if (twi_int_status & RCVSERV) {
> +		if (iface->readNum > 0) {
> +			/* Receive next data */
> +			*(iface->transPtr) = bfin_read_TWI_RCV_DATA8();
> +			if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
> +				/* Change combine mode into sub mode after
> +				 * read first data.
> +				 */
> +				iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
> +				/* Get read number from first byte in block
> +				 * combine mode.
> +				 */
> +				if (iface->readNum == 1 && iface->manual_stop)
> +					iface->readNum = *iface->transPtr+1;
> +			}
> +			iface->transPtr++;
> +			iface->readNum--;
> +		} else if (iface->manual_stop) {
> +			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL()
> +				| STOP);
> +			SSYNC();
> +		}
> +		/* Clear interrupt source */
> +		bfin_write_TWI_INT_STAT(RCVSERV);
> +		SSYNC();
> +	}
> +	if (twi_int_status & MERR) {
> +		bfin_write_TWI_INT_STAT(MERR);
> +		bfin_write_TWI_INT_MASK(0);
> +		bfin_write_TWI_MASTER_STAT(0x3e);
> +		bfin_write_TWI_MASTER_CTL(0);
> +		SSYNC();
> +		iface->result = -1;
> +		/* if both err and complete int stats are set, return proper
> +		 * results.
> +		 */
> +		if (twi_int_status & MCOMP) {
> +			bfin_write_TWI_INT_STAT(MCOMP);
> +			bfin_write_TWI_INT_MASK(0);
> +			bfin_write_TWI_MASTER_CTL(0);
> +			SSYNC();
> +			/* If it is a quick transfer, only address bug no data,
> +			 * not an err, return 1.
> +			 */
> +			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
> +				iface->result = 1;
> +			/* If address not acknowledged return -1,
> +			 * else return 0.
> +			 */
> +			else if (!(mast_stat & ANAK))
> +				iface->result = 0;
> +		}
> +		complete(&iface->complete);
> +		return;
> +	}
> +	if (twi_int_status & MCOMP) {
> +		bfin_write_TWI_INT_STAT(MCOMP);
> +		SSYNC();
> +		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
> +			if (iface->readNum == 0) {
> +				/* set the read number to 1 and ask for manual
> +				 * stop in block combine mode
> +				 */
> +				iface->readNum = 1;
> +				iface->manual_stop = 1;
> +				bfin_write_TWI_MASTER_CTL(
> +					bfin_read_TWI_MASTER_CTL()
> +					| (0xff << 6));
> +			} else {
> +				/* set the readd number in other
> +				 * combine mode.
> +				 */
> +				bfin_write_TWI_MASTER_CTL(
> +					(bfin_read_TWI_MASTER_CTL() &
> +					(~(0xff << 6))) |
> +					( iface->readNum << 6));
> +			}
> +			/* remove restart bit and enable master receive */
> +			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() &
> +				~RSTART);
> +			bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() |
> +				MEN | MDIR);
> +			SSYNC();
> +		} else {
> +			iface->result = 1;
> +			bfin_write_TWI_INT_MASK(0);
> +			bfin_write_TWI_MASTER_CTL(0);
> +			SSYNC();
> +			complete(&iface->complete);
> +		}
> +	}
> +}
> +
> +/* Interrupt handler */
> +static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
> +{
> +	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)dev_id;

Cast isn't needed.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&iface->lock, flags);
> +	del_timer(&iface->timeout_timer);
> +	bfin_twi_handle_interrupt(iface);
> +	spin_unlock_irqrestore(&iface->lock, flags);
> +	return IRQ_HANDLED;
> +}
> +
> +static void bfin_twi_timeout(unsigned long data)
> +{
> +	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&iface->lock, flags);
> +	bfin_twi_handle_interrupt(iface);
> +	if (iface->result == 0) {
> +		iface->timeout_count--;
> +		if (iface->timeout_count > 0) {
> +			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
> +			add_timer(&iface->timeout_timer);
> +		} else {
> +			iface->result = -1;
> +			complete(&iface->complete);
> +		}
> +	}
> +	spin_unlock_irqrestore(&iface->lock, flags);
> +}
> +
> +/*
> + * Generic i2c master transfer entrypoint
> + */
> +static int bfin_twi_master_xfer(struct i2c_adapter *adap,
> +				struct i2c_msg msgs[], int num)

The prefered syntax is struct i2c_msg *msgs.

> +{
> +	struct bfin_twi_iface* iface = adap->algo_data;
> +	struct i2c_msg *pmsg;
> +	int i, ret;
> +	int rc = 0;
> +
> +	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> +		return -ENXIO;
> +
> +	mutex_lock(&iface->twi_lock);
> +
> +	while (bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> +		mutex_unlock(&iface->twi_lock);
> +		cond_resched();
> +		mutex_lock(&iface->twi_lock);
> +	}
> +
> +	ret = 0;
> +	for (i = 0; rc >= 0 && i < num; i++) {
> +		pmsg = &msgs[i];
> +		if (pmsg->flags & I2C_M_TEN) {
> +			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr \
> +					not supported !\n");

This is not how you break a string in C. The right way is:

			printk(KERN_ERR "i2c-bfin-twi: 10 bits addr "
					"not supported!\n");

BTW you should be using dev_err() here instead.

> +			rc = -EINVAL;
> +			break;
> +		}
> +
> +		iface->cur_mode = TWI_I2C_MODE_STANDARD;
> +		iface->manual_stop = 0;
> +		iface->transPtr = pmsg->buf;
> +		iface->writeNum = iface->readNum = pmsg->len;
> +		iface->result = 0;
> +		iface->timeout_count = 10;
> +		/* Set Transmit device address */
> +		bfin_write_TWI_MASTER_ADDR(pmsg->addr);
> +
> +		/* FIFO Initiation. Data in FIFO should be
> +		 *  discarded before start a new operation.
> +		 */
> +		bfin_write_TWI_FIFO_CTL(0x3);
> +		SSYNC();
> +		bfin_write_TWI_FIFO_CTL(0);
> +		SSYNC();
> +
> +		if (pmsg->flags & I2C_M_RD)
> +			iface->read_write = I2C_SMBUS_READ;
> +		else {
> +			iface->read_write = I2C_SMBUS_WRITE;
> +			/* Transmit first data */
> +			if (iface->writeNum > 0) {
> +				bfin_write_TWI_XMT_DATA8(*(iface->transPtr++));
> +				iface->writeNum--;
> +				SSYNC();
> +			}
> +		}
> +
> +		/* clear int stat */
> +		bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
> +
> +		/* Interrupt mask . Enable XMT, RCV interrupt */
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR |
> +			((iface->read_write == I2C_SMBUS_READ)?
> +			RCVSERV : XMTSERV));
> +		SSYNC();
> +
> +		if (pmsg->len > 0 && pmsg->len <= 255)
> +			bfin_write_TWI_MASTER_CTL(pmsg->len << 6);
> +		else if (pmsg->len > 255) {
> +			bfin_write_TWI_MASTER_CTL(0xff << 6);
> +			iface->manual_stop = 1;
> +		} else
> +			break;
> +
> +		iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
> +		add_timer(&iface->timeout_timer);
> +
> +		/* Master enable */
> +		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
> +			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
> +			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
> +		SSYNC();
> +
> +		wait_for_completion(&iface->complete);
> +
> +		rc = iface->result;
> +		if (rc == 1)
> +			ret++;
> +		else if (rc == -1)
> +			break;
> +	}
> +
> +	/* Release mutex */
> +	mutex_unlock(&iface->twi_lock);
> +
> +	return ret;

ret always has the same value as i as far as I can see, so you don't
need to separate variables.

> +}
> +
> +/*
> + * SMBus type transfer entrypoint
> + */
> +
> +int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> +			unsigned short flags, char read_write,
> +			u8 command, int size, union i2c_smbus_data * data)
> +{
> +	struct bfin_twi_iface* iface = adap->algo_data;
> +	int rc = 0;
> +
> +	if (!(bfin_read_TWI_CONTROL() & TWI_ENA))
> +		return -ENXIO;
> +
> +	mutex_lock(&iface->twi_lock);
> +
> +	while(bfin_read_TWI_MASTER_STAT() & BUSBUSY) {
> +		mutex_unlock(&iface->twi_lock);
> +		cond_resched();
> +		mutex_lock(&iface->twi_lock);
> +	}
> +
> +	iface->writeNum = 0;
> +	iface->readNum = 0;
> +
> +	/* Prepare datas & select mode */
> +	switch (size) {
> +	case I2C_SMBUS_QUICK:
> +		iface->transPtr = NULL;
> +		iface->cur_mode = TWI_I2C_MODE_STANDARD;
> +		break;
> +	case I2C_SMBUS_BYTE:
> +		if (data == NULL)
> +			iface->transPtr = NULL;
> +		else {
> +			if (read_write == I2C_SMBUS_READ)
> +				iface->readNum = 1;
> +			else
> +				iface->writeNum = 1;
> +			iface->transPtr = &data->byte;
> +		}
> +		iface->cur_mode = TWI_I2C_MODE_STANDARD;
> +		break;
> +	case I2C_SMBUS_BYTE_DATA:
> +		if (read_write == I2C_SMBUS_READ) {
> +			iface->readNum = 1;
> +			iface->cur_mode = TWI_I2C_MODE_COMBINED;
> +		} else {
> +			iface->writeNum = 1;
> +			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
> +		}
> +		iface->transPtr = &data->byte;
> +		break;
> +	case I2C_SMBUS_WORD_DATA:
> +		if (read_write == I2C_SMBUS_READ) {
> +			iface->readNum = 2;
> +			iface->cur_mode = TWI_I2C_MODE_COMBINED;
> +		} else {
> +			iface->writeNum = 2;
> +			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
> +		}
> +		iface->transPtr = (u8 *)&data->word;
> +		break;
> +	case I2C_SMBUS_PROC_CALL:
> +		iface->writeNum = 2;
> +		iface->readNum = 2;
> +		iface->cur_mode = TWI_I2C_MODE_COMBINED;
> +		iface->transPtr = (u8 *)&data->word;
> +		break;
> +	case I2C_SMBUS_BLOCK_DATA:
> +		if (read_write == I2C_SMBUS_READ) {
> +			iface->readNum = 0;
> +			iface->cur_mode = TWI_I2C_MODE_COMBINED;
> +		} else {
> +			iface->writeNum = data->block[0]+1;
> +			iface->cur_mode = TWI_I2C_MODE_STANDARDSUB;
> +		}
> +		iface->transPtr = data->block;
> +		break;
> +	default:
> +		return -1;
> +	}
> +
> +	iface->result = 0;
> +	iface->manual_stop = 0;
> +	iface->read_write = read_write;
> +	iface->command = command;
> +	iface->timeout_count = 10;
> +
> +	/* FIFO Initiation. Data in FIFO should be discarded before
> +	 * start a new operation.
> +	 */
> +	bfin_write_TWI_FIFO_CTL(0x3);
> +	SSYNC();
> +	bfin_write_TWI_FIFO_CTL(0);
> +
> +	/* clear int stat */
> +	bfin_write_TWI_INT_STAT(MERR|MCOMP|XMTSERV|RCVSERV);
> +
> +	/* Set Transmit device address */
> +	bfin_write_TWI_MASTER_ADDR(addr);
> +	SSYNC();
> +
> +	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
> +	add_timer(&iface->timeout_timer);
> +
> +	switch (iface->cur_mode) {
> +	case TWI_I2C_MODE_STANDARDSUB:
> +		bfin_write_TWI_XMT_DATA8(iface->command);
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR |
> +			((iface->read_write == I2C_SMBUS_READ) ?
> +			RCVSERV : XMTSERV));
> +		SSYNC();
> +
> +		if (iface->writeNum + 1 <= 255)
> +			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
> +		else {
> +			bfin_write_TWI_MASTER_CTL(0xff << 6);
> +			iface->manual_stop = 1;
> +		}
> +		/* Master enable */
> +		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
> +			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
> +		break;
> +	case TWI_I2C_MODE_COMBINED:
> +		bfin_write_TWI_XMT_DATA8(iface->command);
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR | RCVSERV | XMTSERV);
> +		SSYNC();
> +
> +		if (iface->writeNum > 0)
> +			bfin_write_TWI_MASTER_CTL((iface->writeNum+1) << 6);
> +		else
> +			bfin_write_TWI_MASTER_CTL(0x1 << 6);
> +		/* Master enable */
> +		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
> +			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
> +		break;
> +	default:
> +		bfin_write_TWI_MASTER_CTL(0);
> +		if (size != I2C_SMBUS_QUICK) {
> +			/* Don't access xmit data register when this is a
> +			 * read operation.
> +			 */
> +			if (iface->read_write != I2C_SMBUS_READ) {
> +				if (iface->writeNum > 0) {
> +					bfin_write_TWI_XMT_DATA8
> +						(*(iface->transPtr++));
> +					if (iface->writeNum <= 255)
> +						bfin_write_TWI_MASTER_CTL
> +							(iface->writeNum << 6);
> +					else {
> +						bfin_write_TWI_MASTER_CTL
> +							(0xff << 6);
> +						iface->manual_stop = 1;
> +					}
> +					iface->writeNum--;
> +				} else {
> +					bfin_write_TWI_XMT_DATA8
> +						(iface->command);
> +					bfin_write_TWI_MASTER_CTL(1 << 6);
> +				}
> +			} else {
> +				if (iface->readNum > 0
> +					&& iface->readNum <= 255)
> +					bfin_write_TWI_MASTER_CTL
> +						(iface->readNum << 6);
> +				else if (iface->readNum > 255) {
> +					bfin_write_TWI_MASTER_CTL( 0xff << 6 );
> +					iface->manual_stop = 1;
> +				} else {
> +					del_timer(&iface->timeout_timer);
> +					break;
> +				}
> +			}
> +		}
> +		bfin_write_TWI_INT_MASK(MCOMP | MERR |
> +			((iface->read_write == I2C_SMBUS_READ) ?
> +			RCVSERV : XMTSERV));
> +		SSYNC();
> +
> +		/* Master enable */
> +		bfin_write_TWI_MASTER_CTL(bfin_read_TWI_MASTER_CTL() | MEN |
> +			((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
> +			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0));
> +		break;
> +	}
> +	SSYNC();
> +
> +	wait_for_completion(&iface->complete);
> +
> +	rc = (iface->result >= 0) ? 0 : -1;
> +
> +	/* Release mutex */
> +	mutex_unlock(&iface->twi_lock);
> +
> +	return rc;
> +}

i2c-core can emulate SMBus transactions using master_xfer, so in
general when you have a complete master_xfer implementation you do not
need to define a separate smbus_xfer function. This would save a lot of
code.

> +
> +/*
> + * Return what the adapter supports
> + */
> +static u32 bfin_twi_functionality(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> +	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> +	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
> +	       I2C_FUNC_I2C;
> +}
> +
> +
> +static struct i2c_algorithm bfin_twi_algorithm = {
> +	.master_xfer   = bfin_twi_master_xfer,
> +	.smbus_xfer    = bfin_twi_smbus_xfer,
> +	.functionality = bfin_twi_functionality,
> +};
> +
> +static int __init i2c_bfin_twi_init(void)
> +{
> +	struct i2c_adapter *p_adap;
> +	int rc;
> +
> +	mutex_init(&(twi_iface.twi_lock));
> +	spin_lock_init(&twi_iface.lock);
> +	init_completion(&twi_iface.complete);
> +	twi_iface.irq = IRQ_TWI;
> +
> +	init_timer(&twi_iface.timeout_timer);
> +	twi_iface.timeout_timer.function = bfin_twi_timeout;
> +	twi_iface.timeout_timer.data = (unsigned long)&twi_iface;
> +
> +	p_adap = &twi_iface.adap;
> +	p_adap->id = I2C_BLACKFIN_TWI;
> +	strcpy(p_adap->name, "i2c-bfin-twi");

Please use strlcpy intead.

> +	p_adap->algo = &bfin_twi_algorithm;
> +	p_adap->algo_data = &twi_iface;
> +	p_adap->client_register = NULL;
> +	p_adap->client_unregister = NULL;

These are already NULL, no need to set them explicitly.

> +	p_adap->class = I2C_CLASS_ALL;
> +
> +	rc = request_irq(twi_iface.irq, bfin_twi_interrupt_entry,
> +		SA_INTERRUPT, "i2c-bfin-twi", &twi_iface);
> +	if (rc) {
> +		printk(KERN_ERR "i2c-bfin-twi: can't get IRQ %d !\n",

No space before exclamation mark in english.

> +			twi_iface.irq);
> +		return -ENODEV;
> +	}
> +
> +	/* Set TWI internal clock as 10MHz */
> +	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
> +
> +	/* Set Twi interface clock as specified */
> +	if (CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 400)
> +		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) |
> +			(( 5*1024 / 400 ) & 0xFF));
> +	else
> +		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ ) << 8) |
> +			(( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ ) & 0xFF));

Line too long, please fold more.

> +
> +	/* Enable TWI */
> +	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
> +	SSYNC();
> +
> +	rc = i2c_add_adapter(p_adap);

Here too, i2c-core will complain that you failed to provide a valid
parent device. You need to fix this, most probably by converting your
driver to a platform driver.

> +	if(rc < 0)
> +		free_irq(twi_iface.irq, &twi_iface);
> +	return rc;
> +}
> +
> +static void __exit i2c_bfin_twi_exit(void)
> +{
> +	i2c_del_adapter(&twi_iface.adap);
> +	free_irq(twi_iface.irq, &twi_iface);
> +}
> +
> +MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
> +MODULE_DESCRIPTION("I2C-Bus adapter routines for Blackfin TWI");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i2c_bfin_twi_init);
> +module_exit(i2c_bfin_twi_exit);
> Index: linux-2.6/drivers/i2c/busses/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/i2c/busses/Makefile	2007-03-07 17:18:49.000000000 +0800
> +++ linux-2.6/drivers/i2c/busses/Makefile	2007-03-07 17:30:11.000000000 +0800
> @@ -10,6 +10,8 @@
>  obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
>  obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
> +obj-$(CONFIG_I2C_BLACKFIN_GPIO) += i2c-bfin-gpio.o
> +obj-$(CONFIG_I2C_BLACKFIN_TWI)  += i2c-bfin-twi.o
>  obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
>  obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
>  obj-$(CONFIG_I2C_I801)		+= i2c-i801.o

-- 
Jean Delvare

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-08  9:27         ` Jean Delvare
@ 2007-03-09  4:04           ` Sonic Zhang
  2007-03-09  9:47             ` Jean Delvare
  0 siblings, 1 reply; 19+ messages in thread
From: Sonic Zhang @ 2007-03-09  4:04 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Bryan Wu, Andrew Morton, Alexey Dobriyan, Mike Frysinger,
	linux-kernel, mhfan

On 3/8/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Brian,
>
> Thanks for the quick update.



> > +
> > +     rc = (iface->result >= 0) ? 0 : -1;
> > +
> > +     /* Release mutex */
> > +     mutex_unlock(&iface->twi_lock);
> > +
> > +     return rc;
> > +}
>
> i2c-core can emulate SMBus transactions using master_xfer, so in
> general when you have a complete master_xfer implementation you do not
> need to define a separate smbus_xfer function. This would save a lot of
> code.
>

Actually the i2c-core can't emulate SMBus transactions using the
master_xfer function, because the blackfin TWI controller provide
hardware support to the SMBus transactions and the combination of
master_xfer operations can't generate proper signal for SMBus.

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

* Re: [PATCH -mm] Blackfin: blackfin i2c driver
  2007-03-09  4:04           ` Sonic Zhang
@ 2007-03-09  9:47             ` Jean Delvare
  0 siblings, 0 replies; 19+ messages in thread
From: Jean Delvare @ 2007-03-09  9:47 UTC (permalink / raw)
  To: Sonic Zhang
  Cc: Bryan Wu, Andrew Morton, Alexey Dobriyan, Mike Frysinger,
	linux-kernel, mhfan

On Fri, 9 Mar 2007 12:04:59 +0800, Sonic Zhang wrote:
> On 3/8/07, Jean Delvare <khali@linux-fr.org> wrote:
> > i2c-core can emulate SMBus transactions using master_xfer, so in
> > general when you have a complete master_xfer implementation you do not
> > need to define a separate smbus_xfer function. This would save a lot of
> > code.
> 
> Actually the i2c-core can't emulate SMBus transactions using the
> master_xfer function, because the blackfin TWI controller provide
> hardware support to the SMBus transactions and the combination of
> master_xfer operations can't generate proper signal for SMBus.

Did you try? I can't think of any valid reason why it wouldn't work.
All SMBus transactions are compatible with I2C by definition.

Now performance might be better with dedicated code if the hardware
accelerate SMBus transactions, that's a different issue. If you prefer
to keep the smbus_xfer method for performance reasons, that's OK with
me. After all you're the one who will have to maintain the driver ;)

-- 
Jean Delvare

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

end of thread, other threads:[~2007-03-09  9:48 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-06  6:54 [PATCH -mm] Blackfin: blackfin i2c driver Wu, Bryan
2007-03-06  7:18 ` Andrew Morton
2007-03-07  5:17   ` Sonic Zhang
2007-03-07  6:06     ` Andrew Morton
2007-03-07  6:38       ` Wu, Bryan
2007-03-06 21:43 ` Alexey Dobriyan
2007-03-06 23:04   ` Mike Frysinger
2007-03-07  5:57   ` Wu, Bryan
2007-03-07  6:58     ` Jean Delvare
2007-03-07  7:14       ` Andrew Morton
2007-03-07  7:39         ` Wu, Bryan
2007-03-07  7:45           ` Andrew Morton
2007-03-08  3:57             ` trailing whitespace killing (Re: [PATCH -mm] Blackfin: blackfin i2c driver) Oleg Verych
2007-03-08  7:35             ` [PATCH -mm] Blackfin: blackfin i2c driver Jean Delvare
2007-03-07  9:53       ` Wu, Bryan
2007-03-08  9:27         ` Jean Delvare
2007-03-09  4:04           ` Sonic Zhang
2007-03-09  9:47             ` Jean Delvare
2007-03-07  7:02     ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox