[PATCH v3 0/5] AD7173 fixes

[PATCH v3 0/5] AD7173 fixes

[Dumitru Ceclan via B4 Relay]

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 <dumitru.ceclan@analog.com>
---
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/20240527-ad7173-fixes-v2-0-8501b66adb1f@analog.com

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/20240521-ad7173-fixes-v1-0-8161cc7f3ad1@analog.com

---
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 <dumitru.ceclan@analog.com>

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

[Dumitru Ceclan via B4 Relay]

From: Dumitru Ceclan <dumitru.ceclan@analog.com>

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 <dlechner@baylibre.com>
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
 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

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

[Jonathan Cameron]

On Thu, 30 May 2024 15:07:49 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:

> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> 
> 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 <dlechner@baylibre.com>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
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");
> 

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

[Dumitru Ceclan via B4 Relay]

From: Dumitru Ceclan <dumitru.ceclan@analog.com>

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 <dumitru.ceclan@analog.com>
---
 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

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

[Dumitru Ceclan via B4 Relay]

From: Dumitru Ceclan <dumitru.ceclan@analog.com>

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 <dumitru.ceclan@analog.com>
---
 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

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

[Dumitru Ceclan via B4 Relay]

From: Dumitru Ceclan <dumitru.ceclan@analog.com>

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 <dumitru.ceclan@analog.com>
---
 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

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

[Jonathan Cameron]

On Thu, 30 May 2024 15:07:52 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:

> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> 
> 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 <dumitru.ceclan@analog.com>
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)
> 

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

[Dumitru Ceclan via B4 Relay]

From: Dumitru Ceclan <dumitru.ceclan@analog.com>

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 <dumitru.ceclan@analog.com>
---
 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

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

[Jonathan Cameron]

On Thu, 30 May 2024 15:07:53 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:

> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> 
> 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 <dumitru.ceclan@analog.com>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan