[PATCHv2 0/3] PCA9564 platform driver

[PATCHv2 0/3] PCA9564 platform driver

[Wolfram Sang]

Hello,

this is my second try of a patch series adding a platform driver for the
PCA9564. I included most suggestions from last time; if not, I tried to make
clear why. One point I should mention again is that I did not use NO_IRQ as I
read on lkml, that 0 is the equivalent for NO_IRQ and everything else is more
like an unwanted exception.

I built this driver on x86 and blackfin. The actual testing was done using the
latter.

All the best,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCHv2 1/3] Remove trailing whitespaces and unnecessary UTF

[Wolfram Sang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: i2c-pca-trailing-spaces.diff --]
[-- Type: text/plain, Size: 5726 bytes --]

Remove trailing whitespaces to make further patches more readable.  Also remove
an unnecessary UTF-char for simplicity ("us" for microseconds is fine enough).

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
 drivers/i2c/algos/i2c-algo-pca.c |   42 +++++++++++++++++++--------------------
 drivers/i2c/busses/i2c-pca-isa.c |    4 +--
 2 files changed, 23 insertions(+), 23 deletions(-)

Index: linux-playground/drivers/i2c/algos/i2c-algo-pca.c
===================================================================
--- linux-playground.orig/drivers/i2c/algos/i2c-algo-pca.c	2008-02-06 20:15:38.000000000 +0100
+++ linux-playground/drivers/i2c/algos/i2c-algo-pca.c	2008-02-06 21:20:01.000000000 +0100
@@ -1,5 +1,5 @@
 /*
- *  i2c-algo-pca.c i2c driver algorithms for PCA9564 adapters                
+ *  i2c-algo-pca.c i2c driver algorithms for PCA9564 adapters
  *    Copyright (C) 2004 Arcom Control Systems
  *
  *  This program is free software; you can redistribute it and/or modify
@@ -99,7 +99,7 @@
  *
  * returns after the address has been sent
  */
-static void pca_address(struct i2c_algo_pca_data *adap, 
+static void pca_address(struct i2c_algo_pca_data *adap,
 			struct i2c_msg *msg)
 {
 	int sta = pca_get_con(adap);
@@ -108,9 +108,9 @@
 	addr = ( (0x7f & msg->addr) << 1 );
 	if (msg->flags & I2C_M_RD )
 		addr |= 1;
-	DEB2("=== SLAVE ADDRESS %#04x+%c=%#04x\n", 
+	DEB2("=== SLAVE ADDRESS %#04x+%c=%#04x\n",
 	     msg->addr, msg->flags & I2C_M_RD ? 'R' : 'W', addr);
-	
+
 	pca_outw(adap, I2C_PCA_DAT, addr);
 
 	sta &= ~(I2C_PCA_CON_STO|I2C_PCA_CON_STA|I2C_PCA_CON_SI);
@@ -124,7 +124,7 @@
  *
  * Returns after the byte has been transmitted
  */
-static void pca_tx_byte(struct i2c_algo_pca_data *adap, 
+static void pca_tx_byte(struct i2c_algo_pca_data *adap,
 			__u8 b)
 {
 	int sta = pca_get_con(adap);
@@ -142,19 +142,19 @@
  *
  * returns immediately.
  */
-static void pca_rx_byte(struct i2c_algo_pca_data *adap, 
+static void pca_rx_byte(struct i2c_algo_pca_data *adap,
 			__u8 *b, int ack)
 {
 	*b = pca_inw(adap, I2C_PCA_DAT);
 	DEB2("=== READ %#04x %s\n", *b, ack ? "ACK" : "NACK");
 }
 
-/* 
+/*
  * Setup ACK or NACK for next received byte and wait for it to arrive.
  *
  * Returns after next byte has arrived.
  */
-static void pca_rx_ack(struct i2c_algo_pca_data *adap, 
+static void pca_rx_ack(struct i2c_algo_pca_data *adap,
 		       int ack)
 {
 	int sta = pca_get_con(adap);
@@ -168,8 +168,8 @@
 	pca_wait(adap);
 }
 
-/* 
- * Reset the i2c bus / SIO 
+/*
+ * Reset the i2c bus / SIO
  */
 static void pca_reset(struct i2c_algo_pca_data *adap)
 {
@@ -203,14 +203,14 @@
 		for (curmsg = 0; curmsg < num; curmsg++) {
 			int addr, i;
 			msg = &msgs[curmsg];
-			
+
 			addr = (0x7f & msg->addr) ;
-		
+
 			if (msg->flags & I2C_M_RD )
-				printk(KERN_INFO "    [%02d] RD %d bytes from %#02x [%#02x, ...]\n", 
+				printk(KERN_INFO "    [%02d] RD %d bytes from %#02x [%#02x, ...]\n",
 				       curmsg, msg->len, addr, (addr<<1) | 1);
 			else {
-				printk(KERN_INFO "    [%02d] WR %d bytes to %#02x [%#02x%s", 
+				printk(KERN_INFO "    [%02d] WR %d bytes to %#02x [%#02x%s",
 				       curmsg, msg->len, addr, addr<<1,
 				       msg->len == 0 ? "" : ", ");
 				for(i=0; i < msg->len; i++)
@@ -237,7 +237,7 @@
 		case 0x10: /* A repeated start condition has been transmitted */
 			pca_address(adap, msg);
 			break;
-			
+
 		case 0x18: /* SLA+W has been transmitted; ACK has been received */
 		case 0x28: /* Data byte in I2CDAT has been transmitted; ACK has been received */
 			if (numbytes < msg->len) {
@@ -287,7 +287,7 @@
 		case 0x38: /* Arbitration lost during SLA+W, SLA+R or data bytes */
 			DEB2("Arbitration lost\n");
 			goto out;
-			
+
 		case 0x58: /* Data byte has been received; NOT ACK has been returned */
 			if ( numbytes == msg->len - 1 ) {
 				pca_rx_byte(adap, &msg->buf[numbytes], 0);
@@ -320,13 +320,13 @@
 			printk(KERN_ERR DRIVER ": unhandled SIO state 0x%02x\n", state);
 			break;
 		}
-		
+
 	}
 
 	ret = curmsg;
  out:
 	DEB1(KERN_CRIT "}}} transfered %d/%d messages. "
-	     "status is %#04x. control is %#04x\n", 
+	     "status is %#04x. control is %#04x\n",
 	     curmsg, num, pca_status(adap),
 	     pca_get_con(adap));
 	return ret;
@@ -350,7 +350,7 @@
 	pca_outw(adap, I2C_PCA_ADR, own << 1);
 
 	pca_set_con(adap, I2C_PCA_CON_ENSIO | clock);
-	udelay(500); /* 500 µs for oscilator to stabilise */
+	udelay(500); /* 500 us for oscilator to stabilise */
 
 	return 0;
 }
@@ -360,8 +360,8 @@
 	.functionality	= pca_func,
 };
 
-/* 
- * registering functions to load algorithms at runtime 
+/*
+ * registering functions to load algorithms at runtime
  */
 int i2c_pca_add_bus(struct i2c_adapter *adap)
 {
Index: linux-playground/drivers/i2c/busses/i2c-pca-isa.c
===================================================================
--- linux-playground.orig/drivers/i2c/busses/i2c-pca-isa.c	2008-02-06 20:15:38.000000000 +0100
+++ linux-playground/drivers/i2c/busses/i2c-pca-isa.c	2008-02-06 21:20:01.000000000 +0100
@@ -78,7 +78,7 @@
 	int res = inb(base+reg);
 #ifdef DEBUG_IO
 	{
-		static char *names[] = { "STA", "DAT", "ADR", "CON" };	
+		static char *names[] = { "STA", "DAT", "ADR", "CON" };
 		printk("*** read  %s => %#04x\n", names[reg], res);
 	}
 #endif
@@ -93,7 +93,7 @@
 		ret = wait_event_interruptible(pca_wait,
 					       pca_isa_readbyte(adap, I2C_PCA_CON) & I2C_PCA_CON_SI);
 	} else {
-		while ((pca_isa_readbyte(adap, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0) 
+		while ((pca_isa_readbyte(adap, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
 			udelay(100);
 	}
 	return ret;

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry


[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCHv2 2/3] Extend the PCA9564-algorithm and adapt its only user (pca-isa).

[Wolfram Sang]

[-- Attachment #1: i2c-pca-algo_extensions.diff --]
[-- Type: text/plain, Size: 15229 bytes --]

The separation between algorithm and adapter was unsharp at places. This was
partly hidden by the fact, that the ISA-driver allowed just one instance and
had all private data in static variables. This patch makes neccessary
preparations to add a platform driver on top of the algorithm, while still
supporting ISA. Note: Due to lack of hardware, the ISA-driver could not be
tested except that it builds.

Concerning the core struct i2c_algo_pca_data:

- A private data field was added, all hardware dependant data may go here.
  Similar to other algorithms, now a pointer to this data is passed to the
  adapter's functions. In order to make as less changes as possible to the
  ISA-driver, it leaves the private data empty and still only uses its static
  variables.

- A "reset_chip" function pointer was added; such a functionality must come
  from the adapter, not the algorithm.

- use a variable "i2c_clock" instead of a function pointer "get_clock",
  allowing for write access to a default in case a wrong value was supplied.

In the algorithm-file:

- move "i2c-pca-algo.h" into "linux/i2c-algo-pca.h"
- now using per_instance timeout values (i2c_adap->timeout)
- error messages specify the device, not only the driver name
- restructure initialization to easily support "i2c_add_numbered_adapter"
- drop "retries" and "own" (i2c address) as they were unused

(The state-machine for I2C-communication was not touched.)

In the ISA-driver:

- adapt to new algorithm
- updated tests to variable "irq" to the convention that 0 is NO_IRQ

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
 drivers/i2c/algos/i2c-algo-pca.c |   87 +++++++++++++++++++--------------------
 drivers/i2c/algos/i2c-algo-pca.h |   26 -----------
 drivers/i2c/busses/i2c-pca-isa.c |   59 ++++++++++----------------
 include/linux/i2c-algo-pca.h     |   37 ++++++++++++++--
 4 files changed, 98 insertions(+), 111 deletions(-)

Index: linux-playground/include/linux/i2c-algo-pca.h
===================================================================
--- linux-playground.orig/include/linux/i2c-algo-pca.h	2008-02-06 20:15:37.000000000 +0100
+++ linux-playground/include/linux/i2c-algo-pca.h	2008-02-06 20:15:48.000000000 +0100
@@ -1,14 +1,41 @@
 #ifndef _LINUX_I2C_ALGO_PCA_H
 #define _LINUX_I2C_ALGO_PCA_H
 
+/* Clock speeds for the bus */
+#define I2C_PCA_CON_330kHz	0x00
+#define I2C_PCA_CON_288kHz	0x01
+#define I2C_PCA_CON_217kHz	0x02
+#define I2C_PCA_CON_146kHz	0x03
+#define I2C_PCA_CON_88kHz	0x04
+#define I2C_PCA_CON_59kHz	0x05
+#define I2C_PCA_CON_44kHz	0x06
+#define I2C_PCA_CON_36kHz	0x07
+
+/* PCA9564 registers */
+#define I2C_PCA_STA		0x00 /* STATUS  Read Only  */
+#define I2C_PCA_TO		0x00 /* TIMEOUT Write Only */
+#define I2C_PCA_DAT		0x01 /* DATA    Read/Write */
+#define I2C_PCA_ADR		0x02 /* OWN ADR Read/Write */
+#define I2C_PCA_CON		0x03 /* CONTROL Read/Write */
+
+#define I2C_PCA_CON_AA		0x80 /* Assert Acknowledge */
+#define I2C_PCA_CON_ENSIO	0x40 /* Enable */
+#define I2C_PCA_CON_STA		0x20 /* Start */
+#define I2C_PCA_CON_STO		0x10 /* Stop */
+#define I2C_PCA_CON_SI		0x08 /* Serial Interrupt */
+#define I2C_PCA_CON_CR		0x07 /* Clock Rate (MASK) */
+
 struct i2c_algo_pca_data {
-	int  (*get_own)			(struct i2c_algo_pca_data *adap); /* Obtain own address */
-	int  (*get_clock)		(struct i2c_algo_pca_data *adap);
-	void (*write_byte)		(struct i2c_algo_pca_data *adap, int reg, int val);
-	int  (*read_byte)		(struct i2c_algo_pca_data *adap, int reg);
-	int  (*wait_for_interrupt)	(struct i2c_algo_pca_data *adap);
+	void 				*data;	/* private low level data */
+	void (*write_byte)		(void *data, int reg, int val);
+	int  (*read_byte)		(void *data, int reg);
+	int  (*wait_for_completion)	(void *data);
+	void (*reset_chip)		(void *data);
+	/* i2c_clock values are defined in linux/i2c-algo-pca.h */
+	unsigned int			i2c_clock;
 };
 
 int i2c_pca_add_bus(struct i2c_adapter *);
+int i2c_pca_add_numbered_bus(struct i2c_adapter *);
 
 #endif /* _LINUX_I2C_ALGO_PCA_H */
Index: linux-playground/drivers/i2c/algos/i2c-algo-pca.h
===================================================================
--- linux-playground.orig/drivers/i2c/algos/i2c-algo-pca.h	2008-02-06 20:15:37.000000000 +0100
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,26 +0,0 @@
-#ifndef I2C_PCA9564_H
-#define I2C_PCA9564_H 1
-
-#define I2C_PCA_STA		0x00 /* STATUS  Read Only  */
-#define I2C_PCA_TO		0x00 /* TIMEOUT Write Only */
-#define I2C_PCA_DAT		0x01 /* DATA    Read/Write */
-#define I2C_PCA_ADR		0x02 /* OWN ADR Read/Write */
-#define I2C_PCA_CON		0x03 /* CONTROL Read/Write */
-
-#define I2C_PCA_CON_AA		0x80 /* Assert Acknowledge */
-#define I2C_PCA_CON_ENSIO	0x40 /* Enable */
-#define I2C_PCA_CON_STA		0x20 /* Start */
-#define I2C_PCA_CON_STO		0x10 /* Stop */
-#define I2C_PCA_CON_SI		0x08 /* Serial Interrupt */
-#define I2C_PCA_CON_CR		0x07 /* Clock Rate (MASK) */
-
-#define I2C_PCA_CON_330kHz	0x00
-#define I2C_PCA_CON_288kHz	0x01
-#define I2C_PCA_CON_217kHz	0x02
-#define I2C_PCA_CON_146kHz	0x03
-#define I2C_PCA_CON_88kHz	0x04
-#define I2C_PCA_CON_59kHz	0x05
-#define I2C_PCA_CON_44kHz	0x06
-#define I2C_PCA_CON_36kHz	0x07
-
-#endif /* I2C_PCA9564_H */
Index: linux-playground/drivers/i2c/algos/i2c-algo-pca.c
===================================================================
--- linux-playground.orig/drivers/i2c/algos/i2c-algo-pca.c	2008-02-06 20:15:41.000000000 +0100
+++ linux-playground/drivers/i2c/algos/i2c-algo-pca.c	2008-02-06 20:15:48.000000000 +0100
@@ -1,6 +1,7 @@
 /*
  *  i2c-algo-pca.c i2c driver algorithms for PCA9564 adapters
  *    Copyright (C) 2004 Arcom Control Systems
+ *    Copyright (C) 2008 Pengutronix
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -21,14 +22,10 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/delay.h>
-#include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-pca.h>
-#include "i2c-algo-pca.h"
-
-#define DRIVER "i2c-algo-pca"
 
 #define DEB1(fmt, args...) do { if (i2c_debug>=1) printk(fmt, ## args); } while(0)
 #define DEB2(fmt, args...) do { if (i2c_debug>=2) printk(fmt, ## args); } while(0)
@@ -36,15 +33,15 @@
 
 static int i2c_debug;
 
-#define pca_outw(adap, reg, val) adap->write_byte(adap, reg, val)
-#define pca_inw(adap, reg) adap->read_byte(adap, reg)
+#define pca_outw(adap, reg, val) adap->write_byte(adap->data, reg, val)
+#define pca_inw(adap, reg) adap->read_byte(adap->data, reg)
 
 #define pca_status(adap) pca_inw(adap, I2C_PCA_STA)
-#define pca_clock(adap) adap->get_clock(adap)
-#define pca_own(adap) adap->get_own(adap)
+#define pca_clock(adap) adap->i2c_clock
 #define pca_set_con(adap, val) pca_outw(adap, I2C_PCA_CON, val)
 #define pca_get_con(adap) pca_inw(adap, I2C_PCA_CON)
-#define pca_wait(adap) adap->wait_for_interrupt(adap)
+#define pca_wait(adap) adap->wait_for_completion(adap->data)
+#define pca_reset(adap) adap->reset_chip(adap->data)
 
 /*
  * Generate a start condition on the i2c bus.
@@ -168,15 +165,6 @@
 	pca_wait(adap);
 }
 
-/*
- * Reset the i2c bus / SIO
- */
-static void pca_reset(struct i2c_algo_pca_data *adap)
-{
-	/* apparently only an external reset will do it. not a lot can be done */
-	printk(KERN_ERR DRIVER ": Haven't figured out how to do a reset yet\n");
-}
-
 static int pca_xfer(struct i2c_adapter *i2c_adap,
                     struct i2c_msg *msgs,
                     int num)
@@ -187,7 +175,7 @@
 	int numbytes = 0;
 	int state;
 	int ret;
-	int timeout = 100;
+	int timeout = i2c_adap->timeout;
 
 	while ((state = pca_status(adap)) != 0xf8 && timeout--) {
 		msleep(10);
@@ -317,7 +305,7 @@
 			pca_reset(adap);
 			goto out;
 		default:
-			printk(KERN_ERR DRIVER ": unhandled SIO state 0x%02x\n", state);
+			dev_err(&i2c_adap->dev, "unhandled SIO state 0x%02x\n", state);
 			break;
 		}
 
@@ -337,53 +325,64 @@
         return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
 }
 
-static int pca_init(struct i2c_algo_pca_data *adap)
+static const struct i2c_algorithm pca_algo = {
+	.master_xfer	= pca_xfer,
+	.functionality	= pca_func,
+};
+
+static int pca_init(struct i2c_adapter *adap)
 {
 	static int freqs[] = {330,288,217,146,88,59,44,36};
-	int own, clock;
+	int clock;
+	struct i2c_algo_pca_data *pca_data = adap->algo_data;
+
+	if (pca_data->i2c_clock > 7) {
+		dev_warn(&adap->dev, "Invalid I2C clock speed selected. Trying default.\n");
+		pca_data->i2c_clock = I2C_PCA_CON_59kHz;
+	}
+
+	adap->algo = &pca_algo;
 
-	own = pca_own(adap);
-	clock = pca_clock(adap);
-	DEB1(KERN_INFO DRIVER ": own address is %#04x\n", own);
-	DEB1(KERN_INFO DRIVER ": clock freqeuncy is %dkHz\n", freqs[clock]);
+	pca_reset(pca_data);
 
-	pca_outw(adap, I2C_PCA_ADR, own << 1);
+	clock = pca_clock(pca_data);
+	DEB1(KERN_INFO "%s: Clock frequency is %dkHz\n", adap->name, freqs[clock]);
 
-	pca_set_con(adap, I2C_PCA_CON_ENSIO | clock);
+	pca_set_con(pca_data, I2C_PCA_CON_ENSIO | clock);
 	udelay(500); /* 500 us for oscilator to stabilise */
 
 	return 0;
 }
 
-static const struct i2c_algorithm pca_algo = {
-	.master_xfer	= pca_xfer,
-	.functionality	= pca_func,
-};
-
 /*
  * registering functions to load algorithms at runtime
  */
 int i2c_pca_add_bus(struct i2c_adapter *adap)
 {
-	struct i2c_algo_pca_data *pca_adap = adap->algo_data;
 	int rval;
 
-	/* register new adapter to i2c module... */
-	adap->algo = &pca_algo;
+	rval = pca_init(adap);
+	if (rval)
+		return rval;
 
-	adap->timeout = 100;		/* default values, should	*/
-	adap->retries = 3;		/* be replaced by defines	*/
+	return i2c_add_adapter(adap);
+}
+EXPORT_SYMBOL(i2c_pca_add_bus);
 
-	if ((rval = pca_init(pca_adap)))
-		return rval;
+int i2c_pca_add_numbered_bus(struct i2c_adapter *adap)
+{
+	int rval;
 
-	rval = i2c_add_adapter(adap);
+	rval = pca_init(adap);
+	if (rval)
+		return rval;
 
-	return rval;
+	return i2c_add_numbered_adapter(adap);
 }
-EXPORT_SYMBOL(i2c_pca_add_bus);
+EXPORT_SYMBOL(i2c_pca_add_numbered_bus);
 
-MODULE_AUTHOR("Ian Campbell <icampbell-2sJRl1BP9u0AvxtiuMwx3w@public.gmane.org>");
+MODULE_AUTHOR("Ian Campbell <icampbell-2sJRl1BP9u0AvxtiuMwx3w@public.gmane.org>, "
+	"Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
 MODULE_DESCRIPTION("I2C-Bus PCA9564 algorithm");
 MODULE_LICENSE("GPL");
 
Index: linux-playground/drivers/i2c/busses/i2c-pca-isa.c
===================================================================
--- linux-playground.orig/drivers/i2c/busses/i2c-pca-isa.c	2008-02-06 20:15:41.000000000 +0100
+++ linux-playground/drivers/i2c/busses/i2c-pca-isa.c	2008-02-06 20:15:48.000000000 +0100
@@ -1,6 +1,7 @@
 /*
  *  i2c-pca-isa.c driver for PCA9564 on ISA boards
  *    Copyright (C) 2004 Arcom Control Systems
+ *    Copyright (C) 2008 Pengutronix
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -22,11 +23,9 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/delay.h>
-#include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/wait.h>
-
 #include <linux/isa.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-pca.h>
@@ -34,13 +33,9 @@
 #include <asm/io.h>
 #include <asm/irq.h>
 
-#include "../algos/i2c-algo-pca.h"
-
+#define DRIVER "i2c-pca-isa"
 #define IO_SIZE 4
 
-#undef DEBUG_IO
-//#define DEBUG_IO
-
 static unsigned long base   = 0x330;
 static int irq 	  = 10;
 
@@ -48,22 +43,9 @@
  * in the actual clock rate */
 static int clock  = I2C_PCA_CON_59kHz;
 
-static int own    = 0x55;
-
 static wait_queue_head_t pca_wait;
 
-static int pca_isa_getown(struct i2c_algo_pca_data *adap)
-{
-	return (own);
-}
-
-static int pca_isa_getclock(struct i2c_algo_pca_data *adap)
-{
-	return (clock);
-}
-
-static void
-pca_isa_writebyte(struct i2c_algo_pca_data *adap, int reg, int val)
+static void pca_isa_writebyte(void *pd, int reg, int val)
 {
 #ifdef DEBUG_IO
 	static char *names[] = { "T/O", "DAT", "ADR", "CON" };
@@ -72,8 +54,7 @@
 	outb(val, base+reg);
 }
 
-static int
-pca_isa_readbyte(struct i2c_algo_pca_data *adap, int reg)
+static int pca_isa_readbyte(void *pd, int reg)
 {
 	int res = inb(base+reg);
 #ifdef DEBUG_IO
@@ -85,31 +66,37 @@
 	return res;
 }
 
-static int pca_isa_waitforinterrupt(struct i2c_algo_pca_data *adap)
+static int pca_isa_waitforcompletion(void *pd)
 {
 	int ret = 0;
 
-	if (irq > -1) {
+	if (irq) {
 		ret = wait_event_interruptible(pca_wait,
-					       pca_isa_readbyte(adap, I2C_PCA_CON) & I2C_PCA_CON_SI);
+					       pca_isa_readbyte(pd, I2C_PCA_CON) & I2C_PCA_CON_SI);
 	} else {
-		while ((pca_isa_readbyte(adap, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
+		while ((pca_isa_readbyte(pd, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
 			udelay(100);
 	}
 	return ret;
 }
 
+static void pca_isa_resetchip(void *pd)
+{
+	/* apparently only an external reset will do it. not a lot can be done */
+	printk(KERN_WARNING DRIVER ": Haven't figured out how to do a reset yet\n");
+}
+
 static irqreturn_t pca_handler(int this_irq, void *dev_id) {
 	wake_up_interruptible(&pca_wait);
 	return IRQ_HANDLED;
 }
 
 static struct i2c_algo_pca_data pca_isa_data = {
-	.get_own		= pca_isa_getown,
-	.get_clock		= pca_isa_getclock,
+	/* .data intentionally left NULL, not needed with ISA */
 	.write_byte		= pca_isa_writebyte,
 	.read_byte		= pca_isa_readbyte,
-	.wait_for_interrupt	= pca_isa_waitforinterrupt,
+	.wait_for_completion	= pca_isa_waitforcompletion,
+	.reset_chip		= pca_isa_resetchip,
 };
 
 static struct i2c_adapter pca_isa_ops = {
@@ -117,6 +104,7 @@
 	.id		= I2C_HW_A_ISA,
 	.algo_data	= &pca_isa_data,
 	.name		= "PCA9564 ISA Adapter",
+	.timeout	= 100,
 };
 
 static int __devinit pca_isa_probe(struct device *dev, unsigned int id)
@@ -130,13 +118,14 @@
 		goto out;
 	}
 
-	if (irq > -1) {
+	if (irq) {
 		if (request_irq(irq, pca_handler, 0, "i2c-pca-isa", &pca_isa_ops) < 0) {
 			dev_err(dev, "Request irq%d failed\n", irq);
 			goto out_region;
 		}
 	}
 
+	pca_isa_data.i2c_clock = clock;
 	if (i2c_pca_add_bus(&pca_isa_ops) < 0) {
 		dev_err(dev, "Failed to add i2c bus\n");
 		goto out_irq;
@@ -145,7 +134,7 @@
 	return 0;
 
  out_irq:
-	if (irq > -1)
+	if (irq)
 		free_irq(irq, &pca_isa_ops);
  out_region:
 	release_region(base, IO_SIZE);
@@ -157,7 +146,7 @@
 {
 	i2c_del_adapter(&pca_isa_ops);
 
-	if (irq > 0) {
+	if (irq) {
 		disable_irq(irq);
 		free_irq(irq, &pca_isa_ops);
 	}
@@ -171,7 +160,7 @@
 	.remove		= __devexit_p(pca_isa_remove),
 	.driver = {
 		.owner	= THIS_MODULE,
-		.name	= "i2c-pca-isa",
+		.name	= DRIVER,
 	}
 };
 
@@ -197,7 +186,5 @@
 module_param(clock, int, 0);
 MODULE_PARM_DESC(clock, "Clock rate as described in table 1 of PCA9564 datasheet");
 
-module_param(own, int, 0); /* the driver can't do slave mode, so there's no real point in this */
-
 module_init(pca_isa_init);
 module_exit(pca_isa_exit);

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Wolfram Sang]

[-- Attachment #1: i2c-pca-platform_driver.diff --]
[-- Type: text/plain, Size: 9801 bytes --]

Tested on a blackfin.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
 drivers/i2c/busses/Kconfig            |   11 +
 drivers/i2c/busses/Makefile           |    1 
 drivers/i2c/busses/i2c-pca-platform.c |  278 ++++++++++++++++++++++++++++++++++
 include/linux/i2c-pca-platform.h      |   12 +
 4 files changed, 302 insertions(+)

Index: linux-playground/drivers/i2c/busses/i2c-pca-platform.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/drivers/i2c/busses/i2c-pca-platform.c	2008-02-06 20:15:54.000000000 +0100
@@ -0,0 +1,278 @@
+/*
+ *  i2c_pca_platform.c
+ *
+ *  Platform driver for the PCA9564 I2C controller.
+ *
+ *  Copyright (C) 2008 Pengutronix
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/i2c-algo-pca.h>
+#include <linux/i2c-pca-platform.h>
+
+#include <asm/irq.h>
+#include <asm/io.h>
+
+#ifdef GENERIC_GPIO
+#include <asm/gpio.h>
+#endif
+
+#define res_len(r)		((r)->end - (r)->start + 1)
+
+struct i2c_pca_pf_data {
+	void __iomem			*reg_base;
+	int				irq;	/* if 0, use polling */
+	wait_queue_head_t		wait;
+	struct i2c_adapter		adap;
+	struct i2c_algo_pca_data	algo_data;
+	unsigned long			io_base;
+	unsigned long			io_size;
+};
+
+/* Read/Write functions for different register alignments */
+
+static int i2c_pca_pf_readbyte8(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg);
+}
+
+static int i2c_pca_pf_readbyte16(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 2);
+}
+
+static int i2c_pca_pf_readbyte32(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 4);
+}
+
+static void i2c_pca_pf_writebyte8(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg);
+}
+
+static void i2c_pca_pf_writebyte16(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 2);
+}
+
+static void i2c_pca_pf_writebyte32(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 4);
+}
+
+
+static int i2c_pca_pf_waitforcompletion(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	int ret = 0;
+
+	if (i2c->irq) {
+		ret = wait_event_interruptible(i2c->wait,
+			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+			& I2C_PCA_CON_SI);
+	} else {
+		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+				& I2C_PCA_CON_SI) == 0)
+			udelay(100);
+	}
+
+	return ret;
+}
+
+static void i2c_pca_pf_dummyreset(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	dev_warn(&i2c->adap.dev, "No reset-pin found. Chip may get stuck!\n");
+}
+
+#ifdef GENERIC_GPIO
+static void i2c_pca_pf_resetchip(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	struct i2c_pca9564_pf_platform_data *platform_data =
+			i2c->adap.dev.parent->platform_data;
+
+	gpio_set_value(platform_data->gpio, 0);
+	ndelay(100);
+	gpio_set_value(platform_data->gpio, 1);
+}
+#endif
+
+static irqreturn_t i2c_pca_pf_handler(int this_irq, void *dev_id)
+{
+	struct i2c_pca_pf_data *i2c = dev_id;
+
+	if ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
+		return IRQ_NONE;
+
+	wake_up_interruptible(&i2c->wait);
+
+	return IRQ_HANDLED;
+}
+
+
+static int i2c_pca_pf_probe(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c;
+	struct resource *res;
+	struct i2c_pca9564_pf_platform_data *platform_data =
+				pdev->dev.platform_data;
+	int ret;
+	int irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	/* If irq is 0, we do polling. */
+
+	if (res == NULL)
+		return -ENODEV;
+
+	if (!request_mem_region(res->start, res_len(res), res->name))
+		return -ENOMEM;
+
+	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
+	if (!i2c) {
+		ret = -ENOMEM;
+		goto e_alloc;
+	}
+
+	init_waitqueue_head(&i2c->wait);
+
+	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+
+	i2c->adap.owner   = THIS_MODULE;
+	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "i2c-pca9564.%u",
+		pdev->id);
+	i2c->adap.algo_data 	= &i2c->algo_data;
+	i2c->adap.dev.parent 	= &pdev->dev;
+	i2c->adap.timeout	= platform_data->timeout;
+
+	i2c->reg_base = ioremap(res->start, res_len(res));
+	if (!i2c->reg_base) {
+		ret = -EIO;
+		goto e_remap;
+	}
+	i2c->io_base	= res->start;
+	i2c->io_size	= res_len(res);
+	i2c->irq	= irq;
+
+	i2c->algo_data.i2c_clock	= platform_data->i2c_clock_speed;
+	i2c->algo_data.data		= i2c;
+
+	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
+	case IORESOURCE_MEM_32BIT:
+		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte32;
+		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte32;
+		break;
+	case IORESOURCE_MEM_16BIT:
+		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte16;
+		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte16;
+		break;
+	case IORESOURCE_MEM_8BIT:
+	default:
+		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte8;
+		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte8;
+		break;
+	}
+
+	i2c->algo_data.wait_for_completion	= i2c_pca_pf_waitforcompletion;
+
+#ifdef GENERIC_GPIO
+	/* Use NO_GPIO if this macro is in kernel somwhen? */
+	if (platform_data->gpio > -1)
+		i2c->algo_data.reset_chip	= i2c_pca_pf_resetchip;
+	else
+		i2c->algo_data.reset_chip	= i2c_pca_pf_dummyreset;
+#else
+	i2c->algo_data.reset_chip	= i2c_pca_pf_dummyreset;
+#endif
+	if (irq) {
+		ret = request_irq(irq, i2c_pca_pf_handler,
+			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
+		if (ret)
+			goto e_reqirq;
+	}
+
+	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
+		dev_err(&i2c->adap.dev, "Failed to add PCA9564\n");
+		goto e_adapt;
+	}
+
+	platform_set_drvdata(pdev, i2c);
+
+	dev_info(&i2c->adap.dev, "PCA9564 registered.\n");
+	return 0;
+
+e_adapt:
+	if (irq)
+		free_irq(irq, i2c);
+e_reqirq:
+	iounmap(i2c->reg_base);
+e_remap:
+	kfree(i2c);
+e_alloc:
+	release_mem_region(res->start, res_len(res));
+	return ret;
+}
+
+static int i2c_pca_pf_remove(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c = platform_get_drvdata(pdev);
+	platform_set_drvdata(pdev, NULL);
+
+	i2c_del_adapter(&i2c->adap);
+
+	if (i2c->irq)
+		free_irq(i2c->irq, i2c);
+
+	iounmap(i2c->reg_base);
+	release_mem_region(i2c->io_base, i2c->io_size);
+	kfree(i2c);
+
+	return 0;
+}
+
+static struct platform_driver i2c_pca_pf_driver = {
+	.probe		= i2c_pca_pf_probe,
+	.remove		= __devexit_p(i2c_pca_pf_remove),
+	.driver		= {
+		.name	= "pca9564_platform",
+	},
+};
+
+static int __init i2c_pca_pf_init(void)
+{
+	return platform_driver_register(&i2c_pca_pf_driver);
+}
+
+static void __exit i2c_pca_pf_exit(void)
+{
+	platform_driver_unregister(&i2c_pca_pf_driver);
+}
+
+MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_DESCRIPTION("I2C-PCA9564 platform driver");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_pca_pf_init);
+module_exit(i2c_pca_pf_exit);
+
Index: linux-playground/drivers/i2c/busses/Kconfig
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Kconfig	2008-02-06 20:15:36.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Kconfig	2008-02-06 20:15:54.000000000 +0100
@@ -641,6 +641,17 @@
 	  delays when I2C/SMBus chip drivers are loaded (e.g. at boot
 	  time).  If unsure, say N.
 
+config I2C_PCA_PLATFORM
+	tristate "PCA9564 as platform device"
+	select I2C_ALGOPCA
+	default n
+	help
+	  This driver supports a memory mapped Philips PCA 9564
+	  Parallel bus to I2C bus controller
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-pca-platform.
+
 config I2C_MV64XXX
 	tristate "Marvell mv64xxx I2C Controller"
 	depends on (MV64X60 || ARCH_ORION) && EXPERIMENTAL
Index: linux-playground/drivers/i2c/busses/Makefile
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Makefile	2008-02-06 20:15:36.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Makefile	2008-02-06 20:15:55.000000000 +0100
@@ -30,6 +30,7 @@
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
 obj-$(CONFIG_I2C_PASEMI)	+= i2c-pasemi.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
+obj-$(CONFIG_I2C_PCA_PLATFORM)	+= i2c-pca-platform.o
 obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
 obj-$(CONFIG_I2C_PMCMSP)	+= i2c-pmcmsp.o
 obj-$(CONFIG_I2C_PNX)		+= i2c-pnx.o
Index: linux-playground/include/linux/i2c-pca-platform.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/include/linux/i2c-pca-platform.h	2008-02-06 20:15:55.000000000 +0100
@@ -0,0 +1,12 @@
+#ifndef I2C_PCA9564_PLATFORM_H
+#define I2C_PCA9564_PLATFORM_H
+
+struct i2c_pca9564_pf_platform_data {
+	int gpio;		/* pin to reset chip. driver will work when
+				 * not supplied (negative value), but it
+				 * cannot exit some error conditions then */
+	int i2c_clock_speed;	/* values are defined in linux/i2c-algo-pca.h */
+	int timeout;		/* timeout = this value * 10us */
+};
+
+#endif /* I2C_PCA9564_PLATFORM_H */

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 1/3] Remove trailing whitespaces and unnecessary UTF

[Jean Delvare]

Hi Wolfram,

On Wed, 06 Feb 2008 21:20:57 +0100, Wolfram Sang wrote:
> Remove trailing whitespaces to make further patches more readable.  Also remove
> an unnecessary UTF-char for simplicity ("us" for microseconds is fine enough).
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied, thanks.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 2/3] Extend the PCA9564-algorithm and adapt its only user (pca-isa).

[Jean Delvare]

Hi Wolfram,

On Wed, 06 Feb 2008 21:20:58 +0100, Wolfram Sang wrote:
> The separation between algorithm and adapter was unsharp at places. This was
> partly hidden by the fact, that the ISA-driver allowed just one instance and
> had all private data in static variables. This patch makes neccessary
> preparations to add a platform driver on top of the algorithm, while still
> supporting ISA. Note: Due to lack of hardware, the ISA-driver could not be
> tested except that it builds.
> 
> Concerning the core struct i2c_algo_pca_data:
> 
> - A private data field was added, all hardware dependant data may go here.
>   Similar to other algorithms, now a pointer to this data is passed to the
>   adapter's functions. In order to make as less changes as possible to the
>   ISA-driver, it leaves the private data empty and still only uses its static
>   variables.
> 
> - A "reset_chip" function pointer was added; such a functionality must come
>   from the adapter, not the algorithm.
> 
> - use a variable "i2c_clock" instead of a function pointer "get_clock",
>   allowing for write access to a default in case a wrong value was supplied.
> 
> In the algorithm-file:
> 
> - move "i2c-pca-algo.h" into "linux/i2c-algo-pca.h"
> - now using per_instance timeout values (i2c_adap->timeout)
> - error messages specify the device, not only the driver name
> - restructure initialization to easily support "i2c_add_numbered_adapter"
> - drop "retries" and "own" (i2c address) as they were unused
> 
> (The state-machine for I2C-communication was not touched.)
> 
> In the ISA-driver:
> 
> - adapt to new algorithm
> - updated tests to variable "irq" to the convention that 0 is NO_IRQ

I'm fine with everything except this. I'm not saying that the changes in
irq tests aren't correct (the original code looks weird) but this is a
functional change, unrelated with the rest of your patch. So if you
really want to change it, that must be a separate patch.

No need to resend, I've reverted these changes myself. Patch applied,
thanks.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

Hi Wolfram,

On Wed, 06 Feb 2008 21:20:59 +0100, Wolfram Sang wrote:
> Tested on a blackfin.
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> ---
>  drivers/i2c/busses/Kconfig            |   11 +
>  drivers/i2c/busses/Makefile           |    1 
>  drivers/i2c/busses/i2c-pca-platform.c |  278 ++++++++++++++++++++++++++++++++++
>  include/linux/i2c-pca-platform.h      |   12 +
>  4 files changed, 302 insertions(+)
> 
> Index: linux-playground/drivers/i2c/busses/i2c-pca-platform.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-playground/drivers/i2c/busses/i2c-pca-platform.c	2008-02-06 20:15:54.000000000 +0100
> @@ -0,0 +1,278 @@
> +/*
> + *  i2c_pca_platform.c
> + *
> + *  Platform driver for the PCA9564 I2C controller.
> + *
> + *  Copyright (C) 2008 Pengutronix
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c-algo-pca.h>
> +#include <linux/i2c-pca-platform.h>
> +
> +#include <asm/irq.h>
> +#include <asm/io.h>
> +
> +#ifdef GENERIC_GPIO
> +#include <asm/gpio.h>
> +#endif
> +
> +#define res_len(r)		((r)->end - (r)->start + 1)
> +
> +struct i2c_pca_pf_data {
> +	void __iomem			*reg_base;
> +	int				irq;	/* if 0, use polling */
> +	wait_queue_head_t		wait;
> +	struct i2c_adapter		adap;
> +	struct i2c_algo_pca_data	algo_data;
> +	unsigned long			io_base;
> +	unsigned long			io_size;
> +};
> +
> +/* Read/Write functions for different register alignments */
> +
> +static int i2c_pca_pf_readbyte8(void *pd, int reg)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	return ioread8(i2c->reg_base + reg);
> +}
> +
> +static int i2c_pca_pf_readbyte16(void *pd, int reg)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	return ioread8(i2c->reg_base + reg * 2);

Shouldn't this be ioread16?

> +}
> +
> +static int i2c_pca_pf_readbyte32(void *pd, int reg)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	return ioread8(i2c->reg_base + reg * 4);
> +}

And ioread32?

> +
> +static void i2c_pca_pf_writebyte8(void *pd, int reg, int val)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	iowrite8(val, i2c->reg_base + reg);
> +}
> +
> +static void i2c_pca_pf_writebyte16(void *pd, int reg, int val)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	iowrite8(val, i2c->reg_base + reg * 2);
> +}
> +
> +static void i2c_pca_pf_writebyte32(void *pd, int reg, int val)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	iowrite8(val, i2c->reg_base + reg * 4);
> +}
> +
> +
> +static int i2c_pca_pf_waitforcompletion(void *pd)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	int ret = 0;
> +
> +	if (i2c->irq) {
> +		ret = wait_event_interruptible(i2c->wait,
> +			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> +			& I2C_PCA_CON_SI);
> +	} else {
> +		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> +				& I2C_PCA_CON_SI) == 0)
> +			udelay(100);

No timeout?

> +	}
> +
> +	return ret;
> +}
> +
> +static void i2c_pca_pf_dummyreset(void *pd)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	dev_warn(&i2c->adap.dev, "No reset-pin found. Chip may get stuck!\n");
> +}
> +
> +#ifdef GENERIC_GPIO
> +static void i2c_pca_pf_resetchip(void *pd)
> +{
> +	struct i2c_pca_pf_data *i2c = pd;
> +	struct i2c_pca9564_pf_platform_data *platform_data =
> +			i2c->adap.dev.parent->platform_data;
> +
> +	gpio_set_value(platform_data->gpio, 0);

The i2c_clock gets to be copied to the driver data structure, but for
the gpio you have to fetch it from the platform device data? Not very
consistent.

> +	ndelay(100);
> +	gpio_set_value(platform_data->gpio, 1);
> +}
> +#endif
> +
> +static irqreturn_t i2c_pca_pf_handler(int this_irq, void *dev_id)
> +{
> +	struct i2c_pca_pf_data *i2c = dev_id;
> +
> +	if ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
> +		return IRQ_NONE;
> +
> +	wake_up_interruptible(&i2c->wait);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +
> +static int i2c_pca_pf_probe(struct platform_device *pdev)
> +{
> +	struct i2c_pca_pf_data *i2c;
> +	struct resource *res;
> +	struct i2c_pca9564_pf_platform_data *platform_data =
> +				pdev->dev.platform_data;
> +	int ret;
> +	int irq;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	irq = platform_get_irq(pdev, 0);
> +	/* If irq is 0, we do polling. */
> +
> +	if (res == NULL)
> +		return -ENODEV;
> +
> +	if (!request_mem_region(res->start, res_len(res), res->name))
> +		return -ENOMEM;
> +
> +	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
> +	if (!i2c) {
> +		ret = -ENOMEM;
> +		goto e_alloc;
> +	}
> +
> +	init_waitqueue_head(&i2c->wait);
> +
> +	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +
> +	i2c->adap.owner   = THIS_MODULE;

No alignment in code please.

> +	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "i2c-pca9564.%u",
> +		pdev->id);

That's a bit confusing given that the driver isn't named i2c-pca9564.
Other drivers usually include the address to distinguish between
multiple device for example (..., "PCA9564 adapter at %04lx",
res->start).

> +	i2c->adap.algo_data 	= &i2c->algo_data;
> +	i2c->adap.dev.parent 	= &pdev->dev;
> +	i2c->adap.timeout	= platform_data->timeout;
> +
> +	i2c->reg_base = ioremap(res->start, res_len(res));
> +	if (!i2c->reg_base) {
> +		ret = -EIO;
> +		goto e_remap;
> +	}
> +	i2c->io_base	= res->start;
> +	i2c->io_size	= res_len(res);
> +	i2c->irq	= irq;
> +
> +	i2c->algo_data.i2c_clock	= platform_data->i2c_clock_speed;
> +	i2c->algo_data.data		= i2c;
> +
> +	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
> +	case IORESOURCE_MEM_32BIT:
> +		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte32;
> +		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte32;
> +		break;
> +	case IORESOURCE_MEM_16BIT:
> +		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte16;
> +		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte16;
> +		break;
> +	case IORESOURCE_MEM_8BIT:
> +	default:
> +		i2c->algo_data.write_byte	= i2c_pca_pf_writebyte8;
> +		i2c->algo_data.read_byte	= i2c_pca_pf_readbyte8;
> +		break;
> +	}
> +
> +	i2c->algo_data.wait_for_completion	= i2c_pca_pf_waitforcompletion;
> +
> +#ifdef GENERIC_GPIO
> +	/* Use NO_GPIO if this macro is in kernel somwhen? */

No "somwhen" in my dictionary.

> +	if (platform_data->gpio > -1)
> +		i2c->algo_data.reset_chip	= i2c_pca_pf_resetchip;
> +	else
> +		i2c->algo_data.reset_chip	= i2c_pca_pf_dummyreset;
> +#else
> +	i2c->algo_data.reset_chip	= i2c_pca_pf_dummyreset;
> +#endif
> +	if (irq) {
> +		ret = request_irq(irq, i2c_pca_pf_handler,
> +			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
> +		if (ret)
> +			goto e_reqirq;
> +	}
> +
> +	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
> +		dev_err(&i2c->adap.dev, "Failed to add PCA9564\n");
> +		goto e_adapt;
> +	}
> +
> +	platform_set_drvdata(pdev, i2c);
> +
> +	dev_info(&i2c->adap.dev, "PCA9564 registered.\n");
> +	return 0;
> +
> +e_adapt:
> +	if (irq)
> +		free_irq(irq, i2c);
> +e_reqirq:
> +	iounmap(i2c->reg_base);
> +e_remap:
> +	kfree(i2c);
> +e_alloc:
> +	release_mem_region(res->start, res_len(res));
> +	return ret;
> +}
> +
> +static int i2c_pca_pf_remove(struct platform_device *pdev)

Missing __devexit.

> +{
> +	struct i2c_pca_pf_data *i2c = platform_get_drvdata(pdev);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	i2c_del_adapter(&i2c->adap);
> +
> +	if (i2c->irq)
> +		free_irq(i2c->irq, i2c);
> +
> +	iounmap(i2c->reg_base);
> +	release_mem_region(i2c->io_base, i2c->io_size);
> +	kfree(i2c);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver i2c_pca_pf_driver = {
> +	.probe		= i2c_pca_pf_probe,
> +	.remove		= __devexit_p(i2c_pca_pf_remove),
> +	.driver		= {
> +		.name	= "pca9564_platform",

Missing .owner = THIS_MODULE.

> +	},
> +};
> +
> +static int __init i2c_pca_pf_init(void)
> +{
> +	return platform_driver_register(&i2c_pca_pf_driver);
> +}
> +
> +static void __exit i2c_pca_pf_exit(void)
> +{
> +	platform_driver_unregister(&i2c_pca_pf_driver);
> +}
> +
> +MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_DESCRIPTION("I2C-PCA9564 platform driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i2c_pca_pf_init);
> +module_exit(i2c_pca_pf_exit);
> +
> Index: linux-playground/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux-playground.orig/drivers/i2c/busses/Kconfig	2008-02-06 20:15:36.000000000 +0100
> +++ linux-playground/drivers/i2c/busses/Kconfig	2008-02-06 20:15:54.000000000 +0100
> @@ -641,6 +641,17 @@
>  	  delays when I2C/SMBus chip drivers are loaded (e.g. at boot
>  	  time).  If unsure, say N.
>  
> +config I2C_PCA_PLATFORM
> +	tristate "PCA9564 as platform device"
> +	select I2C_ALGOPCA
> +	default n
> +	help
> +	  This driver supports a memory mapped Philips PCA 9564

For consistency, no space between "PCA" and "9564".

> +	  Parallel bus to I2C bus controller

Lowercase P, missing trailing dot.

> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-pca-platform.
> +
>  config I2C_MV64XXX
>  	tristate "Marvell mv64xxx I2C Controller"
>  	depends on (MV64X60 || ARCH_ORION) && EXPERIMENTAL
> Index: linux-playground/drivers/i2c/busses/Makefile
> ===================================================================
> --- linux-playground.orig/drivers/i2c/busses/Makefile	2008-02-06 20:15:36.000000000 +0100
> +++ linux-playground/drivers/i2c/busses/Makefile	2008-02-06 20:15:55.000000000 +0100
> @@ -30,6 +30,7 @@
>  obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
>  obj-$(CONFIG_I2C_PASEMI)	+= i2c-pasemi.o
>  obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
> +obj-$(CONFIG_I2C_PCA_PLATFORM)	+= i2c-pca-platform.o
>  obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
>  obj-$(CONFIG_I2C_PMCMSP)	+= i2c-pmcmsp.o
>  obj-$(CONFIG_I2C_PNX)		+= i2c-pnx.o
> Index: linux-playground/include/linux/i2c-pca-platform.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-playground/include/linux/i2c-pca-platform.h	2008-02-06 20:15:55.000000000 +0100
> @@ -0,0 +1,12 @@
> +#ifndef I2C_PCA9564_PLATFORM_H
> +#define I2C_PCA9564_PLATFORM_H
> +
> +struct i2c_pca9564_pf_platform_data {

"pf" is redundant with "platform", isn't it?

> +	int gpio;		/* pin to reset chip. driver will work when
> +				 * not supplied (negative value), but it
> +				 * cannot exit some error conditions then */
> +	int i2c_clock_speed;	/* values are defined in linux/i2c-algo-pca.h */
> +	int timeout;		/* timeout = this value * 10us */

A rather curious time unit if you ask me.

> +};
> +
> +#endif /* I2C_PCA9564_PLATFORM_H */
> 


-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Wolfram Sang]

Hi Jean,

nearly a month since your reply, I am sorry. Work was shifting in a 
different direction meanwhile.

(Everything I did not comment on is already fixed as suggested)

>> +/* Read/Write functions for different register alignments */
>> +
>> +static int i2c_pca_pf_readbyte8(void *pd, int reg)
>> +{
>> +	struct i2c_pca_pf_data *i2c = pd;
>> +	return ioread8(i2c->reg_base + reg);
>> +}
>> +
>> +static int i2c_pca_pf_readbyte16(void *pd, int reg)
>> +{
>> +	struct i2c_pca_pf_data *i2c = pd;
>> +	return ioread8(i2c->reg_base + reg * 2);
> 
> Shouldn't this be ioread16?
I don't think so. The registers get scattered differently in the 
address-room, but their size itself is still 8 bit. I expect that 
ioread8 gives me just those 8 bits I want, hiding that some busses 
actually do 32-bit accesses only. Am I wrong?

>> +	if (i2c->irq) {
>> +		ret = wait_event_interruptible(i2c->wait,
>> +			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
>> +			& I2C_PCA_CON_SI);
>> +	} else {
>> +		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
>> +				& I2C_PCA_CON_SI) == 0)
>> +			udelay(100);
> 
> No timeout?
You got a point there. The thing is that the underlying pca-algorithm 
does not have a timeout when expecting this condition (interrupt bit 
set). Even when using interrupts instead of polling, it will just sleep 
forever, if there goes something _really_ wrong. I could copy at least 
this behaviour by replacing the udelay(100); with an msleep(x) with x 
being a module parameter. I hope this is good enough, connecting the IRQ 
is preferred anyhow. (I am afraid that I don't have the time to add a 
sane and tested timeout mechanism to the algorithm)

> The i2c_clock gets to be copied to the driver data structure, but for
> the gpio you have to fetch it from the platform device data? Not very
> consistent.
ACK.

>> +	i2c->adap.owner   = THIS_MODULE;
> No alignment in code please.
Really? Well, if you say so...

> That's a bit confusing given that the driver isn't named i2c-pca9564.
> Other drivers usually include the address to distinguish between
> multiple device for example (..., "PCA9564 adapter at %04lx",
> res->start).
Okay, will look for something better.

>> +struct i2c_pca9564_pf_platform_data {
> "pf" is redundant with "platform", isn't it?
My intention was that 'i2c_pca9564_pf_' is the prefix for everything 
related to this platform-driver, and platform_data happens to be 
...well... the platform data :)

>> +	int timeout;		/* timeout = this value * 10us */
> A rather curious time unit if you ask me.
Given by the algorithm. I didn't question it.

Best wishes,

    Wolfram

-- 
   Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

Hi Wolfgang,

On Fri, 07 Mar 2008 16:52:05 +0100, Wolfram Sang wrote:
> nearly a month since your reply, I am sorry. Work was shifting in a 
> different direction meanwhile.

No problem, I have been pretty busy myself.

> (Everything I did not comment on is already fixed as suggested)
> 
> >> +/* Read/Write functions for different register alignments */
> >> +
> >> +static int i2c_pca_pf_readbyte8(void *pd, int reg)
> >> +{
> >> +	struct i2c_pca_pf_data *i2c = pd;
> >> +	return ioread8(i2c->reg_base + reg);
> >> +}
> >> +
> >> +static int i2c_pca_pf_readbyte16(void *pd, int reg)
> >> +{
> >> +	struct i2c_pca_pf_data *i2c = pd;
> >> +	return ioread8(i2c->reg_base + reg * 2);
> > 
> > Shouldn't this be ioread16?
> I don't think so. The registers get scattered differently in the 
> address-room, but their size itself is still 8 bit. I expect that 
> ioread8 gives me just those 8 bits I want, hiding that some busses 
> actually do 32-bit accesses only. Am I wrong?

I really don't know, I am not familiar with the hardware. Leave it as
is, we'll fix it later if someone complains.

> >> +	if (i2c->irq) {
> >> +		ret = wait_event_interruptible(i2c->wait,
> >> +			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> >> +			& I2C_PCA_CON_SI);
> >> +	} else {
> >> +		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> >> +				& I2C_PCA_CON_SI) == 0)
> >> +			udelay(100);
> > 
> > No timeout?
> You got a point there. The thing is that the underlying pca-algorithm 
> does not have a timeout when expecting this condition (interrupt bit 
> set). Even when using interrupts instead of polling, it will just sleep 
> forever, if there goes something _really_ wrong. I could copy at least 
> this behaviour by replacing the udelay(100); with an msleep(x) with x 
> being a module parameter. I hope this is good enough, connecting the IRQ 
> is preferred anyhow. (I am afraid that I don't have the time to add a 
> sane and tested timeout mechanism to the algorithm)

I'd rather not make the driver more complex for a mode you do not want
to encourage anyway. Leave the code as is, once again we can improve it
later if really needed.

Waiting for an updated patch, then I'll push it to -mm. Patches 1/3 and
2/3 are already there.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 11797 bytes --]


