| 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: <4a357e2412efe86cd9efc46baeb2e5d475e29515.camel@gmail.com>
Date: Mon, 26 May 2025 11:00:45 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Pop Ioan Daniel <pop.ioan-daniel@...log.com>, Lars-Peter Clausen
<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, David Lechner
<dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Sergiu Cuciurean <sergiu.cuciurean@...log.com>,
Dragos Bogdan <dragos.bogdan@...log.com>, Antoniu Miclaus
<antoniu.miclaus@...log.com>, Olivier Moysan <olivier.moysan@...s.st.com>,
Javier Carrasco <javier.carrasco.cruz@...il.com>, Matti Vaittinen
<mazziesaccount@...il.com>, Tobias Sperling <tobias.sperling@...ting.com>,
Alisa-Dariana Roman <alisadariana@...il.com>, Marcelo Schmitt
<marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 6/6] iio: adc: ad7405: add ad7405 driver
On Mon, 2025-05-19 at 17:02 +0300, Pop Ioan Daniel wrote:
> Add support for the AD7405/ADUM770x, a high performance isolated ADC,
> 1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
> analog input signal into a high speed, single-bit data stream.
>
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@...log.com>
> ---
Hi,
Just a couple of notes from me...
> changes in v4:
> - changes in "depends on" instead of "select" for IIO_BACKEND in Kconfig
> - remove scale_tables variables
> - remove sample_frequency_tbl, sample_frequency
> - remove ad7405_fill_samp_freq_table function
> - rewrite ad7405_get_scale function as suggested
> - make sampling_frequency read-only
> - implement IIO_CHAN_INFO_OFFSET as suggested
> - fix code style
>
> drivers/iio/adc/Kconfig | 10 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad7405.c | 250 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 261 insertions(+)
> create mode 100644 drivers/iio/adc/ad7405.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index ad06cf556785..43af2070e27f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -251,6 +251,16 @@ config AD7380
> To compile this driver as a module, choose M here: the module will
> be
> called ad7380.
>
> +config AD7405
> + tristate "Analog Device AD7405 ADC Driver"
> + depends on IIO_BACKEND
> + help
> + Say yes here to build support for Analog Devices AD7405, ADUM7701,
> + ADUM7702, ADUM7703 analog to digital converters (ADC).
> +
> + To compile this driver as a module, choose M here: the module will
> be
> + called ad7405.
> +
> config AD7476
> tristate "Analog Devices AD7476 1-channel ADCs driver and other
> similar devices from AD and TI"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 07d4b832c42e..8115f30b7862 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_AD7291) += ad7291.o
> obj-$(CONFIG_AD7292) += ad7292.o
> obj-$(CONFIG_AD7298) += ad7298.o
> obj-$(CONFIG_AD7380) += ad7380.o
> +obj-$(CONFIG_AD7405) += ad7405.o
> obj-$(CONFIG_AD7476) += ad7476.o
> obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
> obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
> diff --git a/drivers/iio/adc/ad7405.c b/drivers/iio/adc/ad7405.c
> new file mode 100644
> index 000000000000..fb249c83b87b
> --- /dev/null
> +++ b/drivers/iio/adc/ad7405.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD7405 driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/util_macros.h>
> +
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +
> +static const unsigned int ad7405_dec_rates[] = {
> + 4096, 2048, 1024, 512, 256, 128, 64, 32,
> +};
> +
> +struct ad7405_chip_info {
> + const char *name;
> + struct iio_chan_spec channel;
> + const unsigned int full_scale_mv;
> +};
> +
> +struct ad7405_state {
> + struct iio_backend *back;
> + const struct ad7405_chip_info *info;
> + unsigned int ref_frequency;
> + unsigned int dec_rate;
> +};
> +
> +static int ad7405_set_dec_rate(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int dec_rate)
> +{
> + struct ad7405_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = iio_backend_oversampling_ratio_set(st->back, chan->scan_index,
> dec_rate);
> + if (ret)
> + return ret;
> +
> + st->dec_rate = dec_rate;
> +
> + return 0;
> +}
> +
> +static int ad7405_get_scale(struct ad7405_state *st, int *val, int *val2)
> +{
> + *val = st->info->full_scale_mv;
> + *val2 = st->info->channel.scan_type.realbits - 1;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> +}
> +
> +static int ad7405_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val,
> + int *val2, long info)
> +{
> + struct ad7405_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad7405_get_scale(st, val, val2);
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *val = st->dec_rate;
I'm typically picky with this but I really think that we need a lock to be used
here and in ad7405_set_dec_rate(). Alternatively we could add
iio_backend_oversampling_ratio_get() but I would likely avoid a new op just for
this.
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = DIV_ROUND_CLOSEST_ULL(st->ref_frequency, st-
> >dec_rate);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OFFSET:
> + *val = 2 << (st->info->channel.scan_type.realbits - 1);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad7405_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long info)
> +{
> + switch (info) {
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + return ad7405_set_dec_rate(indio_dev, chan, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad7405_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long info)
> +{
> + switch (info) {
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = ad7405_dec_rates;
> + *length = ARRAY_SIZE(ad7405_dec_rates);
> + *type = IIO_VAL_INT;
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info ad7405_iio_info = {
> + .read_raw = &ad7405_read_raw,
> + .write_raw = &ad7405_write_raw,
> + .read_avail = &ad7405_read_avail,
> +};
> +
> +#define AD7405_IIO_CHANNEL { \
> + .type = IIO_VOLTAGE, \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .info_mask_shared_by_all = IIO_CHAN_INFO_SAMP_FREQ | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + .info_mask_shared_by_all_available = \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + .indexed = 1, \
> + .channel = 0, \
> + .channel2 = 1, \
> + .differential = 1, \
> + .scan_index = 0, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 16, \
> + .storagebits = 16, \
> + }, \
> +}
> +
> +static const struct ad7405_chip_info ad7405_chip_info = {
> + .name = "AD7405",
> + .full_scale_mv = 320,
> + .channel = AD7405_IIO_CHANNEL,
> +};
> +
> +static const struct ad7405_chip_info adum7701_chip_info = {
> + .name = "ADUM7701",
> + .full_scale_mv = 320,
> + .channel = AD7405_IIO_CHANNEL,
> +};
> +
> +static const struct ad7405_chip_info adum7702_chip_info = {
> + .name = "ADUM7702",
> + .full_scale_mv = 64,
> + .channel = AD7405_IIO_CHANNEL,
> +};
> +
> +static const struct ad7405_chip_info adum7703_chip_info = {
> + .name = "ADUM7703",
> + .full_scale_mv = 320,
> + .channel = AD7405_IIO_CHANNEL,
> +};
> +
> +static const char * const ad7405_power_supplies[] = {
> + "vdd1", "vdd2",
> +};
> +
> +static int ad7405_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct iio_dev *indio_dev;
> + struct ad7405_state *st;
> + struct clk *clk;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> +
> + st->info = device_get_match_data(dev);
> + if (!st->info)
> + return dev_err_probe(dev, -EINVAL, "no chip info\n");
> +
> + ret = devm_regulator_bulk_get_enable(dev,
> ARRAY_SIZE(ad7405_power_supplies),
> + ad7405_power_supplies);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get and enable
> supplies");
> +
> + clk = devm_clk_get_enabled(dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + st->ref_frequency = clk_get_rate(clk);
> + if (!st->ref_frequency)
> + return -EINVAL;
> +
> + indio_dev->dev.parent = dev;
no need for the above
- Nuno Sá
Powered by blists - more mailing lists