Re: [PATCH] iio: adc: ad799x: add reference supply for ad7994

Re: [PATCH] iio: adc: ad799x: add reference supply for ad7994

[Andy Shevchenko]

On Tue, Aug 5, 2025 at 2:28 PM Stefano Manni <stefano.manni@gmail.com> wrote:
>
> AD7994 supports the external reference voltage on pin REFIN.

...

> -               if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)) {
> +               if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)
> +                       (st->id == ad7994)) {

Instead of making this conditional longer and uglier, it is better to
add a boolean field to chip_info and just check it instead. This will
remove the churn when any new chip will require the same change in the
future.

So, please make it two patches:
- introducing a field in chip_info and use it for the existing cases
- add this field to be true for the ad7994 case.

Is it doable?

-- 
With Best Regards,
Andy Shevchenko

[PATCH] iio: adc: ad799x: add reference supply for ad7994

[Stefano Manni]

AD7994 supports the external reference voltage on pin REFIN.

Signed-off-by: Stefano Manni <stefano.manni@gmail.com>
---
 drivers/iio/adc/ad799x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 9c02f9199139..e5167dba925f 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -826,7 +826,8 @@ static int ad799x_probe(struct i2c_client *client)
 		 * Use external reference voltage if supported by hardware.
 		 * This is optional if voltage / regulator present, use VCC otherwise.
 		 */
-		if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)) {
+		if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)
+			(st->id == ad7994)) {
 			dev_info(&client->dev, "Using external reference voltage\n");
 			extra_config |= AD7991_REF_SEL;
 			ret = regulator_enable(st->vref);
-- 
2.48.1

Re: [PATCH] iio: adc: ad799x: add reference supply for ad7994

[Nuno Sá]

On Tue, Aug 05, 2025 at 03:13:06PM +0200, Andy Shevchenko wrote:
> On Tue, Aug 5, 2025 at 2:28 PM Stefano Manni <stefano.manni@gmail.com> wrote:
> >
> > AD7994 supports the external reference voltage on pin REFIN.
> 
> ...
> 
> > -               if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)) {
> > +               if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)
> > +                       (st->id == ad7994)) {
> 
> Instead of making this conditional longer and uglier, it is better to
> add a boolean field to chip_info and just check it instead. This will
> remove the churn when any new chip will require the same change in the
> future.
> 
> So, please make it two patches:
> - introducing a field in chip_info and use it for the existing cases
> - add this field to be true for the ad7994 case.

Agreed!

Plus either is already too late for my eyes but I don't think this patch
is compilable.

- Nuno Sá
> 
> Is it doable?
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] iio: adc: ad799x: add reference supply for ad7994

[Stefano Manni]

Hi Andy,

On mar, ago 5 2025 at 15:13:06 +02:00:00, Andy Shevchenko 
<andy.shevchenko@gmail.com> wrote:
> On Tue, Aug 5, 2025 at 2:28 PM Stefano Manni 
> <stefano.manni@gmail.com> wrote:
>> 
>>  AD7994 supports the external reference voltage on pin REFIN.
> 
> ...
> 
>>  -               if ((st->id == ad7991) || (st->id == ad7995) || 
>> (st->id == ad7999)) {
>>  +               if ((st->id == ad7991) || (st->id == ad7995) || 
>> (st->id == ad7999)
>>  +                       (st->id == ad7994)) {
> 
> Instead of making this conditional longer and uglier, it is better to
> add a boolean field to chip_info and just check it instead. This will
> remove the churn when any new chip will require the same change in the
> future.
> 
> So, please make it two patches:
> - introducing a field in chip_info and use it for the existing cases
> - add this field to be true for the ad7994 case.
> 
> Is it doable?
> 

Yes, it is. A patch series is coming.

> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] iio: adc: ad799x: add reference supply for ad7994

[kernel test robot]

Hi Stefano,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.16 next-20250806]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Stefano-Manni/iio-adc-ad799x-add-reference-supply-for-ad7994/20250806-125049
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250805142423.17710-1-stefano.manni%40gmail.com
patch subject: [PATCH] iio: adc: ad799x: add reference supply for ad7994
config: arc-randconfig-002-20250806 (https://download.01.org/0day-ci/archive/20250806/202508062313.Up1PAzZN-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250806/202508062313.Up1PAzZN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508062313.Up1PAzZN-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iio/adc/ad799x.c: In function 'ad799x_probe':
>> drivers/iio/adc/ad799x.c:829:73: error: called object is not a function or function pointer
     829 |                 if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)
         |                                                                 ~~~~~~~~^~~~~~~~~~


vim +829 drivers/iio/adc/ad799x.c

   777	
   778	static int ad799x_probe(struct i2c_client *client)
   779	{
   780		const struct i2c_device_id *id = i2c_client_get_device_id(client);
   781		int ret;
   782		int extra_config = 0;
   783		struct ad799x_state *st;
   784		struct iio_dev *indio_dev;
   785		const struct ad799x_chip_info *chip_info =
   786			&ad799x_chip_info_tbl[id->driver_data];
   787	
   788		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
   789		if (indio_dev == NULL)
   790			return -ENOMEM;
   791	
   792		st = iio_priv(indio_dev);
   793		/* this is only used for device removal purposes */
   794		i2c_set_clientdata(client, indio_dev);
   795	
   796		st->id = id->driver_data;
   797		if (client->irq > 0 && chip_info->irq_config.info)
   798			st->chip_config = &chip_info->irq_config;
   799		else
   800			st->chip_config = &chip_info->noirq_config;
   801	
   802		/* TODO: Add pdata options for filtering and bit delay */
   803	
   804		st->reg = devm_regulator_get(&client->dev, "vcc");
   805		if (IS_ERR(st->reg))
   806			return PTR_ERR(st->reg);
   807		ret = regulator_enable(st->reg);
   808		if (ret)
   809			return ret;
   810	
   811		/* check if an external reference is supplied */
   812		st->vref = devm_regulator_get_optional(&client->dev, "vref");
   813	
   814		if (IS_ERR(st->vref)) {
   815			if (PTR_ERR(st->vref) == -ENODEV) {
   816				st->vref = NULL;
   817				dev_info(&client->dev, "Using VCC reference voltage\n");
   818			} else {
   819				ret = PTR_ERR(st->vref);
   820				goto error_disable_reg;
   821			}
   822		}
   823	
   824		if (st->vref) {
   825			/*
   826			 * Use external reference voltage if supported by hardware.
   827			 * This is optional if voltage / regulator present, use VCC otherwise.
   828			 */
 > 829			if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)
   830				(st->id == ad7994)) {
   831				dev_info(&client->dev, "Using external reference voltage\n");
   832				extra_config |= AD7991_REF_SEL;
   833				ret = regulator_enable(st->vref);
   834				if (ret)
   835					goto error_disable_reg;
   836			} else {
   837				st->vref = NULL;
   838				dev_warn(&client->dev, "Supplied reference not supported\n");
   839			}
   840		}
   841	
   842		st->client = client;
   843	
   844		indio_dev->name = id->name;
   845		indio_dev->info = st->chip_config->info;
   846	
   847		indio_dev->modes = INDIO_DIRECT_MODE;
   848		indio_dev->channels = st->chip_config->channel;
   849		indio_dev->num_channels = chip_info->num_channels;
   850	
   851		ret = ad799x_update_config(st, st->chip_config->default_config | extra_config);
   852		if (ret)
   853			goto error_disable_vref;
   854	
   855		ret = iio_triggered_buffer_setup(indio_dev, NULL,
   856			&ad799x_trigger_handler, NULL);
   857		if (ret)
   858			goto error_disable_vref;
   859	
   860		if (client->irq > 0) {
   861			ret = devm_request_threaded_irq(&client->dev,
   862							client->irq,
   863							NULL,
   864							ad799x_event_handler,
   865							IRQF_TRIGGER_FALLING |
   866							IRQF_ONESHOT,
   867							client->name,
   868							indio_dev);
   869			if (ret)
   870				goto error_cleanup_ring;
   871		}
   872	
   873		mutex_init(&st->lock);
   874	
   875		ret = iio_device_register(indio_dev);
   876		if (ret)
   877			goto error_cleanup_ring;
   878	
   879		return 0;
   880	
   881	error_cleanup_ring:
   882		iio_triggered_buffer_cleanup(indio_dev);
   883	error_disable_vref:
   884		if (st->vref)
   885			regulator_disable(st->vref);
   886	error_disable_reg:
   887		regulator_disable(st->reg);
   888	
   889		return ret;
   890	}
   891	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki