Jim Cromie wrote:
Im working with scx200_gpio driver, and extending it to operate
gpio pins on pc87366 chip, present on the soekris net-4801
the chip has many functional-units, some of which are driven by
i2c/chips/pc87360.c
both drivers have keep their own locks, which prevent other
uses of the same driver from corrupting operations,
but these offer no protection from interference from the other driver.
Is that correct ?
And if so, how do I resolve it ?
1. 3rd module, which exports a single lock
OK - no one home. I guess its ok to talk to myself ;-)
attached patch compiles, loads, loads its dependencies,
IOW, it appears somewhat workable.
But, Is it sane ?
Is there a better way ?
Depending upon responses here, Ill send to lm-sensors next.
diff -ruN -X exclude-diffs linux-2.6.12-soekris/drivers/char/Makefile i2c/drivers/char/Makefile
--- linux-2.6.12-soekris/drivers/char/Makefile 2005-06-18 07:04:14.000000000 -0600
+++ i2c/drivers/char/Makefile 2005-06-21 12:04:41.000000000 -0600
@@ -12,6 +12,8 @@
obj-$(CONFIG_LEGACY_PTYS) += pty.o
obj-$(CONFIG_UNIX98_PTYS) += pty.o
obj-y += misc.o
+obj-m += sio_lock.o
+
obj-$(CONFIG_VT) += vt_ioctl.o vc_screen.o consolemap.o \
consolemap_deftbl.o selection.o keyboard.o
obj-$(CONFIG_HW_CONSOLE) += vt.o defkeymap.o
diff -ruN -X exclude-diffs linux-2.6.12-soekris/drivers/char/sio_lock.c i2c/drivers/char/sio_lock.c
--- linux-2.6.12-soekris/drivers/char/sio_lock.c 1969-12-31 17:00:00.000000000 -0700
+++ i2c/drivers/char/sio_lock.c 2005-06-21 12:46:23.000000000 -0600
@@ -0,0 +1,47 @@
+
+
+#include <linux/config.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+
+#include <asm-i386/semaphore.h>
+
+MODULE_AUTHOR("Jim Cromie");
+MODULE_LICENSE("Dual BSD/GPL");
+
+/**
+ a module/container for a lock to be shared between
+ drivers/i2c/chips/pc87360 and drivers/char/pc87366_gpio.
+
+ The 1st module uses semaphores, the 2nd spinlocks. Im changing
+ the 2nd, cuz lm-sensors is maintained, and pc87366_gpio is a new
+ extension (ie mine) of an unmaintained module (scx200, so said
+ author, privately).
+
+*/
+
+//static DEFINE_SPINLOCK(sio_lock);
+
+struct semaphore sio_lock;
+struct semaphore sio_update_lock;
+
+int sio_lock_init_module(void)
+{
+ printk(KERN_NOTICE "sio_lock_init_module\n");
+ return 0;
+}
+
+void sio_lock_cleanup_module(void)
+{
+ printk(KERN_NOTICE "sio_lock_cleanup_module\n");
+}
+
+EXPORT_SYMBOL(sio_lock);
+EXPORT_SYMBOL(sio_update_lock);
+
+module_init(sio_lock_init_module);
+module_exit(sio_lock_cleanup_module);
+
+
+
diff -ruN -X exclude-diffs linux-2.6.12-soekris/drivers/i2c/chips/pc87360.c i2c/drivers/i2c/chips/pc87360.c
--- linux-2.6.12-soekris/drivers/i2c/chips/pc87360.c 2005-06-18 07:04:21.000000000 -0600
+++ i2c/drivers/i2c/chips/pc87360.c 2005-06-21 12:48:12.000000000 -0600
@@ -50,6 +50,24 @@
static unsigned int extra_isa[3];
static u8 confreg[4];
+#define SHARED_LOCKS 1
+
+#if SHARED_LOCKS
+extern struct semaphore sio_lock;
+extern struct semaphore sio_update_lock;
+
+#define down_lock down(&sio_lock)
+#define down_updatelock down(&sio_update_lock)
+#define up_lock up(&sio_lock)
+#define up_updatelock up(&sio_update_lock)
+
+#else
+#define down_lock down(&(data->lock))
+#define down_updatelock down(&(data->update_lock))
+#define up_lock up(&(data->lock))
+#define up_updatelock up(&(data->update_lock))
+#endif
+
enum chips { any_chip, pc87360, pc87363, pc87364, pc87365, pc87366 };
static struct i2c_address_data addr_data = {
.normal_i2c = normal_i2c,
@@ -187,8 +205,10 @@
struct pc87360_data {
struct i2c_client client;
+#if ! SHARED_LOCKS
struct semaphore lock;
struct semaphore update_lock;
+#endif
char valid; /* !=0 if following fields are valid */
unsigned long last_updated; /* In jiffies */
@@ -259,7 +279,7 @@
struct pc87360_data *data = i2c_get_clientdata(client);
long fan_min = simple_strtol(buf, NULL, 10);
- down(&data->update_lock);
+ down_updatelock;
fan_min = FAN_TO_REG(fan_min, FAN_DIV_FROM_REG(data->fan_status[nr]));
/* If it wouldn't fit, change clock divisor */
@@ -276,7 +296,7 @@
/* Write new divider, preserve alarm bits */
pc87360_write_value(data, LD_FAN, NO_BANK, PC87360_REG_FAN_STATUS(nr),
data->fan_status[nr] & 0xF9);
- up(&data->update_lock);
+ up_updatelock;
return count;
}
@@ -339,12 +359,12 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->pwm[offset-1] = PWM_TO_REG(val, \
FAN_CONFIG_INVERT(data->fan_conf, offset-1)); \
pc87360_write_value(data, LD_FAN, NO_BANK, PC87360_REG_PWM(offset-1), \
data->pwm[offset-1]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static DEVICE_ATTR(pwm##offset, S_IWUSR | S_IRUGO, \
@@ -384,11 +404,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->in_min[offset] = IN_TO_REG(val, data->in_vref); \
pc87360_write_value(data, LD_IN, offset, PC87365_REG_IN_MIN, \
data->in_min[offset]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static ssize_t set_in##offset##_max(struct device *dev, const char *buf, \
@@ -398,12 +418,12 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->in_max[offset] = IN_TO_REG(val, \
data->in_vref); \
pc87360_write_value(data, LD_IN, offset, PC87365_REG_IN_MAX, \
data->in_max[offset]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static DEVICE_ATTR(in##offset##_input, S_IRUGO, \
@@ -463,11 +483,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->in_min[offset+7] = IN_TO_REG(val, data->in_vref); \
pc87360_write_value(data, LD_IN, offset+7, PC87365_REG_TEMP_MIN, \
data->in_min[offset+7]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static ssize_t set_temp##offset##_max(struct device *dev, const char *buf, \
@@ -477,11 +497,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->in_max[offset+7] = IN_TO_REG(val, data->in_vref); \
pc87360_write_value(data, LD_IN, offset+7, PC87365_REG_TEMP_MAX, \
data->in_max[offset+7]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static ssize_t set_temp##offset##_crit(struct device *dev, const char *buf, \
@@ -491,11 +511,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->in_crit[offset-4] = IN_TO_REG(val, data->in_vref); \
pc87360_write_value(data, LD_IN, offset+7, PC87365_REG_TEMP_CRIT, \
data->in_crit[offset-4]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static DEVICE_ATTR(temp##offset##_input, S_IRUGO, \
@@ -573,11 +593,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->temp_min[offset-1] = TEMP_TO_REG(val); \
pc87360_write_value(data, LD_TEMP, offset-1, PC87365_REG_TEMP_MIN, \
data->temp_min[offset-1]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static ssize_t set_temp##offset##_max(struct device *dev, const char *buf, \
@@ -587,11 +607,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->temp_max[offset-1] = TEMP_TO_REG(val); \
pc87360_write_value(data, LD_TEMP, offset-1, PC87365_REG_TEMP_MAX, \
data->temp_max[offset-1]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static ssize_t set_temp##offset##_crit(struct device *dev, const char *buf, \
@@ -601,11 +621,11 @@
struct pc87360_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
\
- down(&data->update_lock); \
+ down_updatelock; \
data->temp_crit[offset-1] = TEMP_TO_REG(val); \
pc87360_write_value(data, LD_TEMP, offset-1, PC87365_REG_TEMP_CRIT, \
data->temp_crit[offset-1]); \
- up(&data->update_lock); \
+ up_updatelock; \
return count; \
} \
static DEVICE_ATTR(temp##offset##_input, S_IRUGO, \
@@ -749,7 +769,7 @@
new_client = &data->client;
i2c_set_clientdata(new_client, data);
new_client->addr = address;
- init_MUTEX(&data->lock);
+ init_MUTEX(&sio_lock);
new_client->adapter = adapter;
new_client->driver = &pc87360_driver;
new_client->flags = 0;
@@ -782,7 +802,7 @@
strcpy(new_client->name, name);
data->valid = 0;
- init_MUTEX(&data->update_lock);
+ init_MUTEX(&sio_update_lock);
for (i = 0; i < 3; i++) {
if (((data->address[i] = extra_isa[i]))
@@ -1014,11 +1034,11 @@
{
int res;
- down(&(data->lock));
+ down_lock;
if (bank != NO_BANK)
outb_p(bank, data->address[ldi] + PC87365_REG_BANK);
res = inb_p(data->address[ldi] + reg);
- up(&(data->lock));
+ up_lock;
return res;
}
@@ -1026,11 +1046,11 @@
static void pc87360_write_value(struct pc87360_data *data, u8 ldi, u8 bank,
u8 reg, u8 value)
{
- down(&(data->lock));
+ down_lock;
if (bank != NO_BANK)
outb_p(bank, data->address[ldi] + PC87365_REG_BANK);
outb_p(value, data->address[ldi] + reg);
- up(&(data->lock));
+ up_lock;
}
static void pc87360_init_client(struct i2c_client *client, int use_thermistors)
@@ -1202,7 +1222,7 @@
struct pc87360_data *data = i2c_get_clientdata(client);
u8 i;
- down(&data->update_lock);
+ down_updatelock;
if (time_after(jiffies, data->last_updated + HZ * 2) || !data->valid) {
dev_dbg(&client->dev, "Data update\n");
@@ -1302,7 +1322,7 @@
data->valid = 1;
}
- up(&data->update_lock);
+ up_updatelock;
return data;
}