On Sat, Mar 08, 2008 at 11:13:37AM +0100, Jean Delvare wrote:

> Hi Wolfgang,
"Wolfram" please... :)

> Waiting for an updated patch, then I'll push it to -mm. Patches 1/3 and
> 2/3 are already there.
Attached. It works on Blackfin and builds on x86 and x86-64. Hopefully
everything is caught now...

All the best,

   Wolfram

===

Subject: Add platform driver on top of the new pca-algorithm
From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Changes since last revision:
 - check against CONFIG_GENERIC_GPIO (was GENERIC_GPIO :( )
 - don't use platform data anymore, copy all over to own struct
 - give info about mem & irq when booting (and switch to printk
   as device is not yet registered)
 - added comment about problems with polling
 - added proper __devinit and __devexit
 - added owner to module
 - removed whitespace alignment in code
 - driver now named "i2c-pca-platform" (as the module)
 - fixed typos in Kconfig

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
===

Tested on a blackfin.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
 drivers/i2c/busses/Kconfig            |   15 +
 drivers/i2c/busses/Makefile           |    1 
 drivers/i2c/busses/i2c-pca-platform.c |  297 ++++++++++++++++++++++++++++++++++
 include/linux/i2c-pca-platform.h      |   12 +
 4 files changed, 323 insertions(+), 2 deletions(-)

Index: linux-playground/drivers/i2c/busses/i2c-pca-platform.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/drivers/i2c/busses/i2c-pca-platform.c	2008-03-10 12:00:22.000000000 +0100
@@ -0,0 +1,297 @@
+/*
+ *  i2c_pca_platform.c
+ *
+ *  Platform driver for the PCA9564 I2C controller.
+ *
+ *  Copyright (C) 2008 Pengutronix
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/i2c-algo-pca.h>
+#include <linux/i2c-pca-platform.h>
+
+#include <asm/irq.h>
+#include <asm/io.h>
+
+#ifdef CONFIG_GENERIC_GPIO
+#include <asm/gpio.h>
+#endif
+
+#define res_len(r)		((r)->end - (r)->start + 1)
+
+struct i2c_pca_pf_data {
+	void __iomem			*reg_base;
+	int				irq;	/* if 0, use polling */
+#ifdef CONFIG_GENERIC_GPIO
+	int				gpio;
+#endif
+	wait_queue_head_t		wait;
+	struct i2c_adapter		adap;
+	struct i2c_algo_pca_data	algo_data;
+	unsigned long			io_base;
+	unsigned long			io_size;
+};
+
+/* Read/Write functions for different register alignments */
+
+static int i2c_pca_pf_readbyte8(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg);
+}
+
+static int i2c_pca_pf_readbyte16(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 2);
+}
+
+static int i2c_pca_pf_readbyte32(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 4);
+}
+
+static void i2c_pca_pf_writebyte8(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg);
+}
+
+static void i2c_pca_pf_writebyte16(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 2);
+}
+
+static void i2c_pca_pf_writebyte32(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 4);
+}
+
+
+static int i2c_pca_pf_waitforcompletion(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	int ret = 0;
+
+	if (i2c->irq) {
+		ret = wait_event_interruptible(i2c->wait,
+			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+			& I2C_PCA_CON_SI);
+	} else {
+		/*
+		 * Do polling...
+		 * XXX: Could get stuck in extreme cases!
+		 *      Maybe add timeout, but using irqs is preferred anyhow.
+		 */
+		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+				& I2C_PCA_CON_SI) == 0)
+			udelay(100);
+	}
+
+	return ret;
+}
+
+static void i2c_pca_pf_dummyreset(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	printk(KERN_WARNING "%s: No reset-pin found. Chip may get stuck!\n",
+		i2c->adap.name);
+}
+
+#ifdef CONFIG_GENERIC_GPIO
+static void i2c_pca_pf_resetchip(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+
+	gpio_set_value(i2c->gpio, 0);
+	ndelay(100);
+	gpio_set_value(i2c->gpio, 1);
+}
+#endif
+
+static irqreturn_t i2c_pca_pf_handler(int this_irq, void *dev_id)
+{
+	struct i2c_pca_pf_data *i2c = dev_id;
+
+	if ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
+		return IRQ_NONE;
+
+	wake_up_interruptible(&i2c->wait);
+
+	return IRQ_HANDLED;
+}
+
+
+static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c;
+	struct resource *res;
+	struct i2c_pca9564_pf_platform_data *platform_data =
+				pdev->dev.platform_data;
+	int ret;
+	int irq;
+
+	printk(KERN_INFO "Registering PCA9564 ");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	/* If irq is 0, we do polling. */
+
+	if (res == NULL) {
+		ret = -ENODEV;
+		goto e_print;
+	}
+
+	if (!request_mem_region(res->start, res_len(res), res->name)) {
+		ret = -ENOMEM;
+		goto e_print;
+	}
+
+	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
+	if (!i2c) {
+		ret = -ENOMEM;
+		goto e_alloc;
+	}
+
+	init_waitqueue_head(&i2c->wait);
+
+	i2c->reg_base = ioremap(res->start, res_len(res));
+	if (!i2c->reg_base) {
+		ret = -EIO;
+		goto e_remap;
+	}
+	i2c->io_base = res->start;
+	i2c->io_size = res_len(res);
+	i2c->irq = irq;
+
+	printk("at 0x%08x ", res->start);
+
+	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.owner = THIS_MODULE;
+	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "PCA9564 at 0x%08x",
+		res->start);
+	i2c->adap.algo_data = &i2c->algo_data;
+	i2c->adap.dev.parent = &pdev->dev;
+	i2c->adap.timeout = platform_data->timeout;
+
+	i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
+	i2c->algo_data.data = i2c;
+
+	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
+	case IORESOURCE_MEM_32BIT:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte32;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte32;
+		break;
+	case IORESOURCE_MEM_16BIT:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte16;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte16;
+		break;
+	case IORESOURCE_MEM_8BIT:
+	default:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte8;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte8;
+		break;
+	}
+
+	i2c->algo_data.wait_for_completion = i2c_pca_pf_waitforcompletion;
+
+#ifdef CONFIG_GENERIC_GPIO
+	/* Use NO_GPIO when this macro is in mainline */
+	i2c->gpio = platform_data->gpio;
+	if (platform_data->gpio > -1)
+		i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
+	else
+		i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
+#else
+	i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
+#endif
+	if (irq) {
+		ret = request_irq(irq, i2c_pca_pf_handler,
+			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
+		if (ret)
+			goto e_reqirq;
+	}
+
+	printk("using irq %d ", irq);
+
+	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
+		ret = -ENODEV;
+		goto e_adapt;
+	}
+
+	platform_set_drvdata(pdev, i2c);
+
+	printk("succeeded.\n");
+	return 0;
+
+e_adapt:
+	if (irq)
+		free_irq(irq, i2c);
+e_reqirq:
+	iounmap(i2c->reg_base);
+e_remap:
+	kfree(i2c);
+e_alloc:
+	release_mem_region(res->start, res_len(res));
+e_print:
+	printk("FAILED! (%d)\n", ret);
+	return ret;
+}
+
+static int __devexit i2c_pca_pf_remove(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c = platform_get_drvdata(pdev);
+	platform_set_drvdata(pdev, NULL);
+
+	i2c_del_adapter(&i2c->adap);
+
+	if (i2c->irq)
+		free_irq(i2c->irq, i2c);
+
+	iounmap(i2c->reg_base);
+	release_mem_region(i2c->io_base, i2c->io_size);
+	kfree(i2c);
+
+	return 0;
+}
+
+static struct platform_driver i2c_pca_pf_driver = {
+	.probe = i2c_pca_pf_probe,
+	.remove = __devexit_p(i2c_pca_pf_remove),
+	.driver = {
+		.name = "i2c-pca-platform",
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init i2c_pca_pf_init(void)
+{
+	return platform_driver_register(&i2c_pca_pf_driver);
+}
+
+static void __exit i2c_pca_pf_exit(void)
+{
+	platform_driver_unregister(&i2c_pca_pf_driver);
+}
+
+MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_DESCRIPTION("I2C-PCA9564 platform driver");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_pca_pf_init);
+module_exit(i2c_pca_pf_exit);
+
Index: linux-playground/drivers/i2c/busses/Kconfig
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Kconfig	2008-03-10 11:30:06.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Kconfig	2008-03-10 11:31:57.000000000 +0100
@@ -632,8 +632,8 @@
 	select I2C_ALGOPCA
 	default n
 	help
-	  This driver supports ISA boards using the Philips PCA 9564
-	  Parallel bus to I2C bus controller
+	  This driver supports ISA boards using the Philips PCA9564
+	  parallel bus to I2C bus controller.
 	  
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-pca-isa.
@@ -643,6 +643,17 @@
 	  delays when I2C/SMBus chip drivers are loaded (e.g. at boot
 	  time).  If unsure, say N.
 
+config I2C_PCA_PLATFORM
+	tristate "PCA9564 as platform device"
+	select I2C_ALGOPCA
+	default n
+	help
+	  This driver supports a memory mapped Philips PCA9564
+	  parallel bus to I2C bus controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-pca-platform.
+
 config I2C_MV64XXX
 	tristate "Marvell mv64xxx I2C Controller"
 	depends on (MV64X60 || ARCH_ORION) && EXPERIMENTAL
Index: linux-playground/drivers/i2c/busses/Makefile
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Makefile	2008-03-10 11:30:06.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Makefile	2008-03-10 11:31:57.000000000 +0100
@@ -30,6 +30,7 @@
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
 obj-$(CONFIG_I2C_PASEMI)	+= i2c-pasemi.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
+obj-$(CONFIG_I2C_PCA_PLATFORM)	+= i2c-pca-platform.o
 obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
 obj-$(CONFIG_I2C_PMCMSP)	+= i2c-pmcmsp.o
 obj-$(CONFIG_I2C_PNX)		+= i2c-pnx.o
Index: linux-playground/include/linux/i2c-pca-platform.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/include/linux/i2c-pca-platform.h	2008-03-10 11:31:57.000000000 +0100
@@ -0,0 +1,12 @@
+#ifndef I2C_PCA9564_PLATFORM_H
+#define I2C_PCA9564_PLATFORM_H
+
+struct i2c_pca9564_pf_platform_data {
+	int gpio;		/* pin to reset chip. driver will work when
+				 * not supplied (negative value), but it
+				 * cannot exit some error conditions then */
+	int i2c_clock_speed;	/* values are defined in linux/i2c-algo-pca.h */
+	int timeout;		/* timeout = this value * 10us */
+};
+
+#endif /* I2C_PCA9564_PLATFORM_H */

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

Hi Wolfram,

On Mon, 10 Mar 2008 12:26:40 +0100, Wolfram Sang wrote:
> 
> On Sat, Mar 08, 2008 at 11:13:37AM +0100, Jean Delvare wrote:
> 
> > Hi Wolfgang,
> "Wolfram" please... :)
> 
> > Waiting for an updated patch, then I'll push it to -mm. Patches 1/3 and
> > 2/3 are already there.
> Attached. It works on Blackfin and builds on x86 and x86-64. Hopefully
> everything is caught now...
> 
> All the best,
> 
>    Wolfram
> 
> ===
> 
> Subject: Add platform driver on top of the new pca-algorithm
> From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Changes since last revision:
>  - check against CONFIG_GENERIC_GPIO (was GENERIC_GPIO :( )
>  - don't use platform data anymore, copy all over to own struct
>  - give info about mem & irq when booting (and switch to printk
>    as device is not yet registered)

While the i2c_adapter device doesn't exist before i2c_add_adapter() is
called, the underlying platform device exists as soon as probe() is
entered.

>  - added comment about problems with polling
>  - added proper __devinit and __devexit
>  - added owner to module
>  - removed whitespace alignment in code
>  - driver now named "i2c-pca-platform" (as the module)
>  - fixed typos in Kconfig
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ===
> 
> Tested on a blackfin.
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> ---
>  drivers/i2c/busses/Kconfig            |   15 +
>  drivers/i2c/busses/Makefile           |    1 
>  drivers/i2c/busses/i2c-pca-platform.c |  297 ++++++++++++++++++++++++++++++++++
>  include/linux/i2c-pca-platform.h      |   12 +
>  4 files changed, 323 insertions(+), 2 deletions(-)

I'm almost happy now, except for:

> +static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
> +{
> +	struct i2c_pca_pf_data *i2c;
> +	struct resource *res;
> +	struct i2c_pca9564_pf_platform_data *platform_data =
> +				pdev->dev.platform_data;
> +	int ret;
> +	int irq;
> +
> +	printk(KERN_INFO "Registering PCA9564 ");

Do not split printks that way. Other kernel threads might write to the
log buffer as well and then the log is totally messed up. You must
always call printk with complete lines.

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	irq = platform_get_irq(pdev, 0);
> +	/* If irq is 0, we do polling. */
> +
> +	if (res == NULL) {
> +		ret = -ENODEV;
> +		goto e_print;
> +	}
> +
> +	if (!request_mem_region(res->start, res_len(res), res->name)) {
> +		ret = -ENOMEM;
> +		goto e_print;
> +	}
> +
> +	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
> +	if (!i2c) {
> +		ret = -ENOMEM;
> +		goto e_alloc;
> +	}
> +
> +	init_waitqueue_head(&i2c->wait);
> +
> +	i2c->reg_base = ioremap(res->start, res_len(res));
> +	if (!i2c->reg_base) {
> +		ret = -EIO;
> +		goto e_remap;
> +	}
> +	i2c->io_base = res->start;
> +	i2c->io_size = res_len(res);
> +	i2c->irq = irq;
> +
> +	printk("at 0x%08x ", res->start);
> +
> +	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +	i2c->adap.owner = THIS_MODULE;
> +	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "PCA9564 at 0x%08x",
> +		res->start);
> +	i2c->adap.algo_data = &i2c->algo_data;
> +	i2c->adap.dev.parent = &pdev->dev;
> +	i2c->adap.timeout = platform_data->timeout;
> +
> +	i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
> +	i2c->algo_data.data = i2c;
> +
> +	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
> +	case IORESOURCE_MEM_32BIT:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte32;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte32;
> +		break;
> +	case IORESOURCE_MEM_16BIT:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte16;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte16;
> +		break;
> +	case IORESOURCE_MEM_8BIT:
> +	default:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte8;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte8;
> +		break;
> +	}
> +
> +	i2c->algo_data.wait_for_completion = i2c_pca_pf_waitforcompletion;
> +
> +#ifdef CONFIG_GENERIC_GPIO
> +	/* Use NO_GPIO when this macro is in mainline */
> +	i2c->gpio = platform_data->gpio;
> +	if (platform_data->gpio > -1)
> +		i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> +	else
> +		i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
> +#else
> +	i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
> +#endif
> +	if (irq) {
> +		ret = request_irq(irq, i2c_pca_pf_handler,
> +			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
> +		if (ret)
> +			goto e_reqirq;
> +	}
> +
> +	printk("using irq %d ", irq);
> +
> +	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
> +		ret = -ENODEV;
> +		goto e_adapt;
> +	}
> +
> +	platform_set_drvdata(pdev, i2c);
> +
> +	printk("succeeded.\n");
> +	return 0;
> +
> +e_adapt:
> +	if (irq)
> +		free_irq(irq, i2c);
> +e_reqirq:
> +	iounmap(i2c->reg_base);
> +e_remap:
> +	kfree(i2c);
> +e_alloc:
> +	release_mem_region(res->start, res_len(res));
> +e_print:
> +	printk("FAILED! (%d)\n", ret);
> +	return ret;
> +}

