linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] IIO AD7923 iio_consumer support
@ 2013-10-05  8:21 Christophe Leroy
  2013-10-05  8:41 ` Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2013-10-05  8:21 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-kernel, linux-iio, patrick.vasseur

This patch adds support for iio_consumer to Analog Devices AD7923 ADC driver.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Verified-by: Patrick Vasseur <patrick.vasseur@c-s.fr>

diff -urN a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
--- a/drivers/iio/adc/ad7923.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/iio/adc/ad7923.c	2013-02-12 15:16:29.000000000 +0100
@@ -19,11 +19,15 @@
 #include <linux/interrupt.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/iio/machine.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
+#define AD7923_NAME	"ad7923"
+
 #define AD7923_WRITE_CR		(1 << 11)	/* write control register */
 #define AD7923_RANGE		(1 << 1)	/* range to REFin */
 #define AD7923_CODING		(1 << 0)	/* coding is straight binary */
@@ -96,6 +100,7 @@
 			.storagebits = 16,				\
 			.endianness = IIO_BE,				\
 		},							\
+		.datasheet_name = #index,				\
 	}
 
 #define DECLARE_AD7923_CHANNELS(name, bits) \
@@ -195,6 +200,31 @@
 	return IRQ_HANDLED;
 }
 
+/* default maps used by iio consumer */
+static struct iio_map ad7923_default_iio_maps[] = {
+	{
+		.consumer_dev_name = AD7923_NAME,
+		.consumer_channel = "channel_0",
+		.adc_channel_label = "0",
+	},
+	{
+		.consumer_dev_name = AD7923_NAME,
+		.consumer_channel = "channel_1",
+		.adc_channel_label = "1",
+	},
+	{
+		.consumer_dev_name = AD7923_NAME,
+		.consumer_channel = "channel_2",
+		.adc_channel_label = "2",
+	},
+	{
+		.consumer_dev_name = AD7923_NAME,
+		.consumer_channel = "channel_3",
+		.adc_channel_label = "3",
+	},
+	{ }
+};
+
 static int ad7923_scan_direct(struct ad7923_state *st, unsigned ch)
 {
 	int ret, cmd;
@@ -325,12 +355,18 @@
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_device_register(indio_dev);
+	ret = iio_map_array_register(indio_dev, ad7923_default_iio_maps);
 	if (ret)
 		goto error_cleanup_ring;
 
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error_unmap;
+
 	return 0;
 
+error_unmap:
+	iio_map_array_unregister(indio_dev);
 error_cleanup_ring:
 	iio_triggered_buffer_cleanup(indio_dev);
 error_disable_reg:
@@ -349,6 +385,7 @@
 	struct ad7923_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
+	iio_map_array_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	regulator_disable(st->reg);
 	regulator_put(st->reg);
@@ -368,7 +405,7 @@
 
 static struct spi_driver ad7923_driver = {
 	.driver = {
-		.name	= "ad7923",
+		.name	= AD7923_NAME,
 		.owner	= THIS_MODULE,
 	},
 	.probe		= ad7923_probe,

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

* Re: [PATCH] IIO AD7923 iio_consumer support
  2013-10-05  8:21 [PATCH] IIO AD7923 iio_consumer support Christophe Leroy
@ 2013-10-05  8:41 ` Lars-Peter Clausen
  2013-10-05  9:18   ` leroy christophe
  0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-10-05  8:41 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Jonathan Cameron, linux-kernel, linux-iio, patrick.vasseur

On 10/05/2013 10:21 AM, Christophe Leroy wrote:
> This patch adds support for iio_consumer to Analog Devices AD7923 ADC driver.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Verified-by: Patrick Vasseur <patrick.vasseur@c-s.fr>
> 
> diff -urN a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> --- a/drivers/iio/adc/ad7923.c	1970-01-01 01:00:00.000000000 +0100
> +++ b/drivers/iio/adc/ad7923.c	2013-02-12 15:16:29.000000000 +0100
> @@ -19,11 +19,15 @@
>  #include <linux/interrupt.h>
>  
>  #include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/iio/machine.h>

The fact that you need to include machine.h here should ring some alarm
bells. machine.h is meant to be included by machine or board drivers, not by
device drivers.

>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
>  
> +#define AD7923_NAME	"ad7923"
> +
>  #define AD7923_WRITE_CR		(1 << 11)	/* write control register */
>  #define AD7923_RANGE		(1 << 1)	/* range to REFin */
>  #define AD7923_CODING		(1 << 0)	/* coding is straight binary */
> @@ -96,6 +100,7 @@
>  			.storagebits = 16,				\
>  			.endianness = IIO_BE,				\
>  		},							\
> +		.datasheet_name = #index,				\

The names for the pins in the datasheet are VINx

>  	}
>  
>  #define DECLARE_AD7923_CHANNELS(name, bits) \
> @@ -195,6 +200,31 @@
>  	return IRQ_HANDLED;
>  }
>  
> +/* default maps used by iio consumer */
> +static struct iio_map ad7923_default_iio_maps[] = {
> +	{
> +		.consumer_dev_name = AD7923_NAME,

The consumer_dev_name field is the dev_name() of the device that is going to
consume data from the channel. This device provides data on the channel.

> +		.consumer_channel = "channel_0",
> +		.adc_channel_label = "0",
> +	},
> +	{
> +		.consumer_dev_name = AD7923_NAME,
> +		.consumer_channel = "channel_1",
> +		.adc_channel_label = "1",
> +	},
> +	{
> +		.consumer_dev_name = AD7923_NAME,
> +		.consumer_channel = "channel_2",
> +		.adc_channel_label = "2",
> +	},
> +	{
> +		.consumer_dev_name = AD7923_NAME,
> +		.consumer_channel = "channel_3",
> +		.adc_channel_label = "3",
> +	},
> +	{ }
> +};

This is a mapping between channel names of the provider between the channel
names of the consumer. So it is specific to a certain combination of
consumer and provider and usually depend on how things are physically wired
on your board. As such there can be no generic mapping and this needs to go
into your machine/board driver. The mapping is usually passed to the IIO
driver via its platform data.

So e.g. imagine you have a provider like this driver and you have a consumer
that has a "voltage" channel. And on your board channel 3 of the ADC is what
you want to route to that consumer. Then your mapping would look like this:

{
	.consumer_dev_name = "your_consumer_device.1",
	.consumer_channel = "voltage",
	.adc_channel_label = "AIN3",
}

And in your consumer driver you'd do:

channel = iio_channel_get(dev, "voltage");

- Lars

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

* Re: [PATCH] IIO AD7923 iio_consumer support
  2013-10-05  8:41 ` Lars-Peter Clausen
