Re: [v2 4/9] staging:iio:adc:ad7298: Use new triggered buffer setup helper function

[Jonathan Cameron <jic23@cam.ac.uk>]

On 6/18/2012 10:45 AM, Lars-Peter Clausen wrote:
> On 06/08/2012 10:48 AM, Lars-Peter Clausen wrote:
>> Use the new triggered buffer setup helper function to allocate and register
>> buffer and pollfunc.
>>
>> Also as part of the conversion drop scan_timestamp being enabled by default,
>> since it is a left over of an earlier cleanup.
>>
>> Signed-off-by: Lars-Peter Clausen<lars@metafoo.de>
>> Acked-by: Jonathan Cameron<jic23@kernel.org>
>> ---
>>   drivers/staging/iio/adc/Kconfig       |    1 +
>>   drivers/staging/iio/adc/ad7298.h      |    5 +++
>>   drivers/staging/iio/adc/ad7298_core.c |   11 ++----
>>   drivers/staging/iio/adc/ad7298_ring.c |   64 ++++++---------------------------
>>   4 files changed, 18 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
>> index c01df61..bb6fffd 100644
>> --- a/drivers/staging/iio/adc/Kconfig
>> +++ b/drivers/staging/iio/adc/Kconfig
>> @@ -13,6 +13,7 @@ config AD7291
>>   config AD7298
>>   	tristate "Analog Devices AD7298 ADC driver"
>>   	depends on SPI
>> +	select IIO_TRIGGERED_BUFFER if IIO_BUFFER
>>   	help
>>   	  Say yes here to build support for Analog Devices AD7298
>>   	  8 Channel ADC with temperature sensor.
>> diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h
>> index 5051a7e..18f2787 100644
>> --- a/drivers/staging/iio/adc/ad7298.h
>> +++ b/drivers/staging/iio/adc/ad7298.h
>> @@ -55,6 +55,8 @@ struct ad7298_state {
>>   #ifdef CONFIG_IIO_BUFFER
>>   int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev);
>>   void ad7298_ring_cleanup(struct iio_dev *indio_dev);
>> +int ad7298_update_scan_mode(struct iio_dev *indio_dev,
>> +	const unsigned long *active_scan_mask);
>>   #else /* CONFIG_IIO_BUFFER */
>>
>>   static inline int
>> @@ -66,5 +68,8 @@ ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev)
>>   static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev)
>>   {
>>   }
>> +
>> +#define ad7298_update_scan_mode NULL
>> +
>>   #endif /* CONFIG_IIO_BUFFER */
>>   #endif /* IIO_ADC_AD7298_H_ */
>> diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c
>> index c90f2b3..5e68bad 100644
>> --- a/drivers/staging/iio/adc/ad7298_core.c
>> +++ b/drivers/staging/iio/adc/ad7298_core.c
>> @@ -171,6 +171,7 @@ static int ad7298_read_raw(struct iio_dev *indio_dev,
>>
>>   static const struct iio_info ad7298_info = {
>>   	.read_raw =&ad7298_read_raw,
>> +	.update_scan_mode = ad7298_update_scan_mode,
>>   	.driver_module = THIS_MODULE,
>>   };
>>
>> @@ -231,19 +232,12 @@ static int __devinit ad7298_probe(struct spi_device *spi)
>>   	if (ret)
>>   		goto error_disable_reg;
>>
>> -	ret = iio_buffer_register(indio_dev,
>> -				&ad7298_channels[1], /* skip temp0 */
>> -				  ARRAY_SIZE(ad7298_channels) - 1);
>
> Ah, dammit... I guess it's always a good idea to do a final thorough review.
>
> I think we had this discussion before and the reason why iio_buffer_register
> take the channels and num_channels because the channels which can be used in
> buffered mode maybe a subset of the total channels. But right now these
> channels have to be either at the beginning or the end of the channel spec.
> I wonder if the following might be a better alternative:
Works for me.
>
> diff --git a/drivers/iio/industrialio-buffer.c
> b/drivers/iio/industrialio-buffer.c
> index 2f35db9..3d8d187 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -285,6 +285,9 @@ int iio_buffer_register(struct iio_dev *indio_dev,
>   	if (channels) {
>   		/* new magic */
>   		for (i = 0; i<  num_channels; i++) {
> +			if (channels[i].scan_index<  0)
> +				continue;
> +
>   			/* Establish necessary mask length */
>   			if (channels[i].scan_index>
>   			(int)indio_dev->masklength - 1)