Also see David Brownell's objections, which I second.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

Hi Wolfram:

Just noticed the following warnings on x86_64:
drivers/i2c/busses/i2c-pca-platform.c: In function "i2c_pca_pf_probe":
drivers/i2c/busses/i2c-pca-platform.c:181: warning: format "%08x" expects type "unsigned int", but argument 2 has type "resource_size_t"
drivers/i2c/busses/i2c-pca-platform.c:186: warning: format "%08x" expects type "unsigned int", but argument 4 has type "resource_size_t"
drivers/i2c/busses/i2c-pca-platform.c:186: warning: format "%08x" expects type "unsigned int", but argument 4 has type "resource_size_t"

Please fix in next iteration.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 12041 bytes --]

Hello,

I sent this patch to myself a minute ago and could apply it. Dunno what went
wrong the last time. I am very sorry (I know that patch fixing is
annoying).

All the best,

   Wolfram

---

Subject: Add platform driver on top of the new pca-algorithm
From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Changes since last revision:
 - use <linux/gpio.h> and remove #ifdefs CONFIG_GENERIC_GPIO
 - correct reference to gpio_is_valid()
 - register and setup gpio in the driver
 - just print whole lines with printk
 - removed warnings

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---

Changes since last revision:
 - check against CONFIG_GENERIC_GPIO (was GENERIC_GPIO :( )
 - don't use platform data anymore, copy all over to own struct
 - give info about mem & irq when booting (and switch to printk
   as device is not yet registered)
 - add comment about problems with polling
 - added proper __devinit and __devexit
 - added owner to module
 - removed whitespace alignment in code
 - driver now named "i2c-pca-platform" (as the module)
 - fixed typos in Kconfig

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---

Tested on a blackfin.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
 drivers/i2c/busses/Kconfig            |   15 +
 drivers/i2c/busses/Makefile           |    1 
 drivers/i2c/busses/i2c-pca-platform.c |  298 ++++++++++++++++++++++++++++++++++
 include/linux/i2c-pca-platform.h      |   12 +
 4 files changed, 324 insertions(+), 2 deletions(-)

Index: linux-playground/drivers/i2c/busses/i2c-pca-platform.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/drivers/i2c/busses/i2c-pca-platform.c	2008-03-14 14:45:34.000000000 +0100
@@ -0,0 +1,298 @@
+/*
+ *  i2c_pca_platform.c
+ *
+ *  Platform driver for the PCA9564 I2C controller.
+ *
+ *  Copyright (C) 2008 Pengutronix
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/i2c-algo-pca.h>
+#include <linux/i2c-pca-platform.h>
+#include <linux/gpio.h>
+
+#include <asm/irq.h>
+#include <asm/io.h>
+
+#define res_len(r)		((r)->end - (r)->start + 1)
+
+struct i2c_pca_pf_data {
+	void __iomem			*reg_base;
+	int				irq;	/* if 0, use polling */
+	int				gpio;
+	wait_queue_head_t		wait;
+	struct i2c_adapter		adap;
+	struct i2c_algo_pca_data	algo_data;
+	unsigned long			io_base;
+	unsigned long			io_size;
+};
+
+/* Read/Write functions for different register alignments */
+
+static int i2c_pca_pf_readbyte8(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg);
+}
+
+static int i2c_pca_pf_readbyte16(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 2);
+}
+
+static int i2c_pca_pf_readbyte32(void *pd, int reg)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	return ioread8(i2c->reg_base + reg * 4);
+}
+
+static void i2c_pca_pf_writebyte8(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg);
+}
+
+static void i2c_pca_pf_writebyte16(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 2);
+}
+
+static void i2c_pca_pf_writebyte32(void *pd, int reg, int val)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	iowrite8(val, i2c->reg_base + reg * 4);
+}
+
+
+static int i2c_pca_pf_waitforcompletion(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	int ret = 0;
+
+	if (i2c->irq) {
+		ret = wait_event_interruptible(i2c->wait,
+			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+			& I2C_PCA_CON_SI);
+	} else {
+		/*
+		 * Do polling...
+		 * XXX: Could get stuck in extreme cases!
+		 *      Maybe add timeout, but using irqs is preferred anyhow.
+		 */
+		while ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+				& I2C_PCA_CON_SI) == 0)
+			udelay(100);
+	}
+
+	return ret;
+}
+
+static void i2c_pca_pf_dummyreset(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+	printk(KERN_WARNING "%s: No reset-pin found. Chip may get stuck!\n",
+		i2c->adap.name);
+}
+
+static void i2c_pca_pf_resetchip(void *pd)
+{
+	struct i2c_pca_pf_data *i2c = pd;
+
+	gpio_set_value(i2c->gpio, 0);
+	ndelay(100);
+	gpio_set_value(i2c->gpio, 1);
+}
+
+static irqreturn_t i2c_pca_pf_handler(int this_irq, void *dev_id)
+{
+	struct i2c_pca_pf_data *i2c = dev_id;
+
+	if ((i2c->algo_data.read_byte(i2c, I2C_PCA_CON) & I2C_PCA_CON_SI) == 0)
+		return IRQ_NONE;
+
+	wake_up_interruptible(&i2c->wait);
+
+	return IRQ_HANDLED;
+}
+
+
+static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c;
+	struct resource *res;
+	struct i2c_pca9564_pf_platform_data *platform_data =
+				pdev->dev.platform_data;
+	int ret = 0;
+	int irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	/* If irq is 0, we do polling. */
+
+	if (res == NULL) {
+		ret = -ENODEV;
+		goto e_print;
+	}
+
+	if (!request_mem_region(res->start, res_len(res), res->name)) {
+		ret = -ENOMEM;
+		goto e_print;
+	}
+
+	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
+	if (!i2c) {
+		ret = -ENOMEM;
+		goto e_alloc;
+	}
+
+	init_waitqueue_head(&i2c->wait);
+
+	i2c->reg_base = ioremap(res->start, res_len(res));
+	if (!i2c->reg_base) {
+		ret = -EIO;
+		goto e_remap;
+	}
+	i2c->io_base = res->start;
+	i2c->io_size = res_len(res);
+	i2c->irq = irq;
+
+	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.owner = THIS_MODULE;
+	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "PCA9564 at 0x%08lx",
+		(unsigned long) res->start);
+	i2c->adap.algo_data = &i2c->algo_data;
+	i2c->adap.dev.parent = &pdev->dev;
+	i2c->adap.timeout = platform_data->timeout;
+
+	i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
+	i2c->algo_data.data = i2c;
+
+	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
+	case IORESOURCE_MEM_32BIT:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte32;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte32;
+		break;
+	case IORESOURCE_MEM_16BIT:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte16;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte16;
+		break;
+	case IORESOURCE_MEM_8BIT:
+	default:
+		i2c->algo_data.write_byte = i2c_pca_pf_writebyte8;
+		i2c->algo_data.read_byte = i2c_pca_pf_readbyte8;
+		break;
+	}
+
+	i2c->algo_data.wait_for_completion = i2c_pca_pf_waitforcompletion;
+
+
+	i2c->gpio = platform_data->gpio;
+	i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
+
+	/* Use gpio_is_valid() when in mainline */
+	if (i2c->gpio > -1)
+		ret = gpio_request(i2c->gpio, i2c->adap.name);
+		if (ret == 0) {
+			gpio_direction_output(i2c->gpio, 1);
+			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
+		} else {
+			printk(KERN_WARNING "%s: Registering gpio failed!\n",
+				i2c->adap.name);
+			i2c->gpio = ret;
+		}
+
+	if (irq) {
+		ret = request_irq(irq, i2c_pca_pf_handler,
+			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
+		if (ret)
+			goto e_reqirq;
+	}
+
+	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
+		ret = -ENODEV;
+		goto e_adapt;
+	}
+
+	platform_set_drvdata(pdev, i2c);
+
+	printk(KERN_INFO "%s registered.\n", i2c->adap.name);
+
+	return 0;
+
+e_adapt:
+	if (irq)
+		free_irq(irq, i2c);
+e_reqirq:
+	if (i2c->gpio > -1)
+		gpio_free(i2c->gpio);
+
+	iounmap(i2c->reg_base);
+e_remap:
+	kfree(i2c);
+e_alloc:
+	release_mem_region(res->start, res_len(res));
+e_print:
+	printk(KERN_ERR "Registering PCA9564 FAILED! (%d)\n", ret);
+	return ret;
+}
+
+static int __devexit i2c_pca_pf_remove(struct platform_device *pdev)
+{
+	struct i2c_pca_pf_data *i2c = platform_get_drvdata(pdev);
+	platform_set_drvdata(pdev, NULL);
+
+	i2c_del_adapter(&i2c->adap);
+
+	if (i2c->irq)
+		free_irq(i2c->irq, i2c);
+
+	if (i2c->gpio > -1)
+		gpio_free(i2c->gpio);
+
+	iounmap(i2c->reg_base);
+	release_mem_region(i2c->io_base, i2c->io_size);
+	kfree(i2c);
+
+	return 0;
+}
+
+static struct platform_driver i2c_pca_pf_driver = {
+	.probe = i2c_pca_pf_probe,
+	.remove = __devexit_p(i2c_pca_pf_remove),
+	.driver = {
+		.name = "i2c-pca-platform",
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init i2c_pca_pf_init(void)
+{
+	return platform_driver_register(&i2c_pca_pf_driver);
+}
+
+static void __exit i2c_pca_pf_exit(void)
+{
+	platform_driver_unregister(&i2c_pca_pf_driver);
+}
+
+MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_DESCRIPTION("I2C-PCA9564 platform driver");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_pca_pf_init);
+module_exit(i2c_pca_pf_exit);
+
Index: linux-playground/drivers/i2c/busses/Kconfig
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Kconfig	2008-03-10 11:30:06.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Kconfig	2008-03-14 10:42:06.000000000 +0100
@@ -632,8 +632,8 @@
 	select I2C_ALGOPCA
 	default n
 	help
-	  This driver supports ISA boards using the Philips PCA 9564
-	  Parallel bus to I2C bus controller
+	  This driver supports ISA boards using the Philips PCA9564
+	  parallel bus to I2C bus controller.
 	  
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-pca-isa.
@@ -643,6 +643,17 @@
 	  delays when I2C/SMBus chip drivers are loaded (e.g. at boot
 	  time).  If unsure, say N.
 
+config I2C_PCA_PLATFORM
+	tristate "PCA9564 as platform device"
+	select I2C_ALGOPCA
+	default n
+	help
+	  This driver supports a memory mapped Philips PCA9564
+	  parallel bus to I2C bus controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-pca-platform.
+
 config I2C_MV64XXX
 	tristate "Marvell mv64xxx I2C Controller"
 	depends on (MV64X60 || ARCH_ORION) && EXPERIMENTAL
Index: linux-playground/drivers/i2c/busses/Makefile
===================================================================
--- linux-playground.orig/drivers/i2c/busses/Makefile	2008-03-10 11:30:06.000000000 +0100
+++ linux-playground/drivers/i2c/busses/Makefile	2008-03-10 11:31:57.000000000 +0100
@@ -30,6 +30,7 @@
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
 obj-$(CONFIG_I2C_PASEMI)	+= i2c-pasemi.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
+obj-$(CONFIG_I2C_PCA_PLATFORM)	+= i2c-pca-platform.o
 obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
 obj-$(CONFIG_I2C_PMCMSP)	+= i2c-pmcmsp.o
 obj-$(CONFIG_I2C_PNX)		+= i2c-pnx.o
Index: linux-playground/include/linux/i2c-pca-platform.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-playground/include/linux/i2c-pca-platform.h	2008-03-10 11:31:57.000000000 +0100
@@ -0,0 +1,12 @@
+#ifndef I2C_PCA9564_PLATFORM_H
+#define I2C_PCA9564_PLATFORM_H
+
+struct i2c_pca9564_pf_platform_data {
+	int gpio;		/* pin to reset chip. driver will work when
+				 * not supplied (negative value), but it
+				 * cannot exit some error conditions then */
+	int i2c_clock_speed;	/* values are defined in linux/i2c-algo-pca.h */
+	int timeout;		/* timeout = this value * 10us */
+};
+
+#endif /* I2C_PCA9564_PLATFORM_H */
-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

On Fri, 14 Mar 2008 15:50:10 +0100, Wolfram Sang wrote:
> Hello,
> 
> I sent this patch to myself a minute ago and could apply it. Dunno what went
> wrong the last time. I am very sorry (I know that patch fixing is
> annoying).
> 
> All the best,
> 
>    Wolfram
> 
> ---
> 
> Subject: Add platform driver on top of the new pca-algorithm
> From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Changes since last revision:
>  - use <linux/gpio.h> and remove #ifdefs CONFIG_GENERIC_GPIO
>  - correct reference to gpio_is_valid()
>  - register and setup gpio in the driver
>  - just print whole lines with printk
>  - removed warnings
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> 
> Changes since last revision:
>  - check against CONFIG_GENERIC_GPIO (was GENERIC_GPIO :( )
>  - don't use platform data anymore, copy all over to own struct
>  - give info about mem & irq when booting (and switch to printk
>    as device is not yet registered)
>  - add comment about problems with polling
>  - added proper __devinit and __devexit
>  - added owner to module
>  - removed whitespace alignment in code
>  - driver now named "i2c-pca-platform" (as the module)
>  - fixed typos in Kconfig
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> 
> Tested on a blackfin.
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> ---
>  drivers/i2c/busses/Kconfig            |   15 +
>  drivers/i2c/busses/Makefile           |    1 
>  drivers/i2c/busses/i2c-pca-platform.c |  298 ++++++++++++++++++++++++++++++++++
>  include/linux/i2c-pca-platform.h      |   12 +
>  4 files changed, 324 insertions(+), 2 deletions(-)
> 
> Index: linux-playground/drivers/i2c/busses/i2c-pca-platform.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-playground/drivers/i2c/busses/i2c-pca-platform.c	2008-03-14 14:45:34.000000000 +0100
> (...)
> +static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
> +{
> +	struct i2c_pca_pf_data *i2c;
> +	struct resource *res;
> +	struct i2c_pca9564_pf_platform_data *platform_data =
> +				pdev->dev.platform_data;
> +	int ret = 0;
> +	int irq;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	irq = platform_get_irq(pdev, 0);
> +	/* If irq is 0, we do polling. */
> +
> +	if (res == NULL) {
> +		ret = -ENODEV;
> +		goto e_print;
> +	}
> +
> +	if (!request_mem_region(res->start, res_len(res), res->name)) {
> +		ret = -ENOMEM;
> +		goto e_print;
> +	}
> +
> +	i2c = kzalloc(sizeof(struct i2c_pca_pf_data), GFP_KERNEL);
> +	if (!i2c) {
> +		ret = -ENOMEM;
> +		goto e_alloc;
> +	}
> +
> +	init_waitqueue_head(&i2c->wait);
> +
> +	i2c->reg_base = ioremap(res->start, res_len(res));
> +	if (!i2c->reg_base) {
> +		ret = -EIO;
> +		goto e_remap;
> +	}
> +	i2c->io_base = res->start;
> +	i2c->io_size = res_len(res);
> +	i2c->irq = irq;
> +
> +	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +	i2c->adap.owner = THIS_MODULE;
> +	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "PCA9564 at 0x%08lx",
> +		(unsigned long) res->start);
> +	i2c->adap.algo_data = &i2c->algo_data;
> +	i2c->adap.dev.parent = &pdev->dev;
> +	i2c->adap.timeout = platform_data->timeout;
> +
> +	i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
> +	i2c->algo_data.data = i2c;
> +
> +	switch (res->flags & IORESOURCE_MEM_TYPE_MASK) {
> +	case IORESOURCE_MEM_32BIT:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte32;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte32;
> +		break;
> +	case IORESOURCE_MEM_16BIT:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte16;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte16;
> +		break;
> +	case IORESOURCE_MEM_8BIT:
> +	default:
> +		i2c->algo_data.write_byte = i2c_pca_pf_writebyte8;
> +		i2c->algo_data.read_byte = i2c_pca_pf_readbyte8;
> +		break;
> +	}
> +
> +	i2c->algo_data.wait_for_completion = i2c_pca_pf_waitforcompletion;
> +
> +

No double blank lines inside functions please (they confuse patch too
easily.)

> +	i2c->gpio = platform_data->gpio;
> +	i2c->algo_data.reset_chip = i2c_pca_pf_dummyreset;
> +
> +	/* Use gpio_is_valid() when in mainline */
> +	if (i2c->gpio > -1)
> +		ret = gpio_request(i2c->gpio, i2c->adap.name);
> +		if (ret == 0) {
> +			gpio_direction_output(i2c->gpio, 1);
> +			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> +		} else {
> +			printk(KERN_WARNING "%s: Registering gpio failed!\n",
> +				i2c->adap.name);
> +			i2c->gpio = ret;
> +		}

You're missing curly braces around this block, aren't you?

> +
> +	if (irq) {
> +		ret = request_irq(irq, i2c_pca_pf_handler,
> +			IRQF_TRIGGER_FALLING, i2c->adap.name, i2c);
> +		if (ret)
> +			goto e_reqirq;
> +	}
> +
> +	if (i2c_pca_add_numbered_bus(&i2c->adap) < 0) {
> +		ret = -ENODEV;
> +		goto e_adapt;
> +	}
> +
> +	platform_set_drvdata(pdev, i2c);
> +
> +	printk(KERN_INFO "%s registered.\n", i2c->adap.name);
> +
> +	return 0;
> +
> +e_adapt:
> +	if (irq)
> +		free_irq(irq, i2c);
> +e_reqirq:
> +	if (i2c->gpio > -1)
> +		gpio_free(i2c->gpio);
> +
> +	iounmap(i2c->reg_base);
> +e_remap:
> +	kfree(i2c);
> +e_alloc:
> +	release_mem_region(res->start, res_len(res));
> +e_print:
> +	printk(KERN_ERR "Registering PCA9564 FAILED! (%d)\n", ret);
> +	return ret;
> +}

The rest looks OK, so I can fix this myself and queue up your patch for
2.6.26.

Thanks,
-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --]

