Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Lars-Peter Clausen]

On 10/31/2012 04:55 PM, Pantelis Antoniou wrote:
> Add an IIO map interface that consumers can use.
> Also make sure the mfd device doesn't activate a driver which
> the configuration doesn't require.

Same here, two completely different changes in the same patch.

> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/iio/adc/ti_am335x_adc.c      | 53 ++++++++++++++++++++++++++++++------
>  drivers/mfd/ti_am335x_tscadc.c       | 30 ++++++++++++++------
>  include/linux/mfd/ti_am335x_tscadc.h |  8 ++----
>  3 files changed, 68 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 02a43c8..d48fd79 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -20,8 +20,9 @@
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> -#include <linux/io.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/machine.h>
> +#include <linux/iio/driver.h>
>  
>  #include <linux/mfd/ti_am335x_tscadc.h>
>  #include <linux/platform_data/ti_am335x_adc.h>
> @@ -29,6 +30,8 @@
>  struct tiadc_device {
>  	struct ti_tscadc_dev *mfd_tscadc;
>  	int channels;
> +	char *buf;

As far as I can see 'buf' is not used otherwise in this patch.

> +	struct iio_map *map;
>  };
>  
>  static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
> @@ -75,25 +78,57 @@ 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;
> -	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;
>  
> +	size = indio_dev->num_channels * (sizeof(struct iio_chan_spec) + 6);
> +	chan_array = kzalloc(size, GFP_KERNEL);
>  	if (chan_array == NULL)
>  		return -ENOMEM;
>  
> -	for (i = 0; i < (indio_dev->num_channels); i++) {
> -		struct iio_chan_spec *chan = chan_array + i;
> +	/* buffer space is after the array */
> +	s = (char *)(chan_array + indio_dev->num_channels);
> +	chan = chan_array;
> +	for (i = 0; i < indio_dev->num_channels; i++, chan++, s += len + 1) {
> +
> +		len = sprintf(s, "AIN%d", i);
> +
>  		chan->type = IIO_VOLTAGE;
>  		chan->indexed = 1;
>  		chan->channel = i;
> -		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
> +		chan->datasheet_name = s;

> +		chan->scan_type.sign = 'u';
> +		chan->scan_type.realbits = 12;
> +		chan->scan_type.storagebits = 32;
> +		chan->scan_type.shift = 0;

This part is another separate thing done by this patch and is not even in
the patch description.

>  	}
>  
>  	indio_dev->channels = chan_array;
>  
> +	size = (indio_dev->num_channels + 1) * sizeof(struct iio_map);
> +	adc_dev->map = kzalloc(size, GFP_KERNEL);
> +	if (adc_dev->map == NULL) {
> +		kfree(chan_array);
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		adc_dev->map[i].adc_channel_label = chan_array[i].datasheet_name;
> +		adc_dev->map[i].consumer_dev_name = "any";
> +		adc_dev->map[i].consumer_channel = chan_array[i].datasheet_name;
> +	}
> +	adc_dev->map[i].adc_channel_label = NULL;
> +	adc_dev->map[i].consumer_dev_name = NULL;
> +	adc_dev->map[i].consumer_channel = NULL;
> +
> +	ret = iio_map_array_register(indio_dev, adc_dev->map);
> +	if (ret != 0) {
> +		kfree(adc_dev->map);
> +		kfree(chan_array);
> +		return -ENOMEM;
> +	}

The consumer data is supposed to be passed in by platform data, as it will
depend on the actual consumer. E.g. the consumer_dev_name has to match the
name of device which requests the channel. Same goes for the consumer
channel attribute, this is consumer specific as well.

> +
>  	return indio_dev->num_channels;
>  }
>  
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index e947dd8..cbb8b70c 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -176,26 +176,38 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
>  	ctrl |= CNTRLREG_TSCSSENB;
>  	tscadc_writel(tscadc, REG_CTRL, ctrl);
>  
> +	tscadc->used_cells = 0;
> +	tscadc->tsc_cell = -1;
> +	tscadc->adc_cell = -1;
> +
>  	/* TSC Cell */
> -	cell = &tscadc->cells[TSC_CELL];
> -	cell->name = "tsc";
> -	cell->platform_data = tscadc;
> -	cell->pdata_size = sizeof(*tscadc);
> +	if (tsc_wires > 0) {
> +		tscadc->tsc_cell = tscadc->used_cells;
> +		cell = &tscadc->cells[tscadc->used_cells++];
> +		cell->name = "tsc";
> +		cell->platform_data = tscadc;
> +		cell->pdata_size = sizeof(*tscadc);
> +	}
>  
>  	/* ADC Cell */
> -	cell = &tscadc->cells[ADC_CELL];
> -	cell->name = "tiadc";
> -	cell->platform_data = tscadc;
> -	cell->pdata_size = sizeof(*tscadc);
> +	if (adc_channels > 0) {
> +		tscadc->adc_cell = tscadc->used_cells;
> +		cell = &tscadc->cells[tscadc->used_cells++];
> +		cell->name = "tiadc";
> +		cell->platform_data = tscadc;
> +		cell->pdata_size = sizeof(*tscadc);
> +	}
>  
>  	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
> -			TSCADC_CELLS, NULL, 0, NULL);
> +			tscadc->used_cells, NULL, 0, NULL);
>  	if (err < 0)
>  		goto err_disable_clk;
>  
>  	device_init_wakeup(&pdev->dev, true);
>  	platform_set_drvdata(pdev, tscadc);
>  
> +	dev_info(&pdev->dev, "Initialized OK.\n");
> +
>  	return 0;
>  
>  err_disable_clk:
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index 9624fea..50a245f 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -128,11 +128,6 @@
>  
>  #define TSCADC_CELLS		2
>  
> -enum tscadc_cells {
> -	TSC_CELL,
> -	ADC_CELL,
> -};
> -
>  struct mfd_tscadc_board {
>  	struct tsc_data *tsc_init;
>  	struct adc_data *adc_init;
> @@ -143,6 +138,9 @@ struct ti_tscadc_dev {
>  	struct regmap *regmap_tscadc;
>  	void __iomem *tscadc_base;
>  	int irq;
> +	int used_cells;	/* 0-2 */
> +	int tsc_cell;	/* -1 if not used */
> +	int adc_cell;	/* -1 if not used */
>  	struct mfd_cell cells[TSCADC_CELLS];
>  
>  	/* tsc device */

[PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Pantelis Antoniou]

Add an IIO map interface that consumers can use.
Also make sure the mfd device doesn't activate a driver which
the configuration doesn't require.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/iio/adc/ti_am335x_adc.c      | 53 ++++++++++++++++++++++++++++++------
 drivers/mfd/ti_am335x_tscadc.c       | 30 ++++++++++++++------
 include/linux/mfd/ti_am335x_tscadc.h |  8 ++----
 3 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 02a43c8..d48fd79 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -20,8 +20,9 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
-#include <linux/io.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 #include <linux/platform_data/ti_am335x_adc.h>
@@ -29,6 +30,8 @@
 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)
@@ -75,25 +78,57 @@ 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;
-	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;
 
+	size = indio_dev->num_channels * (sizeof(struct iio_chan_spec) + 6);
+	chan_array = kzalloc(size, GFP_KERNEL);
 	if (chan_array == NULL)
 		return -ENOMEM;
 
-	for (i = 0; i < (indio_dev->num_channels); i++) {
-		struct iio_chan_spec *chan = chan_array + i;
+	/* buffer space is after the array */
+	s = (char *)(chan_array + indio_dev->num_channels);
+	chan = chan_array;
+	for (i = 0; i < indio_dev->num_channels; i++, chan++, s += len + 1) {
+
+		len = sprintf(s, "AIN%d", i);
+
 		chan->type = IIO_VOLTAGE;
 		chan->indexed = 1;
 		chan->channel = i;
-		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
+		chan->datasheet_name = s;
+		chan->scan_type.sign = 'u';
+		chan->scan_type.realbits = 12;
+		chan->scan_type.storagebits = 32;
+		chan->scan_type.shift = 0;
 	}
 
 	indio_dev->channels = chan_array;
 
+	size = (indio_dev->num_channels + 1) * sizeof(struct iio_map);
+	adc_dev->map = kzalloc(size, GFP_KERNEL);
+	if (adc_dev->map == NULL) {
+		kfree(chan_array);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		adc_dev->map[i].adc_channel_label = chan_array[i].datasheet_name;
+		adc_dev->map[i].consumer_dev_name = "any";
+		adc_dev->map[i].consumer_channel = chan_array[i].datasheet_name;
+	}
+	adc_dev->map[i].adc_channel_label = NULL;
+	adc_dev->map[i].consumer_dev_name = NULL;
+	adc_dev->map[i].consumer_channel = NULL;
+
+	ret = iio_map_array_register(indio_dev, adc_dev->map);
+	if (ret != 0) {
+		kfree(adc_dev->map);
+		kfree(chan_array);
+		return -ENOMEM;
+	}
+
 	return indio_dev->num_channels;
 }
 
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index e947dd8..cbb8b70c 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -176,26 +176,38 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	ctrl |= CNTRLREG_TSCSSENB;
 	tscadc_writel(tscadc, REG_CTRL, ctrl);
 
+	tscadc->used_cells = 0;
+	tscadc->tsc_cell = -1;
+	tscadc->adc_cell = -1;
+
 	/* TSC Cell */
-	cell = &tscadc->cells[TSC_CELL];
-	cell->name = "tsc";
-	cell->platform_data = tscadc;
-	cell->pdata_size = sizeof(*tscadc);
+	if (tsc_wires > 0) {
+		tscadc->tsc_cell = tscadc->used_cells;
+		cell = &tscadc->cells[tscadc->used_cells++];
+		cell->name = "tsc";
+		cell->platform_data = tscadc;
+		cell->pdata_size = sizeof(*tscadc);
+	}
 
 	/* ADC Cell */
-	cell = &tscadc->cells[ADC_CELL];
-	cell->name = "tiadc";
-	cell->platform_data = tscadc;
-	cell->pdata_size = sizeof(*tscadc);
+	if (adc_channels > 0) {
+		tscadc->adc_cell = tscadc->used_cells;
+		cell = &tscadc->cells[tscadc->used_cells++];
+		cell->name = "tiadc";
+		cell->platform_data = tscadc;
+		cell->pdata_size = sizeof(*tscadc);
+	}
 
 	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
-			TSCADC_CELLS, NULL, 0, NULL);
+			tscadc->used_cells, NULL, 0, NULL);
 	if (err < 0)
 		goto err_disable_clk;
 
 	device_init_wakeup(&pdev->dev, true);
 	platform_set_drvdata(pdev, tscadc);
 
+	dev_info(&pdev->dev, "Initialized OK.\n");
+
 	return 0;
 
 err_disable_clk:
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 9624fea..50a245f 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -128,11 +128,6 @@
 
 #define TSCADC_CELLS		2
 
-enum tscadc_cells {
-	TSC_CELL,
-	ADC_CELL,
-};
-
 struct mfd_tscadc_board {
 	struct tsc_data *tsc_init;
 	struct adc_data *adc_init;
@@ -143,6 +138,9 @@ struct ti_tscadc_dev {
 	struct regmap *regmap_tscadc;
 	void __iomem *tscadc_base;
 	int irq;
+	int used_cells;	/* 0-2 */
+	int tsc_cell;	/* -1 if not used */
+	int adc_cell;	/* -1 if not used */
 	struct mfd_cell cells[TSCADC_CELLS];
 
 	/* tsc device */
-- 
1.7.12

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Jan Luebbe]

Hi Pantelis,

while trying out your patch on a custom AM335x board, I noticed that the
sysfs entries ware missing. This is fixed in the first patch, you might want
to squash that into your original patch.

The second one makes sure that the iio framework does not read invalid data.

Regards,
Jan

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Jan Luebbe]

Hi Pantelis,

while trying out your patch on a custom AM335x board, I noticed that the
sysfs entries ware missing. This is fixed in the first patch, you might want
to squash that into your original patch.

The second one makes sure that the iio framework does not read invalid data.

Regards,
Jan

[PATCH 1/2] iio: adc: ti_am335x_adc: revert info_mask removal

[Jan Luebbe]

Commit "ti_tscadc: Update with IIO map interface & deal with partial activation"
by Pantelis Antoniou <panto@antoniou-consulting.com> removed this line.
Without it, the in_voltage?_raw entries are missing from sysfs.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index ee20c0c..620cc0c 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -107,6 +107,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev,
 		chan->type = IIO_VOLTAGE;
 		chan->indexed = 1;
 		chan->channel = i;
+		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
 		chan->datasheet_name = s;
 		chan->scan_type.sign = 'u';
 		chan->scan_type.realbits = 12;
-- 
1.8.2.rc2

[PATCH 1/2] iio: adc: ti_am335x_adc: revert info_mask removal

[Jan Luebbe]

Commit "ti_tscadc: Update with IIO map interface & deal with partial activation"
by Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> removed this line.
Without it, the in_voltage?_raw entries are missing from sysfs.

Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/iio/adc/ti_am335x_adc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index ee20c0c..620cc0c 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -107,6 +107,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev,
 		chan->type = IIO_VOLTAGE;
 		chan->indexed = 1;
 		chan->channel = i;
+		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
 		chan->datasheet_name = s;
 		chan->scan_type.sign = 'u';
 		chan->scan_type.realbits = 12;
