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 <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
struct device *dev;
struct power_supply bat;
struct device *w1_dev;
- struct task_struct *mutex_holder;
};
enum current_types {
static inline int ds2780_battery_io(struct ds2780_device_info *dev_info,
char *buf, int addr, size_t count, int io)
{
- if (dev_info->mutex_holder == current)
- return w1_ds2780_io_nolock(dev_info->w1_dev, buf, addr, count, io);
- else
- return w1_ds2780_io(dev_info->w1_dev, buf, addr, count, io);
+ return w1_ds2780_io(dev_info->w1_dev, buf, addr, count, io);
}
static inline int ds2780_read8(struct ds2780_device_info *dev_info, u8 *val,
dev_info->bat.properties = ds2780_battery_props;
dev_info->bat.num_properties = ARRAY_SIZE(ds2780_battery_props);
dev_info->bat.get_property = ds2780_battery_get_property;
- dev_info->mutex_holder = current;
ret = power_supply_register(&pdev->dev, &dev_info->bat);
if (ret) {
goto fail_remove_bin_file;
}
- dev_info->mutex_holder = NULL;
-
return 0;
fail_remove_bin_file:
{
struct ds2780_device_info *dev_info = platform_get_drvdata(pdev);
- dev_info->mutex_holder = current;
-
/* remove attributes */
sysfs_remove_group(&dev_info->bat.dev->kobj, &ds2780_attr_group);
struct device *dev;
struct power_supply bat;
struct device *w1_dev;
- struct task_struct *mutex_holder;
};
enum current_types {
static inline int ds2781_battery_io(struct ds2781_device_info *dev_info,
char *buf, int addr, size_t count, int io)
{
- if (dev_info->mutex_holder == current)
- return w1_ds2781_io_nolock(dev_info->w1_dev, buf, addr,
- count, io);
- else
- return w1_ds2781_io(dev_info->w1_dev, buf, addr, count, io);
+ return w1_ds2781_io(dev_info->w1_dev, buf, addr, count, io);
}
int w1_ds2781_read(struct ds2781_device_info *dev_info, char *buf,
dev_info->bat.properties = ds2781_battery_props;
dev_info->bat.num_properties = ARRAY_SIZE(ds2781_battery_props);
dev_info->bat.get_property = ds2781_battery_get_property;
- dev_info->mutex_holder = current;
ret = power_supply_register(&pdev->dev, &dev_info->bat);
if (ret) {
goto fail_remove_bin_file;
}
- dev_info->mutex_holder = NULL;
-
return 0;
fail_remove_bin_file:
{
struct ds2781_device_info *dev_info = platform_get_drvdata(pdev);
- dev_info->mutex_holder = current;
-
/* remove attributes */
sysfs_remove_group(&dev_info->bat.dev->kobj, &ds2781_attr_group);
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;
}
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;
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);
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;
}
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;
}
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;
}
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))
/* 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;
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;
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))
}
error:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return -EIO;
}
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;
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;
}
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;
} else {
c -= snprintf(out_buf + PAGE_SIZE - c, c, "Connection error");
}
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);
return PAGE_SIZE - c;
}
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) {
off += W1_F2D_READ_MAXLEN;
}
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
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;
}
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
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
#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
}
#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;
}
out_up:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return 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;
}
out:
- mutex_unlock(&sl->master->mutex);
+ mutex_unlock(&sl->master->bus_mutex);
return count;
}
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;
}
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);
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);
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 */
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);
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);
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 */
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;
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;
}
}
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;
}
*
* 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;
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))
struct task_struct *thread;
struct mutex mutex;
+ struct mutex bus_mutex;
struct device_driver *driver;
struct device dev;
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);
projects / linux.git / commitdiff