2014-01-16 04:29:49

by David Fries

[permalink] [raw]
Subject: [PATCH 00/14] w1: async netlink, search, fixes, and improvements

This patch series aims to extend and improve the netlink interface to
the one wire system. Netlink is exposed as a socket interface which
is normally assumed to be asynchronous which only blocks waiting on
incoming packets or a fulls end buffer, but were executed on the
calling thread. A bus device search can be a lengthy operation and
temperature conversion can take 750 ms. With this patch series those
lengthy operations are now processed on the thread originally created
to do automatic searching, which gives it a reason to exist when
automatic search is disabled. Some other improvements for netlink is
additional commands to add, remove, and list the known slaves, and
send replies only to the requester. They were previously multicast to
everyone listening on the group, this required an additional argument
to cn_netlink_send. This also takes the bus_mutex for netlink and
search for masters that didn't already do so.

Search improvements include returning all devices found, as the first
that wouldn't fit in a packet would be lost. I marked only this patch
for the stable tree, though you would have to have a lot of devices on
one bus to hit it. I had a fix to support aborting from netlink
rather than just avoiding the oops as the previous patch did.
The search can now pick up where it left off, along with setting the
maximum number of slaves to search for at a time, a large network
could split up the search operation into multiple passes. ds2490 USB
dongle now gains support for hardware search and along with not
undoing the interface selection operation takes the search for my
network from 23.16 seconds to .307346 seconds.

The largest single change is to change the comment style for DocBook
to process in addition to comment some additional functions and
data structures.


2014-01-16 04:30:01

by David Fries

[permalink] [raw]
Subject: [PATCH 01/15] w1: fix w1_send_slave dropping a slave id

Previous logic,
if (avail > 8) {
store slave;
return;
}
send data; clear;

The logic error is, if there isn't space send the buffer and clear,
but the slave wasn't added to the now empty buffer loosing that slave
id. It also should have been "if (avail >= 8)" because when it is 8,
there is space.

Instead, if there isn't space send and clear the buffer, then there is
always space for the slave id.

Signed-off-by: David Fries <[email protected]>
Cc: [email protected]
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1_netlink.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 40788c9..73705af 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -54,28 +54,29 @@ static void w1_send_slave(struct w1_master *dev, u64 rn)
struct w1_netlink_msg *hdr = (struct w1_netlink_msg *)(msg + 1);
struct w1_netlink_cmd *cmd = (struct w1_netlink_cmd *)(hdr + 1);
int avail;
+ u64 *data;

/* update kernel slave list */
w1_slave_found(dev, rn);

avail = dev->priv_size - cmd->len;

- if (avail > 8) {
- u64 *data = (void *)(cmd + 1) + cmd->len;
+ if (avail < 8) {
+ msg->ack++;
+ cn_netlink_send(msg, 0, GFP_KERNEL);

- *data = rn;
- cmd->len += 8;
- hdr->len += 8;
- msg->len += 8;
- return;
+ msg->len = sizeof(struct w1_netlink_msg) +
+ sizeof(struct w1_netlink_cmd);
+ hdr->len = sizeof(struct w1_netlink_cmd);
+ cmd->len = 0;
}

- msg->ack++;
- cn_netlink_send(msg, 0, GFP_KERNEL);
+ data = (void *)(cmd + 1) + cmd->len;

- msg->len = sizeof(struct w1_netlink_msg) + sizeof(struct w1_netlink_cmd);
- hdr->len = sizeof(struct w1_netlink_cmd);
- cmd->len = 0;
+ *data = rn;
+ cmd->len += 8;
+ hdr->len += 8;
+ msg->len += 8;
}

static int w1_process_search_command(struct w1_master *dev, struct cn_msg *msg,
--
1.7.10.4

2014-01-16 04:30:19

by David Fries

[permalink] [raw]
Subject: [PATCH 02/15] w1: fixup search to support abort from netlink

Before 63706172f33 "rework kthread_stop()" kthread_should_stop()
always returned false when called from a non-kthread task, after it
would oops as a non-kthread didn't have that structure and netlink was
calling search from a thread which wasn't a kthread. 9d1817cab2f030
"w1: fix oops when w1_search is called from netlink connector",
modified the code to avoid calling kthread_stop from a netlink thread.

Introduce a w1_master flag and bit W1_ABORT_SEARCH to identify abort
to cleanly support both kthread and netlink search abort. A search
can take seconds to run, so it is important to abort early if the
hardware is removed in the middle of a search.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
Cc: Marcin Jurkowski <[email protected]>
Cc: Josh Boyer <[email protected]>
Cc: Sven Geggus <[email protected]>
---
drivers/w1/w1.c | 3 +--
drivers/w1/w1.h | 10 ++++++++++
drivers/w1/w1_int.c | 2 ++
3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 66efa96..67b6d5f 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -960,8 +960,7 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
tmp64 = (triplet_ret >> 2);
rn |= (tmp64 << i);

- /* ensure we're called from kthread and not by netlink callback */
- if (!dev->priv && kthread_should_stop()) {
+ if (test_bit(W1_ABORT_SEARCH, &dev->flags)) {
mutex_unlock(&dev->bus_mutex);
dev_dbg(&dev->dev, "Abort w1_search\n");
return;
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index ca8081a..bc329d2 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -155,6 +155,14 @@ struct w1_bus_master
u8, w1_slave_found_callback);
};

+/**
+ * enum w1_master_flags - bitfields used in w1_master.flags
+ * @W1_ABORT_SEARCH: abort searching early on shutdown
+ */
+enum w1_master_flags {
+ W1_ABORT_SEARCH = 0,
+};
+
struct w1_master
{
struct list_head w1_master_entry;
@@ -178,6 +186,8 @@ struct w1_master
/** 5V strong pullup duration in milliseconds, zero disabled. */
int pullup_duration;

+ long flags;
+
struct task_struct *thread;
struct mutex mutex;
struct mutex bus_mutex;
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 5a98649..f84e48b 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -184,6 +184,7 @@ 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_ABORT_SEARCH, &dev->flags);
kthread_stop(dev->thread);
#endif
err_out_rm_attr:
@@ -199,6 +200,7 @@ void __w1_remove_master_device(struct w1_master *dev)
struct w1_netlink_msg msg;
struct w1_slave *sl, *sln;

+ set_bit(W1_ABORT_SEARCH, &dev->flags);
kthread_stop(dev->thread);

mutex_lock(&w1_mlock);
--
1.7.10.4

2014-01-16 04:31:20

by David Fries

[permalink] [raw]
Subject: [PATCH 03/15] w1: Only wake up the search process if it is going to be searching

It's valid to set the search count to 0 to stop searching, so don't
wake up the search thread to not search.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 67b6d5f..92766a9 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -243,7 +243,9 @@ static ssize_t w1_master_attribute_store_search(struct device * dev,
mutex_lock(&md->mutex);
md->search_count = tmp;
mutex_unlock(&md->mutex);
- wake_up_process(md->thread);
+ /* Only wake if it is going to be searching. */
+ if (tmp)
+ wake_up_process(md->thread);

return count;
}
--
1.7.10.4

2014-01-16 04:31:44

by David Fries

[permalink] [raw]
Subject: [PATCH 04/15] w1: increase w1_max_slave_count, allow write access

w1_max_slave_count is only used to abort the search early
or take a fast search (when 1), so there isn't any reason to not allow
it to be updated through sysfs. Memory is not allocated based on
the current value and 10 is a rather low base number, increasing to
64, and printing a message the first time the count is reached and
there were more devices to discover to let the user know why not
all the devices were found.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 30 ++++++++++++++++++++++++++++--
drivers/w1/w1.h | 2 ++
2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 92766a9..34ffdc6 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -46,7 +46,7 @@ MODULE_AUTHOR("Evgeniy Polyakov <[email protected]>");
MODULE_DESCRIPTION("Driver for 1-wire Dallas network protocol.");

static int w1_timeout = 10;
-int w1_max_slave_count = 10;
+int w1_max_slave_count = 64;
int w1_max_slave_ttl = 10;

module_param_named(timeout, w1_timeout, int, 0);
@@ -316,6 +316,24 @@ static ssize_t w1_master_attribute_show_timeout(struct device *dev, struct devic
return count;
}

+static ssize_t w1_master_attribute_store_max_slave_count(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 (kstrtol(buf, 0, &tmp) == -EINVAL || tmp < 1)
+ return -EINVAL;
+
+ mutex_lock(&md->mutex);
+ md->max_slave_count = tmp;
+ /* allow each time the max_slave_count is updated */
+ clear_bit(W1_WARN_MAX_COUNT, &md->flags);
+ mutex_unlock(&md->mutex);
+
+ return count;
+}
+
static ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, struct device_attribute *attr, char *buf)
{
struct w1_master *md = dev_to_w1_master(dev);
@@ -518,7 +536,7 @@ static ssize_t w1_master_attribute_store_remove(struct device *dev,
static W1_MASTER_ATTR_RO(name, S_IRUGO);
static W1_MASTER_ATTR_RO(slaves, S_IRUGO);
static W1_MASTER_ATTR_RO(slave_count, S_IRUGO);
-static W1_MASTER_ATTR_RO(max_slave_count, S_IRUGO);
+static W1_MASTER_ATTR_RW(max_slave_count, S_IRUGO | S_IWUSR | S_IWGRP);
static W1_MASTER_ATTR_RO(attempts, S_IRUGO);
static W1_MASTER_ATTR_RO(timeout, S_IRUGO);
static W1_MASTER_ATTR_RO(pointer, S_IRUGO);
@@ -976,6 +994,14 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
desc_bit = last_zero;
cb(dev, rn);
}
+
+ if (!last_device && slave_count == dev->max_slave_count &&
+ !test_bit(W1_WARN_MAX_COUNT, &dev->flags)) {
+ dev_info(&dev->dev, "%s: max_slave_count %d reached, "
+ "additional sensors ignored\n", __func__,
+ dev->max_slave_count);
+ set_bit(W1_WARN_MAX_COUNT, &dev->flags);
+ }
}
}

diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index bc329d2..bd10b3c 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -158,9 +158,11 @@ struct w1_bus_master
/**
* enum w1_master_flags - bitfields used in w1_master.flags
* @W1_ABORT_SEARCH: abort searching early on shutdown
+ * @W1_WARN_MAX_COUNT: limit warning when the maximum count is reached
*/
enum w1_master_flags {
W1_ABORT_SEARCH = 0,
+ W1_WARN_MAX_COUNT = 1,
};

struct w1_master
--
1.7.10.4

2014-01-16 04:32:07

by David Fries

[permalink] [raw]
Subject: [PATCH 05/15] w1: continue slave search where previous left off

Search will detect at most max_slave_count devices per run, if there
are more pick up the next search where the previous left off.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 18 +++++++++++++++---
drivers/w1/w1.h | 3 +++
2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 34ffdc6..4c89f85 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -918,7 +918,8 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
u8 triplet_ret = 0;

search_bit = 0;
- rn = last_rn = 0;
+ rn = dev->search_id;
+ last_rn = 0;
last_device = 0;
last_zero = -1;

@@ -989,16 +990,27 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
mutex_unlock(&dev->bus_mutex);

if ( (triplet_ret & 0x03) != 0x03 ) {
- if ( (desc_bit == last_zero) || (last_zero < 0))
+ if ((desc_bit == last_zero) || (last_zero < 0)) {
last_device = 1;
+ dev->search_id = 0;
+ } else {
+ dev->search_id = rn;
+ }
desc_bit = last_zero;
cb(dev, rn);
}

if (!last_device && slave_count == dev->max_slave_count &&
!test_bit(W1_WARN_MAX_COUNT, &dev->flags)) {
+ /* Only max_slave_count will be scanned in a search,
+ * but it will start where it left off next search
+ * until all ids are identified and then it will start
+ * over. A continued search will report the previous
+ * last id as the first id (provided it is still on the
+ * bus).
+ */
dev_info(&dev->dev, "%s: max_slave_count %d reached, "
- "additional sensors ignored\n", __func__,
+ "will continue next search.\n", __func__,
dev->max_slave_count);
set_bit(W1_WARN_MAX_COUNT, &dev->flags);
}
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index bd10b3c..80fbdf9 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -150,6 +150,7 @@ struct w1_bus_master

/** Really nice hardware can handles the different types of ROM search
* w1_master* is passed to the slave found callback.
+ * u8 is search_type, W1_SEARCH or W1_ALARM_SEARCH
*/
void (*search)(void *, struct w1_master *,
u8, w1_slave_found_callback);
@@ -177,6 +178,8 @@ struct w1_master
int initialized;
u32 id;
int search_count;
+ /* id to start searching on, to continue a search or 0 to restart */
+ u64 search_id;

atomic_t refcnt;

--
1.7.10.4

2014-01-16 04:32:27

by David Fries

[permalink] [raw]
Subject: [PATCH 06/15] w1: new netlink commands, add/remove/list slaves

Introduce new commands to add, remove, and list slave devices through
the netlink interface. This can be useful to skip the search on a
static network. They could previously only be added or removed
through automatic search or sysfs, and this allows a program to only
use netlink.

Only allocate memory when needed, so move kzalloc into w1_get_slaves
where it was used.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 6 +--
drivers/w1/w1.h | 3 ++
drivers/w1/w1_netlink.c | 125 ++++++++++++++++++++++++++++++++++-------------
drivers/w1/w1_netlink.h | 31 +++++++++++-
4 files changed, 126 insertions(+), 39 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 4c89f85..97b35cb 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -56,8 +56,6 @@ 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;
@@ -444,7 +442,7 @@ static int w1_atoreg_num(struct device *dev, const char *buf, size_t count,
/* 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_slave *w1_slave_search_device(struct w1_master *dev,
struct w1_reg_num *rn)
{
struct w1_slave *sl;
@@ -711,7 +709,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
return 0;
}

-static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn)
+int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn)
{
struct w1_slave *sl;
struct w1_family *f;
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 80fbdf9..3376bfb 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -213,6 +213,8 @@ struct w1_slave *w1_search_slave(struct w1_reg_num *id);
void w1_slave_found(struct w1_master *dev, u64 rn);
void w1_search_process_cb(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb);
+struct w1_slave *w1_slave_search_device(struct w1_master *dev,
+ struct w1_reg_num *rn);
struct w1_master *w1_search_master_id(u32 id);

/* Disconnect and reconnect devices in the given family. Used for finding
@@ -221,6 +223,7 @@ struct w1_master *w1_search_master_id(u32 id);
* has just been registered, to 0 when it has been unregistered.
*/
void w1_reconnect_slaves(struct w1_family *f, int attach);
+int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn);
void w1_slave_detach(struct w1_slave *sl);

u8 w1_triplet(struct w1_master *dev, int bdir);
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 73705af..747174b 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -56,9 +56,6 @@ static void w1_send_slave(struct w1_master *dev, u64 rn)
int avail;
u64 *data;

- /* update kernel slave list */
- w1_slave_found(dev, rn);
-
avail = dev->priv_size - cmd->len;

if (avail < 8) {
@@ -79,17 +76,57 @@ static void w1_send_slave(struct w1_master *dev, u64 rn)
msg->len += 8;
}

-static int w1_process_search_command(struct w1_master *dev, struct cn_msg *msg,
- unsigned int avail)
+static void w1_found_send_slave(struct w1_master *dev, u64 rn)
{
- struct w1_netlink_msg *hdr = (struct w1_netlink_msg *)(msg + 1);
- struct w1_netlink_cmd *cmd = (struct w1_netlink_cmd *)(hdr + 1);
- int search_type = (cmd->cmd == W1_CMD_ALARM_SEARCH)?W1_ALARM_SEARCH:W1_SEARCH;
+ /* update kernel slave list */
+ w1_slave_found(dev, rn);
+
+ w1_send_slave(dev, rn);
+}
+
+/* Get the current slave list, or search (with or without alarm) */
+static int w1_get_slaves(struct w1_master *dev,
+ struct cn_msg *req_msg, struct w1_netlink_msg *req_hdr,
+ struct w1_netlink_cmd *req_cmd)
+{
+ struct cn_msg *msg;
+ struct w1_netlink_msg *hdr;
+ struct w1_netlink_cmd *cmd;
+ struct w1_slave *sl;
+
+ msg = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->id = req_msg->id;
+ msg->seq = req_msg->seq;
+ msg->ack = 0;
+ msg->len = sizeof(struct w1_netlink_msg) +
+ sizeof(struct w1_netlink_cmd);
+
+ hdr = (struct w1_netlink_msg *)(msg + 1);
+ cmd = (struct w1_netlink_cmd *)(hdr + 1);
+
+ hdr->type = W1_MASTER_CMD;
+ hdr->id = req_hdr->id;
+ hdr->len = sizeof(struct w1_netlink_cmd);
+
+ cmd->cmd = req_cmd->cmd;
+ cmd->len = 0;

dev->priv = msg;
- dev->priv_size = avail;
+ dev->priv_size = PAGE_SIZE - msg->len - sizeof(struct cn_msg);

- w1_search_process_cb(dev, search_type, w1_send_slave);
+ if (req_cmd->cmd == W1_CMD_LIST_SLAVES) {
+ __u64 rn;
+ list_for_each_entry(sl, &dev->slist, w1_slave_entry) {
+ memcpy(&rn, &sl->reg_num, sizeof(rn));
+ w1_send_slave(dev, rn);
+ }
+ } else {
+ w1_search_process_cb(dev, cmd->cmd == W1_CMD_ALARM_SEARCH ?
+ W1_ALARM_SEARCH : W1_SEARCH, w1_found_send_slave);
+ }

msg->ack = 0;
cn_netlink_send(msg, 0, GFP_KERNEL);
@@ -97,6 +134,8 @@ static int w1_process_search_command(struct w1_master *dev, struct cn_msg *msg,
dev->priv = NULL;
dev->priv_size = 0;

+ kfree(msg);
+
return 0;
}

@@ -164,38 +203,52 @@ static int w1_process_command_io(struct w1_master *dev, struct cn_msg *msg,
return err;
}

-static int w1_process_command_master(struct w1_master *dev, struct cn_msg *req_msg,
- struct w1_netlink_msg *req_hdr, struct w1_netlink_cmd *req_cmd)
+static int w1_process_command_addremove(struct w1_master *dev,
+ struct cn_msg *msg, struct w1_netlink_msg *hdr,
+ struct w1_netlink_cmd *cmd)
{
- int err = -EINVAL;
- struct cn_msg *msg;
- struct w1_netlink_msg *hdr;
- struct w1_netlink_cmd *cmd;
+ struct w1_slave *sl;
+ int err = 0;
+ struct w1_reg_num *id;

- msg = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (cmd->len != 8)
+ return -EINVAL;

- msg->id = req_msg->id;
- msg->seq = req_msg->seq;
- msg->ack = 0;
- msg->len = sizeof(struct w1_netlink_msg) + sizeof(struct w1_netlink_cmd);
+ id = (struct w1_reg_num *)cmd->data;

- hdr = (struct w1_netlink_msg *)(msg + 1);
- cmd = (struct w1_netlink_cmd *)(hdr + 1);
+ sl = w1_slave_search_device(dev, id);
+ switch (cmd->cmd) {
+ case W1_CMD_SLAVE_ADD:
+ if (sl)
+ err = -EINVAL;
+ else
+ err = w1_attach_slave_device(dev, id);
+ break;
+ case W1_CMD_SLAVE_REMOVE:
+ if (sl)
+ w1_slave_detach(sl);
+ else
+ err = -EINVAL;
+ break;
+ default:
+ err = -EINVAL;
+ break;
+ }

- hdr->type = W1_MASTER_CMD;
- hdr->id = req_hdr->id;
- hdr->len = sizeof(struct w1_netlink_cmd);
+ return err;
+}

- cmd->cmd = req_cmd->cmd;
- cmd->len = 0;
+static int w1_process_command_master(struct w1_master *dev,
+ struct cn_msg *req_msg, struct w1_netlink_msg *req_hdr,
+ struct w1_netlink_cmd *req_cmd)
+{
+ int err = -EINVAL;

- switch (cmd->cmd) {
+ switch (req_cmd->cmd) {
case W1_CMD_SEARCH:
case W1_CMD_ALARM_SEARCH:
- err = w1_process_search_command(dev, msg,
- PAGE_SIZE - msg->len - sizeof(struct cn_msg));
+ case W1_CMD_LIST_SLAVES:
+ err = w1_get_slaves(dev, req_msg, req_hdr, req_cmd);
break;
case W1_CMD_READ:
case W1_CMD_WRITE:
@@ -205,12 +258,16 @@ static int w1_process_command_master(struct w1_master *dev, struct cn_msg *req_m
case W1_CMD_RESET:
err = w1_reset_bus(dev);
break;
+ case W1_CMD_SLAVE_ADD:
+ case W1_CMD_SLAVE_REMOVE:
+ err = w1_process_command_addremove(dev, req_msg, req_hdr,
+ req_cmd);
+ break;
default:
err = -EINVAL;
break;
}

- kfree(msg);
return err;
}

diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
index b0922dc..ea9f3e4 100644
--- a/drivers/w1/w1_netlink.h
+++ b/drivers/w1/w1_netlink.h
@@ -27,6 +27,17 @@

#include "w1.h"

+/** enum w1_netlink_message_types - message type
+ *
+ * @W1_SLAVE_ADD: notification that a slave device was added
+ * @W1_SLAVE_REMOVE: notification that a slave device was removed
+ * @W1_MASTER_ADD: notification that a new bus master was added
+ * @W1_MASTER_REMOVE: notification that a bus masterwas removed
+ * @W1_MASTER_CMD: initiate operations on a specific master
+ * @W1_SLAVE_CMD: sends reset, selects the slave, then does a read/write/touch
+ * operation
+ * @W1_LIST_MASTERS: used to determine the bus master identifiers
+ */
enum w1_netlink_message_types {
W1_SLAVE_ADD = 0,
W1_SLAVE_REMOVE,
@@ -52,6 +63,21 @@ struct w1_netlink_msg
__u8 data[0];
};

+/** enum w1_commands - commands available for master or slave operations
+ * @W1_CMD_READ: read len bytes
+ * @W1_CMD_WRITE: write len bytes
+ * @W1_CMD_SEARCH: initiate a standard search, returns only the slave
+ * devices found during that search
+ * @W1_CMD_ALARM_SEARCH: search for devices that are currently alarming
+ * @W1_CMD_TOUCH: Touches a series of bytes.
+ * @W1_CMD_RESET: sends a bus reset on the given master
+ * @W1_CMD_SLAVE_ADD: adds a slave to the given master,
+ * 8 byte slave id at data[0]
+ * @W1_CMD_SLAVE_REMOVE: removes a slave to the given master,
+ * 8 byte slave id at data[0]
+ * @W1_CMD_LIST_SLAVES: list of slaves registered on this master
+ * @W1_CMD_MAX: number of available commands
+ */
enum w1_commands {
W1_CMD_READ = 0,
W1_CMD_WRITE,
@@ -59,7 +85,10 @@ enum w1_commands {
W1_CMD_ALARM_SEARCH,
W1_CMD_TOUCH,
W1_CMD_RESET,
- W1_CMD_MAX,
+ W1_CMD_SLAVE_ADD,
+ W1_CMD_SLAVE_REMOVE,
+ W1_CMD_LIST_SLAVES,
+ W1_CMD_MAX
};

struct w1_netlink_cmd
--
1.7.10.4

2014-01-16 04:32:44

by David Fries

[permalink] [raw]
Subject: [PATCH 07/15] w1: process w1 netlink commands in w1_process thread

Netlink is a socket interface and is expected to be asynchronous.
Clients can now make w1 requests without blocking by making use of the
w1_master thread to process netlink commands which was previously only
used for doing an automatic bus search.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.c | 180 +++++++++++++++++++++++++++++++++--------------
drivers/w1/w1.h | 32 ++++++++-
drivers/w1/w1_int.c | 17 +++--
drivers/w1/w1_netlink.c | 166 +++++++++++++++++++++++++++++++++----------
4 files changed, 300 insertions(+), 95 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 97b35cb..53846c7 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -79,19 +79,10 @@ static void w1_slave_release(struct device *dev)
{
struct w1_slave *sl = dev_to_w1_slave(dev);

- dev_dbg(dev, "%s: Releasing %s.\n", __func__, sl->name);
-
- while (atomic_read(&sl->refcnt)) {
- 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);
- }
+ dev_dbg(dev, "%s: Releasing %s [%p]\n", __func__, sl->name, sl);

w1_family_put(sl->family);
sl->master->slave_count--;
-
- complete(&sl->released);
}

static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -277,7 +268,6 @@ static ssize_t w1_master_attribute_store_pullup(struct device *dev,
mutex_lock(&md->mutex);
md->enable_pullup = tmp;
mutex_unlock(&md->mutex);
- wake_up_process(md->thread);

return count;
}
@@ -370,23 +360,20 @@ static ssize_t w1_master_attribute_show_slaves(struct device *dev,
{
struct w1_master *md = dev_to_w1_master(dev);
int c = PAGE_SIZE;
+ struct list_head *ent, *n;
+ struct w1_slave *sl = NULL;

- mutex_lock(&md->mutex);
-
- if (md->slave_count == 0)
- c -= snprintf(buf + PAGE_SIZE - c, c, "not found.\n");
- else {
- struct list_head *ent, *n;
- struct w1_slave *sl;
+ mutex_lock(&md->list_mutex);

- list_for_each_safe(ent, n, &md->slist) {
- sl = list_entry(ent, struct w1_slave, w1_slave_entry);
+ list_for_each_safe(ent, n, &md->slist) {
+ sl = list_entry(ent, struct w1_slave, w1_slave_entry);

- c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", sl->name);
- }
+ c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", sl->name);
}
+ if (!sl)
+ c -= snprintf(buf + PAGE_SIZE - c, c, "not found.\n");

- mutex_unlock(&md->mutex);
+ mutex_unlock(&md->list_mutex);

return PAGE_SIZE - c;
}
@@ -440,19 +427,22 @@ static int w1_atoreg_num(struct device *dev, const char *buf, size_t count,
}

/* Searches the slaves in the w1_master and returns a pointer or NULL.
- * Note: must hold the mutex
+ * Note: must not hold list_mutex
*/
struct w1_slave *w1_slave_search_device(struct w1_master *dev,
struct w1_reg_num *rn)
{
struct w1_slave *sl;
+ mutex_lock(&dev->list_mutex);
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) {
+ mutex_unlock(&dev->list_mutex);
return sl;
}
}
+ mutex_unlock(&dev->list_mutex);
return NULL;
}

@@ -509,7 +499,10 @@ static ssize_t w1_master_attribute_store_remove(struct device *dev,
mutex_lock(&md->mutex);
sl = w1_slave_search_device(md, &rn);
if (sl) {
- w1_slave_detach(sl);
+ result = w1_slave_detach(sl);
+ /* refcnt 0 means it was detached in the call */
+ if (result == 0)
+ result = count;
} else {
dev_info(dev, "Device %02x-%012llx doesn't exists\n", rn.family,
(unsigned long long)rn.id);
@@ -704,7 +697,9 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
dev_set_uevent_suppress(&sl->dev, false);
kobject_uevent(&sl->dev.kobj, KOBJ_ADD);

+ mutex_lock(&sl->master->list_mutex);
list_add_tail(&sl->w1_slave_entry, &sl->master->slist);
+ mutex_unlock(&sl->master->list_mutex);

return 0;
}
@@ -731,8 +726,8 @@ int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn)

memset(&msg, 0, sizeof(msg));
memcpy(&sl->reg_num, rn, sizeof(sl->reg_num));
- atomic_set(&sl->refcnt, 0);
- init_completion(&sl->released);
+ atomic_set(&sl->refcnt, 1);
+ atomic_inc(&sl->master->refcnt);

/* slave modules need to be loaded in a context with unlocked mutex */
mutex_unlock(&dev->mutex);
@@ -772,23 +767,48 @@ int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn)
return 0;
}

-void w1_slave_detach(struct w1_slave *sl)
+int w1_unref_slave(struct w1_slave *sl)
{
- struct w1_netlink_msg msg;
-
- dev_dbg(&sl->dev, "%s: detaching %s [%p].\n", __func__, sl->name, sl);
-
- list_del(&sl->w1_slave_entry);
-
- memset(&msg, 0, sizeof(msg));
- memcpy(msg.id.id, &sl->reg_num, sizeof(msg.id));
- msg.type = W1_SLAVE_REMOVE;
- w1_netlink_send(sl->master, &msg);
-
- device_unregister(&sl->dev);
+ struct w1_master *dev = sl->master;
+ int refcnt;
+ mutex_lock(&dev->list_mutex);
+ refcnt = atomic_sub_return(1, &sl->refcnt);
+ if (refcnt == 0) {
+ struct w1_netlink_msg msg;
+
+ dev_dbg(&sl->dev, "%s: detaching %s [%p].\n", __func__,
+ sl->name, sl);
+
+ list_del(&sl->w1_slave_entry);
+
+ memset(&msg, 0, sizeof(msg));
+ memcpy(msg.id.id, &sl->reg_num, sizeof(msg.id));
+ msg.type = W1_SLAVE_REMOVE;
+ w1_netlink_send(sl->master, &msg);
+
+ device_unregister(&sl->dev);
+ #ifdef DEBUG
+ memset(sl, 0, sizeof(*sl));
+ #endif
+ kfree(sl);
+ }
+ atomic_dec(&dev->refcnt);
+ mutex_unlock(&dev->list_mutex);
+ return refcnt;
+}

- wait_for_completion(&sl->released);
- kfree(sl);
+int w1_slave_detach(struct w1_slave *sl)
+{
+ /* Only detach a slave once as it decreases the refcnt each time. */
+ int destroy_now;
+ mutex_lock(&sl->master->list_mutex);
+ destroy_now = !test_bit(W1_SLAVE_DETACH, &sl->flags);
+ set_bit(W1_SLAVE_DETACH, &sl->flags);
+ mutex_unlock(&sl->master->list_mutex);
+
+ if (destroy_now)
+ destroy_now = !w1_unref_slave(sl);
+ return destroy_now ? 0 : -EBUSY;
}

struct w1_master *w1_search_master_id(u32 id)
@@ -817,7 +837,7 @@ struct w1_slave *w1_search_slave(struct w1_reg_num *id)

mutex_lock(&w1_mlock);
list_for_each_entry(dev, &w1_masters, w1_master_entry) {
- mutex_lock(&dev->mutex);
+ mutex_lock(&dev->list_mutex);
list_for_each_entry(sl, &dev->slist, w1_slave_entry) {
if (sl->reg_num.family == id->family &&
sl->reg_num.id == id->id &&
@@ -828,7 +848,7 @@ struct w1_slave *w1_search_slave(struct w1_reg_num *id)
break;
}
}
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->list_mutex);

if (found)
break;
@@ -848,6 +868,7 @@ void w1_reconnect_slaves(struct w1_family *f, int attach)
dev_dbg(&dev->dev, "Reconnecting slaves in device %s "
"for family %02x.\n", dev->name, f->fid);
mutex_lock(&dev->mutex);
+ mutex_lock(&dev->list_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
@@ -859,14 +880,19 @@ void w1_reconnect_slaves(struct w1_family *f, int attach)
(!attach && sl->family->fid == f->fid)) {
struct w1_reg_num rn;

+ mutex_unlock(&dev->list_mutex);
memcpy(&rn, &sl->reg_num, sizeof(rn));
- w1_slave_detach(sl);
-
- w1_attach_slave_device(dev, &rn);
+ /* If it was already in use let the automatic
+ * scan pick it up again later.
+ */
+ if (!w1_slave_detach(sl))
+ w1_attach_slave_device(dev, &rn);
+ mutex_lock(&dev->list_mutex);
}
}
dev_dbg(&dev->dev, "Reconnecting slaves in device %s "
"has been finished.\n", dev->name);
+ mutex_unlock(&dev->list_mutex);
mutex_unlock(&dev->mutex);
}
mutex_unlock(&w1_mlock);
@@ -1020,17 +1046,24 @@ void w1_search_process_cb(struct w1_master *dev, u8 search_type,
{
struct w1_slave *sl, *sln;

+ mutex_lock(&dev->list_mutex);
list_for_each_entry(sl, &dev->slist, w1_slave_entry)
clear_bit(W1_SLAVE_ACTIVE, &sl->flags);
+ mutex_unlock(&dev->list_mutex);

w1_search_devices(dev, search_type, cb);

+ mutex_lock(&dev->list_mutex);
list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) {
- if (!test_bit(W1_SLAVE_ACTIVE, &sl->flags) && !--sl->ttl)
+ if (!test_bit(W1_SLAVE_ACTIVE, &sl->flags) && !--sl->ttl) {
+ mutex_unlock(&dev->list_mutex);
w1_slave_detach(sl);
+ mutex_lock(&dev->list_mutex);
+ }
else if (test_bit(W1_SLAVE_ACTIVE, &sl->flags))
sl->ttl = dev->slave_ttl;
}
+ mutex_unlock(&dev->list_mutex);

if (dev->search_count > 0)
dev->search_count--;
@@ -1041,6 +1074,26 @@ static void w1_search_process(struct w1_master *dev, u8 search_type)
w1_search_process_cb(dev, search_type, w1_slave_found);
}

