* Jonathan Cameron | 2013-06-02 18:40:27 [+0100]:

On 05/27/2013 08:11 PM, Sebastian Andrzej Siewior wrote:

quoted

From: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>

Add an IIO map interface that consumers can use.

While we're here fix the mfd device in the case
where a subdevice might not be activated.

Any time I read 'while we're here' it rings alarm bells.
Two issues -> Two patches please.

The level of reworking here is rather high. Probably better
to do the structural changes in one patch then follow with the
new functionality in another.

Okay, will try.

Other comments inline.

quoted

Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
Signed-off-by: Felipe Balbi <redacted>
Signed-off-by: Sebastian Andrzej Siewior <redacted>
---
 drivers/iio/adc/ti_am335x_adc.c      |   53 +++++++++++++++++++++++++++++-----
 drivers/mfd/ti_am335x_tscadc.c       |   29 ++++++++++++-------
 include/linux/mfd/ti_am335x_tscadc.h |    8 ++---
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index d821d88..cceff09 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c

@@ -20,16 +20,19 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
-#include <linux/io.h>

why? Not immediately obvious this has anything to do with the rest of this patch.

Yeah, a patch later or two it is added again :P Will drop this nonsense.

quoted

 #include <linux/iio/iio.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>

 #include <linux/mfd/ti_am335x_tscadc.h>

 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
 	int channels;
+	char *buf;
+	struct iio_map *map;
 };

 static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)

@@ -76,25 +79,59 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
 {
 	struct iio_chan_spec *chan_array;

quoted

-	int i;
-
-	indio_dev->num_channels = channels;
-	chan_array = kcalloc(indio_dev->num_channels,
-			sizeof(struct iio_chan_spec), GFP_KERNEL);
+	struct iio_chan_spec *chan;
+	char *s;
+	int i, len, size, ret;

This is less than beautiful... It'll cost you marginally more to
allocate a separate array for the strings, but will be a lot nicer
to read.  If we are dealing with a relatively small maximum number
then just do it as a static const char * array of all possible
strings.

The EVM board takes 4 and the maximum is 8. This is 20 bytes on EVM and
40 for the max. This probably does not hurt in data segment, will
change.

quoted

+	size = indio_dev->num_channels * (sizeof(struct iio_chan_spec) + 6);
+	chan_array = kzalloc(size, GFP_KERNEL);
 	if (chan_array == NULL)
 		return -ENOMEM;

Sebastian