@ 2013-10-05  9:18   ` leroy christophe
  2013-10-05  9:35     ` Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: leroy christophe @ 2013-10-05  9:18 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Jonathan Cameron, linux-kernel, linux-iio, patrick.vasseur


Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
> On 10/05/2013 10:21 AM, Christophe Leroy wrote:
>> +		.consumer_channel = "channel_0",
>> +		.adc_channel_label = "0",
>> +	},
>> +	{
>> +		.consumer_dev_name = AD7923_NAME,
>> +		.consumer_channel = "channel_1",
>> +		.adc_channel_label = "1",
>> +	},
>> +	{
>> +		.consumer_dev_name = AD7923_NAME,
>> +		.consumer_channel = "channel_2",
>> +		.adc_channel_label = "2",
>> +	},
>> +	{
>> +		.consumer_dev_name = AD7923_NAME,
>> +		.consumer_channel = "channel_3",
>> +		.adc_channel_label = "3",
>> +	},
>> +	{ }
>> +};
> This is a mapping between channel names of the provider between the channel
> names of the consumer. So it is specific to a certain combination of
> consumer and provider and usually depend on how things are physically wired
> on your board. As such there can be no generic mapping and this needs to go
> into your machine/board driver. The mapping is usually passed to the IIO
> driver via its platform data.
>
> So e.g. imagine you have a provider like this driver and you have a consumer
> that has a "voltage" channel. And on your board channel 3 of the ADC is what
> you want to route to that consumer. Then your mapping would look like this:
>
> {
> 	.consumer_dev_name = "your_consumer_device.1",
> 	.consumer_channel = "voltage",
> 	.adc_channel_label = "AIN3",
> }
>
> And in your consumer driver you'd do:
>
> channel = iio_channel_get(dev, "voltage");
>
>
Thanks for the explanation.

Can the mapping be retrieved via of_platform ?

Indeed, the only exemple I found was in the lp8788_adc driver, which 
includes iio/machine.h and declares a default mapping, but it is based 
on platform_data, not of_platform.

Christophe

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

* Re: [PATCH] IIO AD7923 iio_consumer support
  2013-10-05  9:18   ` leroy christophe
@ 2013-10-05  9:35     ` Lars-Peter Clausen
  2013-10-05  9:39       ` leroy christophe
  0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-10-05  9:35 UTC (permalink / raw)
  To: leroy christophe
  Cc: Jonathan Cameron, linux-kernel, linux-iio, patrick.vasseur

On 10/05/2013 11:18 AM, leroy christophe wrote:
> 
> Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
>> On 10/05/2013 10:21 AM, Christophe Leroy wrote:
>>> +        .consumer_channel = "channel_0",
>>> +        .adc_channel_label = "0",
>>> +    },
>>> +    {
>>> +        .consumer_dev_name = AD7923_NAME,
>>> +        .consumer_channel = "channel_1",
>>> +        .adc_channel_label = "1",
>>> +    },
>>> +    {
>>> +        .consumer_dev_name = AD7923_NAME,
>>> +        .consumer_channel = "channel_2",
>>> +        .adc_channel_label = "2",
>>> +    },
>>> +    {
>>> +        .consumer_dev_name = AD7923_NAME,
>>> +        .consumer_channel = "channel_3",
>>> +        .adc_channel_label = "3",
>>> +    },
>>> +    { }
>>> +};
>> This is a mapping between channel names of the provider between the channel
>> names of the consumer. So it is specific to a certain combination of
>> consumer and provider and usually depend on how things are physically wired
>> on your board. As such there can be no generic mapping and this needs to go
>> into your machine/board driver. The mapping is usually passed to the IIO
>> driver via its platform data.
>>
>> So e.g. imagine you have a provider like this driver and you have a consumer
>> that has a "voltage" channel. And on your board channel 3 of the ADC is what
>> you want to route to that consumer. Then your mapping would look like this:
>>
>> {
>>     .consumer_dev_name = "your_consumer_device.1",
>>     .consumer_channel = "voltage",
>>     .adc_channel_label = "AIN3",
>> }
>>
>> And in your consumer driver you'd do:
>>
>> channel = iio_channel_get(dev, "voltage");
>>
>>
> Thanks for the explanation.
> 
> Can the mapping be retrieved via of_platform ?
> 
> Indeed, the only exemple I found was in the lp8788_adc driver, which
> includes iio/machine.h and declares a default mapping, but it is based on
> platform_data, not of_platform.
>

If you are using device tree you can specify the mapping inside the
devicetree. Have a look at
Documentation/devicetree/bindings/iio/iio-bindings.txt

- Lars


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

* Re: [PATCH] IIO AD7923 iio_consumer support
  2013-10-05  9:35     ` Lars-Peter Clausen
@ 2013-10-05  9:39       ` leroy christophe
  2013-10-05  9:43         ` Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: leroy christophe @ 2013-10-05  9:39 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Jonathan Cameron, linux-kernel, linux-iio, patrick.vasseur


Le 05/10/2013 11:35, Lars-Peter Clausen a écrit :
> On 10/05/2013 11:18 AM, leroy christophe wrote:
>> Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
>>> On 10/05/2013 10:21 AM, Christophe Leroy wrote:
>>>> +        .consumer_channel = "channel_0",
>>>> +        .adc_channel_label = "0",
>>>> +    },
>>>> +    {
>>>> +        .consumer_dev_name = AD7923_NAME,
>>>> +        .consumer_channel = "channel_1",
>>>> +        .adc_channel_label = "1",
>>>> +    },
>>>> +    {
>>>> +        .consumer_dev_name = AD7923_NAME,
>>>> +        .consumer_channel = "channel_2",
>>>> +        .adc_channel_label = "2",
>>>> +    },
>>>> +    {
>>>> +        .consumer_dev_name = AD7923_NAME,
>>>> +        .consumer_channel = "channel_3",
>>>> +        .adc_channel_label = "3",
>>>> +    },
>>>> +    { }
>>>> +};
>>> This is a mapping between channel names of the provider between the channel
>>> names of the consumer. So it is specific to a certain combination of
>>> consumer and provider and usually depend on how things are physically wired
>>> on your board. As such there can be no generic mapping and this needs to go
>>> into your machine/board driver. The mapping is usually passed to the IIO
>>> driver via its platform data.
>>>
>>> So e.g. imagine you have a provider like this driver and you have a consumer
>>> that has a "voltage" channel. And on your board channel 3 of the ADC is what
>>> you want to route to that consumer. Then your mapping would look like this:
>>>
>>> {
>>>      .consumer_dev_name = "your_consumer_device.1",
>>>      .consumer_channel = "voltage",
>>>      .adc_channel_label = "AIN3",
>>> }
>>>
>>> And in your consumer driver you'd do:
>>>
>>> channel = iio_channel_get(dev, "voltage");
>>>
>>>
>> Thanks for the explanation.
>>
>> Can the mapping be retrieved via of_platform ?
>>
>> Indeed, the only exemple I found was in the lp8788_adc driver, which
>> includes iio/machine.h and declares a default mapping, but it is based on
>> platform_data, not of_platform.
>>
> If you are using device tree you can specify the mapping inside the
> devicetree. Have a look at
> Documentation/devicetree/bindings/iio/iio-bindings.txt
>
>
Yes, I saw that. Is there anything that shall be done in the driver to 
support it, or is it automatic ?

Christophe

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

* Re: [PATCH] IIO AD7923 iio_consumer support
  2013-10-05  9:39       ` leroy christophe
@ 2013-10-05  9:43         ` Lars-Peter Clausen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-10-05  9:43 UTC (permalink / raw)
  To: leroy christophe
  Cc: Jonathan Cameron, linux-kernel, linux-iio, patrick.vasseur

On 10/05/2013 11:39 AM, leroy christophe wrote:
> 
> Le 05/10/2013 11:35, Lars-Peter Clausen a écrit :
>> On 10/05/2013 11:18 AM, leroy christophe wrote:
>>> Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
>>>> On 10/05/2013 10:21 AM, Christophe Leroy wrote:
>>>>> +        .consumer_channel = "channel_0",
>>>>> +        .adc_channel_label = "0",
>>>>> +    },
>>>>> +    {
>>>>> +        .consumer_dev_name = AD7923_NAME,
>>>>> +        .consumer_channel = "channel_1",
>>>>> +        .adc_channel_label = "1",
>>>>> +    },
>>>>> +    {
>>>>> +        .consumer_dev_name = AD7923_NAME,
>>>>> +        .consumer_channel = "channel_2",
>>>>> +        .adc_channel_label = "2",
>>>>> +    },
>>>>> +    {
>>>>> +        .consumer_dev_name = AD7923_NAME,
>>>>> +        .consumer_channel = "channel_3",
>>>>> +        .adc_channel_label = "3",
>>>>> +    },
>>>>> +    { }
>>>>> +};
>>>> This is a mapping between channel names of the provider between the channel
>>>> names of the consumer. So it is specific to a certain combination of
>>>> consumer and provider and usually depend on how things are physically wired
>>>> on your board. As such there can be no generic mapping and this needs to go
>>>> into your machine/board driver. The mapping is usually passed to the IIO
>>>> driver via its platform data.
>>>>
>>>> So e.g. imagine you have a provider like this driver and you have a
>>>> consumer
>>>> that has a "voltage" channel. And on your board channel 3 of the ADC is
>>>> what
>>>> you want to route to that consumer. Then your mapping would look like this:
>>>>
>>>> {
>>>>      .consumer_dev_name = "your_consumer_device.1",
>>>>      .consumer_channel = "voltage",
>>>>      .adc_channel_label = "AIN3",
>>>> }
>>>>
>>>> And in your consumer driver you'd do:
>>>>
>>>> channel = iio_channel_get(dev, "voltage");
>>>>
>>>>
>>> Thanks for the explanation.
>>>
>>> Can the mapping be retrieved via of_platform ?
>>>
>>> Indeed, the only exemple I found was in the lp8788_adc driver, which
>>> includes iio/machine.h and declares a default mapping, but it is based on
>>> platform_data, not of_platform.
>>>
>> If you are using device tree you can specify the mapping inside the
>> devicetree. Have a look at
>> Documentation/devicetree/bindings/iio/iio-bindings.txt
>>
>>
> Yes, I saw that. Is there anything that shall be done in the driver to
> support it, or is it automatic ?

It should work out of the box. Just make sure to set the #io-channel-cells
property for the ADC device node in the devicetree.

- Lars

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

end of thread, other threads:[~2013-10-05  9:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-05  8:21 [PATCH] IIO AD7923 iio_consumer support Christophe Leroy
2013-10-05  8:41 ` Lars-Peter Clausen
2013-10-05  9:18   ` leroy christophe
2013-10-05  9:35     ` Lars-Peter Clausen
2013-10-05  9:39       ` leroy christophe
2013-10-05  9:43         ` Lars-Peter Clausen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).