+int w1_process_callbacks(struct w1_master *dev)
+{
+ int ret = 0;
+ struct w1_async_cmd *async_cmd, *async_n;
+
+ /* The list can be added to in another thread, loop until it is empty */
+ while (!list_empty(&dev->async_list)) {
+ list_for_each_entry_safe(async_cmd, async_n, &dev->async_list,
+ async_entry) {
+ /* drop the lock, if it is a search it can take a long
+ * time */
+ mutex_unlock(&dev->list_mutex);
+ async_cmd->cb(dev, async_cmd);
+ ret = 1;
+ mutex_lock(&dev->list_mutex);
+ }
+ }
+ return ret;
+}
+
int w1_process(void *data)
{
struct w1_master *dev = (struct w1_master *) data;
@@ -1048,23 +1101,46 @@ int w1_process(void *data)
* time can be calculated in jiffies once.
*/
const unsigned long jtime = msecs_to_jiffies(w1_timeout * 1000);
+ /* remainder if it woke up early */
+ unsigned long jremain = 0;

- while (!kthread_should_stop()) {
- if (dev->search_count) {
+ for (;;) {
+
+ if (!jremain && dev->search_count) {
mutex_lock(&dev->mutex);
w1_search_process(dev, W1_SEARCH);
mutex_unlock(&dev->mutex);
}

+ mutex_lock(&dev->list_mutex);
+ /* Note, w1_process_callback drops the lock while processing,
+ * but locks it again before returning.
+ */
+ if (!w1_process_callbacks(dev) && jremain) {
+ /* a wake up is either to stop the thread, process
+ * callbacks, or search, it isn't process callbacks, so
+ * schedule a search.
+ */
+ jremain = 1;
+ }
+
try_to_freeze();
__set_current_state(TASK_INTERRUPTIBLE);

+ /* hold list_mutex until after interruptible to prevent loosing
+ * the wakeup signal when async_cmd is added.
+ */
+ mutex_unlock(&dev->list_mutex);
+
if (kthread_should_stop())
break;

/* Only sleep when the search is active. */
- if (dev->search_count)
- schedule_timeout(jtime);
+ if (dev->search_count) {
+ if (!jremain)
+ jremain = jtime;
+ jremain = schedule_timeout(jremain);
+ }
else
schedule();
}
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 3376bfb..a096ef4 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -58,6 +58,7 @@ struct w1_reg_num
#define W1_RESUME_CMD 0xA5

#define W1_SLAVE_ACTIVE 0
+#define W1_SLAVE_DETACH 1

struct w1_slave
{
@@ -74,7 +75,6 @@ struct w1_slave
struct w1_family *family;
void *family_data;
struct device dev;
- struct completion released;
};

typedef void (*w1_slave_found_callback)(struct w1_master *, u64);
@@ -171,7 +171,14 @@ struct w1_master
struct list_head w1_master_entry;
struct module *owner;
unsigned char name[W1_MAXNAMELEN];
+ /* list_mutex protects just slist and async_list so slaves can be
+ * searched for and async commands added while the master has
+ * w1_master.mutex locked and is operating on the bus.
+ * lock order w1_mlock, w1_master.mutex, w1_master_list_mutex
+ */
+ struct mutex list_mutex;
struct list_head slist;
+ struct list_head async_list;
int max_slave_count, slave_count;
unsigned long attempts;
int slave_ttl;
@@ -205,11 +212,29 @@ struct w1_master
u32 seq;
};

+/**
+ * struct w1_async_cmd - execute callback from the w1_process kthread
+ * @async_entry: link entry
+ * @cb: callback function, must list_del and destroy this list before
+ * returning
+ *
+ * When inserted into the w1_master async_list, w1_process will execute
+ * the callback. Embed this into the structure with the command details.
+ */
+struct w1_async_cmd {
+ struct list_head async_entry;
+ void (*cb)(struct w1_master *dev, struct w1_async_cmd *async_cmd);
+};
+
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);
+/* call w1_unref_slave to release the reference counts w1_search_slave added */
struct w1_slave *w1_search_slave(struct w1_reg_num *id);
+/* decrements the reference on sl->master and sl, and cleans up if zero
+ * returns the reference count after it has been decremented */
+int w1_unref_slave(struct w1_slave *sl);
void w1_slave_found(struct w1_master *dev, u64 rn);
void w1_search_process_cb(struct w1_master *dev, u8 search_type,
w1_slave_found_callback cb);
@@ -224,7 +249,8 @@ struct w1_master *w1_search_master_id(u32 id);
*/
void w1_reconnect_slaves(struct w1_family *f, int attach);
int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn);
-void w1_slave_detach(struct w1_slave *sl);
+/* 0 success, otherwise EBUSY */
+int w1_slave_detach(struct w1_slave *sl);

u8 w1_triplet(struct w1_master *dev, int bdir);
void w1_write_8(struct w1_master *, u8);
@@ -260,6 +286,8 @@ extern int w1_max_slave_ttl;
extern struct list_head w1_masters;
extern struct mutex w1_mlock;

+/* returns 1 if there were commands to executed 0 otherwise */
+extern int w1_process_callbacks(struct w1_master *dev);
extern int w1_process(void *);

#endif /* __KERNEL__ */
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index f84e48b..ae3b595 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -75,8 +75,10 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
atomic_set(&dev->refcnt, 2);

INIT_LIST_HEAD(&dev->slist);
+ INIT_LIST_HEAD(&dev->async_list);
mutex_init(&dev->mutex);
mutex_init(&dev->bus_mutex);
+ mutex_init(&dev->list_mutex);

memcpy(&dev->dev, device, sizeof(struct device));
dev_set_name(&dev->dev, "w1_bus_master%u", dev->id);
@@ -200,17 +202,22 @@ void __w1_remove_master_device(struct w1_master *dev)
struct w1_netlink_msg msg;
struct w1_slave *sl, *sln;

- set_bit(W1_ABORT_SEARCH, &dev->flags);
- kthread_stop(dev->thread);
-
mutex_lock(&w1_mlock);
list_del(&dev->w1_master_entry);
mutex_unlock(&w1_mlock);

+ set_bit(W1_ABORT_SEARCH, &dev->flags);
+ kthread_stop(dev->thread);
+
mutex_lock(&dev->mutex);
- list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry)
+ mutex_lock(&dev->list_mutex);
+ list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) {
+ mutex_unlock(&dev->list_mutex);
w1_slave_detach(sl);
+ mutex_lock(&dev->list_mutex);
+ }
w1_destroy_master_attributes(dev);
+ mutex_unlock(&dev->list_mutex);
mutex_unlock(&dev->mutex);
atomic_dec(&dev->refcnt);

@@ -220,7 +227,9 @@ void __w1_remove_master_device(struct w1_master *dev)

if (msleep_interruptible(1000))
flush_signals(current);
+ w1_process_callbacks(dev);
}
+ w1_process_callbacks(dev);

