[RFC PATCH 0/5 V5] IIO: in kernel pull interfaces.

[RFC PATCH 0/5 V5] IIO: in kernel pull interfaces.

[Jonathan Cameron]

Hi All,

Firstly this is an RFC rather than a final proposal because I know there is
an issue with the provider being removed before the consumer.  It will be easy
to make this less likely to happen, but I am not entirely sure how to avoid
any possible race (suggestion welcome).

Greg KH raised significant issues with how I originally proposed doing this
map registration (consumer - provider association).  Hence this version is
a lot more conventional (you may well notice that even the header naming is
lifted from the regulator framework).  The original proposal registered this
mapping completely independent of the iio drivers, thus requiring no in driver
changes.  This version requires passing the mapping through them.  There are
advantages to both approaches and we can always bolt the driver independent
version back in if there is sufficient justification for it at a later date.

Note that this series is against the full IIO tree in staging.

All comments welcome.

Jonathan

Jonathan Cameron (5):
  staging:iio:core add in kernel interface mapping and getting IIO
    channels.
  staging:iio: move iio data return types into types.h for use by
    inkern
  staging:iio::hwmon interface client driver.
  staging:iio:Documentation in kernel pull description.
  stargate2: example of map configuration for iio to hwmon example.

 arch/arm/mach-pxa/stargate2.c                  |   22 ++
 drivers/staging/iio/Documentation/inkernel.txt |   59 ++++++
 drivers/staging/iio/Kconfig                    |   16 ++-
 drivers/staging/iio/Makefile                   |    4 +-
 drivers/staging/iio/consumer.h                 |  103 ++++++++++
 drivers/staging/iio/driver.h                   |   34 +++
 drivers/staging/iio/iio.h                      |    6 -
 drivers/staging/iio/iio_core.h                 |    3 +
 drivers/staging/iio/iio_hwmon.c                |  226 +++++++++++++++++++++
 drivers/staging/iio/industrialio-core.c        |    2 +-
 drivers/staging/iio/inkern.c                   |  256 ++++++++++++++++++++++++
 drivers/staging/iio/machine.h                  |   30 +++
 drivers/staging/iio/types.h                    |    4 +
 13 files changed, 756 insertions(+), 9 deletions(-)
 create mode 100644 drivers/staging/iio/Documentation/inkernel.txt
 create mode 100644 drivers/staging/iio/consumer.h
 create mode 100644 drivers/staging/iio/driver.h
 create mode 100644 drivers/staging/iio/iio_hwmon.c
 create mode 100644 drivers/staging/iio/inkern.c
 create mode 100644 drivers/staging/iio/machine.h

-- 
1.7.8.4

[PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Lifted from proposal for in kernel interface built on the out of staging
branch.

Two elements here:
* Map as defined in "inkern.h"
* Matching code to actually get the iio_dev and channel
that we want from the global list of IIO devices.

V2: As per Greg KH suggestion, move over to registration by passing
the tables into the provider drivers (how regulator does it).
This does not prevent us using the original more flexible approach
if at a later date there is a usecase that demands it.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/Kconfig             |    7 +-
 drivers/staging/iio/Makefile            |    2 +-
 drivers/staging/iio/consumer.h          |  103 +++++++++++++
 drivers/staging/iio/driver.h            |   34 ++++
 drivers/staging/iio/iio_core.h          |    3 +
 drivers/staging/iio/industrialio-core.c |    2 +-
 drivers/staging/iio/inkern.c            |  256 +++++++++++++++++++++++++++++++
 drivers/staging/iio/machine.h           |   30 ++++
 8 files changed, 434 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index 90162aa..65c2a8e 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -10,8 +10,14 @@ menuconfig IIO
 	  drivers for many different types of embedded sensors using a
 	  number of different physical interfaces (i2c, spi, etc). See
 	  drivers/staging/iio/Documentation for more information.
+
 if IIO
 
+config IIO_INKERN
+	bool "In kernel support for IIO"
+	help
+	  Support in kernel users of IIO device drivers.
+
 config IIO_BUFFER
 	bool "Enable buffer support within IIO"
 	help
@@ -96,5 +102,4 @@ config IIO_SIMPLE_DUMMY_BUFFER
          Add buffered data capture to the simple dummy driver.
 
 endif # IIO_SIMPLE_DUMMY
-
 endif # IIO
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index 657710b..5da4c78 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -1,9 +1,9 @@
 #
 # Makefile for the industrial I/O core.
 #
-
 obj-$(CONFIG_IIO) += industrialio.o
 industrialio-y := industrialio-core.o industrialio-event.o
+industrialio-$(CONFIG_IIO_INKERN) += inkern.o
 industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 
diff --git a/drivers/staging/iio/consumer.h b/drivers/staging/iio/consumer.h
new file mode 100644
index 0000000..4ac320b
--- /dev/null
+++ b/drivers/staging/iio/consumer.h
@@ -0,0 +1,103 @@
+/*
+ * Industrial I/O in kernel consumer interface
+ *
+ * Copyright (c) 2011 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#ifndef _IIO_INKERN_CONSUMER_H_
+#define _IIO_INKERN_CONSUMER_H
+
+struct iio_dev;
+struct iio_chan_spec;
+
+/**
+ * struct iio_channel - everything needed for a consumer to use a channel
+ * @indio_dev:		Device on which the channel exists.
+ * @channel:		Full description of the channel.
+ */
+struct iio_channel {
+	struct iio_dev *indio_dev;
+	const struct iio_chan_spec *channel;
+};
+
+/**
+ * iio_channel_get() - get description of all that is needed to access channel.
+ * @dev:		Pointer to the struct device for this device as provided
+ *			in the iio_map with which the desired provider to
+ *			consumer mapping was registered.
+ *			Null if the mapping was registered against consumer
+ *			device name.
+ * @name:		Unique name of the device as provided in the iio_map
+ *			with which the desired provider to consumer mapping
+ *			was registered.
+ *			Null if the mapping was instead registered by a struct
+ *			device pointer.
+ * @consumer_channel:	Unique name to identify the channel on the consumer
+ *			side. This typically describes the channels use within
+ *			the consumer. E.g. 'battery_voltage'
+ */
+struct iio_channel *iio_st_channel_get(const struct device *dev,
+				       const char *name,
+				       const char *consumer_channel);
+
+/**
+ * iio_st_channel_release() - release channels obtained via iio_st_channel_get
+ * @chan:``		The channel to be released.
+ */
+void iio_st_channel_release(struct iio_channel *chan);
+
+/**
+ * iio_st_channel_get_all() - get all channels associated with a client
+ * @dev:		consumer device structure (null if name to be used)
+ * @name:		name of consuemr device (null if dev pointer to be used)
+ *
+ * Returns an array of iio_channel structures terminated with one with
+ * null iio_dev pointer.
+ * This function is used by fairly generic consumers to get all the
+ * channels registered as having this consumer.
+ */
+struct iio_channel *iio_st_channel_get_all(const struct device *dev,
+					   const char *name);
+
+/**
+ * iio_st_channel_release_all() - reverse iio_st_get_all
+ * @chan:		Array of channels to be released.
+ */
+void iio_st_channel_release_all(struct iio_channel *chan);
+
+/**
+ * iio_st_read_channel_raw() - read from a given channel
+ * @channel:		The channel being queried.
+ * @val:		Value read back.
+ *
+ * Note raw reads from iio channels are in adc counts and hence
+ * scale will need to be applied if standard units required.
+ */
+int iio_st_read_channel_raw(struct iio_channel *chan,
+			    int *val);
+
+/**
+ * iio_st_get_channel_type() - get the type of a channel
+ * @channel:		The channel being queried.
+ *
+ * returns the enum iio_chan_type of the channel
+ */
+enum iio_chan_type iio_st_get_channel_type(struct iio_channel *channel);
+
+/**
+ * iio_st_read_channel_scale() - read the scale value for a channel
+ * @channel:		The channel being queried.
+ * @val:		First part of value read back.
+ * @val2:		Second part of value read back.
+ *
+ * Note returns a description of what is in val and val2, such
+ * as IIO_VAL_INT_PLUS_MICRO telling us we have a value of val
+ * + val2/1e6
+ */
+int iio_st_read_channel_scale(struct iio_channel *chan, int *val,
+			      int *val2);
+
+#endif
diff --git a/drivers/staging/iio/driver.h b/drivers/staging/iio/driver.h
new file mode 100644
index 0000000..cea8bcd
--- /dev/null
+++ b/drivers/staging/iio/driver.h
@@ -0,0 +1,34 @@
+/*
+ * Industrial I/O in kernel access map interface.
+ *
+ * Copyright (c) 2011 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _IIO_INKERN_H_
+#define _IIO_INKERN_H_
+
+struct iio_map;
+
+/**
+ * iio_map_array_register() - tell the core about inkernel consumers
+ * @indio_dev: provider device
+ * @map: array of mappings specifying association of channel with client
+ */
+int iio_map_array_register(struct iio_dev *indio_dev,
+			   struct iio_map *map);
+
+/**
+ * iio_map_array_unregister() - tell the core to remove consumer mappings
+ * @indio_dev: provider device
+ * @map: array of mappings to remove. Note these must have same memory
+ *       addresses as those originally added not just equal parameter
+ *       values.
+ */
+int iio_map_array_unregister(struct iio_dev *indio_dev,
+			     struct iio_map *map);
+
+#endif
diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
index c9dfcba..a45bb43 100644
--- a/drivers/staging/iio/iio_core.h
+++ b/drivers/staging/iio/iio_core.h
@@ -12,6 +12,9 @@
 
 #ifndef _IIO_CORE_H_
 #define _IIO_CORE_H_
+#include <linux/device.h>
+
+extern struct device_type iio_dev_type;
 
 int __iio_add_chan_devattr(const char *postfix,
 			   struct iio_chan_spec const *chan,
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index e4824fe..876ea17 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -567,7 +567,7 @@ static void iio_dev_release(struct device *device)
 	iio_device_unregister_sysfs(indio_dev);
 }
 
-static struct device_type iio_dev_type = {
+struct device_type iio_dev_type = {
 	.name = "iio_device",
 	.release = iio_dev_release,
 };
diff --git a/drivers/staging/iio/inkern.c b/drivers/staging/iio/inkern.c
new file mode 100644
index 0000000..634d421
--- /dev/null
+++ b/drivers/staging/iio/inkern.c
@@ -0,0 +1,256 @@
+/* The industrial I/O core in kernel channel mapping
+ *
+ * Copyright (c) 2011 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+
+#include "iio.h"
+#include "iio_core.h"
+#include "machine.h"
+#include "driver.h"
+#include "consumer.h"
+
+struct iio_map_internal {
+	struct iio_dev *indio_dev;
+	struct iio_map *map;
+	struct list_head l;
+};
+
+static LIST_HEAD(iio_map_list);
+static DEFINE_MUTEX(iio_map_list_lock);
+
+int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
+{
+	int i = 0, ret = 0;
+	struct iio_map_internal *mapi;
+	if (maps == NULL)
+		return 0;
+	mutex_lock(&iio_map_list_lock);
+	while ((maps[i].consumer_dev != NULL) ||
+	       (maps[i].consumer_dev_name != NULL)) {
+		mapi = kzalloc(sizeof(*mapi), GFP_KERNEL);
+		if (mapi == NULL) {
+			ret = -ENOMEM;
+			goto error_ret;
+		}
+		mapi->map = &maps[i];
+		mapi->indio_dev = indio_dev;
+		list_add(&mapi->l, &iio_map_list);
+		i++;
+	}
+error_ret:
+	mutex_unlock(&iio_map_list_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_map_array_register);
+
+
+/* Assumes the exact same array (e.g. memory locations)
+ * used at unregistration as used at registration rather than
+ * more complex checking of contents.
+ */
+int iio_map_array_unregister(struct iio_dev *indio_dev,
+			     struct iio_map *maps)
+{
+	int i = 0, ret = 0;
+	bool found_it;
+	struct iio_map_internal *mapi;
+
+	if (maps == NULL)
+		return 0;
+	mutex_lock(&iio_map_list_lock);
+	while ((maps[i].consumer_dev != NULL) ||
+	       (maps[i].consumer_dev_name != NULL)) {
+		found_it = false;
+		list_for_each_entry(mapi, &iio_map_list, l)
+			if (&maps[i] == mapi->map) {
+				list_del(&mapi->l);
+				kfree(mapi);
+				found_it = true;
+				break;
+			}
+		if (found_it == false) {
+			ret = -ENODEV;
+			goto error_ret;
+		}
+	}
+error_ret:
+	mutex_unlock(&iio_map_list_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_map_array_unregister);
+
+static const struct iio_chan_spec
+*iio_chan_spec_from_name(const struct iio_dev *indio_dev,
+			 const char *name)
+{
+	int i;
+	const struct iio_chan_spec *chan = NULL;
+	for (i = 0; i < indio_dev->num_channels; i++)
+		if (indio_dev->channels[i].datasheet_name &&
+		    strcmp(name, indio_dev->channels[i].datasheet_name) == 0) {
+			chan = &indio_dev->channels[i];
+			break;
+		}
+	return chan;
+}
+
+
+struct iio_channel *iio_st_channel_get(const struct device *dev,
+				       const char *name,
+				       const char *channel_name)
+{
+	struct iio_map_internal *c_i = NULL, *c = NULL;
+	struct iio_channel *channel;
+
+	if (dev == NULL && name == NULL && channel_name == NULL)
+		return ERR_PTR(-ENODEV);
+	/* first find matching entry the channel map */
+	mutex_lock(&iio_map_list_lock);
+	list_for_each_entry(c_i, &iio_map_list, l) {
+		if ((dev && dev != c_i->map->consumer_dev) ||
+		    (name && strcmp(name, c_i->map->consumer_dev_name) != 0) ||
+		    (channel_name &&
+		     strcmp(channel_name, c_i->map->consumer_channel) != 0))
+			continue;
+		c = c_i;
+		get_device(&c->indio_dev->dev);
+		break;
+	}
+	mutex_unlock(&iio_map_list_lock);
+	if (c == NULL)
+		return ERR_PTR(-ENODEV);
+
+	channel = kmalloc(sizeof(*channel), GFP_KERNEL);
+	if (channel == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	channel->indio_dev = c->indio_dev;
+
+	if (c->map->adc_channel_label)
+		channel->channel =
+			iio_chan_spec_from_name(channel->indio_dev,
+						c->map->adc_channel_label);
+
+	return channel;
+}
+EXPORT_SYMBOL_GPL(iio_st_channel_get);
+
+void iio_st_channel_release(struct iio_channel *channel)
+{
+	put_device(&channel->indio_dev->dev);
+	kfree(channel);
+}
+EXPORT_SYMBOL_GPL(iio_st_channel_release);
+
+struct iio_channel *iio_st_channel_get_all(const struct device *dev,
+					    const char *name)
+{
+	struct iio_channel *chans;
+	struct iio_map_internal *c = NULL;
+	int nummaps = 0;
+	int mapind = 0;
+	int i, ret;
+
+	if (dev == NULL && name == NULL)
+		return ERR_PTR(-EINVAL);
+	/* first count the matching maps */
+	list_for_each_entry(c, &iio_map_list, l)
+		if ((dev && dev != c->map->consumer_dev) ||
+		    (name && strcmp(name, c->map->consumer_dev_name) != 0))
+			continue;
+		else
+			nummaps++;
+
+	if (nummaps == 0) {
+		ret = -ENODEV;
+		goto error_ret;
+	}
+
+	/* NULL terminated array to save passing size */
+	chans = kzalloc(sizeof(*chans)*(nummaps + 1), GFP_KERNEL);
+	if (chans == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	/* for each map fill in the chans element */
+	list_for_each_entry(c, &iio_map_list, l) {
+		if (dev && dev != c->map->consumer_dev)
+			continue;
+		if (name && strcmp(name, c->map->consumer_dev_name) != 0)
+			continue;
+		chans[mapind].indio_dev = c->indio_dev;
+		chans[mapind].channel =
+			iio_chan_spec_from_name(chans[mapind].indio_dev,
+						c->map->adc_channel_label);
+		if (chans[mapind].channel == NULL) {
+			ret = -EINVAL;
+			put_device(&chans[mapind].indio_dev->dev);
+			goto error_free_chans;
+		}
+		get_device(&chans[mapind].indio_dev->dev);
+		mapind++;
+	}
+	mutex_unlock(&iio_map_list_lock);
+	if (mapind == 0) {
+		ret = -ENODEV;
+		goto error_free_chans;
+	}
+	return chans;
+
+error_free_chans:
+	for (i = 0; i < nummaps; i++)
+		if (chans[i].indio_dev)
+			put_device(&chans[i].indio_dev->dev);
+	kfree(chans);
+error_ret:
+	mutex_unlock(&iio_map_list_lock);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(iio_st_channel_get_all);
+
+void iio_st_channel_release_all(struct iio_channel *channels)
+{
+	struct iio_channel *chan = &channels[0];
+
+	while (chan->indio_dev) {
+		put_device(&chan->indio_dev->dev);
+		chan++;
+	}
+	kfree(channels);
+}
+EXPORT_SYMBOL_GPL(iio_st_channel_release_all);
+
+int iio_st_read_channel_raw(struct iio_channel *chan, int *val)
+{
+	int val2;
+	return chan->indio_dev->info->read_raw(chan->indio_dev, chan->channel,
+					       val, &val2, 0);
+}
+EXPORT_SYMBOL_GPL(iio_st_read_channel_raw);
+
+int iio_st_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
+{
+	return chan->indio_dev->info->read_raw(chan->indio_dev,
+					       chan->channel,
+					       val, val2,
+					       IIO_CHAN_INFO_SCALE);
+}
+EXPORT_SYMBOL_GPL(iio_st_read_channel_scale);
+
+enum iio_chan_type iio_st_get_channel_type(struct iio_channel *channel)
+{
+	return channel->channel->type;
+}
+EXPORT_SYMBOL_GPL(iio_st_get_channel_type);
diff --git a/drivers/staging/iio/machine.h b/drivers/staging/iio/machine.h
new file mode 100644
index 0000000..7c7da30
--- /dev/null
+++ b/drivers/staging/iio/machine.h
@@ -0,0 +1,30 @@
+/*
+ * Industrial I/O in kernel access map definitions for board files.
+ *
+ * Copyright (c) 2011 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+/**
+ * struct iio_map - description of link between consumer and device channels
+ * @adc_channel_label:	Label used to identify the channel on the provider.
+ *			This is matched against the datasheet_name element
+ *			of struct iio_chan_spec.
+ * @consumer_dev:	Pointer to device structure for the consumer.
+ *			Either this or consumer_dev_name must be supplied
+ *			to allow the consumer to be identified.
+ * @consumer_dev_name:	Name to uniquely identify the consumer if the
+ *			device pointer is not available when registering this
+ *			mapping.
+ * @consumer_channel:	Unique name used to idenitify the channel on the
+ *			consumer side.
+ */
+struct iio_map {
+	const char *adc_channel_label;
+	struct device *consumer_dev;
+	const char *consumer_dev_name;
+	const char *consumer_channel;
+};
-- 
1.7.8.4

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:

> -static struct device_type iio_dev_type = {
> +struct device_type iio_dev_type = {
>  	.name = "iio_device",
>  	.release = iio_dev_release,
>  };

Why is this exported?

> +	mutex_lock(&iio_map_list_lock);
> +	while ((maps[i].consumer_dev != NULL) ||
> +	       (maps[i].consumer_dev_name != NULL)) {

I'd suggest just dropping the struct device - the reason we support the
struct device directly in the regulator API is that we originally had
only a struct device and kept the code around as a transition measure
(though now it's so old we should be able to kill it off).  This would
simplify the code and the interface a bit.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/30/2012 08:22 PM, Mark Brown wrote:
> On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
> 
>> -static struct device_type iio_dev_type = { +struct device_type
>> iio_dev_type = { .name = "iio_device", .release =
>> iio_dev_release, };
> 
> Why is this exported?
oops.  Hangover from previous version I think. Thanks for pointing it out.
> 
>> +	mutex_lock(&iio_map_list_lock); +	while ((maps[i].consumer_dev
>> != NULL) || +	       (maps[i].consumer_dev_name != NULL)) {
> 
> I'd suggest just dropping the struct device - the reason we support
> the struct device directly in the regulator API is that we
> originally had only a struct device and kept the code around as a
> transition measure (though now it's so old we should be able to
> kill it off).  This would simplify the code and the interface a
> bit.
Ah, that explains your comment on the 5th patch.  Alright, I'll let this
sit for a few days and if no one comes up with a good reason not to
we'll go with just the dev_name option.

Jonathan


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPJv2CAAoJEFSFNJnE9BaIfRYP/R9nGQIY7sCm4UsdBZldyW93
8Eu0jPMXJs/YLgc7zSJisFqUQNS4BJjORVDh2AcV/hu7FXaxtD6QhxaC3/RMjwfa
EgElSYU7N2yPThJLXh1w12qgvt6+zxAT+Gnv11pHeaf6gZrXU2IUBy1FlExCudpI
hc+CnDSSHjZk6aJKWmockmcmwlSGLy4WV7nH4S907pzm2yXnI9UqvVaHwPVtO/Yl
rZyrZo6T8C4Xw2GeB5QdZ4GNXRuNUOezc1OfqdK/FKj3lVo23ZsAfoMoKkS33ZWF
zjU8deSJY6VSYhfJtnfv7ucv5GjjhbuWSGZP0VD/BR+eTjWD54EasVv5AqwOz/Iz
wwbvanHTqsVrEBU76733wvWRFelZXgzYhFoQYEucQB5F4KCtqO1OTi62stDCXLLR
Z9ygHH92t7TlsKbxaMGL8gU7jGDeIytuHgJW2BXd8ekwQr4fdY7eBwoW4A+TRSKl
rCNSp5iuKIXjyQ8ZJviOKXlZDJa2/5mYuDO8juQFcYshe+X29pbsrybLPkBBKdhl
GxOi8DtiMVPCiHB63bKGXhqe38jNeHK0hboiREHZgnEt1/a+ZkLDVnWtLQcUBuBo
UfLfuzQju/DrtqnWj8asmRzpsA675Lh1xmdLU0tv/NEb/aOGnIAy3+BAhhiq2g1w
OU87P/vuWQrgl1D/jUEh
=l3HK
-----END PGP SIGNATURE-----

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Linus Walleij]

On Mon, Jan 30, 2012 at 9:28 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 01/30/2012 08:22 PM, Mark Brown wrote:
>> On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
>>
>>> + =A0 =A0mutex_lock(&iio_map_list_lock); + =A0 =A0 =A0 while ((maps[i].=
consumer_dev
>>> !=3D NULL) || + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (maps[i].consumer_dev_name =
!=3D NULL)) {
>>
>> I'd suggest just dropping the struct device - the reason we support
>> the struct device directly in the regulator API is that we
>> originally had only a struct device and kept the code around as a
>> transition measure (though now it's so old we should be able to
>> kill it off). =A0This would simplify the code and the interface a
>> bit.
>
> Ah, that explains your comment on the 5th patch. =A0Alright, I'll let thi=
s
> sit for a few days and if no one comes up with a good reason not to
> we'll go with just the dev_name option.

I've just deleted the yse of struct device * from the pin control
subsystem and I bet ark will have it deleted from regulator_consumer
before long, so just let it go :-)

Thanks,
Linus Walleij

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Jonathan Cameron]

On 02/01/2012 07:58 PM, Linus Walleij wrote:
> On Mon, Jan 30, 2012 at 9:28 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 01/30/2012 08:22 PM, Mark Brown wrote:
>>> On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
>>>
>>>> +    mutex_lock(&iio_map_list_lock); +       while ((maps[i].consumer_dev
>>>> != NULL) || +               (maps[i].consumer_dev_name != NULL)) {
>>>
>>> I'd suggest just dropping the struct device - the reason we support
>>> the struct device directly in the regulator API is that we
>>> originally had only a struct device and kept the code around as a
>>> transition measure (though now it's so old we should be able to
>>> kill it off).  This would simplify the code and the interface a
>>> bit.
>>
>> Ah, that explains your comment on the 5th patch.  Alright, I'll let this
>> sit for a few days and if no one comes up with a good reason not to
>> we'll go with just the dev_name option.
> 
> I've just deleted the yse of struct device * from the pin control
> subsystem and I bet ark will have it deleted from regulator_consumer
> before long, so just let it go :-)
> 
struct device * use removed from IIO, but I won't be posting just yet
as something has borked my i2c that'll need tracking down before I can
test an updated version.

For reference in the meantime, the other change is that the
iio_device_unregister sets the info pointer to NULL, thus providing
a convenient way of knowing the parent device has gone away without
i think allowing for any raceconditions (this will happen before the
bus unregister which was what was causing me trouble).

Jonathan

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Greg KH]

On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Lifted from proposal for in kernel interface built on the out of staging
> branch.
> 
> Two elements here:
> * Map as defined in "inkern.h"
> * Matching code to actually get the iio_dev and channel
> that we want from the global list of IIO devices.
> 
> V2: As per Greg KH suggestion, move over to registration by passing
> the tables into the provider drivers (how regulator does it).
> This does not prevent us using the original more flexible approach
> if at a later date there is a usecase that demands it.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/Kconfig             |    7 +-
>  drivers/staging/iio/Makefile            |    2 +-
>  drivers/staging/iio/consumer.h          |  103 +++++++++++++
>  drivers/staging/iio/driver.h            |   34 ++++
>  drivers/staging/iio/iio_core.h          |    3 +
>  drivers/staging/iio/industrialio-core.c |    2 +-
>  drivers/staging/iio/inkern.c            |  256 +++++++++++++++++++++++++++++++
>  drivers/staging/iio/machine.h           |   30 ++++
>  8 files changed, 434 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> index 90162aa..65c2a8e 100644
> --- a/drivers/staging/iio/Kconfig
> +++ b/drivers/staging/iio/Kconfig
> @@ -10,8 +10,14 @@ menuconfig IIO
>  	  drivers for many different types of embedded sensors using a
>  	  number of different physical interfaces (i2c, spi, etc). See
>  	  drivers/staging/iio/Documentation for more information.
> +
>  if IIO
>  
> +config IIO_INKERN
> +	bool "In kernel support for IIO"
> +	help
> +	  Support in kernel users of IIO device drivers.

Of course you want this, all of the code in the kernel.org tree is "in
kernel users" :)

Seriously, I still fail to understand what is so special here that makes
this a totally different design pattern from all other busses,
especially when you aren't even a bus at all, but rather a "interface"
to userspace for different device types.

Why can't a driver just depend on this interface type, like all other
interface types are, that way the built-in vs. as-a-module issues are
all handled "automagically" by the config and build system.

confused,

greg k-h

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Jonathan Cameron]

On 02/09/2012 06:10 PM, Greg KH wrote:
> On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
>> From: Jonathan Cameron <jic23@cam.ac.uk>
>>
>> Lifted from proposal for in kernel interface built on the out of staging
>> branch.
>>
>> Two elements here:
>> * Map as defined in "inkern.h"
>> * Matching code to actually get the iio_dev and channel
>> that we want from the global list of IIO devices.
>>
>> V2: As per Greg KH suggestion, move over to registration by passing
>> the tables into the provider drivers (how regulator does it).
>> This does not prevent us using the original more flexible approach
>> if at a later date there is a usecase that demands it.
>>
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>>  drivers/staging/iio/Kconfig             |    7 +-
>>  drivers/staging/iio/Makefile            |    2 +-
>>  drivers/staging/iio/consumer.h          |  103 +++++++++++++
>>  drivers/staging/iio/driver.h            |   34 ++++
>>  drivers/staging/iio/iio_core.h          |    3 +
>>  drivers/staging/iio/industrialio-core.c |    2 +-
>>  drivers/staging/iio/inkern.c            |  256 +++++++++++++++++++++++++++++++
>>  drivers/staging/iio/machine.h           |   30 ++++
>>  8 files changed, 434 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
>> index 90162aa..65c2a8e 100644
>> --- a/drivers/staging/iio/Kconfig
>> +++ b/drivers/staging/iio/Kconfig
>> @@ -10,8 +10,14 @@ menuconfig IIO
>>  	  drivers for many different types of embedded sensors using a
>>  	  number of different physical interfaces (i2c, spi, etc). See
>>  	  drivers/staging/iio/Documentation for more information.
>> +
>>  if IIO
>>  
>> +config IIO_INKERN
>> +	bool "In kernel support for IIO"
>> +	help
>> +	  Support in kernel users of IIO device drivers.
> 
> Of course you want this, all of the code in the kernel.org tree is "in
> kernel users" :)
> 
> Seriously, I still fail to understand what is so special here that makes
> this a totally different design pattern from all other busses,
> especially when you aren't even a bus at all, but rather a "interface"
> to userspace for different device types.
> 
> Why can't a driver just depend on this interface type, like all other
> interface types are, that way the built-in vs. as-a-module issues are
> all handled "automagically" by the config and build system.
> 
Best simple comparison is with regulators.  They provide voltages to
consumers; This iio stuff is about providing readings of voltage
(simplifying massively) to other users.  All these adc's need
their own driver to provide that voltage to those that care.
The client drivers just want to ask 'what is the voltage?'.

So to do it as a conventional module dependency we need a separate
dependency for every IIO device capable of providing a voltage
to every driver that might want to read one.

There are a number of common use cases or which I'll list a few here.
a) Soc adc's.  Typical soc might have an 8 channel adc.  On a typical
board 2 channels are used for input (touchscreen), 2 for monitoring the
battery and
4 brought out to an edge connector.  Which are used for what and how
many of each are used is entirely under control of the board designer.
Thus we could have each SoC type adc driver handle the logic necessary
to register an input device, a hwmon device (and perhaps an IIO device).
This is the usecase that Mark Brown originally brought up.  Right now
Arnd is blocking some SoC adc drivers that do this internally and
sending them in the direction of IIO.

b) High spec devices used for input.  This one we originally proposed
doing via a bridge in userspace and piping back in via uinput.
When I say high spec I meant 300+ dolar imu's etc.  People do it
for niche systems, but the part will never primarily be used for this
so normally sits better in IIO, If we have borderline devices, they
will end up in IIO.  If this stuff had been in place earlier (as
Dmitry pushed for) some of the existing parts in input might
never have been accepted.

c) The other side is generating signals.  People use arbitary waveform
generators to produce inputs for all sort of strange devices.

So the question was whether to sit another subsystem under hwmon, input
IIO etc, or to have IIO evolve into that underlying subsystem.  Given
with the channel mapping stuff we already had all the infrastructure
in place to do this, a vague consensus said do it in IIO.

Now in many ways we'd love to have the IIO userspace stuff act as
just another client, but that's a way off yet basically due to the huge
number of special cases that would need to be handled.  I'm personally
not yet sure that will ever happen.

So in summary we don't do this as a simple dependency because of the
combinatorial explosion in dependencies that would be needed and the
connection modules that would be needed to glue it all together
vs what is a simple mapping that will sit nicely in device tree etc.

Jonathan

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Greg KH]

On Thu, Feb 09, 2012 at 06:34:34PM +0000, Jonathan Cameron wrote:
> On 02/09/2012 06:10 PM, Greg KH wrote:
> > On Sun, Jan 29, 2012 at 11:46:50AM +0000, Jonathan Cameron wrote:
> >> From: Jonathan Cameron <jic23@cam.ac.uk>
> >>
> >> Lifted from proposal for in kernel interface built on the out of staging
> >> branch.
> >>
> >> Two elements here:
> >> * Map as defined in "inkern.h"
> >> * Matching code to actually get the iio_dev and channel
> >> that we want from the global list of IIO devices.
> >>
> >> V2: As per Greg KH suggestion, move over to registration by passing
> >> the tables into the provider drivers (how regulator does it).
> >> This does not prevent us using the original more flexible approach
> >> if at a later date there is a usecase that demands it.
> >>
> >> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> >> ---
> >>  drivers/staging/iio/Kconfig             |    7 +-
> >>  drivers/staging/iio/Makefile            |    2 +-
> >>  drivers/staging/iio/consumer.h          |  103 +++++++++++++
> >>  drivers/staging/iio/driver.h            |   34 ++++
> >>  drivers/staging/iio/iio_core.h          |    3 +
> >>  drivers/staging/iio/industrialio-core.c |    2 +-
> >>  drivers/staging/iio/inkern.c            |  256 +++++++++++++++++++++++++++++++
> >>  drivers/staging/iio/machine.h           |   30 ++++
> >>  8 files changed, 434 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> >> index 90162aa..65c2a8e 100644
> >> --- a/drivers/staging/iio/Kconfig
> >> +++ b/drivers/staging/iio/Kconfig
> >> @@ -10,8 +10,14 @@ menuconfig IIO
> >>  	  drivers for many different types of embedded sensors using a
> >>  	  number of different physical interfaces (i2c, spi, etc). See
> >>  	  drivers/staging/iio/Documentation for more information.
> >> +
> >>  if IIO
> >>  
> >> +config IIO_INKERN
> >> +	bool "In kernel support for IIO"
> >> +	help
> >> +	  Support in kernel users of IIO device drivers.
> > 
> > Of course you want this, all of the code in the kernel.org tree is "in
> > kernel users" :)
> > 
> > Seriously, I still fail to understand what is so special here that makes
> > this a totally different design pattern from all other busses,
> > especially when you aren't even a bus at all, but rather a "interface"
> > to userspace for different device types.
> > 
> > Why can't a driver just depend on this interface type, like all other
> > interface types are, that way the built-in vs. as-a-module issues are
> > all handled "automagically" by the config and build system.
> > 
> Best simple comparison is with regulators.  They provide voltages to
> consumers; This iio stuff is about providing readings of voltage
> (simplifying massively) to other users.  All these adc's need
> their own driver to provide that voltage to those that care.
> The client drivers just want to ask 'what is the voltage?'.
> 
> So to do it as a conventional module dependency we need a separate
> dependency for every IIO device capable of providing a voltage
> to every driver that might want to read one.
> 
> There are a number of common use cases or which I'll list a few here.
> a) Soc adc's.  Typical soc might have an 8 channel adc.  On a typical
> board 2 channels are used for input (touchscreen), 2 for monitoring the
> battery and
> 4 brought out to an edge connector.  Which are used for what and how
> many of each are used is entirely under control of the board designer.
> Thus we could have each SoC type adc driver handle the logic necessary
> to register an input device, a hwmon device (and perhaps an IIO device).
> This is the usecase that Mark Brown originally brought up.  Right now
> Arnd is blocking some SoC adc drivers that do this internally and
> sending them in the direction of IIO.
> 
> b) High spec devices used for input.  This one we originally proposed
> doing via a bridge in userspace and piping back in via uinput.
> When I say high spec I meant 300+ dolar imu's etc.  People do it
> for niche systems, but the part will never primarily be used for this
> so normally sits better in IIO, If we have borderline devices, they
> will end up in IIO.  If this stuff had been in place earlier (as
> Dmitry pushed for) some of the existing parts in input might
> never have been accepted.
> 
> c) The other side is generating signals.  People use arbitary waveform
> generators to produce inputs for all sort of strange devices.
> 
> So the question was whether to sit another subsystem under hwmon, input
> IIO etc, or to have IIO evolve into that underlying subsystem.  Given
> with the channel mapping stuff we already had all the infrastructure
> in place to do this, a vague consensus said do it in IIO.
> 
> Now in many ways we'd love to have the IIO userspace stuff act as
> just another client, but that's a way off yet basically due to the huge
> number of special cases that would need to be handled.  I'm personally
> not yet sure that will ever happen.
> 
> So in summary we don't do this as a simple dependency because of the
> combinatorial explosion in dependencies that would be needed and the
> connection modules that would be needed to glue it all together
> vs what is a simple mapping that will sit nicely in device tree etc.

