| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a1fc70b489cc6396200cb777795183f42ed31719.camel@gmail.com>
Date: Mon, 26 May 2025 11:21:17 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: 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, andy@...nel.org, 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 v3 02/10] iio: adc: Add basic support for AD4170
On Tue, 2025-05-13 at 09:34 -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
> aquisition 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.
>
> Signed-off-by: Ana-Maria Cusco <ana-maria.cusco@...log.com>
> Co-developed-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> ---
Looks very good. Just some small notes...
> Change log v2 -> v3
> - Updated Copyright year.
> - Separated handling of channel setup cases for better understanding of code
> flow.
> - Now comparing setups field by field instead of using memcmp().
> - Disable channel on ad4170_read_sample() error path.
> - Reinit completion before entering single conversion mode.
> - Organized ad4170_sinc3_filt_fs_tbl.
> - Used clamp to simplify configuration value checking.
> - Returned earlier whenever possible.
> - Used HZ_PER_KHZ/MHZ.
> - Declared internal voltage reference constant AD4170_INT_REF_2_5V
> - Many other minor code style and readability improvements.
>
> MAINTAINERS | 1 +
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad4170.c | 1553 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1567 insertions(+)
> create mode 100644 drivers/iio/adc/ad4170.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0a8f2c7a139c..541e37ed304e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1352,6 +1352,7 @@ L: linux-iio@...r.kernel.org
> S: Supported
> W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> +F: drivers/iio/adc/ad4170.c
>
> ANALOG DEVICES INC AD4695 DRIVER
> M: Michael Hennerich <michael.hennerich@...log.com>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 0fe6601e59ed..594b9f55ec0a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -70,6 +70,18 @@ config AD4130
> To compile this driver as a module, choose M here: the module will
> be
> called ad4130.
>
> +
> +config AD4170
> + tristate "Analog Device AD4170 ADC Driver"
> + depends on SPI
> + select REGMAP_SPI
> + help
> + Say yes here to build support for Analog Devices AD4170 SPI analog
> + to digital converters (ADC).
> +
> + To compile this driver as a module, choose M here: the module will
> be
> + called ad4170.
> +
> config AD4695
> tristate "Analog Device AD4695 ADC Driver"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 07d4b832c42e..d3a1376d1f96 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4030) += ad4030.o
> obj-$(CONFIG_AD4130) += ad4130.o
> +obj-$(CONFIG_AD4170) += ad4170.o
> obj-$(CONFIG_AD4695) += ad4695.o
> obj-$(CONFIG_AD4851) += ad4851.o
> obj-$(CONFIG_AD7091R) += ad7091r-base.o
> diff --git a/drivers/iio/adc/ad4170.c b/drivers/iio/adc/ad4170.c
> new file mode 100644
> index 000000000000..bf19b31095ee
> --- /dev/null
> +++ b/drivers/iio/adc/ad4170.c
> @@ -0,0 +1,1553 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2025 Analog Devices, Inc.
> + * Author: Ana-Maria Cusco <ana-maria.cusco@...log.com>
> + * Author: Marcelo Schmitt <marcelo.schmitt@...log.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/bits.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>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
>
...
> +
> +static int ad4170_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg, unsigned int writeval,
> + unsigned int *readval)
> +{
> + struct ad4170_state *st = iio_priv(indio_dev);
> +
> + if (readval)
> + return regmap_read(st->regmap, reg, readval);
> + else
> + return regmap_write(st->regmap, reg, writeval);
redundant else
...
>
> +
> +/*
> + * Sets the ADC operating mode. Supported modes are
> + * - Single conversion mode
> + * - Idle mode
> + */
> +static int ad4170_set_mode(struct ad4170_state *st, unsigned int mode)
> +{
> + return regmap_update_bits(st->regmap, AD4170_ADC_CTRL_REG,
> + AD4170_ADC_CTRL_MODE_MSK,
> + FIELD_PREP(AD4170_ADC_CTRL_MODE_MSK,
> mode));
> +}
I'm usually not a fan of these wrappers.
...
>
> +
> +static int ad4170_parse_firmware(struct iio_dev *indio_dev)
> +{
> + struct ad4170_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + int reg_data, ret;
> + unsigned int i;
> +
> + st->mclk_hz = AD4170_INT_CLOCK_16MHZ;
> +
> + for (i = 0; i < AD4170_NUM_ANALOG_PINS; i++)
> + st->pins_fn[i] = AD4170_PIN_UNASIGNED;
Isn't the above the default already?
- Nuno Sá
Powered by blists - more mailing lists