* [PATCH v2 0/2] Add support for adc101c* and adc121c* @ 2016-04-04 18:21 Crestez Dan Leonard 2016-04-04 18:21 ` [PATCH v2 1/2] ti-adc081c: " Crestez Dan Leonard 2016-04-04 18:21 ` [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support Crestez Dan Leonard 0 siblings, 2 replies; 6+ messages in thread From: Crestez Dan Leonard @ 2016-04-04 18:21 UTC (permalink / raw) To: Jonathan Cameron, linux-iio Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard Changes since last version: * Use struct adcxx1_model in the first patch as well. * Use VLA buf in adc081c_trigger_handler. * Rerrange code so the diffs look nicer. * Fixed other smaller review comments. I did not add an enum for models because it's not clear how adding another layer of indirection would help. Maybe if we ever want to scan the table of models? But that's not useful here. The devices actually have names like ADC121C021/ADC121C027 to identify chips with different pinouts but identical I2C interfaces. Those differences don't currently matter to the driver. Crestez Dan Leonard (2): ti-adc081c: Add support for adc101c* and adc121c* ti-adc081c: Initial triggered buffer support drivers/iio/adc/Kconfig | 6 +-- drivers/iio/adc/ti-adc081c.c | 103 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 94 insertions(+), 15 deletions(-) -- 2.5.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] ti-adc081c: Add support for adc101c* and adc121c* 2016-04-04 18:21 [PATCH v2 0/2] Add support for adc101c* and adc121c* Crestez Dan Leonard @ 2016-04-04 18:21 ` Crestez Dan Leonard 2016-04-10 14:25 ` Jonathan Cameron 2016-04-04 18:21 ` [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support Crestez Dan Leonard 1 sibling, 1 reply; 6+ messages in thread From: Crestez Dan Leonard @ 2016-04-04 18:21 UTC (permalink / raw) To: Jonathan Cameron, linux-iio Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard These chips has an almost identical interface but a deal with a different number of value bits. Datasheet links for comparison: * http://www.ti.com/lit/ds/symlink/adc081c021.pdf * http://www.ti.com/lit/ds/symlink/adc101c021.pdf * http://www.ti.com/lit/ds/symlink/adc121c021.pdf Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com> --- drivers/iio/adc/Kconfig | 6 +++--- drivers/iio/adc/ti-adc081c.c | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 9ddcd5d..a2d0db5 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -378,11 +378,11 @@ config ROCKCHIP_SARADC module will be called rockchip_saradc. config TI_ADC081C - tristate "Texas Instruments ADC081C021/027" + tristate "Texas Instruments ADC081C/ADC101C/ADC121C family" depends on I2C help - If you say yes here you get support for Texas Instruments ADC081C021 - and ADC081C027 ADC chips. + If you say yes here you get support for Texas Instruments ADC081C, + ADC101C and ADC121C ADC chips. This driver can also be built as a module. If so, the module will be called ti-adc081c. diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c index ecbc121..3b3c656 100644 --- a/drivers/iio/adc/ti-adc081c.c +++ b/drivers/iio/adc/ti-adc081c.c @@ -1,9 +1,21 @@ /* + * TI ADC081C/ADC101C/ADC121C 8/10/12-bit ADC driver + * * Copyright (C) 2012 Avionic Design GmbH + * Copyright (C) 2016 Intel * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. + * + * Datasheets: + * http://www.ti.com/lit/ds/symlink/adc081c021.pdf + * http://www.ti.com/lit/ds/symlink/adc101c021.pdf + * http://www.ti.com/lit/ds/symlink/adc121c021.pdf + * + * The devices have a very similar interface and differ mostly in the number of + * bits handled. For the 8-bit and 10-bit models the least-significant 4 or 2 + * bits of value registers are reserved. */ #include <linux/err.h> @@ -17,6 +29,9 @@ struct adc081c { struct i2c_client *i2c; struct regulator *ref; + + /* 8, 10 or 12 */ + int bits; }; #define REG_CONV_RES 0x00 @@ -34,7 +49,7 @@ static int adc081c_read_raw(struct iio_dev *iio, if (err < 0) return err; - *value = (err >> 4) & 0xff; + *value = (err & 0xFFF) >> (12 - adc->bits); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: @@ -43,7 +58,7 @@ static int adc081c_read_raw(struct iio_dev *iio, return err; *value = err / 1000; - *shift = 8; + *shift = adc->bits; return IIO_VAL_FRACTIONAL_LOG2; @@ -60,6 +75,19 @@ static const struct iio_chan_spec adc081c_channel = { .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), }; +struct adcxx1c_model { + int bits; +}; + +#define DEFINE_ADCxx1C_MODEL(_name, _bits) \ + static const struct adcxx1c_model _name ## _model = { \ + .bits = (_bits), \ + } + +DEFINE_ADCxx1C_MODEL(adc081c, 8); +DEFINE_ADCxx1C_MODEL(adc101c, 10); +DEFINE_ADCxx1C_MODEL(adc121c, 12); + static const struct iio_info adc081c_info = { .read_raw = adc081c_read_raw, .driver_module = THIS_MODULE, @@ -70,6 +98,7 @@ static int adc081c_probe(struct i2c_client *client, { struct iio_dev *iio; struct adc081c *adc; + struct adcxx1c_model *model = (struct adcxx1c_model*)id->driver_data; int err; if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA)) @@ -81,6 +110,7 @@ static int adc081c_probe(struct i2c_client *client, adc = iio_priv(iio); adc->i2c = client; + adc->bits = model->bits; adc->ref = devm_regulator_get(&client->dev, "vref"); if (IS_ERR(adc->ref)) @@ -124,7 +154,9 @@ static int adc081c_remove(struct i2c_client *client) } static const struct i2c_device_id adc081c_id[] = { - { "adc081c", 0 }, + { "adc081c", (long)&adc081c_model }, + { "adc101c", (long)&adc101c_model }, + { "adc121c", (long)&adc121c_model }, { } }; MODULE_DEVICE_TABLE(i2c, adc081c_id); @@ -132,6 +164,8 @@ MODULE_DEVICE_TABLE(i2c, adc081c_id); #ifdef CONFIG_OF static const struct of_device_id adc081c_of_match[] = { { .compatible = "ti,adc081c" }, + { .compatible = "ti,adc101c" }, + { .compatible = "ti,adc121c" }, { } }; MODULE_DEVICE_TABLE(of, adc081c_of_match); @@ -149,5 +183,5 @@ static struct i2c_driver adc081c_driver = { module_i2c_driver(adc081c_driver); MODULE_AUTHOR("Thierry Reding <thierry.reding@avionic-design.de>"); -MODULE_DESCRIPTION("Texas Instruments ADC081C021/027 driver"); +MODULE_DESCRIPTION("Texas Instruments ADC081C/ADC101C/ADC121C driver"); MODULE_LICENSE("GPL v2"); -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] ti-adc081c: Add support for adc101c* and adc121c* 2016-04-04 18:21 ` [PATCH v2 1/2] ti-adc081c: " Crestez Dan Leonard @ 2016-04-10 14:25 ` Jonathan Cameron 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Cameron @ 2016-04-10 14:25 UTC (permalink / raw) To: Crestez Dan Leonard, linux-iio Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta On 04/04/16 19:21, Crestez Dan Leonard wrote: > These chips has an almost identical interface but a deal with a > different number of value bits. Datasheet links for comparison: > > * http://www.ti.com/lit/ds/symlink/adc081c021.pdf > * http://www.ti.com/lit/ds/symlink/adc101c021.pdf > * http://www.ti.com/lit/ds/symlink/adc121c021.pdf > > Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com> A trivial little request inline to move over to an array of structures describing the different parts and use an enum indexing that instead of pointers in the i2c_device_id tables. Otherwise looks good. > --- > drivers/iio/adc/Kconfig | 6 +++--- > drivers/iio/adc/ti-adc081c.c | 42 ++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 41 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 9ddcd5d..a2d0db5 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -378,11 +378,11 @@ config ROCKCHIP_SARADC > module will be called rockchip_saradc. > > config TI_ADC081C > - tristate "Texas Instruments ADC081C021/027" > + tristate "Texas Instruments ADC081C/ADC101C/ADC121C family" > depends on I2C > help > - If you say yes here you get support for Texas Instruments ADC081C021 > - and ADC081C027 ADC chips. > + If you say yes here you get support for Texas Instruments ADC081C, > + ADC101C and ADC121C ADC chips. > > This driver can also be built as a module. If so, the module will be > called ti-adc081c. > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c > index ecbc121..3b3c656 100644 > --- a/drivers/iio/adc/ti-adc081c.c > +++ b/drivers/iio/adc/ti-adc081c.c > @@ -1,9 +1,21 @@ > /* > + * TI ADC081C/ADC101C/ADC121C 8/10/12-bit ADC driver > + * > * Copyright (C) 2012 Avionic Design GmbH > + * Copyright (C) 2016 Intel > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 as > * published by the Free Software Foundation. > + * > + * Datasheets: > + * http://www.ti.com/lit/ds/symlink/adc081c021.pdf > + * http://www.ti.com/lit/ds/symlink/adc101c021.pdf > + * http://www.ti.com/lit/ds/symlink/adc121c021.pdf > + * > + * The devices have a very similar interface and differ mostly in the number of > + * bits handled. For the 8-bit and 10-bit models the least-significant 4 or 2 > + * bits of value registers are reserved. > */ > > #include <linux/err.h> > @@ -17,6 +29,9 @@ > struct adc081c { > struct i2c_client *i2c; > struct regulator *ref; > + > + /* 8, 10 or 12 */ > + int bits; > }; > > #define REG_CONV_RES 0x00 > @@ -34,7 +49,7 @@ static int adc081c_read_raw(struct iio_dev *iio, > if (err < 0) > return err; > > - *value = (err >> 4) & 0xff; > + *value = (err & 0xFFF) >> (12 - adc->bits); > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > @@ -43,7 +58,7 @@ static int adc081c_read_raw(struct iio_dev *iio, > return err; > > *value = err / 1000; > - *shift = 8; > + *shift = adc->bits; > > return IIO_VAL_FRACTIONAL_LOG2; > > @@ -60,6 +75,19 @@ static const struct iio_chan_spec adc081c_channel = { > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > }; > > +struct adcxx1c_model { > + int bits; > +}; > + > +#define DEFINE_ADCxx1C_MODEL(_name, _bits) \ > + static const struct adcxx1c_model _name ## _model = { \ > + .bits = (_bits), \ > + } > + > +DEFINE_ADCxx1C_MODEL(adc081c, 8); > +DEFINE_ADCxx1C_MODEL(adc101c, 10); > +DEFINE_ADCxx1C_MODEL(adc121c, 12); As suggested below, I'd prefer these were elements of an array of such structures as it tends to lead to cleaner / more obvious code in my view. > + > static const struct iio_info adc081c_info = { > .read_raw = adc081c_read_raw, > .driver_module = THIS_MODULE, > @@ -70,6 +98,7 @@ static int adc081c_probe(struct i2c_client *client, > { > struct iio_dev *iio; > struct adc081c *adc; > + struct adcxx1c_model *model = (struct adcxx1c_model*)id->driver_data; > int err; > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA)) > @@ -81,6 +110,7 @@ static int adc081c_probe(struct i2c_client *client, > > adc = iio_priv(iio); > adc->i2c = client; > + adc->bits = model->bits; > > adc->ref = devm_regulator_get(&client->dev, "vref"); > if (IS_ERR(adc->ref)) > @@ -124,7 +154,9 @@ static int adc081c_remove(struct i2c_client *client) > } > > static const struct i2c_device_id adc081c_id[] = { > - { "adc081c", 0 }, > + { "adc081c", (long)&adc081c_model }, > + { "adc101c", (long)&adc101c_model }, > + { "adc121c", (long)&adc121c_model }, It's often cleaner to use an enum instead of direct pointers. Avoids some nasty casting like you have needed to do here. The enum can then be used to reference into an array of model description structures. > { } > }; > MODULE_DEVICE_TABLE(i2c, adc081c_id); > @@ -132,6 +164,8 @@ MODULE_DEVICE_TABLE(i2c, adc081c_id); > #ifdef CONFIG_OF > static const struct of_device_id adc081c_of_match[] = { > { .compatible = "ti,adc081c" }, > + { .compatible = "ti,adc101c" }, > + { .compatible = "ti,adc121c" }, > { } > }; > MODULE_DEVICE_TABLE(of, adc081c_of_match); > @@ -149,5 +183,5 @@ static struct i2c_driver adc081c_driver = { > module_i2c_driver(adc081c_driver); > > MODULE_AUTHOR("Thierry Reding <thierry.reding@avionic-design.de>"); > -MODULE_DESCRIPTION("Texas Instruments ADC081C021/027 driver"); > +MODULE_DESCRIPTION("Texas Instruments ADC081C/ADC101C/ADC121C driver"); > MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support 2016-04-04 18:21 [PATCH v2 0/2] Add support for adc101c* and adc121c* Crestez Dan Leonard 2016-04-04 18:21 ` [PATCH v2 1/2] ti-adc081c: " Crestez Dan Leonard @ 2016-04-04 18:21 ` Crestez Dan Leonard 2016-04-04 18:39 ` kbuild test robot 2016-04-10 14:29 ` Jonathan Cameron 1 sibling, 2 replies; 6+ messages in thread From: Crestez Dan Leonard @ 2016-04-04 18:21 UTC (permalink / raw) To: Jonathan Cameron, linux-iio Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard 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 <leonard.crestez@intel.com> --- drivers/iio/adc/ti-adc081c.c | 63 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c index 3b3c656..d024df2 100644 --- a/drivers/iio/adc/ti-adc081c.c +++ b/drivers/iio/adc/ti-adc081c.c @@ -24,6 +24,9 @@ #include <linux/of.h> #include <linux/iio/iio.h> +#include <linux/iio/buffer.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> #include <linux/regulator/consumer.h> struct adc081c { @@ -69,18 +72,31 @@ 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, \ + }, \ +} struct adcxx1c_model { + const struct iio_chan_spec* channels; int bits; }; #define DEFINE_ADCxx1C_MODEL(_name, _bits) \ + static const struct iio_chan_spec _name ## _channels[] = { \ + ADCxx1C_CHAN((_bits)), \ + IIO_CHAN_SOFT_TIMESTAMP(1), \ + }; \ static const struct adcxx1c_model _name ## _model = { \ + .channels = _name ## _channels, \ .bits = (_bits), \ } @@ -88,11 +104,31 @@ DEFINE_ADCxx1C_MODEL(adc081c, 8); DEFINE_ADCxx1C_MODEL(adc101c, 10); DEFINE_ADCxx1C_MODEL(adc121c, 12); +#define ADC081C_NUM_CHANNELS 2 + static const struct iio_info adc081c_info = { .read_raw = adc081c_read_raw, .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[indio_dev->scan_bytes / 2]; + 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) { @@ -125,18 +161,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: +err_buffer_cleanup: + iio_triggered_buffer_cleanup(iio); +err_regulator_disable: regulator_disable(adc->ref); return err; @@ -148,6 +192,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; -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support 2016-04-04 18:21 ` [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support Crestez Dan Leonard @ 2016-04-04 18:39 ` kbuild test robot 2016-04-10 14:29 ` Jonathan Cameron 1 sibling, 0 replies; 6+ messages in thread From: kbuild test robot @ 2016-04-04 18:39 UTC (permalink / raw) To: Crestez Dan Leonard Cc: kbuild-all, Jonathan Cameron, linux-iio, linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard [-- Attachment #1: Type: text/plain, Size: 2026 bytes --] Hi Crestez, [auto build test WARNING on iio/togreg] [also build test WARNING on v4.6-rc2 next-20160404] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Crestez-Dan-Leonard/Add-support-for-adc101c-and-adc121c/20160405-022459 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: s390-allmodconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=s390 All warnings (new ones prefixed by >>): drivers/iio/adc/ti-adc081c.c: In function 'adc081c_trigger_handler': >> drivers/iio/adc/ti-adc081c.c:130:1: warning: 'adc081c_trigger_handler' uses dynamic stack allocation } ^ vim +/adc081c_trigger_handler +130 drivers/iio/adc/ti-adc081c.c 114 static irqreturn_t adc081c_trigger_handler(int irq, void *p) 115 { 116 struct iio_poll_func *pf = p; 117 struct iio_dev *indio_dev = pf->indio_dev; 118 struct adc081c *data = iio_priv(indio_dev); 119 u16 buf[indio_dev->scan_bytes / 2]; 120 int ret; 121 122 ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES); 123 if (ret < 0) 124 goto out; 125 buf[0] = ret; 126 iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns()); 127 out: 128 iio_trigger_notify_done(indio_dev->trig); 129 return IRQ_HANDLED; > 130 } 131 132 static int adc081c_probe(struct i2c_client *client, 133 const struct i2c_device_id *id) 134 { 135 struct iio_dev *iio; 136 struct adc081c *adc; 137 struct adcxx1c_model *model = (struct adcxx1c_model*)id->driver_data; 138 int err; --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/octet-stream, Size: 40033 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support 2016-04-04 18:21 ` [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support Crestez Dan Leonard 2016-04-04 18:39 ` kbuild test robot @ 2016-04-10 14:29 ` Jonathan Cameron 1 sibling, 0 replies; 6+ messages in thread From: Jonathan Cameron @ 2016-04-10 14:29 UTC (permalink / raw) To: Crestez Dan Leonard, linux-iio Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta On 04/04/16 19:21, 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 <leonard.crestez@intel.com> basically fine, but just use a fixed size allocation for your buffer in the trigger handler. > --- > drivers/iio/adc/ti-adc081c.c | 63 +++++++++++++++++++++++++++++++++++++------- > 1 file changed, 54 insertions(+), 9 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c > index 3b3c656..d024df2 100644 > --- a/drivers/iio/adc/ti-adc081c.c > +++ b/drivers/iio/adc/ti-adc081c.c > @@ -24,6 +24,9 @@ > #include <linux/of.h> > > #include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > #include <linux/regulator/consumer.h> > > struct adc081c { > @@ -69,18 +72,31 @@ 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, \ > + }, \ > +} > > struct adcxx1c_model { > + const struct iio_chan_spec* channels; > int bits; > }; > > #define DEFINE_ADCxx1C_MODEL(_name, _bits) \ > + static const struct iio_chan_spec _name ## _channels[] = { \ > + ADCxx1C_CHAN((_bits)), \ > + IIO_CHAN_SOFT_TIMESTAMP(1), \ > + }; \ > static const struct adcxx1c_model _name ## _model = { \ > + .channels = _name ## _channels, \ > .bits = (_bits), \ > } > > @@ -88,11 +104,31 @@ DEFINE_ADCxx1C_MODEL(adc081c, 8); > DEFINE_ADCxx1C_MODEL(adc101c, 10); > DEFINE_ADCxx1C_MODEL(adc121c, 12); > > +#define ADC081C_NUM_CHANNELS 2 > + > static const struct iio_info adc081c_info = { > .read_raw = adc081c_read_raw, > .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[indio_dev->scan_bytes / 2]; Just use a fixed size that is beig enough for whatever you may get. Here you need room for a word + an aligned 8 byte timestamp so 16 bytes, or 8 u16s. It's not variable so don't let the compiler think it is. Also you shouldn't be accessing indio_dev->scan_bytes directly in a driver. A long time back we spent a while pulling that idiom out of drivers as it was causing a lot more trouble than the convenience was worth (can't actually recall why now though!) > + 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) > { > @@ -125,18 +161,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; I'm not sure the define adds anything here! Kind of obvious what 2 means anyway - rather than it being a magic number :) > + > + 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: > +err_buffer_cleanup: > + iio_triggered_buffer_cleanup(iio); > +err_regulator_disable: > regulator_disable(adc->ref); > > return err; > @@ -148,6 +192,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; > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-10 14:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-04 18:21 [PATCH v2 0/2] Add support for adc101c* and adc121c* Crestez Dan Leonard 2016-04-04 18:21 ` [PATCH v2 1/2] ti-adc081c: " Crestez Dan Leonard 2016-04-10 14:25 ` Jonathan Cameron 2016-04-04 18:21 ` [PATCH v2 2/2] ti-adc081c: Initial triggered buffer support Crestez Dan Leonard 2016-04-04 18:39 ` kbuild test robot 2016-04-10 14:29 ` Jonathan Cameron
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).