[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aD27cobHWeBX8o30@smile.fi.intel.com>
Date: Mon, 2 Jun 2025 17:55:46 +0300
From: Andy Shevchenko <andy@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
Ana-Maria Cusco <ana-maria.cusco@...log.com>, jic23@...nel.org,
lars@...afoo.de, Michael.Hennerich@...log.com,
dlechner@...libre.com, nuno.sa@...log.com, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, linus.walleij@...aro.org,
brgl@...ev.pl, marcelo.schmitt1@...il.com
Subject: Re: [PATCH v4 02/11] iio: adc: Add basic support for AD4170
On Mon, Jun 02, 2025 at 08:36:47AM -0300, Marcelo Schmitt wrote:
> From: Ana-Maria Cusco <ana-maria.cusco@...log.com>
>
> The AD4170 is a multichannel, low noise, 24-bit precision sigma-delta
> analog to digital converter. The AD4170 design offers a flexible data
> acquisition solution with crosspoint multiplexed analog inputs,
> configurable ADC voltage reference inputs, ultra-low noise integrated PGA,
> digital filtering, wide range of configurable output data rates, internal
> oscillator and temperature sensor, four GPIOs, and integrated features for
> interfacing with load cell weigh scales, RTD, and thermocouple sensors.
>
> Add basic support for the AD4170 ADC with the following features:
> - Single-shot read.
> - Analog front end PGA configuration.
> - Differential and pseudo-differential input configuration.
...
+ array_size.h
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/bits.h>
bitmap.h guarantees to include the two above, but it;s not a big deal to leave
them.
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
+ types.h
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
...
> +static bool ad4170_setup_eq(struct ad4170_setup *a, struct ad4170_setup *b)
> +{
> + /*
> + * The use of static_assert() here is to make sure that the comparison
> + * is adapted whenever struct ad4170_setup is changed.
> + */
> + static_assert(sizeof(*a) ==
> + sizeof(struct {
> + u16 misc;
> + u16 afe;
> + u16 filter;
> + u16 filter_fs;
> + u32 offset;
> + u32 gain;
> + }));
I think it doesn't make much sense unless one uses memcpy().
> + if (a->misc != b->misc ||
> + a->afe != b->afe ||
> + a->filter != b->filter ||
> + a->filter_fs != b->filter_fs ||
> + a->offset != b->offset ||
> + a->gain != b->gain)
> + return false;
> +
> + return true;
> +}
...
> +static int ad4170_soft_reset(struct ad4170_state *st)
> +{
> + int ret;
> +
> + ret = regmap_write(st->regmap, AD4170_CONFIG_A_REG,
> + AD4170_SW_RESET_MSK);
> + if (ret)
> + return ret;
> +
> + /* AD4170-4 requires 1 ms between reset and any register access. */
> + fsleep(MILLI);
1 * USEC_PER_MSEC
(need time.h)
...
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
+ blank line here.
> + ret = devm_mutex_init(dev, &st->lock);
> + if (ret)
> + return ret;
> + chip = spi_get_device_match_data(spi);
> + if (!chip)
> + return -EINVAL;
> +
> + indio_dev->name = chip->name;
> + indio_dev->info = &ad4170_info;
> + st->spi = spi;
Can be grouped with above st = ...
> + st->regmap = devm_regmap_init(dev, NULL, st, &ad4170_regmap_config);
> + if (IS_ERR(st->regmap))
> + return dev_err_probe(dev, PTR_ERR(st->regmap),
> + "Failed to initialize regmap\n");
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists