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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a45c60fe9fff0f517032a7e9eb3881cf340a8c1e.camel@gmail.com>
Date: Sat, 18 Jan 2025 15:10:39 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, jic23@...nel.org, 
	robh@...nel.org, conor+dt@...nel.org, linux-iio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pwm@...r.kernel.org
Subject: Re: [PATCH v10 8/8] iio: adc: ad4851: add ad485x driver

On Fri, 2025-01-17 at 15:07 +0200, Antoniu Miclaus wrote:
> Add support for the AD485X a fully buffered, 8-channel simultaneous
> sampling, 16/20-bit, 1 MSPS data acquisition system (DAS) with
> differential, wide common-mode range inputs.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> changes in v10:
>  - use u64 for scale_tbl cast.
>  - use IIO_VAL_FRACTIONAL_LOG2
>  - add comment for not using bulk_read
>  - use _u and _b suffix for scan_type structures.
>  - add comment for not setting scan_type in macro definitions.
>  - fix some wrapping.
>  - rework vrefbuf_en and vrefio_en handling.
>  - add depends on PWM in Kconfig
>  - add unipolar/bipolar suffix where requested.
>  - rename REFSEL REFBUF macros.
>  - use indio_dev->channels[i].channel for iio_backend_chan_enable
>  - drop initialization for 'buf'
>  - use st->bipolar_ch[chan->channel], not chan->differential
>  - use u32 for softspan_val
>  - move info_mask_separate above as suggested.
>  - handle sign in parse channel function and use `u` as default in macro.
>  - check `reg` range.
>  - use fwnode_property_read_bool
>  - drop redundant `reg` parsing
>  - rework channel2 statement in channel macro and parse_channels.
>  - set ad4851_channels->has_ext_scan_type = 1
>  drivers/iio/adc/Kconfig  |   14 +
>  drivers/iio/adc/Makefile |    1 +
>  drivers/iio/adc/ad4851.c | 1293 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1308 insertions(+)
>  create mode 100644 drivers/iio/adc/ad4851.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 849c90203071..afd83fddda76 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -61,6 +61,20 @@ config AD4695
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ad4695.
>  
> +config AD4851
> +	tristate "Analog Device AD4851 DAS Driver"
> +	depends on SPI
> +	depends on PWM
> +	select REGMAP_SPI
> +	select IIO_BACKEND
> +	help
> +	  Say yes here to build support for Analog Devices AD4851, AD4852,
> +	  AD4853, AD4854, AD4855, AD4856, AD4857, AD4858, AD4858I high speed
> +	  data acquisition system (DAS).
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad4851.
> +
>  config AD7091R
>  	tristate
>  
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ee19afba62b7..e4d8ba12f841 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
>  obj-$(CONFIG_AD4000) += ad4000.o
>  obj-$(CONFIG_AD4130) += ad4130.o
>  obj-$(CONFIG_AD4695) += ad4695.o
> +obj-$(CONFIG_AD4851) += ad4851.o
>  obj-$(CONFIG_AD7091R) += ad7091r-base.o
>  obj-$(CONFIG_AD7091R5) += ad7091r5.o
>  obj-$(CONFIG_AD7091R8) += ad7091r8.o
> diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c
> new file mode 100644
> index 000000000000..a39d863bf62d
> --- /dev/null
> +++ b/drivers/iio/adc/ad4851.c
> @@ -0,0 +1,1293 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD4851 DAS driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +

...

> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +
> +#define AD4851_REG_INTERFACE_CONFIG_A	0x00
> +#define AD4851_REG_INTERFACE_CONFIG_B	0x01
> +#define AD4851_REG_PRODUCT_ID_L		0x04
> +#define AD4851_REG_PRODUCT_ID_H		0x05
> +#define AD4851_REG_DEVICE_CTRL		0x25
> +#define AD4851_REG_PACKET		0x26
> +#define AD4851_REG_OVERSAMPLE		0x27
> +
> +#define AD4851_REG_CH_CONFIG_BASE	0x2A
> +#define AD4851_REG_CHX_SOFTSPAN(ch)	((0x12 * (ch)) +
> AD4851_REG_CH_CONFIG_BASE)
> +#define AD4851_REG_CHX_OFFSET(ch)	(AD4851_REG_CHX_SOFTSPAN(ch) + 0x01)
> +#define AD4851_REG_CHX_OFFSET_LSB(ch)	AD4851_REG_CHX_OFFSET(ch)
> +#define AD4851_REG_CHX_OFFSET_MID(ch)	(AD4851_REG_CHX_OFFSET_LSB(ch) + 0x01)
> +#define AD4851_REG_CHX_OFFSET_MSB(ch)	(AD4851_REG_CHX_OFFSET_MID(ch) + 0x01)
> +#define AD4851_REG_CHX_GAIN(ch)		(AD4851_REG_CHX_OFFSET(ch) + 0x03)
> +#define AD4851_REG_CHX_GAIN_LSB(ch)	AD4851_REG_CHX_GAIN(ch)
> +#define AD4851_REG_CHX_GAIN_MSB(ch)	(AD4851_REG_CHX_GAIN(ch) + 0x01)
> +#define AD4851_REG_CHX_PHASE(ch)	(AD4851_REG_CHX_GAIN(ch) + 0x02)
> +#define AD4851_REG_CHX_PHASE_LSB(ch)	AD4851_REG_CHX_PHASE(ch)
> +#define AD4851_REG_CHX_PHASE_MSB(ch)	(AD4851_REG_CHX_PHASE_LSB(ch) + 0x01)
> +
> +#define AD4851_REG_TESTPAT_0(c)		(0x38 + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_1(c)		(0x39 + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_2(c)		(0x3A + (c) * 0x12)
> +#define AD4851_REG_TESTPAT_3(c)		(0x3B + (c) * 0x12)
> +
> +#define AD4851_SW_RESET			(BIT(7) | BIT(0))
> +#define AD4851_SDO_ENABLE		BIT(4)
> +#define AD4851_SINGLE_INSTRUCTION	BIT(7)
> +#define AD4851_REFBUF			BIT(2)
> +#define AD4851_REFSEL			BIT(1)
> +#define AD4851_ECHO_CLOCK_MODE		BIT(0)
> +
> +#define AD4851_PACKET_FORMAT_0		0
> +#define AD4851_PACKET_FORMAT_1		1
> +#define AD4851_PACKET_FORMAT_MASK	GENMASK(1, 0)
> +
> +#define AD4851_OS_EN_MSK		BIT(7)
> +#define AD4851_OS_RATIO_MSK		GENMASK(3, 0)
> +
> +#define AD4851_TEST_PAT			BIT(2)
> +
> +#define AD4858_PACKET_SIZE_20		0
> +#define AD4858_PACKET_SIZE_24		1
> +#define AD4858_PACKET_SIZE_32		2
> +
> +#define AD4857_PACKET_SIZE_16		0
> +#define AD4857_PACKET_SIZE_24		1
> +
> +#define AD4851_TESTPAT_0_DEFAULT	0x2A
> +#define AD4851_TESTPAT_1_DEFAULT	0x3C
> +#define AD4851_TESTPAT_2_DEFAULT	0xCE
> +#define AD4851_TESTPAT_3_DEFAULT(c)	(0x0A + (0x10 * (c)))
> +
> +#define AD4851_SOFTSPAN_0V_2V5		0
> +#define AD4851_SOFTSPAN_N2V5_2V5	1
> +#define AD4851_SOFTSPAN_0V_5V		2
> +#define AD4851_SOFTSPAN_N5V_5V		3
> +#define AD4851_SOFTSPAN_0V_6V25		4
> +#define AD4851_SOFTSPAN_N6V25_6V25	5
> +#define AD4851_SOFTSPAN_0V_10V		6
> +#define AD4851_SOFTSPAN_N10V_10V	7
> +#define AD4851_SOFTSPAN_0V_12V5		8
> +#define AD4851_SOFTSPAN_N12V5_12V5	9
> +#define AD4851_SOFTSPAN_0V_20V		10
> +#define AD4851_SOFTSPAN_N20V_20V	11
> +#define AD4851_SOFTSPAN_0V_25V		12
> +#define AD4851_SOFTSPAN_N25V_25V	13
> +#define AD4851_SOFTSPAN_0V_40V		14
> +#define AD4851_SOFTSPAN_N40V_40V	15
> +
> +#define AD4851_MAX_LANES		8
> +#define AD4851_MAX_IODELAY		32
> +
> +#define AD4851_T_CNVH_NS		40
> +#define AD4851_T_CNVH_NS_MARGIN		10
> +
> +#define AD4841_MAX_SCALE_AVAIL		8
> +
> +#define AD4851_MAX_CH_NR		8
> +#define AD4851_CH_START			0
> +
> +struct ad4851_scale {
> +	unsigned int scale_val;
> +	u8 reg_val;
> +};
> +
> +static const struct ad4851_scale ad4851_scale_table_unipolar[] = {
> +	{ 2500, 0x0 },
> +	{ 5000, 0x2 },
> +	{ 6250, 0x4 },
> +	{ 10000, 0x6 },
> +	{ 12500, 0x8 },
> +	{ 20000, 0xA },
> +	{ 25000, 0xC },
> +	{ 40000, 0xE },
> +};
> +
> +static const struct ad4851_scale ad4851_scale_table_bipolar[] = {
> +	{ 5000, 0x1 },
> +	{ 10000, 0x3 },
> +	{ 12500, 0x5 },
> +	{ 20000, 0x7 },
> +	{ 25000, 0x9 },
> +	{ 40000, 0xB },
> +	{ 50000, 0xD },
> +	{ 80000, 0xF },
> +};
> +
> +static const unsigned int ad4851_scale_avail_unipolar[] = {
> +	2500,
> +	5000,
> +	6250,
> +	10000,
> +	12500,
> +	20000,
> +	25000,
> +	40000,
> +};
> +
> +static const unsigned int ad4851_scale_avail_bipolar[] = {
> +	5000,
> +	10000,
> +	12500,
> +	20000,
> +	25000,
> +	40000,
> +	50000,
> +	80000,
> +};
> +
> +struct ad4851_chip_info {
> +	const char *name;
> +	unsigned int product_id;
> +	int num_scales;
> +	unsigned long max_sample_rate_hz;
> +	unsigned int resolution;
> +	int (*parse_channels)(struct iio_dev *indio_dev);
> +};
> +
> +enum {
> +	AD4851_SCAN_TYPE_NORMAL,
> +	AD4851_SCAN_TYPE_RESOLUTION_BOOST,
> +};
> +
> +struct ad4851_state {
> +	struct spi_device *spi;
> +	struct pwm_device *cnv;
> +	struct iio_backend *back;
> +	/*
> +	 * Synchronize access to members the of driver state, and ensure
> +	 * atomicity of consecutive regmap operations.
> +	 */
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	const struct ad4851_chip_info *info;
> +	struct gpio_desc *pd_gpio;
> +	bool resolution_boost_enabled;
> +	unsigned long cnv_trigger_rate_hz;
> +	unsigned int osr;
> +	bool vrefbuf_en;
> +	bool vrefio_en;
> +	bool bipolar_ch[AD4851_MAX_CH_NR];
> +	unsigned int scales_unipolar[AD4841_MAX_SCALE_AVAIL][2];
> +	unsigned int scales_bipolar[AD4841_MAX_SCALE_AVAIL][2];
> +};
> +
> +static int ad4851_reg_access(struct iio_dev *indio_dev,
> +			     unsigned int reg,
> +			     unsigned int writeval,
> +			     unsigned int *readval)
> +{
> +	struct ad4851_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +

no need for the above lock. I guess you're not disabling regmap internal lock...

> +	if (readval)
> +		return regmap_read(st->regmap, reg, readval);
> +
> +	return regmap_write(st->regmap, reg, writeval);
> +}
> +

...

> 
> +static int ad4851_set_oversampling_ratio(struct iio_dev *indio_dev,
> +					 const struct iio_chan_spec *chan,
> +					 unsigned int osr)
> +{
> +	struct ad4851_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	if (osr == 1) {
> +		ret = regmap_clear_bits(st->regmap, AD4851_REG_OVERSAMPLE,
> +					AD4851_OS_EN_MSK);
> +		if (ret)
> +			return ret;
> +	} else {
> +		val = ad4851_osr_to_regval(osr);
> +		if (val < 0)
> +			return -EINVAL;
> +
> +		ret = regmap_update_bits(st->regmap, AD4851_REG_OVERSAMPLE,
> +					 AD4851_OS_EN_MSK |
> +					 AD4851_OS_RATIO_MSK,
> +					 FIELD_PREP(AD4851_OS_EN_MSK, 1) |
> +					 FIELD_PREP(AD4851_OS_RATIO_MSK, val));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = iio_backend_oversampling_ratio_set(st->back, osr);
> +	if (ret)
> +		return ret;
> +
> +	switch (st->info->resolution) {
> +	case 20:
> +		switch (osr) {
> +		case 0:
> +			return -EINVAL;
> +		case 1:
> +			val = 20;
> +			break;
> +		default:
> +			val = 24;
> +			break;
> +		}
> +		break;
> +	case 16:
> +		val = 16;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = iio_backend_data_size_set(st->back, val);
> +	if (ret)
> +		return ret;
> +
> +	if (osr == 1 || st->info->resolution == 16) {
> +		ret = regmap_clear_bits(st->regmap, AD4851_REG_PACKET,
> +					AD4851_PACKET_FORMAT_MASK);
> +		if (ret)
> +			return ret;
> +
> +		st->resolution_boost_enabled = false;
> +	} else {
> +		ret = regmap_update_bits(st->regmap, AD4851_REG_PACKET,
> +					 AD4851_PACKET_FORMAT_MASK,
> +					 FIELD_PREP(AD4851_PACKET_FORMAT_MASK,
> 1));
> +		if (ret)
> +			return ret;
> +
> +		st->resolution_boost_enabled = true;
> +	}
> +
> +	if (st->osr != osr) {
> +		ret = ad4851_scale_fill(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		st->osr = osr;
> +	}

nit: I would instead do

if (st->osr == osr)
    return 0;

...

> +
> +	return 0;
> +}
> +
> +static int ad4851_get_oversampling_ratio(struct ad4851_state *st, unsigned int
> *val)
> +{
> +	unsigned int osr;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = regmap_read(st->regmap, AD4851_REG_OVERSAMPLE, &osr);
> +	if (ret)
> +		return ret;
> +
> +	if (!FIELD_GET(AD4851_OS_EN_MSK, osr))
> +		*val = 1;
> +	else
> +		*val = ad4851_oversampling_ratios[FIELD_GET(AD4851_OS_RATIO_MSK,
> osr) + 1];
> +
> +	st->osr = *val;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static void ad4851_pwm_disable(void *data)
> +{
> +	pwm_disable(data);
> +}
> +
> +static int ad4851_setup(struct ad4851_state *st)
> +{
> +	unsigned int product_id;
> +	int ret;
> +
> +	if (st->pd_gpio) {
> +		/* To initiate a global reset, bring the PD pin high twice */
> +		gpiod_set_value(st->pd_gpio, 1);
> +		fsleep(1);
> +		gpiod_set_value(st->pd_gpio, 0);
> +		fsleep(1);
> +		gpiod_set_value(st->pd_gpio, 1);
> +		fsleep(1);
> +		gpiod_set_value(st->pd_gpio, 0);
> +		fsleep(1000);
> +	} else {
> +		ret = regmap_set_bits(st->regmap, AD4851_REG_INTERFACE_CONFIG_A,
> +				      AD4851_SW_RESET);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (st->vrefbuf_en) {
> +		ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> +				      AD4851_REFBUF);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (st->vrefio_en) {
> +		ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> +				      AD4851_REFSEL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, AD4851_REG_INTERFACE_CONFIG_B,
> +			   AD4851_SINGLE_INSTRUCTION);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4851_REG_INTERFACE_CONFIG_A,
> +			   AD4851_SDO_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4851_REG_PRODUCT_ID_L, &product_id);
> +	if (ret)
> +		return ret;
> +
> +	if (product_id != st->info->product_id)
> +		dev_info(&st->spi->dev, "Unknown product ID: 0x%02X\n",
> +			 product_id);
> +
> +	ret = regmap_set_bits(st->regmap, AD4851_REG_DEVICE_CTRL,
> +			      AD4851_ECHO_CLOCK_MODE);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, AD4851_REG_PACKET, 0);
> +}
> +
> +static int ad4851_find_opt(bool *field, u32 size, u32 *ret_start)
> +{
> +	unsigned int i, cnt = 0, max_cnt = 0, max_start = 0;
> +	int start;
> +
> +	for (i = 0, start = -1; i < size; i++) {
> +		if (field[i] == 0) {
> +			if (start == -1)
> +				start = i;
> +			cnt++;
> +		} else {
> +			if (cnt > max_cnt) {
> +				max_cnt = cnt;
> +				max_start = start;
> +			}
> +			start = -1;
> +			cnt = 0;
> +		}
> +	}
> +	/*
> +	 * Find the longest consecutive sequence of false values from field
> +	 * and return starting index.
> +	 */
> +	if (cnt > max_cnt) {
> +		max_cnt = cnt;
> +		max_start = start;
> +	}
> +
> +	if (!max_cnt)
> +		return -ENOENT;
> +
> +	*ret_start = max_start;
> +
> +	return max_cnt;
> +}
> 

Your status seems to be a bitmap now, can you take a look in here and see if you can
do something similar?

https://elixir.bootlin.com/linux/v6.12.6/source/drivers/iio/adc/ad9467.c#L633

Anyways, look at the bitmap API. It might have helpers to make the above function
simpler.

> +
> +static int ad4851_calibrate(struct iio_dev *indio_dev)
> +{
> +	struct ad4851_state *st = iio_priv(indio_dev);
> +	unsigned int opt_delay, num_lanes, delay, i, s, c;
> +	enum iio_backend_interface_type interface_type;
> +	DECLARE_BITMAP(pn_status, AD4851_MAX_LANES * AD4851_MAX_IODELAY);
> +	bool status;
> +	int ret;
> +
> +	ret = iio_backend_interface_type_get(st->back, &interface_type);
> +	if (ret)
> +		return ret;
> +
> +	switch (interface_type) {
> +	case IIO_BACKEND_INTERFACE_SERIAL_CMOS:
> +		num_lanes = indio_dev->num_channels;
> +		break;
> +	case IIO_BACKEND_INTERFACE_SERIAL_LVDS:
> +		num_lanes = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (st->info->resolution == 16) {
> +		ret = iio_backend_data_size_set(st->back, 24);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4851_REG_PACKET,
> +				   AD4851_TEST_PAT | AD4857_PACKET_SIZE_24);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = iio_backend_data_size_set(st->back, 32);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4851_REG_PACKET,
> +				   AD4851_TEST_PAT | AD4858_PACKET_SIZE_32);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_0(i),
> +				   AD4851_TESTPAT_0_DEFAULT);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_1(i),
> +				   AD4851_TESTPAT_1_DEFAULT);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_2(i),
> +				   AD4851_TESTPAT_2_DEFAULT);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4851_REG_TESTPAT_3(i),
> +				   AD4851_TESTPAT_3_DEFAULT(i));
> +		if (ret)
> +			return ret;
> +
> +		ret = iio_backend_chan_enable(st->back,
> +					      indio_dev->channels[i].channel);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < num_lanes; i++) {
> +		for (delay = 0; delay < AD4851_MAX_IODELAY; delay++) {
> +			ret = iio_backend_iodelay_set(st->back, i, delay);
> +			if (ret)
> +				return ret;
> +
> +			ret = iio_backend_chan_status(st->back, i, &status);
> +			if (ret)
> +				return ret;
> +
> +			if (status)
> +				set_bit(i * AD4851_MAX_IODELAY + delay,
> pn_status);
> +			else
> +				clear_bit(i * AD4851_MAX_IODELAY + delay,
> pn_status);
> +		}

you can use the lighter forms __set_bit()/__clear_bit(). But going one step further,
maybe __assign_bit()?

> +	}
> +
> +	for (i = 0; i < num_lanes; i++) {
> +		status = test_bit(i * AD4851_MAX_IODELAY, pn_status);
> +		c = ad4851_find_opt(&status, AD4851_MAX_IODELAY, &s);

status is just a bool and it seems your iterating on it? Am I missing something?

> +		if (c < 0)
> +			return c;
> +
> +		opt_delay = s + c / 2;
> +		ret = iio_backend_iodelay_set(st->back, i, opt_delay);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		ret = iio_backend_chan_disable(st->back, i);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = iio_backend_data_size_set(st->back, 20);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, AD4851_REG_PACKET, 0);
> +}
> +
> 

...

> +static int ad4851_read_raw(struct iio_dev *indio_dev,
> +			   const struct iio_chan_spec *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad4851_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->cnv_trigger_rate_hz / st->osr;
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		return ad4851_get_calibscale(st, chan->channel, val, val2);
> +	case IIO_CHAN_INFO_SCALE:
> +		return ad4851_get_scale(indio_dev, chan, val, val2);

Maybe this was discussed already and I missed it but I'm a bit puzzled. Don't we
still need OFFSET for differential channels? How do you express negative voltages?

- Nuno Sá


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