What follows is a long list of fixes and enhancements to the one wire
system, and even some documentation.
I no longer have any deadlocks, a thread was eliminated (along with
its one second wakeup interval), the cpu and time overhead are much
reduced for one wire accesses. The time for the ds2490 to read a
temperature sensor went from 3.91 seconds (.002s user, 3.001s system)
to 0.860 seconds (0.004s user, 0.004s system). I also added support
for the strong pullup to provide more current when requested.
Note, only the documentation patch 29 differs from the previous set of
patches sent July 11th.
--
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
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(-)
diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index 10211e4..ea894bf 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 7293c9b..25640f6 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -46,20 +46,16 @@ MODULE_AUTHOR("Evgeniy Polyakov <[email protected]>");
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 f1df534..13e0ea8 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 a3c95bd..8c35f9c 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 ef1e1da..296a87e 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 6840dfe..ed32280 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 30b6fbf..0056ef6 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);
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 25640f6..aac03f1 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 ) {
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 19 ++++++-------------
drivers/w1/w1_int.c | 26 +++++++++++++++++---------
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index aac03f1..730faa4 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 ed32280..36ff402 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);
--
1.4.4.4
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.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 30 ++++++++++++++++++++++
drivers/w1/w1.h | 12 +++++++++
drivers/w1/w1_int.c | 16 ++++++++++++
drivers/w1/w1_io.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++---
4 files changed, 122 insertions(+), 4 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 9b5c117..76274ae 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -270,6 +270,34 @@ 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)
+{
+ struct w1_master *md = dev_to_w1_master(dev);
+
+ mutex_lock(&md->mutex);
+ md->enable_pullup = simple_strtol(buf, NULL, 0);
+ 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 +393,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 +404,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 34ee01e..00b84ab 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 bd877b2..9d723ef 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 0056ef6..a2f0d5f 100644
--- a/drivers/w1/w1_io.c
+++ b/drivers/w1/w1_io.c
@@ -93,6 +93,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.
*
* @param dev the master device
@@ -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);
--
1.4.4.4
Added strong pullup to thermal sensor driver and general documentation
on the sensor.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
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(-)
diff --git a/Documentation/w1/00-INDEX b/Documentation/w1/00-INDEX
index 5270cf4..cb49802 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 0000000..f8101d6
--- /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 0000000..0403aaa
--- /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 <[email protected]>
+
+
+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 fb28aca..e87f464 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 <[email protected]>");
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)) {
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 20 +++++++++++++++++---
drivers/w1/w1.h | 4 ----
drivers/w1/w1_int.c | 2 --
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 730faa4..9b5c117 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 13e0ea8..34ee01e 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 36ff402..bd877b2 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);
--
1.4.4.4
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.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 136 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 76274ae..24312e0 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;
@@ -349,7 +351,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;
@@ -374,6 +377,134 @@ 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;
+ u64 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, \
@@ -394,6 +525,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,
@@ -405,6 +538,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
};
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 26 ++++++--------------------
1 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 24312e0..4bf2001 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -836,9 +836,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);
@@ -846,24 +844,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);
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_int.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 9d723ef..3fd6e66 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
--
1.4.4.4
Document w1_master_add, w1_master_remove, search_count, and pullup.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
Documentation/w1/w1.generic | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/Documentation/w1/w1.generic b/Documentation/w1/w1.generic
index 4c6509d..e3333ee 100644
--- a/Documentation/w1/w1.generic
+++ b/Documentation/w1/w1.generic
@@ -79,10 +79,13 @@ w1 master sysfs interface
<xx-xxxxxxxxxxxxx> - a directory for a found device. The format is family-serial
bus - (standard) symlink to the w1 bus
driver - (standard) symlink to the w1 driver
+w1_master_add - Manually register a slave device
w1_master_attempts - the number of times a search was attempted
w1_master_max_slave_count
- the maximum slaves that may be attached to a master
w1_master_name - the name of the device (w1_bus_masterX)
+w1_master_pullup - 5V strong pullup 0 enabled, 1 disabled
+w1_master_remove - Manually remove a slave device
w1_master_search - the number of searches left to do, -1=continual (default)
w1_master_slave_count
- the number of slaves found
@@ -90,7 +93,13 @@ w1_master_slaves - the names of the slaves, one per line
w1_master_timeout - the delay in seconds between searches
If you have a w1 bus that never changes (you don't add or remove devices),
-you can set w1_master_search to a positive value to disable searches.
+you can set the module parameter search_count to a small positive number
+for an initially small number of bus searches. Alternatively it could be
+set to zero, then manually add the slave device serial numbers by
+w1_master_add device file. The w1_master_add and w1_master_remove files
+generally only make sense when searching is disabled, as a search will
+redetect manually removed devices that are present and timeout manually
+added devices that aren't on the bus.
w1 slave sysfs interface
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 35 ++++++++++-------------------------
1 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 4bf2001..67b4b99 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 */
@@ -641,7 +626,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",
@@ -663,7 +648,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:
@@ -745,7 +730,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);
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/slaves/w1_therm.c | 55 +++++++++++++++--------------------------
drivers/w1/w1.h | 1 -
2 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index e87f464..7de99df 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 00b84ab..cdaa6ff 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
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_family.c | 7 +------
drivers/w1/w1_family.h | 1 -
2 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/w1/w1_family.c b/drivers/w1/w1_family.c
index 8c35f9c..4a09904 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 296a87e..3053133 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/slaves/w1_therm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 7de99df..2c8dff9 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));
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_int.c | 32 ++++++++++++++++++++++++++------
1 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 3fd6e66..a3a5456 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);
--
1.4.4.4
s/printk/dev_dbg/
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 67b4b99..0084619 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);
@@ -911,7 +911,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;
}
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_io.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
index a2f0d5f..dd2ecef 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds1wm.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index ea894bf..29e144f 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 |
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index b63b5e0..c8365fb 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;
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 76 +++++++++++++++++++++---------------------
1 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index c8365fb..8ab2c86 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 8ab2c86..7acf6d2 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 7acf6d2..6bfd6a9 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 21 ++++-----------------
1 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 6bfd6a9..b1ae1a0 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)
--
1.4.4.4
- 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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 134 +++++++++++++++++++++++++++----------------
1 files changed, 84 insertions(+), 50 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index b1ae1a0..0bcdd3f 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<count; ++i)
printk("%02x ", buf[i]);
- printk("\n");
+ printk(KERN_INFO "\n");
if (count >= 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;
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 0bcdd3f..eb902ab 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index eb902ab..754a4bb 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
--
1.4.4.4
This replaces some magic numbers with marcos and corrects one marco.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 754a4bb..d9e07d6 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;
--
1.4.4.4
Drop the extra ds_wait_status() in ds_write_block().
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index d9e07d6..466048f 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;
--
1.4.4.4
Provide some additional details about the status of the driver and the
ds2490 hardware.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
Cc: Randy Dunlap <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
Documentation/w1/masters/ds2490 | 52 +++++++++++++++++++++++++++++++++++++++
1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/Documentation/w1/masters/ds2490 b/Documentation/w1/masters/ds2490
index 239f9ae..28176de 100644
--- a/Documentation/w1/masters/ds2490
+++ b/Documentation/w1/masters/ds2490
@@ -16,3 +16,55 @@ which allows to build USB <-> W1 bridges.
DS9490(R) is a USB <-> W1 bus master device
which has 0x81 family ID integrated chip and DS2490
low-level operational chip.
+
+Notes and limitations.
+- The weak pullup current is a minimum of 0.9mA and maximum of 6.0mA.
+- The 5V strong pullup is supported with a minimum of 5.9mA and a
+ maximum of 30.4 mA. (From DS2490.pdf)
+- While the ds2490 supports a hardware search the code doesn't take
+ advantage of it (in tested case it only returned first device).
+- The hardware will detect when devices are attached to the bus on the
+ next bus (reset?) operation, however only a message is printed as
+ the core w1 code doesn't make use of the information. Connecting
+ one device tends to give multiple new device notifications.
+- The number of USB bus transactions could be reduced if w1_reset_send
+ was added to the API. The name is just a suggestion. It would take
+ a write buffer and a read buffer (along with sizes) as arguments.
+ The ds2490 block I/O command supports reset, write buffer, read
+ buffer, and strong pullup all in one command, instead of the current
+ 1 reset bus, 2 write the match rom command and slave rom id, 3 block
+ write and read data. The write buffer needs to have the match rom
+ command and slave rom id prepended to the front of the requested
+ write buffer, both of which are known to the driver.
+- The hardware supports normal, flexible, and overdrive bus
+ communication speeds, but only the normal is supported.
+- The registered w1_bus_master functions don't define error
+ conditions. If a bus search is in progress and the ds2490 is
+ removed it can produce a good amount of error output before the bus
+ search finishes.
+- The hardware supports detecting some error conditions, such as
+ short, alarming presence on reset, and no presence on reset, but the
+ driver doesn't query those values.
+- The ds2490 specification doesn't cover short bulk in reads in
+ detail, but my observation is if fewer bytes are requested than are
+ available, the bulk read will return an error and the hardware will
+ clear the entire bulk in buffer. It would be possible to read the
+ maximum buffer size to not run into this error condition, only extra
+ bytes in the buffer is a logic error in the driver. The code should
+ should match reads and writes as well as data sizes. Reads and
+ writes are serialized and the status verifies that the chip is idle
+ (and data is available) before the read is executed, so it should
+ not happen.
+- Running x86_64 2.6.24 UHCI under qemu 0.9.0 under x86_64 2.6.22-rc6
+ with a OHCI controller, ds2490 running in the guest would operate
+ normally the first time the module was loaded after qemu attached
+ the ds2490 hardware, but if the module was unloaded, then reloaded
+ most of the time one of the bulk out or in, and usually the bulk in
+ would fail. qemu sets a 50ms timeout and the bulk in would timeout
+ even when the status shows data available. A bulk out write would
+ show a successful completion, but the ds2490 status register would
+ show 0 bytes written. Detaching qemu from the ds2490 hardware and
+ reattaching would clear the problem. usbmon output in the guest and
+ host did not explain the problem. My guess is a bug in either qemu
+ or the host OS and more likely the host OS.
+-- 03-06-2008 David Fries <[email protected]>
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 64 ++++++++++++++++++++++++++++++-------------
1 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 466048f..d38ac01 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;
--
1.4.4.4
On Mon, 28 Jul 2008 21:04:33 -0500
David Fries <[email protected]> wrote:
> What follows is a long list of fixes and enhancements to the one wire
> system, and even some documentation.
>
> I no longer have any deadlocks, a thread was eliminated (along with
> its one second wakeup interval), the cpu and time overhead are much
> reduced for one wire accesses. The time for the ds2490 to read a
> temperature sensor went from 3.91 seconds (.002s user, 3.001s system)
> to 0.860 seconds (0.004s user, 0.004s system). I also added support
> for the strong pullup to provide more current when requested.
This is all dreadfully late for 2.6.27, but it does seem to be rather
important, so let's aim for 2.6.27.
The w1 code seems to have rather a lot of comments which start with /**
but which aren't kerneldoc comments. But /** is exclusively used to
signify the start of a kerneldoc comment. Please let's not invent new
commenting styles like this. Documentation/CodingStyle is there to
help.
Please be aware that this:
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
protocol="application/pgp-signature"; boundary="OgqxwSJOaUobr8KG"
is rather receiver-hostile. My MUA (at least) (sylpheed) manages to
make a complete mess when saving-to-file, so I needed to go through all
the patches and do various manual steps to fix this up. I may still have
some "=066"s in the changelogs.
Also, some MUAs (but not sylpheed) may be unable to quote the patch
text when someone does a reply to your email, which is also a bit of a
hassle.
Nothing beats plain old ascii text. Preferably non-wordwrapped,
non-tab-replaced, non-spacestuffed ascii text (which is becoming
increasingly rare).
On Mon, 28 Jul 2008 21:14:51 -0500
David Fries <[email protected]> wrote:
> 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.
>
> Signed-off-by: David Fries <[email protected]>
> Signed-off-by: Evgeniy Polyakov <[email protected]>
> ---
> drivers/w1/w1.c | 30 ++++++++++++++++++++++
> drivers/w1/w1.h | 12 +++++++++
> drivers/w1/w1_int.c | 16 ++++++++++++
> drivers/w1/w1_io.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 4 files changed, 122 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 9b5c117..76274ae 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -270,6 +270,34 @@ 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)
> +{
> + struct w1_master *md = dev_to_w1_master(dev);
> +
> + mutex_lock(&md->mutex);
> + md->enable_pullup = simple_strtol(buf, NULL, 0);
> + mutex_unlock(&md->mutex);
> + wake_up_process(md->thread);
> +
> + return count;
> +}
checkpatch says:
WARNING: consider using strict_strtol in preference to simple_strtol
#49: FILE: drivers/w1/w1.c:280:
+ md->enable_pullup = simple_strtol(buf, NULL, 0);
for good reasons. The code which you have implemented will treat the
value "45foo" as 45, which is sloppy. strict_strtoul() (if
handled appropriately) will perform the necessary checking.
A simple_strtol->strict_strtol conversion is a non-backward-compatible
userspace interface change (albeit a pretty non-risky one) and should
be done with caution.
Please use checkpatch.
On Tue, Jul 29, 2008 at 04:13:56PM -0700, Andrew Morton wrote:
> On Mon, 28 Jul 2008 21:04:33 -0500
> David Fries <[email protected]> wrote:
>
> > What follows is a long list of fixes and enhancements to the one wire
> > system, and even some documentation.
> >
> > I no longer have any deadlocks, a thread was eliminated (along with
> > its one second wakeup interval), the cpu and time overhead are much
> > reduced for one wire accesses. The time for the ds2490 to read a
> > temperature sensor went from 3.91 seconds (.002s user, 3.001s system)
> > to 0.860 seconds (0.004s user, 0.004s system). I also added support
> > for the strong pullup to provide more current when requested.
>
> This is all dreadfully late for 2.6.27, but it does seem to be rather
> important, so let's aim for 2.6.27.
Arguments for sooner: fixes some bad bugs, lower risk as it is
isolated to the w1 driver.
Arguments for later, the bugs aren't new, the first version of the
patch was sent in March and I have yet to get a response from anyone
using the ds1wm master (in some ARM handhelds for battery readings),
maybe if it gets in the merge window someone with the hardware will
actually try it before a kernel release.
I'm fine with either.
> The w1 code seems to have rather a lot of comments which start with /**
> but which aren't kerneldoc comments. But /** is exclusively used to
> signify the start of a kerneldoc comment. Please let's not invent new
> commenting styles like this. Documentation/CodingStyle is there to
> help.
I did a quick look, some should be api documentation and updated for
kerneldoc, some clearly aren't. I'll let Evgeniy Polyakov address
those.
> Please be aware that this:
>
> Mime-Version: 1.0
> Content-Type: multipart/signed; micalg=pgp-sha1;
> protocol="application/pgp-signature"; boundary="OgqxwSJOaUobr8KG"
>
> is rather receiver-hostile. My MUA (at least) (sylpheed) manages to
> make a complete mess when saving-to-file, so I needed to go through all
> the patches and do various manual steps to fix this up. I may still have
> some "=066"s in the changelogs.
I would have resubmitted it if you had asked, I'm using mutt, which
isn't exactly new or unknown, I guess the incompatibility is why
encryption and signing e-mail hasn't taken taken off as it could have.
There are some problems left in the changelog.
How about one tar.gz?
> WARNING: consider using strict_strtol in preference to simple_strtol
> Please use checkpatch.
I did use checkpatch.pl, simple_strtol was the only warning (I ignored
it to be consistent, the fix follows). strict_strtol is safe here as
it only reads one integer from sysfs. The patch didn't change,
checkpatch.pl did, I've updated to that as well.
The next set of patches has these minor updates, and checkpatch.pl
returns no errors or warnings.
0005-W1-feature-enable-hardware-strong-pullup.txt
Switch to strict_strtol, code style fixups.
0017-W1-w1_io.c-reset-comments-and-msleep.txt
Fix changelog long lines.
0020-W1-ds2490.c-add-support-for-strong-pullup.txt
Whitespace code style fixups.
0030-W1-ds2490.c-optimize-ds_set_pullup.txt
Whitespace code style fixups.
--
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 25640f6..aac03f1 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 ) {
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
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(-)
diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index 10211e4..ea894bf 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 7293c9b..25640f6 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -46,20 +46,16 @@ MODULE_AUTHOR("Evgeniy Polyakov <[email protected]>");
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 f1df534..13e0ea8 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 a3c95bd..8c35f9c 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 ef1e1da..296a87e 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 6840dfe..ed32280 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 30b6fbf..0056ef6 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);
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 19 ++++++-------------
drivers/w1/w1_int.c | 26 +++++++++++++++++---------
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index aac03f1..730faa4 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 ed32280..36ff402 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);
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 20 +++++++++++++++++---
drivers/w1/w1.h | 4 ----
drivers/w1/w1_int.c | 2 --
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 730faa4..9b5c117 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 13e0ea8..34ee01e 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 36ff402..bd877b2 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);
--
1.4.4.4
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 <[email protected]>
Cc: Evgeniy Polyakov <[email protected]>
---
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(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 9b5c117..32418d4 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 34ee01e..00b84ab 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 bd877b2..9d723ef 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 0056ef6..97b338a 100644
--- a/drivers/w1/w1_io.c
+++ b/drivers/w1/w1_io.c
@@ -93,6 +93,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.
*
* @param dev the master device
@@ -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);
--
1.4.4.4
Added strong pullup to thermal sensor driver and general documentation
on the sensor.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
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(-)
diff --git a/Documentation/w1/00-INDEX b/Documentation/w1/00-INDEX
index 5270cf4..cb49802 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 0000000..f8101d6
--- /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 0000000..0403aaa
--- /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 <[email protected]>
+
+
+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 fb28aca..e87f464 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 <[email protected]>");
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)) {
--
1.4.4.4
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.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 136 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 32418d4..354c12a 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,134 @@ 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;
+ u64 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 +533,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 +546,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
};
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 35 ++++++++++-------------------------
1 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index cd5bd23..3b15c57 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 */
@@ -649,7 +634,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",
@@ -671,7 +656,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:
@@ -753,7 +738,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);
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 26 ++++++--------------------
1 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 354c12a..cd5bd23 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -844,9 +844,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);
@@ -854,24 +852,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);
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_family.c | 7 +------
drivers/w1/w1_family.h | 1 -
2 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/w1/w1_family.c b/drivers/w1/w1_family.c
index 8c35f9c..4a09904 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 296a87e..3053133 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_int.c | 32 ++++++++++++++++++++++++++------
1 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 3fd6e66..a3a5456 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);
--
1.4.4.4
s/printk/dev_dbg/
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 3b15c57..355109f 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);
@@ -919,7 +919,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;
}
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_io.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
index 97b338a..f4f82f1 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_int.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 9d723ef..3fd6e66 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
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds1wm.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index ea894bf..29e144f 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 |
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/slaves/w1_therm.c | 55 +++++++++++++++--------------------------
drivers/w1/w1.h | 1 -
2 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index e87f464..7de99df 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 00b84ab..cdaa6ff 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
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/slaves/w1_therm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 7de99df..2c8dff9 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));
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index b63b5e0..c8365fb 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;
}
--
1.4.4.4
Document w1_master_add, w1_master_remove, search_count, and pullup.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
Documentation/w1/w1.generic | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/Documentation/w1/w1.generic b/Documentation/w1/w1.generic
index 4c6509d..e3333ee 100644
--- a/Documentation/w1/w1.generic
+++ b/Documentation/w1/w1.generic
@@ -79,10 +79,13 @@ w1 master sysfs interface
<xx-xxxxxxxxxxxxx> - a directory for a found device. The format is family-serial
bus - (standard) symlink to the w1 bus
driver - (standard) symlink to the w1 driver
+w1_master_add - Manually register a slave device
w1_master_attempts - the number of times a search was attempted
w1_master_max_slave_count
- the maximum slaves that may be attached to a master
w1_master_name - the name of the device (w1_bus_masterX)
+w1_master_pullup - 5V strong pullup 0 enabled, 1 disabled
+w1_master_remove - Manually remove a slave device
w1_master_search - the number of searches left to do, -1=continual (default)
w1_master_slave_count
- the number of slaves found
@@ -90,7 +93,13 @@ w1_master_slaves - the names of the slaves, one per line
w1_master_timeout - the delay in seconds between searches
If you have a w1 bus that never changes (you don't add or remove devices),
-you can set w1_master_search to a positive value to disable searches.
+you can set the module parameter search_count to a small positive number
+for an initially small number of bus searches. Alternatively it could be
+set to zero, then manually add the slave device serial numbers by
+w1_master_add device file. The w1_master_add and w1_master_remove files
+generally only make sense when searching is disabled, as a search will
+redetect manually removed devices that are present and timeout manually
+added devices that aren't on the bus.
w1 slave sysfs interface
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 76 +++++++++++++++++++++---------------------
1 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index c8365fb..6b188e8 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 64 ++++++++++++++++++++++++++++++-------------
1 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 4faf4f9..59ad6e9 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 6b188e8..6fabf58 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 6fabf58..1b632d5 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 21 ++++-----------------
1 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 1b632d5..c4ff70b 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)
--
1.4.4.4
- 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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 134 +++++++++++++++++++++++++++----------------
1 files changed, 84 insertions(+), 50 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index c4ff70b..065042d 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<count; ++i)
printk("%02x ", buf[i]);
- printk("\n");
+ printk(KERN_INFO "\n");
if (count >= 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;
}
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 065042d..9a7fd71 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;
--
1.4.4.4
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 <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 9a7fd71..0f35693 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
--
1.4.4.4
This replaces some magic numbers with marcos and corrects one marco.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 0f35693..29df1d2 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;
--
1.4.4.4
Drop the extra ds_wait_status() in ds_write_block().
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 29df1d2..4faf4f9 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;
--
1.4.4.4
Provide some additional details about the status of the driver and the
ds2490 hardware.
Signed-off-by: David Fries <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
Cc: Randy Dunlap <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
Documentation/w1/masters/ds2490 | 52 +++++++++++++++++++++++++++++++++++++++
1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/Documentation/w1/masters/ds2490 b/Documentation/w1/masters/ds2490
index 239f9ae..28176de 100644
--- a/Documentation/w1/masters/ds2490
+++ b/Documentation/w1/masters/ds2490
@@ -16,3 +16,55 @@ which allows to build USB <-> W1 bridges.
DS9490(R) is a USB <-> W1 bus master device
which has 0x81 family ID integrated chip and DS2490
low-level operational chip.
+
+Notes and limitations.
+- The weak pullup current is a minimum of 0.9mA and maximum of 6.0mA.
+- The 5V strong pullup is supported with a minimum of 5.9mA and a
+ maximum of 30.4 mA. (From DS2490.pdf)
+- While the ds2490 supports a hardware search the code doesn't take
+ advantage of it (in tested case it only returned first device).
+- The hardware will detect when devices are attached to the bus on the
+ next bus (reset?) operation, however only a message is printed as
+ the core w1 code doesn't make use of the information. Connecting
+ one device tends to give multiple new device notifications.
+- The number of USB bus transactions could be reduced if w1_reset_send
+ was added to the API. The name is just a suggestion. It would take
+ a write buffer and a read buffer (along with sizes) as arguments.
+ The ds2490 block I/O command supports reset, write buffer, read
+ buffer, and strong pullup all in one command, instead of the current
+ 1 reset bus, 2 write the match rom command and slave rom id, 3 block
+ write and read data. The write buffer needs to have the match rom
+ command and slave rom id prepended to the front of the requested
+ write buffer, both of which are known to the driver.
+- The hardware supports normal, flexible, and overdrive bus
+ communication speeds, but only the normal is supported.
+- The registered w1_bus_master functions don't define error
+ conditions. If a bus search is in progress and the ds2490 is
+ removed it can produce a good amount of error output before the bus
+ search finishes.
+- The hardware supports detecting some error conditions, such as
+ short, alarming presence on reset, and no presence on reset, but the
+ driver doesn't query those values.
+- The ds2490 specification doesn't cover short bulk in reads in
+ detail, but my observation is if fewer bytes are requested than are
+ available, the bulk read will return an error and the hardware will
+ clear the entire bulk in buffer. It would be possible to read the
+ maximum buffer size to not run into this error condition, only extra
+ bytes in the buffer is a logic error in the driver. The code should
+ should match reads and writes as well as data sizes. Reads and
+ writes are serialized and the status verifies that the chip is idle
+ (and data is available) before the read is executed, so it should
+ not happen.
+- Running x86_64 2.6.24 UHCI under qemu 0.9.0 under x86_64 2.6.22-rc6
+ with a OHCI controller, ds2490 running in the guest would operate
+ normally the first time the module was loaded after qemu attached
+ the ds2490 hardware, but if the module was unloaded, then reloaded
+ most of the time one of the bulk out or in, and usually the bulk in
+ would fail. qemu sets a 50ms timeout and the bulk in would timeout
+ even when the status shows data available. A bulk out write would
+ show a successful completion, but the ds2490 status register would
+ show 0 bytes written. Detaching qemu from the ds2490 hardware and
+ reattaching would clear the problem. usbmon output in the guest and
+ host did not explain the problem. My guess is a bug in either qemu
+ or the host OS and more likely the host OS.
+-- 03-06-2008 David Fries <[email protected]>
--
1.4.4.4
On Wed, 30 Jul 2008 21:37:02 -0500 David Fries <[email protected]> wrote:
> On Tue, Jul 29, 2008 at 04:13:56PM -0700, Andrew Morton wrote:
> > On Mon, 28 Jul 2008 21:04:33 -0500
> > David Fries <[email protected]> wrote:
> >
> > > What follows is a long list of fixes and enhancements to the one wire
> > > system, and even some documentation.
> > >
> > > I no longer have any deadlocks, a thread was eliminated (along with
> > > its one second wakeup interval), the cpu and time overhead are much
> > > reduced for one wire accesses. The time for the ds2490 to read a
> > > temperature sensor went from 3.91 seconds (.002s user, 3.001s system)
> > > to 0.860 seconds (0.004s user, 0.004s system). I also added support
> > > for the strong pullup to provide more current when requested.
> >
> > This is all dreadfully late for 2.6.27, but it does seem to be rather
> > important, so let's aim for 2.6.27.
>
> Arguments for sooner: fixes some bad bugs, lower risk as it is
> isolated to the w1 driver.
>
> Arguments for later, the bugs aren't new, the first version of the
> patch was sent in March and I have yet to get a response from anyone
> using the ds1wm master (in some ARM handhelds for battery readings),
> maybe if it gets in the merge window someone with the hardware will
> actually try it before a kernel release.
>
> I'm fine with either.
It all seems to fix more than it looks like it breaks ;) Let's shoot
for 2.6.27.
>
> > Please be aware that this:
> >
> > Mime-Version: 1.0
> > Content-Type: multipart/signed; micalg=pgp-sha1;
> > protocol="application/pgp-signature"; boundary="OgqxwSJOaUobr8KG"
> >
> > is rather receiver-hostile. My MUA (at least) (sylpheed) manages to
> > make a complete mess when saving-to-file, so I needed to go through all
> > the patches and do various manual steps to fix this up. I may still have
> > some "=066"s in the changelogs.
>
> I would have resubmitted it if you had asked, I'm using mutt, which
> isn't exactly new or unknown, I guess the incompatibility is why
> encryption and signing e-mail hasn't taken taken off as it could have.
> There are some problems left in the changelog.
>
> How about one tar.gz?
eek, terror. Lots of error-prone handwork is needed for that.
One patch per text/plain email is always preferred, please. That's
what everyone's automation is designed for.
> > WARNING: consider using strict_strtol in preference to simple_strtol
>
> > Please use checkpatch.
>
> I did use checkpatch.pl, simple_strtol was the only warning (I ignored
> it to be consistent, the fix follows). strict_strtol is safe here as
> it only reads one integer from sysfs. The patch didn't change,
> checkpatch.pl did, I've updated to that as well.
>
>
> The next set of patches has these minor updates, and checkpatch.pl
> returns no errors or warnings.
>
> 0005-W1-feature-enable-hardware-strong-pullup.txt
> Switch to strict_strtol, code style fixups.
>
> 0017-W1-w1_io.c-reset-comments-and-msleep.txt
> Fix changelog long lines.
>
> 0020-W1-ds2490.c-add-support-for-strong-pullup.txt
> Whitespace code style fixups.
>
> 0030-W1-ds2490.c-optimize-ds_set_pullup.txt
> Whitespace code style fixups.
um, OK, I'll do a full drop-and-remerge.