memset(&msg, 0, sizeof(msg));
msg.id.mst.id = dev->id;
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 747174b..06d614a 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -119,10 +119,12 @@ static int w1_get_slaves(struct w1_master *dev,

if (req_cmd->cmd == W1_CMD_LIST_SLAVES) {
__u64 rn;
+ mutex_lock(&dev->list_mutex);
list_for_each_entry(sl, &dev->slist, w1_slave_entry) {
memcpy(&rn, &sl->reg_num, sizeof(rn));
w1_send_slave(dev, rn);
}
+ mutex_unlock(&dev->list_mutex);
} else {
w1_search_process_cb(dev, cmd->cmd == W1_CMD_ALARM_SEARCH ?
W1_ALARM_SEARCH : W1_SEARCH, w1_found_send_slave);
@@ -368,29 +370,134 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
return error;
}

+/* Bundle together a reference count, the full message, and broken out
+ * commands to be executed on each w1 master kthread in one memory allocation.
+ */
+struct w1_cb_block {
+ atomic_t refcnt;
+ struct cn_msg msg;
+ /* cn_msg data */
+ /* one or more variable length struct w1_cb_node */
+};
+struct w1_cb_node {
+ struct w1_async_cmd async;
+ /* pointers within w1_cb_block and msg data */
+ struct w1_cb_block *block;
+ struct w1_netlink_msg *m;
+ struct w1_slave *sl;
+ struct w1_master *dev;
+};
+
+static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
+{
+ struct w1_cb_node *node = container_of(async_cmd, struct w1_cb_node,
+ async);
+ u16 mlen = node->m->len;
+ u8 *cmd_data = node->m->data;
+ int err = 0;
+ struct w1_slave *sl = node->sl;
+ struct w1_netlink_cmd *cmd = NULL;
+
+ mutex_lock(&dev->mutex);
+ if (sl && w1_reset_select_slave(sl))
+ err = -ENODEV;
+
+ while (mlen && !err) {
+ cmd = (struct w1_netlink_cmd *)cmd_data;
+
+ if (cmd->len + sizeof(struct w1_netlink_cmd) > mlen) {
+ err = -E2BIG;
+ break;
+ }
+
+ if (sl)
+ err = w1_process_command_slave(sl, &node->block->msg,
+ node->m, cmd);
+ else
+ err = w1_process_command_master(dev, &node->block->msg,
+ node->m, cmd);
+
+ w1_netlink_send_error(&node->block->msg, node->m, cmd, err);
+ err = 0;
+
+ cmd_data += cmd->len + sizeof(struct w1_netlink_cmd);
+ mlen -= cmd->len + sizeof(struct w1_netlink_cmd);
+ }
+
+ if (!cmd || err)
+ w1_netlink_send_error(&node->block->msg, node->m, cmd, err);
+
+ if (sl)
+ w1_unref_slave(sl);
+ else
+ atomic_dec(&dev->refcnt);
+ mutex_unlock(&dev->mutex);
+
+ mutex_lock(&dev->list_mutex);
+ list_del(&async_cmd->async_entry);
+ mutex_unlock(&dev->list_mutex);
+
+ if (atomic_sub_return(1, &node->block->refcnt) == 0)
+ kfree(node->block);
+}
+
static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct w1_netlink_msg *m = (struct w1_netlink_msg *)(msg + 1);
- struct w1_netlink_cmd *cmd;
struct w1_slave *sl;
struct w1_master *dev;
+ u16 msg_len;
int err = 0;
+ struct w1_cb_block *block = NULL;
+ struct w1_cb_node *node = NULL;
+ int node_count = 0;
+
+ /* Count the number of master or slave commands there are to allocate
+ * space for one cb_node each.
+ */
+ msg_len = msg->len;
+ while (msg_len && !err) {
+ if (m->len + sizeof(struct w1_netlink_msg) > msg_len) {
+ err = -E2BIG;
+ break;
+ }
+
+ if (m->type == W1_MASTER_CMD || m->type == W1_SLAVE_CMD)
+ ++node_count;
+
+ msg_len -= sizeof(struct w1_netlink_msg) + m->len;
+ m = (struct w1_netlink_msg *)(((u8 *)m) +
+ sizeof(struct w1_netlink_msg) + m->len);
+ }
+ m = (struct w1_netlink_msg *)(msg + 1);
+ if (node_count) {
+ /* msg->len doesn't include itself */
+ long size = sizeof(struct w1_cb_block) + msg->len +
+ node_count*sizeof(struct w1_cb_node);
+ block = kmalloc(size, GFP_KERNEL);
+ if (!block) {
+ w1_netlink_send_error(msg, m, NULL, -ENOMEM);
+ return;
+ }
+ atomic_set(&block->refcnt, 1);
+ memcpy(&block->msg, msg, sizeof(*msg) + msg->len);
+ node = (struct w1_cb_node *)((u8 *)block->msg.data + msg->len);
+ }

- while (msg->len && !err) {
+ msg_len = msg->len;
+ while (msg_len && !err) {
struct w1_reg_num id;
u16 mlen = m->len;
- u8 *cmd_data = m->data;

dev = NULL;
sl = NULL;
- cmd = NULL;

memcpy(&id, m->id.id, sizeof(id));
#if 0
printk("%s: %02x.%012llx.%02x: type=%02x, len=%u.\n",
__func__, id.family, (unsigned long long)id.id, id.crc, m->type, m->len);
#endif
- if (m->len + sizeof(struct w1_netlink_msg) > msg->len) {
+ if (m->len + sizeof(struct w1_netlink_msg) > msg_len) {
err = -E2BIG;
break;
}
@@ -415,41 +522,24 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
if (!mlen)
goto out_cont;

- mutex_lock(&dev->mutex);
+ atomic_inc(&block->refcnt);
+ node->async.cb = w1_process_cb;
+ node->block = block;
+ node->m = (struct w1_netlink_msg *)((u8 *)&block->msg +
+ (size_t)((u8 *)m - (u8 *)msg));
+ node->sl = sl;
+ node->dev = dev;

- if (sl && w1_reset_select_slave(sl)) {
- err = -ENODEV;
- goto out_up;
- }
-
- while (mlen) {
- cmd = (struct w1_netlink_cmd *)cmd_data;
-
- if (cmd->len + sizeof(struct w1_netlink_cmd) > mlen) {
- err = -E2BIG;
- break;
- }
-
- if (sl)
- err = w1_process_command_slave(sl, msg, m, cmd);
- else
- err = w1_process_command_master(dev, msg, m, cmd);
+ mutex_lock(&dev->list_mutex);
+ list_add_tail(&node->async.async_entry, &dev->async_list);
+ wake_up_process(dev->thread);
+ mutex_unlock(&dev->list_mutex);
+ ++node;

- w1_netlink_send_error(msg, m, cmd, err);
- err = 0;
-
- cmd_data += cmd->len + sizeof(struct w1_netlink_cmd);
- mlen -= cmd->len + sizeof(struct w1_netlink_cmd);
- }
-out_up:
- atomic_dec(&dev->refcnt);
- if (sl)
- atomic_dec(&sl->refcnt);
- mutex_unlock(&dev->mutex);
out_cont:
- if (!cmd || err)
- w1_netlink_send_error(msg, m, cmd, err);
- msg->len -= sizeof(struct w1_netlink_msg) + m->len;
+ if (err)
+ w1_netlink_send_error(msg, m, NULL, err);
+ msg_len -= sizeof(struct w1_netlink_msg) + m->len;
m = (struct w1_netlink_msg *)(((u8 *)m) + sizeof(struct w1_netlink_msg) + m->len);

/*
@@ -458,6 +548,8 @@ out_cont:
if (err == -ENODEV)
err = 0;
}
+ if (block && atomic_sub_return(1, &block->refcnt) == 0)
+ kfree(block);
}

int w1_init_netlink(void)
--
1.7.10.4

2014-01-16 04:32:57

by David Fries

[permalink] [raw]
Subject: [PATCH 08/15] connector: add portid to unicast in addition to broadcasting

This allows replying only to the requestor portid while still
supporting broadcasting. Pass 0 to portid for the previous behavior.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
Documentation/connector/cn_test.c | 2 +-
drivers/connector/cn_proc.c | 18 +++++++++---------
drivers/connector/connector.c | 20 +++++++++++++-------
drivers/hv/hv_kvp.c | 4 ++--
drivers/hv/hv_snapshot.c | 2 +-
drivers/md/dm-log-userspace-transfer.c | 2 +-
drivers/video/uvesafb.c | 4 ++--
drivers/w1/w1_netlink.c | 14 +++++++-------
include/linux/connector.h | 2 +-
9 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/Documentation/connector/cn_test.c b/Documentation/connector/cn_test.c
index adcca03..d12cc94 100644
--- a/Documentation/connector/cn_test.c
+++ b/Documentation/connector/cn_test.c
@@ -145,7 +145,7 @@ static void cn_test_timer_func(unsigned long __data)

memcpy(m + 1, data, m->len);

- cn_netlink_send(m, 0, GFP_ATOMIC);
+ cn_netlink_send(m, 0, 0, GFP_ATOMIC);
kfree(m);
}

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 18c5b9b..148d707 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -95,7 +95,7 @@ void proc_fork_connector(struct task_struct *task)
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
/* If cn_netlink_send() failed, the data is not sent */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_exec_connector(struct task_struct *task)
@@ -122,7 +122,7 @@ void proc_exec_connector(struct task_struct *task)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_id_connector(struct task_struct *task, int which_id)
@@ -163,7 +163,7 @@ void proc_id_connector(struct task_struct *task, int which_id)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_sid_connector(struct task_struct *task)
@@ -190,7 +190,7 @@ void proc_sid_connector(struct task_struct *task)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_ptrace_connector(struct task_struct *task, int ptrace_id)
@@ -225,7 +225,7 @@ void proc_ptrace_connector(struct task_struct *task, int ptrace_id)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_comm_connector(struct task_struct *task)
@@ -253,7 +253,7 @@ void proc_comm_connector(struct task_struct *task)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_coredump_connector(struct task_struct *task)
@@ -280,7 +280,7 @@ void proc_coredump_connector(struct task_struct *task)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

void proc_exit_connector(struct task_struct *task)
@@ -309,7 +309,7 @@ void proc_exit_connector(struct task_struct *task)
msg->ack = 0; /* not used */
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

/*
@@ -343,7 +343,7 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack)
msg->ack = rcvd_ack + 1;
msg->len = sizeof(*ev);
msg->flags = 0; /* not used */
- cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+ cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_KERNEL);
}

/**
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index a36749f..77afe74 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -50,7 +50,7 @@ static int cn_already_initialized;
*
* Sequence number is incremented with each message to be sent.
*
- * If we expect reply to our message then the sequence number in
+ * If we expect a reply to our message then the sequence number in
* received message MUST be the same as in original message, and
* acknowledge number MUST be the same + 1.
*
@@ -62,8 +62,11 @@ static int cn_already_initialized;
* the acknowledgement number in the original message + 1, then it is
* a new message.
*
+ * The message is sent to, the portid if given, the group if given, both if
+ * both, or if both are zero then the group is looked up and sent there.
*/
-int cn_netlink_send(struct cn_msg *msg, u32 __group, gfp_t gfp_mask)
+int cn_netlink_send(struct cn_msg *msg, u32 portid, u32 __group,
+ gfp_t gfp_mask)
{
struct cn_callback_entry *__cbq;
unsigned int size;
@@ -74,7 +77,9 @@ int cn_netlink_send(struct cn_msg *msg, u32 __group, gfp_t gfp_mask)
u32 group = 0;
int found = 0;

- if (!__group) {
+ if (portid || __group) {
+ group = __group;
+ } else {
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(__cbq, &dev->cbdev->queue_list,
callback_entry) {
@@ -88,11 +93,9 @@ int cn_netlink_send(struct cn_msg *msg, u32 __group, gfp_t gfp_mask)

if (!found)
return -ENODEV;
- } else {
- group = __group;
}

- if (!netlink_has_listeners(dev->nls, group))
+ if (!portid && !netlink_has_listeners(dev->nls, group))
return -ESRCH;

size = sizeof(*msg) + msg->len;
@@ -113,7 +116,10 @@ int cn_netlink_send(struct cn_msg *msg, u32 __group, gfp_t gfp_mask)

NETLINK_CB(skb).dst_group = group;

- return netlink_broadcast(dev->nls, skb, 0, group, gfp_mask);
+ if (group)
+ return netlink_broadcast(dev->nls, skb, portid, group,
+ gfp_mask);
+ return netlink_unicast(dev->nls, skb, portid, !(gfp_mask&__GFP_WAIT));
}
EXPORT_SYMBOL_GPL(cn_netlink_send);

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 09988b2..ea85253 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -113,7 +113,7 @@ kvp_register(int reg_value)
kvp_msg->kvp_hdr.operation = reg_value;
strcpy(version, HV_DRV_VERSION);
msg->len = sizeof(struct hv_kvp_msg);
- cn_netlink_send(msg, 0, GFP_ATOMIC);
+ cn_netlink_send(msg, 0, 0, GFP_ATOMIC);
kfree(msg);
}
}
@@ -435,7 +435,7 @@ kvp_send_key(struct work_struct *dummy)
}

msg->len = sizeof(struct hv_kvp_msg);
- cn_netlink_send(msg, 0, GFP_ATOMIC);
+ cn_netlink_send(msg, 0, 0, GFP_ATOMIC);
kfree(msg);

return;
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 0c35462..34f14fd 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -98,7 +98,7 @@ static void vss_send_op(struct work_struct *dummy)
vss_msg->vss_hdr.operation = op;
msg->len = sizeof(struct hv_vss_msg);

- cn_netlink_send(msg, 0, GFP_ATOMIC);
+ cn_netlink_send(msg, 0, 0, GFP_ATOMIC);
kfree(msg);

return;
diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index 08d9a20..b428c0a 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -66,7 +66,7 @@ static int dm_ulog_sendto_server(struct dm_ulog_request *tfr)
msg->seq = tfr->seq;
msg->len = sizeof(struct dm_ulog_request) + tfr->data_size;

- r = cn_netlink_send(msg, 0, gfp_any());
+ r = cn_netlink_send(msg, 0, 0, gfp_any());

return r;
}
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 256fba7..1f38445 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -190,7 +190,7 @@ static int uvesafb_exec(struct uvesafb_ktask *task)
uvfb_tasks[seq] = task;
mutex_unlock(&uvfb_lock);

- err = cn_netlink_send(m, 0, GFP_KERNEL);
+ err = cn_netlink_send(m, 0, 0, GFP_KERNEL);
if (err == -ESRCH) {
/*
* Try to start the userspace helper if sending
@@ -204,7 +204,7 @@ static int uvesafb_exec(struct uvesafb_ktask *task)
"helper is installed and executable\n");
} else {
v86d_started = 1;
- err = cn_netlink_send(m, 0, gfp_any());
+ err = cn_netlink_send(m, 0, 0, gfp_any());
if (err == -ENOBUFS)
err = 0;
}
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 06d614a..b63109a 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -45,7 +45,7 @@ void w1_netlink_send(struct w1_master *dev, struct w1_netlink_msg *msg)

memcpy(w, msg, sizeof(struct w1_netlink_msg));

- cn_netlink_send(m, 0, GFP_KERNEL);
+ cn_netlink_send(m, 0, 0, GFP_KERNEL);
}

static void w1_send_slave(struct w1_master *dev, u64 rn)
@@ -60,7 +60,7 @@ static void w1_send_slave(struct w1_master *dev, u64 rn)

if (avail < 8) {
msg->ack++;
- cn_netlink_send(msg, 0, GFP_KERNEL);
+ cn_netlink_send(msg, 0, 0, GFP_KERNEL);

msg->len = sizeof(struct w1_netlink_msg) +
sizeof(struct w1_netlink_cmd);
@@ -131,7 +131,7 @@ static int w1_get_slaves(struct w1_master *dev,
}

msg->ack = 0;
- cn_netlink_send(msg, 0, GFP_KERNEL);
+ cn_netlink_send(msg, 0, 0, GFP_KERNEL);

dev->priv = NULL;
dev->priv_size = 0;
@@ -173,7 +173,7 @@ static int w1_send_read_reply(struct cn_msg *msg, struct w1_netlink_msg *hdr,

memcpy(c->data, cmd->data, c->len);

- err = cn_netlink_send(cm, 0, GFP_KERNEL);
+ err = cn_netlink_send(cm, 0, 0, GFP_KERNEL);

kfree(data);

@@ -316,7 +316,7 @@ static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mc
mutex_lock(&w1_mlock);
list_for_each_entry(m, &w1_masters, w1_master_entry) {
if (cn->len + sizeof(*id) > PAGE_SIZE - sizeof(struct cn_msg)) {
- cn_netlink_send(cn, 0, GFP_KERNEL);
+ cn_netlink_send(cn, 0, 0, GFP_KERNEL);
cn->ack++;
cn->len = sizeof(struct w1_netlink_msg);
w->len = 0;
@@ -329,7 +329,7 @@ static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mc
id++;
}
cn->ack = 0;
- cn_netlink_send(cn, 0, GFP_KERNEL);
+ cn_netlink_send(cn, 0, 0, GFP_KERNEL);
mutex_unlock(&w1_mlock);

kfree(cn);
@@ -364,7 +364,7 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
cmsg->len += sizeof(*cmd);
}

- error = cn_netlink_send(cmsg, 0, GFP_KERNEL);
+ error = cn_netlink_send(cmsg, 0, 0, GFP_KERNEL);
kfree(cmsg);

return error;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index b2b5a41..be9c4747 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -71,7 +71,7 @@ struct cn_dev {
int cn_add_callback(struct cb_id *id, const char *name,
void (*callback)(struct cn_msg *, struct netlink_skb_parms *));
void cn_del_callback(struct cb_id *);
-int cn_netlink_send(struct cn_msg *, u32, gfp_t);
+int cn_netlink_send(struct cn_msg *msg, u32 portid, u32 group, gfp_t gfp_mask);

int cn_queue_add_callback(struct cn_queue_dev *dev, const char *name,
struct cb_id *id,
--
1.7.10.4

2014-01-16 04:33:14

by David Fries

[permalink] [raw]
Subject: [PATCH 09/15] w1: reply only to the requester portid

Unicast one wire replies back to the sender portid to avoid multiple
programs getting each other's messages, especially as the response
can't be uniquely identified with the sequence coming from the
requesting program when both programs generate the same id. Continue
to broadcast events such as add/remove master/slave devices.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/w1.h | 5 +++++
drivers/w1/w1_netlink.c | 42 +++++++++++++++++++++++++-----------------
2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index a096ef4..390a730 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -210,6 +210,11 @@ struct w1_master
struct w1_bus_master *bus_master;

u32 seq;
+ /* port id to send netlink responses to. The value is temporarily
+ * stored here while processing a message, set after locking the
+ * mutex, zero before unlocking the mutex.
+ */
+ u32 portid;
};

/**
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index b63109a..a5dc219 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -45,7 +45,7 @@ void w1_netlink_send(struct w1_master *dev, struct w1_netlink_msg *msg)

memcpy(w, msg, sizeof(struct w1_netlink_msg));

- cn_netlink_send(m, 0, 0, GFP_KERNEL);
+ cn_netlink_send(m, dev->portid, 0, GFP_KERNEL);
}

static void w1_send_slave(struct w1_master *dev, u64 rn)
@@ -60,7 +60,7 @@ static void w1_send_slave(struct w1_master *dev, u64 rn)

if (avail < 8) {
msg->ack++;
- cn_netlink_send(msg, 0, 0, GFP_KERNEL);
+ cn_netlink_send(msg, dev->portid, 0, GFP_KERNEL);

msg->len = sizeof(struct w1_netlink_msg) +
sizeof(struct w1_netlink_cmd);
@@ -131,7 +131,7 @@ static int w1_get_slaves(struct w1_master *dev,
}

msg->ack = 0;
- cn_netlink_send(msg, 0, 0, GFP_KERNEL);
+ cn_netlink_send(msg, dev->portid, 0, GFP_KERNEL);

dev->priv = NULL;
dev->priv_size = 0;
@@ -142,7 +142,7 @@ static int w1_get_slaves(struct w1_master *dev,
}

static int w1_send_read_reply(struct cn_msg *msg, struct w1_netlink_msg *hdr,
- struct w1_netlink_cmd *cmd)
+ struct w1_netlink_cmd *cmd, u32 portid)
{
void *data;
struct w1_netlink_msg *h;
@@ -173,7 +173,7 @@ static int w1_send_read_reply(struct cn_msg *msg, struct w1_netlink_msg *hdr,

memcpy(c->data, cmd->data, c->len);

- err = cn_netlink_send(cm, 0, 0, GFP_KERNEL);
+ err = cn_netlink_send(cm, portid, 0, GFP_KERNEL);

kfree(data);

@@ -188,11 +188,11 @@ static int w1_process_command_io(struct w1_master *dev, struct cn_msg *msg,
switch (cmd->cmd) {
case W1_CMD_TOUCH:
w1_touch_block(dev, cmd->data, cmd->len);
- w1_send_read_reply(msg, hdr, cmd);
+ w1_send_read_reply(msg, hdr, cmd, dev->portid);
break;
case W1_CMD_READ:
w1_read_block(dev, cmd->data, cmd->len);
- w1_send_read_reply(msg, hdr, cmd);
+ w1_send_read_reply(msg, hdr, cmd, dev->portid);
break;
case W1_CMD_WRITE:
w1_write_block(dev, cmd->data, cmd->len);
@@ -283,7 +283,8 @@ static int w1_process_command_slave(struct w1_slave *sl, struct cn_msg *msg,
return w1_process_command_io(sl->master, msg, hdr, cmd);
}

-static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mcmd)
+static int w1_process_command_root(struct cn_msg *msg,
+ struct w1_netlink_msg *mcmd, u32 portid)
{
struct w1_master *m;
struct cn_msg *cn;
@@ -316,7 +317,7 @@ static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mc
mutex_lock(&w1_mlock);
list_for_each_entry(m, &w1_masters, w1_master_entry) {
if (cn->len + sizeof(*id) > PAGE_SIZE - sizeof(struct cn_msg)) {
- cn_netlink_send(cn, 0, 0, GFP_KERNEL);
+ cn_netlink_send(cn, portid, 0, GFP_KERNEL);
cn->ack++;
cn->len = sizeof(struct w1_netlink_msg);
w->len = 0;
@@ -329,7 +330,7 @@ static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mc
id++;
}
cn->ack = 0;
- cn_netlink_send(cn, 0, 0, GFP_KERNEL);
+ cn_netlink_send(cn, portid, 0, GFP_KERNEL);
mutex_unlock(&w1_mlock);

kfree(cn);
@@ -337,7 +338,7 @@ static int w1_process_command_root(struct cn_msg *msg, struct w1_netlink_msg *mc
}

static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rmsg,
- struct w1_netlink_cmd *rcmd, int error)
+ struct w1_netlink_cmd *rcmd, int portid, int error)
{
struct cn_msg *cmsg;
struct w1_netlink_msg *msg;
@@ -364,7 +365,7 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
cmsg->len += sizeof(*cmd);
}

- error = cn_netlink_send(cmsg, 0, 0, GFP_KERNEL);
+ error = cn_netlink_send(cmsg, portid, 0, GFP_KERNEL);
kfree(cmsg);

return error;
@@ -375,6 +376,7 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
*/
struct w1_cb_block {
atomic_t refcnt;
+ u32 portid; /* Sending process port ID */
struct cn_msg msg;
/* cn_msg data */
/* one or more variable length struct w1_cb_node */
@@ -399,6 +401,7 @@ static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
struct w1_netlink_cmd *cmd = NULL;

mutex_lock(&dev->mutex);
+ dev->portid = node->block->portid;
if (sl && w1_reset_select_slave(sl))
err = -ENODEV;

@@ -417,7 +420,8 @@ static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
err = w1_process_command_master(dev, &node->block->msg,
node->m, cmd);

- w1_netlink_send_error(&node->block->msg, node->m, cmd, err);
+ w1_netlink_send_error(&node->block->msg, node->m, cmd,
+ node->block->portid, err);
err = 0;

cmd_data += cmd->len + sizeof(struct w1_netlink_cmd);
@@ -425,12 +429,14 @@ static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
}

if (!cmd || err)
- w1_netlink_send_error(&node->block->msg, node->m, cmd, err);
+ w1_netlink_send_error(&node->block->msg, node->m, cmd,
+ node->block->portid, err);

if (sl)
w1_unref_slave(sl);
else
atomic_dec(&dev->refcnt);
+ dev->portid = 0;
mutex_unlock(&dev->mutex);

mutex_lock(&dev->list_mutex);
@@ -476,10 +482,12 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
node_count*sizeof(struct w1_cb_node);
block = kmalloc(size, GFP_KERNEL);
if (!block) {
- w1_netlink_send_error(msg, m, NULL, -ENOMEM);
+ w1_netlink_send_error(msg, m, NULL, nsp->portid,
+ -ENOMEM);
return;
}
atomic_set(&block->refcnt, 1);
+ block->portid = nsp->portid;
memcpy(&block->msg, msg, sizeof(*msg) + msg->len);
node = (struct w1_cb_node *)((u8 *)block->msg.data + msg->len);
}
@@ -509,7 +517,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
if (sl)
dev = sl->master;
} else {
- err = w1_process_command_root(msg, m);
+ err = w1_process_command_root(msg, m, nsp->portid);
goto out_cont;
}

@@ -538,7 +546,7 @@ static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)

out_cont:
if (err)
- w1_netlink_send_error(msg, m, NULL, err);
+ w1_netlink_send_error(msg, m, NULL, nsp->portid, err);
msg_len -= sizeof(struct w1_netlink_msg) + m->len;
m = (struct w1_netlink_msg *)(((u8 *)m) + sizeof(struct w1_netlink_msg) + m->len);

--
1.7.10.4

2014-01-16 04:33:28

by David Fries

[permalink] [raw]
Subject: [PATCH 10/15] w1: ds2490 reduce magic numbers

Use a #define for the usb vendor request type, clear the status
byte and use that instead of a magic offset in checking if idle.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 4f7e1d7..6a3d0a1 100644
--- a/drivers/w1/masters/ds2490.c
+++ b/drivers/w1/masters/ds2490.c
@@ -1,5 +1,5 @@
/*
- * dscore.c
+ * ds2490.c USB to one wire bridge
*
* Copyright (c) 2004 Evgeniy Polyakov <[email protected]>
*
@@ -28,6 +28,10 @@
#include "../w1_int.h"
#include "../w1.h"

+/* USB Standard */
+/* USB Control request vendor type */
+#define VENDOR 0x40
+
/* COMMAND TYPE CODES */
#define CONTROL_CMD 0x00
#define COMM_CMD 0x01
@@ -107,6 +111,8 @@
#define ST_HALT 0x10 /* DS2490 is currently halted */
#define ST_IDLE 0x20 /* DS2490 is currently idle */
#define ST_EPOF 0x80
+/* Status transfer size, 16 bytes status, 16 byte result flags */
+#define ST_SIZE 0x20

/* Result Register flags */
#define RR_DETECT 0xA5 /* New device detected */
@@ -198,7 +204,7 @@ static int ds_send_control_cmd(struct ds_device *dev, u16 value, u16 index)
int err;

err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, dev->ep[EP_CONTROL]),
- CONTROL_CMD, 0x40, value, index, NULL, 0, 1000);
+ CONTROL_CMD, VENDOR, value, index, NULL, 0, 1000);
if (err < 0) {
printk(KERN_ERR "Failed to send command control message %x.%x: err=%d.\n",
value, index, err);
@@ -213,7 +219,7 @@ static int ds_send_control_mode(struct ds_device *dev, u16 value, u16 index)
int err;

err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, dev->ep[EP_CONTROL]),
- MODE_CMD, 0x40, value, index, NULL, 0, 1000);
+ MODE_CMD, VENDOR, value, index, NULL, 0, 1000);
if (err < 0) {
printk(KERN_ERR "Failed to send mode control message %x.%x: err=%d.\n",
value, index, err);
@@ -228,7 +234,7 @@ static int ds_send_control(struct ds_device *dev, u16 value, u16 index)
int err;

err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, dev->ep[EP_CONTROL]),
- COMM_CMD, 0x40, value, index, NULL, 0, 1000);
+ COMM_CMD, VENDOR, value, index, NULL, 0, 1000);
if (err < 0) {
printk(KERN_ERR "Failed to send control message %x.%x: err=%d.\n",
value, index, err);
@@ -353,7 +359,7 @@ 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];
+ u8 buf[ST_SIZE];
int count;

