All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] iio: Devicetree support
@ 2013-02-04 20:26 ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse, Guenter Roeck

This patch set adds basic device tree support to the IIO subsystem. It is the
result of discussions [1] and [2].

Patch 1 changes the first parameter to iio_channel_get() to be the pointer to
the consumer device instead of the consumer device name.

Patch 2 adds basic OF support to the IIO subsystem. Support is modeled after
the clock subsystem.

The patches are now based on the to togreg branch of iio kernel repository.
Already applied patches are no longer part of the patch set.

I increased the audience to all affected subsystems. Sorry for missing out
power and extcon earlier, which are both affected by the first patch.

Guenter

---
[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
[2] http://marc.info/?l=lm-sensors&m=135375101529918&w=1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 0/2] iio: Devicetree support
@ 2013-02-04 20:26 ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse,
	Guenter Roeck

This patch set adds basic device tree support to the IIO subsystem. It is the
result of discussions [1] and [2].

Patch 1 changes the first parameter to iio_channel_get() to be the pointer to
the consumer device instead of the consumer device name.

Patch 2 adds basic OF support to the IIO subsystem. Support is modeled after
the clock subsystem.

The patches are now based on the to togreg branch of iio kernel repository.
Already applied patches are no longer part of the patch set.

I increased the audience to all affected subsystems. Sorry for missing out
power and extcon earlier, which are both affected by the first patch.

Guenter

---
[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
[2] http://marc.info/?l=lm-sensors&m=135375101529918&w=1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
  2013-02-04 20:26 ` Guenter Roeck
@ 2013-02-04 20:26     ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse, Guenter Roeck

For iio_channel_get to work with OF based configurations, it needs the
consumer device pointer instead of the consumer device name as argument.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
 drivers/extcon/extcon-adc-jack.c    |    3 +--
 drivers/iio/inkern.c                |   11 ++++++++++-
 drivers/power/generic-adc-battery.c |    4 ++--
 drivers/power/lp8788-charger.c      |    8 ++++----
 include/linux/iio/consumer.h        |    5 +++--
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index eda2a1a..d0233cd 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
 		;
 	data->num_conditions = i;
 
-	data->chan = iio_channel_get(dev_name(&pdev->dev),
-			pdata->consumer_channel);
+	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
 	if (IS_ERR(data->chan)) {
 		err = PTR_ERR(data->chan);
 		goto out;
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c42aba6..b289915 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -93,7 +93,8 @@ static const struct iio_chan_spec
 }
 
 
-struct iio_channel *iio_channel_get(const char *name, const char *channel_name)
+static struct iio_channel *iio_channel_get_sys(const char *name,
+					       const char *channel_name)
 {
 	struct iio_map_internal *c_i = NULL, *c = NULL;
 	struct iio_channel *channel;
@@ -144,6 +145,14 @@ error_no_mem:
 	iio_device_put(c->indio_dev);
 	return ERR_PTR(err);
 }
+
+struct iio_channel *iio_channel_get(struct device *dev,
+				    const char *channel_name)
+{
+	const char *name = dev ? dev_name(dev) : NULL;
+
+	return iio_channel_get_sys(name, channel_name);
+}
 EXPORT_SYMBOL_GPL(iio_channel_get);
 
 void iio_channel_release(struct iio_channel *channel)
diff --git a/drivers/power/generic-adc-battery.c b/drivers/power/generic-adc-battery.c
index 32ce17e..42733c4 100644
--- a/drivers/power/generic-adc-battery.c
+++ b/drivers/power/generic-adc-battery.c
@@ -287,8 +287,8 @@ static int gab_probe(struct platform_device *pdev)
 	 * based on the channel supported by consumer device.
 	 */
 	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
-		adc_bat->channel[chan] = iio_channel_get(dev_name(&pdev->dev),
-						gab_chan_name[chan]);
+		adc_bat->channel[chan] = iio_channel_get(&pdev->dev,
+							 gab_chan_name[chan]);
 		if (IS_ERR(adc_bat->channel[chan])) {
 			ret = PTR_ERR(adc_bat->channel[chan]);
 		} else {
diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 22b6407c..2788908 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -580,7 +580,7 @@ static void lp8788_irq_unregister(struct platform_device *pdev,
 	}
 }
 
-static void lp8788_setup_adc_channel(const char *consumer_name,
+static void lp8788_setup_adc_channel(struct device *dev,
 				struct lp8788_charger *pchg)
 {
 	struct lp8788_charger_platform_data *pdata = pchg->pdata;
@@ -590,11 +590,11 @@ static void lp8788_setup_adc_channel(const char *consumer_name,
 		return;
 
 	/* ADC channel for battery voltage */
-	chan = iio_channel_get(consumer_name, pdata->adc_vbatt);
+	chan = iio_channel_get(dev, pdata->adc_vbatt);
 	pchg->chan[LP8788_VBATT] = IS_ERR(chan) ? NULL : chan;
 
 	/* ADC channel for battery temperature */
-	chan = iio_channel_get(consumer_name, pdata->adc_batt_temp);
+	chan = iio_channel_get(dev, pdata->adc_batt_temp);
 	pchg->chan[LP8788_BATT_TEMP] = IS_ERR(chan) ? NULL : chan;
 }
 
@@ -704,7 +704,7 @@ static int lp8788_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	lp8788_setup_adc_channel(pdev->name, pchg);
+	lp8788_setup_adc_channel(&pdev->dev, pchg);
 
 	ret = lp8788_psy_register(pdev, pchg);
 	if (ret)
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a85787a..833926c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -31,14 +31,15 @@ struct iio_channel {
 
 /**
  * iio_channel_get() - get description of all that is needed to access channel.
- * @name:		Unique name of the device as provided in the iio_map
+ * @dev:		Pointer to consumer device. Device name must match
+ *			the name of the device as provided in the iio_map
  *			with which the desired provider to consumer mapping
  *			was registered.
  * @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_channel_get(const char *name,
+struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *consumer_channel);
 
 /**
-- 
1.7.9.7

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
@ 2013-02-04 20:26     ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse,
	Guenter Roeck

For iio_channel_get to work with OF based configurations, it needs the
consumer device pointer instead of the consumer device name as argument.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/extcon/extcon-adc-jack.c    |    3 +--
 drivers/iio/inkern.c                |   11 ++++++++++-
 drivers/power/generic-adc-battery.c |    4 ++--
 drivers/power/lp8788-charger.c      |    8 ++++----
 include/linux/iio/consumer.h        |    5 +++--
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index eda2a1a..d0233cd 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
 		;
 	data->num_conditions = i;
 
-	data->chan = iio_channel_get(dev_name(&pdev->dev),
-			pdata->consumer_channel);
+	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
 	if (IS_ERR(data->chan)) {
 		err = PTR_ERR(data->chan);
 		goto out;
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c42aba6..b289915 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -93,7 +93,8 @@ static const struct iio_chan_spec
 }
 
 
-struct iio_channel *iio_channel_get(const char *name, const char *channel_name)
+static struct iio_channel *iio_channel_get_sys(const char *name,
+					       const char *channel_name)
 {
 	struct iio_map_internal *c_i = NULL, *c = NULL;
 	struct iio_channel *channel;
@@ -144,6 +145,14 @@ error_no_mem:
 	iio_device_put(c->indio_dev);
 	return ERR_PTR(err);
 }
+
+struct iio_channel *iio_channel_get(struct device *dev,
+				    const char *channel_name)
+{
+	const char *name = dev ? dev_name(dev) : NULL;
+
+	return iio_channel_get_sys(name, channel_name);
+}
 EXPORT_SYMBOL_GPL(iio_channel_get);
 
 void iio_channel_release(struct iio_channel *channel)
diff --git a/drivers/power/generic-adc-battery.c b/drivers/power/generic-adc-battery.c
index 32ce17e..42733c4 100644
--- a/drivers/power/generic-adc-battery.c
+++ b/drivers/power/generic-adc-battery.c
@@ -287,8 +287,8 @@ static int gab_probe(struct platform_device *pdev)
 	 * based on the channel supported by consumer device.
 	 */
 	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
-		adc_bat->channel[chan] = iio_channel_get(dev_name(&pdev->dev),
-						gab_chan_name[chan]);
+		adc_bat->channel[chan] = iio_channel_get(&pdev->dev,
+							 gab_chan_name[chan]);
 		if (IS_ERR(adc_bat->channel[chan])) {
 			ret = PTR_ERR(adc_bat->channel[chan]);
 		} else {
diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 22b6407c..2788908 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -580,7 +580,7 @@ static void lp8788_irq_unregister(struct platform_device *pdev,
 	}
 }
 
-static void lp8788_setup_adc_channel(const char *consumer_name,
+static void lp8788_setup_adc_channel(struct device *dev,
 				struct lp8788_charger *pchg)
 {
 	struct lp8788_charger_platform_data *pdata = pchg->pdata;
@@ -590,11 +590,11 @@ static void lp8788_setup_adc_channel(const char *consumer_name,
 		return;
 
 	/* ADC channel for battery voltage */
-	chan = iio_channel_get(consumer_name, pdata->adc_vbatt);
+	chan = iio_channel_get(dev, pdata->adc_vbatt);
 	pchg->chan[LP8788_VBATT] = IS_ERR(chan) ? NULL : chan;
 
 	/* ADC channel for battery temperature */
-	chan = iio_channel_get(consumer_name, pdata->adc_batt_temp);
+	chan = iio_channel_get(dev, pdata->adc_batt_temp);
 	pchg->chan[LP8788_BATT_TEMP] = IS_ERR(chan) ? NULL : chan;
 }
 
@@ -704,7 +704,7 @@ static int lp8788_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	lp8788_setup_adc_channel(pdev->name, pchg);
+	lp8788_setup_adc_channel(&pdev->dev, pchg);
 
 	ret = lp8788_psy_register(pdev, pchg);
 	if (ret)
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a85787a..833926c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -31,14 +31,15 @@ struct iio_channel {
 
 /**
  * iio_channel_get() - get description of all that is needed to access channel.
- * @name:		Unique name of the device as provided in the iio_map
+ * @dev:		Pointer to consumer device. Device name must match
+ *			the name of the device as provided in the iio_map
  *			with which the desired provider to consumer mapping
  *			was registered.
  * @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_channel_get(const char *name,
+struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *consumer_channel);
 
 /**
-- 
1.7.9.7


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/2] iio: Add OF support
  2013-02-04 20:26 ` Guenter Roeck
@ 2013-02-04 20:26     ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse, Guenter Roeck

Provide bindings and parse OF data during initialization.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
One open question is how to assign of_node to the iio device. We can either do it
in each driver (which turns out to be a huge patchset), or add something like the
following to iio_device_register.

	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;

v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
  of strcmp. Rename iio_dev_type to iio_device_type (to match other device
  types) and make it global for that purpose. Check the OF node first, then the
  device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
  __of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
  an error if there is a problem with consistency or if the provider device is
  not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
  or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
  not used in the code, and it is not entirely clear what it would be used for.
  If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
  if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
  name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.

 .../devicetree/bindings/iio/iio-bindings.txt       |   90 +++++++++++
 drivers/iio/iio_core.h                             |    1 +
 drivers/iio/industrialio-core.c                    |    4 +-
 drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
 4 files changed, 264 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 0000000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+		   with a single IIO output and 1 for nodes with multiple
+		   IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+	adc: voltage-sensor@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+Example for a configuration with trigger:
+
+	adc@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+
+		adc: iio-device {
+			#io-channel-cells = <1>;
+		};
+		trigger: iio-trigger {
+			#io-channel-cells = <1>;
+		};
+	};
+
+==IIO consumers==
+
+Required properties:
+io-channels:	List of phandle and IIO specifier pairs, one pair
+		for each IIO input to the device. Note: if the
+		IIO provider specifies '0' for #io-channel-cells,
+		then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+		List of IIO input name strings sorted in the same
+		order as the io-channels property. Consumers drivers
+		will use io-channel-names to match IIO input names
+		with IIO specifiers.
+io-channel-ranges:
+		Empty property indicating that child nodes can inherit named
+		IIO channels from this node. Useful for bus nodes to provide
+		and IIO channel to their children.
+
+For example:
+
+	device {
+		io-channels = <&adc 1>, <&ref 0>;
+		io-channel-names = "vcc", "vdd";
+	};
+
+This represents a device with two IIO inputs, named "vcc" and "vdd".
+The vcc channel is connected to output 1 of the &adc device, and the
+vdd channel is connected to output 0 of the &ref device.
+
+==Example==
+
+	adc: max1139@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+	...
+
+	iio_hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
+			<&adc 3>, <&adc 4>, <&adc 5>,
+			<&adc 6>, <&adc 7>, <&adc 8>,
+			<&adc 9>, <&adc 10>, <&adc 11>;
+		io-channel-names = "vcc", "vdd", "vref", "1.2V";
+	};
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index f652e6a..05c1b74 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -18,6 +18,7 @@
 struct iio_chan_spec;
 struct iio_dev;
 
+extern struct device_type iio_device_type;
 
 int __iio_add_chan_devattr(const char *postfix,
 			   struct iio_chan_spec const *chan,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8848f16..81e3594 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
 	kfree(indio_dev);
 }
 
-static struct device_type iio_dev_type = {
+struct device_type iio_device_type = {
 	.name = "iio_device",
 	.release = iio_dev_release,
 };
@@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
 
 	if (dev) {
 		dev->dev.groups = dev->groups;
-		dev->dev.type = &iio_dev_type;
+		dev->dev.type = &iio_device_type;
 		dev->dev.bus = &iio_bus_type;
 		device_initialize(&dev->dev);
 		dev_set_drvdata(&dev->dev, (void *)dev);
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..913428e 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include "iio_core.h"
@@ -92,6 +93,164 @@ static const struct iio_chan_spec
 	return chan;
 }
 
+#ifdef CONFIG_OF
+
+static int iio_dev_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data && dev->type == &iio_device_type;
+}
+
+static int __of_iio_channel_get(struct iio_channel *channel,
+				struct device_node *np, int index)
+{
+	struct device *idev;
+	struct iio_dev *indio_dev;
+	int err;
+	struct of_phandle_args iiospec;
+
+	err = of_parse_phandle_with_args(np, "io-channels",
+					 "#io-channel-cells",
+					 index, &iiospec);
+	if (err)
+		return err;
+
+	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
+			       iio_dev_node_match);
+	of_node_put(iiospec.np);
+	if (idev == NULL)
+		return -EPROBE_DEFER;
+
+	indio_dev = dev_to_iio_dev(idev);
+	channel->indio_dev = indio_dev;
+	index = iiospec.args_count ? iiospec.args[0] : 0;
+	if (index >= indio_dev->num_channels) {
+		return -EINVAL;
+		goto err_put;
+	}
+	channel->channel = &indio_dev->channels[index];
+
+	return 0;
+
+err_put:
+	iio_device_put(indio_dev);
+	return err;
+}
+
+static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
+{
+	struct iio_channel *channel;
+	int err;
+
+	if (index < 0)
+		return ERR_PTR(-EINVAL);
+
+	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
+	if (channel == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	err = __of_iio_channel_get(channel, np, index);
+	if (err)
+		goto err_free_channel;
+
+	return channel;
+
+err_free_channel:
+	kfree(channel);
+	return ERR_PTR(err);
+}
+
+static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
+						      const char *name)
+{
+	struct iio_channel *chan = NULL;
+
+	/* Walk up the tree of devices looking for a matching iio channel */
+	while (np) {
+		int index = 0;
+
+		/*
+		 * For named iio channels, first look up the name in the
+		 * "io-channel-names" property.  If it cannot be found, the
+		 * index will be an error code, and of_iio_channel_get()
+		 * will fail.
+		 */
+		if (name)
+			index = of_property_match_string(np, "io-channel-names",
+							 name);
+		chan = of_iio_channel_get(np, index);
+		if (!IS_ERR(chan))
+			break;
+		else if (name && index >= 0) {
+			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
+				np->full_name, name ? name : "", index);
+			return chan;
+		}
+
+		/*
+		 * No matching IIO channel found on this node.
+		 * If the parent node has a "io-channel-ranges" property,
+		 * then we can try one of its channels.
+		 */
+		np = np->parent;
+		if (np && !of_get_property(np, "io-channel-ranges", NULL))
+			break;
+	}
+	return chan;
+}
+
+static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	struct iio_channel *chans;
+	int i, mapind, nummaps = 0;
+	int ret;
+
+	do {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "io-channels",
+						 "#io-channel-cells",
+						 nummaps, NULL);
+		if (ret < 0)
+			break;
+	} while (++nummaps);
+
+	if (nummaps == 0)	/* no error, return NULL to search map table */
+		return NULL;
+
+	/* NULL terminated array to save passing size */
+	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
+	if (chans == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	/* Search for OF matches */
+	for (mapind = 0; mapind < nummaps; mapind++) {
+		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
+					   mapind);
+		if (ret)
+			goto error_free_chans;
+	}
+	return chans;
+
+error_free_chans:
+	for (i = 0; i < mapind; i++)
+		iio_device_put(chans[i].indio_dev);
+	kfree(chans);
+	return ERR_PTR(ret);
+}
+
+#else /* CONFIG_OF */
+
+static inline struct iio_channel *
+of_iio_channel_get_by_name(struct device *dev, const char *name)
+{
+	return ERR_PTR(-ENOENT);
+}
+
+static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
 
 static struct iio_channel *iio_channel_get_sys(const char *name,
 					       const char *channel_name)
@@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *channel_name)
 {
 	const char *name = dev ? dev_name(dev) : NULL;
+	struct iio_channel *channel;
 
+	if (dev) {
+		channel = of_iio_channel_get_by_name(dev->of_node,
+						     channel_name);
+		if (channel != NULL)
+			return channel;
+	}
 	return iio_channel_get_sys(name, channel_name);
 }
 EXPORT_SYMBOL_GPL(iio_channel_get);
@@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 
 	if (dev == NULL)
 		return ERR_PTR(-EINVAL);
+
+	chans = of_iio_channel_get_all(dev);
+	if (chans)
+		return chans;
+
 	name = dev_name(dev);
 
 	mutex_lock(&iio_map_list_lock);
-- 
1.7.9.7

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/2] iio: Add OF support
@ 2013-02-04 20:26     ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-04 20:26 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse,
	Guenter Roeck

Provide bindings and parse OF data during initialization.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
One open question is how to assign of_node to the iio device. We can either do it
in each driver (which turns out to be a huge patchset), or add something like the
following to iio_device_register.

	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;

v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
  of strcmp. Rename iio_dev_type to iio_device_type (to match other device
  types) and make it global for that purpose. Check the OF node first, then the
  device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
  __of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
  an error if there is a problem with consistency or if the provider device is
  not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
  or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
  not used in the code, and it is not entirely clear what it would be used for.
  If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
  if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
  name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.

 .../devicetree/bindings/iio/iio-bindings.txt       |   90 +++++++++++
 drivers/iio/iio_core.h                             |    1 +
 drivers/iio/industrialio-core.c                    |    4 +-
 drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
 4 files changed, 264 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 0000000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+		   with a single IIO output and 1 for nodes with multiple
+		   IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+	adc: voltage-sensor@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+Example for a configuration with trigger:
+
+	adc@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+
+		adc: iio-device {
+			#io-channel-cells = <1>;
+		};
+		trigger: iio-trigger {
+			#io-channel-cells = <1>;
+		};
+	};
+
+==IIO consumers==
+
+Required properties:
+io-channels:	List of phandle and IIO specifier pairs, one pair
+		for each IIO input to the device. Note: if the
+		IIO provider specifies '0' for #io-channel-cells,
+		then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+		List of IIO input name strings sorted in the same
+		order as the io-channels property. Consumers drivers
+		will use io-channel-names to match IIO input names
+		with IIO specifiers.
+io-channel-ranges:
+		Empty property indicating that child nodes can inherit named
+		IIO channels from this node. Useful for bus nodes to provide
+		and IIO channel to their children.
+
+For example:
+
+	device {
+		io-channels = <&adc 1>, <&ref 0>;
+		io-channel-names = "vcc", "vdd";
+	};
+
+This represents a device with two IIO inputs, named "vcc" and "vdd".
+The vcc channel is connected to output 1 of the &adc device, and the
+vdd channel is connected to output 0 of the &ref device.
+
+==Example==
+
+	adc: max1139@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+	...
+
+	iio_hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
+			<&adc 3>, <&adc 4>, <&adc 5>,
+			<&adc 6>, <&adc 7>, <&adc 8>,
+			<&adc 9>, <&adc 10>, <&adc 11>;
+		io-channel-names = "vcc", "vdd", "vref", "1.2V";
+	};
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index f652e6a..05c1b74 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -18,6 +18,7 @@
 struct iio_chan_spec;
 struct iio_dev;
 
+extern struct device_type iio_device_type;
 
 int __iio_add_chan_devattr(const char *postfix,
 			   struct iio_chan_spec const *chan,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8848f16..81e3594 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
 	kfree(indio_dev);
 }
 
-static struct device_type iio_dev_type = {
+struct device_type iio_device_type = {
 	.name = "iio_device",
 	.release = iio_dev_release,
 };
@@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
 
 	if (dev) {
 		dev->dev.groups = dev->groups;
-		dev->dev.type = &iio_dev_type;
+		dev->dev.type = &iio_device_type;
 		dev->dev.bus = &iio_bus_type;
 		device_initialize(&dev->dev);
 		dev_set_drvdata(&dev->dev, (void *)dev);
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..913428e 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include "iio_core.h"
@@ -92,6 +93,164 @@ static const struct iio_chan_spec
 	return chan;
 }
 
+#ifdef CONFIG_OF
+
+static int iio_dev_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data && dev->type == &iio_device_type;
+}
+
+static int __of_iio_channel_get(struct iio_channel *channel,
+				struct device_node *np, int index)
+{
+	struct device *idev;
+	struct iio_dev *indio_dev;
+	int err;
+	struct of_phandle_args iiospec;
+
+	err = of_parse_phandle_with_args(np, "io-channels",
+					 "#io-channel-cells",
+					 index, &iiospec);
+	if (err)
+		return err;
+
+	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
+			       iio_dev_node_match);
+	of_node_put(iiospec.np);
+	if (idev == NULL)
+		return -EPROBE_DEFER;
+
+	indio_dev = dev_to_iio_dev(idev);
+	channel->indio_dev = indio_dev;
+	index = iiospec.args_count ? iiospec.args[0] : 0;
+	if (index >= indio_dev->num_channels) {
+		return -EINVAL;
+		goto err_put;
+	}
+	channel->channel = &indio_dev->channels[index];
+
+	return 0;
+
+err_put:
+	iio_device_put(indio_dev);
+	return err;
+}
+
+static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
+{
+	struct iio_channel *channel;
+	int err;
+
+	if (index < 0)
+		return ERR_PTR(-EINVAL);
+
+	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
+	if (channel == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	err = __of_iio_channel_get(channel, np, index);
+	if (err)
+		goto err_free_channel;
+
+	return channel;
+
+err_free_channel:
+	kfree(channel);
+	return ERR_PTR(err);
+}
+
+static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
+						      const char *name)
+{
+	struct iio_channel *chan = NULL;
+
+	/* Walk up the tree of devices looking for a matching iio channel */
+	while (np) {
+		int index = 0;
+
+		/*
+		 * For named iio channels, first look up the name in the
+		 * "io-channel-names" property.  If it cannot be found, the
+		 * index will be an error code, and of_iio_channel_get()
+		 * will fail.
+		 */
+		if (name)
+			index = of_property_match_string(np, "io-channel-names",
+							 name);
+		chan = of_iio_channel_get(np, index);
+		if (!IS_ERR(chan))
+			break;
+		else if (name && index >= 0) {
+			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
+				np->full_name, name ? name : "", index);
+			return chan;
+		}
+
+		/*
+		 * No matching IIO channel found on this node.
+		 * If the parent node has a "io-channel-ranges" property,
+		 * then we can try one of its channels.
+		 */
+		np = np->parent;
+		if (np && !of_get_property(np, "io-channel-ranges", NULL))
+			break;
+	}
+	return chan;
+}
+
+static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	struct iio_channel *chans;
+	int i, mapind, nummaps = 0;
+	int ret;
+
+	do {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "io-channels",
+						 "#io-channel-cells",
+						 nummaps, NULL);
+		if (ret < 0)
+			break;
+	} while (++nummaps);
+
+	if (nummaps == 0)	/* no error, return NULL to search map table */
+		return NULL;
+
+	/* NULL terminated array to save passing size */
+	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
+	if (chans == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	/* Search for OF matches */
+	for (mapind = 0; mapind < nummaps; mapind++) {
+		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
+					   mapind);
+		if (ret)
+			goto error_free_chans;
+	}
+	return chans;
+
+error_free_chans:
+	for (i = 0; i < mapind; i++)
+		iio_device_put(chans[i].indio_dev);
+	kfree(chans);
+	return ERR_PTR(ret);
+}
+
+#else /* CONFIG_OF */
+
+static inline struct iio_channel *
+of_iio_channel_get_by_name(struct device *dev, const char *name)
+{
+	return ERR_PTR(-ENOENT);
+}
+
+static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
 
 static struct iio_channel *iio_channel_get_sys(const char *name,
 					       const char *channel_name)
@@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *channel_name)
 {
 	const char *name = dev ? dev_name(dev) : NULL;
+	struct iio_channel *channel;
 
+	if (dev) {
+		channel = of_iio_channel_get_by_name(dev->of_node,
+						     channel_name);
+		if (channel != NULL)
+			return channel;
+	}
 	return iio_channel_get_sys(name, channel_name);
 }
 EXPORT_SYMBOL_GPL(iio_channel_get);
@@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 
 	if (dev == NULL)
 		return ERR_PTR(-EINVAL);
+
+	chans = of_iio_channel_get_all(dev);
+	if (chans)
+		return chans;
+
 	name = dev_name(dev);
 
 	mutex_lock(&iio_map_list_lock);
-- 
1.7.9.7

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
  2013-02-04 20:26     ` Guenter Roeck
@ 2013-02-04 21:38         ` Anton Vorontsov
  -1 siblings, 0 replies; 28+ messages in thread
From: Anton Vorontsov @ 2013-02-04 21:38 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:05PM -0800, Guenter Roeck wrote:
> For iio_channel_get to work with OF based configurations, it needs the
> consumer device pointer instead of the consumer device name as argument.
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> ---
>  drivers/extcon/extcon-adc-jack.c    |    3 +--
>  drivers/iio/inkern.c                |   11 ++++++++++-
>  drivers/power/generic-adc-battery.c |    4 ++--
>  drivers/power/lp8788-charger.c      |    8 ++++----
>  include/linux/iio/consumer.h        |    5 +++--
>  5 files changed, 20 insertions(+), 11 deletions(-)

Looks OK to me, Ack for drivers/power part.

Thanks.
Anton

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
@ 2013-02-04 21:38         ` Anton Vorontsov
  0 siblings, 0 replies; 28+ messages in thread
From: Anton Vorontsov @ 2013-02-04 21:38 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio, Jonathan Cameron, devicetree-discuss,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:05PM -0800, Guenter Roeck wrote:
> For iio_channel_get to work with OF based configurations, it needs the
> consumer device pointer instead of the consumer device name as argument.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/extcon/extcon-adc-jack.c    |    3 +--
>  drivers/iio/inkern.c                |   11 ++++++++++-
>  drivers/power/generic-adc-battery.c |    4 ++--
>  drivers/power/lp8788-charger.c      |    8 ++++----
>  include/linux/iio/consumer.h        |    5 +++--
>  5 files changed, 20 insertions(+), 11 deletions(-)

Looks OK to me, Ack for drivers/power part.

Thanks.
Anton

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
  2013-02-04 20:26     ` Guenter Roeck
@ 2013-02-04 23:57         ` Chanwoo Choi
  -1 siblings, 0 replies; 28+ messages in thread
From: Chanwoo Choi @ 2013-02-04 23:57 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Anton Vorontsov, David Woodhouse

On 02/05/2013 05:26 AM, Guenter Roeck wrote:
> For iio_channel_get to work with OF based configurations, it needs the
> consumer device pointer instead of the consumer device name as argument.
>
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> ---
>   drivers/extcon/extcon-adc-jack.c    |    3 +--
>   drivers/iio/inkern.c                |   11 ++++++++++-
>   drivers/power/generic-adc-battery.c |    4 ++--
>   drivers/power/lp8788-charger.c      |    8 ++++----
>   include/linux/iio/consumer.h        |    5 +++--
>   5 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
> index eda2a1a..d0233cd 100644
> --- a/drivers/extcon/extcon-adc-jack.c
> +++ b/drivers/extcon/extcon-adc-jack.c
> @@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
>   		;
>   	data->num_conditions = i;
>   
> -	data->chan = iio_channel_get(dev_name(&pdev->dev),
> -			pdata->consumer_channel);
> +	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
>   	if (IS_ERR(data->chan)) {
>   		err = PTR_ERR(data->chan);
>   		goto out;
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c42aba6..b289915 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -93,7 +93,8 @@ static const struct iio_chan_spec
Ack for drivers/extcon .

Acked-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Thanks,
Chanwoo Choi

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
@ 2013-02-04 23:57         ` Chanwoo Choi
  0 siblings, 0 replies; 28+ messages in thread
From: Chanwoo Choi @ 2013-02-04 23:57 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio, Jonathan Cameron, devicetree-discuss,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Anton Vorontsov, David Woodhouse

On 02/05/2013 05:26 AM, Guenter Roeck wrote:
> For iio_channel_get to work with OF based configurations, it needs the
> consumer device pointer instead of the consumer device name as argument.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   drivers/extcon/extcon-adc-jack.c    |    3 +--
>   drivers/iio/inkern.c                |   11 ++++++++++-
>   drivers/power/generic-adc-battery.c |    4 ++--
>   drivers/power/lp8788-charger.c      |    8 ++++----
>   include/linux/iio/consumer.h        |    5 +++--
>   5 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
> index eda2a1a..d0233cd 100644
> --- a/drivers/extcon/extcon-adc-jack.c
> +++ b/drivers/extcon/extcon-adc-jack.c
> @@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
>   		;
>   	data->num_conditions = i;
>   
> -	data->chan = iio_channel_get(dev_name(&pdev->dev),
> -			pdata->consumer_channel);
> +	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
>   	if (IS_ERR(data->chan)) {
>   		err = PTR_ERR(data->chan);
>   		goto out;
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c42aba6..b289915 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -93,7 +93,8 @@ static const struct iio_chan_spec
Ack for drivers/extcon .

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/2] iio: Add OF support
  2013-02-04 20:26     ` Guenter Roeck
@ 2013-02-05  5:20         ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-05  5:20 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

Note: One of the parameters to the dummy function of_iio_channel_get_by_name()
used when CONFIG_OF is undefined is wrong. I'll fix that in the next version of
the patch.

> ---
> One open question is how to assign of_node to the iio device. We can either do it
> in each driver (which turns out to be a huge patchset), or add something like the
> following to iio_device_register.
> 
> 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> 
> v3:
> - Cleaned up documentation (formatting, left-over clock references)
> - Updated bindings description to permit sub-devices
> - When searching for iio devices, use the pointer to the iio device type instead
>   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
>   types) and make it global for that purpose. Check the OF node first, then the
>   device type, as the node is less likely to match.
> - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
>   __of_iio_channel_get.
> - Return NULL from of_iio_channel_get_by_name if nothing is found, or
>   an error if there is a problem with consistency or if the provider device is
>   not yet available.
> - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
>   or an error, and continue otherwise.
> v2:
> - Rebased to iio/togreg
> - Documentation update per feedback
> - Dropped io-channel-output-names from the bindings document. The property is
>   not used in the code, and it is not entirely clear what it would be used for.
>   If there is a need for it, we can add it back in later on.
> - Don't export OF specific API calls
> - For OF support, no longer depend on iio_map
> - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
>   if it is not selected.
> - Change iio_channel_get to take device pointer as argument instead of device
>   name. Retain old API as of_iio_channel_get_sys.
> - iio_channel_get now works for both OF and non-OF configurations
> - Use regulator to get vref for max1363 driver.
> 
>  .../devicetree/bindings/iio/iio-bindings.txt       |   90 +++++++++++
>  drivers/iio/iio_core.h                             |    1 +
>  drivers/iio/industrialio-core.c                    |    4 +-
>  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
>  4 files changed, 264 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> new file mode 100644
> index 0000000..2475c2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -0,0 +1,90 @@
> +This binding is a work-in-progress. It is derived from clock bindings,
> +and based on suggestions from Lars-Peter Clausen [1].
> +
> +Sources of IIO channels can be represented by any node in the device
> +tree. Those nodes are designated as IIO providers. IIO consumer
> +nodes use a phandle and IIO specifier pair to connect IIO provider
> +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> +specifier is an array of one or more cells identifying the IIO
> +output on a device. The length of an IIO specifier is defined by the
> +value of a #io-channel-cells property in the IIO provider node.
> +
> +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> +
> +==IIO providers==
> +
> +Required properties:
> +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> +		   with a single IIO output and 1 for nodes with multiple
> +		   IIO outputs.
> +
> +Example for a simple configuration with no trigger:
> +
> +	adc: voltage-sensor@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +Example for a configuration with trigger:
> +
> +	adc@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +
> +		adc: iio-device {
> +			#io-channel-cells = <1>;
> +		};
> +		trigger: iio-trigger {
> +			#io-channel-cells = <1>;
> +		};
> +	};
> +
> +==IIO consumers==
> +
> +Required properties:
> +io-channels:	List of phandle and IIO specifier pairs, one pair
> +		for each IIO input to the device. Note: if the
> +		IIO provider specifies '0' for #io-channel-cells,
> +		then only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +io-channel-names:
> +		List of IIO input name strings sorted in the same
> +		order as the io-channels property. Consumers drivers
> +		will use io-channel-names to match IIO input names
> +		with IIO specifiers.
> +io-channel-ranges:
> +		Empty property indicating that child nodes can inherit named
> +		IIO channels from this node. Useful for bus nodes to provide
> +		and IIO channel to their children.
> +
> +For example:
> +
> +	device {
> +		io-channels = <&adc 1>, <&ref 0>;
> +		io-channel-names = "vcc", "vdd";
> +	};
> +
> +This represents a device with two IIO inputs, named "vcc" and "vdd".
> +The vcc channel is connected to output 1 of the &adc device, and the
> +vdd channel is connected to output 0 of the &ref device.
> +
> +==Example==
> +
> +	adc: max1139@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +	...
> +
> +	iio_hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> +			<&adc 3>, <&adc 4>, <&adc 5>,
> +			<&adc 6>, <&adc 7>, <&adc 8>,
> +			<&adc 9>, <&adc 10>, <&adc 11>;
> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> +	};
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index f652e6a..05c1b74 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -18,6 +18,7 @@
>  struct iio_chan_spec;
>  struct iio_dev;
>  
> +extern struct device_type iio_device_type;
>  
>  int __iio_add_chan_devattr(const char *postfix,
>  			   struct iio_chan_spec const *chan,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8848f16..81e3594 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
>  	kfree(indio_dev);
>  }
>  
> -static struct device_type iio_dev_type = {
> +struct device_type iio_device_type = {
>  	.name = "iio_device",
>  	.release = iio_dev_release,
>  };
> @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>  
>  	if (dev) {
>  		dev->dev.groups = dev->groups;
> -		dev->dev.type = &iio_dev_type;
> +		dev->dev.type = &iio_device_type;
>  		dev->dev.bus = &iio_bus_type;
>  		device_initialize(&dev->dev);
>  		dev_set_drvdata(&dev->dev, (void *)dev);
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index b289915..913428e 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -10,6 +10,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
> @@ -92,6 +93,164 @@ static const struct iio_chan_spec
>  	return chan;
>  }
>  
> +#ifdef CONFIG_OF
> +
> +static int iio_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data && dev->type == &iio_device_type;
> +}
> +
> +static int __of_iio_channel_get(struct iio_channel *channel,
> +				struct device_node *np, int index)
> +{
> +	struct device *idev;
> +	struct iio_dev *indio_dev;
> +	int err;
> +	struct of_phandle_args iiospec;
> +
> +	err = of_parse_phandle_with_args(np, "io-channels",
> +					 "#io-channel-cells",
> +					 index, &iiospec);
> +	if (err)
> +		return err;
> +
> +	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
> +			       iio_dev_node_match);
> +	of_node_put(iiospec.np);
> +	if (idev == NULL)
> +		return -EPROBE_DEFER;
> +
> +	indio_dev = dev_to_iio_dev(idev);
> +	channel->indio_dev = indio_dev;
> +	index = iiospec.args_count ? iiospec.args[0] : 0;
> +	if (index >= indio_dev->num_channels) {
> +		return -EINVAL;
> +		goto err_put;
> +	}
> +	channel->channel = &indio_dev->channels[index];
> +
> +	return 0;
> +
> +err_put:
> +	iio_device_put(indio_dev);
> +	return err;
> +}
> +
> +static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
> +{
> +	struct iio_channel *channel;
> +	int err;
> +
> +	if (index < 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (channel == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = __of_iio_channel_get(channel, np, index);
> +	if (err)
> +		goto err_free_channel;
> +
> +	return channel;
> +
> +err_free_channel:
> +	kfree(channel);
> +	return ERR_PTR(err);
> +}
> +
> +static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> +						      const char *name)
> +{
> +	struct iio_channel *chan = NULL;
> +
> +	/* Walk up the tree of devices looking for a matching iio channel */
> +	while (np) {
> +		int index = 0;
> +
> +		/*
> +		 * For named iio channels, first look up the name in the
> +		 * "io-channel-names" property.  If it cannot be found, the
> +		 * index will be an error code, and of_iio_channel_get()
> +		 * will fail.
> +		 */
> +		if (name)
> +			index = of_property_match_string(np, "io-channel-names",
> +							 name);
> +		chan = of_iio_channel_get(np, index);
> +		if (!IS_ERR(chan))
> +			break;
> +		else if (name && index >= 0) {
> +			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
> +				np->full_name, name ? name : "", index);
> +			return chan;
> +		}
> +
> +		/*
> +		 * No matching IIO channel found on this node.
> +		 * If the parent node has a "io-channel-ranges" property,
> +		 * then we can try one of its channels.
> +		 */
> +		np = np->parent;
> +		if (np && !of_get_property(np, "io-channel-ranges", NULL))
> +			break;
> +	}
> +	return chan;
> +}
> +
> +static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	struct iio_channel *chans;
> +	int i, mapind, nummaps = 0;
> +	int ret;
> +
> +	do {
> +		ret = of_parse_phandle_with_args(dev->of_node,
> +						 "io-channels",
> +						 "#io-channel-cells",
> +						 nummaps, NULL);
> +		if (ret < 0)
> +			break;
> +	} while (++nummaps);
> +
> +	if (nummaps == 0)	/* no error, return NULL to search map table */
> +		return NULL;
> +
> +	/* NULL terminated array to save passing size */
> +	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> +	if (chans == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Search for OF matches */
> +	for (mapind = 0; mapind < nummaps; mapind++) {
> +		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
> +					   mapind);
> +		if (ret)
> +			goto error_free_chans;
> +	}
> +	return chans;
> +
> +error_free_chans:
> +	for (i = 0; i < mapind; i++)
> +		iio_device_put(chans[i].indio_dev);
> +	kfree(chans);
> +	return ERR_PTR(ret);
> +}
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device *dev, const char *name)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
> +
> +static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
>  
>  static struct iio_channel *iio_channel_get_sys(const char *name,
>  					       const char *channel_name)
> @@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  				    const char *channel_name)
>  {
>  	const char *name = dev ? dev_name(dev) : NULL;
> +	struct iio_channel *channel;
>  
> +	if (dev) {
> +		channel = of_iio_channel_get_by_name(dev->of_node,
> +						     channel_name);
> +		if (channel != NULL)
> +			return channel;
> +	}
>  	return iio_channel_get_sys(name, channel_name);
>  }
>  EXPORT_SYMBOL_GPL(iio_channel_get);
> @@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  
>  	if (dev == NULL)
>  		return ERR_PTR(-EINVAL);
> +
> +	chans = of_iio_channel_get_all(dev);
> +	if (chans)
> +		return chans;
> +
>  	name = dev_name(dev);
>  
>  	mutex_lock(&iio_map_list_lock);
> -- 
> 1.7.9.7
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/2] iio: Add OF support
@ 2013-02-05  5:20         ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-05  5:20 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Note: One of the parameters to the dummy function of_iio_channel_get_by_name()
used when CONFIG_OF is undefined is wrong. I'll fix that in the next version of
the patch.

> ---
> One open question is how to assign of_node to the iio device. We can either do it
> in each driver (which turns out to be a huge patchset), or add something like the
> following to iio_device_register.
> 
> 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> 
> v3:
> - Cleaned up documentation (formatting, left-over clock references)
> - Updated bindings description to permit sub-devices
> - When searching for iio devices, use the pointer to the iio device type instead
>   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
>   types) and make it global for that purpose. Check the OF node first, then the
>   device type, as the node is less likely to match.
> - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
>   __of_iio_channel_get.
> - Return NULL from of_iio_channel_get_by_name if nothing is found, or
>   an error if there is a problem with consistency or if the provider device is
>   not yet available.
> - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
>   or an error, and continue otherwise.
> v2:
> - Rebased to iio/togreg
> - Documentation update per feedback
> - Dropped io-channel-output-names from the bindings document. The property is
>   not used in the code, and it is not entirely clear what it would be used for.
>   If there is a need for it, we can add it back in later on.
> - Don't export OF specific API calls
> - For OF support, no longer depend on iio_map
> - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
>   if it is not selected.
> - Change iio_channel_get to take device pointer as argument instead of device
>   name. Retain old API as of_iio_channel_get_sys.
> - iio_channel_get now works for both OF and non-OF configurations
> - Use regulator to get vref for max1363 driver.
> 
>  .../devicetree/bindings/iio/iio-bindings.txt       |   90 +++++++++++
>  drivers/iio/iio_core.h                             |    1 +
>  drivers/iio/industrialio-core.c                    |    4 +-
>  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
>  4 files changed, 264 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> new file mode 100644
> index 0000000..2475c2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -0,0 +1,90 @@
> +This binding is a work-in-progress. It is derived from clock bindings,
> +and based on suggestions from Lars-Peter Clausen [1].
> +
> +Sources of IIO channels can be represented by any node in the device
> +tree. Those nodes are designated as IIO providers. IIO consumer
> +nodes use a phandle and IIO specifier pair to connect IIO provider
> +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> +specifier is an array of one or more cells identifying the IIO
> +output on a device. The length of an IIO specifier is defined by the
> +value of a #io-channel-cells property in the IIO provider node.
> +
> +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> +
> +==IIO providers==
> +
> +Required properties:
> +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> +		   with a single IIO output and 1 for nodes with multiple
> +		   IIO outputs.
> +
> +Example for a simple configuration with no trigger:
> +
> +	adc: voltage-sensor@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +Example for a configuration with trigger:
> +
> +	adc@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +
> +		adc: iio-device {
> +			#io-channel-cells = <1>;
> +		};
> +		trigger: iio-trigger {
> +			#io-channel-cells = <1>;
> +		};
> +	};
> +
> +==IIO consumers==
> +
> +Required properties:
> +io-channels:	List of phandle and IIO specifier pairs, one pair
> +		for each IIO input to the device. Note: if the
> +		IIO provider specifies '0' for #io-channel-cells,
> +		then only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +io-channel-names:
> +		List of IIO input name strings sorted in the same
> +		order as the io-channels property. Consumers drivers
> +		will use io-channel-names to match IIO input names
> +		with IIO specifiers.
> +io-channel-ranges:
> +		Empty property indicating that child nodes can inherit named
> +		IIO channels from this node. Useful for bus nodes to provide
> +		and IIO channel to their children.
> +
> +For example:
> +
> +	device {
> +		io-channels = <&adc 1>, <&ref 0>;
> +		io-channel-names = "vcc", "vdd";
> +	};
> +
> +This represents a device with two IIO inputs, named "vcc" and "vdd".
> +The vcc channel is connected to output 1 of the &adc device, and the
> +vdd channel is connected to output 0 of the &ref device.
> +
> +==Example==
> +
> +	adc: max1139@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +	...
> +
> +	iio_hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> +			<&adc 3>, <&adc 4>, <&adc 5>,
> +			<&adc 6>, <&adc 7>, <&adc 8>,
> +			<&adc 9>, <&adc 10>, <&adc 11>;
> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> +	};
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index f652e6a..05c1b74 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -18,6 +18,7 @@
>  struct iio_chan_spec;
>  struct iio_dev;
>  
> +extern struct device_type iio_device_type;
>  
>  int __iio_add_chan_devattr(const char *postfix,
>  			   struct iio_chan_spec const *chan,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8848f16..81e3594 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
>  	kfree(indio_dev);
>  }
>  
> -static struct device_type iio_dev_type = {
> +struct device_type iio_device_type = {
>  	.name = "iio_device",
>  	.release = iio_dev_release,
>  };
> @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>  
>  	if (dev) {
>  		dev->dev.groups = dev->groups;
> -		dev->dev.type = &iio_dev_type;
> +		dev->dev.type = &iio_device_type;
>  		dev->dev.bus = &iio_bus_type;
>  		device_initialize(&dev->dev);
>  		dev_set_drvdata(&dev->dev, (void *)dev);
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index b289915..913428e 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -10,6 +10,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
> @@ -92,6 +93,164 @@ static const struct iio_chan_spec
>  	return chan;
>  }
>  
> +#ifdef CONFIG_OF
> +
> +static int iio_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data && dev->type == &iio_device_type;
> +}
> +
> +static int __of_iio_channel_get(struct iio_channel *channel,
> +				struct device_node *np, int index)
> +{
> +	struct device *idev;
> +	struct iio_dev *indio_dev;
> +	int err;
> +	struct of_phandle_args iiospec;
> +
> +	err = of_parse_phandle_with_args(np, "io-channels",
> +					 "#io-channel-cells",
> +					 index, &iiospec);
> +	if (err)
> +		return err;
> +
> +	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
> +			       iio_dev_node_match);
> +	of_node_put(iiospec.np);
> +	if (idev == NULL)
> +		return -EPROBE_DEFER;
> +
> +	indio_dev = dev_to_iio_dev(idev);
> +	channel->indio_dev = indio_dev;
> +	index = iiospec.args_count ? iiospec.args[0] : 0;
> +	if (index >= indio_dev->num_channels) {
> +		return -EINVAL;
> +		goto err_put;
> +	}
> +	channel->channel = &indio_dev->channels[index];
> +
> +	return 0;
> +
> +err_put:
> +	iio_device_put(indio_dev);
> +	return err;
> +}
> +
> +static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
> +{
> +	struct iio_channel *channel;
> +	int err;
> +
> +	if (index < 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (channel == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = __of_iio_channel_get(channel, np, index);
> +	if (err)
> +		goto err_free_channel;
> +
> +	return channel;
> +
> +err_free_channel:
> +	kfree(channel);
> +	return ERR_PTR(err);
> +}
> +
> +static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> +						      const char *name)
> +{
> +	struct iio_channel *chan = NULL;
> +
> +	/* Walk up the tree of devices looking for a matching iio channel */
> +	while (np) {
> +		int index = 0;
> +
> +		/*
> +		 * For named iio channels, first look up the name in the
> +		 * "io-channel-names" property.  If it cannot be found, the
> +		 * index will be an error code, and of_iio_channel_get()
> +		 * will fail.
> +		 */
> +		if (name)
> +			index = of_property_match_string(np, "io-channel-names",
> +							 name);
> +		chan = of_iio_channel_get(np, index);
> +		if (!IS_ERR(chan))
> +			break;
> +		else if (name && index >= 0) {
> +			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
> +				np->full_name, name ? name : "", index);
> +			return chan;
> +		}
> +
> +		/*
> +		 * No matching IIO channel found on this node.
> +		 * If the parent node has a "io-channel-ranges" property,
> +		 * then we can try one of its channels.
> +		 */
> +		np = np->parent;
> +		if (np && !of_get_property(np, "io-channel-ranges", NULL))
> +			break;
> +	}
> +	return chan;
> +}
> +
> +static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	struct iio_channel *chans;
> +	int i, mapind, nummaps = 0;
> +	int ret;
> +
> +	do {
> +		ret = of_parse_phandle_with_args(dev->of_node,
> +						 "io-channels",
> +						 "#io-channel-cells",
> +						 nummaps, NULL);
> +		if (ret < 0)
> +			break;
> +	} while (++nummaps);
> +
> +	if (nummaps == 0)	/* no error, return NULL to search map table */
> +		return NULL;
> +
> +	/* NULL terminated array to save passing size */
> +	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> +	if (chans == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Search for OF matches */
> +	for (mapind = 0; mapind < nummaps; mapind++) {
> +		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
> +					   mapind);
> +		if (ret)
> +			goto error_free_chans;
> +	}
> +	return chans;
> +
> +error_free_chans:
> +	for (i = 0; i < mapind; i++)
> +		iio_device_put(chans[i].indio_dev);
> +	kfree(chans);
> +	return ERR_PTR(ret);
> +}
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device *dev, const char *name)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
> +
> +static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
>  
>  static struct iio_channel *iio_channel_get_sys(const char *name,
>  					       const char *channel_name)
> @@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  				    const char *channel_name)
>  {
>  	const char *name = dev ? dev_name(dev) : NULL;
> +	struct iio_channel *channel;
>  
> +	if (dev) {
> +		channel = of_iio_channel_get_by_name(dev->of_node,
> +						     channel_name);
> +		if (channel != NULL)
> +			return channel;
> +	}
>  	return iio_channel_get_sys(name, channel_name);
>  }
>  EXPORT_SYMBOL_GPL(iio_channel_get);
> @@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  
>  	if (dev == NULL)
>  		return ERR_PTR(-EINVAL);
> +
> +	chans = of_iio_channel_get_all(dev);
> +	if (chans)
> +		return chans;
> +
>  	name = dev_name(dev);
>  
>  	mutex_lock(&iio_map_list_lock);
> -- 
> 1.7.9.7
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/2] iio: Add OF support
  2013-02-04 20:26     ` Guenter Roeck
@ 2013-02-05 15:22         ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-05 15:22 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> ---
> One open question is how to assign of_node to the iio device. We can either do it
> in each driver (which turns out to be a huge patchset), or add something like the
> following to iio_device_register.
> 
> 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> 
[ ... ]
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device *dev, const char *name)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
> +

The above should be

static inline struct iio_channel *
of_iio_channel_get_by_name(struct device_node *np, const char *name)
{
	return NULL;
}

Will be fixed in the next version.

Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/2] iio: Add OF support
@ 2013-02-05 15:22         ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-05 15:22 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> One open question is how to assign of_node to the iio device. We can either do it
> in each driver (which turns out to be a huge patchset), or add something like the
> following to iio_device_register.
> 
> 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> 
[ ... ]
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device *dev, const char *name)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
> +

The above should be

static inline struct iio_channel *
of_iio_channel_get_by_name(struct device_node *np, const char *name)
{
	return NULL;
}

Will be fixed in the next version.

Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4 2/2] iio: Add OF support
  2013-02-04 20:26     ` Guenter Roeck
@ 2013-02-06 18:29         ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-06 18:29 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

Provide bindings and parse OF data during initialization.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
  undefined, and wrong return value.
- Initialize indio_dev->of_node in iio_device_register if the calling driver
  neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
  of strcmp. Rename iio_dev_type to iio_device_type (to match other device
  types) and make it global for that purpose. Check the OF node first, then the
  device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
  __of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
  an error if there is a problem with consistency or if the provider device is
  not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
  or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
  not used in the code, and it is not entirely clear what it would be used for.
  If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
  if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
  name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.

 drivers/iio/iio_core.h                             |    1 +
 drivers/iio/industrialio-core.c                    |    8 +-
 drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
 4 files changed, 268 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 0000000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+		   with a single IIO output and 1 for nodes with multiple
+		   IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+	adc: voltage-sensor@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+Example for a configuration with trigger:
+
+	adc@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+
+		adc: iio-device {
+			#io-channel-cells = <1>;
+		};
+		trigger: iio-trigger {
+			#io-channel-cells = <1>;
+		};
+	};
+
+==IIO consumers==
+
+Required properties:
+io-channels:	List of phandle and IIO specifier pairs, one pair
+		for each IIO input to the device. Note: if the
+		IIO provider specifies '0' for #io-channel-cells,
+		then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+		List of IIO input name strings sorted in the same
+		order as the io-channels property. Consumers drivers
+		will use io-channel-names to match IIO input names
+		with IIO specifiers.
+io-channel-ranges:
+		Empty property indicating that child nodes can inherit named
+		IIO channels from this node. Useful for bus nodes to provide
+		and IIO channel to their children.
+
+For example:
+
+	device {
+		io-channels = <&adc 1>, <&ref 0>;
+		io-channel-names = "vcc", "vdd";
+	};
+
+This represents a device with two IIO inputs, named "vcc" and "vdd".
+The vcc channel is connected to output 1 of the &adc device, and the
+vdd channel is connected to output 0 of the &ref device.
+
+==Example==
+
+	adc: max1139@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+	...
+
+	iio_hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
+			<&adc 3>, <&adc 4>, <&adc 5>,
+			<&adc 6>, <&adc 7>, <&adc 8>,
+			<&adc 9>, <&adc 10>, <&adc 11>;
+		io-channel-names = "vcc", "vdd", "vref", "1.2V";
+	};
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index f652e6a..05c1b74 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -18,6 +18,7 @@
 struct iio_chan_spec;
 struct iio_dev;
 
+extern struct device_type iio_device_type;
 
 int __iio_add_chan_devattr(const char *postfix,
 			   struct iio_chan_spec const *chan,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8848f16..6d8b027 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
 	kfree(indio_dev);
 }
 
-static struct device_type iio_dev_type = {
+struct device_type iio_device_type = {
 	.name = "iio_device",
 	.release = iio_dev_release,
 };
@@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
 
 	if (dev) {
 		dev->dev.groups = dev->groups;
-		dev->dev.type = &iio_dev_type;
+		dev->dev.type = &iio_device_type;
 		dev->dev.bus = &iio_bus_type;
 		device_initialize(&dev->dev);
 		dev_set_drvdata(&dev->dev, (void *)dev);
@@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
 {
 	int ret;
 
+	/* If the calling driver did not initialize of_node, do it here */
+	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
+		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
+
 	/* configure elements for the chrdev */
 	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
 
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..795d100 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include "iio_core.h"
@@ -92,6 +93,164 @@ static const struct iio_chan_spec
 	return chan;
 }
 
+#ifdef CONFIG_OF
+
+static int iio_dev_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data && dev->type == &iio_device_type;
+}
+
+static int __of_iio_channel_get(struct iio_channel *channel,
+				struct device_node *np, int index)
+{
+	struct device *idev;
+	struct iio_dev *indio_dev;
+	int err;
+	struct of_phandle_args iiospec;
+
+	err = of_parse_phandle_with_args(np, "io-channels",
+					 "#io-channel-cells",
+					 index, &iiospec);
+	if (err)
+		return err;
+
+	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
+			       iio_dev_node_match);
+	of_node_put(iiospec.np);
+	if (idev == NULL)
+		return -EPROBE_DEFER;
+
+	indio_dev = dev_to_iio_dev(idev);
+	channel->indio_dev = indio_dev;
+	index = iiospec.args_count ? iiospec.args[0] : 0;
+	if (index >= indio_dev->num_channels) {
+		return -EINVAL;
+		goto err_put;
+	}
+	channel->channel = &indio_dev->channels[index];
+
+	return 0;
+
+err_put:
+	iio_device_put(indio_dev);
+	return err;
+}
+
+static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
+{
+	struct iio_channel *channel;
+	int err;
+
+	if (index < 0)
+		return ERR_PTR(-EINVAL);
+
+	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
+	if (channel == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	err = __of_iio_channel_get(channel, np, index);
+	if (err)
+		goto err_free_channel;
+
+	return channel;
+
+err_free_channel:
+	kfree(channel);
+	return ERR_PTR(err);
+}
+
+static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
+						      const char *name)
+{
+	struct iio_channel *chan = NULL;
+
+	/* Walk up the tree of devices looking for a matching iio channel */
+	while (np) {
+		int index = 0;
+
+		/*
+		 * For named iio channels, first look up the name in the
+		 * "io-channel-names" property.  If it cannot be found, the
+		 * index will be an error code, and of_iio_channel_get()
+		 * will fail.
+		 */
+		if (name)
+			index = of_property_match_string(np, "io-channel-names",
+							 name);
+		chan = of_iio_channel_get(np, index);
+		if (!IS_ERR(chan))
+			break;
+		else if (name && index >= 0) {
+			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
+				np->full_name, name ? name : "", index);
+			return chan;
+		}
+
+		/*
+		 * No matching IIO channel found on this node.
+		 * If the parent node has a "io-channel-ranges" property,
+		 * then we can try one of its channels.
+		 */
+		np = np->parent;
+		if (np && !of_get_property(np, "io-channel-ranges", NULL))
+			break;
+	}
+	return chan;
+}
+
+static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	struct iio_channel *chans;
+	int i, mapind, nummaps = 0;
+	int ret;
+
+	do {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "io-channels",
+						 "#io-channel-cells",
+						 nummaps, NULL);
+		if (ret < 0)
+			break;
+	} while (++nummaps);
+
+	if (nummaps == 0)	/* no error, return NULL to search map table */
+		return NULL;
+
+	/* NULL terminated array to save passing size */
+	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
+	if (chans == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	/* Search for OF matches */
+	for (mapind = 0; mapind < nummaps; mapind++) {
+		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
+					   mapind);
+		if (ret)
+			goto error_free_chans;
+	}
+	return chans;
+
+error_free_chans:
+	for (i = 0; i < mapind; i++)
+		iio_device_put(chans[i].indio_dev);
+	kfree(chans);
+	return ERR_PTR(ret);
+}
+
+#else /* CONFIG_OF */
+
+static inline struct iio_channel *
+of_iio_channel_get_by_name(struct device_node *np, const char *name)
+{
+	return NULL;
+}
+
+static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
 
 static struct iio_channel *iio_channel_get_sys(const char *name,
 					       const char *channel_name)
@@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *channel_name)
 {
 	const char *name = dev ? dev_name(dev) : NULL;
+	struct iio_channel *channel;
 
+	if (dev) {
+		channel = of_iio_channel_get_by_name(dev->of_node,
+						     channel_name);
+		if (channel != NULL)
+			return channel;
+	}
 	return iio_channel_get_sys(name, channel_name);
 }
 EXPORT_SYMBOL_GPL(iio_channel_get);
@@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 
 	if (dev == NULL)
 		return ERR_PTR(-EINVAL);
+
+	chans = of_iio_channel_get_all(dev);
+	if (chans)
+		return chans;
+
 	name = dev_name(dev);
 
 	mutex_lock(&iio_map_list_lock);
-- 
1.7.9.7

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v4 2/2] iio: Add OF support
@ 2013-02-06 18:29         ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-06 18:29 UTC (permalink / raw)
  To: linux-iio, Jonathan Cameron
  Cc: devicetree-discuss, Naveen Krishna Chatradhi, Lars-Peter Clausen,
	Doug Anderson, Tomasz Figa, Grant Likely, Rob Herring,
	MyungJoo Ham, Chanwoo Choi, Anton Vorontsov, David Woodhouse

Provide bindings and parse OF data during initialization.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
  undefined, and wrong return value.
- Initialize indio_dev->of_node in iio_device_register if the calling driver
  neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
  of strcmp. Rename iio_dev_type to iio_device_type (to match other device
  types) and make it global for that purpose. Check the OF node first, then the
  device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
  __of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
  an error if there is a problem with consistency or if the provider device is
  not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
  or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
  not used in the code, and it is not entirely clear what it would be used for.
  If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
  if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
  name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.

 drivers/iio/iio_core.h                             |    1 +
 drivers/iio/industrialio-core.c                    |    8 +-
 drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
 4 files changed, 268 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 0000000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+		   with a single IIO output and 1 for nodes with multiple
+		   IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+	adc: voltage-sensor@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+Example for a configuration with trigger:
+
+	adc@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+
+		adc: iio-device {
+			#io-channel-cells = <1>;
+		};
+		trigger: iio-trigger {
+			#io-channel-cells = <1>;
+		};
+	};
+
+==IIO consumers==
+
+Required properties:
+io-channels:	List of phandle and IIO specifier pairs, one pair
+		for each IIO input to the device. Note: if the
+		IIO provider specifies '0' for #io-channel-cells,
+		then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+		List of IIO input name strings sorted in the same
+		order as the io-channels property. Consumers drivers
+		will use io-channel-names to match IIO input names
+		with IIO specifiers.
+io-channel-ranges:
+		Empty property indicating that child nodes can inherit named
+		IIO channels from this node. Useful for bus nodes to provide
+		and IIO channel to their children.
+
+For example:
+
+	device {
+		io-channels = <&adc 1>, <&ref 0>;
+		io-channel-names = "vcc", "vdd";
+	};
+
+This represents a device with two IIO inputs, named "vcc" and "vdd".
+The vcc channel is connected to output 1 of the &adc device, and the
+vdd channel is connected to output 0 of the &ref device.
+
+==Example==
+
+	adc: max1139@35 {
+		compatible = "maxim,max1139";
+		reg = <0x35>;
+		#io-channel-cells = <1>;
+	};
+
+	...
+
+	iio_hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
+			<&adc 3>, <&adc 4>, <&adc 5>,
+			<&adc 6>, <&adc 7>, <&adc 8>,
+			<&adc 9>, <&adc 10>, <&adc 11>;
+		io-channel-names = "vcc", "vdd", "vref", "1.2V";
+	};
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index f652e6a..05c1b74 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -18,6 +18,7 @@
 struct iio_chan_spec;
 struct iio_dev;
 
+extern struct device_type iio_device_type;
 
 int __iio_add_chan_devattr(const char *postfix,
 			   struct iio_chan_spec const *chan,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8848f16..6d8b027 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
 	kfree(indio_dev);
 }
 
-static struct device_type iio_dev_type = {
+struct device_type iio_device_type = {
 	.name = "iio_device",
 	.release = iio_dev_release,
 };
@@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
 
 	if (dev) {
 		dev->dev.groups = dev->groups;
-		dev->dev.type = &iio_dev_type;
+		dev->dev.type = &iio_device_type;
 		dev->dev.bus = &iio_bus_type;
 		device_initialize(&dev->dev);
 		dev_set_drvdata(&dev->dev, (void *)dev);
@@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
 {
 	int ret;
 
+	/* If the calling driver did not initialize of_node, do it here */
+	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
+		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
+
 	/* configure elements for the chrdev */
 	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
 
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..795d100 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include "iio_core.h"
@@ -92,6 +93,164 @@ static const struct iio_chan_spec
 	return chan;
 }
 
+#ifdef CONFIG_OF
+
+static int iio_dev_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data && dev->type == &iio_device_type;
+}
+
+static int __of_iio_channel_get(struct iio_channel *channel,
+				struct device_node *np, int index)
+{
+	struct device *idev;
+	struct iio_dev *indio_dev;
+	int err;
+	struct of_phandle_args iiospec;
+
+	err = of_parse_phandle_with_args(np, "io-channels",
+					 "#io-channel-cells",
+					 index, &iiospec);
+	if (err)
+		return err;
+
+	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
+			       iio_dev_node_match);
+	of_node_put(iiospec.np);
+	if (idev == NULL)
+		return -EPROBE_DEFER;
+
+	indio_dev = dev_to_iio_dev(idev);
+	channel->indio_dev = indio_dev;
+	index = iiospec.args_count ? iiospec.args[0] : 0;
+	if (index >= indio_dev->num_channels) {
+		return -EINVAL;
+		goto err_put;
+	}
+	channel->channel = &indio_dev->channels[index];
+
+	return 0;
+
+err_put:
+	iio_device_put(indio_dev);
+	return err;
+}
+
+static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
+{
+	struct iio_channel *channel;
+	int err;
+
+	if (index < 0)
+		return ERR_PTR(-EINVAL);
+
+	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
+	if (channel == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	err = __of_iio_channel_get(channel, np, index);
+	if (err)
+		goto err_free_channel;
+
+	return channel;
+
+err_free_channel:
+	kfree(channel);
+	return ERR_PTR(err);
+}
+
+static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
+						      const char *name)
+{
+	struct iio_channel *chan = NULL;
+
+	/* Walk up the tree of devices looking for a matching iio channel */
+	while (np) {
+		int index = 0;
+
+		/*
+		 * For named iio channels, first look up the name in the
+		 * "io-channel-names" property.  If it cannot be found, the
+		 * index will be an error code, and of_iio_channel_get()
+		 * will fail.
+		 */
+		if (name)
+			index = of_property_match_string(np, "io-channel-names",
+							 name);
+		chan = of_iio_channel_get(np, index);
+		if (!IS_ERR(chan))
+			break;
+		else if (name && index >= 0) {
+			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
+				np->full_name, name ? name : "", index);
+			return chan;
+		}
+
+		/*
+		 * No matching IIO channel found on this node.
+		 * If the parent node has a "io-channel-ranges" property,
+		 * then we can try one of its channels.
+		 */
+		np = np->parent;
+		if (np && !of_get_property(np, "io-channel-ranges", NULL))
+			break;
+	}
+	return chan;
+}
+
+static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	struct iio_channel *chans;
+	int i, mapind, nummaps = 0;
+	int ret;
+
+	do {
+		ret = of_parse_phandle_with_args(dev->of_node,
+						 "io-channels",
+						 "#io-channel-cells",
+						 nummaps, NULL);
+		if (ret < 0)
+			break;
+	} while (++nummaps);
+
+	if (nummaps == 0)	/* no error, return NULL to search map table */
+		return NULL;
+
+	/* NULL terminated array to save passing size */
+	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
+	if (chans == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	/* Search for OF matches */
+	for (mapind = 0; mapind < nummaps; mapind++) {
+		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
+					   mapind);
+		if (ret)
+			goto error_free_chans;
+	}
+	return chans;
+
+error_free_chans:
+	for (i = 0; i < mapind; i++)
+		iio_device_put(chans[i].indio_dev);
+	kfree(chans);
+	return ERR_PTR(ret);
+}
+
+#else /* CONFIG_OF */
+
+static inline struct iio_channel *
+of_iio_channel_get_by_name(struct device_node *np, const char *name)
+{
+	return NULL;
+}
+
+static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_OF */
 
 static struct iio_channel *iio_channel_get_sys(const char *name,
 					       const char *channel_name)
@@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
 				    const char *channel_name)
 {
 	const char *name = dev ? dev_name(dev) : NULL;
+	struct iio_channel *channel;
 
+	if (dev) {
+		channel = of_iio_channel_get_by_name(dev->of_node,
+						     channel_name);
+		if (channel != NULL)
+			return channel;
+	}
 	return iio_channel_get_sys(name, channel_name);
 }
 EXPORT_SYMBOL_GPL(iio_channel_get);
@@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 
 	if (dev == NULL)
 		return ERR_PTR(-EINVAL);
+
+	chans = of_iio_channel_get_all(dev);
+	if (chans)
+		return chans;
+
 	name = dev_name(dev);
 
 	mutex_lock(&iio_map_list_lock);
-- 
1.7.9.7

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
  2013-02-04 23:57         ` Chanwoo Choi
@ 2013-02-06 19:30             ` Jonathan Cameron
  -1 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-06 19:30 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: Guenter Roeck, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Anton Vorontsov, David Woodhouse

On 02/04/2013 11:57 PM, Chanwoo Choi wrote:
> On 02/05/2013 05:26 AM, Guenter Roeck wrote:
>> For iio_channel_get to work with OF based configurations, it needs the
>> consumer device pointer instead of the consumer device name as argument.
>>
>> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Applied to togreg branch of iio.git.

Thanks all.

I'll hold off on taking the actual device tree patch (2 of this series)
for a day or two to give more time for comments on that.
>> ---
>>   drivers/extcon/extcon-adc-jack.c    |    3 +--
>>   drivers/iio/inkern.c                |   11 ++++++++++-
>>   drivers/power/generic-adc-battery.c |    4 ++--
>>   drivers/power/lp8788-charger.c      |    8 ++++----
>>   include/linux/iio/consumer.h        |    5 +++--
>>   5 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
>> index eda2a1a..d0233cd 100644
>> --- a/drivers/extcon/extcon-adc-jack.c
>> +++ b/drivers/extcon/extcon-adc-jack.c
>> @@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
>>           ;
>>       data->num_conditions = i;
>>   -    data->chan = iio_channel_get(dev_name(&pdev->dev),
>> -            pdata->consumer_channel);
>> +    data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
>>       if (IS_ERR(data->chan)) {
>>           err = PTR_ERR(data->chan);
>>           goto out;
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index c42aba6..b289915 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -93,7 +93,8 @@ static const struct iio_chan_spec
> Ack for drivers/extcon .
> 
> Acked-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Thanks,
> Chanwoo Choi
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
@ 2013-02-06 19:30             ` Jonathan Cameron
  0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-06 19:30 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: Guenter Roeck, linux-iio, devicetree-discuss,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Anton Vorontsov, David Woodhouse

On 02/04/2013 11:57 PM, Chanwoo Choi wrote:
> On 02/05/2013 05:26 AM, Guenter Roeck wrote:
>> For iio_channel_get to work with OF based configurations, it needs the
>> consumer device pointer instead of the consumer device name as argument.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Applied to togreg branch of iio.git.

Thanks all.

I'll hold off on taking the actual device tree patch (2 of this series)
for a day or two to give more time for comments on that.
>> ---
>>   drivers/extcon/extcon-adc-jack.c    |    3 +--
>>   drivers/iio/inkern.c                |   11 ++++++++++-
>>   drivers/power/generic-adc-battery.c |    4 ++--
>>   drivers/power/lp8788-charger.c      |    8 ++++----
>>   include/linux/iio/consumer.h        |    5 +++--
>>   5 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
>> index eda2a1a..d0233cd 100644
>> --- a/drivers/extcon/extcon-adc-jack.c
>> +++ b/drivers/extcon/extcon-adc-jack.c
>> @@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
>>           ;
>>       data->num_conditions = i;
>>   -    data->chan = iio_channel_get(dev_name(&pdev->dev),
>> -            pdata->consumer_channel);
>> +    data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
>>       if (IS_ERR(data->chan)) {
>>           err = PTR_ERR(data->chan);
>>           goto out;
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index c42aba6..b289915 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -93,7 +93,8 @@ static const struct iio_chan_spec
> Ack for drivers/extcon .
> 
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> 
> Thanks,
> Chanwoo Choi
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
  2013-02-06 18:29         ` Guenter Roeck
@ 2013-02-06 19:37             ` Jonathan Cameron
  -1 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-06 19:37 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
looks good to me.  Couple of little queries inline.

> ---
> v4:
> - Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
>   undefined, and wrong return value.
> - Initialize indio_dev->of_node in iio_device_register if the calling driver
>   neglected to do it.
> v3:
> - Cleaned up documentation (formatting, left-over clock references)
> - Updated bindings description to permit sub-devices
> - When searching for iio devices, use the pointer to the iio device type instead
>   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
>   types) and make it global for that purpose. Check the OF node first, then the
>   device type, as the node is less likely to match.
> - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
>   __of_iio_channel_get.
> - Return NULL from of_iio_channel_get_by_name if nothing is found, or
>   an error if there is a problem with consistency or if the provider device is
>   not yet available.
> - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
>   or an error, and continue otherwise.
> v2:
> - Rebased to iio/togreg
> - Documentation update per feedback
> - Dropped io-channel-output-names from the bindings document. The property is
>   not used in the code, and it is not entirely clear what it would be used for.
>   If there is a need for it, we can add it back in later on.
> - Don't export OF specific API calls
> - For OF support, no longer depend on iio_map
> - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
>   if it is not selected.
> - Change iio_channel_get to take device pointer as argument instead of device
>   name. Retain old API as of_iio_channel_get_sys.
> - iio_channel_get now works for both OF and non-OF configurations
> - Use regulator to get vref for max1363 driver.
> 
>  drivers/iio/iio_core.h                             |    1 +
>  drivers/iio/industrialio-core.c                    |    8 +-
>  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
>  4 files changed, 268 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> new file mode 100644
> index 0000000..2475c2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -0,0 +1,90 @@
> +This binding is a work-in-progress. It is derived from clock bindings,
> +and based on suggestions from Lars-Peter Clausen [1].
> +
> +Sources of IIO channels can be represented by any node in the device
> +tree. Those nodes are designated as IIO providers. IIO consumer
> +nodes use a phandle and IIO specifier pair to connect IIO provider
> +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> +specifier is an array of one or more cells identifying the IIO
> +output on a device. The length of an IIO specifier is defined by the
> +value of a #io-channel-cells property in the IIO provider node.
> +
> +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> +
> +==IIO providers==
> +
> +Required properties:
> +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> +		   with a single IIO output and 1 for nodes with multiple
> +		   IIO outputs.
> +
> +Example for a simple configuration with no trigger:
> +
> +	adc: voltage-sensor@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +Example for a configuration with trigger:
> +
> +	adc@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +
> +		adc: iio-device {
> +			#io-channel-cells = <1>;
> +		};
> +		trigger: iio-trigger {
> +			#io-channel-cells = <1>;
Why would the trigger have channel-cells?
So far we don't have any device tree stuff for the triggers.
> +		};
> +	};
> +
> +==IIO consumers==
> +
> +Required properties:
> +io-channels:	List of phandle and IIO specifier pairs, one pair
> +		for each IIO input to the device. Note: if the
> +		IIO provider specifies '0' for #io-channel-cells,
> +		then only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +io-channel-names:
> +		List of IIO input name strings sorted in the same
> +		order as the io-channels property. Consumers drivers
> +		will use io-channel-names to match IIO input names
> +		with IIO specifiers.
> +io-channel-ranges:
> +		Empty property indicating that child nodes can inherit named
> +		IIO channels from this node. Useful for bus nodes to provide
> +		and IIO channel to their children.
> +
> +For example:
> +
> +	device {
> +		io-channels = <&adc 1>, <&ref 0>;
> +		io-channel-names = "vcc", "vdd";
> +	};
> +
> +This represents a device with two IIO inputs, named "vcc" and "vdd".
> +The vcc channel is connected to output 1 of the &adc device, and the
> +vdd channel is connected to output 0 of the &ref device.
> +
> +==Example==
> +
> +	adc: max1139@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +	...
> +
> +	iio_hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> +			<&adc 3>, <&adc 4>, <&adc 5>,
> +			<&adc 6>, <&adc 7>, <&adc 8>,
> +			<&adc 9>, <&adc 10>, <&adc 11>;
> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
This example still seems a little odd. Why give one where there are
more channels than names?
> +	};
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index f652e6a..05c1b74 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -18,6 +18,7 @@
>  struct iio_chan_spec;
>  struct iio_dev;
>  
> +extern struct device_type iio_device_type;
>  
>  int __iio_add_chan_devattr(const char *postfix,
>  			   struct iio_chan_spec const *chan,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8848f16..6d8b027 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
>  	kfree(indio_dev);
>  }
>  
> -static struct device_type iio_dev_type = {
> +struct device_type iio_device_type = {
>  	.name = "iio_device",
>  	.release = iio_dev_release,
>  };
> @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>  
>  	if (dev) {
>  		dev->dev.groups = dev->groups;
> -		dev->dev.type = &iio_dev_type;
> +		dev->dev.type = &iio_device_type;
>  		dev->dev.bus = &iio_bus_type;
>  		device_initialize(&dev->dev);
>  		dev_set_drvdata(&dev->dev, (void *)dev);
> @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
>  {
>  	int ret;
>  
> +	/* If the calling driver did not initialize of_node, do it here */
> +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> +
I guess that's sensible, though I'd be inclined to check it and throw a wobbly
if it isn't defined.  That way we have one 'right' place to do it.

>  	/* configure elements for the chrdev */
>  	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
>  
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index b289915..795d100 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -10,6 +10,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
> @@ -92,6 +93,164 @@ static const struct iio_chan_spec
>  	return chan;
>  }
>  
> +#ifdef CONFIG_OF
> +
> +static int iio_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data && dev->type == &iio_device_type;
> +}
> +
> +static int __of_iio_channel_get(struct iio_channel *channel,
> +				struct device_node *np, int index)
> +{
> +	struct device *idev;
> +	struct iio_dev *indio_dev;
> +	int err;
> +	struct of_phandle_args iiospec;
> +
> +	err = of_parse_phandle_with_args(np, "io-channels",
> +					 "#io-channel-cells",
> +					 index, &iiospec);
> +	if (err)
> +		return err;
> +
> +	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
> +			       iio_dev_node_match);
> +	of_node_put(iiospec.np);
> +	if (idev == NULL)
> +		return -EPROBE_DEFER;
> +
> +	indio_dev = dev_to_iio_dev(idev);
> +	channel->indio_dev = indio_dev;
> +	index = iiospec.args_count ? iiospec.args[0] : 0;
> +	if (index >= indio_dev->num_channels) {
> +		return -EINVAL;
> +		goto err_put;
> +	}
> +	channel->channel = &indio_dev->channels[index];
> +
> +	return 0;
> +
> +err_put:
> +	iio_device_put(indio_dev);
> +	return err;
> +}
> +
> +static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
> +{
> +	struct iio_channel *channel;
> +	int err;
> +
> +	if (index < 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (channel == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = __of_iio_channel_get(channel, np, index);
> +	if (err)
> +		goto err_free_channel;
> +
> +	return channel;
> +
> +err_free_channel:
> +	kfree(channel);
> +	return ERR_PTR(err);
> +}
> +
> +static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> +						      const char *name)
> +{
> +	struct iio_channel *chan = NULL;
> +
> +	/* Walk up the tree of devices looking for a matching iio channel */
> +	while (np) {
> +		int index = 0;
> +
> +		/*
> +		 * For named iio channels, first look up the name in the
> +		 * "io-channel-names" property.  If it cannot be found, the
> +		 * index will be an error code, and of_iio_channel_get()
> +		 * will fail.
> +		 */
> +		if (name)
> +			index = of_property_match_string(np, "io-channel-names",
> +							 name);
> +		chan = of_iio_channel_get(np, index);
> +		if (!IS_ERR(chan))
> +			break;
> +		else if (name && index >= 0) {
> +			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
> +				np->full_name, name ? name : "", index);
> +			return chan;
> +		}
> +
> +		/*
> +		 * No matching IIO channel found on this node.
> +		 * If the parent node has a "io-channel-ranges" property,
> +		 * then we can try one of its channels.
> +		 */
> +		np = np->parent;
> +		if (np && !of_get_property(np, "io-channel-ranges", NULL))
> +			break;
> +	}
> +	return chan;
> +}
> +
> +static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	struct iio_channel *chans;
> +	int i, mapind, nummaps = 0;
> +	int ret;
> +
> +	do {
> +		ret = of_parse_phandle_with_args(dev->of_node,
> +						 "io-channels",
> +						 "#io-channel-cells",
> +						 nummaps, NULL);
> +		if (ret < 0)
> +			break;
> +	} while (++nummaps);
> +
> +	if (nummaps == 0)	/* no error, return NULL to search map table */
> +		return NULL;
> +
> +	/* NULL terminated array to save passing size */
> +	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> +	if (chans == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Search for OF matches */
> +	for (mapind = 0; mapind < nummaps; mapind++) {
> +		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
> +					   mapind);
> +		if (ret)
> +			goto error_free_chans;
> +	}
> +	return chans;
> +
> +error_free_chans:
> +	for (i = 0; i < mapind; i++)
> +		iio_device_put(chans[i].indio_dev);
> +	kfree(chans);
> +	return ERR_PTR(ret);
> +}
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device_node *np, const char *name)
> +{
> +	return NULL;
> +}
> +
> +static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
>  
>  static struct iio_channel *iio_channel_get_sys(const char *name,
>  					       const char *channel_name)
> @@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  				    const char *channel_name)
>  {
>  	const char *name = dev ? dev_name(dev) : NULL;
> +	struct iio_channel *channel;
>  
> +	if (dev) {
> +		channel = of_iio_channel_get_by_name(dev->of_node,
> +						     channel_name);
> +		if (channel != NULL)
> +			return channel;
> +	}
>  	return iio_channel_get_sys(name, channel_name);
>  }
>  EXPORT_SYMBOL_GPL(iio_channel_get);
> @@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  
>  	if (dev == NULL)
>  		return ERR_PTR(-EINVAL);
> +
> +	chans = of_iio_channel_get_all(dev);
> +	if (chans)
> +		return chans;
> +
>  	name = dev_name(dev);
>  
>  	mutex_lock(&iio_map_list_lock);
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
@ 2013-02-06 19:37             ` Jonathan Cameron
  0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-06 19:37 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-iio, devicetree-discuss, Naveen Krishna Chatradhi,
	Lars-Peter Clausen, Doug Anderson, Tomasz Figa, Grant Likely,
	Rob Herring, MyungJoo Ham, Chanwoo Choi, Anton Vorontsov,
	David Woodhouse

On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> Provide bindings and parse OF data during initialization.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
looks good to me.  Couple of little queries inline.

> ---
> v4:
> - Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
>   undefined, and wrong return value.
> - Initialize indio_dev->of_node in iio_device_register if the calling driver
>   neglected to do it.
> v3:
> - Cleaned up documentation (formatting, left-over clock references)
> - Updated bindings description to permit sub-devices
> - When searching for iio devices, use the pointer to the iio device type instead
>   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
>   types) and make it global for that purpose. Check the OF node first, then the
>   device type, as the node is less likely to match.
> - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
>   __of_iio_channel_get.
> - Return NULL from of_iio_channel_get_by_name if nothing is found, or
>   an error if there is a problem with consistency or if the provider device is
>   not yet available.
> - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
>   or an error, and continue otherwise.
> v2:
> - Rebased to iio/togreg
> - Documentation update per feedback
> - Dropped io-channel-output-names from the bindings document. The property is
>   not used in the code, and it is not entirely clear what it would be used for.
>   If there is a need for it, we can add it back in later on.
> - Don't export OF specific API calls
> - For OF support, no longer depend on iio_map
> - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
>   if it is not selected.
> - Change iio_channel_get to take device pointer as argument instead of device
>   name. Retain old API as of_iio_channel_get_sys.
> - iio_channel_get now works for both OF and non-OF configurations
> - Use regulator to get vref for max1363 driver.
> 
>  drivers/iio/iio_core.h                             |    1 +
>  drivers/iio/industrialio-core.c                    |    8 +-
>  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
>  4 files changed, 268 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> new file mode 100644
> index 0000000..2475c2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -0,0 +1,90 @@
> +This binding is a work-in-progress. It is derived from clock bindings,
> +and based on suggestions from Lars-Peter Clausen [1].
> +
> +Sources of IIO channels can be represented by any node in the device
> +tree. Those nodes are designated as IIO providers. IIO consumer
> +nodes use a phandle and IIO specifier pair to connect IIO provider
> +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> +specifier is an array of one or more cells identifying the IIO
> +output on a device. The length of an IIO specifier is defined by the
> +value of a #io-channel-cells property in the IIO provider node.
> +
> +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> +
> +==IIO providers==
> +
> +Required properties:
> +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> +		   with a single IIO output and 1 for nodes with multiple
> +		   IIO outputs.
> +
> +Example for a simple configuration with no trigger:
> +
> +	adc: voltage-sensor@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +Example for a configuration with trigger:
> +
> +	adc@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +
> +		adc: iio-device {
> +			#io-channel-cells = <1>;
> +		};
> +		trigger: iio-trigger {
> +			#io-channel-cells = <1>;
Why would the trigger have channel-cells?
So far we don't have any device tree stuff for the triggers.
> +		};
> +	};
> +
> +==IIO consumers==
> +
> +Required properties:
> +io-channels:	List of phandle and IIO specifier pairs, one pair
> +		for each IIO input to the device. Note: if the
> +		IIO provider specifies '0' for #io-channel-cells,
> +		then only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +io-channel-names:
> +		List of IIO input name strings sorted in the same
> +		order as the io-channels property. Consumers drivers
> +		will use io-channel-names to match IIO input names
> +		with IIO specifiers.
> +io-channel-ranges:
> +		Empty property indicating that child nodes can inherit named
> +		IIO channels from this node. Useful for bus nodes to provide
> +		and IIO channel to their children.
> +
> +For example:
> +
> +	device {
> +		io-channels = <&adc 1>, <&ref 0>;
> +		io-channel-names = "vcc", "vdd";
> +	};
> +
> +This represents a device with two IIO inputs, named "vcc" and "vdd".
> +The vcc channel is connected to output 1 of the &adc device, and the
> +vdd channel is connected to output 0 of the &ref device.
> +
> +==Example==
> +
> +	adc: max1139@35 {
> +		compatible = "maxim,max1139";
> +		reg = <0x35>;
> +		#io-channel-cells = <1>;
> +	};
> +
> +	...
> +
> +	iio_hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> +			<&adc 3>, <&adc 4>, <&adc 5>,
> +			<&adc 6>, <&adc 7>, <&adc 8>,
> +			<&adc 9>, <&adc 10>, <&adc 11>;
> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
This example still seems a little odd. Why give one where there are
more channels than names?
> +	};
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index f652e6a..05c1b74 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -18,6 +18,7 @@
>  struct iio_chan_spec;
>  struct iio_dev;
>  
> +extern struct device_type iio_device_type;
>  
>  int __iio_add_chan_devattr(const char *postfix,
>  			   struct iio_chan_spec const *chan,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8848f16..6d8b027 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
>  	kfree(indio_dev);
>  }
>  
> -static struct device_type iio_dev_type = {
> +struct device_type iio_device_type = {
>  	.name = "iio_device",
>  	.release = iio_dev_release,
>  };
> @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>  
>  	if (dev) {
>  		dev->dev.groups = dev->groups;
> -		dev->dev.type = &iio_dev_type;
> +		dev->dev.type = &iio_device_type;
>  		dev->dev.bus = &iio_bus_type;
>  		device_initialize(&dev->dev);
>  		dev_set_drvdata(&dev->dev, (void *)dev);
> @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
>  {
>  	int ret;
>  
> +	/* If the calling driver did not initialize of_node, do it here */
> +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> +
I guess that's sensible, though I'd be inclined to check it and throw a wobbly
if it isn't defined.  That way we have one 'right' place to do it.

>  	/* configure elements for the chrdev */
>  	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
>  
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index b289915..795d100 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -10,6 +10,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
> @@ -92,6 +93,164 @@ static const struct iio_chan_spec
>  	return chan;
>  }
>  
> +#ifdef CONFIG_OF
> +
> +static int iio_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data && dev->type == &iio_device_type;
> +}
> +
> +static int __of_iio_channel_get(struct iio_channel *channel,
> +				struct device_node *np, int index)
> +{
> +	struct device *idev;
> +	struct iio_dev *indio_dev;
> +	int err;
> +	struct of_phandle_args iiospec;
> +
> +	err = of_parse_phandle_with_args(np, "io-channels",
> +					 "#io-channel-cells",
> +					 index, &iiospec);
> +	if (err)
> +		return err;
> +
> +	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
> +			       iio_dev_node_match);
> +	of_node_put(iiospec.np);
> +	if (idev == NULL)
> +		return -EPROBE_DEFER;
> +
> +	indio_dev = dev_to_iio_dev(idev);
> +	channel->indio_dev = indio_dev;
> +	index = iiospec.args_count ? iiospec.args[0] : 0;
> +	if (index >= indio_dev->num_channels) {
> +		return -EINVAL;
> +		goto err_put;
> +	}
> +	channel->channel = &indio_dev->channels[index];
> +
> +	return 0;
> +
> +err_put:
> +	iio_device_put(indio_dev);
> +	return err;
> +}
> +
> +static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
> +{
> +	struct iio_channel *channel;
> +	int err;
> +
> +	if (index < 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (channel == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = __of_iio_channel_get(channel, np, index);
> +	if (err)
> +		goto err_free_channel;
> +
> +	return channel;
> +
> +err_free_channel:
> +	kfree(channel);
> +	return ERR_PTR(err);
> +}
> +
> +static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> +						      const char *name)
> +{
> +	struct iio_channel *chan = NULL;
> +
> +	/* Walk up the tree of devices looking for a matching iio channel */
> +	while (np) {
> +		int index = 0;
> +
> +		/*
> +		 * For named iio channels, first look up the name in the
> +		 * "io-channel-names" property.  If it cannot be found, the
> +		 * index will be an error code, and of_iio_channel_get()
> +		 * will fail.
> +		 */
> +		if (name)
> +			index = of_property_match_string(np, "io-channel-names",
> +							 name);
> +		chan = of_iio_channel_get(np, index);
> +		if (!IS_ERR(chan))
> +			break;
> +		else if (name && index >= 0) {
> +			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
> +				np->full_name, name ? name : "", index);
> +			return chan;
> +		}
> +
> +		/*
> +		 * No matching IIO channel found on this node.
> +		 * If the parent node has a "io-channel-ranges" property,
> +		 * then we can try one of its channels.
> +		 */
> +		np = np->parent;
> +		if (np && !of_get_property(np, "io-channel-ranges", NULL))
> +			break;
> +	}
> +	return chan;
> +}
> +
> +static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	struct iio_channel *chans;
> +	int i, mapind, nummaps = 0;
> +	int ret;
> +
> +	do {
> +		ret = of_parse_phandle_with_args(dev->of_node,
> +						 "io-channels",
> +						 "#io-channel-cells",
> +						 nummaps, NULL);
> +		if (ret < 0)
> +			break;
> +	} while (++nummaps);
> +
> +	if (nummaps == 0)	/* no error, return NULL to search map table */
> +		return NULL;
> +
> +	/* NULL terminated array to save passing size */
> +	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> +	if (chans == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Search for OF matches */
> +	for (mapind = 0; mapind < nummaps; mapind++) {
> +		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
> +					   mapind);
> +		if (ret)
> +			goto error_free_chans;
> +	}
> +	return chans;
> +
> +error_free_chans:
> +	for (i = 0; i < mapind; i++)
> +		iio_device_put(chans[i].indio_dev);
> +	kfree(chans);
> +	return ERR_PTR(ret);
> +}
> +
> +#else /* CONFIG_OF */
> +
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device_node *np, const char *name)
> +{
> +	return NULL;
> +}
> +
> +static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
>  
>  static struct iio_channel *iio_channel_get_sys(const char *name,
>  					       const char *channel_name)
> @@ -150,7 +309,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  				    const char *channel_name)
>  {
>  	const char *name = dev ? dev_name(dev) : NULL;
> +	struct iio_channel *channel;
>  
> +	if (dev) {
> +		channel = of_iio_channel_get_by_name(dev->of_node,
> +						     channel_name);
> +		if (channel != NULL)
> +			return channel;
> +	}
>  	return iio_channel_get_sys(name, channel_name);
>  }
>  EXPORT_SYMBOL_GPL(iio_channel_get);
> @@ -173,6 +339,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  
>  	if (dev == NULL)
>  		return ERR_PTR(-EINVAL);
> +
> +	chans = of_iio_channel_get_all(dev);
> +	if (chans)
> +		return chans;
> +
>  	name = dev_name(dev);
>  
>  	mutex_lock(&iio_map_list_lock);
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
  2013-02-06 19:37             ` Jonathan Cameron
@ 2013-02-06 20:05                 ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-06 20:05 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > Provide bindings and parse OF data during initialization.
> > 
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> looks good to me.  Couple of little queries inline.
> 
> > ---
> > v4:
> > - Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
> >   undefined, and wrong return value.
> > - Initialize indio_dev->of_node in iio_device_register if the calling driver
> >   neglected to do it.
> > v3:
> > - Cleaned up documentation (formatting, left-over clock references)
> > - Updated bindings description to permit sub-devices
> > - When searching for iio devices, use the pointer to the iio device type instead
> >   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
> >   types) and make it global for that purpose. Check the OF node first, then the
> >   device type, as the node is less likely to match.
> > - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
> >   __of_iio_channel_get.
> > - Return NULL from of_iio_channel_get_by_name if nothing is found, or
> >   an error if there is a problem with consistency or if the provider device is
> >   not yet available.
> > - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
> >   or an error, and continue otherwise.
> > v2:
> > - Rebased to iio/togreg
> > - Documentation update per feedback
> > - Dropped io-channel-output-names from the bindings document. The property is
> >   not used in the code, and it is not entirely clear what it would be used for.
> >   If there is a need for it, we can add it back in later on.
> > - Don't export OF specific API calls
> > - For OF support, no longer depend on iio_map
> > - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
> >   if it is not selected.
> > - Change iio_channel_get to take device pointer as argument instead of device
> >   name. Retain old API as of_iio_channel_get_sys.
> > - iio_channel_get now works for both OF and non-OF configurations
> > - Use regulator to get vref for max1363 driver.
> > 
> >  drivers/iio/iio_core.h                             |    1 +
> >  drivers/iio/industrialio-core.c                    |    8 +-
> >  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
> >  4 files changed, 268 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > new file mode 100644
> > index 0000000..2475c2e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > @@ -0,0 +1,90 @@
> > +This binding is a work-in-progress. It is derived from clock bindings,
> > +and based on suggestions from Lars-Peter Clausen [1].
> > +
> > +Sources of IIO channels can be represented by any node in the device
> > +tree. Those nodes are designated as IIO providers. IIO consumer
> > +nodes use a phandle and IIO specifier pair to connect IIO provider
> > +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> > +specifier is an array of one or more cells identifying the IIO
> > +output on a device. The length of an IIO specifier is defined by the
> > +value of a #io-channel-cells property in the IIO provider node.
> > +
> > +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> > +
> > +==IIO providers==
> > +
> > +Required properties:
> > +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> > +		   with a single IIO output and 1 for nodes with multiple
> > +		   IIO outputs.
> > +
> > +Example for a simple configuration with no trigger:
> > +
> > +	adc: voltage-sensor@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +Example for a configuration with trigger:
> > +
> > +	adc@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +
> > +		adc: iio-device {
> > +			#io-channel-cells = <1>;
> > +		};
> > +		trigger: iio-trigger {
> > +			#io-channel-cells = <1>;
> Why would the trigger have channel-cells?
> So far we don't have any device tree stuff for the triggers.

This comes from one of the examples passed around earlier.

Presumably, when triggers are supported through devicetree, you would have more
than one per device, meaning you need the same logic as for devices.

Though on the other side that isn't supported yet - I don't have hardware to
test, so I can not implement - or, rather, test - devicetree based trigger support.
I can take the entire thing out if you prefer - I don't really have a strong
opinion.

> > +		};
> > +	};
> > +
> > +==IIO consumers==
> > +
> > +Required properties:
> > +io-channels:	List of phandle and IIO specifier pairs, one pair
> > +		for each IIO input to the device. Note: if the
> > +		IIO provider specifies '0' for #io-channel-cells,
> > +		then only the phandle portion of the pair will appear.
> > +
> > +Optional properties:
> > +io-channel-names:
> > +		List of IIO input name strings sorted in the same
> > +		order as the io-channels property. Consumers drivers
> > +		will use io-channel-names to match IIO input names
> > +		with IIO specifiers.
> > +io-channel-ranges:
> > +		Empty property indicating that child nodes can inherit named
> > +		IIO channels from this node. Useful for bus nodes to provide
> > +		and IIO channel to their children.
> > +
> > +For example:
> > +
> > +	device {
> > +		io-channels = <&adc 1>, <&ref 0>;
> > +		io-channel-names = "vcc", "vdd";
> > +	};
> > +
> > +This represents a device with two IIO inputs, named "vcc" and "vdd".
> > +The vcc channel is connected to output 1 of the &adc device, and the
> > +vdd channel is connected to output 0 of the &ref device.
> > +
> > +==Example==
> > +
> > +	adc: max1139@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +	...
> > +
> > +	iio_hwmon {
> > +		compatible = "iio-hwmon";
> > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> This example still seems a little odd. Why give one where there are
> more channels than names?

I just didn't have a better idea. Do you know of another iio consumer I could
use as example, one which doesn't use the get_all API ?

> > +	};
> > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > index f652e6a..05c1b74 100644
> > --- a/drivers/iio/iio_core.h
> > +++ b/drivers/iio/iio_core.h
> > @@ -18,6 +18,7 @@
> >  struct iio_chan_spec;
> >  struct iio_dev;
> >  
> > +extern struct device_type iio_device_type;
> >  
> >  int __iio_add_chan_devattr(const char *postfix,
> >  			   struct iio_chan_spec const *chan,
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 8848f16..6d8b027 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
> >  	kfree(indio_dev);
> >  }
> >  
> > -static struct device_type iio_dev_type = {
> > +struct device_type iio_device_type = {
> >  	.name = "iio_device",
> >  	.release = iio_dev_release,
> >  };
> > @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> >  
> >  	if (dev) {
> >  		dev->dev.groups = dev->groups;
> > -		dev->dev.type = &iio_dev_type;
> > +		dev->dev.type = &iio_device_type;
> >  		dev->dev.bus = &iio_bus_type;
> >  		device_initialize(&dev->dev);
> >  		dev_set_drvdata(&dev->dev, (void *)dev);
> > @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
> >  {
> >  	int ret;
> >  
> > +	/* If the calling driver did not initialize of_node, do it here */
> > +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> > +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> > +
> I guess that's sensible, though I'd be inclined to check it and throw a wobbly
> if it isn't defined.  That way we have one 'right' place to do it.
> 
I had to look that one up :). "throw a wobbly" = "reached the end of rational thought and action"
or "to become suddenly very agitated or angry". Is that about right ?

I assume you mean the case where indio_dev->dev.of_node is NULL ? Problem is
that this can hapen anytime (and always if CONFIG_OF is undefined).
I'd be more concerned if indio_dev->dev.parent is NULL, which happens with
a couple of drivers if I interpret the code correctly. Otherwise it only matters
if the node should really point to a sub-node and not to the parent's node
itself - but presumably programmers should detect that while writing the driver.

Alternative would be to set of_node in all calling drivers. Unfortunately, there
are about one hundred callers, so that would be a substantial effort.

Not sure myself what the right approach is here - that is why I asked for
feedback about it in v3. I am open to suggestions.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
@ 2013-02-06 20:05                 ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-06 20:05 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, devicetree-discuss, Naveen Krishna Chatradhi,
	Lars-Peter Clausen, Doug Anderson, Tomasz Figa, Grant Likely,
	Rob Herring, MyungJoo Ham, Chanwoo Choi, Anton Vorontsov,
	David Woodhouse

On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > Provide bindings and parse OF data during initialization.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> looks good to me.  Couple of little queries inline.
> 
> > ---
> > v4:
> > - Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
> >   undefined, and wrong return value.
> > - Initialize indio_dev->of_node in iio_device_register if the calling driver
> >   neglected to do it.
> > v3:
> > - Cleaned up documentation (formatting, left-over clock references)
> > - Updated bindings description to permit sub-devices
> > - When searching for iio devices, use the pointer to the iio device type instead
> >   of strcmp. Rename iio_dev_type to iio_device_type (to match other device
> >   types) and make it global for that purpose. Check the OF node first, then the
> >   device type, as the node is less likely to match.
> > - Move the common code in of_iio_channel_get and of_iio_channel_get_all to
> >   __of_iio_channel_get.
> > - Return NULL from of_iio_channel_get_by_name if nothing is found, or
> >   an error if there is a problem with consistency or if the provider device is
> >   not yet available.
> > - In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
> >   or an error, and continue otherwise.
> > v2:
> > - Rebased to iio/togreg
> > - Documentation update per feedback
> > - Dropped io-channel-output-names from the bindings document. The property is
> >   not used in the code, and it is not entirely clear what it would be used for.
> >   If there is a need for it, we can add it back in later on.
> > - Don't export OF specific API calls
> > - For OF support, no longer depend on iio_map
> > - Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
> >   if it is not selected.
> > - Change iio_channel_get to take device pointer as argument instead of device
> >   name. Retain old API as of_iio_channel_get_sys.
> > - iio_channel_get now works for both OF and non-OF configurations
> > - Use regulator to get vref for max1363 driver.
> > 
> >  drivers/iio/iio_core.h                             |    1 +
> >  drivers/iio/industrialio-core.c                    |    8 +-
> >  drivers/iio/inkern.c                               |  171 ++++++++++++++++++++
> >  4 files changed, 268 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > new file mode 100644
> > index 0000000..2475c2e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > @@ -0,0 +1,90 @@
> > +This binding is a work-in-progress. It is derived from clock bindings,
> > +and based on suggestions from Lars-Peter Clausen [1].
> > +
> > +Sources of IIO channels can be represented by any node in the device
> > +tree. Those nodes are designated as IIO providers. IIO consumer
> > +nodes use a phandle and IIO specifier pair to connect IIO provider
> > +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> > +specifier is an array of one or more cells identifying the IIO
> > +output on a device. The length of an IIO specifier is defined by the
> > +value of a #io-channel-cells property in the IIO provider node.
> > +
> > +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> > +
> > +==IIO providers==
> > +
> > +Required properties:
> > +#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
> > +		   with a single IIO output and 1 for nodes with multiple
> > +		   IIO outputs.
> > +
> > +Example for a simple configuration with no trigger:
> > +
> > +	adc: voltage-sensor@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +Example for a configuration with trigger:
> > +
> > +	adc@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +
> > +		adc: iio-device {
> > +			#io-channel-cells = <1>;
> > +		};
> > +		trigger: iio-trigger {
> > +			#io-channel-cells = <1>;
> Why would the trigger have channel-cells?
> So far we don't have any device tree stuff for the triggers.

This comes from one of the examples passed around earlier.

Presumably, when triggers are supported through devicetree, you would have more
than one per device, meaning you need the same logic as for devices.

Though on the other side that isn't supported yet - I don't have hardware to
test, so I can not implement - or, rather, test - devicetree based trigger support.
I can take the entire thing out if you prefer - I don't really have a strong
opinion.

> > +		};
> > +	};
> > +
> > +==IIO consumers==
> > +
> > +Required properties:
> > +io-channels:	List of phandle and IIO specifier pairs, one pair
> > +		for each IIO input to the device. Note: if the
> > +		IIO provider specifies '0' for #io-channel-cells,
> > +		then only the phandle portion of the pair will appear.
> > +
> > +Optional properties:
> > +io-channel-names:
> > +		List of IIO input name strings sorted in the same
> > +		order as the io-channels property. Consumers drivers
> > +		will use io-channel-names to match IIO input names
> > +		with IIO specifiers.
> > +io-channel-ranges:
> > +		Empty property indicating that child nodes can inherit named
> > +		IIO channels from this node. Useful for bus nodes to provide
> > +		and IIO channel to their children.
> > +
> > +For example:
> > +
> > +	device {
> > +		io-channels = <&adc 1>, <&ref 0>;
> > +		io-channel-names = "vcc", "vdd";
> > +	};
> > +
> > +This represents a device with two IIO inputs, named "vcc" and "vdd".
> > +The vcc channel is connected to output 1 of the &adc device, and the
> > +vdd channel is connected to output 0 of the &ref device.
> > +
> > +==Example==
> > +
> > +	adc: max1139@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +	...
> > +
> > +	iio_hwmon {
> > +		compatible = "iio-hwmon";
> > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> This example still seems a little odd. Why give one where there are
> more channels than names?

I just didn't have a better idea. Do you know of another iio consumer I could
use as example, one which doesn't use the get_all API ?

> > +	};
> > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > index f652e6a..05c1b74 100644
> > --- a/drivers/iio/iio_core.h
> > +++ b/drivers/iio/iio_core.h
> > @@ -18,6 +18,7 @@
> >  struct iio_chan_spec;
> >  struct iio_dev;
> >  
> > +extern struct device_type iio_device_type;
> >  
> >  int __iio_add_chan_devattr(const char *postfix,
> >  			   struct iio_chan_spec const *chan,
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 8848f16..6d8b027 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -847,7 +847,7 @@ static void iio_dev_release(struct device *device)
> >  	kfree(indio_dev);
> >  }
> >  
> > -static struct device_type iio_dev_type = {
> > +struct device_type iio_device_type = {
> >  	.name = "iio_device",
> >  	.release = iio_dev_release,
> >  };
> > @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> >  
> >  	if (dev) {
> >  		dev->dev.groups = dev->groups;
> > -		dev->dev.type = &iio_dev_type;
> > +		dev->dev.type = &iio_device_type;
> >  		dev->dev.bus = &iio_bus_type;
> >  		device_initialize(&dev->dev);
> >  		dev_set_drvdata(&dev->dev, (void *)dev);
> > @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev *indio_dev)
> >  {
> >  	int ret;
> >  
> > +	/* If the calling driver did not initialize of_node, do it here */
> > +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> > +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> > +
> I guess that's sensible, though I'd be inclined to check it and throw a wobbly
> if it isn't defined.  That way we have one 'right' place to do it.
> 
I had to look that one up :). "throw a wobbly" = "reached the end of rational thought and action"
or "to become suddenly very agitated or angry". Is that about right ?

I assume you mean the case where indio_dev->dev.of_node is NULL ? Problem is
that this can hapen anytime (and always if CONFIG_OF is undefined).
I'd be more concerned if indio_dev->dev.parent is NULL, which happens with
a couple of drivers if I interpret the code correctly. Otherwise it only matters
if the node should really point to a sub-node and not to the parent's node
itself - but presumably programmers should detect that while writing the driver.

Alternative would be to set of_node in all calling drivers. Unfortunately, there
are about one hundred callers, so that would be a substantial effort.

Not sure myself what the right approach is here - that is why I asked for
feedback about it in v3. I am open to suggestions.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
  2013-02-06 20:05                 ` Guenter Roeck
@ 2013-02-06 20:12                     ` Tomasz Figa
  -1 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-02-06 20:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Lars-Peter Clausen, Anton Vorontsov,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring,
	Chanwoo Choi, MyungJoo Ham, Naveen Krishna Chatradhi,
	Jonathan Cameron

On Wednesday 06 of February 2013 12:05:13 Guenter Roeck wrote:
> On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> > On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > > Provide bindings and parse OF data during initialization.
> > > 
> > > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > 
> > looks good to me.  Couple of little queries inline.
> > 
> > > ---
> > > v4:
> > > - Fixed wrong parameter to dummy of_iio_channel_get_by_name if
> > > CONFIG_OF is> > 
> > >   undefined, and wrong return value.
> > > 
> > > - Initialize indio_dev->of_node in iio_device_register if the
> > > calling driver> > 
> > >   neglected to do it.
> > > 
> > > v3:
> > > - Cleaned up documentation (formatting, left-over clock references)
> > > - Updated bindings description to permit sub-devices
> > > - When searching for iio devices, use the pointer to the iio device
> > > type instead> > 
> > >   of strcmp. Rename iio_dev_type to iio_device_type (to match other
> > >   device types) and make it global for that purpose. Check the OF
> > >   node first, then the device type, as the node is less likely to
> > >   match.
> > > 
> > > - Move the common code in of_iio_channel_get and
> > > of_iio_channel_get_all to> > 
> > >   __of_iio_channel_get.
> > > 
> > > - Return NULL from of_iio_channel_get_by_name if nothing is found,
> > > or
> > > 
> > >   an error if there is a problem with consistency or if the provider
> > >   device is not yet available.
> > > 
> > > - In iio_channel_get, return if of_iio_channel_get_by_name() returns
> > > a channel> > 
> > >   or an error, and continue otherwise.
> > > 
> > > v2:
> > > - Rebased to iio/togreg
> > > - Documentation update per feedback
> > > - Dropped io-channel-output-names from the bindings document. The
> > > property is> > 
> > >   not used in the code, and it is not entirely clear what it would
> > >   be used for. If there is a need for it, we can add it back in
> > >   later on.
> > > 
> > > - Don't export OF specific API calls
> > > - For OF support, no longer depend on iio_map
> > > - Add #ifdef CONFIG_OF where appropriate, and ensure that the code
> > > still builds> > 
> > >   if it is not selected.
> > > 
> > > - Change iio_channel_get to take device pointer as argument instead
> > > of device> > 
> > >   name. Retain old API as of_iio_channel_get_sys.
> > > 
> > > - iio_channel_get now works for both OF and non-OF configurations
> > > - Use regulator to get vref for max1363 driver.
> > > 
> > >  drivers/iio/iio_core.h                             |    1 +
> > >  drivers/iio/industrialio-core.c                    |    8 +-
> > >  drivers/iio/inkern.c                               |  171
> > >  ++++++++++++++++++++ 4 files changed, 268 insertions(+), 2
> > >  deletions(-)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/iio/iio-bindings.txt> > 
> > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > > b/Documentation/devicetree/bindings/iio/iio-bindings.txt new file
> > > mode 100644
> > > index 0000000..2475c2e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > > @@ -0,0 +1,90 @@
> > > +This binding is a work-in-progress. It is derived from clock
> > > bindings,
> > > +and based on suggestions from Lars-Peter Clausen [1].
> > > +
> > > +Sources of IIO channels can be represented by any node in the
> > > device
> > > +tree. Those nodes are designated as IIO providers. IIO consumer
> > > +nodes use a phandle and IIO specifier pair to connect IIO provider
> > > +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> > > +specifier is an array of one or more cells identifying the IIO
> > > +output on a device. The length of an IIO specifier is defined by
> > > the
> > > +value of a #io-channel-cells property in the IIO provider node.
> > > +
> > > +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> > > +
> > > +==IIO providers==
> > > +
> > > +Required properties:
> > > +#io-channel-cells: Number of cells in an IIO specifier; Typically 0
> > > for nodes +		   with a single IIO output and 1 for nodes with
> > > multiple
> > > +		   IIO outputs.
> > > +
> > > +Example for a simple configuration with no trigger:
> > > +
> > > +	adc: voltage-sensor@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +		#io-channel-cells = <1>;
> > > +	};
> > > +
> > > +Example for a configuration with trigger:
> > > +
> > > +	adc@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +
> > > +		adc: iio-device {
> > > +			#io-channel-cells = <1>;
> > > +		};
> > > +		trigger: iio-trigger {
> > > +			#io-channel-cells = <1>;
> > 
> > Why would the trigger have channel-cells?
> > So far we don't have any device tree stuff for the triggers.
> 
> This comes from one of the examples passed around earlier.
> 
> Presumably, when triggers are supported through devicetree, you would
> have more than one per device, meaning you need the same logic as for
> devices.
> 
> Though on the other side that isn't supported yet - I don't have
> hardware to test, so I can not implement - or, rather, test -
> devicetree based trigger support. I can take the entire thing out if
> you prefer - I don't really have a strong opinion.
> 
> > > +		};
> > > +	};
> > > +
> > > +==IIO consumers==
> > > +
> > > +Required properties:
> > > +io-channels:	List of phandle and IIO specifier pairs, one pair
> > > +		for each IIO input to the device. Note: if the
> > > +		IIO provider specifies '0' for #io-channel-cells,
> > > +		then only the phandle portion of the pair will appear.
> > > +
> > > +Optional properties:
> > > +io-channel-names:
> > > +		List of IIO input name strings sorted in the same
> > > +		order as the io-channels property. Consumers drivers
> > > +		will use io-channel-names to match IIO input names
> > > +		with IIO specifiers.
> > > +io-channel-ranges:
> > > +		Empty property indicating that child nodes can inherit 
named
> > > +		IIO channels from this node. Useful for bus nodes to 
provide
> > > +		and IIO channel to their children.
> > > +
> > > +For example:
> > > +
> > > +	device {
> > > +		io-channels = <&adc 1>, <&ref 0>;
> > > +		io-channel-names = "vcc", "vdd";
> > > +	};
> > > +
> > > +This represents a device with two IIO inputs, named "vcc" and
> > > "vdd".
> > > +The vcc channel is connected to output 1 of the &adc device, and
> > > the
> > > +vdd channel is connected to output 0 of the &ref device.
> > > +
> > > +==Example==
> > > +
> > > +	adc: max1139@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +		#io-channel-cells = <1>;
> > > +	};
> > > +
> > > +	...
> > > +
> > > +	iio_hwmon {
> > > +		compatible = "iio-hwmon";
> > > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> > 
> > This example still seems a little odd. Why give one where there are
> > more channels than names?
> 
> I just didn't have a better idea. Do you know of another iio consumer I
> could use as example, one which doesn't use the get_all API ?
> 
> > > +	};
> > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > > index f652e6a..05c1b74 100644
> > > --- a/drivers/iio/iio_core.h
> > > +++ b/drivers/iio/iio_core.h
> > > @@ -18,6 +18,7 @@
> > > 
> > >  struct iio_chan_spec;
> > >  struct iio_dev;
> > > 
> > > +extern struct device_type iio_device_type;
> > > 
> > >  int __iio_add_chan_devattr(const char *postfix,
> > >  
> > >  			   struct iio_chan_spec const *chan,
> > > 
> > > diff --git a/drivers/iio/industrialio-core.c
> > > b/drivers/iio/industrialio-core.c index 8848f16..6d8b027 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -847,7 +847,7 @@ static void iio_dev_release(struct device
> > > *device)
> > > 
> > >  	kfree(indio_dev);
> > >  
> > >  }
> > > 
> > > -static struct device_type iio_dev_type = {
> > > +struct device_type iio_device_type = {
> > > 
> > >  	.name = "iio_device",
> > >  	.release = iio_dev_release,
> > >  
> > >  };
> > > 
> > > @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int
> > > sizeof_priv)
> > > 
> > >  	if (dev) {
> > >  	
> > >  		dev->dev.groups = dev->groups;
> > > 
> > > -		dev->dev.type = &iio_dev_type;
> > > +		dev->dev.type = &iio_device_type;
> > > 
> > >  		dev->dev.bus = &iio_bus_type;
> > >  		device_initialize(&dev->dev);
> > >  		dev_set_drvdata(&dev->dev, (void *)dev);
> > > 
> > > @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev
> > > *indio_dev)
> > > 
> > >  {
> > >  
> > >  	int ret;
> > > 
> > > +	/* If the calling driver did not initialize of_node, do it here 
*/
> > > +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> > > +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> > > +
> > 
> > I guess that's sensible, though I'd be inclined to check it and throw
> > a wobbly if it isn't defined.  That way we have one 'right' place to
> > do it.
> I had to look that one up :). "throw a wobbly" = "reached the end of
> rational thought and action" or "to become suddenly very agitated or
> angry". Is that about right ?
> 
> I assume you mean the case where indio_dev->dev.of_node is NULL ?
> Problem is that this can hapen anytime (and always if CONFIG_OF is
> undefined). I'd be more concerned if indio_dev->dev.parent is NULL,
> which happens with a couple of drivers if I interpret the code
> correctly. Otherwise it only matters if the node should really point to
> a sub-node and not to the parent's node itself - but presumably
> programmers should detect that while writing the driver.
> 
> Alternative would be to set of_node in all calling drivers.
> Unfortunately, there are about one hundred callers, so that would be a
> substantial effort.
> 
> Not sure myself what the right approach is here - that is why I asked
> for feedback about it in v3. I am open to suggestions.

I'm not sure if I understand the point, so following might be just some 
random thoughts:

There is no need to check for of_node. Case where there is no of_node 
attached to the device is fine, as lots of systems are being booted 
without device tree support.

Getting of_node from parent device seems reasonable to me, as this will 
automatically handle all the simple cases where iio_device node would be 
equal to physical device node.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
@ 2013-02-06 20:12                     ` Tomasz Figa
  0 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-02-06 20:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jonathan Cameron, linux-iio, devicetree-discuss,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Grant Likely, Rob Herring, MyungJoo Ham, Chanwoo Choi,
	Anton Vorontsov, David Woodhouse

On Wednesday 06 of February 2013 12:05:13 Guenter Roeck wrote:
> On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> > On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > > Provide bindings and parse OF data during initialization.
> > > 
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > 
> > looks good to me.  Couple of little queries inline.
> > 
> > > ---
> > > v4:
> > > - Fixed wrong parameter to dummy of_iio_channel_get_by_name if
> > > CONFIG_OF is> > 
> > >   undefined, and wrong return value.
> > > 
> > > - Initialize indio_dev->of_node in iio_device_register if the
> > > calling driver> > 
> > >   neglected to do it.
> > > 
> > > v3:
> > > - Cleaned up documentation (formatting, left-over clock references)
> > > - Updated bindings description to permit sub-devices
> > > - When searching for iio devices, use the pointer to the iio device
> > > type instead> > 
> > >   of strcmp. Rename iio_dev_type to iio_device_type (to match other
> > >   device types) and make it global for that purpose. Check the OF
> > >   node first, then the device type, as the node is less likely to
> > >   match.
> > > 
> > > - Move the common code in of_iio_channel_get and
> > > of_iio_channel_get_all to> > 
> > >   __of_iio_channel_get.
> > > 
> > > - Return NULL from of_iio_channel_get_by_name if nothing is found,
> > > or
> > > 
> > >   an error if there is a problem with consistency or if the provider
> > >   device is not yet available.
> > > 
> > > - In iio_channel_get, return if of_iio_channel_get_by_name() returns
> > > a channel> > 
> > >   or an error, and continue otherwise.
> > > 
> > > v2:
> > > - Rebased to iio/togreg
> > > - Documentation update per feedback
> > > - Dropped io-channel-output-names from the bindings document. The
> > > property is> > 
> > >   not used in the code, and it is not entirely clear what it would
> > >   be used for. If there is a need for it, we can add it back in
> > >   later on.
> > > 
> > > - Don't export OF specific API calls
> > > - For OF support, no longer depend on iio_map
> > > - Add #ifdef CONFIG_OF where appropriate, and ensure that the code
> > > still builds> > 
> > >   if it is not selected.
> > > 
> > > - Change iio_channel_get to take device pointer as argument instead
> > > of device> > 
> > >   name. Retain old API as of_iio_channel_get_sys.
> > > 
> > > - iio_channel_get now works for both OF and non-OF configurations
> > > - Use regulator to get vref for max1363 driver.
> > > 
> > >  drivers/iio/iio_core.h                             |    1 +
> > >  drivers/iio/industrialio-core.c                    |    8 +-
> > >  drivers/iio/inkern.c                               |  171
> > >  ++++++++++++++++++++ 4 files changed, 268 insertions(+), 2
> > >  deletions(-)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/iio/iio-bindings.txt> > 
> > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > > b/Documentation/devicetree/bindings/iio/iio-bindings.txt new file
> > > mode 100644
> > > index 0000000..2475c2e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > > @@ -0,0 +1,90 @@
> > > +This binding is a work-in-progress. It is derived from clock
> > > bindings,
> > > +and based on suggestions from Lars-Peter Clausen [1].
> > > +
> > > +Sources of IIO channels can be represented by any node in the
> > > device
> > > +tree. Those nodes are designated as IIO providers. IIO consumer
> > > +nodes use a phandle and IIO specifier pair to connect IIO provider
> > > +outputs to IIO inputs. Similar to the gpio specifiers, an IIO
> > > +specifier is an array of one or more cells identifying the IIO
> > > +output on a device. The length of an IIO specifier is defined by
> > > the
> > > +value of a #io-channel-cells property in the IIO provider node.
> > > +
> > > +[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> > > +
> > > +==IIO providers==
> > > +
> > > +Required properties:
> > > +#io-channel-cells: Number of cells in an IIO specifier; Typically 0
> > > for nodes +		   with a single IIO output and 1 for nodes with
> > > multiple
> > > +		   IIO outputs.
> > > +
> > > +Example for a simple configuration with no trigger:
> > > +
> > > +	adc: voltage-sensor@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +		#io-channel-cells = <1>;
> > > +	};
> > > +
> > > +Example for a configuration with trigger:
> > > +
> > > +	adc@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +
> > > +		adc: iio-device {
> > > +			#io-channel-cells = <1>;
> > > +		};
> > > +		trigger: iio-trigger {
> > > +			#io-channel-cells = <1>;
> > 
> > Why would the trigger have channel-cells?
> > So far we don't have any device tree stuff for the triggers.
> 
> This comes from one of the examples passed around earlier.
> 
> Presumably, when triggers are supported through devicetree, you would
> have more than one per device, meaning you need the same logic as for
> devices.
> 
> Though on the other side that isn't supported yet - I don't have
> hardware to test, so I can not implement - or, rather, test -
> devicetree based trigger support. I can take the entire thing out if
> you prefer - I don't really have a strong opinion.
> 
> > > +		};
> > > +	};
> > > +
> > > +==IIO consumers==
> > > +
> > > +Required properties:
> > > +io-channels:	List of phandle and IIO specifier pairs, one pair
> > > +		for each IIO input to the device. Note: if the
> > > +		IIO provider specifies '0' for #io-channel-cells,
> > > +		then only the phandle portion of the pair will appear.
> > > +
> > > +Optional properties:
> > > +io-channel-names:
> > > +		List of IIO input name strings sorted in the same
> > > +		order as the io-channels property. Consumers drivers
> > > +		will use io-channel-names to match IIO input names
> > > +		with IIO specifiers.
> > > +io-channel-ranges:
> > > +		Empty property indicating that child nodes can inherit 
named
> > > +		IIO channels from this node. Useful for bus nodes to 
provide
> > > +		and IIO channel to their children.
> > > +
> > > +For example:
> > > +
> > > +	device {
> > > +		io-channels = <&adc 1>, <&ref 0>;
> > > +		io-channel-names = "vcc", "vdd";
> > > +	};
> > > +
> > > +This represents a device with two IIO inputs, named "vcc" and
> > > "vdd".
> > > +The vcc channel is connected to output 1 of the &adc device, and
> > > the
> > > +vdd channel is connected to output 0 of the &ref device.
> > > +
> > > +==Example==
> > > +
> > > +	adc: max1139@35 {
> > > +		compatible = "maxim,max1139";
> > > +		reg = <0x35>;
> > > +		#io-channel-cells = <1>;
> > > +	};
> > > +
> > > +	...
> > > +
> > > +	iio_hwmon {
> > > +		compatible = "iio-hwmon";
> > > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> > 
> > This example still seems a little odd. Why give one where there are
> > more channels than names?
> 
> I just didn't have a better idea. Do you know of another iio consumer I
> could use as example, one which doesn't use the get_all API ?
> 
> > > +	};
> > > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > > index f652e6a..05c1b74 100644
> > > --- a/drivers/iio/iio_core.h
> > > +++ b/drivers/iio/iio_core.h
> > > @@ -18,6 +18,7 @@
> > > 
> > >  struct iio_chan_spec;
> > >  struct iio_dev;
> > > 
> > > +extern struct device_type iio_device_type;
> > > 
> > >  int __iio_add_chan_devattr(const char *postfix,
> > >  
> > >  			   struct iio_chan_spec const *chan,
> > > 
> > > diff --git a/drivers/iio/industrialio-core.c
> > > b/drivers/iio/industrialio-core.c index 8848f16..6d8b027 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -847,7 +847,7 @@ static void iio_dev_release(struct device
> > > *device)
> > > 
> > >  	kfree(indio_dev);
> > >  
> > >  }
> > > 
> > > -static struct device_type iio_dev_type = {
> > > +struct device_type iio_device_type = {
> > > 
> > >  	.name = "iio_device",
> > >  	.release = iio_dev_release,
> > >  
> > >  };
> > > 
> > > @@ -869,7 +869,7 @@ struct iio_dev *iio_device_alloc(int
> > > sizeof_priv)
> > > 
> > >  	if (dev) {
> > >  	
> > >  		dev->dev.groups = dev->groups;
> > > 
> > > -		dev->dev.type = &iio_dev_type;
> > > +		dev->dev.type = &iio_device_type;
> > > 
> > >  		dev->dev.bus = &iio_bus_type;
> > >  		device_initialize(&dev->dev);
> > >  		dev_set_drvdata(&dev->dev, (void *)dev);
> > > 
> > > @@ -960,6 +960,10 @@ int iio_device_register(struct iio_dev
> > > *indio_dev)
> > > 
> > >  {
> > >  
> > >  	int ret;
> > > 
> > > +	/* If the calling driver did not initialize of_node, do it here 
*/
> > > +	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
> > > +		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
> > > +
> > 
> > I guess that's sensible, though I'd be inclined to check it and throw
> > a wobbly if it isn't defined.  That way we have one 'right' place to
> > do it.
> I had to look that one up :). "throw a wobbly" = "reached the end of
> rational thought and action" or "to become suddenly very agitated or
> angry". Is that about right ?
> 
> I assume you mean the case where indio_dev->dev.of_node is NULL ?
> Problem is that this can hapen anytime (and always if CONFIG_OF is
> undefined). I'd be more concerned if indio_dev->dev.parent is NULL,
> which happens with a couple of drivers if I interpret the code
> correctly. Otherwise it only matters if the node should really point to
> a sub-node and not to the parent's node itself - but presumably
> programmers should detect that while writing the driver.
> 
> Alternative would be to set of_node in all calling drivers.
> Unfortunately, there are about one hundred callers, so that would be a
> substantial effort.
> 
> Not sure myself what the right approach is here - that is why I asked
> for feedback about it in v3. I am open to suggestions.

I'm not sure if I understand the point, so following might be just some 
random thoughts:

There is no need to check for of_node. Case where there is no of_node 
attached to the device is fine, as lots of systems are being booted 
without device tree support.

Getting of_node from parent device seems reasonable to me, as this will 
automatically handle all the simple cases where iio_device node would be 
equal to physical device node.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
  2013-02-06 19:37             ` Jonathan Cameron
