Subject: [PATCH v3 0/5] AD7173 fixes

This patch series adds fixes for ad7173 driver that were originally
sent along AD411x series. To ensure that they are included in this
current rc cycle they are sent in a separate series with the Fixes tag.

Signed-off-by: Dumitru Ceclan <[email protected]>
---
Changes in v3:
iio: adc: ad7173: Fix sampling frequency setting
-Remove IIO_CHAN_INFO_SAMP_FREQ from the shared mask
iio: adc: ad7173: Clear append status bit
iio: adc: ad7173: Remove index from temp channel
iio: adc: ad7173: Add ad7173_device_info names
iio: adc: ad7173: fix buffers enablement for ad7176-2
-No changes

- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
iio: adc: ad7173: Fix sampling frequency setting
-Patch created
iio: adc: ad7173: Clear append status bit
-Patch created
iio: adc: ad7173: Remove index from temp channel
iio: adc: ad7173: Add ad7173_device_info names
iio: adc: ad7173: fix buffers enablement for ad7176-2
-No changes

- Link to v1: https://lore.kernel.org/r/[email protected]

---
Dumitru Ceclan (5):
iio: adc: ad7173: fix buffers enablement for ad7176-2
iio: adc: ad7173: Add ad7173_device_info names
iio: adc: ad7173: Remove index from temp channel
iio: adc: ad7173: Clear append status bit
iio: adc: ad7173: Fix sampling frequency setting

drivers/iio/adc/ad7173.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
---
base-commit: 5ab61121a34759eb2418977f0b3589b7edc57776
change-id: 20240521-ad7173-fixes-4e2e5a061ef2

Best regards,
--
Dumitru Ceclan <[email protected]>




Subject: [PATCH v3 3/5] iio: adc: ad7173: Remove index from temp channel

From: Dumitru Ceclan <[email protected]>

