From c30c9b15187e977ab5928f7276e9dfcd8d6f9460 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:38 -0700 Subject: W1: fix deadlocks and remove w1_control_thread w1_control_thread was removed which would wake up every second and process newly registered family codes and complete some final cleanup for a removed master. Those routines were moved to the threads that were previously requesting those operations. A new function w1_reconnect_slaves takes care of reconnecting existing slave devices when a new family code is registered or removed. The removal case was missing and would cause a deadlock waiting for the family code reference count to decrease, which will now happen. A problem with registering a family code was fixed. A slave device would be unattached if it wasn't yet claimed, then attached at the end of the list, two unclaimed slaves would cause an infinite loop. The struct w1_bus_master.search now takes a pointer to the struct w1_master device to avoid searching for it, which would have caused a lock ordering deadlock with the removal of w1_control_thread. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds1wm.c | 6 +- drivers/w1/w1.c | 146 +++++++++++---------------------------------- drivers/w1/w1.h | 19 ++++-- drivers/w1/w1_family.c | 6 +- drivers/w1/w1_family.h | 1 - drivers/w1/w1_int.c | 12 ++++ drivers/w1/w1_io.c | 3 +- 7 files changed, 71 insertions(+), 122 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c index 10211e493001..ea894bf18113 100644 --- a/drivers/w1/masters/ds1wm.c +++ b/drivers/w1/masters/ds1wm.c @@ -274,8 +274,8 @@ static u8 ds1wm_reset_bus(void *data) return 0; } -static void ds1wm_search(void *data, u8 search_type, - w1_slave_found_callback slave_found) +static void ds1wm_search(void *data, struct w1_master *master_dev, + u8 search_type, w1_slave_found_callback slave_found) { struct ds1wm_data *ds1wm_data = data; int i; @@ -313,7 +313,7 @@ static void ds1wm_search(void *data, u8 search_type, ds1wm_write_register(ds1wm_data, DS1WM_CMD, ~DS1WM_CMD_SRA); ds1wm_reset(ds1wm_data); - slave_found(ds1wm_data, rom_id); + slave_found(master_dev, rom_id); } /* --------------------------------------------------------------------- */ diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 7293c9b11f91..25640f681729 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -46,20 +46,16 @@ MODULE_AUTHOR("Evgeniy Polyakov "); MODULE_DESCRIPTION("Driver for 1-wire Dallas network protocol."); static int w1_timeout = 10; -static int w1_control_timeout = 1; int w1_max_slave_count = 10; int w1_max_slave_ttl = 10; module_param_named(timeout, w1_timeout, int, 0); -module_param_named(control_timeout, w1_control_timeout, int, 0); module_param_named(max_slave_count, w1_max_slave_count, int, 0); module_param_named(slave_ttl, w1_max_slave_ttl, int, 0); DEFINE_MUTEX(w1_mlock); LIST_HEAD(w1_masters); -static struct task_struct *w1_control_thread; - static int w1_master_match(struct device *dev, struct device_driver *drv) { return 1; @@ -390,7 +386,7 @@ int w1_create_master_attributes(struct w1_master *master) return sysfs_create_group(&master->dev.kobj, &w1_master_defattr_group); } -static void w1_destroy_master_attributes(struct w1_master *master) +void w1_destroy_master_attributes(struct w1_master *master) { sysfs_remove_group(&master->dev.kobj, &w1_master_defattr_group); } @@ -567,7 +563,7 @@ static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn) return 0; } -static void w1_slave_detach(struct w1_slave *sl) +void w1_slave_detach(struct w1_slave *sl) { struct w1_netlink_msg msg; @@ -591,24 +587,6 @@ static void w1_slave_detach(struct w1_slave *sl) kfree(sl); } -static struct w1_master *w1_search_master(void *data) -{ - struct w1_master *dev; - int found = 0; - - mutex_lock(&w1_mlock); - list_for_each_entry(dev, &w1_masters, w1_master_entry) { - if (dev->bus_master->data == data) { - found = 1; - atomic_inc(&dev->refcnt); - break; - } - } - mutex_unlock(&w1_mlock); - - return (found)?dev:NULL; -} - struct w1_master *w1_search_master_id(u32 id) { struct w1_master *dev; @@ -656,34 +634,49 @@ struct w1_slave *w1_search_slave(struct w1_reg_num *id) return (found)?sl:NULL; } -void w1_reconnect_slaves(struct w1_family *f) +void w1_reconnect_slaves(struct w1_family *f, int attach) { + struct w1_slave *sl, *sln; struct w1_master *dev; mutex_lock(&w1_mlock); list_for_each_entry(dev, &w1_masters, w1_master_entry) { - dev_dbg(&dev->dev, "Reconnecting slaves in %s into new family %02x.\n", - dev->name, f->fid); - set_bit(W1_MASTER_NEED_RECONNECT, &dev->flags); + dev_dbg(&dev->dev, "Reconnecting slaves in device %s " + "for family %02x.\n", dev->name, f->fid); + mutex_lock(&dev->mutex); + list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) { + /* If it is a new family, slaves with the default + * family driver and are that family will be + * connected. If the family is going away, devices + * matching that family are reconneced. + */ + if ((attach && sl->family->fid == W1_FAMILY_DEFAULT + && sl->reg_num.family == f->fid) || + (!attach && sl->family->fid == f->fid)) { + struct w1_reg_num rn; + + memcpy(&rn, &sl->reg_num, sizeof(rn)); + w1_slave_detach(sl); + + w1_attach_slave_device(dev, &rn); + } + } + dev_dbg(&dev->dev, "Reconnecting slaves in device %s " + "has been finished.\n", dev->name); + mutex_unlock(&dev->mutex); } mutex_unlock(&w1_mlock); } -static void w1_slave_found(void *data, u64 rn) +static void w1_slave_found(struct w1_master *dev, u64 rn) { int slave_count; struct w1_slave *sl; struct list_head *ent; struct w1_reg_num *tmp; - struct w1_master *dev; u64 rn_le = cpu_to_le64(rn); - dev = w1_search_master(data); - if (!dev) { - printk(KERN_ERR "Failed to find w1 master device for data %p, " - "it is impossible.\n", data); - return; - } + atomic_inc(&dev->refcnt); tmp = (struct w1_reg_num *) &rn; @@ -785,76 +778,11 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb if ( (desc_bit == last_zero) || (last_zero < 0)) last_device = 1; desc_bit = last_zero; - cb(dev->bus_master->data, rn); + cb(dev, rn); } } } -static int w1_control(void *data) -{ - struct w1_slave *sl, *sln; - struct w1_master *dev, *n; - int have_to_wait = 0; - - set_freezable(); - while (!kthread_should_stop() || have_to_wait) { - have_to_wait = 0; - - try_to_freeze(); - msleep_interruptible(w1_control_timeout * 1000); - - list_for_each_entry_safe(dev, n, &w1_masters, w1_master_entry) { - if (!kthread_should_stop() && !dev->flags) - continue; - /* - * Little race: we can create thread but not set the flag. - * Get a chance for external process to set flag up. - */ - if (!dev->initialized) { - have_to_wait = 1; - continue; - } - - if (kthread_should_stop() || test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) { - set_bit(W1_MASTER_NEED_EXIT, &dev->flags); - - mutex_lock(&w1_mlock); - list_del(&dev->w1_master_entry); - mutex_unlock(&w1_mlock); - - mutex_lock(&dev->mutex); - list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) { - w1_slave_detach(sl); - } - w1_destroy_master_attributes(dev); - mutex_unlock(&dev->mutex); - atomic_dec(&dev->refcnt); - continue; - } - - if (test_bit(W1_MASTER_NEED_RECONNECT, &dev->flags)) { - dev_dbg(&dev->dev, "Reconnecting slaves in device %s.\n", dev->name); - mutex_lock(&dev->mutex); - list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) { - if (sl->family->fid == W1_FAMILY_DEFAULT) { - struct w1_reg_num rn; - - memcpy(&rn, &sl->reg_num, sizeof(rn)); - w1_slave_detach(sl); - - w1_attach_slave_device(dev, &rn); - } - } - dev_dbg(&dev->dev, "Reconnecting slaves in device %s has been finished.\n", dev->name); - clear_bit(W1_MASTER_NEED_RECONNECT, &dev->flags); - mutex_unlock(&dev->mutex); - } - } - } - - return 0; -} - void w1_search_process(struct w1_master *dev, u8 search_type) { struct w1_slave *sl, *sln; @@ -932,18 +860,13 @@ static int w1_init(void) goto err_out_master_unregister; } - w1_control_thread = kthread_run(w1_control, NULL, "w1_control"); - if (IS_ERR(w1_control_thread)) { - retval = PTR_ERR(w1_control_thread); - printk(KERN_ERR "Failed to create control thread. err=%d\n", - retval); - goto err_out_slave_unregister; - } - return 0; +#if 0 +/* For undoing the slave register if there was a step after it. */ err_out_slave_unregister: driver_unregister(&w1_slave_driver); +#endif err_out_master_unregister: driver_unregister(&w1_master_driver); @@ -959,13 +882,12 @@ static void w1_fini(void) { struct w1_master *dev; + /* Set netlink removal messages and some cleanup */ list_for_each_entry(dev, &w1_masters, w1_master_entry) __w1_remove_master_device(dev); w1_fini_netlink(); - kthread_stop(w1_control_thread); - driver_unregister(&w1_slave_driver); driver_unregister(&w1_master_driver); bus_unregister(&w1_bus_type); diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h index f1df5343f4ad..13e0ea880bf8 100644 --- a/drivers/w1/w1.h +++ b/drivers/w1/w1.h @@ -77,7 +77,7 @@ struct w1_slave struct completion released; }; -typedef void (* w1_slave_found_callback)(void *, u64); +typedef void (*w1_slave_found_callback)(struct w1_master *, u64); /** @@ -142,12 +142,14 @@ struct w1_bus_master */ u8 (*reset_bus)(void *); - /** Really nice hardware can handles the different types of ROM search */ - void (*search)(void *, u8, w1_slave_found_callback); + /** Really nice hardware can handles the different types of ROM search + * w1_master* is passed to the slave found callback. + */ + void (*search)(void *, struct w1_master *, + u8, w1_slave_found_callback); }; #define W1_MASTER_NEED_EXIT 0 -#define W1_MASTER_NEED_RECONNECT 1 struct w1_master { @@ -181,12 +183,21 @@ struct w1_master }; int w1_create_master_attributes(struct w1_master *); +void w1_destroy_master_attributes(struct w1_master *master); void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb); void w1_search_devices(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb); struct w1_slave *w1_search_slave(struct w1_reg_num *id); void w1_search_process(struct w1_master *dev, u8 search_type); struct w1_master *w1_search_master_id(u32 id); +/* Disconnect and reconnect devices in the given family. Used for finding + * unclaimed devices after a family has been registered or releasing devices + * after a family has been unregistered. Set attach to 1 when a new family + * has just been registered, to 0 when it has been unregistered. + */ +void w1_reconnect_slaves(struct w1_family *f, int attach); +void w1_slave_detach(struct w1_slave *sl); + u8 w1_triplet(struct w1_master *dev, int bdir); void w1_write_8(struct w1_master *, u8); int w1_reset_bus(struct w1_master *); diff --git a/drivers/w1/w1_family.c b/drivers/w1/w1_family.c index a3c95bd6890a..8c35f9ccb689 100644 --- a/drivers/w1/w1_family.c +++ b/drivers/w1/w1_family.c @@ -53,7 +53,8 @@ int w1_register_family(struct w1_family *newf) } spin_unlock(&w1_flock); - w1_reconnect_slaves(newf); + /* check default devices against the new set of drivers */ + w1_reconnect_slaves(newf, 1); return ret; } @@ -77,6 +78,9 @@ void w1_unregister_family(struct w1_family *fent) spin_unlock(&w1_flock); + /* deatch devices using this family code */ + w1_reconnect_slaves(fent, 0); + while (atomic_read(&fent->refcnt)) { printk(KERN_INFO "Waiting for family %u to become free: refcnt=%d.\n", fent->fid, atomic_read(&fent->refcnt)); diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h index ef1e1dafa19a..296a87edd922 100644 --- a/drivers/w1/w1_family.h +++ b/drivers/w1/w1_family.h @@ -63,6 +63,5 @@ void __w1_family_get(struct w1_family *); struct w1_family * w1_family_registered(u8); void w1_unregister_family(struct w1_family *); int w1_register_family(struct w1_family *); -void w1_reconnect_slaves(struct w1_family *f); #endif /* __W1_FAMILY_H */ diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index 6840dfebe4d4..ed3228017dad 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -148,10 +148,22 @@ err_out_free_dev: void __w1_remove_master_device(struct w1_master *dev) { struct w1_netlink_msg msg; + struct w1_slave *sl, *sln; set_bit(W1_MASTER_NEED_EXIT, &dev->flags); kthread_stop(dev->thread); + mutex_lock(&w1_mlock); + list_del(&dev->w1_master_entry); + mutex_unlock(&w1_mlock); + + mutex_lock(&dev->mutex); + list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) + w1_slave_detach(sl); + w1_destroy_master_attributes(dev); + mutex_unlock(&dev->mutex); + atomic_dec(&dev->refcnt); + while (atomic_read(&dev->refcnt)) { dev_info(&dev->dev, "Waiting for %s to become free: refcnt=%d.\n", dev->name, atomic_read(&dev->refcnt)); diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c index 30b6fbf83bd4..0056ef69009b 100644 --- a/drivers/w1/w1_io.c +++ b/drivers/w1/w1_io.c @@ -277,7 +277,8 @@ void w1_search_devices(struct w1_master *dev, u8 search_type, w1_slave_found_cal { dev->attempts++; if (dev->bus_master->search) - dev->bus_master->search(dev->bus_master->data, search_type, cb); + dev->bus_master->search(dev->bus_master->data, dev, + search_type, cb); else w1_search(dev, search_type, cb); } -- cgit v1.2.3 From 0d671b272af9eb06260ab3fd210d454e98dd4216 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:39 -0700 Subject: W1: abort search early on on exit Early abort if the master driver or the hardware goes away in the middle of a bus search operation. The alternative is to spam the print buffer up to 64*64 times with read errors in the case of USB. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 25640f681729..aac03f151fe0 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -772,6 +772,11 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb /* extract the direction taken & update the device number */ tmp64 = (triplet_ret >> 2); rn |= (tmp64 << i); + + if (test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) { + printk(KERN_INFO "Abort w1_search (exiting)\n"); + return; + } } if ( (triplet_ret & 0x03) != 0x03 ) { -- cgit v1.2.3 From 01e14d6db9654be005a0a5384090aea2cde39976 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:40 -0700 Subject: W1: don't delay search start Move the creation of the w1_process thread to after the device has been initialized. This way w1_process doesn't have to check to see if it has been initialized and the bus search can proceed without sleeping. That also eliminates two checks in the w1_process loop. The sleep now happens at the end of the loop not the beginning. Also added a comment for why the atomic_set was 2. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 19 ++++++------------- drivers/w1/w1_int.c | 26 +++++++++++++++++--------- 2 files changed, 23 insertions(+), 22 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index aac03f151fe0..730faa49d8dc 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -813,21 +813,14 @@ int w1_process(void *data) struct w1_master *dev = (struct w1_master *) data; while (!kthread_should_stop() && !test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) { + if (dev->search_count) { + mutex_lock(&dev->mutex); + w1_search_process(dev, W1_SEARCH); + mutex_unlock(&dev->mutex); + } + try_to_freeze(); msleep_interruptible(w1_timeout * 1000); - - if (kthread_should_stop() || test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) - break; - - if (!dev->initialized) - continue; - - if (dev->search_count == 0) - continue; - - mutex_lock(&dev->mutex); - w1_search_process(dev, W1_SEARCH); - mutex_unlock(&dev->mutex); } atomic_dec(&dev->refcnt); diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index ed3228017dad..36ff40250b61 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -61,6 +61,9 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl, dev->slave_ttl = slave_ttl; dev->search_count = -1; /* continual scan */ + /* 1 for w1_process to decrement + * 1 for __w1_remove_master_device to decrement + */ atomic_set(&dev->refcnt, 2); INIT_LIST_HEAD(&dev->slist); @@ -109,23 +112,23 @@ int w1_add_master_device(struct w1_bus_master *master) if (!dev) return -ENOMEM; + retval = w1_create_master_attributes(dev); + if (retval) + goto err_out_free_dev; + + memcpy(dev->bus_master, master, sizeof(struct w1_bus_master)); + + dev->initialized = 1; + dev->thread = kthread_run(&w1_process, dev, "%s", dev->name); if (IS_ERR(dev->thread)) { retval = PTR_ERR(dev->thread); dev_err(&dev->dev, "Failed to create new kernel thread. err=%d\n", retval); - goto err_out_free_dev; + goto err_out_rm_attr; } - retval = w1_create_master_attributes(dev); - if (retval) - goto err_out_kill_thread; - - memcpy(dev->bus_master, master, sizeof(struct w1_bus_master)); - - dev->initialized = 1; - mutex_lock(&w1_mlock); list_add(&dev->w1_master_entry, &w1_masters); mutex_unlock(&w1_mlock); @@ -137,8 +140,13 @@ int w1_add_master_device(struct w1_bus_master *master) return 0; +#if 0 /* Thread cleanup code, not required currently. */ err_out_kill_thread: + set_bit(W1_MASTER_NEED_EXIT, &dev->flags); kthread_stop(dev->thread); +#endif +err_out_rm_attr: + w1_destroy_master_attributes(dev); err_out_free_dev: w1_free_dev(dev); -- cgit v1.2.3 From 3c52e4e627896b42152cc6ff98216c302932227e Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:41 -0700 Subject: W1: w1_process, block or sleep The w1_process thread's sleeping and termination has been modified. msleep_interruptible was replaced by schedule_timeout and schedule to allow for kthread_stop and wake_up_process to interrupt the sleep and the unbounded sleeping when a bus search is disabled. The W1_MASTER_NEED_EXIT and flags variable were removed as they were redundant with kthread_should_stop and kthread_stop. If w1_process is sleeping, requesting a search will immediately wake it up rather than waiting for the end of msleep_interruptible previously. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 20 +++++++++++++++++--- drivers/w1/w1.h | 4 ---- drivers/w1/w1_int.c | 2 -- 3 files changed, 17 insertions(+), 9 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 730faa49d8dc..9b5c11701c37 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -251,6 +251,7 @@ static ssize_t w1_master_attribute_store_search(struct device * dev, mutex_lock(&md->mutex); md->search_count = simple_strtol(buf, NULL, 0); mutex_unlock(&md->mutex); + wake_up_process(md->thread); return count; } @@ -773,7 +774,7 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb tmp64 = (triplet_ret >> 2); rn |= (tmp64 << i); - if (test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) { + if (kthread_should_stop()) { printk(KERN_INFO "Abort w1_search (exiting)\n"); return; } @@ -811,8 +812,12 @@ void w1_search_process(struct w1_master *dev, u8 search_type) int w1_process(void *data) { struct w1_master *dev = (struct w1_master *) data; + /* As long as w1_timeout is only set by a module parameter the sleep + * time can be calculated in jiffies once. + */ + const unsigned long jtime = msecs_to_jiffies(w1_timeout * 1000); - while (!kthread_should_stop() && !test_bit(W1_MASTER_NEED_EXIT, &dev->flags)) { + while (!kthread_should_stop()) { if (dev->search_count) { mutex_lock(&dev->mutex); w1_search_process(dev, W1_SEARCH); @@ -820,7 +825,16 @@ int w1_process(void *data) } try_to_freeze(); - msleep_interruptible(w1_timeout * 1000); + __set_current_state(TASK_INTERRUPTIBLE); + + if (kthread_should_stop()) + break; + + /* Only sleep when the search is active. */ + if (dev->search_count) + schedule_timeout(jtime); + else + schedule(); } atomic_dec(&dev->refcnt); diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h index 13e0ea880bf8..34ee01e008ad 100644 --- a/drivers/w1/w1.h +++ b/drivers/w1/w1.h @@ -149,8 +149,6 @@ struct w1_bus_master u8, w1_slave_found_callback); }; -#define W1_MASTER_NEED_EXIT 0 - struct w1_master { struct list_head w1_master_entry; @@ -169,8 +167,6 @@ struct w1_master void *priv; int priv_size; - long flags; - struct task_struct *thread; struct mutex mutex; diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index 36ff40250b61..bd877b24ce42 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -142,7 +142,6 @@ int w1_add_master_device(struct w1_bus_master *master) #if 0 /* Thread cleanup code, not required currently. */ err_out_kill_thread: - set_bit(W1_MASTER_NEED_EXIT, &dev->flags); kthread_stop(dev->thread); #endif err_out_rm_attr: @@ -158,7 +157,6 @@ void __w1_remove_master_device(struct w1_master *dev) struct w1_netlink_msg msg; struct w1_slave *sl, *sln; - set_bit(W1_MASTER_NEED_EXIT, &dev->flags); kthread_stop(dev->thread); mutex_lock(&w1_mlock); -- cgit v1.2.3 From 6a158c0de791a81eb761ccf26ead1bd0834abac2 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:42 -0700 Subject: W1: feature, enable hardware strong pullup Add a strong pullup option to the w1 system. This supplies extra power for parasite powered devices. There is a w1_master_pullup sysfs entry and enable_pullup module parameter to enable or disable the strong pullup. The one wire bus requires at a minimum one wire and ground. The common wire is used for sending and receiving data as well as supplying power to devices that are parasite powered of which temperature sensors can be one example. The bus must be idle and left high while a temperature conversion is in progress, in addition the normal pullup resister on larger networks or even higher temperatures might not supply enough power. The pullup resister can't provide too much pullup current, because devices need to pull the bus down to write a value. This enables the strong pullup for supported hardware, which can supply more current when requested. Unsupported hardware will just delay with the bus high. The hardware USB 2490 one wire bus master has a bit on some commands which will enable the strong pullup as soon as the command finishes executing. To use strong pullup, call the new w1_next_pullup function to register the duration. The next write command will call set_pullup before sending the data, and reset the duration to zero once it returns. Switched from simple_strtol to strict_strtol. Signed-off-by: David Fries Cc: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 40 ++++++++++++++++++++++++++++++- drivers/w1/w1.h | 12 ++++++++++ drivers/w1/w1_int.c | 16 +++++++++++++ drivers/w1/w1_io.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 131 insertions(+), 5 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 9b5c11701c37..32418d4e555a 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -246,10 +246,14 @@ static ssize_t w1_master_attribute_store_search(struct device * dev, struct device_attribute *attr, const char * buf, size_t count) { + long tmp; struct w1_master *md = dev_to_w1_master(dev); + if (strict_strtol(buf, 0, &tmp) == -EINVAL) + return -EINVAL; + mutex_lock(&md->mutex); - md->search_count = simple_strtol(buf, NULL, 0); + md->search_count = tmp; mutex_unlock(&md->mutex); wake_up_process(md->thread); @@ -270,6 +274,38 @@ static ssize_t w1_master_attribute_show_search(struct device *dev, return count; } +static ssize_t w1_master_attribute_store_pullup(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + long tmp; + struct w1_master *md = dev_to_w1_master(dev); + + if (strict_strtol(buf, 0, &tmp) == -EINVAL) + return -EINVAL; + + mutex_lock(&md->mutex); + md->enable_pullup = tmp; + mutex_unlock(&md->mutex); + wake_up_process(md->thread); + + return count; +} + +static ssize_t w1_master_attribute_show_pullup(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct w1_master *md = dev_to_w1_master(dev); + ssize_t count; + + mutex_lock(&md->mutex); + count = sprintf(buf, "%d\n", md->enable_pullup); + mutex_unlock(&md->mutex); + + return count; +} + static ssize_t w1_master_attribute_show_pointer(struct device *dev, struct device_attribute *attr, char *buf) { struct w1_master *md = dev_to_w1_master(dev); @@ -365,6 +401,7 @@ static W1_MASTER_ATTR_RO(attempts, S_IRUGO); static W1_MASTER_ATTR_RO(timeout, S_IRUGO); static W1_MASTER_ATTR_RO(pointer, S_IRUGO); static W1_MASTER_ATTR_RW(search, S_IRUGO | S_IWUGO); +static W1_MASTER_ATTR_RW(pullup, S_IRUGO | S_IWUGO); static struct attribute *w1_master_default_attrs[] = { &w1_master_attribute_name.attr, @@ -375,6 +412,7 @@ static struct attribute *w1_master_default_attrs[] = { &w1_master_attribute_timeout.attr, &w1_master_attribute_pointer.attr, &w1_master_attribute_search.attr, + &w1_master_attribute_pullup.attr, NULL }; diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h index 34ee01e008ad..00b84ab22808 100644 --- a/drivers/w1/w1.h +++ b/drivers/w1/w1.h @@ -142,6 +142,12 @@ struct w1_bus_master */ u8 (*reset_bus)(void *); + /** + * Put out a strong pull-up pulse of the specified duration. + * @return -1=Error, 0=completed + */ + u8 (*set_pullup)(void *, int); + /** Really nice hardware can handles the different types of ROM search * w1_master* is passed to the slave found callback. */ @@ -167,6 +173,11 @@ struct w1_master void *priv; int priv_size; + /** 5V strong pullup enabled flag, 1 enabled, zero disabled. */ + int enable_pullup; + /** 5V strong pullup duration in milliseconds, zero disabled. */ + int pullup_duration; + struct task_struct *thread; struct mutex mutex; @@ -201,6 +212,7 @@ u8 w1_calc_crc8(u8 *, int); void w1_write_block(struct w1_master *, const u8 *, int); u8 w1_read_block(struct w1_master *, u8 *, int); int w1_reset_select_slave(struct w1_slave *sl); +void w1_next_pullup(struct w1_master *, int); static inline struct w1_slave* dev_to_w1_slave(struct device *dev) { diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index bd877b24ce42..9d723efdf915 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -31,6 +31,9 @@ static u32 w1_ids = 1; +static int w1_enable_pullup = 1; +module_param_named(enable_pullup, w1_enable_pullup, int, 0); + static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl, struct device_driver *driver, struct device *device) @@ -59,6 +62,7 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl, dev->initialized = 0; dev->id = id; dev->slave_ttl = slave_ttl; + dev->enable_pullup = w1_enable_pullup; dev->search_count = -1; /* continual scan */ /* 1 for w1_process to decrement @@ -107,6 +111,18 @@ int w1_add_master_device(struct w1_bus_master *master) printk(KERN_ERR "w1_add_master_device: invalid function set\n"); return(-EINVAL); } + /* While it would be electrically possible to make a device that + * generated a strong pullup in bit bang mode, only hardare that + * controls 1-wire time frames are even expected to support a strong + * pullup. w1_io.c would need to support calling set_pullup before + * the last write_bit operation of a w1_write_8 which it currently + * doesn't. + */ + if (!master->write_byte && !master->touch_bit && master->set_pullup) { + printk(KERN_ERR "w1_add_master_device: set_pullup requires " + "write_byte or touch_bit, disabling\n"); + master->set_pullup = NULL; + } dev = w1_alloc_dev(w1_ids++, w1_max_slave_count, w1_max_slave_ttl, &w1_master_driver, &w1_master_device); if (!dev) diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c index 0056ef69009b..97b338a16abc 100644 --- a/drivers/w1/w1_io.c +++ b/drivers/w1/w1_io.c @@ -92,6 +92,40 @@ static void w1_write_bit(struct w1_master *dev, int bit) } } +/** + * Pre-write operation, currently only supporting strong pullups. + * Program the hardware for a strong pullup, if one has been requested and + * the hardware supports it. + * + * @param dev the master device + */ +static void w1_pre_write(struct w1_master *dev) +{ + if (dev->pullup_duration && + dev->enable_pullup && dev->bus_master->set_pullup) { + dev->bus_master->set_pullup(dev->bus_master->data, + dev->pullup_duration); + } +} + +/** + * Post-write operation, currently only supporting strong pullups. + * If a strong pullup was requested, clear it if the hardware supports + * them, or execute the delay otherwise, in either case clear the request. + * + * @param dev the master device + */ +static void w1_post_write(struct w1_master *dev) +{ + if (dev->pullup_duration) { + if (dev->enable_pullup && dev->bus_master->set_pullup) + dev->bus_master->set_pullup(dev->bus_master->data, 0); + else + msleep(dev->pullup_duration); + dev->pullup_duration = 0; + } +} + /** * Writes 8 bits. * @@ -102,11 +136,17 @@ void w1_write_8(struct w1_master *dev, u8 byte) { int i; - if (dev->bus_master->write_byte) + if (dev->bus_master->write_byte) { + w1_pre_write(dev); dev->bus_master->write_byte(dev->bus_master->data, byte); + } else - for (i = 0; i < 8; ++i) + for (i = 0; i < 8; ++i) { + if (i == 7) + w1_pre_write(dev); w1_touch_bit(dev, (byte >> i) & 0x1); + } + w1_post_write(dev); } EXPORT_SYMBOL_GPL(w1_write_8); @@ -203,11 +243,14 @@ void w1_write_block(struct w1_master *dev, const u8 *buf, int len) { int i; - if (dev->bus_master->write_block) + if (dev->bus_master->write_block) { + w1_pre_write(dev); dev->bus_master->write_block(dev->bus_master->data, buf, len); + } else for (i = 0; i < len; ++i) - w1_write_8(dev, buf[i]); + w1_write_8(dev, buf[i]); /* calls w1_pre_write */ + w1_post_write(dev); } EXPORT_SYMBOL_GPL(w1_write_block); @@ -306,3 +349,20 @@ int w1_reset_select_slave(struct w1_slave *sl) return 0; } EXPORT_SYMBOL_GPL(w1_reset_select_slave); + +/** + * Put out a strong pull-up of the specified duration after the next write + * operation. Not all hardware supports strong pullups. Hardware that + * doesn't support strong pullups will sleep for the given time after the + * write operation without a strong pullup. This is a one shot request for + * the next write, specifying zero will clear a previous request. + * The w1 master lock must be held. + * + * @param delay time in milliseconds + * @return 0=success, anything else=error + */ +void w1_next_pullup(struct w1_master *dev, int delay) +{ + dev->pullup_duration = delay; +} +EXPORT_SYMBOL_GPL(w1_next_pullup); -- cgit v1.2.3 From 6cd159744eaf212f3729d154f3881230a7c19eb2 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:43 -0700 Subject: W1: feature, w1_therm.c use strong pullup and documentation Added strong pullup to thermal sensor driver and general documentation on the sensor. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- Documentation/w1/00-INDEX | 2 ++ Documentation/w1/slaves/00-INDEX | 4 ++++ Documentation/w1/slaves/w1_therm | 41 ++++++++++++++++++++++++++++++++++++++++ drivers/w1/slaves/w1_therm.c | 15 +++++++++++++-- 4 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 Documentation/w1/slaves/00-INDEX create mode 100644 Documentation/w1/slaves/w1_therm (limited to 'drivers/w1') diff --git a/Documentation/w1/00-INDEX b/Documentation/w1/00-INDEX index 5270cf4cb109..cb49802745dc 100644 --- a/Documentation/w1/00-INDEX +++ b/Documentation/w1/00-INDEX @@ -1,5 +1,7 @@ 00-INDEX - This file +slaves/ + - Drivers that provide support for specific family codes. masters/ - Individual chips providing 1-wire busses. w1.generic diff --git a/Documentation/w1/slaves/00-INDEX b/Documentation/w1/slaves/00-INDEX new file mode 100644 index 000000000000..f8101d6b07b7 --- /dev/null +++ b/Documentation/w1/slaves/00-INDEX @@ -0,0 +1,4 @@ +00-INDEX + - This file +w1_therm + - The Maxim/Dallas Semiconductor ds18*20 temperature sensor. diff --git a/Documentation/w1/slaves/w1_therm b/Documentation/w1/slaves/w1_therm new file mode 100644 index 000000000000..0403aaaba878 --- /dev/null +++ b/Documentation/w1/slaves/w1_therm @@ -0,0 +1,41 @@ +Kernel driver w1_therm +==================== + +Supported chips: + * Maxim ds18*20 based temperature sensors. + +Author: Evgeniy Polyakov + + +Description +----------- + +w1_therm provides basic temperature conversion for ds18*20 devices. +supported family codes: +W1_THERM_DS18S20 0x10 +W1_THERM_DS1822 0x22 +W1_THERM_DS18B20 0x28 + +Support is provided through the sysfs w1_slave file. Each open and +read sequence will initiate a temperature conversion then provide two +lines of ASCII output. The first line contains the nine hex bytes +read along with a calculated crc value and YES or NO if it matched. +If the crc matched the returned values are retained. The second line +displays the retained values along with a temperature in millidegrees +Centigrade after t=. + +Parasite powered devices are limited to one slave performing a +temperature conversion at a time. If none of the devices are parasite +powered it would be possible to convert all the devices at the same +time and then go back to read individual sensors. That isn't +currently supported. The driver also doesn't support reduced +precision (which would also reduce the conversion time). + +The module parameter strong_pullup can be set to 0 to disable the +strong pullup or 1 to enable. If enabled the 5V strong pullup will be +enabled when the conversion is taking place provided the master driver +must support the strong pullup (or it falls back to a pullup +resistor). The DS18b20 temperature sensor specification lists a +maximum current draw of 1.5mA and that a 5k pullup resistor is not +sufficient. The strong pullup is designed to provide the additional +current required. diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index fb28acaeed6c..e87f464a6fb0 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -37,6 +37,14 @@ MODULE_LICENSE("GPL"); MODULE_AUTHOR("Evgeniy Polyakov "); MODULE_DESCRIPTION("Driver for 1-wire Dallas network protocol, temperature family."); +/* Allow the strong pullup to be disabled, but default to enabled. + * If it was disabled a parasite powered device might not get the require + * current to do a temperature conversion. If it is enabled parasite powered + * devices have a better chance of getting the current required. + */ +static int w1_strong_pullup = 1; +module_param_named(strong_pullup, w1_strong_pullup, int, 0); + static u8 bad_roms[][9] = { {0xaa, 0x00, 0x4b, 0x46, 0xff, 0xff, 0x0c, 0x10, 0x87}, {} @@ -192,9 +200,12 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj, int count = 0; unsigned int tm = 750; + /* 750ms strong pullup (or delay) after the convert */ + if (w1_strong_pullup) + w1_next_pullup(dev, tm); w1_write_8(dev, W1_CONVERT_TEMP); - - msleep(tm); + if (!w1_strong_pullup) + msleep(tm); if (!w1_reset_select_slave(sl)) { -- cgit v1.2.3 From 9b46741119590bf23c5c519b49024eb2001cfafa Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:43 -0700 Subject: W1: be able to manually add and remove slaves sysfs entries were added to manually add and remove slave devices. This is useful if the automatic bus searching is disabled, and the device ids are already known. [akpm@linux-foundation.org: fix printk types] Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 32418d4e555a..f3be2991e6e8 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -56,6 +56,8 @@ module_param_named(slave_ttl, w1_max_slave_ttl, int, 0); DEFINE_MUTEX(w1_mlock); LIST_HEAD(w1_masters); +static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn); + static int w1_master_match(struct device *dev, struct device_driver *drv) { return 1; @@ -357,7 +359,8 @@ static ssize_t w1_master_attribute_show_slave_count(struct device *dev, struct d return count; } -static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device_attribute *attr, char *buf) +static ssize_t w1_master_attribute_show_slaves(struct device *dev, + struct device_attribute *attr, char *buf) { struct w1_master *md = dev_to_w1_master(dev); int c = PAGE_SIZE; @@ -382,6 +385,135 @@ static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device return PAGE_SIZE - c; } +static ssize_t w1_master_attribute_show_add(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int c = PAGE_SIZE; + c -= snprintf(buf+PAGE_SIZE - c, c, + "write device id xx-xxxxxxxxxxxx to add slave\n"); + return PAGE_SIZE - c; +} + +static int w1_atoreg_num(struct device *dev, const char *buf, size_t count, + struct w1_reg_num *rn) +{ + unsigned int family; + unsigned long long id; + int i; + u64 rn64_le; + + /* The CRC value isn't read from the user because the sysfs directory + * doesn't include it and most messages from the bus search don't + * print it either. It would be unreasonable for the user to then + * provide it. + */ + const char *error_msg = "bad slave string format, expecting " + "ff-dddddddddddd\n"; + + if (buf[2] != '-') { + dev_err(dev, "%s", error_msg); + return -EINVAL; + } + i = sscanf(buf, "%02x-%012llx", &family, &id); + if (i != 2) { + dev_err(dev, "%s", error_msg); + return -EINVAL; + } + rn->family = family; + rn->id = id; + + rn64_le = cpu_to_le64(*(u64 *)rn); + rn->crc = w1_calc_crc8((u8 *)&rn64_le, 7); + +#if 0 + dev_info(dev, "With CRC device is %02x.%012llx.%02x.\n", + rn->family, (unsigned long long)rn->id, rn->crc); +#endif + + return 0; +} + +/* Searches the slaves in the w1_master and returns a pointer or NULL. + * Note: must hold the mutex + */ +static struct w1_slave *w1_slave_search_device(struct w1_master *dev, + struct w1_reg_num *rn) +{ + struct w1_slave *sl; + list_for_each_entry(sl, &dev->slist, w1_slave_entry) { + if (sl->reg_num.family == rn->family && + sl->reg_num.id == rn->id && + sl->reg_num.crc == rn->crc) { + return sl; + } + } + return NULL; +} + +static ssize_t w1_master_attribute_store_add(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct w1_master *md = dev_to_w1_master(dev); + struct w1_reg_num rn; + struct w1_slave *sl; + ssize_t result = count; + + if (w1_atoreg_num(dev, buf, count, &rn)) + return -EINVAL; + + mutex_lock(&md->mutex); + sl = w1_slave_search_device(md, &rn); + /* It would be nice to do a targeted search one the one-wire bus + * for the new device to see if it is out there or not. But the + * current search doesn't support that. + */ + if (sl) { + dev_info(dev, "Device %s already exists\n", sl->name); + result = -EINVAL; + } else { + w1_attach_slave_device(md, &rn); + } + mutex_unlock(&md->mutex); + + return result; +} + +static ssize_t w1_master_attribute_show_remove(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int c = PAGE_SIZE; + c -= snprintf(buf+PAGE_SIZE - c, c, + "write device id xx-xxxxxxxxxxxx to remove slave\n"); + return PAGE_SIZE - c; +} + +static ssize_t w1_master_attribute_store_remove(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct w1_master *md = dev_to_w1_master(dev); + struct w1_reg_num rn; + struct w1_slave *sl; + ssize_t result = count; + + if (w1_atoreg_num(dev, buf, count, &rn)) + return -EINVAL; + + mutex_lock(&md->mutex); + sl = w1_slave_search_device(md, &rn); + if (sl) { + w1_slave_detach(sl); + } else { + dev_info(dev, "Device %02x-%012llx doesn't exists\n", rn.family, + (unsigned long long)rn.id); + result = -EINVAL; + } + mutex_unlock(&md->mutex); + + return result; +} + #define W1_MASTER_ATTR_RO(_name, _mode) \ struct device_attribute w1_master_attribute_##_name = \ __ATTR(w1_master_##_name, _mode, \ @@ -402,6 +534,8 @@ static W1_MASTER_ATTR_RO(timeout, S_IRUGO); static W1_MASTER_ATTR_RO(pointer, S_IRUGO); static W1_MASTER_ATTR_RW(search, S_IRUGO | S_IWUGO); static W1_MASTER_ATTR_RW(pullup, S_IRUGO | S_IWUGO); +static W1_MASTER_ATTR_RW(add, S_IRUGO | S_IWUGO); +static W1_MASTER_ATTR_RW(remove, S_IRUGO | S_IWUGO); static struct attribute *w1_master_default_attrs[] = { &w1_master_attribute_name.attr, @@ -413,6 +547,8 @@ static struct attribute *w1_master_default_attrs[] = { &w1_master_attribute_pointer.attr, &w1_master_attribute_search.attr, &w1_master_attribute_pullup.attr, + &w1_master_attribute_add.attr, + &w1_master_attribute_remove.attr, NULL }; -- cgit v1.2.3 From cd7b28d33d0cabdc86fa7d546da07b9385274bbb Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:44 -0700 Subject: W1: recode w1_slave_found logic Simplified the logic in w1_slave_found by using the new w1_attach_slave_device function to find a slave and mark it as active or add the device if the crc checks. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index f3be2991e6e8..b1b21df835f5 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -845,9 +845,7 @@ void w1_reconnect_slaves(struct w1_family *f, int attach) static void w1_slave_found(struct w1_master *dev, u64 rn) { - int slave_count; struct w1_slave *sl; - struct list_head *ent; struct w1_reg_num *tmp; u64 rn_le = cpu_to_le64(rn); @@ -855,24 +853,12 @@ static void w1_slave_found(struct w1_master *dev, u64 rn) tmp = (struct w1_reg_num *) &rn; - slave_count = 0; - list_for_each(ent, &dev->slist) { - - sl = list_entry(ent, struct w1_slave, w1_slave_entry); - - if (sl->reg_num.family == tmp->family && - sl->reg_num.id == tmp->id && - sl->reg_num.crc == tmp->crc) { - set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags); - break; - } - - slave_count++; - } - - if (slave_count == dev->slave_count && - rn && ((rn >> 56) & 0xff) == w1_calc_crc8((u8 *)&rn_le, 7)) { - w1_attach_slave_device(dev, tmp); + sl = w1_slave_search_device(dev, tmp); + if (sl) { + set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags); + } else { + if (rn && tmp->crc == w1_calc_crc8((u8 *)&rn_le, 7)) + w1_attach_slave_device(dev, tmp); } atomic_dec(&dev->refcnt); -- cgit v1.2.3 From 9141f57c7edd40a48a41b7e31427c4b2831a36af Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:45 -0700 Subject: W1: new module parameter search_count Added a new module parameter search_count which allows overriding the default search count. -1 continual, 0 disabled, N that many times. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1_int.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index 9d723efdf915..3fd6e6651fbe 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -30,6 +30,8 @@ #include "w1_int.h" static u32 w1_ids = 1; +static int w1_search_count = -1; /* Default is continual scan */ +module_param_named(search_count, w1_search_count, int, 0); static int w1_enable_pullup = 1; module_param_named(enable_pullup, w1_enable_pullup, int, 0); @@ -62,8 +64,8 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl, dev->initialized = 0; dev->id = id; dev->slave_ttl = slave_ttl; + dev->search_count = w1_search_count; dev->enable_pullup = w1_enable_pullup; - dev->search_count = -1; /* continual scan */ /* 1 for w1_process to decrement * 1 for __w1_remove_master_device to decrement -- cgit v1.2.3 From 07e003417b88deac4b887c98f499fc3b01bc8df0 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:50 -0700 Subject: W1: w1_slave_read_id read bug, use device_attribute Fix bug reading the id sysfs file. If less than the full 8 bytes were read, the next read would start at the first byte instead of continuing. It needed the offset added to memcpy, or the better solution was to replace it with the device attribute instead of bin attribute. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index b1b21df835f5..04e7de4b266a 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -103,35 +103,20 @@ static ssize_t w1_slave_read_name(struct device *dev, struct device_attribute *a return sprintf(buf, "%s\n", sl->name); } -static ssize_t w1_slave_read_id(struct kobject *kobj, - struct bin_attribute *bin_attr, - char *buf, loff_t off, size_t count) +static ssize_t w1_slave_read_id(struct device *dev, + struct device_attribute *attr, char *buf) { - struct w1_slave *sl = kobj_to_w1_slave(kobj); - - if (off > 8) { - count = 0; - } else { - if (off + count > 8) - count = 8 - off; - - memcpy(buf, (u8 *)&sl->reg_num, count); - } + struct w1_slave *sl = dev_to_w1_slave(dev); + ssize_t count = sizeof(sl->reg_num); + memcpy(buf, (u8 *)&sl->reg_num, count); return count; } static struct device_attribute w1_slave_attr_name = __ATTR(name, S_IRUGO, w1_slave_read_name, NULL); - -static struct bin_attribute w1_slave_attr_bin_id = { - .attr = { - .name = "id", - .mode = S_IRUGO, - }, - .size = 8, - .read = w1_slave_read_id, -}; +static struct device_attribute w1_slave_attr_id = + __ATTR(id, S_IRUGO, w1_slave_read_id, NULL); /* Default family */ @@ -650,7 +635,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl) } /* Create "id" entry */ - err = sysfs_create_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id); + err = device_create_file(&sl->dev, &w1_slave_attr_id); if (err < 0) { dev_err(&sl->dev, "sysfs file creation for [%s] failed. err=%d\n", @@ -672,7 +657,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl) return 0; out_rem2: - sysfs_remove_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id); + device_remove_file(&sl->dev, &w1_slave_attr_id); out_rem1: device_remove_file(&sl->dev, &w1_slave_attr_name); out_unreg: @@ -754,7 +739,7 @@ void w1_slave_detach(struct w1_slave *sl) msg.type = W1_SLAVE_REMOVE; w1_netlink_send(sl->master, &msg); - sysfs_remove_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id); + device_remove_file(&sl->dev, &w1_slave_attr_id); device_remove_file(&sl->dev, &w1_slave_attr_name); device_unregister(&sl->dev); -- cgit v1.2.3 From 347ba8a588c3e49f357291e5a1ac38a11d7e052d Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:51 -0700 Subject: W1: w1_therm fix user buffer overflow and cat Fixed data reading bug by replacing binary attribute with device one. Switching the sysfs read from bin_attribute to device_attribute. The data is far under PAGE_SIZE so the binary interface isn't required. As the device_attribute interface will make one call to w1_therm_read per file open and buffer, the result is, the following problems go away. buffer overflow: Execute a short read on w1_slave and w1_therm_read_bin would still return the full string size worth of data clobbering the user space buffer when it returned. Switching to device_attribute avoids the buffer overflow problems. With the snprintf formatted output dealing with short reads without doing a conversion per read would have been difficult. bad behavior: `cat w1_slave` would cause two temperature conversions to take place. Previously the code assumed W1_SLAVE_DATA_SIZE would be returned with each read. It would not return 0 unless the offset was less than W1_SLAVE_DATA_SIZE. The result was the first read did a temperature conversion, filled the buffer and returned, the offset in the second read would be less than W1_SLAVE_DATA_SIZE and also fill the buffer and return, the third read would finnally have a big enough offset to return 0 and cause cat to stop. Now w1_therm_read will be called at most once per open. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/slaves/w1_therm.c | 55 ++++++++++++++++---------------------------- drivers/w1/w1.h | 1 - 2 files changed, 20 insertions(+), 36 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index e87f464a6fb0..7de99dfd11c8 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -50,26 +50,20 @@ static u8 bad_roms[][9] = { {} }; -static ssize_t w1_therm_read_bin(struct kobject *, struct bin_attribute *, - char *, loff_t, size_t); +static ssize_t w1_therm_read(struct device *device, + struct device_attribute *attr, char *buf); -static struct bin_attribute w1_therm_bin_attr = { - .attr = { - .name = "w1_slave", - .mode = S_IRUGO, - }, - .size = W1_SLAVE_DATA_SIZE, - .read = w1_therm_read_bin, -}; +static struct device_attribute w1_therm_attr = + __ATTR(w1_slave, S_IRUGO, w1_therm_read, NULL); static int w1_therm_add_slave(struct w1_slave *sl) { - return sysfs_create_bin_file(&sl->dev.kobj, &w1_therm_bin_attr); + return device_create_file(&sl->dev, &w1_therm_attr); } static void w1_therm_remove_slave(struct w1_slave *sl) { - sysfs_remove_bin_file(&sl->dev.kobj, &w1_therm_bin_attr); + device_remove_file(&sl->dev, &w1_therm_attr); } static struct w1_family_ops w1_therm_fops = { @@ -168,30 +162,19 @@ static int w1_therm_check_rom(u8 rom[9]) return 0; } -static ssize_t w1_therm_read_bin(struct kobject *kobj, - struct bin_attribute *bin_attr, - char *buf, loff_t off, size_t count) +static ssize_t w1_therm_read(struct device *device, + struct device_attribute *attr, char *buf) { - struct w1_slave *sl = kobj_to_w1_slave(kobj); + struct w1_slave *sl = dev_to_w1_slave(device); struct w1_master *dev = sl->master; u8 rom[9], crc, verdict; int i, max_trying = 10; + ssize_t c = PAGE_SIZE; mutex_lock(&sl->master->mutex); - if (off > W1_SLAVE_DATA_SIZE) { - count = 0; - goto out; - } - if (off + count > W1_SLAVE_DATA_SIZE) { - count = 0; - goto out; - } - - memset(buf, 0, count); memset(rom, 0, sizeof(rom)); - count = 0; verdict = 0; crc = 0; @@ -211,7 +194,9 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj, w1_write_8(dev, W1_READ_SCRATCHPAD); if ((count = w1_read_block(dev, rom, 9)) != 9) { - dev_warn(&dev->dev, "w1_read_block() returned %d instead of 9.\n", count); + dev_warn(device, "w1_read_block() " + "returned %u instead of 9.\n", + count); } crc = w1_calc_crc8(rom, 8); @@ -226,22 +211,22 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj, } for (i = 0; i < 9; ++i) - count += sprintf(buf + count, "%02x ", rom[i]); - count += sprintf(buf + count, ": crc=%02x %s\n", + c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", rom[i]); + c -= snprintf(buf + PAGE_SIZE - c, c, ": crc=%02x %s\n", crc, (verdict) ? "YES" : "NO"); if (verdict) memcpy(sl->rom, rom, sizeof(sl->rom)); else - dev_warn(&dev->dev, "18S20 doesn't respond to CONVERT_TEMP.\n"); + dev_warn(device, "18S20 doesn't respond to CONVERT_TEMP.\n"); for (i = 0; i < 9; ++i) - count += sprintf(buf + count, "%02x ", sl->rom[i]); + c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", sl->rom[i]); - count += sprintf(buf + count, "t=%d\n", w1_convert_temp(rom, sl->family->fid)); -out: + c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n", + w1_convert_temp(rom, sl->family->fid)); mutex_unlock(&dev->mutex); - return count; + return PAGE_SIZE - c; } static int __init w1_therm_init(void) diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h index 00b84ab22808..cdaa6fffbfc7 100644 --- a/drivers/w1/w1.h +++ b/drivers/w1/w1.h @@ -46,7 +46,6 @@ struct w1_reg_num #include "w1_family.h" #define W1_MAXNAMELEN 32 -#define W1_SLAVE_DATA_SIZE 128 #define W1_SEARCH 0xF0 #define W1_ALARM_SEARCH 0xEC -- cgit v1.2.3 From fe3cb82364332b9db3b574e9e41de9c27eff470a Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:52 -0700 Subject: W1: w1_family, remove unused variable need_exit Removed the w1_family structure member variable need_exit. It was only being set and never used. Even if it were to be used it is a polling type operation. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1_family.c | 7 +------ drivers/w1/w1_family.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1_family.c b/drivers/w1/w1_family.c index 8c35f9ccb689..4a099041f28a 100644 --- a/drivers/w1/w1_family.c +++ b/drivers/w1/w1_family.c @@ -48,7 +48,6 @@ int w1_register_family(struct w1_family *newf) if (!ret) { atomic_set(&newf->refcnt, 0); - newf->need_exit = 0; list_add_tail(&newf->family_entry, &w1_families); } spin_unlock(&w1_flock); @@ -73,9 +72,6 @@ void w1_unregister_family(struct w1_family *fent) break; } } - - fent->need_exit = 1; - spin_unlock(&w1_flock); /* deatch devices using this family code */ @@ -113,8 +109,7 @@ struct w1_family * w1_family_registered(u8 fid) static void __w1_family_put(struct w1_family *f) { - if (atomic_dec_and_test(&f->refcnt)) - f->need_exit = 1; + atomic_dec(&f->refcnt); } void w1_family_put(struct w1_family *f) diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h index 296a87edd922..30531331a643 100644 --- a/drivers/w1/w1_family.h +++ b/drivers/w1/w1_family.h @@ -53,7 +53,6 @@ struct w1_family struct w1_family_ops *fops; atomic_t refcnt; - u8 need_exit; }; extern spinlock_t w1_flock; -- cgit v1.2.3 From e0d29c7699de723432da268748aefe9624fc8529 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:52 -0700 Subject: W1: w1_therm consistent mutex access code cleanup sl->master->mutex and dev->mutex refer to the same mutex variable, but be consistent and use the same set of pointers for the lock and unlock calls. It is less confusing (and one less pointer dereference this way). Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/slaves/w1_therm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index 7de99dfd11c8..2c8dff9f77da 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -171,7 +171,7 @@ static ssize_t w1_therm_read(struct device *device, int i, max_trying = 10; ssize_t c = PAGE_SIZE; - mutex_lock(&sl->master->mutex); + mutex_lock(&dev->mutex); memset(rom, 0, sizeof(rom)); -- cgit v1.2.3 From af00a2d5a047455b35d1e7dc4c7d9993c2bcfb93 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:53 -0700 Subject: W1: w1_int.c use first available master number Follow the example of other devices (like the joystick device). Pick the first available id for each detected device. Currently for USB devices, suspending and resuming would cause the number to increment. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1_int.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c index 3fd6e6651fbe..a3a54567bfba 100644 --- a/drivers/w1/w1_int.c +++ b/drivers/w1/w1_int.c @@ -29,7 +29,6 @@ #include "w1_netlink.h" #include "w1_int.h" -static u32 w1_ids = 1; static int w1_search_count = -1; /* Default is continual scan */ module_param_named(search_count, w1_search_count, int, 0); @@ -102,9 +101,10 @@ static void w1_free_dev(struct w1_master *dev) int w1_add_master_device(struct w1_bus_master *master) { - struct w1_master *dev; + struct w1_master *dev, *entry; int retval = 0; struct w1_netlink_msg msg; + int id, found; /* validate minimum functionality */ if (!(master->touch_bit && master->reset_bus) && @@ -126,13 +126,33 @@ int w1_add_master_device(struct w1_bus_master *master) master->set_pullup = NULL; } - dev = w1_alloc_dev(w1_ids++, w1_max_slave_count, w1_max_slave_ttl, &w1_master_driver, &w1_master_device); - if (!dev) + /* Lock until the device is added (or not) to w1_masters. */ + mutex_lock(&w1_mlock); + /* Search for the first available id (starting at 1). */ + id = 0; + do { + ++id; + found = 0; + list_for_each_entry(entry, &w1_masters, w1_master_entry) { + if (entry->id == id) { + found = 1; + break; + } + } + } while (found); + + dev = w1_alloc_dev(id, w1_max_slave_count, w1_max_slave_ttl, + &w1_master_driver, &w1_master_device); + if (!dev) { + mutex_unlock(&w1_mlock); return -ENOMEM; + } retval = w1_create_master_attributes(dev); - if (retval) + if (retval) { + mutex_unlock(&w1_mlock); goto err_out_free_dev; + } memcpy(dev->bus_master, master, sizeof(struct w1_bus_master)); @@ -144,10 +164,10 @@ int w1_add_master_device(struct w1_bus_master *master) dev_err(&dev->dev, "Failed to create new kernel thread. err=%d\n", retval); + mutex_unlock(&w1_mlock); goto err_out_rm_attr; } - mutex_lock(&w1_mlock); list_add(&dev->w1_master_entry, &w1_masters); mutex_unlock(&w1_mlock); -- cgit v1.2.3 From 7dc8f527ef20bf95143dfbe2ecc01dc70b1e6ab7 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:04:58 -0700 Subject: W1: w1.c s/printk/dev_dbg/ s/printk/dev_dbg/ Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 04e7de4b266a..3b615d4022ee 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -81,10 +81,10 @@ static void w1_slave_release(struct device *dev) { struct w1_slave *sl = dev_to_w1_slave(dev); - printk("%s: Releasing %s.\n", __func__, sl->name); + dev_dbg(dev, "%s: Releasing %s.\n", __func__, sl->name); while (atomic_read(&sl->refcnt)) { - printk("Waiting for %s to become free: refcnt=%d.\n", + dev_dbg(dev, "Waiting for %s to become free: refcnt=%d.\n", sl->name, atomic_read(&sl->refcnt)); if (msleep_interruptible(1000)) flush_signals(current); @@ -920,7 +920,7 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb rn |= (tmp64 << i); if (kthread_should_stop()) { - printk(KERN_INFO "Abort w1_search (exiting)\n"); + dev_dbg(&dev->dev, "Abort w1_search\n"); return; } } -- cgit v1.2.3 From 8e3dae2b4727dc216e2dc16d2f0271b5f31b680c Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:01 -0700 Subject: W1: w1_io.c reset comments and msleep w1_reset_bus, added some comments about the timing and switched to msleep for the later delay. I don't have the hardware to test the sleep after reset change. The one wire doesn't have a timing requirement between commands so it is fine. I do have the USB hardware and it would be in big trouble with 10ms interrupt transfers to find that the reset completed. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1_io.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c index 97b338a16abc..f4f82f1f486e 100644 --- a/drivers/w1/w1_io.c +++ b/drivers/w1/w1_io.c @@ -293,12 +293,24 @@ int w1_reset_bus(struct w1_master *dev) result = dev->bus_master->reset_bus(dev->bus_master->data) & 0x1; else { dev->bus_master->write_bit(dev->bus_master->data, 0); + /* minimum 480, max ? us + * be nice and sleep, except 18b20 spec lists 960us maximum, + * so until we can sleep with microsecond accuracy, spin. + * Feel free to come up with some other way to give up the + * cpu for such a short amount of time AND get it back in + * the maximum amount of time. + */ w1_delay(480); dev->bus_master->write_bit(dev->bus_master->data, 1); w1_delay(70); result = dev->bus_master->read_bit(dev->bus_master->data) & 0x1; - w1_delay(410); + /* minmum 70 (above) + 410 = 480 us + * There aren't any timing requirements between a reset and + * the following transactions. Sleeping is safe here. + */ + /* w1_delay(410); min required time */ + msleep(1); } return result; -- cgit v1.2.3 From cadd486cfc838ead0ad899db129cff9f61ef4267 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:01 -0700 Subject: W1: ds1wm.c msleep for reset Like the previous w1_io.c reset coments and msleep patch, I don't have the hardware to verify the change, but I think it is safe. It also helps to see a comment like this in the code. "We'll wait a bit longer just to be sure." If they are going to calculate delaying 324.9us, but actually delay 500us, why not just give up the CPU and sleep? This is designed for a battery powered ARM system, avoiding busywaiting has to be good for battery life. I sent a request for testers March 7, 2008 to the Linux kernel mailing list and two developers who have patches for ds1wm.c, but I didn't get any respons. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds1wm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c index ea894bf18113..29e144f81cbe 100644 --- a/drivers/w1/masters/ds1wm.c +++ b/drivers/w1/masters/ds1wm.c @@ -160,8 +160,10 @@ static int ds1wm_reset(struct ds1wm_data *ds1wm_data) * 625 us - 60 us - 240 us - 100 ns = 324.9 us * * We'll wait a bit longer just to be sure. + * Was udelay(500), but if it is going to busywait the cpu that long, + * might as well come back later. */ - udelay(500); + msleep(1); ds1wm_write_register(ds1wm_data, DS1WM_INT_EN, DS1WM_INTEN_ERBF | DS1WM_INTEN_ETMT | DS1WM_INTEN_EPD | -- cgit v1.2.3 From 95cfaebf61dff28612bd280d89efc6e2c6716dfa Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:02 -0700 Subject: W1: ds2490.c correct print message Corrected print message, it was writing not reading, this also prints the endpoint used for the write instead of hardcoding it. Failed to write 1-wire data to ep0x%x: err=%d. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index b63b5e044a4c..c8365fb20b4f 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -341,7 +341,8 @@ static int ds_send_data(struct ds_device *dev, unsigned char *buf, int len) count = 0; err = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, dev->ep[EP_DATA_OUT]), buf, len, &count, 1000); if (err < 0) { - printk(KERN_ERR "Failed to read 1-wire data from 0x02: err=%d.\n", err); + printk(KERN_ERR "Failed to write 1-wire data to ep0x%x: " + "err=%d.\n", dev->ep[EP_DATA_OUT], err); return err; } -- cgit v1.2.3 From 1f4ec2d7f6c4560a9d0c1abab2e8effe9ba93921 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:03 -0700 Subject: W1: ds2490.c add support for strong pullup Add strong pullup support for ds2490 driver, also drop mdelay(750), which busy waits, usage in favour of msleep for long delays. Now with msleep only being called when the strong pullup is active, one wire bus operations are only taking minimal system overhead. The new set_pullup will only enable the strong pullup when requested, which is expected to be the only write operation that will benefit from a strong pullup. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 76 ++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 38 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index c8365fb20b4f..6b188e8008e4 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -98,11 +98,6 @@ #define BRANCH_MAIN 0xCC #define BRANCH_AUX 0x33 -/* - * Duration of the strong pull-up pulse in milliseconds. - */ -#define PULLUP_PULSE_DURATION 750 - /* Status flags */ #define ST_SPUA 0x01 /* Strong Pull-up is active */ #define ST_PRGA 0x02 /* 12V programming pulse is being generated */ @@ -131,6 +126,11 @@ struct ds_device int ep[NUM_EP]; + /* Strong PullUp + * 0: pullup not active, else duration in milliseconds + */ + int spu_sleep; + struct w1_bus_master master; }; @@ -192,7 +192,7 @@ static int ds_send_control_cmd(struct ds_device *dev, u16 value, u16 index) return err; } -#if 0 + static int ds_send_control_mode(struct ds_device *dev, u16 value, u16 index) { int err; @@ -207,7 +207,7 @@ static int ds_send_control_mode(struct ds_device *dev, u16 value, u16 index) return err; } -#endif + static int ds_send_control(struct ds_device *dev, u16 value, u16 index) { int err; @@ -294,14 +294,6 @@ static int ds_recv_status(struct ds_device *dev, struct ds_status *st) if (count < 0) return err; } -#if 0 - if (st->status & ST_IDLE) { - printk(KERN_INFO "Resetting pulse after ST_IDLE.\n"); - err = ds_start_pulse(dev, PULLUP_PULSE_DURATION); - if (err) - return err; - } -#endif return err; } @@ -472,32 +464,26 @@ static int ds_set_speed(struct ds_device *dev, int speed) } #endif /* 0 */ -static int ds_start_pulse(struct ds_device *dev, int delay) +static int ds_set_pullup(struct ds_device *dev, int delay) { int err; u8 del = 1 + (u8)(delay >> 4); - struct ds_status st; - -#if 0 - err = ds_stop_pulse(dev, 10); - if (err) - return err; - - err = ds_send_control_mode(dev, MOD_PULSE_EN, PULSE_SPUE); - if (err) - return err; -#endif - err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del); - if (err) - return err; - err = ds_send_control(dev, COMM_PULSE | COMM_IM | COMM_F, 0); + dev->spu_sleep = 0; + err = ds_send_control_mode(dev, MOD_PULSE_EN, delay ? PULSE_SPUE : 0); if (err) return err; - mdelay(delay); + if (delay) { + err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del); + if (err) + return err; - ds_wait_status(dev, &st); + /* Just storing delay would not get the trunication and + * roundup. + */ + dev->spu_sleep = del<<4; + } return err; } @@ -558,6 +544,9 @@ static int ds_write_byte(struct ds_device *dev, u8 byte) if (err) return err; + if (dev->spu_sleep) + msleep(dev->spu_sleep); + err = ds_wait_status(dev, &st); if (err) return err; @@ -566,8 +555,6 @@ static int ds_write_byte(struct ds_device *dev, u8 byte) if (err < 0) return err; - ds_start_pulse(dev, PULLUP_PULSE_DURATION); - return !(byte == rbyte); } @@ -603,7 +590,7 @@ static int ds_read_block(struct ds_device *dev, u8 *buf, int len) if (err < 0) return err; - err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | COMM_SPU, len); + err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM, len); if (err) return err; @@ -630,14 +617,15 @@ static int ds_write_block(struct ds_device *dev, u8 *buf, int len) if (err) return err; + if (dev->spu_sleep) + msleep(dev->spu_sleep); + ds_wait_status(dev, &st); err = ds_recv_data(dev, buf, len); if (err < 0) return err; - ds_start_pulse(dev, PULLUP_PULSE_DURATION); - return !(err == len); } @@ -803,6 +791,16 @@ static u8 ds9490r_reset(void *data) return 0; } +static u8 ds9490r_set_pullup(void *data, int delay) +{ + struct ds_device *dev = data; + + if (ds_set_pullup(dev, delay)) + return 1; + + return 0; +} + static int ds_w1_init(struct ds_device *dev) { memset(&dev->master, 0, sizeof(struct w1_bus_master)); @@ -816,6 +814,7 @@ static int ds_w1_init(struct ds_device *dev) dev->master.read_block = &ds9490r_read_block; dev->master.write_block = &ds9490r_write_block; dev->master.reset_bus = &ds9490r_reset; + dev->master.set_pullup = &ds9490r_set_pullup; return w1_add_master_device(&dev->master); } @@ -839,6 +838,7 @@ static int ds_probe(struct usb_interface *intf, printk(KERN_INFO "Failed to allocate new DS9490R structure.\n"); return -ENOMEM; } + dev->spu_sleep = 0; dev->udev = usb_get_dev(udev); if (!dev->udev) { err = -ENOMEM; -- cgit v1.2.3 From e1c86d226daf95407d66246ced8fe087055acc6b Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:04 -0700 Subject: W1: ds2490.c ds_write_bit, grouping error, disable readback ds_write_bit doesn't read the input buffer, so add COMM_ICP and a comment that it will no longer generate a read back data byte. If there is an extra data byte later on then it will cause an error and discard what data was there. Corrected operator ordering for ds_send_control. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 6b188e8008e4..6fabf584395f 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -525,7 +525,12 @@ static int ds_write_bit(struct ds_device *dev, u8 bit) int err; struct ds_status st; - err = ds_send_control(dev, COMM_BIT_IO | COMM_IM | (bit) ? COMM_D : 0, 0); + /* Set COMM_ICP to write without a readback. Note, this will + * produce one time slot, a down followed by an up with COMM_D + * only determing the timing. + */ + err = ds_send_control(dev, COMM_BIT_IO | COMM_IM | COMM_ICP | + (bit ? COMM_D : 0), 0); if (err) return err; -- cgit v1.2.3 From a08e2d338bab17ac5c51a8f2f25185da18f6710c Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:04 -0700 Subject: W1: ds2490.c disable bit read and write Don't export read and write bit operations, they didn't work, they weren't used, and they can't be made to work. The one wire low level bit operations expect to set high or low levels, the ds2490 hardware only supports complete read or write time slots, better to just comment them out. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 6fabf584395f..1b632d549e79 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -520,6 +520,7 @@ static int ds_touch_bit(struct ds_device *dev, u8 bit, u8 *tbit) return 0; } +#if 0 static int ds_write_bit(struct ds_device *dev, u8 bit) { int err; @@ -538,6 +539,7 @@ static int ds_write_bit(struct ds_device *dev, u8 bit) return 0; } +#endif static int ds_write_byte(struct ds_device *dev, u8 byte) { @@ -722,6 +724,7 @@ static u8 ds9490r_touch_bit(void *data, u8 bit) return ret; } +#if 0 static void ds9490r_write_bit(void *data, u8 bit) { struct ds_device *dev = data; @@ -729,13 +732,6 @@ static void ds9490r_write_bit(void *data, u8 bit) ds_write_bit(dev, bit); } -static void ds9490r_write_byte(void *data, u8 byte) -{ - struct ds_device *dev = data; - - ds_write_byte(dev, byte); -} - static u8 ds9490r_read_bit(void *data) { struct ds_device *dev = data; @@ -748,6 +744,14 @@ static u8 ds9490r_read_bit(void *data) return bit & 1; } +#endif + +static void ds9490r_write_byte(void *data, u8 byte) +{ + struct ds_device *dev = data; + + ds_write_byte(dev, byte); +} static u8 ds9490r_read_byte(void *data) { @@ -812,8 +816,15 @@ static int ds_w1_init(struct ds_device *dev) dev->master.data = dev; dev->master.touch_bit = &ds9490r_touch_bit; + /* read_bit and write_bit in w1_bus_master are expected to set and + * sample the line level. For write_bit that means it is expected to + * set it to that value and leave it there. ds2490 only supports an + * individual time slot at the lowest level. The requirement from + * pulling the bus state down to reading the state is 15us, something + * that isn't realistic on the USB bus anyway. dev->master.read_bit = &ds9490r_read_bit; dev->master.write_bit = &ds9490r_write_bit; + */ dev->master.read_byte = &ds9490r_read_byte; dev->master.write_byte = &ds9490r_write_byte; dev->master.read_block = &ds9490r_read_block; -- cgit v1.2.3 From 6e10f65427ed800ad1026dbf8064ca536ea98afc Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:05 -0700 Subject: W1: ds2490.c simplify and fix ds_touch_bit Simplify and fix ds_touch_bit. If a device is attached in the middle of a bus search the status register will return more than the default 16 bytes. The additional bytes indicate that it has detected a new device. The way ds_wait_status is coded, if it doesn't read 16 status bytes it returns an error value. ds_touch_bit then will detect that error and return an error. In that case it doesn't read the input buffer and returns uninitialized data. It doesn't stop there. The next transaction will not expect the extra byte in the input buffer and the short read will cause an error and clear out both the old byte and new data in the input buffer. Just ignore the value of ds_wait_status. It is still required to wait until ds2490 is again idle and there is data to read when ds_recv_data is called. This also removes the while loop. None of the other commands wait and verify that the issued command is in the status register. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 1b632d549e79..c4ff70b2c7c4 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -490,28 +490,15 @@ static int ds_set_pullup(struct ds_device *dev, int delay) static int ds_touch_bit(struct ds_device *dev, u8 bit, u8 *tbit) { - int err, count; + int err; struct ds_status st; - u16 value = (COMM_BIT_IO | COMM_IM) | ((bit) ? COMM_D : 0); - u16 cmd; - err = ds_send_control(dev, value, 0); + err = ds_send_control(dev, COMM_BIT_IO | COMM_IM | (bit ? COMM_D : 0), + 0); if (err) return err; - count = 0; - do { - err = ds_wait_status(dev, &st); - if (err) - return err; - - cmd = st.command0 | (st.command1 << 8); - } while (cmd != value && ++count < 10); - - if (err < 0 || count >= 10) { - printk(KERN_ERR "Failed to obtain status.\n"); - return -EINVAL; - } + ds_wait_status(dev, &st); err = ds_recv_data(dev, tbit, sizeof(*tbit)); if (err < 0) -- cgit v1.2.3 From 4b9cf1bc329e626f3fa655370ee8cc156ab29a55 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:06 -0700 Subject: W1: ds2490.c ds_dump_status rework - add result register #defines - rename ds_dump_status to ds_print_msg - rename ds_recv_status to ds_dump_status - ds_dump_status prints the requested status and no longer reads the status, this is because the second status read can return different data for example the result register - the result register will be printed, though limited to detecting a new device, detecting other values such as a short would require additional reporting methods - ST_EPOF was moved to ds_wait_status to clear the error condition sooner Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 134 +++++++++++++++++++++++++++----------------- 1 file changed, 84 insertions(+), 50 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index c4ff70b2c7c4..065042db69f7 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -107,6 +107,17 @@ #define ST_IDLE 0x20 /* DS2490 is currently idle */ #define ST_EPOF 0x80 +/* Result Register flags */ +#define RR_DETECT 0xA5 /* New device detected */ +#define RR_NRS 0x01 /* Reset no presence or ... */ +#define RR_SH 0x02 /* short on reset or set path */ +#define RR_APP 0x04 /* alarming presence on reset */ +#define RR_VPP 0x08 /* 12V expected not seen */ +#define RR_CMP 0x10 /* compare error */ +#define RR_CRC 0x20 /* CRC error detected */ +#define RR_RDP 0x40 /* redirected page */ +#define RR_EOS 0x80 /* end of search error */ + #define SPEED_NORMAL 0x00 #define SPEED_FLEXIBLE 0x01 #define SPEED_OVERDRIVE 0x02 @@ -164,7 +175,6 @@ MODULE_DEVICE_TABLE(usb, ds_id_table); static int ds_probe(struct usb_interface *, const struct usb_device_id *); static void ds_disconnect(struct usb_interface *); -static inline void ds_dump_status(unsigned char *, unsigned char *, int); static int ds_send_control(struct ds_device *, u16, u16); static int ds_send_control_cmd(struct ds_device *, u16, u16); @@ -223,11 +233,6 @@ static int ds_send_control(struct ds_device *dev, u16 value, u16 index) return err; } -static inline void ds_dump_status(unsigned char *buf, unsigned char *str, int off) -{ - printk("%45s: %8x\n", str, buf[off]); -} - static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st, unsigned char *buf, int size) { @@ -248,54 +253,62 @@ static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st, return count; } -static int ds_recv_status(struct ds_device *dev, struct ds_status *st) +static inline void ds_print_msg(unsigned char *buf, unsigned char *str, int off) { - unsigned char buf[64]; - int count, err = 0, i; - - memcpy(st, buf, sizeof(*st)); + printk(KERN_INFO "%45s: %8x\n", str, buf[off]); +} - count = ds_recv_status_nodump(dev, st, buf, sizeof(buf)); - if (count < 0) - return err; +static void ds_dump_status(struct ds_device *dev, unsigned char *buf, int count) +{ + int i; - printk("0x%x: count=%d, status: ", dev->ep[EP_STATUS], count); + printk(KERN_INFO "0x%x: count=%d, status: ", dev->ep[EP_STATUS], count); for (i=0; i= 16) { - ds_dump_status(buf, "enable flag", 0); - ds_dump_status(buf, "1-wire speed", 1); - ds_dump_status(buf, "strong pullup duration", 2); - ds_dump_status(buf, "programming pulse duration", 3); - ds_dump_status(buf, "pulldown slew rate control", 4); - ds_dump_status(buf, "write-1 low time", 5); - ds_dump_status(buf, "data sample offset/write-0 recovery time", 6); - ds_dump_status(buf, "reserved (test register)", 7); - ds_dump_status(buf, "device status flags", 8); - ds_dump_status(buf, "communication command byte 1", 9); - ds_dump_status(buf, "communication command byte 2", 10); - ds_dump_status(buf, "communication command buffer status", 11); - ds_dump_status(buf, "1-wire data output buffer status", 12); - ds_dump_status(buf, "1-wire data input buffer status", 13); - ds_dump_status(buf, "reserved", 14); - ds_dump_status(buf, "reserved", 15); + ds_print_msg(buf, "enable flag", 0); + ds_print_msg(buf, "1-wire speed", 1); + ds_print_msg(buf, "strong pullup duration", 2); + ds_print_msg(buf, "programming pulse duration", 3); + ds_print_msg(buf, "pulldown slew rate control", 4); + ds_print_msg(buf, "write-1 low time", 5); + ds_print_msg(buf, "data sample offset/write-0 recovery time", + 6); + ds_print_msg(buf, "reserved (test register)", 7); + ds_print_msg(buf, "device status flags", 8); + ds_print_msg(buf, "communication command byte 1", 9); + ds_print_msg(buf, "communication command byte 2", 10); + ds_print_msg(buf, "communication command buffer status", 11); + ds_print_msg(buf, "1-wire data output buffer status", 12); + ds_print_msg(buf, "1-wire data input buffer status", 13); + ds_print_msg(buf, "reserved", 14); + ds_print_msg(buf, "reserved", 15); } - - memcpy(st, buf, sizeof(*st)); - - if (st->status & ST_EPOF) { - printk(KERN_INFO "Resetting device after ST_EPOF.\n"); - err = ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); - if (err) - return err; - count = ds_recv_status_nodump(dev, st, buf, sizeof(buf)); - if (count < 0) - return err; + for (i = 16; i < count; ++i) { + if (buf[i] == RR_DETECT) { + ds_print_msg(buf, "new device detect", i); + continue; + } + ds_print_msg(buf, "Result Register Value: ", i); + if (buf[i] & RR_NRS) + printk(KERN_INFO "NRS: Reset no presence or ...\n"); + if (buf[i] & RR_SH) + printk(KERN_INFO "SH: short on reset or set path\n"); + if (buf[i] & RR_APP) + printk(KERN_INFO "APP: alarming presence on reset\n"); + if (buf[i] & RR_VPP) + printk(KERN_INFO "VPP: 12V expected not seen\n"); + if (buf[i] & RR_CMP) + printk(KERN_INFO "CMP: compare error\n"); + if (buf[i] & RR_CRC) + printk(KERN_INFO "CRC: CRC error detected\n"); + if (buf[i] & RR_RDP) + printk(KERN_INFO "RDP: redirected page\n"); + if (buf[i] & RR_EOS) + printk(KERN_INFO "EOS: end of search error\n"); } - - return err; } static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size) @@ -307,9 +320,14 @@ static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size) err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]), buf, size, &count, 1000); if (err < 0) { + u8 buf[0x20]; + int count; + printk(KERN_INFO "Clearing ep0x%x.\n", dev->ep[EP_DATA_IN]); usb_clear_halt(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN])); - ds_recv_status(dev, &st); + + count = ds_recv_status_nodump(dev, &st, buf, sizeof(buf)); + ds_dump_status(dev, buf, count); return err; } @@ -390,7 +408,7 @@ int ds_detect(struct ds_device *dev, struct ds_status *st) if (err) return err; - err = ds_recv_status(dev, st); + err = ds_dump_status(dev, st); return err; } @@ -415,11 +433,27 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st) #endif } while(!(buf[0x08] & 0x20) && !(err < 0) && ++count < 100); + if (err >= 16 && st->status & ST_EPOF) { + printk(KERN_INFO "Resetting device after ST_EPOF.\n"); + ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); + /* Always dump the device status. */ + count = 101; + } + + /* Dump the status for errors or if there is extended return data. + * The extended status includes new device detection (maybe someone + * can do something with it). + */ + if (err > 16 || count >= 100 || err < 0) + ds_dump_status(dev, buf, err); - if (((err > 16) && (buf[0x10] & 0x01)) || count >= 100 || err < 0) { - ds_recv_status(dev, st); + /* Extended data isn't an error. Well, a short is, but the dump + * would have already told the user that and we can't do anything + * about it in software anyway. + */ + if (count >= 100 || err < 0) return -1; - } else + else return 0; } -- cgit v1.2.3 From 7a4b9706ed762373f74311f96f5122fb74212192 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:07 -0700 Subject: W1: ds2490.c ds_reset remove ds_wait_status ds_reset no longer calls ds_wait_status, the result wasn't used and it would only delay the following data operations. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 065042db69f7..9a7fd71e1461 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -457,7 +457,7 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st) return 0; } -static int ds_reset(struct ds_device *dev, struct ds_status *st) +static int ds_reset(struct ds_device *dev) { int err; @@ -466,14 +466,6 @@ static int ds_reset(struct ds_device *dev, struct ds_status *st) if (err) return err; - ds_wait_status(dev, st); -#if 0 - if (st->command_buffer_status) { - printk(KERN_INFO "Short circuit.\n"); - return -EIO; - } -#endif - return 0; } @@ -809,12 +801,9 @@ static u8 ds9490r_read_block(void *data, u8 *buf, int len) static u8 ds9490r_reset(void *data) { struct ds_device *dev = data; - struct ds_status st; int err; - memset(&st, 0, sizeof(st)); - - err = ds_reset(dev, &st); + err = ds_reset(dev); if (err) return 1; -- cgit v1.2.3 From e464af24734c40853dd68ec694d83a82e3930d66 Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:08 -0700 Subject: W1: ds2490.c reset ds2490 in init Reset the device in init as it can be in a bad state. This is necessary because a block write will wait for data to be placed in the output buffer and block any later commands which will keep accumulating and the device will not be idle. Another case is removing the ds2490 module while a bus search is in progress, somehow a few commands get through, but the input transfers fail leaving data in the input buffer. This will cause the next read to fail see the note in ds_recv_data. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 9a7fd71e1461..0f356939b349 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -316,6 +316,15 @@ static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size) int count, err; struct ds_status st; + /* Careful on size. If size is less than what is available in + * the input buffer, the device fails the bulk transfer and + * clears the input buffer. It could read the maximum size of + * the data buffer, but then do you return the first, last, or + * some set of the middle size bytes? As long as the rest of + * the code is correct there will be size bytes waiting. A + * call to ds_wait_status will wait until the device is idle + * and any data to be received would have been available. + */ count = 0; err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]), buf, size, &count, 1000); @@ -824,6 +833,18 @@ static int ds_w1_init(struct ds_device *dev) { memset(&dev->master, 0, sizeof(struct w1_bus_master)); + /* Reset the device as it can be in a bad state. + * This is necessary because a block write will wait for data + * to be placed in the output buffer and block any later + * commands which will keep accumulating and the device will + * not be idle. Another case is removing the ds2490 module + * while a bus search is in progress, somehow a few commands + * get through, but the input transfers fail leaving data in + * the input buffer. This will cause the next read to fail + * see the note in ds_recv_data. + */ + ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); + dev->master.data = dev; dev->master.touch_bit = &ds9490r_touch_bit; /* read_bit and write_bit in w1_bus_master are expected to set and -- cgit v1.2.3 From 19e7184f75354c50bc196d856ff903b2add5ff5a Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:08 -0700 Subject: W1: ds2490.c magic number work This replaces some magic numbers with marcos and corrects one marco. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 0f356939b349..29df1d2998b2 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -88,7 +88,7 @@ #define COMM_DT 0x2000 #define COMM_SPU 0x1000 #define COMM_F 0x0800 -#define COMM_NTP 0x0400 +#define COMM_NTF 0x0400 #define COMM_ICP 0x0200 #define COMM_RST 0x0100 @@ -440,7 +440,7 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st) printk("\n"); } #endif - } while(!(buf[0x08] & 0x20) && !(err < 0) && ++count < 100); + } while (!(buf[0x08] & ST_IDLE) && !(err < 0) && ++count < 100); if (err >= 16 && st->status & ST_EPOF) { printk(KERN_INFO "Resetting device after ST_EPOF.\n"); @@ -470,8 +470,16 @@ static int ds_reset(struct ds_device *dev) { int err; - //err = ds_send_control(dev, COMM_1_WIRE_RESET | COMM_F | COMM_IM | COMM_SE, SPEED_FLEXIBLE); - err = ds_send_control(dev, 0x43, SPEED_NORMAL); + /* Other potentionally interesting flags for reset. + * + * COMM_NTF: Return result register feedback. This could be used to + * detect some conditions such as short, alarming presence, or + * detect if a new device was detected. + * + * COMM_SE which allows SPEED_NORMAL, SPEED_FLEXIBLE, SPEED_OVERDRIVE: + * Select the data transfer rate. + */ + err = ds_send_control(dev, COMM_1_WIRE_RESET | COMM_IM, SPEED_NORMAL); if (err) return err; -- cgit v1.2.3 From cbf4a49afa9f9a527ed4f3dab4ec355586ba890e Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:09 -0700 Subject: W1: ds2490.c ds_write_block remove extra ds_wait_status Drop the extra ds_wait_status() in ds_write_block(). Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 29df1d2998b2..4faf4f9ec068 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -648,8 +648,6 @@ static int ds_write_block(struct ds_device *dev, u8 *buf, int len) if (err < 0) return err; - ds_wait_status(dev, &st); - err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | COMM_SPU, len); if (err) return err; -- cgit v1.2.3 From ade6d810b585d749db24d734947a30a29470cccd Mon Sep 17 00:00:00 2001 From: David Fries Date: Wed, 15 Oct 2008 22:05:10 -0700 Subject: W1: ds2490.c optimize ds_set_pullup Optimize the ds_set_pullup function. For a strong pullup to be sent the ds2490 has to have both the strong pullup mode enabled, and the specific write operation has to have the SPU bit enabled. Previously the write always had the SPU bit enabled and both the duration and model was set when a strong pullup was requested. Now the strong pullup mode is enabled at initialization time, the delay is updated only when the value changes, and the write SPU bit is set only when a strong pullup is required. This removes two or three bus transactions per strong pullup request. Signed-off-by: David Fries Signed-off-by: Evgeniy Polyakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/masters/ds2490.c | 64 +++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) (limited to 'drivers/w1') diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c index 4faf4f9ec068..59ad6e95af8f 100644 --- a/drivers/w1/masters/ds2490.c +++ b/drivers/w1/masters/ds2490.c @@ -141,6 +141,10 @@ struct ds_device * 0: pullup not active, else duration in milliseconds */ int spu_sleep; + /* spu_bit contains COMM_SPU or 0 depending on if the strong pullup + * should be active or not for writes. + */ + u16 spu_bit; struct w1_bus_master master; }; @@ -311,6 +315,25 @@ static void ds_dump_status(struct ds_device *dev, unsigned char *buf, int count) } } +static void ds_reset_device(struct ds_device *dev) +{ + ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); + /* Always allow strong pullup which allow individual writes to use + * the strong pullup. + */ + if (ds_send_control_mode(dev, MOD_PULSE_EN, PULSE_SPUE)) + printk(KERN_ERR "ds_reset_device: " + "Error allowing strong pullup\n"); + /* Chip strong pullup time was cleared. */ + if (dev->spu_sleep) { + /* lower 4 bits are 0, see ds_set_pullup */ + u8 del = dev->spu_sleep>>4; + if (ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del)) + printk(KERN_ERR "ds_reset_device: " + "Error setting duration\n"); + } +} + static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size) { int count, err; @@ -444,7 +467,7 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st) if (err >= 16 && st->status & ST_EPOF) { printk(KERN_INFO "Resetting device after ST_EPOF.\n"); - ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); + ds_reset_device(dev); /* Always dump the device status. */ count = 101; } @@ -509,24 +532,26 @@ static int ds_set_speed(struct ds_device *dev, int speed) static int ds_set_pullup(struct ds_device *dev, int delay) { - int err; + int err = 0; u8 del = 1 + (u8)(delay >> 4); + /* Just storing delay would not get the trunication and roundup. */ + int ms = del<<4; + + /* Enable spu_bit if a delay is set. */ + dev->spu_bit = delay ? COMM_SPU : 0; + /* If delay is zero, it has already been disabled, if the time is + * the same as the hardware was last programmed to, there is also + * nothing more to do. Compare with the recalculated value ms + * rather than del or delay which can have a different value. + */ + if (delay == 0 || ms == dev->spu_sleep) + return err; - dev->spu_sleep = 0; - err = ds_send_control_mode(dev, MOD_PULSE_EN, delay ? PULSE_SPUE : 0); + err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del); if (err) return err; - if (delay) { - err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del); - if (err) - return err; - - /* Just storing delay would not get the trunication and - * roundup. - */ - dev->spu_sleep = del<<4; - } + dev->spu_sleep = ms; return err; } @@ -577,11 +602,11 @@ static int ds_write_byte(struct ds_device *dev, u8 byte) struct ds_status st; u8 rbyte; - err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | COMM_SPU, byte); + err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | dev->spu_bit, byte); if (err) return err; - if (dev->spu_sleep) + if (dev->spu_bit) msleep(dev->spu_sleep); err = ds_wait_status(dev, &st); @@ -648,11 +673,11 @@ static int ds_write_block(struct ds_device *dev, u8 *buf, int len) if (err < 0) return err; - err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | COMM_SPU, len); + err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | dev->spu_bit, len); if (err) return err; - if (dev->spu_sleep) + if (dev->spu_bit) msleep(dev->spu_sleep); ds_wait_status(dev, &st); @@ -849,7 +874,7 @@ static int ds_w1_init(struct ds_device *dev) * the input buffer. This will cause the next read to fail * see the note in ds_recv_data. */ - ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0); + ds_reset_device(dev); dev->master.data = dev; dev->master.touch_bit = &ds9490r_touch_bit; @@ -892,6 +917,7 @@ static int ds_probe(struct usb_interface *intf, return -ENOMEM; } dev->spu_sleep = 0; + dev->spu_bit = 0; dev->udev = usb_get_dev(udev); if (!dev->udev) { err = -ENOMEM; -- cgit v1.2.3 From d8273674721faaf84bec2190c0c7a82972b37f73 Mon Sep 17 00:00:00 2001 From: Bernhard Weirich Date: Wed, 15 Oct 2008 22:05:11 -0700 Subject: w1: new driver. DS2431 chip [akpm@linux-foundation.org: minor fixlets and cleanups] Signed-off-by: Bernhard Weirich Signed-off-by: Evgeniy Polyakov Cc: Ben Gardner Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/slaves/w1_ds2431.c | 312 ++++++++++++++++++++++++++++++++++++++++++ drivers/w1/w1_family.h | 1 + 2 files changed, 313 insertions(+) create mode 100644 drivers/w1/slaves/w1_ds2431.c (limited to 'drivers/w1') diff --git a/drivers/w1/slaves/w1_ds2431.c b/drivers/w1/slaves/w1_ds2431.c new file mode 100644 index 000000000000..2c6c0cf6a20f --- /dev/null +++ b/drivers/w1/slaves/w1_ds2431.c @@ -0,0 +1,312 @@ +/* + * w1_ds2431.c - w1 family 2d (DS2431) driver + * + * Copyright (c) 2008 Bernhard Weirich + * + * Heavily inspired by w1_DS2433 driver from Ben Gardner + * + * This source code is licensed under the GNU General Public License, + * Version 2. See the file COPYING for more details. + */ + +#include +#include +#include +#include +#include +#include + +#include "../w1.h" +#include "../w1_int.h" +#include "../w1_family.h" + +#define W1_F2D_EEPROM_SIZE 128 +#define W1_F2D_PAGE_COUNT 4 +#define W1_F2D_PAGE_BITS 5 +#define W1_F2D_PAGE_SIZE (1< size) + return 0; + + if ((off + count) > size) + return size - off; + + return count; +} + +/* + * Read a block from W1 ROM two times and compares the results. + * If they are equal they are returned, otherwise the read + * is repeated W1_F2D_READ_RETRIES times. + * + * count must not exceed W1_F2D_READ_MAXLEN. + */ +static int w1_f2d_readblock(struct w1_slave *sl, int off, int count, char *buf) +{ + u8 wrbuf[3]; + u8 cmp[W1_F2D_READ_MAXLEN]; + int tries = W1_F2D_READ_RETRIES; + + do { + wrbuf[0] = W1_F2D_READ_EEPROM; + wrbuf[1] = off & 0xff; + wrbuf[2] = off >> 8; + + if (w1_reset_select_slave(sl)) + return -1; + + w1_write_block(sl->master, wrbuf, 3); + w1_read_block(sl->master, buf, count); + + if (w1_reset_select_slave(sl)) + return -1; + + w1_write_block(sl->master, wrbuf, 3); + w1_read_block(sl->master, cmp, count); + + if (!memcmp(cmp, buf, count)) + return 0; + } while (--tries); + + dev_err(&sl->dev, "proof reading failed %d times\n", + W1_F2D_READ_RETRIES); + + return -1; +} + +static ssize_t w1_f2d_read_bin(struct kobject *kobj, + struct bin_attribute *bin_attr, + char *buf, loff_t off, size_t count) +{ + struct w1_slave *sl = kobj_to_w1_slave(kobj); + int todo = count; + + count = w1_f2d_fix_count(off, count, W1_F2D_EEPROM_SIZE); + if (count == 0) + return 0; + + mutex_lock(&sl->master->mutex); + + /* read directly from the EEPROM in chunks of W1_F2D_READ_MAXLEN */ + while (todo > 0) { + int block_read; + + if (todo >= W1_F2D_READ_MAXLEN) + block_read = W1_F2D_READ_MAXLEN; + else + block_read = todo; + + if (w1_f2d_readblock(sl, off, block_read, buf) < 0) + count = -EIO; + + todo -= W1_F2D_READ_MAXLEN; + buf += W1_F2D_READ_MAXLEN; + off += W1_F2D_READ_MAXLEN; + } + + mutex_unlock(&sl->master->mutex); + + return count; +} + +/* + * Writes to the scratchpad and reads it back for verification. + * Then copies the scratchpad to EEPROM. + * The data must be aligned at W1_F2D_SCRATCH_SIZE bytes and + * must be W1_F2D_SCRATCH_SIZE bytes long. + * The master must be locked. + * + * @param sl The slave structure + * @param addr Address for the write + * @param len length must be <= (W1_F2D_PAGE_SIZE - (addr & W1_F2D_PAGE_MASK)) + * @param data The data to write + * @return 0=Success -1=failure + */ +static int w1_f2d_write(struct w1_slave *sl, int addr, int len, const u8 *data) +{ + int tries = W1_F2D_READ_RETRIES; + u8 wrbuf[4]; + u8 rdbuf[W1_F2D_SCRATCH_SIZE + 3]; + u8 es = (addr + len - 1) % W1_F2D_SCRATCH_SIZE; + +retry: + + /* Write the data to the scratchpad */ + if (w1_reset_select_slave(sl)) + return -1; + + wrbuf[0] = W1_F2D_WRITE_SCRATCH; + wrbuf[1] = addr & 0xff; + wrbuf[2] = addr >> 8; + + w1_write_block(sl->master, wrbuf, 3); + w1_write_block(sl->master, data, len); + + /* Read the scratchpad and verify */ + if (w1_reset_select_slave(sl)) + return -1; + + w1_write_8(sl->master, W1_F2D_READ_SCRATCH); + w1_read_block(sl->master, rdbuf, len + 3); + + /* Compare what was read against the data written */ + if ((rdbuf[0] != wrbuf[1]) || (rdbuf[1] != wrbuf[2]) || + (rdbuf[2] != es) || (memcmp(data, &rdbuf[3], len) != 0)) { + + if (--tries) + goto retry; + + dev_err(&sl->dev, + "could not write to eeprom, scratchpad compare failed %d times\n", + W1_F2D_READ_RETRIES); + + return -1; + } + + /* Copy the scratchpad to EEPROM */ + if (w1_reset_select_slave(sl)) + return -1; + + wrbuf[0] = W1_F2D_COPY_SCRATCH; + wrbuf[3] = es; + w1_write_block(sl->master, wrbuf, 4); + + /* Sleep for tprog ms to wait for the write to complete */ + msleep(W1_F2D_TPROG_MS); + + /* Reset the bus to wake up the EEPROM */ + w1_reset_bus(sl->master); + + return 0; +} + +static ssize_t w1_f2d_write_bin(struct kobject *kobj, + struct bin_attribute *bin_attr, + char *buf, loff_t off, size_t count) +{ + struct w1_slave *sl = kobj_to_w1_slave(kobj); + int addr, len; + int copy; + + count = w1_f2d_fix_count(off, count, W1_F2D_EEPROM_SIZE); + if (count == 0) + return 0; + + mutex_lock(&sl->master->mutex); + + /* Can only write data in blocks of the size of the scratchpad */ + addr = off; + len = count; + while (len > 0) { + + /* if len too short or addr not aligned */ + if (len < W1_F2D_SCRATCH_SIZE || addr & W1_F2D_SCRATCH_MASK) { + char tmp[W1_F2D_SCRATCH_SIZE]; + + /* read the block and update the parts to be written */ + if (w1_f2d_readblock(sl, addr & ~W1_F2D_SCRATCH_MASK, + W1_F2D_SCRATCH_SIZE, tmp)) { + count = -EIO; + goto out_up; + } + + /* copy at most to the boundary of the PAGE or len */ + copy = W1_F2D_SCRATCH_SIZE - + (addr & W1_F2D_SCRATCH_MASK); + + if (copy > len) + copy = len; + + memcpy(&tmp[addr & W1_F2D_SCRATCH_MASK], buf, copy); + if (w1_f2d_write(sl, addr & ~W1_F2D_SCRATCH_MASK, + W1_F2D_SCRATCH_SIZE, tmp) < 0) { + count = -EIO; + goto out_up; + } + } else { + + copy = W1_F2D_SCRATCH_SIZE; + if (w1_f2d_write(sl, addr, copy, buf) < 0) { + count = -EIO; + goto out_up; + } + } + buf += copy; + addr += copy; + len -= copy; + } + +out_up: + mutex_unlock(&sl->master->mutex); + + return count; +} + +static struct bin_attribute w1_f2d_bin_attr = { + .attr = { + .name = "eeprom", + .mode = S_IRUGO | S_IWUSR, + }, + .size = W1_F2D_EEPROM_SIZE, + .read = w1_f2d_read_bin, + .write = w1_f2d_write_bin, +}; + +static int w1_f2d_add_slave(struct w1_slave *sl) +{ + return sysfs_create_bin_file(&sl->dev.kobj, &w1_f2d_bin_attr); +} + +static void w1_f2d_remove_slave(struct w1_slave *sl) +{ + sysfs_remove_bin_file(&sl->dev.kobj, &w1_f2d_bin_attr); +} + +static struct w1_family_ops w1_f2d_fops = { + .add_slave = w1_f2d_add_slave, + .remove_slave = w1_f2d_remove_slave, +}; + +static struct w1_family w1_family_2d = { + .fid = W1_EEPROM_DS2431, + .fops = &w1_f2d_fops, +}; + +static int __init w1_f2d_init(void) +{ + return w1_register_family(&w1_family_2d); +} + +static void __exit w1_f2d_fini(void) +{ + w1_unregister_family(&w1_family_2d); +} + +module_init(w1_f2d_init); +module_exit(w1_f2d_fini); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Bernhard Weirich "); +MODULE_DESCRIPTION("w1 family 2d driver for DS2431, 1kb EEPROM"); diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h index 30531331a643..3ca1b9298f21 100644 --- a/drivers/w1/w1_family.h +++ b/drivers/w1/w1_family.h @@ -33,6 +33,7 @@ #define W1_THERM_DS1822 0x22 #define W1_EEPROM_DS2433 0x23 #define W1_THERM_DS18B20 0x28 +#define W1_EEPROM_DS2431 0x2D #define W1_FAMILY_DS2760 0x30 #define MAXNAMELEN 32 -- cgit v1.2.3