So, would the "keep looping until all dependancies are met" patches from
Grant that have been floating around for a while, solve this issue?
Modules would properly initialize only when the needed other bits are
present, which solves the problems the regulators and other drivers have
today.

If so, then I suggest we get that code into the tree soon, and then all
of this "wierd" logic would go away, right?

thanks,

greg k-h

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Thu, Feb 09, 2012 at 10:57:54AM -0800, Greg KH wrote:

> So, would the "keep looping until all dependancies are met" patches from
> Grant that have been floating around for a while, solve this issue?
> Modules would properly initialize only when the needed other bits are
> present, which solves the problems the regulators and other drivers have
> today.

> If so, then I suggest we get that code into the tree soon, and then all
> of this "wierd" logic would go away, right?

Could you identify the logic you're talking about here?  Looking at the
last patch set I can find[1] this all looks very vanilla for this sort of
subsystem?  You've got a mapping mechanism and a get function but that's
about it which is all pretty normal.  Grant's changes make module
probing nicer in that you don't need to worry about the order in which
things get registered but I can't see anything here which is
particularly related to that ordering.

All the code is doing is providing a mechanism for drivers that need IIO
facilities to locate the thing that's providing their IIO facilities
(which is in the general case going to be board specific on a totally
unrelated chip in a different part of the system and discovered through
reading the schematic) without requiring custom code in every single
single IIO using device to do the mapping.  Instead the boards use a
standard subsystem facility to tell the IIO core what's wired where and
then the core matches things up for them.

This is nothing to do with init ordering, it's all about the two devices
finding each other at all.

[1] http://article.gmane.org/gmane.linux.kernel.iio/3287

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Greg KH]

On Thu, Feb 09, 2012 at 07:15:10PM +0000, Mark Brown wrote:
> On Thu, Feb 09, 2012 at 10:57:54AM -0800, Greg KH wrote:
> 
> > So, would the "keep looping until all dependancies are met" patches from
> > Grant that have been floating around for a while, solve this issue?
> > Modules would properly initialize only when the needed other bits are
> > present, which solves the problems the regulators and other drivers have
> > today.
> 
> > If so, then I suggest we get that code into the tree soon, and then all
> > of this "wierd" logic would go away, right?
> 
> Could you identify the logic you're talking about here?  Looking at the
> last patch set I can find[1] this all looks very vanilla for this sort of
> subsystem?  You've got a mapping mechanism and a get function but that's
> about it which is all pretty normal.  Grant's changes make module
> probing nicer in that you don't need to worry about the order in which
> things get registered but I can't see anything here which is
> particularly related to that ordering.
> 
> All the code is doing is providing a mechanism for drivers that need IIO
> facilities to locate the thing that's providing their IIO facilities
> (which is in the general case going to be board specific on a totally
> unrelated chip in a different part of the system and discovered through
> reading the schematic) without requiring custom code in every single
> single IIO using device to do the mapping.  Instead the boards use a
> standard subsystem facility to tell the IIO core what's wired where and
> then the core matches things up for them.
> 
> This is nothing to do with init ordering, it's all about the two devices
> finding each other at all.

