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 -> v1
- replace old macros with FIELD_PREP() in commit "Use bitfield
access macros"
- update the other commits accordingly
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]>
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..5790adbb1cc1 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 eed3de02c26d..8987b78865f3 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_PREP(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
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 64bc09ce3cb1..eed3de02c26d 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_PREP(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_PREP(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_PREP(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_PREP(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_PREP(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_PREP(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_PREP(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]>
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.
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..64bc09ce3cb1 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 */
+#define AD7192_MODE_RATE_MASK GENMASK(9, 0)
+ /* Filter Update Rate Select Mask */
/* 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, "%d\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, "%d\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_PREP(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_PREP(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_PREP(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_PREP(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_PREP(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_PREP(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
On Wed, 20 Sep 2023 03:33:40 +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.
>
> Signed-off-by: Alisa-Dariana Roman <[email protected]>
Hi Alisa-Darinana,
Please fix the author for these to match your sign off
git commit --amend --author="Alisa-Dariana Roman <[email protected]>"
should fix that up.
There are a few interesting corners in the original code where a single
macro was used for the get and set paths. I think a few places you picked
the one with the wrong semantics whilst cleaning it up.
Also some unnecessary !! horribleness now we are extracting the value rather
than a shifted version of the value.
Thanks,
Jonathan
> ---
> 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..64bc09ce3cb1 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 */
> +#define AD7192_MODE_RATE_MASK GENMASK(9, 0)
> + /* Filter Update Rate Select Mask */
Put the comment on the line above the thing it is talking about, not the line below.
>
> /* 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, "%d\n", !!FIELD_GET(AD7192_CONF_ACX, st->conf));
Can drop the !! as FIELD_GET() will shift it so the value is 0 or 1 anyway.
> }
>
> 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, "%d\n",
> + !!FIELD_GET(AD7192_GPOCON_BPDSW, st->gpocon));
Drop the !!
> }
>
> 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_PREP(AD7192_MODE_RATE_MASK, st->mode));
FIELD_GET()
> 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_PREP(AD7192_MODE_RATE_MASK, st->mode));
FIELD_GET()
> 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_PREP(AD7192_MODE_RATE_MASK, st->mode));
As below FIELD_GET()
> 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_PREP(AD7192_MODE_RATE_MASK, st->mode));
As below. I think this should be a FIELD_GET()
>
> - 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_PREP(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_PREP(AD7192_MODE_RATE_MASK, st->mode));
FIELD_GET() I think. Though not 100% sure of intent and as it just masks
it isn't a code change to flip from one to the other.
> 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)
On Wed, 20 Sep 2023 03:33:42 +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]>
> ---
> .../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..5790adbb1cc1 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.
Trivial but RMS as it's an acronym.
> +
> +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 eed3de02c26d..8987b78865f3 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) {
Hmm. This does match with local style, but I'd have preferred if the driver
put all this information in the chip_info structure rather than scattered in code
thoughout the driver. That would be a bigger and mostly unrelated cleanup however
and this doesn't make the situation any worse really so I'm fine with this
patch as it stands.
Jonathan
> + st->avg_avail[0] = 1;
> + st->avg_avail[1] = 2;
> + st->avg_avail[2] = 8;
> + st->avg_avail[3] = 16;
> + }
> +
> return 0;
> }
>