Skip to contentSkip to conversation

EMMA2RH I2C driver

PATCH Markus Gothe 2 participants 2 messages Apr 12, 2007 MIPS

lore
Share & tools
lore
#0 lore raw

-----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

#1 lore raw
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 -3
        if (!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