This patch updates the i2c-core locking mechanism. Most of the work was done by Ky?sti M?lkki. Original comment follows: *** Replace use of ADAP_LOCK and DRV_LOCK with down(&core_lists). In many cases we need both, some return path forgot UNLOCK, and locks were claimed in varying order and sometimes not at all. Replace use of I2C_LOCK with down(&adapter->bus). Remove unused adapter, driver and client counts. *** >From Ky?sti's original patch, I removed a few things that did not stricly belong to it. A few locks are still missing, even after applying this patch. All these changes will be merged into a more generic i2c-core code cleanup patch, to be submitted later. diff -ru linux-2.4.24-pre2-k2/drivers/i2c/i2c-core.c linux-2.4.24-pre2-k3/drivers/i2c/i2c-core.c --- linux-2.4.24-pre2-k2/drivers/i2c/i2c-core.c Tue Dec 23 19:02:05 2003 +++ linux-2.4.24-pre2-k3/drivers/i2c/i2c-core.c Wed Dec 24 19:04:55 2003 @@ -39,32 +39,14 @@ /* ----- global defines ---------------------------------------------------- */ -/* exclusive access to the bus */ -#define I2C_LOCK(adap) down(&adap->lock) -#define I2C_UNLOCK(adap) up(&adap->lock) - -#define ADAP_LOCK() down(&adap_lock) -#define ADAP_UNLOCK() up(&adap_lock) - -#define DRV_LOCK() down(&driver_lock) -#define DRV_UNLOCK() up(&driver_lock) - #define DEB(x) if (i2c_debug>=1) x; #define DEB2(x) if (i2c_debug>=2) x; /* ----- global variables -------------------------------------------------- */ -/**** lock for writing to global variables: the adapter & driver list */ -struct semaphore adap_lock; -struct semaphore driver_lock; - -/**** adapter list */ +DECLARE_MUTEX(core_lists); static struct i2c_adapter *adapters[I2C_ADAP_MAX]; -static int adap_count; - -/**** drivers list */ static struct i2c_driver *drivers[I2C_DRIVER_MAX]; -static int driver_count; /**** debug level */ static int i2c_debug=1; @@ -112,9 +94,9 @@ */ int i2c_add_adapter(struct i2c_adapter *adap) { - int i,j,res; + int i,j,res = 0; - ADAP_LOCK(); + down(&core_lists); for (i = 0; i < I2C_ADAP_MAX; i++) if (NULL == adapters[i]) break; @@ -127,11 +109,10 @@ } adapters[i] = adap; - adap_count++; - ADAP_UNLOCK(); /* init data types */ - init_MUTEX(&adap->lock); + init_MUTEX(&adap->bus); + init_MUTEX(&adap->list); #ifdef CONFIG_PROC_FS @@ -146,7 +127,7 @@ printk("i2c-core.o: Could not create /proc/bus/%s\n", name); res = -ENOENT; - goto ERROR1; + goto ERROR0; } proc_entry->proc_fops = &i2cproc_operations; @@ -157,36 +138,25 @@ #endif /* def CONFIG_PROC_FS */ /* inform drivers of new adapters */ - DRV_LOCK(); for (j=0;j<I2C_DRIVER_MAX;j++) if (drivers[j]!=NULL && (drivers[j]->flags&(I2C_DF_NOTIFY|I2C_DF_DUMMY))) /* We ignore the return code; if it fails, too bad */ drivers[j]->attach_adapter(adap); - DRV_UNLOCK(); DEB(printk(KERN_DEBUG "i2c-core.o: adapter %s registered as adapter %d.\n", adap->name,i)); - - return 0; - - -ERROR1: - ADAP_LOCK(); - adapters[i] = NULL; - adap_count--; ERROR0: - ADAP_UNLOCK(); + up(&core_lists); return res; } int i2c_del_adapter(struct i2c_adapter *adap) { - int i,j,res; - - ADAP_LOCK(); + int i,j,res = 0; + down(&core_lists); for (i = 0; i < I2C_ADAP_MAX; i++) if (adap == adapters[i]) break; @@ -202,17 +172,14 @@ * *detach* it! Of course, each dummy driver should know about * this or hell will break loose... */ - DRV_LOCK(); for (j = 0; j < I2C_DRIVER_MAX; j++) if (drivers[j] && (drivers[j]->flags & I2C_DF_DUMMY)) if ((res = drivers[j]->attach_adapter(adap))) { printk(KERN_WARNING "i2c-core.o: can't detach adapter %s " "while detaching driver %s: driver not " "detached!",adap->name,drivers[j]->name); - goto ERROR1; + goto ERROR0; } - DRV_UNLOCK(); - /* detach any active clients. This must be done first, because * it can fail; in which case we give upp. */ @@ -240,17 +207,9 @@ #endif /* def CONFIG_PROC_FS */ adapters[i] = NULL; - adap_count--; - - ADAP_UNLOCK(); DEB(printk(KERN_DEBUG "i2c-core.o: adapter unregistered: %s\n",adap->name)); - return 0; - ERROR0: - ADAP_UNLOCK(); - return res; -ERROR1: - DRV_UNLOCK(); + up(&core_lists); return res; } @@ -264,7 +223,8 @@ int i2c_add_driver(struct i2c_driver *driver) { int i; - DRV_LOCK(); + + down(&core_lists); for (i = 0; i < I2C_DRIVER_MAX; i++) if (NULL == drivers[i]) break; @@ -273,19 +233,12 @@ " i2c-core.o: register_driver(%s) " "- enlarge I2C_DRIVER_MAX.\n", driver->name); - DRV_UNLOCK(); + up(&core_lists); return -ENOMEM; } - drivers[i] = driver; - driver_count++; - - DRV_UNLOCK(); /* driver was successfully added */ - DEB(printk(KERN_DEBUG "i2c-core.o: driver %s registered.\n",driver->name)); - ADAP_LOCK(); - /* now look for instances of driver on our adapters */ if (driver->flags& (I2C_DF_NOTIFY|I2C_DF_DUMMY)) { @@ -294,15 +247,15 @@ /* Ignore errors */ driver->attach_adapter(adapters[i]); } - ADAP_UNLOCK(); + up(&core_lists); return 0; } int i2c_del_driver(struct i2c_driver *driver) { - int i,j,k,res; + int i,j,k,res = 0; - DRV_LOCK(); + down(&core_lists); for (i = 0; i < I2C_DRIVER_MAX; i++) if (driver == drivers[i]) break; @@ -310,7 +263,7 @@ printk(KERN_WARNING " i2c-core.o: unregister_driver: " "[%s] not found\n", driver->name); - DRV_UNLOCK(); + up(&core_lists); return -ENODEV; } /* Have a look at each adapter, if clients of this driver are still @@ -322,7 +275,6 @@ * invalid operation might (will!) result, when using stale client * pointers. */ - ADAP_LOCK(); /* should be moved inside the if statement... */ for (k=0;k<I2C_ADAP_MAX;k++) { struct i2c_adapter *adap = adapters[k]; if (adap == NULL) /* skip empty entries. */ @@ -341,8 +293,7 @@ "not be detached properly; driver " "not unloaded!",driver->name, adap->name); - ADAP_UNLOCK(); - return res; + goto ERROR0; } } else { for (j=0;j<I2C_CLIENT_MAX;j++) { @@ -365,31 +316,41 @@ driver->name, client->addr, adap->name); - ADAP_UNLOCK(); - return res; + goto ERROR0; } } } } } - ADAP_UNLOCK(); drivers[i] = NULL; - driver_count--; - DRV_UNLOCK(); - DEB(printk(KERN_DEBUG "i2c-core.o: driver unregistered: %s\n",driver->name)); - return 0; + +ERROR0: + up(&core_lists); + return res; } -int i2c_check_addr (struct i2c_adapter *adapter, int addr) +static int __i2c_check_addr (struct i2c_adapter *adapter, int addr) { int i; for (i = 0; i < I2C_CLIENT_MAX ; i++) if (adapter->clients[i] && (adapter->clients[i]->addr == addr)) return -EBUSY; + return 0; } +int i2c_check_addr (struct i2c_adapter *adapter, int addr) +{ + int rval; + + down(&adapter->list); + rval = __i2c_check_addr(adapter, addr); + up(&adapter->list); + + return rval; +} + int i2c_attach_client(struct i2c_client *client) { struct i2c_adapter *adapter = client->adapter; @@ -398,6 +359,7 @@ if (i2c_check_addr(client->adapter,client->addr)) return -EBUSY; + down(&adapter->list); for (i = 0; i < I2C_CLIENT_MAX; i++) if (NULL == adapter->clients[i]) break; @@ -405,11 +367,11 @@ printk(KERN_WARNING " i2c-core.o: attach_client(%s) - enlarge I2C_CLIENT_MAX.\n", client->name); + up(&adapter->list); return -ENOMEM; } - adapter->clients[i] = client; - adapter->client_count++; + up(&adapter->list); if (adapter->client_register) if (adapter->client_register(client)) @@ -431,6 +393,7 @@ struct i2c_adapter *adapter = client->adapter; int i,res; + down(&adapter->list); for (i = 0; i < I2C_CLIENT_MAX; i++) if (client == adapter->clients[i]) break; @@ -438,6 +401,7 @@ printk(KERN_WARNING " i2c-core.o: unregister_client " "[%s] not found\n", client->name); + up(&adapter->list); return -ENODEV; } @@ -453,7 +417,7 @@ } adapter->clients[i] = NULL; - adapter->client_count--; + up(&adapter->list); DEB(printk(KERN_DEBUG "i2c-core.o: client [%s] unregistered.\n",client->name)); return 0; @@ -595,6 +559,7 @@ int i; int nr = 0; /* Note that it is safe to write a `little' beyond len. Yes, really. */ + down(&core_lists); for (i = 0; (i < I2C_ADAP_MAX) && (nr < len); i++) if (adapters[i]) { nr += sprintf(buf+nr, "i2c-%d\t", i); @@ -611,6 +576,7 @@ adapters[i]->name, adapters[i]->algo->name); } + up(&core_lists); return nr; } @@ -728,9 +694,9 @@ DEB2(printk(KERN_DEBUG "i2c-core.o: master_xfer: %s with %d msgs.\n", adap->name,num)); - I2C_LOCK(adap); + down(&adap->bus); ret = adap->algo->master_xfer(adap,msgs,num); - I2C_UNLOCK(adap); + up(&adap->bus); return ret; } else { @@ -755,9 +721,9 @@ DEB2(printk(KERN_DEBUG "i2c-core.o: master_send: writing %d bytes on %s.\n", count,client->adapter->name)); - I2C_LOCK(adap); + down(&adap->bus); ret = adap->algo->master_xfer(adap,&msg,1); - I2C_UNLOCK(adap); + up(&adap->bus); /* if everything went ok (i.e. 1 msg transmitted), return #bytes * transmitted, else error code. @@ -785,9 +751,9 @@ DEB2(printk(KERN_DEBUG "i2c-core.o: master_recv: reading %d bytes on %s.\n", count,client->adapter->name)); - I2C_LOCK(adap); + down(&adap->bus); ret = adap->algo->master_xfer(adap,&msg,1); - I2C_UNLOCK(adap); + up(&adap->bus); DEB2(printk(KERN_DEBUG "i2c-core.o: master_recv: return:%d (count:%d, addr:0x%02x)\n", ret, count, client->addr)); @@ -1195,10 +1161,10 @@ s32 res; flags = flags & I2C_M_TEN; if (adapter->algo->smbus_xfer) { - I2C_LOCK(adapter); + down(&adapter->bus); res = adapter->algo->smbus_xfer(adapter,addr,flags,read_write, command,size,data); - I2C_UNLOCK(adapter); + up(&adapter->bus); } else res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write, command,size,data); @@ -1228,12 +1194,7 @@ printk(KERN_INFO "i2c-core.o: i2c core module version %s (%s)\n", I2C_VERSION, I2C_DATE); memset(adapters,0,sizeof(adapters)); memset(drivers,0,sizeof(drivers)); - adap_count=0; - driver_count=0; - init_MUTEX(&adap_lock); - init_MUTEX(&driver_lock); - #ifdef CONFIG_PROC_FS return i2cproc_init(); #else diff -ru linux-2.4.24-pre2-k2/drivers/i2c/i2c-sibyte.c linux-2.4.24-pre2-k3/drivers/i2c/i2c-sibyte.c --- linux-2.4.24-pre2-k2/drivers/i2c/i2c-sibyte.c Mon Dec 22 22:04:00 2003 +++ linux-2.4.24-pre2-k3/drivers/i2c/i2c-sibyte.c Wed Dec 24 19:04:55 2003 @@ -67,7 +67,6 @@ dec_use: sibyte_dec_use, client_register: sibyte_reg, client_unregister: sibyte_unreg, - client_count: 0 } , { name: "SiByte SMBus 1", @@ -78,7 +77,6 @@ dec_use: sibyte_dec_use, client_register: sibyte_reg, client_unregister: sibyte_unreg, - client_count: 0 } }; diff -ru linux-2.4.24-pre2-k2/include/linux/i2c.h linux-2.4.24-pre2-k3/include/linux/i2c.h --- linux-2.4.24-pre2-k2/include/linux/i2c.h Tue Dec 23 20:05:40 2003 +++ linux-2.4.24-pre2-k3/include/linux/i2c.h Wed Dec 24 20:12:52 2003 @@ -241,11 +241,11 @@ /* and can be set via the i2c_ioctl call */ /* data fields that are valid for all devices */ - struct semaphore lock; + struct semaphore bus; + struct semaphore list; unsigned int flags;/* flags specifying div. data */ struct i2c_client *clients[I2C_CLIENT_MAX]; - int client_count; int timeout; int retries; -- Jean Delvare http://www.ensicaen.ismra.fr/~delvare/