Temperature channel is unique per device, index is not needed.
This is breaking userspace: Include fixes tag to be released within the
same rc cycle.

Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 58620be41ef5..eb512878c30e 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -815,7 +815,6 @@ static const struct iio_chan_spec ad7173_channel_template = {

static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
.type = IIO_TEMP,
- .indexed = 1,
.channel = AD7173_AIN_TEMP_POS,
.channel2 = AD7173_AIN_TEMP_NEG,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |

--
2.43.0



Subject: [PATCH v3 2/5] iio: adc: ad7173: Add ad7173_device_info names

From: Dumitru Ceclan <[email protected]>

Add missing names from the device info struct for 3 models to ensure
consistency with the rest of the models.

Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 850574437bda..58620be41ef5 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -220,6 +220,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
},
[ID_AD7172_4] = {
+ .name = "ad7172-4",
.id = AD7172_4_ID,
.num_inputs = 9,
.num_channels = 8,
@@ -262,6 +263,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
},
[ID_AD7175_8] = {
+ .name = "ad7175-8",
.id = AD7175_8_ID,
.num_inputs = 17,
.num_channels = 16,
@@ -290,6 +292,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
},
[ID_AD7177_2] = {
+ .name = "ad7177-2",
.id = AD7177_ID,
.num_inputs = 5,
.num_channels = 4,

--
2.43.0



Subject: [PATCH v3 4/5] iio: adc: ad7173: Clear append status bit

From: Dumitru Ceclan <[email protected]>

The previous value of the append status bit was not cleared before
setting the new value. This caused the bit to remain set after enabling
buffered mode for multiple channels and not permit further buffered
reads from a single channel after the fact.

Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index eb512878c30e..e66a137a76be 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -543,6 +543,7 @@ static int ad7173_append_status(struct ad_sigma_delta *sd, bool append)
unsigned int interface_mode = st->interface_mode;
int ret;

+ interface_mode &= ~AD7173_INTERFACE_DATA_STAT;
interface_mode |= AD7173_INTERFACE_DATA_STAT_EN(append);
ret = ad_sd_write_reg(&st->sd, AD7173_REG_INTERFACE_MODE, 2, interface_mode);
if (ret)

--
2.43.0



Subject: [PATCH v3 1/5] iio: adc: ad7173: fix buffers enablement for ad7176-2

From: Dumitru Ceclan <[email protected]>

AD7176-2 does not feature input buffers and marks corespondent register
bits as read only. Enable buffers only on supported models.

Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
Reviewed-by: David Lechner <[email protected]>
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index f6d29abe1d04..850574437bda 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -145,6 +145,7 @@ struct ad7173_device_info {
unsigned int id;
char *name;
bool has_temp;
+ bool has_input_buf;
bool has_int_ref;
bool has_ref2;
u8 num_gpios;
@@ -212,6 +213,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
@@ -224,6 +226,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = false,
+ .has_input_buf = true,
.has_ref2 = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
@@ -237,6 +240,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
.clock = 2 * HZ_PER_MHZ,
@@ -251,6 +255,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
@@ -263,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
.clock = 16 * HZ_PER_MHZ,
@@ -277,6 +283,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = false,
+ .has_input_buf = false,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
@@ -289,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.odr_start_value = AD7177_ODR_START_VALUE,
@@ -932,7 +940,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
AD7173_CH_ADDRESS(chan_arr[chan_index].channel,
chan_arr[chan_index].channel2);
chan_st_priv->cfg.bipolar = false;
- chan_st_priv->cfg.input_buf = true;
+ chan_st_priv->cfg.input_buf = st->info->has_input_buf;
chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
st->adc_mode |= AD7173_ADC_MODE_REF_EN;

@@ -989,7 +997,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)

chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
chan_st_priv->chan_reg = chan_index;
- chan_st_priv->cfg.input_buf = true;
+ chan_st_priv->cfg.input_buf = st->info->has_input_buf;
chan_st_priv->cfg.odr = 0;

chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");

--
2.43.0



Subject: [PATCH v3 5/5] iio: adc: ad7173: Fix sampling frequency setting

From: Dumitru Ceclan <[email protected]>

This patch fixes two issues regarding the sampling frequency setting:
-The attribute was set as per device, not per channel. As such, when
setting the sampling frequency, the configuration was always done for
the slot 0, and the correct configuration was applied on the next
channel configuration call by the LRU mechanism.
-The LRU implementation does not take into account external settings of
the slot registers. When setting the sampling frequency directly to a
slot register in write_raw(), there is no guarantee that other channels
were not also using that slot and now incorrectly retain their config
as live.

Set the sampling frequency attribute as separate in the channel templates.
Do not set the sampling directly to the slot register in write_raw(),
just mark the config as not live and let the LRU mechanism handle it.
As the reg variable is no longer used, remove it.

Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index e66a137a76be..ebf248741b34 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -717,7 +717,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
{
struct ad7173_state *st = iio_priv(indio_dev);
struct ad7173_channel_config *cfg;
- unsigned int freq, i, reg;
+ unsigned int freq, i;
int ret;

ret = iio_device_claim_direct_mode(indio_dev);
@@ -733,16 +733,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,

cfg = &st->channels[chan->address].cfg;
cfg->odr = i;
-
- if (!cfg->live)
- break;
-
- ret = ad_sd_read_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, &reg);
- if (ret)
- break;
- reg &= ~AD7173_FILTER_ODR0_MASK;
- reg |= FIELD_PREP(AD7173_FILTER_ODR0_MASK, i);
- ret = ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, reg);
+ cfg->live = false;
break;

default:
@@ -804,8 +795,7 @@ static const struct iio_chan_spec ad7173_channel_template = {
.type = IIO_VOLTAGE,
.indexed = 1,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
- BIT(IIO_CHAN_INFO_SCALE),
- .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
.scan_type = {
.sign = 'u',
.realbits = 24,
@@ -819,8 +809,8 @@ static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
.channel = AD7173_AIN_TEMP_POS,
.channel2 = AD7173_AIN_TEMP_NEG,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
- BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
- .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
.scan_type = {
.sign = 'u',
.realbits = 24,

--
2.43.0



2024-06-02 12:19:58

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] iio: adc: ad7173: fix buffers enablement for ad7176-2

On Thu, 30 May 2024 15:07:49 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:

> From: Dumitru Ceclan <[email protected]>
>
> AD7176-2 does not feature input buffers and marks corespondent register
> bits as read only. Enable buffers only on supported models.
>
> Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
> Reviewed-by: David Lechner <[email protected]>
> Signed-off-by: Dumitru Ceclan <[email protected]>
I already picked up v2 of patches 1-3 Must have crossed with this.

Jonathan

> ---
> drivers/iio/adc/ad7173.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index f6d29abe1d04..850574437bda 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -145,6 +145,7 @@ struct ad7173_device_info {
> unsigned int id;
> char *name;
> bool has_temp;
> + bool has_input_buf;
> bool has_int_ref;
> bool has_ref2;
> u8 num_gpios;
> @@ -212,6 +213,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -224,6 +226,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = false,
> + .has_input_buf = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -237,6 +240,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> @@ -251,6 +255,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7175_sinc5_data_rates,
> @@ -263,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 16 * HZ_PER_MHZ,
> @@ -277,6 +283,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = false,
> + .has_input_buf = false,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7175_sinc5_data_rates,
> @@ -289,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .odr_start_value = AD7177_ODR_START_VALUE,
> @@ -932,7 +940,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> AD7173_CH_ADDRESS(chan_arr[chan_index].channel,
> chan_arr[chan_index].channel2);
> chan_st_priv->cfg.bipolar = false;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>
> @@ -989,7 +997,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>
> chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
> chan_st_priv->chan_reg = chan_index;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.odr = 0;
>
> chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
>


2024-06-02 12:22:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] iio: adc: ad7173: Clear append status bit

On Thu, 30 May 2024 15:07:52 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:

> From: Dumitru Ceclan <[email protected]>
>
> The previous value of the append status bit was not cleared before
> setting the new value. This caused the bit to remain set after enabling
> buffered mode for multiple channels and not permit further buffered
> reads from a single channel after the fact.
>
> Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
> Signed-off-by: Dumitru Ceclan <[email protected]>
Applied to the fixes-togreg branch of iio.git.

> ---
> drivers/iio/adc/ad7173.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index eb512878c30e..e66a137a76be 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -543,6 +543,7 @@ static int ad7173_append_status(struct ad_sigma_delta *sd, bool append)
> unsigned int interface_mode = st->interface_mode;
> int ret;
>
> + interface_mode &= ~AD7173_INTERFACE_DATA_STAT;
> interface_mode |= AD7173_INTERFACE_DATA_STAT_EN(append);
> ret = ad_sd_write_reg(&st->sd, AD7173_REG_INTERFACE_MODE, 2, interface_mode);
> if (ret)
>


2024-06-02 12:22:23

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] iio: adc: ad7173: Fix sampling frequency setting

On Thu, 30 May 2024 15:07:53 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:

> From: Dumitru Ceclan <[email protected]>
>
> This patch fixes two issues regarding the sampling frequency setting:
> -The attribute was set as per device, not per channel. As such, when
> setting the sampling frequency, the configuration was always done for
> the slot 0, and the correct configuration was applied on the next
> channel configuration call by the LRU mechanism.
> -The LRU implementation does not take into account external settings of
> the slot registers. When setting the sampling frequency directly to a
> slot register in write_raw(), there is no guarantee that other channels
> were not also using that slot and now incorrectly retain their config
> as live.
>
> Set the sampling frequency attribute as separate in the channel templates.
> Do not set the sampling directly to the slot register in write_raw(),
> just mark the config as not live and let the LRU mechanism handle it.
> As the reg variable is no longer used, remove it.
>
> Fixes: 8eb903272f75 ("iio: adc: ad7173: add AD7173 driver")
> Signed-off-by: Dumitru Ceclan <[email protected]>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan