Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751727AbcDQJqg (ORCPT ); Sun, 17 Apr 2016 05:46:36 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:52725 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751463AbcDQJqf (ORCPT ); Sun, 17 Apr 2016 05:46:35 -0400 Subject: Re: [PATCH v3 2/2] ti-adc081c: Initial triggered buffer support To: Crestez Dan Leonard , linux-iio@vger.kernel.org References: <0031e3eb1f3a5bb4a29a201698cc6a6ba75dc7ec.1460384543.git.leonard.crestez@intel.com> Cc: linux-kernel@vger.kernel.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Daniel Baluta From: Jonathan Cameron Message-ID: <57135B75.6010103@kernel.org> Date: Sun, 17 Apr 2016 10:46:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <0031e3eb1f3a5bb4a29a201698cc6a6ba75dc7ec.1460384543.git.leonard.crestez@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4808 Lines: 162 On 11/04/16 15:24, Crestez Dan Leonard wrote: > Using this requires software triggers like CONFIG_IIO_HRTIMER_TRIGGER. > > The device can be configured to do internal periodic sampling but does not > offer some sort of interrupt on data ready. Interrupts can only trigger when > values get out of a specific range. > > Signed-off-by: Crestez Dan Leonard One totally trivial comment inline. Applied to the togreg branch of iio.git - pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/iio/adc/ti-adc081c.c | 77 ++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 64 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c > index b977340..9fd032d 100644 > --- a/drivers/iio/adc/ti-adc081c.c > +++ b/drivers/iio/adc/ti-adc081c.c > @@ -24,6 +24,9 @@ > #include > > #include > +#include > +#include > +#include > #include > > struct adc081c { > @@ -69,21 +72,42 @@ static int adc081c_read_raw(struct iio_dev *iio, > return -EINVAL; > } > > -static const struct iio_chan_spec adc081c_channel = { > - .type = IIO_VOLTAGE, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > -}; > +#define ADCxx1C_CHAN(_bits) { \ > + .type = IIO_VOLTAGE, \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = (_bits), \ > + .storagebits = 16, \ > + .shift = 12 - (_bits), \ > + .endianness = IIO_CPU, \ > + }, \ > +} > + > +#define DEFINE_ADCxx1C_CHANNELS(_name, _bits) \ > + static const struct iio_chan_spec _name ## _channels[] = { \ > + ADCxx1C_CHAN((_bits)), \ > + IIO_CHAN_SOFT_TIMESTAMP(1), \ > + }; \ > + > +#define ADC081C_NUM_CHANNELS 2 > > struct adcxx1c_model { > + const struct iio_chan_spec* channels; > int bits; > }; > > -#define ADCxx1C_MODEL(_bits) \ > +#define ADCxx1C_MODEL(_name, _bits) \ > { \ > + .channels = _name ## _channels, \ > .bits = (_bits), \ > } > > +DEFINE_ADCxx1C_CHANNELS(adc081c, 8); > +DEFINE_ADCxx1C_CHANNELS(adc101c, 10); > +DEFINE_ADCxx1C_CHANNELS(adc121c, 12); > + > /* Model ids are indexes in _models array */ > enum adcxx1c_model_id { > ADC081C = 0, > @@ -92,9 +116,9 @@ enum adcxx1c_model_id { > }; > > static struct adcxx1c_model adcxx1c_models[] = { > - ADCxx1C_MODEL( 8), > - ADCxx1C_MODEL(10), > - ADCxx1C_MODEL(12), > + ADCxx1C_MODEL(adc081c, 8), > + ADCxx1C_MODEL(adc101c, 10), > + ADCxx1C_MODEL(adc121c, 12), > }; > > static const struct iio_info adc081c_info = { > @@ -102,6 +126,24 @@ static const struct iio_info adc081c_info = { > .driver_module = THIS_MODULE, > }; > > +static irqreturn_t adc081c_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct adc081c *data = iio_priv(indio_dev); > + u16 buf[8]; /* 2 bytes data + 6 bytes padding + 8 bytes timestamp */ > + int ret; > + > + ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES); > + if (ret < 0) > + goto out; > + buf[0] = ret; > + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns()); > +out: > + iio_trigger_notify_done(indio_dev->trig); > + return IRQ_HANDLED; > +} > + > static int adc081c_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -134,18 +176,26 @@ static int adc081c_probe(struct i2c_client *client, > iio->modes = INDIO_DIRECT_MODE; > iio->info = &adc081c_info; > > - iio->channels = &adc081c_channel; > - iio->num_channels = 1; > + iio->channels = model->channels; > + iio->num_channels = ADC081C_NUM_CHANNELS; > + > + err = iio_triggered_buffer_setup(iio, NULL, adc081c_trigger_handler, NULL); > + if (err < 0) { > + dev_err(&client->dev, "iio triggered buffer setup failed\n"); > + goto err_regulator_disable; > + } > > err = iio_device_register(iio); > if (err < 0) > - goto regulator_disable; > + goto err_buffer_cleanup; > > i2c_set_clientdata(client, iio); > > return 0; > > -regulator_disable: Technically an unrelated change, but can't say I really care! > +err_buffer_cleanup: > + iio_triggered_buffer_cleanup(iio); > +err_regulator_disable: > regulator_disable(adc->ref); > > return err; > @@ -157,6 +207,7 @@ static int adc081c_remove(struct i2c_client *client) > struct adc081c *adc = iio_priv(iio); > > iio_device_unregister(iio); > + iio_triggered_buffer_cleanup(iio); > regulator_disable(adc->ref); > > return 0; >