-- 
1.8.2.rc2

[PATCH 2/2] iio: adc: ti_am335x_adc: make tiadc_read_raw() more robust

[Jan Luebbe]

Check that mask is actually IIO_CHAN_INFO_RAW.

Also handle the case where not enough data is in the fifo.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c | 48 ++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 620cc0c..4b764a9 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -157,26 +157,34 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	int i;
 	unsigned int fifo1count, readx1;
 
-	/*
-	 * When the sub-system is first enabled,
-	 * the sequencer will always start with the
-	 * lowest step (1) and continue until step (16).
-	 * For ex: If we have enabled 4 ADC channels and
-	 * currently use only 1 out of them, the
-	 * sequencer still configures all the 4 steps,
-	 * leading to 3 unwanted data.
-	 * Hence we need to flush out this data.
-	 */
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		/*
+		 * When the sub-system is first enabled,
+		 * the sequencer will always start with the
+		 * lowest step (1) and continue until step (16).
+		 * For ex: If we have enabled 4 ADC channels and
+		 * currently use only 1 out of them, the
+		 * sequencer still configures all the 4 steps,
+		 * leading to 3 unwanted data.
+		 * Hence we need to flush out this data.
+		 */
+
+		fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+		for (i = 0; i < fifo1count; i++) {
+			readx1 = tiadc_readl(adc_dev, REG_FIFO1);
+			if (i == chan->channel)
+				*val = readx1 & 0xfff;
+		}
+		tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
 
-	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
-	for (i = 0; i < fifo1count; i++) {
-		readx1 = tiadc_readl(adc_dev, REG_FIFO1);
-		if (i == chan->channel)
-			*val = readx1 & 0xfff;
-	}
-	tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+		if (fifo1count <= chan->channel)
+			return -EINVAL;
 
-	return IIO_VAL_INT;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
 }
 
 static const struct iio_info tiadc_info = {
@@ -213,7 +221,7 @@ static int tiadc_probe(struct platform_device *pdev)
 	else {
 		node = of_get_child_by_name(node, "adc");
 		if (!node)
-			return  -EINVAL;
+			return	-EINVAL;
 		else {
 			err = of_property_read_u32(node,
 					"ti,adc-channels", &val32);
@@ -310,7 +318,7 @@ static const struct dev_pm_ops tiadc_pm_ops = {
 
 static struct platform_driver tiadc_driver = {
 	.driver = {
-		.name   = "tiadc",
+		.name	= "tiadc",
 		.owner	= THIS_MODULE,
 		.pm	= TIADC_PM_OPS,
 	},
-- 
1.8.2.rc2

[PATCH 2/2] iio: adc: ti_am335x_adc: make tiadc_read_raw() more robust

[Jan Luebbe]

Check that mask is actually IIO_CHAN_INFO_RAW.

Also handle the case where not enough data is in the fifo.

Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/iio/adc/ti_am335x_adc.c | 48 ++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 620cc0c..4b764a9 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -157,26 +157,34 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	int i;
 	unsigned int fifo1count, readx1;
 
-	/*
-	 * When the sub-system is first enabled,
-	 * the sequencer will always start with the
-	 * lowest step (1) and continue until step (16).
-	 * For ex: If we have enabled 4 ADC channels and
-	 * currently use only 1 out of them, the
-	 * sequencer still configures all the 4 steps,
-	 * leading to 3 unwanted data.
-	 * Hence we need to flush out this data.
-	 */
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		/*
+		 * When the sub-system is first enabled,
+		 * the sequencer will always start with the
+		 * lowest step (1) and continue until step (16).
+		 * For ex: If we have enabled 4 ADC channels and
+		 * currently use only 1 out of them, the
+		 * sequencer still configures all the 4 steps,
+		 * leading to 3 unwanted data.
+		 * Hence we need to flush out this data.
+		 */
+
+		fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+		for (i = 0; i < fifo1count; i++) {
+			readx1 = tiadc_readl(adc_dev, REG_FIFO1);
+			if (i == chan->channel)
+				*val = readx1 & 0xfff;
+		}
+		tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
 
-	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
-	for (i = 0; i < fifo1count; i++) {
-		readx1 = tiadc_readl(adc_dev, REG_FIFO1);
-		if (i == chan->channel)
-			*val = readx1 & 0xfff;
-	}
-	tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+		if (fifo1count <= chan->channel)
+			return -EINVAL;
 
-	return IIO_VAL_INT;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
 }
 
 static const struct iio_info tiadc_info = {
@@ -213,7 +221,7 @@ static int tiadc_probe(struct platform_device *pdev)
 	else {
 		node = of_get_child_by_name(node, "adc");
 		if (!node)
-			return  -EINVAL;
+			return	-EINVAL;
 		else {
 			err = of_property_read_u32(node,
 					"ti,adc-channels", &val32);
@@ -310,7 +318,7 @@ static const struct dev_pm_ops tiadc_pm_ops = {
 
 static struct platform_driver tiadc_driver = {
 	.driver = {
-		.name   = "tiadc",
+		.name	= "tiadc",
 		.owner	= THIS_MODULE,
 		.pm	= TIADC_PM_OPS,
 	},
-- 
1.8.2.rc2

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Jonathan Cameron]

On 05/06/2013 01:48 PM, Jan Luebbe wrote:
> Hi Pantelis,
> 
> while trying out your patch on a custom AM335x board, I noticed that the
> sysfs entries ware missing. This is fixed in the first patch, you might want
> to squash that into your original patch.
> 
> The second one makes sure that the iio framework does not read invalid data.
> 
> Regards,
> Jan

I don't believe Pantelis ever came back with a response to the various issues
raised with the original patches?

If Pantelis has moved on, Jan feel free to pick up the patches, respin them
and resend to linux-iio if you like (with appropriate crediting naturally)

Until these are appropriately fixed up and resent, I'm lazy so will ignore these.
(your patches look reasonable to me based on a really superficial look)

Jonathan

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Pantelis Antoniou]

Hi Jonathan,

On May 6, 2013, at 7:12 PM, Jonathan Cameron wrote:

> On 05/06/2013 01:48 PM, Jan Luebbe wrote:
>> Hi Pantelis,
>>=20
>> while trying out your patch on a custom AM335x board, I noticed that =
the
>> sysfs entries ware missing. This is fixed in the first patch, you =
might want
>> to squash that into your original patch.
>>=20
>> The second one makes sure that the iio framework does not read =
invalid data.
>>=20
>> Regards,
>> Jan
>=20
> I don't believe Pantelis ever came back with a response to the various =
issues
> raised with the original patches?
>=20

I am aware. We've been quite busy getting the new beaglebone out. I plan =
to
revisit the full patchset for all the various fixes we had to do in a =
month
or so.

> If Pantelis has moved on, Jan feel free to pick up the patches, respin =
them
> and resend to linux-iio if you like (with appropriate crediting =
naturally)
>=20

I wouldn't mind Jan to pick up the IIO patches.

> Until these are appropriately fixed up and resent, I'm lazy so will =
ignore these.
> (your patches look reasonable to me based on a really superficial =
look)
>=20
> Jonathan

Regards

-- Pantelis

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Pantelis Antoniou]

Hi Jonathan,

On May 6, 2013, at 7:12 PM, Jonathan Cameron wrote:

> On 05/06/2013 01:48 PM, Jan Luebbe wrote:
>> Hi Pantelis,
>> 
>> while trying out your patch on a custom AM335x board, I noticed that the
>> sysfs entries ware missing. This is fixed in the first patch, you might want
>> to squash that into your original patch.
>> 
>> The second one makes sure that the iio framework does not read invalid data.
>> 
>> Regards,
>> Jan
> 
> I don't believe Pantelis ever came back with a response to the various issues
> raised with the original patches?
> 

I am aware. We've been quite busy getting the new beaglebone out. I plan to
revisit the full patchset for all the various fixes we had to do in a month
or so.

> If Pantelis has moved on, Jan feel free to pick up the patches, respin them
> and resend to linux-iio if you like (with appropriate crediting naturally)
> 

I wouldn't mind Jan to pick up the IIO patches.

> Until these are appropriately fixed up and resent, I'm lazy so will ignore these.
> (your patches look reasonable to me based on a really superficial look)
> 
> Jonathan

Regards

-- Pantelis

Re: [PATCH] ti_tscadc: Update with IIO map interface & deal with partial activation

[Jan Lübbe]

On Tue, 2013-05-07 at 16:08 +0300, Pantelis Antoniou wrote:
> Hi Jonathan,
> 
> On May 6, 2013, at 7:12 PM, Jonathan Cameron wrote:
> 
> > On 05/06/2013 01:48 PM, Jan Luebbe wrote:
> >> Hi Pantelis,
> >> 
> >> while trying out your patch on a custom AM335x board, I noticed that the
> >> sysfs entries ware missing. This is fixed in the first patch, you might want
> >> to squash that into your original patch.
> >> 
> >> The second one makes sure that the iio framework does not read invalid data.
> >> 
> >> Regards,
> >> Jan
> > 
> > I don't believe Pantelis ever came back with a response to the various issues
> > raised with the original patches?
> > 
> 
> I am aware. We've been quite busy getting the new beaglebone out. I plan to
> revisit the full patchset for all the various fixes we had to do in a month
> or so.

It seems that TI continued working on this in their vendor kernel:
http://arago-project.org/git/projects/?p=linux-am33x.git;a=history;f=drivers/staging/iio;hb=v3.2-staging
We could possibly reuse/port some of that work.

> > If Pantelis has moved on, Jan feel free to pick up the patches, respin them
> > and resend to linux-iio if you like (with appropriate crediting naturally)
> > 
> 
> I wouldn't mind Jan to pick up the IIO patches.

I'm currently also busy with other things, so I'm not optimistic that
I'd find some time to do this. :/

Thanks,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |