[PATCH] iio: inkern: Add of_xlate function to struct iio_dev

[Srinivas Pandruvada]

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.

Do you have an example of a device that doesn't want to use the default
mapping? If yes please include it in the commit message, otherwise it is
fairly hard to say whether this makes sense or not.

Still not mainlined. You can find more detailed description of the
issue here[1] and driver here[2].

I see your need. I wonder this mapping be done in individual client
driver instead of creating another callback.

Thanks,
Srinivas

Regards,
Ivan
[1] https://lkml.org/lkml/2014/10/2/123
[2] http://www.spinics.net/lists/devicetree/msg50717.html

[Ivan T. Ivanov]

Post by Srinivas Pandruvada

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.

Do you have an example of a device that doesn't want to use the default
mapping? If yes please include it in the commit message, otherwise it is
fairly hard to say whether this makes sense or not.

Still not mainlined. You can find more detailed description of the
issue here[1] and driver here[2].

I see your need. I wonder this mapping be done in individual client
driver instead of creating another callback.

I am not sure I got you question. The idea is to provide default
1:1 mapping in core, which can be overwritten by client callback.

Regards,
Ivan

[Jonathan Cameron]

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.
Add simple translation function, suitable for the most 1:1
mapped channels in IIO chips, and use it when driver did not
provide custom implementation.

Any more comments on this? Been sat a while and the discussions seems
to have died out.

As Ivan has pointed out, very similar approaches are used
elsewhere (gpio for example).

---
drivers/iio/inkern.c | 32 +++++++++++++++++++++++++++-----
include/linux/iio/iio.h | 8 ++++++++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f084610..6c3e478 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -100,6 +100,28 @@ static int iio_dev_node_match(struct device *dev, void *data)
return dev->of_node == data && dev->type == &iio_device_type;
}
+/**
+ * __of_iio_simple_xlate - translate iiospec to the IIO channel index
+ *
+ * This is simple translation function, suitable for the most 1:1 mapped
+ * whether IIO index is less than num_channels (that is specified in the
+ * iio_dev).
+ */
+static int __of_iio_simple_xlate(struct iio_dev *indio_dev,
+ const struct of_phandle_args *iiospec)
+{
+ if (!iiospec->args_count)
+ return 0;
+
+ if (iiospec->args[0] >= indio_dev->num_channels)
+ return -EINVAL;
+
+ return iiospec->args[0];
+}
+
static int __of_iio_channel_get(struct iio_channel *channel,
struct device_node *np, int index)
{
@@ -122,18 +144,18 @@ static int __of_iio_channel_get(struct iio_channel *channel,
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) {
- err = -EINVAL;
+ if (!indio_dev->of_xlate)
+ indio_dev->of_xlate = __of_iio_simple_xlate;
+ index = indio_dev->of_xlate(indio_dev, &iiospec);
+ if (index < 0)
goto err_put;
- }
channel->channel = &indio_dev->channels[index];
return 0;
iio_device_put(indio_dev);
- return err;
+ return index;
}
static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 15dc6bc..d5bb219 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -13,6 +13,7 @@
#include <linux/device.h>
#include <linux/cdev.h>
#include <linux/iio/types.h>
+#include <linux/of.h>
/* IIO TODO LIST */
/*
* Provide means of adjusting timer accuracy.
@@ -413,6 +414,11 @@ struct iio_buffer_setup_ops {
* and owner
+ * index. When #iio-cells is greater than '0', the driver
+ * could provide a custom of_xlate function that reads
+ * the *args* and returns the appropriate index in
+ * registered IIO channels array.
@@ -451,6 +457,8 @@ struct iio_dev {
int currentmode;
struct device dev;
+ int (*of_xlate)(struct iio_dev *indio_dev,
+ const struct of_phandle_args *iiospec);
struct iio_event_interface *event_interface;
struct iio_buffer *buffer;

[Lars-Peter Clausen]

Post by Jonathan Cameron

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.
Add simple translation function, suitable for the most 1:1
mapped channels in IIO chips, and use it when driver did not
provide custom implementation.

Any more comments on this? Been sat a while and the discussions seems
to have died out.
As Ivan has pointed out, very similar approaches are used
elsewhere (gpio for example).

Looks good to me:

Reviewed-by: Lars-Peter Clausen <lars-Qo5EllUWu/***@public.gmane.org>

When we initially added the DT support to IIO I was hoping that we can get
away with just using the simple and generic xlate function for all devices.
But it looks as if some more complex devices need to overwrite it. We should
be careful about adding new driver specific xlate implementations and make
sure that it is actually needed.

One thing we might want to consider though is instead of adding the xlate
callback to the iio_dev struct add it to the iio_info struct since it should
be the same for different device instances of the same driver. And this is
also where all the other callbacks are.

- Lars

[Jonathan Cameron]

Post by Jonathan Cameron

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.
Add simple translation function, suitable for the most 1:1
mapped channels in IIO chips, and use it when driver did not
provide custom implementation.

Any more comments on this? Been sat a while and the discussions seems
to have died out.
As Ivan has pointed out, very similar approaches are used
elsewhere (gpio for example).

When we initially added the DT support to IIO I was hoping that we can get away
with just using the simple and generic xlate function for all devices. But it
looks as if some more complex devices need to overwrite it. We should be careful
about adding new driver specific xlate implementations and make sure that it is
actually needed.
One thing we might want to consider though is instead of adding the xlate
callback to the iio_dev struct add it to the iio_info struct since it should be
the same for different device instances of the same driver. And this is also
where all the other callbacks are.

Good point - would definitely prefer that.

J

- Lars

[Ivan T. Ivanov]

Post by Jonathan Cameron

Post by Jonathan Cameron

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.
Add simple translation function, suitable for the most 1:1
mapped channels in IIO chips, and use it when driver did not
provide custom implementation.

Any more comments on this? Been sat a while and the
discussions seems
to have died out.
As Ivan has pointed out, very similar approaches are used
elsewhere (gpio for example).

When we initially added the DT support to IIO I was hoping that we can get away
with just using the simple and generic xlate function for all devices. But it
looks as if some more complex devices need to overwrite it. We should be careful
about adding new driver specific xlate implementations and make sure that it is
actually needed.
One thing we might want to consider though is instead of adding the xlate
callback to the iio_dev struct add it to the iio_info struct since it should be
the same for different device instances of the same driver. And this is also
where all the other callbacks are.

Good point - would definitely prefer that.

Thank you. Will rework it as suggested.

Regards,
Ivan

Post by Jonathan Cameron
J

- Lars

[Srinivas Pandruvada]

Post by Jonathan Cameron

When #iio-cells is greater than '0', the driver could provide
a custom of_xlate function that reads the *args* and returns
the appropriate index in registered IIO channels array.
Add simple translation function, suitable for the most 1:1
mapped channels in IIO chips, and use it when driver did not
provide custom implementation.

Any more comments on this? Been sat a while and the discussions seems
to have died out.
As Ivan has pointed out, very similar approaches are used
elsewhere (gpio for example).

Looks useful to me.
Thanks,
Srinivas

Post by Jonathan Cameron

---
drivers/iio/inkern.c | 32 +++++++++++++++++++++++++++-----
include/linux/iio/iio.h | 8 ++++++++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f084610..6c3e478 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -100,6 +100,28 @@ static int iio_dev_node_match(struct device *dev, void *data)
return dev->of_node == data && dev->type == &iio_device_type;
}
+/**
+ * __of_iio_simple_xlate - translate iiospec to the IIO channel index
+ *
+ * This is simple translation function, suitable for the most 1:1 mapped
+ * whether IIO index is less than num_channels (that is specified in the
+ * iio_dev).
+ */
+static int __of_iio_simple_xlate(struct iio_dev *indio_dev,
+ const struct of_phandle_args *iiospec)
+{
+ if (!iiospec->args_count)
+ return 0;
+
+ if (iiospec->args[0] >= indio_dev->num_channels)
+ return -EINVAL;
+
+ return iiospec->args[0];
+}
+
static int __of_iio_channel_get(struct iio_channel *channel,
struct device_node *np, int index)
{
@@ -122,18 +144,18 @@ static int __of_iio_channel_get(struct iio_channel *channel,
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) {
- err = -EINVAL;
+ if (!indio_dev->of_xlate)
+ indio_dev->of_xlate = __of_iio_simple_xlate;
+ index = indio_dev->of_xlate(indio_dev, &iiospec);
+ if (index < 0)
goto err_put;
- }
channel->channel = &indio_dev->channels[index];
return 0;
iio_device_put(indio_dev);
- return err;
+ return index;
}
static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 15dc6bc..d5bb219 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -13,6 +13,7 @@
#include <linux/device.h>
#include <linux/cdev.h>
#include <linux/iio/types.h>
+#include <linux/of.h>
/* IIO TODO LIST */
/*
* Provide means of adjusting timer accuracy.
@@ -413,6 +414,11 @@ struct iio_buffer_setup_ops {
* and owner
+ * index. When #iio-cells is greater than '0', the driver
+ * could provide a custom of_xlate function that reads
+ * the *args* and returns the appropriate index in
+ * registered IIO channels array.
@@ -451,6 +457,8 @@ struct iio_dev {
int currentmode;
struct device dev;
+ int (*of_xlate)(struct iio_dev *indio_dev,
+ const struct of_phandle_args *iiospec);
struct iio_event_interface *event_interface;
struct iio_buffer *buffer;