* [PATCH v2 1/3] iio: adc: ad7192: Use bitfield access macros
2023-09-20 0:33 [PATCH v2 0/3] iio: adc: ad7192: Add improvements and feature alisadariana
@ 2023-09-20 0:33 ` alisadariana
2023-09-24 14:39 ` Jonathan Cameron
2023-09-20 0:33 ` [PATCH v2 2/3] iio: adc: ad7192: Improve f_order computation alisadariana
2023-09-20 0:33 ` [PATCH v2 3/3] iio: adc: ad7192: Add fast settling support alisadariana
2 siblings, 1 reply; 6+ messages in thread
From: alisadariana @ 2023-09-20 0:33 UTC (permalink / raw)
Cc: Alisa-Dariana Roman, Alisa-Dariana Roman, Jonathan Cameron,
Lars-Peter Clausen, Alexandru Tachici, Michael Hennerich,
linux-iio, linux-kernel
From: Alisa-Dariana Roman <alisadariana@gmail.com>
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 <alisa.roman@analog.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] iio: adc: ad7192: Use bitfield access macros
2023-09-20 0:33 ` [PATCH v2 1/3] iio: adc: ad7192: Use bitfield access macros alisadariana
@ 2023-09-24 14:39 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-09-24 14:39 UTC (permalink / raw)
To: alisadariana
Cc: Alisa-Dariana Roman, Lars-Peter Clausen, Alexandru Tachici,
Michael Hennerich, linux-iio, linux-kernel
On Wed, 20 Sep 2023 03:33:40 +0300
alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisadariana@gmail.com>
>
> 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 <alisa.roman@analog.com>
Hi Alisa-Darinana,
Please fix the author for these to match your sign off
git commit --amend --author="Alisa-Dariana Roman <alisa.roman@analog.com>"
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] iio: adc: ad7192: Improve f_order computation
2023-09-20 0:33 [PATCH v2 0/3] iio: adc: ad7192: Add improvements and feature alisadariana
2023-09-20 0:33 ` [PATCH v2 1/3] iio: adc: ad7192: Use bitfield access macros alisadariana
@ 2023-09-20 0:33 ` alisadariana
2023-09-20 0:33 ` [PATCH v2 3/3] iio: adc: ad7192: Add fast settling support alisadariana
2 siblings, 0 replies; 6+ messages in thread
From: alisadariana @ 2023-09-20 0:33 UTC (permalink / raw)
Cc: Alisa-Dariana Roman, Alisa-Dariana Roman, Jonathan Cameron,
Lars-Peter Clausen, Alexandru Tachici, Michael Hennerich,
linux-iio, linux-kernel
From: Alisa-Dariana Roman <alisadariana@gmail.com>
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 <alisa.roman@analog.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] iio: adc: ad7192: Add fast settling support
2023-09-20 0:33 [PATCH v2 0/3] iio: adc: ad7192: Add improvements and feature alisadariana
2023-09-20 0:33 ` [PATCH v2 1/3] iio: adc: ad7192: Use bitfield access macros alisadariana
2023-09-20 0:33 ` [PATCH v2 2/3] iio: adc: ad7192: Improve f_order computation alisadariana
@ 2023-09-20 0:33 ` alisadariana
2023-09-24 14:47 ` Jonathan Cameron
2 siblings, 1 reply; 6+ messages in thread
From: alisadariana @ 2023-09-20 0:33 UTC (permalink / raw)
Cc: Alisa-Dariana Roman, Alisa-Dariana Roman, Jonathan Cameron,
Lars-Peter Clausen, Alexandru Tachici, Michael Hennerich,
linux-iio, linux-kernel
From: Alisa-Dariana Roman <alisadariana@gmail.com>
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 <alisa.roman@analog.com>
---
.../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: linux-iio@vger.kernel.org
+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: linux-iio@vger.kernel.org
+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: linux-iio@vger.kernel.org
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] iio: adc: ad7192: Add fast settling support
2023-09-20 0:33 ` [PATCH v2 3/3] iio: adc: ad7192: Add fast settling support alisadariana
@ 2023-09-24 14:47 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-09-24 14:47 UTC (permalink / raw)
To: alisadariana
Cc: Alisa-Dariana Roman, Lars-Peter Clausen, Alexandru Tachici,
Michael Hennerich, linux-iio, linux-kernel
On Wed, 20 Sep 2023 03:33:42 +0300
alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisadariana@gmail.com>
>
> 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 <alisa.roman@analog.com>
> ---
> .../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: linux-iio@vger.kernel.org
> +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: linux-iio@vger.kernel.org
> +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: linux-iio@vger.kernel.org
> 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;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread