Skip to contentSkip to conversation

Four new i2c drivers and __init/__exit cleanup to i2c

PATCH New Albert Cranford 2 participants 4 messages May 19, 2005 Kbuild

lore
Share & tools
lore

Albert Cranford

author
#0 lore raw

PATCH 2/9 Four new i2c drivers and __init/__exit cleanup to i2c

This specific change set was for i2c-pport.c and 2.5.34. I propose the following changes to satisfy 2.5 kernel requirements. Of course the #include changes and MODULE_LICENSE will not be checked into CVS.

The request to abandon udelay for sleep was not addressed.

Anyone see a problem with 2.2 or 2.4 and these changes? Regards,
Albert

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +10 -27
@@ -36,12 +36,10 @@
 #include <linux/ioport.h>
 #include <asm/io.h>
 #include <linux/errno.h>
-#include "i2c.h"
-#include "i2c-algo-bit.h"
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
 
-#ifdef MODULE_LICENSE
 MODULE_LICENSE("GPL");
-#endif
 
 #define DEFAULT_BASE 0x378
 static int base=0;

@@ -112,9 +110,7 @@
 
 static int bit_pport_init(void)
 {
-       //release_region( (base+2) ,1);
-
-       if (check_region((base+2),1) < 0 ) {
+       if (request_region((base+2),1, "i2c (PPORT adapter)") < 0 ) {
                return -ENODEV; 
        } else {
 

@@ -148,11 +144,6 @@
        return 0;
 }
 
-static void __exit bit_pport_exit(void)
-{
-       release_region((base+2),1);
-}
-
 static int bit_pport_reg(struct i2c_client *client)
 {
        return 0;

@@ -231,24 +222,16 @@
 }
 
 
-EXPORT_NO_SYMBOLS;
+static void __exit i2c_bitpport_exit(void)
+{
+       i2c_bit_del_bus(&bit_pport_ops);
+       release_region((base+2),1);
+}
 
-#ifdef MODULE
 MODULE_AUTHOR("Daniel Smolik <[email protected]>");
 MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
 ;
-
 MODULE_PARM(base, "i");
 
-int init_module(void)
-{
-       return i2c_bitpport_init();
-}
-
-void cleanup_module(void)
-{
-       i2c_bit_del_bus(&bit_pport_ops);
-       bit_pport_exit();
-}
-
-#endif
+module_init(i2c_bitpport_init);
+module_exit(i2c_bitpport_exit);
Quoted 76 lines · Jeff Garzik

Review

Quoted 3 lines · Albert Cranford
(quoted diff) +3 -0
+#ifdef MODULE_LICENSE
+MODULE_LICENSE("GPL");
+#endif

kill the ifdef

Review

Quoted 5 lines
(quoted diff) +5 -0
+static int bit_pport_init(void)
+{
+     //release_region( (base+2) ,1);
+
+     if (check_region((base+2),1) < 0 ) {

wrong. race. use request_region, and check its return value. check_region should never be used.

Quoted 9 lines
(quoted diff) +9 -0
+             return -ENODEV;
+     } else {
+
+             /* test for PPORT adap.         */
+
+
+             PortData=inb(base+2);
+             PortData= (PortData SET_SDA) SET_SCL;
+             outb(PortData,base+2);

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+             if (!(inb(base+2) | 0x06)) {    /* SDA and SCL will be high     */
+                     DEBINIT(printk("i2c-pport.o: SDA and SCL was low.\n"));
+                     return -ENODEV;
+             } else {
+
+                     /*SCL high and SDA low*/
+                     PortData = PortData SET_SCL CLR_SDA;
+                     outb(PortData,base+2);
+                     udelay(400);

long udelay in process context, where you should sleep instead

Review

Quoted 12 lines
(quoted diff) +12 -0
+static void bit_pport_inc_use(struct i2c_adapter *adap)
+{
+#ifdef MODULE
+     MOD_INC_USE_COUNT;
+#endif
+}
+
+static void bit_pport_dec_use(struct i2c_adapter *adap)
+{
+#ifdef MODULE
+     MOD_DEC_USE_COUNT;
+#endif

kill the ifdef. use ->owner instead if possible.

Quoted 9 lines
(quoted diff) +9 -0
+#ifdef MODULE
+MODULE_AUTHOR("Daniel Smolik <[email protected]>");
+MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
+;
+
+MODULE_PARM(base, "i");
+
+int init_module(void)
+{

Review

Quoted 10 lines
(quoted diff) +10 -0
+     return i2c_bitpport_init();
+}
+
+void cleanup_module(void)
+{
+     i2c_bit_del_bus(&bit_pport_ops);
+     bit_pport_exit();
+}
+
+#endif

kill the ifdef, use module_init, module_exit

Signature · 2 lines
Albert Cranford Deerfield Beach FL USA
[email protected]
#1 lore raw

PATCH 2/9 Four new i2c drivers and __init/__exit cleanup to

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 line 44

Quoted 3 lines · Albert Cranford
usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +0 -2
-#ifdef MODULE_LICENSE
 MODULE_LICENSE("GPL");
-#endif

This change requires that you define MODULE_LICENSE in a kernel compatibility header, for 2.2.

It will work without such assistance in 2.4 and 2.5.

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 line 116

Quoted 13 lines

#define DEFAULT_BASE 0x378
static int base=0;

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +1 -3
@@ -112,9 +110,7 @@
 
 static int bit_pport_init(void)
 {
-       //release_region( (base+2) ,1);
-
-       if (check_region((base+2),1) < 0 ) {
+       if (request_region((base+2),1, "i2c (PPORT adapter)") < 0 ) {
                return -ENODEV; 
        } else {

request_region returns non-zero on success...

Quoted 13 lines
usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +0 -5
@@ -148,11 +144,6 @@
        return 0;
 }
 
-static void __exit bit_pport_exit(void)
-{
-       release_region((base+2),1);
-}
-
 static int bit_pport_reg(struct i2c_client *client)
 {
        return 0;

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 line 234

Quoted 4 lines
usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +0 -1
@@ -231,24 +222,16 @@
 }
 
 
-EXPORT_NO_SYMBOLS;

Why remove export-no-symbols?

Quoted 17 lines
usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +5 -6
+static void __exit i2c_bitpport_exit(void)
+{
+       i2c_bit_del_bus(&bit_pport_ops);
+       release_region((base+2),1);
+}
 
-#ifdef MODULE
 MODULE_AUTHOR("Daniel Smolik <[email protected]>");
 MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
 ;
-
 MODULE_PARM(base, "i");
 
-int init_module(void)
-{
-       return i2c_bitpport_init();
-}

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 line 237

Quoted 10 lines
usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-16 09:30:00.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +2 -8
-
-void cleanup_module(void)
-{
-       i2c_bit_del_bus(&bit_pport_ops);
-       bit_pport_exit();
-}
-
-#endif
+module_init(i2c_bitpport_init);
+module_exit(i2c_bitpport_exit);

This looks ok to me.

author
#2 lore raw

PATCH 2/9 Four new i2c drivers and __init/__exit cleanup toi2c

In reply to Albert Cranford

Hi Jeff,
I think I noted that compatibility stuff will not be checked in, but noted here for linux-2.5.xx. This is something that I have to manually ignore when diffing CVS and kernel. And yes EXPORT_NO_SYMBOLS is explicitly defined now in 2.5.xx, but no harm in redundency, so I'll leave it in the code.

I repaired the call to request_region and also removed an unneeded call to request_region from the else path.

Now I need someone to test it in the different versions.

New change set.
Albert

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-17 08:39:20.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +11 -29
@@ -36,12 +36,10 @@
 #include <linux/ioport.h>
 #include <asm/io.h>
 #include <linux/errno.h>
-#include "i2c.h"
-#include "i2c-algo-bit.h"
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
 
-#ifdef MODULE_LICENSE
 MODULE_LICENSE("GPL");
-#endif
 
 #define DEFAULT_BASE 0x378
 static int base=0;

@@ -112,9 +110,7 @@
 
 static int bit_pport_init(void)
 {
-       //release_region( (base+2) ,1);
-
-       if (check_region((base+2),1) < 0 ) {
+       if (!request_region((base+2),1, "i2c (PPORT adapter)")) {
                return -ENODEV; 
        } else {
 

@@ -140,19 +136,12 @@
                                return -ENODEV;
                        }
                }
-               request_region((base+2),1,
-                       "i2c (PPORT adapter)");
                bit_pport_setsda((void*)base,1);
                bit_pport_setscl((void*)base,1);
        }
        return 0;
 }
 
-static void __exit bit_pport_exit(void)
-{
-       release_region((base+2),1);
-}
-
 static int bit_pport_reg(struct i2c_client *client)
 {
        return 0;

@@ -231,24 +220,17 @@
 }
 
 
-EXPORT_NO_SYMBOLS;
+static void __exit i2c_bitpport_exit(void)
+{
+       i2c_bit_del_bus(&bit_pport_ops);
+       release_region((base+2),1);
+}
 
-#ifdef MODULE
+EXPORT_NO_SYMBOLS;
 MODULE_AUTHOR("Daniel Smolik <[email protected]>");
 MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
 ;
-
 MODULE_PARM(base, "i");
 
-int init_module(void)
-{
-       return i2c_bitpport_init();
-}
-
-void cleanup_module(void)
-{
-       i2c_bit_del_bus(&bit_pport_ops);
-       bit_pport_exit();
-}
-
-#endif
+module_init(i2c_bitpport_init);
+module_exit(i2c_bitpport_exit);
Quoted 76 lines · Jeff Garzik

Review

Quoted 3 lines · Albert Cranford
(quoted diff) +0 -2
-#ifdef MODULE_LICENSE
 MODULE_LICENSE("GPL");
-#endif

This change requires that you define MODULE_LICENSE in a kernel compatibility header, for 2.2.

It will work without such assistance in 2.4 and 2.5.

Review line 116

Quoted 13 lines

#define DEFAULT_BASE 0x378
static int base=0;

(quoted diff) +1 -3
@@ -112,9 +110,7 @@
 
 static int bit_pport_init(void)
 {
-       //release_region( (base+2) ,1);
-
-       if (check_region((base+2),1) < 0 ) {
+       if (request_region((base+2),1, "i2c (PPORT adapter)") < 0 ) {
                return -ENODEV;
        } else {

request_region returns non-zero on success...

Quoted 13 lines
(quoted diff) +0 -5
@@ -148,11 +144,6 @@
        return 0;
 }
 
-static void __exit bit_pport_exit(void)
-{
-       release_region((base+2),1);
-}
-
 static int bit_pport_reg(struct i2c_client *client)
 {
        return 0;

Review line 234

Quoted 4 lines
(quoted diff) +0 -1
@@ -231,24 +222,16 @@
 }
 
 
-EXPORT_NO_SYMBOLS;

Why remove export-no-symbols?

Quoted 17 lines
(quoted diff) +5 -6
+static void __exit i2c_bitpport_exit(void)
+{
+       i2c_bit_del_bus(&bit_pport_ops);
+       release_region((base+2),1);
+}
 
-#ifdef MODULE
 MODULE_AUTHOR("Daniel Smolik <[email protected]>");
 MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
 ;
-
 MODULE_PARM(base, "i");
 
-int init_module(void)
-{
-       return i2c_bitpport_init();
-}

Review line 237

Quoted 10 lines
(quoted diff) +2 -8
-
-void cleanup_module(void)
-{
-       i2c_bit_del_bus(&bit_pport_ops);
-       bit_pport_exit();
-}
-
-#endif
+module_init(i2c_bitpport_init);
+module_exit(i2c_bitpport_exit);

This looks ok to me.

Signature · 2 lines
Albert Cranford Deerfield Beach FL USA
[email protected]
#3 lore raw

PATCH 2/9 Four new i2c drivers and __init/__exit cleanup toi2c

In reply to Albert Cranford

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-17 08:39:20.000000000 -0400

Quoted 17 lines · Albert Cranford

Hi Jeff,
I think I noted that compatibility stuff will not be checked in, but noted here for linux-2.5.xx. This is something that I have to manually ignore when diffing CVS and kernel. And yes EXPORT_NO_SYMBOLS is explicitly defined now in 2.5.xx, but no harm in redundency, so I'll leave it in the code.

I repaired the call to request_region and also removed an unneeded call to request_region from the else path.

Now I need someone to test it in the different versions.

New change set.
Albert

usr/src/linux/drivers/i2c/i2c-pport.c 2002-09-17 08:39:20.000000000 -0400 renamed from i2c-pport.c 2002-09-12 02:55:47.000000000 -0400 +0 -0

Yes, this patch looks fine to me...