@ 2013-02-07  1:53                 ` Guenter Roeck
  -1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-07  1:53 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > Provide bindings and parse OF data during initialization.
> > 
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> looks good to me.  Couple of little queries inline.
> 
[ ... ]

> > +
> > +Example for a configuration with trigger:
> > +
> > +	adc@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +
> > +		adc: iio-device {
> > +			#io-channel-cells = <1>;
> > +		};
> > +		trigger: iio-trigger {
> > +			#io-channel-cells = <1>;
> Why would the trigger have channel-cells?
> So far we don't have any device tree stuff for the triggers.

I thought about keeping the example and adding a note that trigger support is
not currently implemented, but a better idea is to replace it with a second iio
device. I'll do that unless someone objects.

	adc@35 {
		compatible = "some-vendor,some-adc";
		reg = <0x35>;

		adc1: iio-device@0 {
			#io-channel-cells = <1>;
			/* other properties */
		};
		adc2: iio-device@1 {
			#io-channel-cells = <1>;
			/* other properties */
		};
	};

> > +
> > +==Example==
> > +
> > +	adc: max1139@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +	...
> > +
> > +	iio_hwmon {
> > +		compatible = "iio-hwmon";
> > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> This example still seems a little odd. Why give one where there are
> more channels than names?

I replaced the example with

	iio_hwmon {
		compatible = "iio-hwmon";
		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
			<&adc 3>, <&adc 4>, <&adc 5>,
			<&adc 6>, <&adc 7>, <&adc 8>,
			<&adc 9>;
		}; 

	some_consumer {
		compatible = "some-consumer";
		io-channels = <&adc 10>, <&adc 11>;
		io-channel-names = "adc1", "adc2";
	};

Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
@ 2013-02-07  1:53                 ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2013-02-07  1:53 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, devicetree-discuss, Naveen Krishna Chatradhi,
	Lars-Peter Clausen, Doug Anderson, Tomasz Figa, Grant Likely,
	Rob Herring, MyungJoo Ham, Chanwoo Choi, Anton Vorontsov,
	David Woodhouse

On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
> > Provide bindings and parse OF data during initialization.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> looks good to me.  Couple of little queries inline.
> 
[ ... ]

> > +
> > +Example for a configuration with trigger:
> > +
> > +	adc@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +
> > +		adc: iio-device {
> > +			#io-channel-cells = <1>;
> > +		};
> > +		trigger: iio-trigger {
> > +			#io-channel-cells = <1>;
> Why would the trigger have channel-cells?
> So far we don't have any device tree stuff for the triggers.

I thought about keeping the example and adding a note that trigger support is
not currently implemented, but a better idea is to replace it with a second iio
device. I'll do that unless someone objects.

	adc@35 {
		compatible = "some-vendor,some-adc";
		reg = <0x35>;

		adc1: iio-device@0 {
			#io-channel-cells = <1>;
			/* other properties */
		};
		adc2: iio-device@1 {
			#io-channel-cells = <1>;
			/* other properties */
		};
	};

> > +
> > +==Example==
> > +
> > +	adc: max1139@35 {
> > +		compatible = "maxim,max1139";
> > +		reg = <0x35>;
> > +		#io-channel-cells = <1>;
> > +	};
> > +
> > +	...
> > +
> > +	iio_hwmon {
> > +		compatible = "iio-hwmon";
> > +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> > +			<&adc 3>, <&adc 4>, <&adc 5>,
> > +			<&adc 6>, <&adc 7>, <&adc 8>,
> > +			<&adc 9>, <&adc 10>, <&adc 11>;
> > +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
> This example still seems a little odd. Why give one where there are
> more channels than names?

I replaced the example with

	iio_hwmon {
		compatible = "iio-hwmon";
		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
			<&adc 3>, <&adc 4>, <&adc 5>,
			<&adc 6>, <&adc 7>, <&adc 8>,
			<&adc 9>;
		}; 

	some_consumer {
		compatible = "some-consumer";
		io-channels = <&adc 10>, <&adc 11>;
		io-channel-names = "adc1", "adc2";
	};

Guenter

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
  2013-02-07  1:53                 ` Guenter Roeck
@ 2013-02-07  9:00                     ` Jonathan Cameron
  -1 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-07  9:00 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On 07/02/13 01:53, Guenter Roeck wrote:
> On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
>> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
>>> Provide bindings and parse OF data during initialization.
>>>
>>> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
>> looks good to me.  Couple of little queries inline.
>>
> [ ... ]
>
>>> +
>>> +Example for a configuration with trigger:
>>> +
>>> +	adc@35 {
>>> +		compatible = "maxim,max1139";
>>> +		reg = <0x35>;
>>> +
>>> +		adc: iio-device {
>>> +			#io-channel-cells = <1>;
>>> +		};
>>> +		trigger: iio-trigger {
>>> +			#io-channel-cells = <1>;
>> Why would the trigger have channel-cells?
>> So far we don't have any device tree stuff for the triggers.
>
> I thought about keeping the example and adding a note that trigger support is
> not currently implemented, but a better idea is to replace it with a second iio
> device. I'll do that unless someone objects.
>
> 	adc@35 {
> 		compatible = "some-vendor,some-adc";
> 		reg = <0x35>;
>
> 		adc1: iio-device@0 {
> 			#io-channel-cells = <1>;
> 			/* other properties */
> 		};
> 		adc2: iio-device@1 {
> 			#io-channel-cells = <1>;
> 			/* other properties */
> 		};
> 	};
Agreed.

>
>>> +
>>> +==Example==
>>> +
>>> +	adc: max1139@35 {
>>> +		compatible = "maxim,max1139";
>>> +		reg = <0x35>;
>>> +		#io-channel-cells = <1>;
>>> +	};
>>> +
>>> +	...
>>> +
>>> +	iio_hwmon {
>>> +		compatible = "iio-hwmon";
>>> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
>>> +			<&adc 3>, <&adc 4>, <&adc 5>,
>>> +			<&adc 6>, <&adc 7>, <&adc 8>,
>>> +			<&adc 9>, <&adc 10>, <&adc 11>;
>>> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
>> This example still seems a little odd. Why give one where there are
>> more channels than names?
>
> I replaced the example with
>
> 	iio_hwmon {
> 		compatible = "iio-hwmon";
> 		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> 			<&adc 3>, <&adc 4>, <&adc 5>,
> 			<&adc 6>, <&adc 7>, <&adc 8>,
> 			<&adc 9>;
> 		};
>
> 	some_consumer {
> 		compatible = "some-consumer";
> 		io-channels = <&adc 10>, <&adc 11>;
> 		io-channel-names = "adc1", "adc2";
> 	};
Good as well. Much happier with these examples as they
won't cause any confusion.
>
> Guenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4 2/2] iio: Add OF support
@ 2013-02-07  9:00                     ` Jonathan Cameron
  0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2013-02-07  9:00 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jonathan Cameron, linux-iio, devicetree-discuss,
	Naveen Krishna Chatradhi, Lars-Peter Clausen, Doug Anderson,
	Tomasz Figa, Grant Likely, Rob Herring, MyungJoo Ham,
	Chanwoo Choi, Anton Vorontsov, David Woodhouse

On 07/02/13 01:53, Guenter Roeck wrote:
> On Wed, Feb 06, 2013 at 07:37:37PM +0000, Jonathan Cameron wrote:
>> On 02/06/2013 06:29 PM, Guenter Roeck wrote:
>>> Provide bindings and parse OF data during initialization.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> looks good to me.  Couple of little queries inline.
>>
> [ ... ]
>
>>> +
>>> +Example for a configuration with trigger:
>>> +
>>> +	adc@35 {
>>> +		compatible = "maxim,max1139";
>>> +		reg = <0x35>;
>>> +
>>> +		adc: iio-device {
>>> +			#io-channel-cells = <1>;
>>> +		};
>>> +		trigger: iio-trigger {
>>> +			#io-channel-cells = <1>;
>> Why would the trigger have channel-cells?
>> So far we don't have any device tree stuff for the triggers.
>
> I thought about keeping the example and adding a note that trigger support is
> not currently implemented, but a better idea is to replace it with a second iio
> device. I'll do that unless someone objects.
>
> 	adc@35 {
> 		compatible = "some-vendor,some-adc";
> 		reg = <0x35>;
>
> 		adc1: iio-device@0 {
> 			#io-channel-cells = <1>;
> 			/* other properties */
> 		};
> 		adc2: iio-device@1 {
> 			#io-channel-cells = <1>;
> 			/* other properties */
> 		};
> 	};
Agreed.

>
>>> +
>>> +==Example==
>>> +
>>> +	adc: max1139@35 {
>>> +		compatible = "maxim,max1139";
>>> +		reg = <0x35>;
>>> +		#io-channel-cells = <1>;
>>> +	};
>>> +
>>> +	...
>>> +
>>> +	iio_hwmon {
>>> +		compatible = "iio-hwmon";
>>> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
>>> +			<&adc 3>, <&adc 4>, <&adc 5>,
>>> +			<&adc 6>, <&adc 7>, <&adc 8>,
>>> +			<&adc 9>, <&adc 10>, <&adc 11>;
>>> +		io-channel-names = "vcc", "vdd", "vref", "1.2V";
>> This example still seems a little odd. Why give one where there are
>> more channels than names?
>
> I replaced the example with
>
> 	iio_hwmon {
> 		compatible = "iio-hwmon";
> 		io-channels = <&adc 0>, <&adc 1>, <&adc 2>,
> 			<&adc 3>, <&adc 4>, <&adc 5>,
> 			<&adc 6>, <&adc 7>, <&adc 8>,
> 			<&adc 9>;
> 		};
>
> 	some_consumer {
> 		compatible = "some-consumer";
> 		io-channels = <&adc 10>, <&adc 11>;
> 		io-channel-names = "adc1", "adc2";
> 	};
Good as well. Much happier with these examples as they
won't cause any confusion.
>
> Guenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2013-02-07  9:00 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 20:26 [PATCH v3 0/2] iio: Devicetree support Guenter Roeck
2013-02-04 20:26 ` Guenter Roeck
     [not found] ` <1360009566-26347-1-git-send-email-linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-04 20:26   ` [PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument Guenter Roeck
2013-02-04 20:26     ` Guenter Roeck
     [not found]     ` <1360009566-26347-2-git-send-email-linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-04 21:38       ` Anton Vorontsov
2013-02-04 21:38         ` Anton Vorontsov
2013-02-04 23:57       ` Chanwoo Choi
2013-02-04 23:57         ` Chanwoo Choi
     [not found]         ` <51104AE7.2020909-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-02-06 19:30           ` Jonathan Cameron
2013-02-06 19:30             ` Jonathan Cameron
2013-02-04 20:26   ` [PATCH v3 2/2] iio: Add OF support Guenter Roeck
2013-02-04 20:26     ` Guenter Roeck
     [not found]     ` <1360009566-26347-3-git-send-email-linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-05  5:20       ` Guenter Roeck
2013-02-05  5:20         ` Guenter Roeck
2013-02-05 15:22       ` Guenter Roeck
2013-02-05 15:22         ` Guenter Roeck
2013-02-06 18:29       ` [PATCH v4 " Guenter Roeck
2013-02-06 18:29         ` Guenter Roeck
     [not found]         ` <20130206182923.GA14417-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-06 19:37           ` Jonathan Cameron
2013-02-06 19:37             ` Jonathan Cameron
     [not found]             ` <5112B101.7040709-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-02-06 20:05               ` Guenter Roeck
2013-02-06 20:05                 ` Guenter Roeck
     [not found]                 ` <20130206200513.GA18135-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-06 20:12                   ` Tomasz Figa
2013-02-06 20:12                     ` Tomasz Figa
2013-02-07  1:53               ` Guenter Roeck
2013-02-07  1:53                 ` Guenter Roeck
     [not found]                 ` <20130207015346.GA15240-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2013-02-07  9:00                   ` Jonathan Cameron
2013-02-07  9:00                     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.