summaryrefslogtreecommitdiff
path: root/drivers/w1
diff options
context:
space:
mode:
authorNeilBrown <neilb@suse.de>2012-05-18 09:59:52 +0400
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2012-06-14 03:38:40 +0400
commitb02f8bede217a4b145ecc16d3940c78d83941147 (patch)
treecbd02191cc2698c460b9abedea44de13855510c6 /drivers/w1
parentb7e938d06d0de43bdbee8844a8736c81480c1031 (diff)
downloadlinux-b02f8bede217a4b145ecc16d3940c78d83941147.tar.xz
W1: split master mutex to avoid deadlocks.
The 'mutex' in struct w1_master is use for two very different purposes. Firstly it protects various data structures such as the list of all slaves. Secondly it protects the w1 buss against concurrent accesses. This can lead to deadlocks when the ->probe code called while adding a slave needs to talk on the bus, as is the case for power_supply devices. ds2780 and ds2781 drivers contain a work around to track which process hold the lock simply to avoid this deadlock. bq27000 doesn't have that work around and so deadlocks. There are other possible deadlocks involving sysfs. When removing a device the sysfs s_active lock is held, so the lock that protects the slave list must take precedence over s_active. However when access power_supply attributes via sysfs, the s_active lock must take precedence over the lock that protects accesses to the bus. So to avoid deadlocks between w1 slaves and sysfs, these must be two separate locks. Making them separate means that the work around in ds2780 and ds2781 can be removed. So this patch: - adds a new mutex: "bus_mutex" which serialises access to the bus. - takes in mutex in w1_search and ds1wm_search while they access the bus for searching. The mutex is dropped before calling the callback which adds the slave. - changes all slaves to use bus_mutex instead of mutex to protect access to the bus - removes w1_ds2790_io_nolock and w1_ds2781_io_nolock, and the related code from drivers/power/ds278[01]_battery.c which calls them. Signed-off-by: NeilBrown <neilb@suse.de> Acked-by: Evgeniy Polyakov <zbr@ioremap.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/w1')
-rw-r--r--drivers/w1/masters/ds1wm.c4
-rw-r--r--drivers/w1/slaves/w1_bq27000.c4
-rw-r--r--drivers/w1/slaves/w1_ds2408.c24
-rw-r--r--drivers/w1/slaves/w1_ds2423.c4
-rw-r--r--drivers/w1/slaves/w1_ds2431.c8
-rw-r--r--drivers/w1/slaves/w1_ds2433.c8
-rw-r--r--drivers/w1/slaves/w1_ds2760.c8
-rw-r--r--drivers/w1/slaves/w1_ds2780.c22
-rw-r--r--drivers/w1/slaves/w1_ds2780.h2
-rw-r--r--drivers/w1/slaves/w1_ds2781.c22
-rw-r--r--drivers/w1/slaves/w1_ds2781.h2
-rw-r--r--drivers/w1/slaves/w1_therm.c10
-rw-r--r--drivers/w1/w1.c9
-rw-r--r--drivers/w1/w1.h1
-rw-r--r--drivers/w1/w1_int.c1
15 files changed, 55 insertions, 74 deletions
diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index a0c8965c1a79..530a2d309063 100644
--- a/drivers/w1/masters/ds1wm.c
+++ b/drivers/w1/masters/ds1wm.c
@@ -334,7 +334,9 @@ static void ds1wm_search(void *data, struct w1_master *master_dev,
return;
}
+ mutex_lock(&master_dev->bus_mutex);
if (ds1wm_reset(ds1wm_data)) {
+ mutex_unlock(&master_dev->bus_mutex);
dev_dbg(&ds1wm_data->pdev->dev,
"pass: %d reset error (or no slaves)\n", pass);
break;
@@ -387,6 +389,7 @@ static void ds1wm_search(void *data, struct w1_master *master_dev,
}
if (ds1wm_data->read_error) {
+ mutex_unlock(&master_dev->bus_mutex);
dev_err(&ds1wm_data->pdev->dev,
"pass: %d read error, retrying\n", pass);
break;
@@ -400,6 +403,7 @@ static void ds1wm_search(void *data, struct w1_master *master_dev,
dev_dbg(&ds1wm_data->pdev->dev,
"pass: %d resetting bus\n", pass);
ds1wm_reset(ds1wm_data);
+ mutex_unlock(&master_dev->bus_mutex);
if ((r_prime & ((u64)1 << 63)) && (d & ((u64)1 << 63))) {
dev_err(&ds1wm_data->pdev->dev,
"pass: %d bus error, retrying\n", pass);
diff --git a/drivers/w1/slaves/w1_bq27000.c b/drivers/w1/slaves/w1_bq27000.c
index 52ad812fa1e7..773dca5beafe 100644
--- a/drivers/w1/slaves/w1_bq27000.c
+++ b/drivers/w1/slaves/w1_bq27000.c
@@ -31,10 +31,10 @@ static int w1_bq27000_read(struct device *dev, unsigned int reg)
u8 val;
struct w1_slave *sl = container_of(dev->parent, struct w1_slave, dev);
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
w1_write_8(sl->master, HDQ_CMD_READ | reg);
val = w1_read_8(sl->master);
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return val;
}
diff --git a/drivers/w1/slaves/w1_ds2408.c b/drivers/w1/slaves/w1_ds2408.c
index 8e813eed0f0a..441ad3a3b586 100644
--- a/drivers/w1/slaves/w1_ds2408.c
+++ b/drivers/w1/slaves/w1_ds2408.c
@@ -52,11 +52,11 @@ static int _read_reg(struct w1_slave *sl, u8 address, unsigned char* buf)
if (!buf)
return -EINVAL;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
dev_dbg(&sl->dev, "mutex locked");
if (w1_reset_select_slave(sl)) {
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return -EIO;
}
@@ -66,7 +66,7 @@ static int _read_reg(struct w1_slave *sl, u8 address, unsigned char* buf)
w1_write_block(sl->master, wrbuf, 3);
*buf = w1_read_8(sl->master);
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
dev_dbg(&sl->dev, "mutex unlocked");
return 1;
}
@@ -165,7 +165,7 @@ static ssize_t w1_f29_write_output(
return -EFAULT;
dev_dbg(&sl->dev, "locking mutex for write_output");
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
dev_dbg(&sl->dev, "mutex locked");
if (w1_reset_select_slave(sl))
@@ -200,14 +200,14 @@ static ssize_t w1_f29_write_output(
/* read the result of the READ_PIO_REGS command */
if (w1_read_8(sl->master) == *buf) {
/* success! */
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
dev_dbg(&sl->dev,
"mutex unlocked, retries:%d", retries);
return 1;
}
}
error:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
dev_dbg(&sl->dev, "mutex unlocked in error, retries:%d", retries);
return -EIO;
@@ -228,7 +228,7 @@ static ssize_t w1_f29_write_activity(
if (count != 1 || off != 0)
return -EFAULT;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (w1_reset_select_slave(sl))
goto error;
@@ -236,7 +236,7 @@ static ssize_t w1_f29_write_activity(
while (retries--) {
w1_write_8(sl->master, W1_F29_FUNC_RESET_ACTIVITY_LATCHES);
if (w1_read_8(sl->master) == W1_F29_SUCCESS_CONFIRM_BYTE) {
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 1;
}
if (w1_reset_resume_command(sl->master))
@@ -244,7 +244,7 @@ static ssize_t w1_f29_write_activity(
}
error:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return -EIO;
}
@@ -263,7 +263,7 @@ static ssize_t w1_f29_write_status_control(
if (count != 1 || off != 0)
return -EFAULT;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (w1_reset_select_slave(sl))
goto error;
@@ -285,12 +285,12 @@ static ssize_t w1_f29_write_status_control(
w1_write_block(sl->master, w1_buf, 3);
if (w1_read_8(sl->master) == *buf) {
/* success! */
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 1;
}
}
error:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return -EIO;
}
diff --git a/drivers/w1/slaves/w1_ds2423.c b/drivers/w1/slaves/w1_ds2423.c
index 7a7dbe5026f1..40a10b5ed120 100644
--- a/drivers/w1/slaves/w1_ds2423.c
+++ b/drivers/w1/slaves/w1_ds2423.c
@@ -66,7 +66,7 @@ static ssize_t w1_counter_read(struct device *device,
wrbuf[0] = 0xA5;
wrbuf[1] = rom_addr & 0xFF;
wrbuf[2] = rom_addr >> 8;
- mutex_lock(&dev->mutex);
+ mutex_lock(&dev->bus_mutex);
if (!w1_reset_select_slave(sl)) {
w1_write_block(dev, wrbuf, 3);
read_byte_count = 0;
@@ -124,7 +124,7 @@ static ssize_t w1_counter_read(struct device *device,
} else {
c -= snprintf(out_buf + PAGE_SIZE - c, c, "Connection error");
}
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);
return PAGE_SIZE - c;
}
diff --git a/drivers/w1/slaves/w1_ds2431.c b/drivers/w1/slaves/w1_ds2431.c
index 84e2410aec1d..984b30331a45 100644
--- a/drivers/w1/slaves/w1_ds2431.c
+++ b/drivers/w1/slaves/w1_ds2431.c
@@ -107,7 +107,7 @@ static ssize_t w1_f2d_read_bin(struct file *filp, struct kobject *kobj,
if (count == 0)
return 0;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
/* read directly from the EEPROM in chunks of W1_F2D_READ_MAXLEN */
while (todo > 0) {
@@ -126,7 +126,7 @@ static ssize_t w1_f2d_read_bin(struct file *filp, struct kobject *kobj,
off += W1_F2D_READ_MAXLEN;
}
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
@@ -214,7 +214,7 @@ static ssize_t w1_f2d_write_bin(struct file *filp, struct kobject *kobj,
if (count == 0)
return 0;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
/* Can only write data in blocks of the size of the scratchpad */
addr = off;
@@ -259,7 +259,7 @@ static ssize_t w1_f2d_write_bin(struct file *filp, struct kobject *kobj,
}
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
diff --git a/drivers/w1/slaves/w1_ds2433.c b/drivers/w1/slaves/w1_ds2433.c
index 0f7b8f9c509a..85f2cdb27fa2 100644
--- a/drivers/w1/slaves/w1_ds2433.c
+++ b/drivers/w1/slaves/w1_ds2433.c
@@ -107,7 +107,7 @@ static ssize_t w1_f23_read_bin(struct file *filp, struct kobject *kobj,
if ((count = w1_f23_fix_count(off, count, W1_EEPROM_SIZE)) == 0)
return 0;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
#ifdef CONFIG_W1_SLAVE_DS2433_CRC
@@ -138,7 +138,7 @@ static ssize_t w1_f23_read_bin(struct file *filp, struct kobject *kobj,
#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
@@ -233,7 +233,7 @@ static ssize_t w1_f23_write_bin(struct file *filp, struct kobject *kobj,
}
#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
/* Can only write data to one page at a time */
idx = 0;
@@ -251,7 +251,7 @@ static ssize_t w1_f23_write_bin(struct file *filp, struct kobject *kobj,
}
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
diff --git a/drivers/w1/slaves/w1_ds2760.c b/drivers/w1/slaves/w1_ds2760.c
index 5754c9a4f58b..aa7bd5fa2fa8 100644
--- a/drivers/w1/slaves/w1_ds2760.c
+++ b/drivers/w1/slaves/w1_ds2760.c
@@ -31,7 +31,7 @@ static int w1_ds2760_io(struct device *dev, char *buf, int addr, size_t count,
if (!dev)
return 0;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (addr > DS2760_DATA_SIZE || addr < 0) {
count = 0;
@@ -54,7 +54,7 @@ static int w1_ds2760_io(struct device *dev, char *buf, int addr, size_t count,
}
out:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
@@ -76,14 +76,14 @@ static int w1_ds2760_eeprom_cmd(struct device *dev, int addr, int cmd)
if (!dev)
return -EINVAL;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (w1_reset_select_slave(sl) == 0) {
w1_write_8(sl->master, cmd);
w1_write_8(sl->master, addr);
}
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 0;
}
diff --git a/drivers/w1/slaves/w1_ds2780.c b/drivers/w1/slaves/w1_ds2780.c
index 39f78c0b143c..7b09307de0ef 100644
--- a/drivers/w1/slaves/w1_ds2780.c
+++ b/drivers/w1/slaves/w1_ds2780.c
@@ -60,30 +60,16 @@ int w1_ds2780_io(struct device *dev, char *buf, int addr, size_t count,
if (!dev)
return -ENODEV;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
ret = w1_ds2780_do_io(dev, buf, addr, count, io);
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return ret;
}
EXPORT_SYMBOL(w1_ds2780_io);
-int w1_ds2780_io_nolock(struct device *dev, char *buf, int addr, size_t count,
- int io)
-{
- int ret;
-
- if (!dev)
- return -ENODEV;
-
- ret = w1_ds2780_do_io(dev, buf, addr, count, io);
-
- return ret;
-}
-EXPORT_SYMBOL(w1_ds2780_io_nolock);
-
int w1_ds2780_eeprom_cmd(struct device *dev, int addr, int cmd)
{
struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
@@ -91,14 +77,14 @@ int w1_ds2780_eeprom_cmd(struct device *dev, int addr, int cmd)
if (!dev)
return -EINVAL;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (w1_reset_select_slave(sl) == 0) {
w1_write_8(sl->master, cmd);
w1_write_8(sl->master, addr);
}
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 0;
}
EXPORT_SYMBOL(w1_ds2780_eeprom_cmd);
diff --git a/drivers/w1/slaves/w1_ds2780.h b/drivers/w1/slaves/w1_ds2780.h
index 737379365021..a1fba79eb1b5 100644
--- a/drivers/w1/slaves/w1_ds2780.h
+++ b/drivers/w1/slaves/w1_ds2780.h
@@ -124,8 +124,6 @@
extern int w1_ds2780_io(struct device *dev, char *buf, int addr, size_t count,
int io);
-extern int w1_ds2780_io_nolock(struct device *dev, char *buf, int addr,
- size_t count, int io);
extern int w1_ds2780_eeprom_cmd(struct device *dev, int addr, int cmd);
#endif /* !_W1_DS2780_H */
diff --git a/drivers/w1/slaves/w1_ds2781.c b/drivers/w1/slaves/w1_ds2781.c
index 0d0c7985293f..877daf74159c 100644
--- a/drivers/w1/slaves/w1_ds2781.c
+++ b/drivers/w1/slaves/w1_ds2781.c
@@ -58,30 +58,16 @@ int w1_ds2781_io(struct device *dev, char *buf, int addr, size_t count,
if (!dev)
return -ENODEV;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
ret = w1_ds2781_do_io(dev, buf, addr, count, io);
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return ret;
}
EXPORT_SYMBOL(w1_ds2781_io);
-int w1_ds2781_io_nolock(struct device *dev, char *buf, int addr, size_t count,
- int io)
-{
- int ret;
-
- if (!dev)
- return -ENODEV;
-
- ret = w1_ds2781_do_io(dev, buf, addr, count, io);
-
- return ret;
-}
-EXPORT_SYMBOL(w1_ds2781_io_nolock);
-
int w1_ds2781_eeprom_cmd(struct device *dev, int addr, int cmd)
{
struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
@@ -89,14 +75,14 @@ int w1_ds2781_eeprom_cmd(struct device *dev, int addr, int cmd)
if (!dev)
return -EINVAL;
- mutex_lock(&sl->master->mutex);
+ mutex_lock(&sl->master->bus_mutex);
if (w1_reset_select_slave(sl) == 0) {
w1_write_8(sl->master, cmd);
w1_write_8(sl->master, addr);
}
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 0;
}
EXPORT_SYMBOL(w1_ds2781_eeprom_cmd);
diff --git a/drivers/w1/slaves/w1_ds2781.h b/drivers/w1/slaves/w1_ds2781.h
index 82bc66497b43..557dfb0b4f64 100644
--- a/drivers/w1/slaves/w1_ds2781.h
+++ b/drivers/w1/slaves/w1_ds2781.h
@@ -129,8 +129,6 @@
extern int w1_ds2781_io(struct device *dev, char *buf, int addr, size_t count,
int io);
-extern int w1_ds2781_io_nolock(struct device *dev, char *buf, int addr,
- size_t count, int io);
extern int w1_ds2781_eeprom_cmd(struct device *dev, int addr, int cmd);
#endif /* !_W1_DS2781_H */
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index ff29ae747ee8..d90062b211f8 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -179,7 +179,7 @@ static ssize_t w1_therm_read(struct device *device,
int i, max_trying = 10;
ssize_t c = PAGE_SIZE;
- i = mutex_lock_interruptible(&dev->mutex);
+ i = mutex_lock_interruptible(&dev->bus_mutex);
if (i != 0)
return i;
@@ -207,19 +207,19 @@ static ssize_t w1_therm_read(struct device *device,
w1_write_8(dev, W1_CONVERT_TEMP);
if (external_power) {
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);
sleep_rem = msleep_interruptible(tm);
if (sleep_rem != 0)
return -EINTR;
- i = mutex_lock_interruptible(&dev->mutex);
+ i = mutex_lock_interruptible(&dev->bus_mutex);
if (i != 0)
return i;
} else if (!w1_strong_pullup) {
sleep_rem = msleep_interruptible(tm);
if (sleep_rem != 0) {
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);
return -EINTR;
}
}
@@ -258,7 +258,7 @@ static ssize_t w1_therm_read(struct device *device,
c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n",
w1_convert_temp(rom, sl->family->fid));
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);
return PAGE_SIZE - c;
}
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index bfb898641029..1a574370d2cd 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -885,16 +885,21 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
*
* Return 0 - device(s) present, 1 - no devices present.
*/
+ mutex_lock(&dev->bus_mutex);
if (w1_reset_bus(dev)) {
+ mutex_unlock(&dev->bus_mutex);
dev_dbg(&dev->dev, "No devices present on the wire.\n");
break;
}
/* Do fast search on single slave bus */
if (dev->max_slave_count == 1) {
+ int rv;
w1_write_8(dev, W1_READ_ROM);
+ rv = w1_read_block(dev, (u8 *)&rn, 8);
+ mutex_unlock(&dev->bus_mutex);
- if (w1_read_block(dev, (u8 *)&rn, 8) == 8 && rn)
+ if (rv == 8 && rn)
cb(dev, rn);
break;
@@ -927,10 +932,12 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
rn |= (tmp64 << i);
if (kthread_should_stop()) {
+ mutex_unlock(&dev->bus_mutex);
dev_dbg(&dev->dev, "Abort w1_search\n");
return;
}
}
+ mutex_unlock(&dev->bus_mutex);
if ( (triplet_ret & 0x03) != 0x03 ) {
if ( (desc_bit == last_zero) || (last_zero < 0))
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 4d012ca3f32c..45908e56c2f8 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -180,6 +180,7 @@ struct w1_master
struct task_struct *thread;
struct mutex mutex;
+ struct mutex bus_mutex;
struct device_driver *driver;
struct device dev;
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 68288355727a..531434180afd 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -76,6 +76,7 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
INIT_LIST_HEAD(&dev->slist);
mutex_init(&dev->mutex);
+ mutex_init(&dev->bus_mutex);
memcpy(&dev->dev, device, sizeof(struct device));
dev_set_name(&dev->dev, "w1_bus_master%u", dev->id);