On Fri, Mar 14, 2008 at 07:46:28PM +0100, Jean Delvare wrote:

> > +	/* Use gpio_is_valid() when in mainline */
> > +	if (i2c->gpio > -1)
> > +		ret = gpio_request(i2c->gpio, i2c->adap.name);
> > +		if (ret == 0) {
> > +			gpio_direction_output(i2c->gpio, 1);
> > +			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> > +		} else {
> > +			printk(KERN_WARNING "%s: Registering gpio failed!\n",
> > +				i2c->adap.name);
> > +			i2c->gpio = ret;
> > +		}
> 
> You're missing curly braces around this block, aren't you?
Holy ..., yes, you are right. This is why I used to have braces even
around single instructions after if. This is, where CodingStyle gives me
most trouble.

> The rest looks OK, so I can fix this myself and queue up your patch for
> 2.6.26.
Ah, finally! \o/ Thanks a _lot_ for your help, I learnt a lot and this
will hopefully show in my next submission :)

Best wishes,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Jean Delvare]

On Sun, 16 Mar 2008 15:28:34 +0100, Wolfram Sang wrote:
> On Fri, Mar 14, 2008 at 07:46:28PM +0100, Jean Delvare wrote:
> 
> > > +	/* Use gpio_is_valid() when in mainline */
> > > +	if (i2c->gpio > -1)
> > > +		ret = gpio_request(i2c->gpio, i2c->adap.name);
> > > +		if (ret == 0) {
> > > +			gpio_direction_output(i2c->gpio, 1);
> > > +			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> > > +		} else {
> > > +			printk(KERN_WARNING "%s: Registering gpio failed!\n",
> > > +				i2c->adap.name);
> > > +			i2c->gpio = ret;
> > > +		}
> > 
> > You're missing curly braces around this block, aren't you?
> Holy ..., yes, you are right. This is why I used to have braces even
> around single instructions after if. This is, where CodingStyle gives me
> most trouble.

I can't agree more... This type of warning I am carefully ignoring when
checkpatch.pl complains. In many cases, the extra braces don't hurt and
protect us against future mistakes.

> > The rest looks OK, so I can fix this myself and queue up your patch for
> > 2.6.26.
>
> Ah, finally! \o/ Thanks a _lot_ for your help, I learnt a lot and this
> will hopefully show in my next submission :)

You're welcome. It has been a pleasure to work with you and I am
looking forward to your next submissions!

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Trent Piepho]

On Sun, 16 Mar 2008, Jean Delvare wrote:
> On Sun, 16 Mar 2008 15:28:34 +0100, Wolfram Sang wrote:
> > On Fri, Mar 14, 2008 at 07:46:28PM +0100, Jean Delvare wrote:
> >
> > > > +	/* Use gpio_is_valid() when in mainline */
> > > > +	if (i2c->gpio > -1)
> > > > +		ret = gpio_request(i2c->gpio, i2c->adap.name);
> > > > +		if (ret == 0) {
> > > > +			gpio_direction_output(i2c->gpio, 1);
> > > > +			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> > > > +		} else {
> > > > +			printk(KERN_WARNING "%s: Registering gpio failed!\n",
> > > > +				i2c->adap.name);
> > > > +			i2c->gpio = ret;
> > > > +		}
> > >
> > > You're missing curly braces around this block, aren't you?
> > Holy ..., yes, you are right. This is why I used to have braces even
> > around single instructions after if. This is, where CodingStyle gives me
> > most trouble.
>
> I can't agree more... This type of warning I am carefully ignoring when
> checkpatch.pl complains. In many cases, the extra braces don't hurt and
> protect us against future mistakes.

I've been bit by this exact same problem enough times that I've come to the
same conclusion.  Adding a second statement to a one-statement if and then
forgetting to add the brances almost always produces code that compiles
without warnings and almost works.  It ends up being very hard to track
down, and even when you do, it's easy to stare at right at the bug and not
see the missing braces.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c