Ok, if that's all this is for, they why name it "iio in-kernel logic"?
That made me believe that this is something that is needed if anyone who
happens to want to use the iio interface needs it.

If it's just "iio discovery" then call it that :)

thanks,

greg k-h

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Thu, Feb 09, 2012 at 11:17:18AM -0800, Greg KH wrote:
> On Thu, Feb 09, 2012 at 07:15:10PM +0000, Mark Brown wrote:

> > This is nothing to do with init ordering, it's all about the two devices
> > finding each other at all.

> Ok, if that's all this is for, they why name it "iio in-kernel logic"?
> That made me believe that this is something that is needed if anyone who
> happens to want to use the iio interface needs it.

> If it's just "iio discovery" then call it that :)

I think that's all due to the overall goal of the patch series - the
original use case for IIO was purely for userspace users and what the
patch series is doing is adding the ability to use IIO devices from
within the kernel.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/09/2012 07:20 PM, Mark Brown wrote:
> On Thu, Feb 09, 2012 at 11:17:18AM -0800, Greg KH wrote:
>> On Thu, Feb 09, 2012 at 07:15:10PM +0000, Mark Brown wrote:
> 
>>> This is nothing to do with init ordering, it's all about the
>>> two devices finding each other at all.
> 
>> Ok, if that's all this is for, they why name it "iio in-kernel
>> logic"? That made me believe that this is something that is
>> needed if anyone who happens to want to use the iio interface
>> needs it.
> 
>> If it's just "iio discovery" then call it that :)
> 
> I think that's all due to the overall goal of the patch series -
> the original use case for IIO was purely for userspace users and
> what the patch series is doing is adding the ability to use IIO
> devices from within the kernel.
That was my thinking.  Happy to take suggestions for clearer naming!

Jonathan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPNDilAAoJEFSFNJnE9BaI4wMP/21QLUcAzWgrlK+Vv5c8TAvf
pXDQ2iZgtaA/JcTVpTh6VwI4cXJYI4Oq/MrokZaLJBS1avx9QEiyHjoXS8khLyi0
hKU0J5xgG61y6h/MMugqt06O4rIuuByOSuk7uWAvv9TN19ukVDsYG84Ct44XJeKb
MfOVPm/YDRpHMAOvWPgEGJAfa3CcYPTf6E5MBsjcx2+kLxkzvNwpA5LAw+nLwq+q
wEYNubsgp2U16S+Nf9Sqwhzzrg7UDbqOwWT/VkU1M9g5VtGcFYrAMhrnK36dTSXj
Rkl1o7UIW6tRWggIvQ9TInYS+PMybumdGNL3zjxJcTCfGoxeGlMH5mGsviCIMDrg
exDRyAoE6fiT6RxFe8OADwBuDunk5RQcZRq9LIC+lH68qq9n3FrqerCBue52yVL5
fzKgsQSlOfpvbSQPqGZh76xKSEXpANIJD8cLSaa6spO5QmwzK6hRtExWD84mtcgB
k7s+G9vEoHo7CeaLZbwdbsdk3CZ3+PXgtWDOhx6+e4JhIoIR7UneFhJBw6jlkSFf
9a4cr6rW48M4Vh8Z3u6rsMl+ZJbiD5LXfKNtGhmOKPGHtEsxYCuJk3HXZO+voE0q
44RlIO0ijt8bV0xJcxSWI3+ArIhHEN9bW/8Zx8ELUoFA+Rs29SUAlHhEL0XGoHKZ
H3RbDS7Cj9ji4gVxzocn
=hQ9x
-----END PGP SIGNATURE-----

Re: [PATCH 1/5] staging:iio:core add in kernel interface mapping and getting IIO channels.

[Linus Walleij]

On Thu, Feb 9, 2012 at 10:20 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> [Mark]
>> the original use case for IIO was purely for userspace users and
>> what the patch series is doing is adding the ability to use IIO
>> devices from within the kernel.
>
> That was my thinking. =A0Happy to take suggestions for clearer naming!

I would suggest not adding a config option at all but default-enable
the in-kernel interface. To me that part is certainly the key component,
userspace IIO to me is just some obscurity ...

Yours,
Linus Walleij

[PATCH 2/5] staging:iio: move iio data return types into types.h for use by inkern

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

In kernel interfaces need these, so make them available.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/iio.h   |    6 ------
 drivers/staging/iio/types.h |    4 ++++
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index be6ced3..1e1f5a0 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -197,12 +197,6 @@ static inline s64 iio_get_time_ns(void)
 #define INDIO_ALL_BUFFER_MODES					\
 	(INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_HARDWARE)
 
-/* Vast majority of this is set by the industrialio subsystem on a
- * call to iio_device_register. */
-#define IIO_VAL_INT 1
-#define IIO_VAL_INT_PLUS_MICRO 2
-#define IIO_VAL_INT_PLUS_NANO 3
-
 struct iio_trigger; /* forward declaration */
 struct iio_dev;
 
diff --git a/drivers/staging/iio/types.h b/drivers/staging/iio/types.h
index b7d2647..0c32136 100644
--- a/drivers/staging/iio/types.h
+++ b/drivers/staging/iio/types.h
@@ -46,4 +46,8 @@ enum iio_modifier {
 	IIO_MOD_LIGHT_IR,
 };
 
+#define IIO_VAL_INT 1
+#define IIO_VAL_INT_PLUS_MICRO 2
+#define IIO_VAL_INT_PLUS_NANO 3
+
 #endif /* _IIO_TYPES_H_ */
-- 
1.7.8.4

[PATCH 3/5] staging:iio::hwmon interface client driver.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Direct copy of version proposed for the non staging branch.
Needed here to allow testing of more advanced inkernel
interface code.

Minimal support of simple in, curr and temp attributes
so far.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/Kconfig     |    9 ++
 drivers/staging/iio/Makefile    |    2 +
 drivers/staging/iio/iio_hwmon.c |  226 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index 65c2a8e..dab0189 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -18,6 +18,15 @@ config IIO_INKERN
 	help
 	  Support in kernel users of IIO device drivers.
 
+config IIO_ST_HWMON
+	tristate "Hwmon driver that uses channels specified via iio maps"
+	depends on HWMON
+	depends on IIO_INKERN
+	help
+	  This is a platform driver that in combination with a suitable
+	  map allows IIO devices to provide  basic hwmon functionality
+	  for those channels specified in the map.
+
 config IIO_BUFFER
 	bool "Enable buffer support within IIO"
 	help
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index 5da4c78..3ca3ffb 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -17,6 +17,8 @@ iio_dummy-$(CONFIG_IIO_SIMPLE_DUMMY_BUFFER) += iio_simple_dummy_buffer.o
 
 obj-$(CONFIG_IIO_DUMMY_EVGEN) += iio_dummy_evgen.o
 
+obj-$(CONFIG_IIO_ST_HWMON) += iio_hwmon.o
+
 obj-y += accel/
 obj-y += adc/
 obj-y += addac/
