EMMA2RH I2C driver
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
As Ralf pointed out in march I've been polishing the IIC-driver for EMMA2RH.
I've shaped up the I2C-driver to be a platform-device-driver and added accurately memory-mapping/unmapping and irq-request/free.
There was a datastructure missing which was pretty straight forward to figure out how to rebuild (i.e. i2c_algo_emma_data).
The patch (for the patch) is attached.
//Markus
- --
_______________________________________
Mr Markus Gothe
Software Engineer
Phone: +46 (0)13 21 81 20 (ext. 1046)
Fax: +46 (0)13 21 21 15
Mobile: +46 (0)73 718 72 80
Diskettgatan 11, SE-583 35 Linköping, Sweden
www.27m.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFGHfEc6I0XmJx2NrwRCMZ8AJ0QgruqOX2y+MxF/+RbpTbSciUsGwCfcH7M efI4rmnlkfxNezQbOWENjRY=
=rJPp
-----END PGP SIGNATURE-----
i2c/algos/i2c-algo-emma2rh.c renamed from i2c/algos/i2c-algo-emma2rh.c.orig +10 -9
@@ -14,7 +14,7 @@ Copyright (C) 1995-1997 Simon G. Vogl 1998-2000 Hans Berglund - With some changes from Kyösti Mälkki <[email protected]> and + With some changes from Ky�sti M�lkki <[email protected]> and Frodo Looijaard <[email protected]> ,and also from Martin Bailey <[email protected]>
@@ -51,13 +51,11 @@#include <linux/interrupt.h> #include <linux/i2c.h> -#include <linux/i2c-algo-emma2rh.h> - -#include <asm/emma2rh/emma2rh.h> +#include "i2c-algo-emma2rh.h" #ifdef DEBUG #define i2c_emma2rh_debug (level,op) do { if (i2c_debug>=(level)) { op; } } while (0) -static int i2c_debug; +static int i2c_debug =9 ;module_param (i2c_debug, int,S_IRUGO |S_IWUSR );MODULE_PARM_DESC (i2c_debug,"debug level - 0 off; 1 normal; 2,3 more verbose; 9 i2c-protocol" );
@@ -73,13 +71,15 @@#define EMMA2RH_I2C_RETRIES 3#define EMMA2RH_I2C_TIMEOUT 100 -/* --- setting states on the bus with the right timing: --------------- */ +/* --- setting states on the bus with the right timing: --------------- +*/ #define set_emma (adap, ctl, val) adap->setemma(adap->data, ctl, val)#define get_emma (adap, ctl) adap->getemma(adap->data, ctl)#define get_own (adap) adap->getown(adap->data)#define get_clock (adap) adap->getclock(adap->data) -/* --- other auxiliary functions -------------------------------------- */ +/* --- other auxiliary functions -------------------------------------- +*/ static void i2c_start (struct i2c_algo_emma_data *adap) {
@@ -168,7 +168,8 @@udelay (adap->udelay); } DEB2(if (i) - printk(KERN_DEBUG "%s: needed %d retries for %d\n", __FUNCTION__, i, addr)) ; + printk(KERN_DEBUG "%s: needed %d retries for %d\n", __FUNCTION__, +i, addr)) ;return ret; }
@@ -352,7 +353,7 @@static u32 emma_func (struct i2c_adapter *adap) { -return I2C_FUNC_SMBUS_EMUL |I2C_FUNC_PROTOCOL_MANGLING ; +return I2C_FUNC_I2C |I2C_FUNC_SMBUS_EMUL |I2C_FUNC_PROTOCOL_MANGLING ; }/* --- exported algorithm data ---------------------------------------- */
i2c/algos/i2c-algo-emma2rh.h renamed from i2c/algos/i2c-algo-emma2rh.h.orig +17 -1
@@ -17,7 +17,8 @@You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA*/ -/* -------------------------------------------------------------------- */ +/* -------------------------------------------------------------------- +*/ #ifndef I2C_EMMA2RH_H #define I2C_EMMA2RH_H
@@ -102,4 +103,19 @@#define I2C_EMMA_SHR 0x40#define I2C_EMMA_INT 0x50#define I2C_EMMA_INTM 0x60 + +struct i2c_algo_emma_data { +void *data;/* private data for lolevel routines */ +void (*setemma) (void *data,int ctl,int val); +int (*getemma) (void *data,int ctl); +int (*getown) (void *data); +int (*getclock) (void *data); +void (*waitforpin) (void *data); + +/* local settings */ +int udelay; +int timeout; + +}; +#endif /* I2C_EMMA2RH_H */
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +72 -45
193 lines not on this page · raw message
Quoted 8 lines · Markus Gothe
As Ralf pointed out in march I've been polishing the IIC-driver for EMMA2RH.
I've shaped up the I2C-driver to be a platform-device-driver and added accurately memory-mapping/unmapping and irq-request/free.
There was a datastructure missing which was pretty straight forward to figure out how to rebuild (i.e.
i2c_algo_emma_data).
Below are a few comments...
Quoted 2 lines
i2c/algos/i2c-algo-emma2rh.c renamed from i2c/algos/i2c-algo-emma2rh.c.orig +0
-0
[...]
Quoted 19 lines
i2c/algos/i2c-algo-emma2rh.c renamed from i2c/algos/i2c-algo-emma2rh.c.orig +4
-2@@ -73,13 +71,15 @@#define EMMA2RH_I2C_RETRIES 3#define EMMA2RH_I2C_TIMEOUT 100 -/* --- setting states on the bus with the right timing: --------------- */ +/* --- setting states on the bus with the right timing: --------------- +*/ #define set_emma (adap, ctl, val) adap->setemma(adap->data, ctl, val)#define get_emma (adap, ctl) adap->getemma(adap->data, ctl)#define get_own (adap) adap->getown(adap->data)#define get_clock (adap) adap->getclock(adap->data) -/* --- other auxiliary functions -------------------------------------- */ +/* --- other auxiliary functions -------------------------------------- +*/ static void i2c_start (struct i2c_algo_emma_data *adap) {
i2c/algos/i2c-algo-emma2rh.c line 172
Quoted 6 lines
i2c/algos/i2c-algo-emma2rh.c renamed from i2c/algos/i2c-algo-emma2rh.c.orig +2
-1@@ -168,7 +168,8 @@udelay (adap->udelay); } DEB2(if (i) - printk(KERN_DEBUG "%s: needed %d retries for %d\n", __FUNCTION__, i, addr)) ; + printk(KERN_DEBUG "%s: needed %d retries for %d\n", __FUNCTION__, +i, addr)) ;
Please don't "uglify" the code. If you intend to carry it to the new line indent properlu (by starting it under paren).
i2c/algos/i2c-algo-emma2rh.h line 21
Quoted 9 lines
i2c/algos/i2c-algo-emma2rh.h renamed from i2c/algos/i2c-algo-emma2rh.h.orig +2
-1@@ -17,7 +17,8 @@You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA*/ -/* -------------------------------------------------------------------- */ +/* -------------------------------------------------------------------- +*/
No need to make the comment more ugly too.
Quoted 25 lines
i2c/algos/i2c-algo-emma2rh.h renamed from i2c/algos/i2c-algo-emma2rh.h.orig +15
-0#ifndef I2C_EMMA2RH_H #define I2C_EMMA2RH_H @@ -102,4 +103,19 @@#define I2C_EMMA_SHR 0x40#define I2C_EMMA_INT 0x50#define I2C_EMMA_INTM 0x60 + +struct i2c_algo_emma_data { +void *data;/* private data for lolevel routines */ +void (*setemma) (void *data,int ctl,int val); +int (*getemma) (void *data,int ctl); +int (*getown) (void *data); +int (*getclock) (void *data); +void (*waitforpin) (void *data); + +/* local settings */ +int udelay; +int timeout; + +}; +#endif /* I2C_EMMA2RH_H */
i2c/busses/i2c-emma2rh.c line 18
Quoted 6 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +1
-1@@ -14,7 +14,7 @@ Copyright (C) 1995-97 Simon G. Vogl 1998-99 Hans Berglund - With some changes from KyУЖsti MУЄlkki <[email protected]> and even + With some changes from Kyіsti Mфlkki <[email protected]> and even Frodo Looijaard <[email protected]>
I'm not sure what that change is.
Quoted 17 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +1
-4@@ -167,81 +167,108 @@ dd->alg.getclock = i2c_emma_getclock; dd->alg.waitforpin = i2c_emma_waitforpin; dd->alg.udelay =80 ; - dd->alg.mdelay =80 ; dd->alg.timeout =200 ; -strcpy (dd->adap.name, dev->bus_id); +strcpy (dd->adap.name, pdev->name); dd->adap.id =0x00 ; dd->adap.algo =NULL ; dd->adap.algo_data = &dd->alg; dd->adap.client_register = i2c_emma_reg; dd->adap.client_unregister = i2c_emma_unreg; -spin_lock_init (&dd->lock); -
i2c/busses/i2c-emma2rh.c line 181
Quoted 10 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +2
-7-atomic_set (&dd->pending,0 ); -init_waitqueue_head (&dd->wait); - -dev_set_drvdata (dev, dd); - - r =platform_get_resource (pdev,0 ,0 ); -/* get resource of type '0' with #0 */ + r =platform_get_resource (pdev,IORESOURCE_MEM ,0 );/* get resource of type '0' with #0 */ +
Put at least one space between ; and comment, and no trailing whitespace, please.
i2c/busses/i2c-emma2rh.c line 191
Quoted 13 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +6
-3if (!r) {printk ("Cannot get resource\n" ); err = -ENODEV ; goto out_free; - } - dd->base = r->start; - + } + +if (!request_mem_region (r->start, r->end - r->start +1 , pdev->name)) { +printk ("Memory region busy\n" ); + err = -EBUSY ; + goto out_mem_region;
You've already failed to request region, why then free it?
i2c/busses/i2c-emma2rh.c line 198
Quoted 7 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +7
-0+ } + + dd->base =ioremap_nocache (r->start, r->end - r->start); +if (!dd->base) { +printk ("Unable to map registers\n" ); + err = -EIO ; + goto out_ioremap;
You've already failed to ioremap() it, why call iounmap()?
i2c/busses/i2c-emma2rh.c line 207
Quoted 10 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +3
-1+ } + dd->irq =platform_get_irq (pdev,0 ); dd->clock =FAST397 ; dd->own =0x40 + pdev->id *4 ; - err =request_irq (dd->irq, i2c_emma_handler,0 , dev->bus_id, dd); + err =request_irq (dd->irq, i2c_emma_handler,0 , pdev->name, dd);if (err <0 ) goto out_free;
And you've forgetten to call iounmap() and release_mem_region() in this case...
Quoted 17 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +10
-1+ +spin_lock_init (&dd->lock); +atomic_set (&dd->pending,0 ); +init_waitqueue_head (&dd->wait); + +platform_set_drvdata (pdev, dd); +if ((err =i2c_emma_add_bus (&dd->adap)) <0 ) goto out_irq;return 0 ; + out_irq: -free_irq (dd->irq, dev->bus_id); +free_irq (dd->irq, dd); +out_ioremap:
i2c/busses/i2c-emma2rh.c line 231
Quoted 10 lines
i2c/busses/i2c-emma2rh.c renamed from i2c/busses/i2c-emma2rh.c.orig +5
-2+iounmap (dd->base); +out_mem_region: +release_mem_region (r->start, r->end - r->start +1 ); out_free: -kfree (dd); +kfree (dd); out: -return err; +return err; }
The cleanup code needs fixing, as you can see...
WBR, Sergei