From: Alisa-Dariana Roman <[email protected]>
Hello again,
Thank you Jonathan for the feedback! Here is the updated series of
patches for the ad7192 driver.
Quick reminder that the patches need to be applied in order.
v2 -> v3
- move comment line above
- correct FIELD_PREP to FIELD_GET where needed
- remove unnecessary !!
- "rms" -> "RMS"
Link: https://lore.kernel.org/all/[email protected]/
v1 -> v2
- replace old macros with FIELD_PREP() in commit "Use bitfield
access macros"
- update the other commits accordingly
Link: https://lore.kernel.org/all/[email protected]/
Kind regards,
Alisa-Dariana Roman (3):
iio: adc: ad7192: Use bitfield access macros
iio: adc: ad7192: Improve f_order computation
iio: adc: ad7192: Add fast settling support
.../ABI/testing/sysfs-bus-iio-adc-ad7192 | 18 ++
drivers/iio/adc/ad7192.c | 241 ++++++++++++++----
2 files changed, 206 insertions(+), 53 deletions(-)
--
2.34.1
From: Alisa-Dariana Roman <[email protected]>
Include bitfield.h and update driver to use bitfield access macros
GENMASK, FIELD_PREP and FIELD_GET.
Remove old macros in favor of using FIELD_PREP and masks.
Change %d to %ld to match the type of FIELD_GET().
Signed-off-by: Alisa-Dariana Roman <[email protected]>
---
drivers/iio/adc/ad7192.c | 73 ++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 69d1103b9508..d693f2ce8a20 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -6,6 +6,7 @@
*/
#include <linux/interrupt.h>
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/device.h>
#include <linux/kernel.h>
@@ -43,7 +44,7 @@
#define AD7192_COMM_WEN BIT(7) /* Write Enable */
#define AD7192_COMM_WRITE 0 /* Write Operation */
#define AD7192_COMM_READ BIT(6) /* Read Operation */
-#define AD7192_COMM_ADDR(x) (((x) & 0x7) << 3) /* Register Address */
+#define AD7192_COMM_ADDR_MASK GENMASK(5, 3) /* Register Address Mask */
#define AD7192_COMM_CREAD BIT(2) /* Continuous Read of Data Register */
/* Status Register Bit Designations (AD7192_REG_STAT) */
@@ -56,17 +57,16 @@
#define AD7192_STAT_CH1 BIT(0) /* Channel 1 */
/* Mode Register Bit Designations (AD7192_REG_MODE) */
-#define AD7192_MODE_SEL(x) (((x) & 0x7) << 21) /* Operation Mode Select */
-#define AD7192_MODE_SEL_MASK (0x7 << 21) /* Operation Mode Select Mask */
-#define AD7192_MODE_STA(x) (((x) & 0x1) << 20) /* Status Register transmission */
+#define AD7192_MODE_SEL_MASK GENMASK(23, 21) /* Operation Mode Select Mask */
#define AD7192_MODE_STA_MASK BIT(20) /* Status Register transmission Mask */
-#define AD7192_MODE_CLKSRC(x) (((x) & 0x3) << 18) /* Clock Source Select */
+#define AD7192_MODE_CLKSRC_MASK GENMASK(19, 18) /* Clock Source Select Mask */
#define AD7192_MODE_SINC3 BIT(15) /* SINC3 Filter Select */
#define AD7192_MODE_ENPAR BIT(13) /* Parity Enable */
#define AD7192_MODE_CLKDIV BIT(12) /* Clock divide by 2 (AD7190/2 only)*/
#define AD7192_MODE_SCYCLE BIT(11) /* Single cycle conversion */
#define AD7192_MODE_REJ60 BIT(10) /* 50/60Hz notch filter */
-#define AD7192_MODE_RATE(x) ((x) & 0x3FF) /* Filter Update Rate Select */
+ /* Filter Update Rate Select Mask */
+#define AD7192_MODE_RATE_MASK GENMASK(9, 0)
/* Mode Register: AD7192_MODE_SEL options */
#define AD7192_MODE_CONT 0 /* Continuous Conversion Mode */
@@ -92,13 +92,12 @@
#define AD7192_CONF_CHOP BIT(23) /* CHOP enable */
#define AD7192_CONF_ACX BIT(22) /* AC excitation enable(AD7195 only) */
#define AD7192_CONF_REFSEL BIT(20) /* REFIN1/REFIN2 Reference Select */
-#define AD7192_CONF_CHAN(x) ((x) << 8) /* Channel select */
-#define AD7192_CONF_CHAN_MASK (0x7FF << 8) /* Channel select mask */
+#define AD7192_CONF_CHAN_MASK GENMASK(18, 8) /* Channel select mask */
#define AD7192_CONF_BURN BIT(7) /* Burnout current enable */
#define AD7192_CONF_REFDET BIT(6) /* Reference detect enable */
#define AD7192_CONF_BUF BIT(4) /* Buffered Mode Enable */
#define AD7192_CONF_UNIPOLAR BIT(3) /* Unipolar/Bipolar Enable */
-#define AD7192_CONF_GAIN(x) ((x) & 0x7) /* Gain Select */
+#define AD7192_CONF_GAIN_MASK GENMASK(2, 0) /* Gain Select */
#define AD7192_CH_AIN1P_AIN2M BIT(0) /* AIN1(+) - AIN2(-) */
#define AD7192_CH_AIN3P_AIN4M BIT(1) /* AIN3(+) - AIN4(-) */
@@ -130,7 +129,7 @@
#define CHIPID_AD7192 0x0
#define CHIPID_AD7193 0x2
#define CHIPID_AD7195 0x6
-#define AD7192_ID_MASK 0x0F
+#define AD7192_ID_MASK GENMASK(3, 0)
/* GPOCON Register Bit Designations (AD7192_REG_GPOCON) */
#define AD7192_GPOCON_BPDSW BIT(6) /* Bridge power-down switch enable */
@@ -272,7 +271,7 @@ static int ad7192_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
struct ad7192_state *st = ad_sigma_delta_to_ad7192(sd);
st->conf &= ~AD7192_CONF_CHAN_MASK;
- st->conf |= AD7192_CONF_CHAN(channel);
+ st->conf |= FIELD_PREP(AD7192_CONF_CHAN_MASK, channel);
return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
}
@@ -283,7 +282,7 @@ static int ad7192_set_mode(struct ad_sigma_delta *sd,
struct ad7192_state *st = ad_sigma_delta_to_ad7192(sd);
st->mode &= ~AD7192_MODE_SEL_MASK;
- st->mode |= AD7192_MODE_SEL(mode);
+ st->mode |= FIELD_PREP(AD7192_MODE_SEL_MASK, mode);
return ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
}
@@ -295,7 +294,7 @@ static int ad7192_append_status(struct ad_sigma_delta *sd, bool append)
int ret;
mode &= ~AD7192_MODE_STA_MASK;
- mode |= AD7192_MODE_STA(append);
+ mode |= FIELD_PREP(AD7192_MODE_STA_MASK, append);
ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, mode);
if (ret < 0)
@@ -399,17 +398,17 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
if (ret)
return ret;
- id &= AD7192_ID_MASK;
+ id = FIELD_GET(AD7192_ID_MASK, id);
if (id != st->chip_info->chip_id)
dev_warn(&st->sd.spi->dev, "device ID query failed (0x%X != 0x%X)\n",
id, st->chip_info->chip_id);
- st->mode = AD7192_MODE_SEL(AD7192_MODE_IDLE) |
- AD7192_MODE_CLKSRC(st->clock_sel) |
- AD7192_MODE_RATE(480);
+ st->mode = FIELD_PREP(AD7192_MODE_SEL_MASK, AD7192_MODE_IDLE) |
+ FIELD_PREP(AD7192_MODE_CLKSRC_MASK, st->clock_sel) |
+ FIELD_PREP(AD7192_MODE_RATE_MASK, 480);
- st->conf = AD7192_CONF_GAIN(0);
+ st->conf = FIELD_PREP(AD7192_CONF_GAIN_MASK, 0);
rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
if (rej60_en)
@@ -455,7 +454,7 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
scale_uv = ((u64)st->int_vref_mv * 100000000)
>> (indio_dev->channels[0].scan_type.realbits -
- ((st->conf & AD7192_CONF_UNIPOLAR) ? 0 : 1));
+ !FIELD_GET(AD7192_CONF_UNIPOLAR, st->conf));
scale_uv >>= i;
st->scale_avail[i][1] = do_div(scale_uv, 100000000) * 10;
@@ -472,7 +471,7 @@ static ssize_t ad7192_show_ac_excitation(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7192_state *st = iio_priv(indio_dev);
- return sysfs_emit(buf, "%d\n", !!(st->conf & AD7192_CONF_ACX));
+ return sysfs_emit(buf, "%ld\n", FIELD_GET(AD7192_CONF_ACX, st->conf));
}
static ssize_t ad7192_show_bridge_switch(struct device *dev,
@@ -482,7 +481,8 @@ static ssize_t ad7192_show_bridge_switch(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7192_state *st = iio_priv(indio_dev);
- return sysfs_emit(buf, "%d\n", !!(st->gpocon & AD7192_GPOCON_BPDSW));
+ return sysfs_emit(buf, "%ld\n",
+ FIELD_GET(AD7192_GPOCON_BPDSW, st->gpocon));
}
static ssize_t ad7192_set(struct device *dev,
@@ -537,14 +537,14 @@ static void ad7192_get_available_filter_freq(struct ad7192_state *st,
/* Formulas for filter at page 25 of the datasheet */
fadc = DIV_ROUND_CLOSEST(st->fclk,
- AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
+ AD7192_SYNC4_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
fadc = DIV_ROUND_CLOSEST(st->fclk,
- AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode));
+ AD7192_SYNC3_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
- fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
+ fadc = DIV_ROUND_CLOSEST(st->fclk, FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
}
@@ -665,11 +665,11 @@ static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
unsigned int fadc;
fadc = DIV_ROUND_CLOSEST(st->fclk,
- st->f_order * AD7192_MODE_RATE(st->mode));
+ st->f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
- if (st->conf & AD7192_CONF_CHOP)
+ if (FIELD_GET(AD7192_CONF_CHOP, st->conf))
return DIV_ROUND_CLOSEST(fadc * 240, 1024);
- if (st->mode & AD7192_MODE_SINC3)
+ if (FIELD_GET(AD7192_MODE_SINC3, st->mode))
return DIV_ROUND_CLOSEST(fadc * 272, 1024);
else
return DIV_ROUND_CLOSEST(fadc * 230, 1024);
@@ -682,7 +682,8 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
long m)
{
struct ad7192_state *st = iio_priv(indio_dev);
- bool unipolar = !!(st->conf & AD7192_CONF_UNIPOLAR);
+ bool unipolar = !!FIELD_GET(AD7192_CONF_UNIPOLAR, st->conf);
+ u8 gain = FIELD_GET(AD7192_CONF_GAIN_MASK, st->conf);
switch (m) {
case IIO_CHAN_INFO_RAW:
@@ -691,8 +692,8 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
switch (chan->type) {
case IIO_VOLTAGE:
mutex_lock(&st->lock);
- *val = st->scale_avail[AD7192_CONF_GAIN(st->conf)][0];
- *val2 = st->scale_avail[AD7192_CONF_GAIN(st->conf)][1];
+ *val = st->scale_avail[gain][0];
+ *val2 = st->scale_avail[gain][1];
mutex_unlock(&st->lock);
return IIO_VAL_INT_PLUS_NANO;
case IIO_TEMP:
@@ -713,7 +714,7 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SAMP_FREQ:
*val = st->fclk /
- (st->f_order * 1024 * AD7192_MODE_RATE(st->mode));
+ (st->f_order * 1024 * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
return IIO_VAL_INT;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
*val = ad7192_get_3db_filter_freq(st);
@@ -746,8 +747,8 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
if (val2 == st->scale_avail[i][1]) {
ret = 0;
tmp = st->conf;
- st->conf &= ~AD7192_CONF_GAIN(-1);
- st->conf |= AD7192_CONF_GAIN(i);
+ st->conf &= ~AD7192_CONF_GAIN_MASK;
+ st->conf |= FIELD_PREP(AD7192_CONF_GAIN_MASK, i);
if (tmp == st->conf)
break;
ad_sd_write_reg(&st->sd, AD7192_REG_CONF,
@@ -769,8 +770,8 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
break;
}
- st->mode &= ~AD7192_MODE_RATE(-1);
- st->mode |= AD7192_MODE_RATE(div);
+ st->mode &= ~AD7192_MODE_RATE_MASK;
+ st->mode |= FIELD_PREP(AD7192_MODE_RATE_MASK, div);
ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
break;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
@@ -830,7 +831,7 @@ static int ad7192_update_scan_mode(struct iio_dev *indio_dev, const unsigned lon
conf &= ~AD7192_CONF_CHAN_MASK;
for_each_set_bit(i, scan_mask, 8)
- conf |= AD7192_CONF_CHAN(i);
+ conf |= FIELD_PREP(AD7192_CONF_CHAN_MASK, i);
ret = ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, conf);
if (ret < 0)
--
2.34.1
From: Alisa-Dariana Roman <[email protected]>
Instead of using the f_order member of ad7192_state, a function that
computes the f_order coefficient makes more sense. This coefficient is a
function of the sinc filter and chop filter states.
Remove f_order member of ad7192_state structure. Instead use
ad7192_compute_f_order function to compute the f_order coefficient
according to the sinc filter and chop filter states passed as
parameters.
Add ad7192_get_f_order function that returns the current f_order
coefficient of the device.
Add ad7192_compute_f_adc function that computes the f_adc value
according to the sinc filter and chop filter states passed as
parameters.
Add ad7192_get_f_adc function that returns the current f_adc value of
the device.
Signed-off-by: Alisa-Dariana Roman <[email protected]>
---
drivers/iio/adc/ad7192.c | 62 +++++++++++++++++++++++++++++-----------
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index d693f2ce8a20..0f9d33002d35 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -179,7 +179,6 @@ struct ad7192_state {
struct clk *mclk;
u16 int_vref_mv;
u32 fclk;
- u32 f_order;
u32 mode;
u32 conf;
u32 scale_avail[8][2];
@@ -419,7 +418,6 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
st->conf |= AD7192_CONF_REFSEL;
st->conf &= ~AD7192_CONF_CHOP;
- st->f_order = AD7192_NO_SYNC_FILTER;
buf_en = of_property_read_bool(np, "adi,buffer-enable");
if (buf_en)
@@ -530,22 +528,60 @@ static ssize_t ad7192_set(struct device *dev,
return ret ? ret : len;
}
+static int ad7192_compute_f_order(bool sinc3_en, bool chop_en)
+{
+ if (!chop_en)
+ return 1;
+
+ if (sinc3_en)
+ return AD7192_SYNC3_FILTER;
+
+ return AD7192_SYNC4_FILTER;
+}
+
+static int ad7192_get_f_order(struct ad7192_state *st)
+{
+ bool sinc3_en, chop_en;
+
+ sinc3_en = FIELD_GET(AD7192_MODE_SINC3, st->mode);
+ chop_en = FIELD_GET(AD7192_CONF_CHOP, st->conf);
+
+ return ad7192_compute_f_order(sinc3_en, chop_en);
+}
+
+static int ad7192_compute_f_adc(struct ad7192_state *st, bool sinc3_en,
+ bool chop_en)
+{
+ unsigned int f_order = ad7192_compute_f_order(sinc3_en, chop_en);
+
+ return DIV_ROUND_CLOSEST(st->fclk,
+ f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+}
+
+static int ad7192_get_f_adc(struct ad7192_state *st)
+{
+ unsigned int f_order = ad7192_get_f_order(st);
+
+ return DIV_ROUND_CLOSEST(st->fclk,
+ f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+}
+
static void ad7192_get_available_filter_freq(struct ad7192_state *st,
int *freq)
{
unsigned int fadc;
/* Formulas for filter at page 25 of the datasheet */
- fadc = DIV_ROUND_CLOSEST(st->fclk,
- AD7192_SYNC4_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+ fadc = ad7192_compute_f_adc(st, false, true);
freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
- fadc = DIV_ROUND_CLOSEST(st->fclk,
- AD7192_SYNC3_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+ fadc = ad7192_compute_f_adc(st, true, true);
freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
- fadc = DIV_ROUND_CLOSEST(st->fclk, FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+ fadc = ad7192_compute_f_adc(st, false, false);
freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
+
+ fadc = ad7192_compute_f_adc(st, true, false);
freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
}
@@ -628,25 +664,21 @@ static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
switch (idx) {
case 0:
- st->f_order = AD7192_SYNC4_FILTER;
st->mode &= ~AD7192_MODE_SINC3;
st->conf |= AD7192_CONF_CHOP;
break;
case 1:
- st->f_order = AD7192_SYNC3_FILTER;
st->mode |= AD7192_MODE_SINC3;
st->conf |= AD7192_CONF_CHOP;
break;
case 2:
- st->f_order = AD7192_NO_SYNC_FILTER;
st->mode &= ~AD7192_MODE_SINC3;
st->conf &= ~AD7192_CONF_CHOP;
break;
case 3:
- st->f_order = AD7192_NO_SYNC_FILTER;
st->mode |= AD7192_MODE_SINC3;
st->conf &= ~AD7192_CONF_CHOP;
@@ -664,8 +696,7 @@ static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
{
unsigned int fadc;
- fadc = DIV_ROUND_CLOSEST(st->fclk,
- st->f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+ fadc = ad7192_get_f_adc(st);
if (FIELD_GET(AD7192_CONF_CHOP, st->conf))
return DIV_ROUND_CLOSEST(fadc * 240, 1024);
@@ -713,8 +744,7 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
*val -= 273 * ad7192_get_temp_scale(unipolar);
return IIO_VAL_INT;
case IIO_CHAN_INFO_SAMP_FREQ:
- *val = st->fclk /
- (st->f_order * 1024 * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
+ *val = DIV_ROUND_CLOSEST(ad7192_get_f_adc(st), 1024);
return IIO_VAL_INT;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
*val = ad7192_get_3db_filter_freq(st);
@@ -764,7 +794,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
break;
}
- div = st->fclk / (val * st->f_order * 1024);
+ div = st->fclk / (val * ad7192_get_f_order(st) * 1024);
if (div < 1 || div > 1023) {
ret = -EINVAL;
break;
--
2.34.1
From: Alisa-Dariana Roman <[email protected]>
Add fast settling mode support for AD7193.
Add fast_settling_average_factor attribute. Expose to user the current
value of the fast settling average factor. User can change the value by
writing a new one.
Add fast_settling_average_factor_available attribute. Expose to user
possible values for the fast settling average factor.
Signed-off-by: Alisa-Dariana Roman <[email protected]>
---
.../ABI/testing/sysfs-bus-iio-adc-ad7192 | 18 +++
drivers/iio/adc/ad7192.c | 128 ++++++++++++++++--
2 files changed, 134 insertions(+), 12 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
index f8315202c8f0..780c6841b0c3 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
@@ -19,6 +19,24 @@ Description:
the bridge can be disconnected (when it is not being used
using the bridge_switch_en attribute.
+What: /sys/bus/iio/devices/iio:deviceX/fast_settling_average_factor
+KernelVersion:
+Contact: [email protected]
+Description:
+ This attribute, if available, is used to activate or deactivate
+ fast settling mode and set the value of the average factor to
+ 1, 2, 8 or 16. If the average factor is set to 1, the fast
+ settling mode is disabled. The data from the sinc filter is
+ averaged by chosen value. The averaging reduces the output data
+ rate for a given FS word, however, the RMS noise improves.
+
+What: /sys/bus/iio/devices/iio:deviceX/fast_settling_average_factor_available
+KernelVersion:
+Contact: [email protected]
+Description:
+ Reading returns a list with the possible values for the fast
+ settling average factor: 1, 2, 8, 16.
+
What: /sys/bus/iio/devices/iio:deviceX/in_voltagex_sys_calibration
KernelVersion:
Contact: [email protected]
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 0f9d33002d35..3b7de23b024e 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -60,6 +60,8 @@
#define AD7192_MODE_SEL_MASK GENMASK(23, 21) /* Operation Mode Select Mask */
#define AD7192_MODE_STA_MASK BIT(20) /* Status Register transmission Mask */
#define AD7192_MODE_CLKSRC_MASK GENMASK(19, 18) /* Clock Source Select Mask */
+#define AD7192_MODE_AVG_MASK GENMASK(17, 16)
+ /* Fast Settling Filter Average Select Mask (AD7193 only) */
#define AD7192_MODE_SINC3 BIT(15) /* SINC3 Filter Select */
#define AD7192_MODE_ENPAR BIT(13) /* Parity Enable */
#define AD7192_MODE_CLKDIV BIT(12) /* Clock divide by 2 (AD7190/2 only)*/
@@ -182,6 +184,7 @@ struct ad7192_state {
u32 mode;
u32 conf;
u32 scale_avail[8][2];
+ u8 avg_avail[4];
u8 gpocon;
u8 clock_sel;
struct mutex lock; /* protect sensor state */
@@ -459,6 +462,13 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
st->scale_avail[i][0] = scale_uv;
}
+ if (st->chip_info->chip_id == CHIPID_AD7193) {
+ st->avg_avail[0] = 1;
+ st->avg_avail[1] = 2;
+ st->avg_avail[2] = 8;
+ st->avg_avail[3] = 16;
+ }
+
return 0;
}
@@ -483,6 +493,18 @@ static ssize_t ad7192_show_bridge_switch(struct device *dev,
FIELD_GET(AD7192_GPOCON_BPDSW, st->gpocon));
}
+static ssize_t ad7192_show_average_factor(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad7192_state *st = iio_priv(indio_dev);
+ u8 avg_factor_index;
+
+ avg_factor_index = FIELD_GET(AD7192_MODE_AVG_MASK, st->mode);
+ return sysfs_emit(buf, "%d\n", st->avg_avail[avg_factor_index]);
+}
+
static ssize_t ad7192_set(struct device *dev,
struct device_attribute *attr,
const char *buf,
@@ -491,12 +513,10 @@ static ssize_t ad7192_set(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7192_state *st = iio_priv(indio_dev);
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+ bool val, ret_einval;
+ u8 val_avg_factor;
+ unsigned int i;
int ret;
- bool val;
-
- ret = kstrtobool(buf, &val);
- if (ret < 0)
- return ret;
ret = iio_device_claim_direct_mode(indio_dev);
if (ret)
@@ -504,6 +524,10 @@ static ssize_t ad7192_set(struct device *dev,
switch ((u32)this_attr->address) {
case AD7192_REG_GPOCON:
+ ret = kstrtobool(buf, &val);
+ if (ret < 0)
+ return ret;
+
if (val)
st->gpocon |= AD7192_GPOCON_BPDSW;
else
@@ -512,6 +536,10 @@ static ssize_t ad7192_set(struct device *dev,
ad_sd_write_reg(&st->sd, AD7192_REG_GPOCON, 1, st->gpocon);
break;
case AD7192_REG_CONF:
+ ret = kstrtobool(buf, &val);
+ if (ret < 0)
+ return ret;
+
if (val)
st->conf |= AD7192_CONF_ACX;
else
@@ -519,6 +547,27 @@ static ssize_t ad7192_set(struct device *dev,
ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
break;
+ case AD7192_REG_MODE:
+ ret = kstrtou8(buf, 10, &val_avg_factor);
+ if (ret < 0)
+ return ret;
+
+ ret_einval = true;
+ for (i = 0; i < ARRAY_SIZE(st->avg_avail); i++) {
+ if (val_avg_factor == st->avg_avail[i]) {
+ st->mode &= ~AD7192_MODE_AVG_MASK;
+ st->mode |= FIELD_PREP(AD7192_MODE_AVG_MASK, i);
+ ret_einval = false;
+ }
+ }
+
+ if (ret_einval)
+ return -EINVAL;
+
+ ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+ if (ret)
+ return ret;
+ break;
default:
ret = -EINVAL;
}
@@ -528,15 +577,22 @@ static ssize_t ad7192_set(struct device *dev,
return ret ? ret : len;
}
-static int ad7192_compute_f_order(bool sinc3_en, bool chop_en)
+static int ad7192_compute_f_order(struct ad7192_state *st, bool sinc3_en, bool chop_en)
{
- if (!chop_en)
+ u8 avg_factor;
+ u8 avg_factor_selected;
+
+ avg_factor_selected = FIELD_GET(AD7192_MODE_AVG_MASK, st->mode);
+
+ if (!avg_factor_selected && !chop_en)
return 1;
+ avg_factor = st->avg_avail[avg_factor_selected];
+
if (sinc3_en)
- return AD7192_SYNC3_FILTER;
+ return AD7192_SYNC3_FILTER + avg_factor - 1;
- return AD7192_SYNC4_FILTER;
+ return AD7192_SYNC4_FILTER + avg_factor - 1;
}
static int ad7192_get_f_order(struct ad7192_state *st)
@@ -546,13 +602,13 @@ static int ad7192_get_f_order(struct ad7192_state *st)
sinc3_en = FIELD_GET(AD7192_MODE_SINC3, st->mode);
chop_en = FIELD_GET(AD7192_CONF_CHOP, st->conf);
- return ad7192_compute_f_order(sinc3_en, chop_en);
+ return ad7192_compute_f_order(st, sinc3_en, chop_en);
}
static int ad7192_compute_f_adc(struct ad7192_state *st, bool sinc3_en,
bool chop_en)
{
- unsigned int f_order = ad7192_compute_f_order(sinc3_en, chop_en);
+ unsigned int f_order = ad7192_compute_f_order(st, sinc3_en, chop_en);
return DIV_ROUND_CLOSEST(st->fclk,
f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
@@ -605,9 +661,29 @@ static ssize_t ad7192_show_filter_avail(struct device *dev,
return len;
}
+static ssize_t ad7192_show_avg_factor_avail(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad7192_state *st = iio_priv(indio_dev);
+ unsigned int i;
+ size_t len = 0;
+
+ for (i = 0; i < ARRAY_SIZE(st->avg_avail); i++)
+ len += sysfs_emit_at(buf, len, "%d ", st->avg_avail[i]);
+
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
0444, ad7192_show_filter_avail, NULL, 0);
+static IIO_DEVICE_ATTR(fast_settling_average_factor_available,
+ 0444, ad7192_show_avg_factor_avail, NULL, 0);
+
static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
ad7192_show_bridge_switch, ad7192_set,
AD7192_REG_GPOCON);
@@ -616,6 +692,10 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
ad7192_show_ac_excitation, ad7192_set,
AD7192_REG_CONF);
+static IIO_DEVICE_ATTR(fast_settling_average_factor, 0644,
+ ad7192_show_average_factor, ad7192_set,
+ AD7192_REG_MODE);
+
static struct attribute *ad7192_attributes[] = {
&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
&iio_dev_attr_bridge_switch_en.dev_attr.attr,
@@ -626,6 +706,18 @@ static const struct attribute_group ad7192_attribute_group = {
.attrs = ad7192_attributes,
};
+static struct attribute *ad7193_attributes[] = {
+ &iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
+ &iio_dev_attr_bridge_switch_en.dev_attr.attr,
+ &iio_dev_attr_fast_settling_average_factor.dev_attr.attr,
+ &iio_dev_attr_fast_settling_average_factor_available.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group ad7193_attribute_group = {
+ .attrs = ad7193_attributes,
+};
+
static struct attribute *ad7195_attributes[] = {
&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
&iio_dev_attr_bridge_switch_en.dev_attr.attr,
@@ -882,6 +974,16 @@ static const struct iio_info ad7192_info = {
.update_scan_mode = ad7192_update_scan_mode,
};
+static const struct iio_info ad7193_info = {
+ .read_raw = ad7192_read_raw,
+ .write_raw = ad7192_write_raw,
+ .write_raw_get_fmt = ad7192_write_raw_get_fmt,
+ .read_avail = ad7192_read_avail,
+ .attrs = &ad7193_attribute_group,
+ .validate_trigger = ad_sd_validate_trigger,
+ .update_scan_mode = ad7192_update_scan_mode,
+};
+
static const struct iio_info ad7195_info = {
.read_raw = ad7192_read_raw,
.write_raw = ad7192_write_raw,
@@ -1056,7 +1158,9 @@ static int ad7192_probe(struct spi_device *spi)
if (ret < 0)
return ret;
- if (st->chip_info->chip_id == CHIPID_AD7195)
+ if (st->chip_info->chip_id == CHIPID_AD7193)
+ indio_dev->info = &ad7193_info;
+ else if (st->chip_info->chip_id == CHIPID_AD7195)
indio_dev->info = &ad7195_info;
else
indio_dev->info = &ad7192_info;
--
2.34.1
On Mon, 25 Sep 2023 00:51:46 +0300
[email protected] wrote:
> From: Alisa-Dariana Roman <[email protected]>
>
> Include bitfield.h and update driver to use bitfield access macros
> GENMASK, FIELD_PREP and FIELD_GET.
>
> Remove old macros in favor of using FIELD_PREP and masks.
>
> Change %d to %ld to match the type of FIELD_GET().
>
> Signed-off-by: Alisa-Dariana Roman <[email protected]>
Hi Alisa-Dariana,
One more !! in here. I'll get rid of that whilst applying.
Applied to the togreg branch of iio.git.
Thanks,
Jonathan
> @@ -682,7 +682,8 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
> long m)
> {
> struct ad7192_state *st = iio_priv(indio_dev);
> - bool unipolar = !!(st->conf & AD7192_CONF_UNIPOLAR);
> + bool unipolar = !!FIELD_GET(AD7192_CONF_UNIPOLAR, st->conf);
This !! not needed.
> + u8 gain = FIELD_GET(AD7192_CONF_GAIN_MASK, st->conf);
>
> switch (m) {
> case IIO_CHAN_INFO_RAW:
On Mon, 25 Sep 2023 00:51:47 +0300
[email protected] wrote:
> From: Alisa-Dariana Roman <[email protected]>
>
> Instead of using the f_order member of ad7192_state, a function that
> computes the f_order coefficient makes more sense. This coefficient is a
> function of the sinc filter and chop filter states.
>
> Remove f_order member of ad7192_state structure. Instead use
> ad7192_compute_f_order function to compute the f_order coefficient
> according to the sinc filter and chop filter states passed as
> parameters.
>
> Add ad7192_get_f_order function that returns the current f_order
> coefficient of the device.
>
> Add ad7192_compute_f_adc function that computes the f_adc value
> according to the sinc filter and chop filter states passed as
> parameters.
>
> Add ad7192_get_f_adc function that returns the current f_adc value of
> the device.
>
> Signed-off-by: Alisa-Dariana Roman <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing
for 0-day to see if it can find anything we missed.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7192.c | 62 +++++++++++++++++++++++++++++-----------
> 1 file changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index d693f2ce8a20..0f9d33002d35 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -179,7 +179,6 @@ struct ad7192_state {
> struct clk *mclk;
> u16 int_vref_mv;
> u32 fclk;
> - u32 f_order;
> u32 mode;
> u32 conf;
> u32 scale_avail[8][2];
> @@ -419,7 +418,6 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
> st->conf |= AD7192_CONF_REFSEL;
>
> st->conf &= ~AD7192_CONF_CHOP;
> - st->f_order = AD7192_NO_SYNC_FILTER;
>
> buf_en = of_property_read_bool(np, "adi,buffer-enable");
> if (buf_en)
> @@ -530,22 +528,60 @@ static ssize_t ad7192_set(struct device *dev,
> return ret ? ret : len;
> }
>
> +static int ad7192_compute_f_order(bool sinc3_en, bool chop_en)
> +{
> + if (!chop_en)
> + return 1;
> +
> + if (sinc3_en)
> + return AD7192_SYNC3_FILTER;
> +
> + return AD7192_SYNC4_FILTER;
> +}
> +
> +static int ad7192_get_f_order(struct ad7192_state *st)
> +{
> + bool sinc3_en, chop_en;
> +
> + sinc3_en = FIELD_GET(AD7192_MODE_SINC3, st->mode);
> + chop_en = FIELD_GET(AD7192_CONF_CHOP, st->conf);
> +
> + return ad7192_compute_f_order(sinc3_en, chop_en);
> +}
> +
> +static int ad7192_compute_f_adc(struct ad7192_state *st, bool sinc3_en,
> + bool chop_en)
> +{
> + unsigned int f_order = ad7192_compute_f_order(sinc3_en, chop_en);
> +
> + return DIV_ROUND_CLOSEST(st->fclk,
> + f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> +}
> +
> +static int ad7192_get_f_adc(struct ad7192_state *st)
> +{
> + unsigned int f_order = ad7192_get_f_order(st);
> +
> + return DIV_ROUND_CLOSEST(st->fclk,
> + f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> +}
> +
> static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> int *freq)
> {
> unsigned int fadc;
>
> /* Formulas for filter at page 25 of the datasheet */
> - fadc = DIV_ROUND_CLOSEST(st->fclk,
> - AD7192_SYNC4_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> + fadc = ad7192_compute_f_adc(st, false, true);
> freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
>
> - fadc = DIV_ROUND_CLOSEST(st->fclk,
> - AD7192_SYNC3_FILTER * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> + fadc = ad7192_compute_f_adc(st, true, true);
> freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
>
> - fadc = DIV_ROUND_CLOSEST(st->fclk, FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> + fadc = ad7192_compute_f_adc(st, false, false);
> freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
> +
> + fadc = ad7192_compute_f_adc(st, true, false);
> freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
> }
>
> @@ -628,25 +664,21 @@ static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
>
> switch (idx) {
> case 0:
> - st->f_order = AD7192_SYNC4_FILTER;
> st->mode &= ~AD7192_MODE_SINC3;
>
> st->conf |= AD7192_CONF_CHOP;
> break;
> case 1:
> - st->f_order = AD7192_SYNC3_FILTER;
> st->mode |= AD7192_MODE_SINC3;
>
> st->conf |= AD7192_CONF_CHOP;
> break;
> case 2:
> - st->f_order = AD7192_NO_SYNC_FILTER;
> st->mode &= ~AD7192_MODE_SINC3;
>
> st->conf &= ~AD7192_CONF_CHOP;
> break;
> case 3:
> - st->f_order = AD7192_NO_SYNC_FILTER;
> st->mode |= AD7192_MODE_SINC3;
>
> st->conf &= ~AD7192_CONF_CHOP;
> @@ -664,8 +696,7 @@ static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
> {
> unsigned int fadc;
>
> - fadc = DIV_ROUND_CLOSEST(st->fclk,
> - st->f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> + fadc = ad7192_get_f_adc(st);
>
> if (FIELD_GET(AD7192_CONF_CHOP, st->conf))
> return DIV_ROUND_CLOSEST(fadc * 240, 1024);
> @@ -713,8 +744,7 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
> *val -= 273 * ad7192_get_temp_scale(unipolar);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - *val = st->fclk /
> - (st->f_order * 1024 * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> + *val = DIV_ROUND_CLOSEST(ad7192_get_f_adc(st), 1024);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> *val = ad7192_get_3db_filter_freq(st);
> @@ -764,7 +794,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
> break;
> }
>
> - div = st->fclk / (val * st->f_order * 1024);
> + div = st->fclk / (val * ad7192_get_f_order(st) * 1024);
> if (div < 1 || div > 1023) {
> ret = -EINVAL;
> break;
On Mon, 25 Sep 2023 00:51:48 +0300
[email protected] wrote:
> From: Alisa-Dariana Roman <[email protected]>
>
> Add fast settling mode support for AD7193.
>
> Add fast_settling_average_factor attribute. Expose to user the current
> value of the fast settling average factor. User can change the value by
> writing a new one.
>
> Add fast_settling_average_factor_available attribute. Expose to user
> possible values for the fast settling average factor.
>
> Signed-off-by: Alisa-Dariana Roman <[email protected]>
Hi Alisa-Dariana,
Some questions inline.
Jonathan
> ---
> .../ABI/testing/sysfs-bus-iio-adc-ad7192 | 18 +++
> drivers/iio/adc/ad7192.c | 128 ++++++++++++++++--
> 2 files changed, 134 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
> index f8315202c8f0..780c6841b0c3 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192
> @@ -19,6 +19,24 @@ Description:
> the bridge can be disconnected (when it is not being used
> using the bridge_switch_en attribute.
>
> +What: /sys/bus/iio/devices/iio:deviceX/fast_settling_average_factor
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + This attribute, if available, is used to activate or deactivate
> + fast settling mode and set the value of the average factor to
> + 1, 2, 8 or 16. If the average factor is set to 1, the fast
> + settling mode is disabled. The data from the sinc filter is
> + averaged by chosen value. The averaging reduces the output data
> + rate for a given FS word, however, the RMS noise improves.
So I got distracted in earlier patches, and didn't read your description closely
so was assuming this was some weird filter control.
Despite it being described as being closely related to the sinc filter, is this
just an oversampling ratio control? If it is, then you can use the standard
ABI for this. Making sure to reflect changes in this on the sampling frequency
as the apparent sampling frequency will reduce.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/fast_settling_average_factor_available
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Reading returns a list with the possible values for the fast
> + settling average factor: 1, 2, 8, 16.
> +
> What: /sys/bus/iio/devices/iio:deviceX/in_voltagex_sys_calibration
> KernelVersion:
> Contact: [email protected]
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 0f9d33002d35..3b7de23b024e 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -60,6 +60,8 @@
> #define AD7192_MODE_SEL_MASK GENMASK(23, 21) /* Operation Mode Select Mask */
> #define AD7192_MODE_STA_MASK BIT(20) /* Status Register transmission Mask */
> #define AD7192_MODE_CLKSRC_MASK GENMASK(19, 18) /* Clock Source Select Mask */
> +#define AD7192_MODE_AVG_MASK GENMASK(17, 16)
> + /* Fast Settling Filter Average Select Mask (AD7193 only) */
> #define AD7192_MODE_SINC3 BIT(15) /* SINC3 Filter Select */
> #define AD7192_MODE_ENPAR BIT(13) /* Parity Enable */
> #define AD7192_MODE_CLKDIV BIT(12) /* Clock divide by 2 (AD7190/2 only)*/
> @@ -182,6 +184,7 @@ struct ad7192_state {
> u32 mode;
> u32 conf;
> u32 scale_avail[8][2];
> + u8 avg_avail[4];
> u8 gpocon;
> u8 clock_sel;
> struct mutex lock; /* protect sensor state */
> @@ -459,6 +462,13 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
> st->scale_avail[i][0] = scale_uv;
> }
>
> + if (st->chip_info->chip_id == CHIPID_AD7193) {
> + st->avg_avail[0] = 1;
> + st->avg_avail[1] = 2;
> + st->avg_avail[2] = 8;
> + st->avg_avail[3] = 16;
> + }
> +
> return 0;
> }
>
> @@ -483,6 +493,18 @@ static ssize_t ad7192_show_bridge_switch(struct device *dev,
> FIELD_GET(AD7192_GPOCON_BPDSW, st->gpocon));
> }
>
> +static ssize_t ad7192_show_average_factor(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ad7192_state *st = iio_priv(indio_dev);
> + u8 avg_factor_index;
> +
> + avg_factor_index = FIELD_GET(AD7192_MODE_AVG_MASK, st->mode);
> + return sysfs_emit(buf, "%d\n", st->avg_avail[avg_factor_index]);
> +}
> +
> static ssize_t ad7192_set(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> @@ -491,12 +513,10 @@ static ssize_t ad7192_set(struct device *dev,
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct ad7192_state *st = iio_priv(indio_dev);
> struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + bool val, ret_einval;
> + u8 val_avg_factor;
There isn't really that much code shared between the new use of this
and previous ones. I'd introduce a new callback for that particular sysfs
attribute instead of changing this one.
> + unsigned int i;
> int ret;
> - bool val;
> -
> - ret = kstrtobool(buf, &val);
> - if (ret < 0)
> - return ret;
>
> ret = iio_device_claim_direct_mode(indio_dev);
> if (ret)
> @@ -504,6 +524,10 @@ static ssize_t ad7192_set(struct device *dev,
>
> switch ((u32)this_attr->address) {
> case AD7192_REG_GPOCON:
> + ret = kstrtobool(buf, &val);
> + if (ret < 0)
> + return ret;
> +
> if (val)
> st->gpocon |= AD7192_GPOCON_BPDSW;
> else
> @@ -512,6 +536,10 @@ static ssize_t ad7192_set(struct device *dev,
> ad_sd_write_reg(&st->sd, AD7192_REG_GPOCON, 1, st->gpocon);
> break;
> case AD7192_REG_CONF:
> + ret = kstrtobool(buf, &val);
> + if (ret < 0)
> + return ret;
> +
> if (val)
> st->conf |= AD7192_CONF_ACX;
> else
> @@ -519,6 +547,27 @@ static ssize_t ad7192_set(struct device *dev,
>
> ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
> break;
> + case AD7192_REG_MODE:
> + ret = kstrtou8(buf, 10, &val_avg_factor);
> + if (ret < 0)
> + return ret;
> +
> + ret_einval = true;
I'd rather this relected the positive condition.
So maybe
bool found = false;
> + for (i = 0; i < ARRAY_SIZE(st->avg_avail); i++) {
> + if (val_avg_factor == st->avg_avail[i]) {
> + st->mode &= ~AD7192_MODE_AVG_MASK;
> + st->mode |= FIELD_PREP(AD7192_MODE_AVG_MASK, i);
> + ret_einval = false;
found = true;
break;
> + }
> + }
> +
> + if (ret_einval)
if (!found)
return -EINVAL;
> + return -EINVAL;
> +
> + ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
> + if (ret)
> + return ret;
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -528,15 +577,22 @@ static ssize_t ad7192_set(struct device *dev,
> return ret ? ret : len;
> }