printk(KERN_INFO "Clearing ep0x%x.\n", dev->ep[EP_DATA_IN]);
@@ -398,7 +404,7 @@ int ds_stop_pulse(struct ds_device *dev, int limit)
{
struct ds_status st;
int count = 0, err = 0;
- u8 buf[0x20];
+ u8 buf[ST_SIZE];

do {
err = ds_send_control(dev, CTL_HALT_EXE_IDLE, 0);
@@ -450,10 +456,11 @@ int ds_detect(struct ds_device *dev, struct ds_status *st)

static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
{
- u8 buf[0x20];
+ u8 buf[ST_SIZE];
int err, count = 0;

do {
+ st->status = 0;
err = ds_recv_status_nodump(dev, st, buf, sizeof(buf));
#if 0
if (err >= 0) {
@@ -464,7 +471,7 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
printk("\n");
}
#endif
- } while (!(buf[0x08] & ST_IDLE) && !(err < 0) && ++count < 100);
+ } while (!(st->status & ST_IDLE) && !(err < 0) && ++count < 100);

if (err >= 16 && st->status & ST_EPOF) {
printk(KERN_INFO "Resetting device after ST_EPOF.\n");
--
1.7.10.4

2014-01-16 04:33:40

by David Fries

[permalink] [raw]
Subject: [PATCH 11/15] w1: ds2490 USB setup fixes

Calling usb_reset_configuration after usb_set_interface resets the
interface that was just selected, so call reset first.
Using alternative 3 greatly speeds the one wire search.
alt 0 or 1, 10ms int, 23.16 seconds
alt 2 or 3, 1ms int, 2.99 to 3.05 seconds

Use usb_interrupt_msg not usb_bulk_msg as it is an interrupt pipe
(bulk worked, it was just technically the wrong call).

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds2490.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 6a3d0a1..cd59e12 100644
--- a/drivers/w1/masters/ds2490.c
+++ b/drivers/w1/masters/ds2490.c
@@ -252,7 +252,8 @@ static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st,
memset(st, 0, sizeof(*st));

count = 0;
- err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_STATUS]), buf, size, &count, 100);
+ err = usb_interrupt_msg(dev->udev, usb_rcvintpipe(dev->udev,
+ dev->ep[EP_STATUS]), buf, size, &count, 100);
if (err < 0) {
printk(KERN_ERR "Failed to read 1-wire data from 0x%x: err=%d.\n", dev->ep[EP_STATUS], err);
return err;
@@ -917,7 +918,7 @@ static int ds_probe(struct usb_interface *intf,
struct usb_endpoint_descriptor *endpoint;
struct usb_host_interface *iface_desc;
struct ds_device *dev;
- int i, err;
+ int i, err, alt;

dev = kmalloc(sizeof(struct ds_device), GFP_KERNEL);
if (!dev) {
@@ -935,20 +936,25 @@ static int ds_probe(struct usb_interface *intf,

usb_set_intfdata(intf, dev);

- err = usb_set_interface(dev->udev, intf->altsetting[0].desc.bInterfaceNumber, 3);
+ err = usb_reset_configuration(dev->udev);
if (err) {
- printk(KERN_ERR "Failed to set alternative setting 3 for %d interface: err=%d.\n",
- intf->altsetting[0].desc.bInterfaceNumber, err);
+ dev_err(&dev->udev->dev,
+ "Failed to reset configuration: err=%d.\n", err);
goto err_out_clear;
}

- err = usb_reset_configuration(dev->udev);
+ /* alternative 3, 1ms interrupt (greatly speeds search), 64 byte bulk */
+ alt = 3;
+ err = usb_set_interface(dev->udev,
+ intf->altsetting[alt].desc.bInterfaceNumber, alt);
if (err) {
- printk(KERN_ERR "Failed to reset configuration: err=%d.\n", err);
+ dev_err(&dev->udev->dev, "Failed to set alternative setting %d "
+ "for %d interface: err=%d.\n", alt,
+ intf->altsetting[alt].desc.bInterfaceNumber, err);
goto err_out_clear;
}

- iface_desc = &intf->altsetting[0];
+ iface_desc = &intf->altsetting[alt];
if (iface_desc->desc.bNumEndpoints != NUM_EP-1) {
printk(KERN_INFO "Num endpoints=%d. It is not DS9490R.\n", iface_desc->desc.bNumEndpoints);
err = -EINVAL;
--
1.7.10.4

2014-01-16 04:33:54

by David Fries

[permalink] [raw]
Subject: [PATCH 12/15] w1: ds2490 fix and enable hardware search

The hardware search was failing without the COMM_RST flag. Enabled
the flag and rewrote the function to handle more than one buffer of
results and to continuing where the search left off. Remove hardware
search note from the limitations now that it works. The "w1: ds2490
USB setup fixes" change went from 23.16 seconds to about 3 seconds,
this takes the time for the search down to .307346 seconds.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
Documentation/w1/masters/ds2490 | 2 -
drivers/w1/masters/ds2490.c | 108 +++++++++++++++++++++++++++++++--------
2 files changed, 86 insertions(+), 24 deletions(-)

diff --git a/Documentation/w1/masters/ds2490 b/Documentation/w1/masters/ds2490
index 28176de..3e09115 100644
--- a/Documentation/w1/masters/ds2490
+++ b/Documentation/w1/masters/ds2490
@@ -21,8 +21,6 @@ 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
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index cd59e12..db0bf32 100644
--- a/drivers/w1/masters/ds2490.c
+++ b/drivers/w1/masters/ds2490.c
@@ -698,37 +698,102 @@ static int ds_write_block(struct ds_device *dev, u8 *buf, int len)
return !(err == len);
}

-#if 0
-
-static int ds_search(struct ds_device *dev, u64 init, u64 *buf, u8 id_number, int conditional_search)
+static void ds9490r_search(void *data, struct w1_master *master,
+ u8 search_type, w1_slave_found_callback callback)
{
+ /* When starting with an existing id, the first id returned will
+ * be that device (if it is still on the bus most likely).
+ *
+ * If the number of devices found is less than or equal to the
+ * search_limit, that number of IDs will be returned. If there are
+ * more, search_limit IDs will be returned followed by a non-zero
+ * discrepency value.
+ */
+ struct ds_device *dev = data;
int err;
u16 value, index;
struct ds_status st;
+ u8 st_buf[ST_SIZE];
+ int search_limit;
+ int found = 0;
+ int i;

- memset(buf, 0, sizeof(buf));
-
- err = ds_send_data(ds_dev, (unsigned char *)&init, 8);
- if (err)
- return err;
+ /* DS18b20 spec, 13.16 ms per device, 75 per second, sleep for
+ * discovering 8 devices (1 bulk transfer and 1/2 FIFO size) at a time.
+ */
+ const unsigned long jtime = msecs_to_jiffies(1000*8/75);
+ /* FIFO 128 bytes, bulk packet size 64, read a multiple of the
+ * packet size.
+ */
+ u64 buf[2*64/8];

- ds_wait_status(ds_dev, &st);
+ /* address to start searching at */
+ if (ds_send_data(dev, (u8 *)&master->search_id, 8) < 0)
+ return;
+ master->search_id = 0;
+
+ value = COMM_SEARCH_ACCESS | COMM_IM | COMM_RST | COMM_SM | COMM_F |
+ COMM_RTS;
+ search_limit = master->max_slave_count;
+ if (search_limit > 255)
+ search_limit = 0;
+ index = search_type | (search_limit << 8);
+ if (ds_send_control(dev, value, index) < 0)
+ return;

- value = COMM_SEARCH_ACCESS | COMM_IM | COMM_SM | COMM_F | COMM_RTS;
- index = (conditional_search ? 0xEC : 0xF0) | (id_number << 8);
- err = ds_send_control(ds_dev, value, index);
- if (err)
- return err;
+ do {
+ schedule_timeout(jtime);

- ds_wait_status(ds_dev, &st);
+ if (ds_recv_status_nodump(dev, &st, st_buf, sizeof(st_buf)) <
+ sizeof(st)) {
+ break;
+ }

- err = ds_recv_data(ds_dev, (unsigned char *)buf, 8*id_number);
- if (err < 0)
- return err;
+ if (st.data_in_buffer_status) {
+ /* Bulk in can receive partial ids, but when it does
+ * they fail crc and will be discarded anyway.
+ * That has only been seen when status in buffer
+ * is 0 and bulk is read anyway, so don't read
+ * bulk without first checking if status says there
+ * is data to read.
+ */
+ err = ds_recv_data(dev, (u8 *)buf, sizeof(buf));
+ if (err < 0)
+ break;
+ for (i = 0; i < err/8; ++i) {
+ ++found;
+ if (found <= search_limit)
+ callback(master, buf[i]);
+ /* can't know if there will be a discrepancy
+ * value after until the next id */
+ if (found == search_limit)
+ master->search_id = buf[i];
+ }
+ }

- return err/8;
+ if (test_bit(W1_ABORT_SEARCH, &master->flags))
+ break;
+ } while (!(st.status & (ST_IDLE | ST_HALT)));
+
+ /* only continue the search if some weren't found */
+ if (found <= search_limit) {
+ master->search_id = 0;
+ } else if (!test_bit(W1_WARN_MAX_COUNT, &master->flags)) {
+ /* Only max_slave_count will be scanned in a search,
+ * but it will start where it left off next search
+ * until all ids are identified and then it will start
+ * over. A continued search will report the previous
+ * last id as the first id (provided it is still on the
+ * bus).
+ */
+ dev_info(&dev->udev->dev, "%s: max_slave_count %d reached, "
+ "will continue next search.\n", __func__,
+ master->max_slave_count);
+ set_bit(W1_WARN_MAX_COUNT, &master->flags);
+ }
}

+#if 0
static int ds_match_access(struct ds_device *dev, u64 init)
{
int err;
@@ -902,6 +967,7 @@ static int ds_w1_init(struct ds_device *dev)
dev->master.write_block = &ds9490r_write_block;
dev->master.reset_bus = &ds9490r_reset;
dev->master.set_pullup = &ds9490r_set_pullup;
+ dev->master.search = &ds9490r_search;

return w1_add_master_device(&dev->master);
}
@@ -920,13 +986,11 @@ static int ds_probe(struct usb_interface *intf,
struct ds_device *dev;
int i, err, alt;

- dev = kmalloc(sizeof(struct ds_device), GFP_KERNEL);
+ dev = kzalloc(sizeof(struct ds_device), GFP_KERNEL);
if (!dev) {
printk(KERN_INFO "Failed to allocate new DS9490R structure.\n");
return -ENOMEM;
}
- dev->spu_sleep = 0;
- dev->spu_bit = 0;
dev->udev = usb_get_dev(udev);
if (!dev->udev) {
err = -ENOMEM;
--
1.7.10.4

2014-01-16 04:34:04

by David Fries

[permalink] [raw]
Subject: [PATCH 13/15] w1: use family_data instead of rom in w1_slave

The first line printed from w1_slave gives the context of the w1
device. So does the second line, but if the CRC check failed, the
second line contains the last successful result. It is confusing when
it prints the temperature next to the line that might be a previous
conversion and has nothing to do with that printed temperature value.
Modify the code to store the last good conversion in family_data,
which is designed for custom data structures.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/slaves/w1_therm.c | 21 +++++++++++++++++++--
drivers/w1/w1.h | 1 -
2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 8b5ff33..1f11a20 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -27,6 +27,7 @@
#include <linux/sched.h>
#include <linux/device.h>
#include <linux/types.h>
+#include <linux/slab.h>
#include <linux/delay.h>

#include "../w1.h"
@@ -58,6 +59,19 @@ MODULE_ALIAS("w1-family-" __stringify(W1_THERM_DS28EA00));
static int w1_strong_pullup = 1;
module_param_named(strong_pullup, w1_strong_pullup, int, 0);

+static int w1_therm_add_slave(struct w1_slave *sl)
+{
+ sl->family_data = kzalloc(9, GFP_KERNEL);
+ if (!sl->family_data)
+ return -ENOMEM;
+ return 0;
+}
+
+static void w1_therm_remove_slave(struct w1_slave *sl)
+{
+ kfree(sl->family_data);
+ sl->family_data = NULL;
+}

static ssize_t w1_slave_show(struct device *device,
struct device_attribute *attr, char *buf);
@@ -71,6 +85,8 @@ static struct attribute *w1_therm_attrs[] = {
ATTRIBUTE_GROUPS(w1_therm);

static struct w1_family_ops w1_therm_fops = {
+ .add_slave = w1_therm_add_slave,
+ .remove_slave = w1_therm_remove_slave,
.groups = w1_therm_groups,
};

@@ -253,12 +269,13 @@ static ssize_t w1_slave_show(struct device *device,
c -= snprintf(buf + PAGE_SIZE - c, c, ": crc=%02x %s\n",
crc, (verdict) ? "YES" : "NO");
if (verdict)
- memcpy(sl->rom, rom, sizeof(sl->rom));
+ memcpy(sl->family_data, rom, sizeof(rom));
else
dev_warn(device, "Read failed CRC check\n");

for (i = 0; i < 9; ++i)
- c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", sl->rom[i]);
+ c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ",
+ ((u8 *)sl->family_data)[i]);

c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n",
w1_convert_temp(rom, sl->family->fid));
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 390a730..0eb5050 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -67,7 +67,6 @@ struct w1_slave
struct list_head w1_slave_entry;
struct w1_reg_num reg_num;
atomic_t refcnt;
- u8 rom[9];
int ttl;
unsigned long flags;

--
1.7.10.4

2014-01-16 04:34:18

by David Fries

[permalink] [raw]
Subject: [PATCH 14/15] w1: format for DocBook and fixes

Switch the code documentation format style to DocBook format, enable
DocBook documentation generation, and fix some comments.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
Documentation/DocBook/Makefile | 2 +-
Documentation/DocBook/w1.tmpl | 101 +++++++++++++++++++++++++++++
Documentation/w1/w1.netlink | 8 +--
drivers/w1/w1.c | 30 +++++++--
drivers/w1/w1.h | 136 +++++++++++++++++++++++++++-------------
drivers/w1/w1_family.c | 8 +++
drivers/w1/w1_family.h | 13 ++++
drivers/w1/w1_int.c | 8 +++
drivers/w1/w1_io.c | 102 +++++++++++++++++-------------
drivers/w1/w1_netlink.h | 6 +-
10 files changed, 315 insertions(+), 99 deletions(-)
create mode 100644 Documentation/DocBook/w1.tmpl

diff --git a/Documentation/DocBook/Makefile b/Documentation/DocBook/Makefile
index bc3d9f8..662e4ef 100644
--- a/Documentation/DocBook/Makefile
+++ b/Documentation/DocBook/Makefile
@@ -14,7 +14,7 @@ DOCBOOKS := z8530book.xml device-drivers.xml \
genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml \
80211.xml debugobjects.xml sh.xml regulator.xml \
alsa-driver-api.xml writing-an-alsa-driver.xml \
- tracepoint.xml drm.xml media_api.xml
+ tracepoint.xml drm.xml media_api.xml w1.xml

include $(srctree)/Documentation/DocBook/media/Makefile

diff --git a/Documentation/DocBook/w1.tmpl b/Documentation/DocBook/w1.tmpl
new file mode 100644
index 0000000..b0228d4
--- /dev/null
+++ b/Documentation/DocBook/w1.tmpl
@@ -0,0 +1,101 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
+ "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
+
+<book id="w1id">
+ <bookinfo>
+ <title>W1: Dallas' 1-wire bus</title>
+
+ <authorgroup>
+ <author>
+ <firstname>David</firstname>
+ <surname>Fries</surname>
+ <affiliation>
+ <address>
+ <email>[email protected]</email>
+ </address>
+ </affiliation>
+ </author>
+
+ </authorgroup>
+
+ <copyright>
+ <year>2013</year>
+ <!--
+ <holder></holder>
+ -->
+ </copyright>
+
+ <legalnotice>
+ <para>
+ This documentation is free software; you can redistribute
+ it and/or modify it under the terms of the GNU General Public
+ License version 2.
+ </para>
+
+ <para>
+ This program is distributed in the hope that it will be
+ useful, but WITHOUT ANY WARRANTY; without even the implied
+ warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ For more details see the file COPYING in the source
+ distribution of Linux.
+ </para>
+ </legalnotice>
+ </bookinfo>
+
+ <toc></toc>
+
+ <chapter id="w1_internal">
+ <title>W1 API internal to the kernel</title>
+
+ <sect1 id="w1_internal_api">
+ <title>W1 API internal to the kernel</title>
+ <sect2 id="w1.h">
+ <title>drivers/w1/w1.h</title>
+ <para>W1 core functions.</para>
+!Idrivers/w1/w1.h
+ </sect2>
+
+ <sect2 id="w1.c">
+ <title>drivers/w1/w1.c</title>
+ <para>W1 core functions.</para>
+!Idrivers/w1/w1.c
+ </sect2>
+
+ <sect2 id="w1_family.h">
+ <title>drivers/w1/w1_family.h</title>
+ <para>Allows registering device family operations.</para>
+!Idrivers/w1/w1_family.h
+ </sect2>
+
+ <sect2 id="w1_family.c">
+ <title>drivers/w1/w1_family.c</title>
+ <para>Allows registering device family operations.</para>
+!Edrivers/w1/w1_family.c
+ </sect2>
+
+ <sect2 id="w1_int.c">
+ <title>drivers/w1/w1_int.c</title>
+ <para>W1 internal initialization for master devices.</para>
+!Edrivers/w1/w1_int.c
+ </sect2>
+
+ <sect2 id="w1_netlink.h">
+ <title>drivers/w1/w1_netlink.h</title>
+ <para>W1 external netlink API structures and commands.</para>
+!Idrivers/w1/w1_netlink.h
+ </sect2>
+
+ <sect2 id="w1_io.c">
+ <title>drivers/w1/w1_io.c</title>
+ <para>W1 input/output.</para>
+!Edrivers/w1/w1_io.c
+!Idrivers/w1/w1_io.c
+ </sect2>
+
+ </sect1>
+
+
+ </chapter>
+
+</book>
diff --git a/Documentation/w1/w1.netlink b/Documentation/w1/w1.netlink
index f59a319..927a52c 100644
--- a/Documentation/w1/w1.netlink
+++ b/Documentation/w1/w1.netlink
@@ -5,8 +5,8 @@ Message types.
=============

There are three types of messages between w1 core and userspace:
-1. Events. They are generated each time new master or slave device
- found either due to automatic or requested search.
+1. Events. They are generated each time a new master or slave device
+ is found either due to automatic or requested search.
2. Userspace commands.
3. Replies to userspace commands.

@@ -131,7 +131,7 @@ of the w1_netlink_cmd structure and cn_msg.len will be equal to the sum
of the sizeof(struct w1_netlink_msg) and sizeof(struct w1_netlink_cmd).
If reply is generated for master or root command (which do not have
w1_netlink_cmd attached), reply will contain only cn_msg and w1_netlink_msg
-structires.
+structures.

w1_netlink_msg.status field will carry positive error value
(EINVAL for example) or zero in case of success.
@@ -160,7 +160,7 @@ procedure is started to select given device.
Then all requested in w1_netlink_msg operations are performed one by one.
If command requires reply (like read command) it is sent on command completion.

-When all commands (w1_netlink_cmd) are processed muster device is unlocked
+When all commands (w1_netlink_cmd) are processed master device is unlocked
and next w1_netlink_msg header processing started.


diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 53846c7..9eb816b 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -50,8 +50,21 @@ int w1_max_slave_count = 64;
int w1_max_slave_ttl = 10;

module_param_named(timeout, w1_timeout, int, 0);
+MODULE_PARM_DESC(timeout, "time in seconds between automatic slave searches");
+/* A search stops when w1_max_slave_count devices have been found in that
+ * search. The next search will start over and detect the same set of devices
+ * on a static 1-wire bus. Memory is not allocated based on this number, just
+ * on the number of devices known to the kernel. Having a high number does not
+ * consume additional resources. As a special case, if there is only one
+ * device on the network and w1_max_slave_count is set to 1, the device id can
+ * be read directly skipping the normal slower search process.
+ */
module_param_named(max_slave_count, w1_max_slave_count, int, 0);
+MODULE_PARM_DESC(max_slave_count,
+ "maximum number of slaves detected in a search");
module_param_named(slave_ttl, w1_max_slave_ttl, int, 0);
+MODULE_PARM_DESC(slave_ttl,
+ "Number of searches not seeing a slave before it will be removed");

DEFINE_MUTEX(w1_mlock);
LIST_HEAD(w1_masters);
@@ -920,7 +933,12 @@ void w1_slave_found(struct w1_master *dev, u64 rn)
}

/**
- * Performs a ROM Search & registers any devices found.
+ * w1_search() - Performs a ROM Search & registers any devices found.
+ * @dev: The master device to search
+ * @search_type: W1_SEARCH to search all devices, or W1_ALARM_SEARCH
+ * to return only devices in the alarmed state
+ * @cb: Function to call when a device is found
+ *
* The 1-wire search is a simple binary tree search.
* For each bit of the address, we read two bits and write one bit.
* The bit written will put to sleep all devies that don't match that bit.
@@ -930,8 +948,6 @@ void w1_slave_found(struct w1_master *dev, u64 rn)
*
* See "Application note 187 1-wire search algorithm" at http://www.maxim-ic.com
*
- * @dev The master device to search
- * @cb Function to call when a device is found
*/
void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb)
{
@@ -990,7 +1006,7 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
else
search_bit = ((last_rn >> i) & 0x1);

- /** Read two bits and write one bit */
+ /* Read two bits and write one bit */
triplet_ret = w1_triplet(dev, search_bit);

/* quit if no device responded */
@@ -1074,6 +1090,12 @@ static void w1_search_process(struct w1_master *dev, u8 search_type)
w1_search_process_cb(dev, search_type, w1_slave_found);
}

+/**
+ * w1_process_callbacks() - execute each dev->async_list callback entry
+ * @dev: w1_master device
+ *
+ * Return: 1 if there were commands to executed 0 otherwise
+ */
int w1_process_callbacks(struct w1_master *dev)
{
int ret = 0;
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 0eb5050..734dab7 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -22,6 +22,13 @@
#ifndef __W1_H
#define __W1_H

+/**
+ * struct w1_reg_num - broken out slave device id
+ *
+ * @family: identifies the type of device
+ * @id: along with family is the unique device id
+ * @crc: checksum of the other bytes
+ */
struct w1_reg_num
{
#if defined(__LITTLE_ENDIAN_BITFIELD)
@@ -60,6 +67,22 @@ struct w1_reg_num
#define W1_SLAVE_ACTIVE 0
#define W1_SLAVE_DETACH 1

+/**
+ * struct w1_slave - holds a single slave device on the bus
+ *
+ * @owner: Points to the one wire "wire" kernel module.
+ * @name: Device id is ascii.
+ * @w1_slave_entry: data for the linked list
+ * @reg_num: the slave id in binary
+ * @refcnt: reference count, delete when 0
+ * @flags: bit flags for W1_SLAVE_ACTIVE W1_SLAVE_DETACH
+ * @ttl: decrement per search this slave isn't found, deatch at 0
+ * @master: bus which this slave is on
+ * @family: module for device family type
+ * @family_data: pointer for use by the family module
+ * @dev: kernel device identifier
+ *
+ */
struct w1_slave
{
struct module *owner;
@@ -80,77 +103,74 @@ typedef void (*w1_slave_found_callback)(struct w1_master *, u64);


/**
+ * struct w1_bus_master - operations available on a bus master
+ *
+ * @data: the first parameter in all the functions below
+ *
+ * @read_bit: Sample the line level @return the level read (0 or 1)
+ *
+ * @write_bit: Sets the line level
+ *
+ * @touch_bit: the lowest-level function for devices that really support the
+ * 1-wire protocol.
+ * touch_bit(0) = write-0 cycle
+ * touch_bit(1) = write-1 / read cycle
+ * @return the bit read (0 or 1)
+ *
+ * @read_byte: Reads a bytes. Same as 8 touch_bit(1) calls.
+ * @return the byte read
+ *
+ * @write_byte: Writes a byte. Same as 8 touch_bit(x) calls.
+ *
+ * @read_block: Same as a series of read_byte() calls
+ * @return the number of bytes read
+ *
+ * @write_block: Same as a series of write_byte() calls
+ *
+ * @triplet: Combines two reads and a smart write for ROM searches
+ * @return bit0=Id bit1=comp_id bit2=dir_taken
+ *
+ * @reset_bus: long write-0 with a read for the presence pulse detection
+ * @return -1=Error, 0=Device present, 1=No device present
+ *
+ * @set_pullup: Put out a strong pull-up pulse of the specified duration.
+ * @return -1=Error, 0=completed
+ *
+ * @search: Really nice hardware can handles the different types of ROM search
+ * w1_master* is passed to the slave found callback.
+ * u8 is search_type, W1_SEARCH or W1_ALARM_SEARCH
+ *
* Note: read_bit and write_bit are very low level functions and should only
* be used with hardware that doesn't really support 1-wire operations,
* like a parallel/serial port.
* Either define read_bit and write_bit OR define, at minimum, touch_bit and
* reset_bus.
+ *
*/
struct w1_bus_master
{
- /** the first parameter in all the functions below */
void *data;

- /**
- * Sample the line level
- * @return the level read (0 or 1)
- */
u8 (*read_bit)(void *);

- /** Sets the line level */
void (*write_bit)(void *, u8);

- /**
- * touch_bit is the lowest-level function for devices that really
- * support the 1-wire protocol.
- * touch_bit(0) = write-0 cycle
- * touch_bit(1) = write-1 / read cycle
- * @return the bit read (0 or 1)
- */
u8 (*touch_bit)(void *, u8);

- /**
- * Reads a bytes. Same as 8 touch_bit(1) calls.
- * @return the byte read
- */
u8 (*read_byte)(void *);

- /**
- * Writes a byte. Same as 8 touch_bit(x) calls.
- */
void (*write_byte)(void *, u8);

- /**
- * Same as a series of read_byte() calls
- * @return the number of bytes read
- */
u8 (*read_block)(void *, u8 *, int);

- /** Same as a series of write_byte() calls */
void (*write_block)(void *, const u8 *, int);

- /**
- * Combines two reads and a smart write for ROM searches
- * @return bit0=Id bit1=comp_id bit2=dir_taken
- */
u8 (*triplet)(void *, u8);

- /**
- * long write-0 with a read for the presence pulse detection
- * @return -1=Error, 0=Device present, 1=No device present
- */
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.
- * u8 is search_type, W1_SEARCH or W1_ALARM_SEARCH
- */
void (*search)(void *, struct w1_master *,
u8, w1_slave_found_callback);
};
@@ -165,6 +185,37 @@ enum w1_master_flags {
W1_WARN_MAX_COUNT = 1,
};

+/**
+ * struct w1_master - one per bus master
+ * @w1_master_entry: master linked list
+ * @owner: module owner
+ * @name: dynamically allocate bus name
+ * @list_mutex: protect slist and async_list
+ * @slist: linked list of slaves
+ * @async_list: linked list of netlink commands to execute
+ * @max_slave_count: maximum number of slaves to search for at a time
+ * @slave_count: current number of slaves known
+ * @attempts: number of searches ran
+ * @slave_ttl: number of searches before a slave is timed out
+ * @initialized: prevent init/removal race conditions
+ * @id: w1 bus number
+ * @search_count: number of automatic searches to run, -1 unlimited
+ * @search_id: allows continuing a search
+ * @refcnt: reference count
+ * @priv: private data storage
+ * @priv_size: size allocated
+ * @enable_pullup: allows a strong pullup
+ * @pullup_duration: time for the next strong pullup
+ * @flags: one of w1_master_flags
+ * @thread: thread for bus search and netlink commands
+ * @mutex: protect most of w1_master
+ * @bus_mutex: pretect concurrent bus access
+ * @driver: sysfs driver
+ * @dev: sysfs device
+ * @bus_master: io operations available
+ * @seq: sequence number used for netlink broadcasts
+ * @portid: destination for the current netlink command
+ */
struct w1_master
{
struct list_head w1_master_entry;
@@ -173,7 +224,7 @@ struct w1_master
/* list_mutex protects just slist and async_list so slaves can be
* searched for and async commands added while the master has
* w1_master.mutex locked and is operating on the bus.
- * lock order w1_mlock, w1_master.mutex, w1_master_list_mutex
+ * lock order w1_mlock, w1_master.mutex, w1_master.list_mutex
*/
struct mutex list_mutex;
struct list_head slist;
@@ -290,7 +341,6 @@ extern int w1_max_slave_ttl;
extern struct list_head w1_masters;
extern struct mutex w1_mlock;

-/* returns 1 if there were commands to executed 0 otherwise */
extern int w1_process_callbacks(struct w1_master *dev);
extern int w1_process(void *);

diff --git a/drivers/w1/w1_family.c b/drivers/w1/w1_family.c
index e9309778..3bff6b3 100644
--- a/drivers/w1/w1_family.c
+++ b/drivers/w1/w1_family.c
@@ -31,6 +31,10 @@
DEFINE_SPINLOCK(w1_flock);
static LIST_HEAD(w1_families);

+/**
+ * w1_register_family() - register a device family driver
+ * @newf: family to register
+ */
int w1_register_family(struct w1_family *newf)
{
struct list_head *ent, *n;
@@ -59,6 +63,10 @@ int w1_register_family(struct w1_family *newf)
return ret;
}

+/**
+ * w1_unregister_family() - unregister a device family driver
+ * @fent: family to unregister
+ */
void w1_unregister_family(struct w1_family *fent)
{
struct list_head *ent, *n;
diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
index 4ad0e81..26ca134 100644
--- a/drivers/w1/w1_family.h
+++ b/drivers/w1/w1_family.h
@@ -48,6 +48,12 @@

struct w1_slave;

+/**
+ * struct w1_family_ops - operations for a family type
+ * @add_slave: add_slave
+ * @remove_slave: remove_slave
+ * @groups: sysfs group
+ */
struct w1_family_ops
{
int (* add_slave)(struct w1_slave *);
@@ -55,6 +61,13 @@ struct w1_family_ops
const struct attribute_group **groups;
};

+/**
+ * struct w1_family - reference counted family structure.
+ * @family_entry: family linked list
+ * @fid: 8 bit family identifier
+ * @fops: operations for this family
+ * @refcnt: reference counter
+ */
struct w1_family
{
struct list_head family_entry;
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index ae3b595..6badf2b 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -105,6 +105,10 @@ static void w1_free_dev(struct w1_master *dev)
device_unregister(&dev->dev);
}

+/**
+ * w1_add_master_device() - registers a new master device
+ * @master: master bus device to register
+ */
int w1_add_master_device(struct w1_bus_master *master)
{
struct w1_master *dev, *entry;
@@ -239,6 +243,10 @@ void __w1_remove_master_device(struct w1_master *dev)
w1_free_dev(dev);
}

+/**
+ * w1_remove_master_device() - unregister a master device
+ * @bm: master bus device to remove
+ */
void w1_remove_master_device(struct w1_bus_master *bm)
{
struct w1_master *dev, *found = NULL;
diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
index e10acc2..2820924 100644
--- a/drivers/w1/w1_io.c
+++ b/drivers/w1/w1_io.c
@@ -62,7 +62,9 @@ static void w1_write_bit(struct w1_master *dev, int bit);
static u8 w1_read_bit(struct w1_master *dev);

/**
- * Generates a write-0 or write-1 cycle and samples the level.
+ * w1_touch_bit() - Generates a write-0 or write-1 cycle and samples the level.
+ * @dev: the master device
+ * @bit: 0 - write a 0, 1 - write a 0 read the level
*/
static u8 w1_touch_bit(struct w1_master *dev, int bit)
{
@@ -77,7 +79,10 @@ static u8 w1_touch_bit(struct w1_master *dev, int bit)
}

/**
- * Generates a write-0 or write-1 cycle.
+ * w1_write_bit() - Generates a write-0 or write-1 cycle.
+ * @dev: the master device
+ * @bit: bit to write
+ *
* Only call if dev->bus_master->touch_bit is NULL
*/
static void w1_write_bit(struct w1_master *dev, int bit)
@@ -102,11 +107,12 @@ static void w1_write_bit(struct w1_master *dev, int bit)
}

/**
+ * w1_pre_write() - pre-write operations
+ * @dev: the master device
+ *
* 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)
{
@@ -118,11 +124,12 @@ static void w1_pre_write(struct w1_master *dev)
}

/**
+ * w1_post_write() - post-write options
+ * @dev: the master device
+ *
* 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)
{
@@ -136,10 +143,9 @@ static void w1_post_write(struct w1_master *dev)
}

/**
- * Writes 8 bits.
- *
- * @param dev the master device
- * @param byte the byte to write
+ * w1_write_8() - Writes 8 bits.
+ * @dev: the master device
+ * @byte: the byte to write
*/
void w1_write_8(struct w1_master *dev, u8 byte)
{
@@ -161,7 +167,9 @@ EXPORT_SYMBOL_GPL(w1_write_8);


/**
- * Generates a write-1 cycle and samples the level.
+ * w1_read_bit() - Generates a write-1 cycle and samples the level.
+ * @dev: the master device
+ *
* Only call if dev->bus_master->touch_bit is NULL
*/
static u8 w1_read_bit(struct w1_master *dev)
@@ -185,16 +193,17 @@ static u8 w1_read_bit(struct w1_master *dev)
}

/**
- * Does a triplet - used for searching ROM addresses.
+ * w1_triplet() - * Does a triplet - used for searching ROM addresses.
+ * @dev: the master device
+ * @bdir: the bit to write if both id_bit and comp_bit are 0
+ *
* Return bits:
* bit 0 = id_bit
* bit 1 = comp_bit
* bit 2 = dir_taken
* If both bits 0 & 1 are set, the search should be restarted.
*
- * @param dev the master device
- * @param bdir the bit to write if both id_bit and comp_bit are 0
- * @return bit fields - see above
+ * Return: bit fields - see above
*/
u8 w1_triplet(struct w1_master *dev, int bdir)
{
@@ -226,10 +235,10 @@ u8 w1_triplet(struct w1_master *dev, int bdir)
}

/**
- * Reads 8 bits.
+ * w1_read_8() - Reads 8 bits.
+ * @dev: the master device
*
- * @param dev the master device
- * @return the byte read
+ * Return: the byte read
*/
u8 w1_read_8(struct w1_master *dev)
{
@@ -247,11 +256,10 @@ u8 w1_read_8(struct w1_master *dev)
EXPORT_SYMBOL_GPL(w1_read_8);

/**
- * Writes a series of bytes.
- *
- * @param dev the master device
- * @param buf pointer to the data to write
- * @param len the number of bytes to write
+ * w1_write_block() - Writes a series of bytes.
+ * @dev: the master device
+ * @buf: pointer to the data to write
+ * @len: the number of bytes to write
*/
void w1_write_block(struct w1_master *dev, const u8 *buf, int len)
{
@@ -269,11 +277,10 @@ void w1_write_block(struct w1_master *dev, const u8 *buf, int len)
EXPORT_SYMBOL_GPL(w1_write_block);

/**
- * Touches a series of bytes.
- *
- * @param dev the master device
- * @param buf pointer to the data to write
- * @param len the number of bytes to write
+ * w1_touch_block() - Touches a series of bytes.
+ * @dev: the master device
+ * @buf: pointer to the data to write
+ * @len: the number of bytes to write
*/
void w1_touch_block(struct w1_master *dev, u8 *buf, int len)
{
@@ -294,12 +301,11 @@ void w1_touch_block(struct w1_master *dev, u8 *buf, int len)
EXPORT_SYMBOL_GPL(w1_touch_block);

/**
- * Reads a series of bytes.
- *
- * @param dev the master device
- * @param buf pointer to the buffer to fill
- * @param len the number of bytes to read
- * @return the number of bytes read
+ * w1_read_block() - Reads a series of bytes.
+ * @dev: the master device
+ * @buf: pointer to the buffer to fill
+ * @len: the number of bytes to read
+ * Return: the number of bytes read
*/
u8 w1_read_block(struct w1_master *dev, u8 *buf, int len)
{
@@ -319,10 +325,9 @@ u8 w1_read_block(struct w1_master *dev, u8 *buf, int len)
EXPORT_SYMBOL_GPL(w1_read_block);

/**
- * Issues a reset bus sequence.
- *
- * @param dev The bus master pointer
- * @return 0=Device present, 1=No device present or error
+ * w1_reset_bus() - Issues a reset bus sequence.
+ * @dev: the master device
+ * Return: 0=Device present, 1=No device present or error
*/
int w1_reset_bus(struct w1_master *dev)
{
@@ -383,12 +388,15 @@ void w1_search_devices(struct w1_master *dev, u8 search_type, w1_slave_found_cal
}

/**
+ * w1_reset_select_slave() - reset and select a slave
+ * @sl: the slave to select
+ *
* Resets the bus and then selects the slave by sending either a skip rom
- * or a rom match.
+ * or a rom match. A skip rom is issued if there is only one device
+ * registered on the bus.
* The w1 master lock must be held.
*
- * @param sl the slave to select
- * @return 0=success, anything else=error
+ * Return: 0=success, anything else=error
*/
int w1_reset_select_slave(struct w1_slave *sl)
{
@@ -409,6 +417,9 @@ int w1_reset_select_slave(struct w1_slave *sl)
EXPORT_SYMBOL_GPL(w1_reset_select_slave);

/**
+ * w1_reset_resume_command() - resume instead of another match ROM
+ * @dev: the master device
+ *
* When the workflow with a slave amongst many requires several
* successive commands a reset between each, this function is similar
* to doing a reset then a match ROM for the last matched ROM. The
@@ -420,8 +431,6 @@ EXPORT_SYMBOL_GPL(w1_reset_select_slave);
* doesn't work of course, but the resume command is the next best thing.
*
* The w1 master lock must be held.
- *
- * @param dev the master device
*/
int w1_reset_resume_command(struct w1_master *dev)
{
@@ -435,6 +444,10 @@ int w1_reset_resume_command(struct w1_master *dev)
EXPORT_SYMBOL_GPL(w1_reset_resume_command);

/**
+ * w1_next_pullup() - register for a strong pullup
+ * @dev: the master device
+ * @delay: time in milliseconds
+ *
* 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
@@ -442,8 +455,7 @@ EXPORT_SYMBOL_GPL(w1_reset_resume_command);
* 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
+ * Return: 0=success, anything else=error
*/
void w1_next_pullup(struct w1_master *dev, int delay)
{
diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
index ea9f3e4..1e9504e 100644
--- a/drivers/w1/w1_netlink.h
+++ b/drivers/w1/w1_netlink.h
@@ -27,7 +27,8 @@

#include "w1.h"

-/** enum w1_netlink_message_types - message type
+/**
+ * enum w1_netlink_message_types - message type
*
* @W1_SLAVE_ADD: notification that a slave device was added
* @W1_SLAVE_REMOVE: notification that a slave device was removed
@@ -63,7 +64,8 @@ struct w1_netlink_msg
__u8 data[0];
};

-/** enum w1_commands - commands available for master or slave operations
+/**
+ * enum w1_commands - commands available for master or slave operations
* @W1_CMD_READ: read len bytes
* @W1_CMD_WRITE: write len bytes
* @W1_CMD_SEARCH: initiate a standard search, returns only the slave
--
1.7.10.4

2014-01-16 04:34:33

by David Fries

[permalink] [raw]
Subject: [PATCH 15/15] w1: hold bus_mutex in netlink and search

The bus_mutex needs to be taken to serialize access to a specific bus.
netlink wasn't updated when bus_mutex was added and was calling
without that lock held, and not all of the masters were holding the
bus_mutex in a search. This was causing the ds2490 hardware to stop
responding when both netlink and /sys slaves were executing bus
commands at the same time.

Signed-off-by: David Fries <[email protected]>
Acked-by: Evgeniy Polyakov <[email protected]>
---
drivers/w1/masters/ds1wm.c | 4 +++-
drivers/w1/masters/ds2490.c | 8 ++++++--
drivers/w1/w1_netlink.c | 13 +++++++++++--
3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index 02df3b1..b077b8b 100644
--- a/drivers/w1/masters/ds1wm.c
+++ b/drivers/w1/masters/ds1wm.c
@@ -326,13 +326,14 @@ static void ds1wm_search(void *data, struct w1_master *master_dev,
unsigned slaves_found = 0;
unsigned int pass = 0;

+ mutex_lock(&master_dev->bus_mutex);
dev_dbg(&ds1wm_data->pdev->dev, "search begin\n");
while (true) {
++pass;
if (pass > 100) {
dev_dbg(&ds1wm_data->pdev->dev,
"too many attempts (100), search aborted\n");
- return;
+ break;
}

mutex_lock(&master_dev->bus_mutex);
@@ -439,6 +440,7 @@ static void ds1wm_search(void *data, struct w1_master *master_dev,
dev_dbg(&ds1wm_data->pdev->dev,
"pass: %d total: %d search done ms d bit pos: %d\n", pass,
slaves_found, ms_discrep_bit);
+ mutex_unlock(&master_dev->bus_mutex);
}

/* --------------------------------------------------------------------- */
diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index db0bf32..7404ad30 100644
--- a/drivers/w1/masters/ds2490.c
+++ b/drivers/w1/masters/ds2490.c
@@ -727,9 +727,11 @@ static void ds9490r_search(void *data, struct w1_master *master,
*/
u64 buf[2*64/8];

+ mutex_lock(&master->bus_mutex);
+
/* address to start searching at */
if (ds_send_data(dev, (u8 *)&master->search_id, 8) < 0)
- return;
+ goto search_out;
master->search_id = 0;

value = COMM_SEARCH_ACCESS | COMM_IM | COMM_RST | COMM_SM | COMM_F |
@@ -739,7 +741,7 @@ static void ds9490r_search(void *data, struct w1_master *master,
search_limit = 0;
index = search_type | (search_limit << 8);
if (ds_send_control(dev, value, index) < 0)
- return;
+ goto search_out;

do {
schedule_timeout(jtime);
@@ -791,6 +793,8 @@ static void ds9490r_search(void *data, struct w1_master *master,
master->max_slave_count);
set_bit(W1_WARN_MAX_COUNT, &master->flags);
}
+search_out:
+ mutex_unlock(&master->bus_mutex);
}

#if 0
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index a5dc219..5234964 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -246,11 +246,16 @@ static int w1_process_command_master(struct w1_master *dev,
{
int err = -EINVAL;

+ /* drop bus_mutex for search (does it's own locking), and add/remove
+ * which doesn't use the bus
+ */
switch (req_cmd->cmd) {
case W1_CMD_SEARCH:
case W1_CMD_ALARM_SEARCH:
case W1_CMD_LIST_SLAVES:
+ mutex_unlock(&dev->bus_mutex);
err = w1_get_slaves(dev, req_msg, req_hdr, req_cmd);
+ mutex_lock(&dev->bus_mutex);
break;
case W1_CMD_READ:
case W1_CMD_WRITE:
@@ -262,8 +267,12 @@ static int w1_process_command_master(struct w1_master *dev,
break;
case W1_CMD_SLAVE_ADD:
case W1_CMD_SLAVE_REMOVE:
+ mutex_unlock(&dev->bus_mutex);
+ mutex_lock(&dev->mutex);
err = w1_process_command_addremove(dev, req_msg, req_hdr,
req_cmd);
+ mutex_unlock(&dev->mutex);
+ mutex_lock(&dev->bus_mutex);
break;
default:
err = -EINVAL;
@@ -400,7 +409,7 @@ static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
struct w1_slave *sl = node->sl;
struct w1_netlink_cmd *cmd = NULL;

- mutex_lock(&dev->mutex);
+ mutex_lock(&dev->bus_mutex);
dev->portid = node->block->portid;
if (sl && w1_reset_select_slave(sl))
err = -ENODEV;
@@ -437,7 +446,7 @@ static void w1_process_cb(struct w1_master *dev, struct w1_async_cmd *async_cmd)
else
atomic_dec(&dev->refcnt);
dev->portid = 0;
- mutex_unlock(&dev->mutex);
+ mutex_unlock(&dev->bus_mutex);

mutex_lock(&dev->list_mutex);
list_del(&async_cmd->async_entry);
--
1.7.10.4