diff --git a/drivers/staging/iio/iio_hwmon.c b/drivers/staging/iio/iio_hwmon.c
new file mode 100644
index 0000000..17deb0b
--- /dev/null
+++ b/drivers/staging/iio/iio_hwmon.c
@@ -0,0 +1,226 @@
+/* Hwmon client for industrial I/O devices
+ *
+ * Copyright (c) 2011 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include "consumer.h"
+#include "types.h"
+
+/**
+ * struct iio_hwmon_state - device instance state
+ * @channels:		filled with array of channels from iio
+ * @num_channels:	number of channels in channels (saves counting twice)
+ * @hwmon_dev:		associated hwmon device
+ * @attr_group:	the group of attributes
+ * @attrs:		null terminated array of attribute pointers.
+ */
+struct iio_hwmon_state {
+	struct iio_channel *channels;
+	int num_channels;
+	struct device *hwmon_dev;
+	struct attribute_group attr_group;
+	struct attribute **attrs;
+};
+
+/*
+ * Assumes that IIO and hwmon operate in the same base units.
+ * This is supposed to be true, but needs verification for
+ * new channel types.
+ */
+static ssize_t iio_hwmon_read_val(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	long result;
+	int val, ret, scaleint, scalepart;
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	struct iio_hwmon_state *state = dev_get_drvdata(dev);
+
+	/*
+	 * No locking between this pair, so theoretically possible
+	 * the scale has changed.
+	 */
+	ret = iio_st_read_channel_raw(&state->channels[sattr->index],
+				      &val);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_st_read_channel_scale(&state->channels[sattr->index],
+					&scaleint, &scalepart);
+	if (ret < 0)
+		return ret;
+	switch (ret) {
+	case IIO_VAL_INT:
+		result = val * scaleint;
+		break;
+	case IIO_VAL_INT_PLUS_MICRO:
+		result = (s64)val * (s64)scaleint +
+			div_s64((s64)val * (s64)scalepart, 1000000LL);
+		break;
+	case IIO_VAL_INT_PLUS_NANO:
+		result = (s64)val * (s64)scaleint +
+			div_s64((s64)val * (s64)scalepart, 1000000000LL);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return sprintf(buf, "%ld\n", result);
+}
+
+static void iio_hwmon_free_attrs(struct iio_hwmon_state *st)
+{
+	int i;
+	struct sensor_device_attribute *a;
+	for (i = 0; i < st->num_channels; i++)
+		if (st->attrs[i]) {
+			a = to_sensor_dev_attr(
+				container_of(st->attrs[i],
+					     struct device_attribute,
+					     attr));
+			kfree(a);
+		}
+}
+
+static int __devinit iio_hwmon_probe(struct platform_device *pdev)
+{
+	struct iio_hwmon_state *st;
+	struct sensor_device_attribute *a;
+	int ret, i;
+	int in_i = 1, temp_i = 1, curr_i = 1;
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (st == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	st->channels = iio_st_channel_get_all(&pdev->dev, NULL);
+	if (IS_ERR(st->channels)) {
+		ret = PTR_ERR(st->channels);
+		goto error_free_state;
+	}
+
+	/* count how many attributes we have */
+	while (st->channels[st->num_channels].indio_dev)
+		st->num_channels++;
+
+	st->attrs = kzalloc(sizeof(st->attrs) * (st->num_channels + 1),
+			    GFP_KERNEL);
+	if (st->attrs == NULL) {
+		ret = -ENOMEM;
+		goto error_release_channels;
+	}
+	for (i = 0; i < st->num_channels; i++) {
+		a = kzalloc(sizeof(*a), GFP_KERNEL);
+		if (a == NULL) {
+			ret = -ENOMEM;
+			goto error_free_attrs;
+		}
+
+		sysfs_attr_init(&a->dev_attr.attr);
+		switch (iio_st_get_channel_type(&st->channels[i])) {
+		case IIO_VOLTAGE:
+			a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+							  "in%d_input",
+							  in_i++);
+			break;
+		case IIO_TEMP:
+			a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+							  "temp%d_input",
+							  temp_i++);
+			break;
+		case IIO_CURRENT:
+			a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+							  "curr%d_input",
+							  curr_i++);
+			break;
+		default:
+			ret = -EINVAL;
+			kfree(a);
+			goto error_free_attrs;
+		}
+		if (a->dev_attr.attr.name == NULL) {
+			kfree(a);
+			ret = -ENOMEM;
+			goto error_free_attrs;
+		}
+		a->dev_attr.show = iio_hwmon_read_val;
+		a->dev_attr.attr.mode = S_IRUGO;
+		a->index = i;
+		st->attrs[i] = &a->dev_attr.attr;
+	}
+
+	st->attr_group.attrs = st->attrs;
+	platform_set_drvdata(pdev, st);
+	ret = sysfs_create_group(&pdev->dev.kobj, &st->attr_group);
+	if (ret < 0)
+		goto error_free_attrs;
+
+	st->hwmon_dev = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(st->hwmon_dev)) {
+		ret = PTR_ERR(st->hwmon_dev);
+		goto error_remove_group;
+	}
+	return 0;
+
+error_remove_group:
+	sysfs_remove_group(&pdev->dev.kobj, &st->attr_group);
+error_free_attrs:
+	iio_hwmon_free_attrs(st);
+	kfree(st->attrs);
+error_release_channels:
+	iio_st_channel_release_all(st->channels);
+error_free_state:
+	kfree(st);
+error_ret:
+	return ret;
+}
+
+static int __devexit iio_hwmon_remove(struct platform_device *pdev)
+{
+	struct iio_hwmon_state *st = platform_get_drvdata(pdev);
+
+	hwmon_device_unregister(st->hwmon_dev);
+	sysfs_remove_group(&pdev->dev.kobj, &st->attr_group);
+	iio_hwmon_free_attrs(st);
+	kfree(st->attrs);
+	iio_st_channel_release_all(st->channels);
+
+	return 0;
+}
+
+static struct platform_driver __refdata iio_hwmon_driver = {
+	.driver = {
+		.name = "iio_hwmon",
+		.owner = THIS_MODULE,
+	},
+	.probe = iio_hwmon_probe,
+	.remove = __devexit_p(iio_hwmon_remove),
+};
+
+static int iio_inkern_init(void)
+{
+	return platform_driver_register(&iio_hwmon_driver);
+}
+module_init(iio_inkern_init);
+
+static void iio_inkern_exit(void)
+{
+	platform_driver_unregister(&iio_hwmon_driver);
+}
+module_exit(iio_inkern_exit);
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>");
+MODULE_DESCRIPTION("IIO to hwmon driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.8.4

[PATCH 4/5] staging:iio:Documentation in kernel pull description.

[Jonathan Cameron]

Very basic description of the way iio consumers work and how to use
this functionality.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/Documentation/inkernel.txt |   59 ++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/Documentation/inkernel.txt b/drivers/staging/iio/Documentation/inkernel.txt
new file mode 100644
index 0000000..b9a4d0a
--- /dev/null
+++ b/drivers/staging/iio/Documentation/inkernel.txt
@@ -0,0 +1,59 @@
+Industrial I/O Subsystemm in kernel consumers.
+
+The IIO subsystem can act as a layer under other elements of the kernel
+providing a means of obtaining adc type readings or of driving dac type
+signals.  The functionality supported will grow as use cases arise.
+
+Describing the channel mapping (iio/machine.h)
+
+Channel associations are described using:
+
+struct iio_map {
+	const char *adc_channel_label;
+	struct device *consumer_dev;
+	const char *consumer_dev_name;
+	const char *consumer_channel;
+};
+
+adc_channel label identifies the channel on the IIO device by being
+matched against the datasheet_name field of the iio_chan_spec.
+
+consumer_dev or consumer_dev_name allow identification of the consumer device.
+These are then used to find the channel mapping from the consumer device (see
+below).
+
+Finally consumer_channel is a string identifying the channel to the consumer.
+(Perhaps 'battery_voltage' or similar).
+
+An array of these structures is then passed to the IIO driver.
+
+Supporting in kernel interfaces in the driver (driver.h)
+
+The driver must provide datasheet_name values for its channels and
+must pass the iio_map structures and a pointer to its own iio_dev structure
+ on to the core via a call to iio_map_array_register.  On removal,
+iio_map_array_unregister reverses this process.
+
+The result of this is that the IIO core now has all the information needed
+to associate a given channel with the consumer requesting it.
+
+Acting as an IIO consumer (consumer.h)
+
+The consumer first has to obtain an iio_channel structure from the core
+by calling iio_channel_get().  The correct channel is identified by:
+
+* matching dev or dev_name against consumer_dev and consumer_dev_name
+* matching consumer_channel against consumer_channel in the map
+
+There are then a number of functions that can be used to get information
+about this channel such as it's current reading.
+
+e.g.
+iio_st_read_channel_raw() - get a reading
+iio_st_read_channel_type() - get the type of channel
+
+There is also provision for retrieving all of the channels associated
+with a given consumer.  This is useful for generic drivers such as
+iio_hwmon where the number and naming of channels is not known by the
+consumer driver.  To do this, use iio_st_channel_get_all.
+
-- 
1.7.8.4

[PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Do not commit.
---
 arch/arm/mach-pxa/stargate2.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
index b0656e15..d2d9a27 100644
--- a/arch/arm/mach-pxa/stargate2.c
+++ b/arch/arm/mach-pxa/stargate2.c
@@ -54,6 +54,8 @@
 #include <linux/mfd/da903x.h>
 #include <linux/sht15.h>
 
+#include "../../../drivers/staging/iio/machine.h"
+
 #include "devices.h"
 #include "generic.h"
 
@@ -406,6 +408,23 @@ static struct i2c_pxa_platform_data i2c_pdata = {
 	.fast_mode = 1,
 };
 
+static struct platform_device iio_hwmon_test = {
+	.name = "iio_hwmon",
+};
+
+static struct iio_map max1363_consumer_map[] = {
+	{
+		.adc_channel_label = "AIN1",
+		.consumer_dev = &iio_hwmon_test.dev,
+		.consumer_channel = "testchan1",
+	}, {
+		.adc_channel_label = "AIN2",
+		.consumer_dev = &iio_hwmon_test.dev,
+		.consumer_channel = "testchan2",
+	},
+	{}
+};
+
 static void __init imote2_stargate2_init(void)
 {
 
@@ -561,6 +580,7 @@ static struct i2c_board_info __initdata imote2_i2c_board_info[] = {
 		 * pull up resistors are missing.
 		 */
 		.irq = PXA_GPIO_TO_IRQ(99),
+		.platform_data = max1363_consumer_map,
 	}, { /* ITS400 Sensor board only */
 		.type = "tsl2561",
 		.addr = 0x49,
@@ -945,6 +965,7 @@ static struct i2c_board_info __initdata stargate2_i2c_board_info[] = {
 		 * pull up resistors are missing.
 		 */
 		.irq = PXA_GPIO_TO_IRQ(99),
+		.platform_data = max1363_consumer_map,
 	}, { /* ITS400 Sensor board only */
 		.type = "tsl2561",
 		.addr = 0x49,
@@ -977,6 +998,7 @@ static struct platform_device *stargate2_devices[] = {
 	&stargate2_sram,
 	&smc91x_device,
 	&sht15,
+	&iio_hwmon_test,
 };
 
 static void __init stargate2_init(void)
-- 
1.7.8.4

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Sun, Jan 29, 2012 at 11:46:54AM +0000, Jonathan Cameron wrote:

> +static struct iio_map max1363_consumer_map[] = {
> +	{
> +		.adc_channel_label = "AIN1",
> +		.consumer_dev = &iio_hwmon_test.dev,
> +		.consumer_channel = "testchan1",

I do think it's better to use dev_name here rather than a struct device
pointer - for several of the buses it's not actually possible to get a
struct device until a device has been instantiated which isn't helpful
for setting up the mappings.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/30/2012 07:33 PM, Mark Brown wrote:
> On Sun, Jan 29, 2012 at 11:46:54AM +0000, Jonathan Cameron wrote:
> 
>> +static struct iio_map max1363_consumer_map[] = { +	{ +
>> .adc_channel_label = "AIN1", +		.consumer_dev =
>> &iio_hwmon_test.dev, +		.consumer_channel = "testchan1",
> 
> I do think it's better to use dev_name here rather than a struct
> device pointer - for several of the buses it's not actually
> possible to get a struct device until a device has been
> instantiated which isn't helpful for setting up the mappings.
We allow both.  In cases like this where the dev pointer is explicitly
available what gain do we get from not using it?

I could easily enough fake an example for using dev_name as well I
suppose or add a commented out version in here to make it obvious that
both options would be fine.  Note this example is actually garbage
anyway as that particular chip is simply wired to some screw terminals
on the side of this board.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPJvy+AAoJEFSFNJnE9BaI8GwQAIRQ2MP+UhVUGiPVQpTIVxIV
B+3GvoDhdF8GEaWLMb5jXS6p1kQ9m1IxSfTfzBPa/lLDMpZMH72FD2nguNFID0qK
aRu2zv7ooEAdhBMPYJJC9XUlq1vl7Mas82zH8zK2z/iIuIUJTJlO/l+vgpu+EhFJ
H5DODW9oRuWpbNMbpTKExf+fyMRkp/o+0It++Zk4eJPs4q1ajAYnDpDeIeUVyyIM
5OTQSvrGVL8uRtHhcqaJkVNUizhR+eMgDYef25dgTO2CnKjoO5q3CjikcoG6RYrQ
tVvZw0slGULry2jGcf1K/scxiuIjCtYGIPFkeorxtAow6JB3QRvQP01JYa+4gpb1
HSp0dkvmUF4V0JS52bxzp788C9hpAG7Vd7Mul3M+7BuwYxAOYgD8eL5tfuQWPaCJ
nN/YlXUDw1Oy3obBKZvma1o18zaQvclF74FIAw4AMz/8r8QC3vcpvUqR58YP7B2Z
qYheZKIIII4kZmG09QcYVjXB3O1IvP9B9EN/QPXYTp8vHQ55wVPA6ZQQzpeSnDeC
5PW0TaNMqroYdb2BuPZ4+gVNt3rqmDNTaexxdgWJtHAKiukh6EbNEfEsTk/LTOHn
s+jTnpni3h+z+tgTRvxgGmW40EyUlrmnR/c0vBohvcknOI1ugrViRr88RCsSX01S
Wv/kaVXUcT5visYRcHls
=Z0kq
-----END PGP SIGNATURE-----

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

On Mon, Jan 30, 2012 at 08:26:10PM +0000, Jonathan Cameron wrote:
> On 01/30/2012 07:33 PM, Mark Brown wrote:
> > On Sun, Jan 29, 2012 at 11:46:54AM +0000, Jonathan Cameron wrote:

> >> +static struct iio_map max1363_consumer_map[] = { +	{ +
> >> .adc_channel_label = "AIN1", +		.consumer_dev =
> >> &iio_hwmon_test.dev, +		.consumer_channel = "testchan1",

> > I do think it's better to use dev_name here rather than a struct
> > device pointer - for several of the buses it's not actually
> > possible to get a struct device until a device has been
> > instantiated which isn't helpful for setting up the mappings.

> We allow both.  In cases like this where the dev pointer is explicitly
> available what gain do we get from not using it?

You avoid user confusion from having two ways of doing the same thing
and you save a little complexity in the implementation.  The user
confusion does happen in practice.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/30/2012 09:22 PM, Mark Brown wrote:
> On Mon, Jan 30, 2012 at 08:26:10PM +0000, Jonathan Cameron wrote:
>> On 01/30/2012 07:33 PM, Mark Brown wrote:
>>> On Sun, Jan 29, 2012 at 11:46:54AM +0000, Jonathan Cameron
>>> wrote:
> 
>>>> +static struct iio_map max1363_consumer_map[] = { +	{ + 
>>>> .adc_channel_label = "AIN1", +		.consumer_dev = 
>>>> &iio_hwmon_test.dev, +		.consumer_channel = "testchan1",
> 
>>> I do think it's better to use dev_name here rather than a
>>> struct device pointer - for several of the buses it's not
>>> actually possible to get a struct device until a device has
>>> been instantiated which isn't helpful for setting up the
>>> mappings.
> 
>> We allow both.  In cases like this where the dev pointer is
>> explicitly available what gain do we get from not using it?
> 
> You avoid user confusion from having two ways of doing the same
> thing and you save a little complexity in the implementation.  The
> user confusion does happen in practice.
Good reasoning.  Will drop the dev pointer option.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPJxBHAAoJEFSFNJnE9BaIGUwP/2SQjWhWZP28GWpWemx5Av7O
5CUKvDZ/qk+ehevvp0fOKOD/DvciSNWDd/dTmGRllbYzpz7RkbGmV9sm+BFydqyG
beTzbF5h5vuvCNfvVrXwePh2/fcm3kSMIOYQSnaDmn5rmZTR0qK6nkh1GdBtMa2T
Hbk+qjdQFcrAvQcL1R4FQO+kq2xS/F3zQAt1rqxu4e+JWQjYDmGCK9zeWjJDmakv
2DDeUUezNIA0tjvQy9EUOdybtoKvBlmgTujwCX2qpeyJTll2yxPuWFx8nB7oNGCQ
ljoa9YO6mpRgiAQNpnvOeMLtAwa3EjEZLf1XBRRXUPoAvehcPLdL5C3IaxzpmaTY
r7qXAPWwMPGIj5pTXdP1mQleZiepdKi3oOWygBjG8nKDqcLjuiMBPaE+S6xjIn4O
dseszmeWhOsQgdBr3CpkaqBZrT3NDKcHkaFyThCQCDBIATTEK5LVgGVi9KX3toAe
Emov7mOW+xSTJ2CLtbrmmjRm/mJSCXUVPBurD+LTXJnxOmCipTmd0Gt/s8dzPyd0
uaY4XBSypYjUvakFgazuZ13pgDvfbQe329ShRNzM4CAPboy8q/X1W/JDsZvfJqrj
oo04TKVxjbg44FjzhbxxBIavq+BUIdJMW4/MCt5YRq5rZPHq5E0TAbZ4tQpmvHUJ
I9UZthFWchN/lD2AEteV
=QK3s
-----END PGP SIGNATURE-----

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Linus Walleij]

On Mon, Jan 30, 2012 at 10:22 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Jan 30, 2012 at 08:26:10PM +0000, Jonathan Cameron wrote:
>> On 01/30/2012 07:33 PM, Mark Brown wrote:
>> > On Sun, Jan 29, 2012 at 11:46:54AM +0000, Jonathan Cameron wrote:
>
>> >> +static struct iio_map max1363_consumer_map[] =3D { + =A0 =A0 =A0 =A0=
{ +
>> >> .adc_channel_label =3D "AIN1", + =A0 =A0 =A0 =A0 =A0 =A0 .consumer_de=
v =3D
>> >> &iio_hwmon_test.dev, + =A0 =A0 =A0 =A0 =A0 =A0 .consumer_channel =3D =
"testchan1",
>
>> > I do think it's better to use dev_name here rather than a struct
>> > device pointer - for several of the buses it's not actually
>> > possible to get a struct device until a device has been
>> > instantiated which isn't helpful for setting up the mappings.
>
>> We allow both. =A0In cases like this where the dev pointer is explicitly
>> available what gain do we get from not using it?
>
> You avoid user confusion from having two ways of doing the same thing
> and you save a little complexity in the implementation. =A0The user
> confusion does happen in practice.

The clockdev API is stricter and only allows for names I noticed.

So I guess this pattern comes from the regulator framework
struct regulator_consumer_supply design pattern initially.

We allow both struct device * in regulators and pin muxes.
So, not having it for ADCs could be confusing as well.

But we should then try to rid it from the kernel at large, no big deal,
I have only one in-kernel user of this ATM I think so I can kill it off fro=
m
the pin control API if you think you can kill it from regulators.

I just want to make sure that this is the direction we want to go...

Yours,
Linus Walleij

Re: [PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Tue, Jan 31, 2012 at 09:39:31AM +0100, Linus Walleij wrote:
> On Mon, Jan 30, 2012 at 10:22 PM, Mark Brown

> > You avoid user confusion from having two ways of doing the same thing
> > and you save a little complexity in the implementation.  The user
> > confusion does happen in practice.

> So I guess this pattern comes from the regulator framework
> struct regulator_consumer_supply design pattern initially.

Yes, and the only reason the regulator API does this is historical.

> But we should then try to rid it from the kernel at large, no big deal,
> I have only one in-kernel user of this ATM I think so I can kill it off from
> the pin control API if you think you can kill it from regulators.

Well volunteered :)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH 0/5 V5] IIO: in kernel pull interfaces.

[Linus Walleij]

On Sun, Jan 29, 2012 at 12:46 PM, Jonathan Cameron <jic23@kernel.org> wrote=
:

> Firstly this is an RFC rather than a final proposal because I know there =
is
> an issue with the provider being removed before the consumer. =A0It will =
be easy
> to make this less likely to happen, but I am not entirely sure how to avo=
id
> any possible race (suggestion welcome).

I basically think this looks good... :-)

Yours,
Linus Walleij

[PATCH 0/5 V4] IIO in kernel interfaces (pull)

[Jonathan Cameron]

Dear All,

Changes since V3 are all in processing of raw values to give sensible
hwmon outputs in response to Guenter's comments.

Also rebased on the new version of the IIO core.

Note this is just the association infrastructure and the pull interfaces.
Thus we can handle drivers polling for values.  Doing push interfaces
(interrupt triggered more or less) requires a lot more of the IIO
intrastructure to be in place and so will have to wait a little while.
This is available for the staging tree but will will require the second
large part of IIO to move out of staging (triggered and buffered capture)
plus needs a little more work and a lot more testing.  As the cover letter
for the push interface epxlains there are some complexities of the
interaction of push and pull interfaces on the same physical device.  These
are independent of the external interfaces so shouldn't block this support
merging.

The final patch is not intended for merging, but rather acts as an
example of the board configuration needed.


Since V2:

Fixes as per Guenter's comments on the iio_hwmon driver.
Only one that differred slightly from Guenters suggestion
is the value calculation where I reordered things to reduce
the chance of overflow as well as casting to longs.
Thanks Guenter!

Since V1:
Uses the bus's list of devices instead of having a local one.
     (thanks to Lars-Peter).

Added are interfaces for getting all channels mapped to a particular
device.

Hwmon driver is now vaguely complete - hence cc'd hwmon. If people
are happy I'd expect this to ultimately end up in drivers/hmwon.
Right now it is acting as a test driver for these interfaces.

Thanks and all comments welcome. I've added hwmon to the cc list 
this time round.

V1 message:

There are obviously some rough corners in here that will need cleaning up.
For now I've just put this out there to see if anyone radically disagrees
on the direction.  It sits on top the recent rfc to move first bit of IIO
out of staging with a few buglets fixed.  Best bet if anyone wants to
test is to pull from:

https://github.com/jic23/linux-iio/tree/outofstaging

which now also includes these patches.

Intereresting patches are 3 and 4.  5 gives a trivial example of
a driver using this (hwmon driver that only takes first matching channel
and sticks it out as in1_input - breaks all sorts elements of the hwmon
interface spec.)

For now I've gone with Mark Brown's suggestion of a datasheet_name for
finding the channels on the device.  Patch 2 hacks this name into the
max1363 driver.  I'll probably put a version matching on channel number and
type in at a later date.

Here we just have a pull interface.  Push is considerably harder to do
and needs quite a lot more of the IIO infrastructure to be in place
(triggers / buffers etc). Events obviously require IIO event handling
to be there. Hence all of that will have to wait until those elements
have in of themselves been posted for review.

It is pretty clear to me that hwmon interface for starters needs the
ability to say - 'give me all mappings that correspond to me'.

This is what I intend to add next followed by some utility functions
to make it easy to match the hwmon interface.

At that point I'll propose the hwmon driver goes into drivers/hwmon
(subject to the underlying iio stuff merging).

Fun fun fun.  Thanks to Linus an Mark for their input on this.
Hope this is roughly what you guys were looking for.

Also on my list to do is to check this very thoroughly for any
possible race conditions around the removal of the underlying
device.

Jonathan

Jonathan Cameron (5):
  IIO: core: add datasheet_name to chan_spec
  IIO:ADC:max1363 add datasheet_name entries.
  IIO:CORE add in kernel interface mapping and getting IIO channels.
  IIO:hwmon interface client driver.
  stargate2: example of map configuration for iio to hwmon example.

 arch/arm/mach-pxa/stargate2.c  |   23 ++++
 drivers/Makefile               |    2 +-
 drivers/iio/Kconfig            |    8 ++
 drivers/iio/Makefile           |    2 +
 drivers/iio/adc/max1363_core.c |    2 +
 drivers/iio/iio.c              |  264 +++++++++++++++++++++++++++++++++++++++-
 drivers/iio/iio_hwmon.c        |  227 ++++++++++++++++++++++++++++++++++
 drivers/iio/inkern.c           |   20 +++
 include/linux/iio/iio.h        |    7 +-
 include/linux/iio/inkern.h     |   95 ++++++++++++++
 10 files changed, 643 insertions(+), 7 deletions(-)
 create mode 100644 drivers/iio/iio_hwmon.c
 create mode 100644 drivers/iio/inkern.c
 create mode 100644 include/linux/iio/inkern.h

-- 
1.7.7.2

[PATCH 5/5] stargate2: example of map configuration for iio to hwmon example.

[Jonathan Cameron]

From: Jonathan Cameron <jic23@cam.ac.uk>

Do not commit.
---
 arch/arm/mach-pxa/stargate2.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
index 4c9a48b..25a69c7 100644
--- a/arch/arm/mach-pxa/stargate2.c
+++ b/arch/arm/mach-pxa/stargate2.c
@@ -54,6 +54,8 @@
 #include <linux/mfd/da903x.h>
 #include <linux/sht15.h>
 
+#include <linux/iio/inkern.h>
+
 #include "devices.h"
 #include "generic.h"
 
@@ -406,6 +408,24 @@ static struct i2c_pxa_platform_data i2c_pdata = {
 	.fast_mode = 1,
 };
 
+static struct platform_device iio_hwmon_test = {
+	.name = "iio_hwmon",
+};
+
+static struct iio_map adc_map[] = {
+	{
+		.adc_dev_name = "0-0035",
+		.adc_channel_label = "AIN1",
+		.consumer_dev = &iio_hwmon_test.dev,
+		.consumer_channel = "testchan1",
+	}, {
+		.adc_dev_name = "0-0035",
+		.adc_channel_label = "AIN2",
+		.consumer_dev = &iio_hwmon_test.dev,
+		.consumer_channel = "testchan2",
+	},
+};
+
 static void __init imote2_stargate2_init(void)
 {
 
@@ -423,6 +443,8 @@ static void __init imote2_stargate2_init(void)
 
 	pxa27x_set_i2c_power_info(&i2c_pwr_pdata);
 	pxa_set_i2c_info(&i2c_pdata);
+
+	iio_map_array_register(adc_map, ARRAY_SIZE(adc_map));
 }
 
 #ifdef CONFIG_MACH_INTELMOTE2
@@ -971,6 +993,7 @@ static struct platform_device *stargate2_devices[] = {
 	&stargate2_sram,
 	&smc91x_device,
 	&sht15,
+	&iio_hwmon_test,
 };
 
 static void __init stargate2_init(void)
-- 
1.7.7.2