From: Dumitru Ceclan <[email protected]>
Sigma delta ADCs with a sequencer need to disable the previously enabled
channel when reading using ad_sigma_delta_single_conversion(). This was
done manually in drivers for devices with sequencers.
This patch implements handling of single channel disabling after a
single conversion.
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7124.c | 14 ++++++++------
drivers/iio/adc/ad7173.c | 11 ++++++-----
drivers/iio/adc/ad_sigma_delta.c | 6 ++++++
include/linux/iio/adc/ad_sigma_delta.h | 14 ++++++++++++++
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index e7b1d517d3de..3beed78496c5 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -555,10 +555,18 @@ static int ad7124_disable_all(struct ad_sigma_delta *sd)
return 0;
}
+static int ad7124_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
+{
+ struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
+
+ return ad7124_spi_write_mask(st, AD7124_CHANNEL(chan), AD7124_CHANNEL_EN_MSK, 0, 2);
+}
+
static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
.set_channel = ad7124_set_channel,
.append_status = ad7124_append_status,
.disable_all = ad7124_disable_all,
+ .disable_one = ad7124_disable_one,
.set_mode = ad7124_set_mode,
.has_registers = true,
.addr_shift = 0,
@@ -582,12 +590,6 @@ static int ad7124_read_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- /* After the conversion is performed, disable the channel */
- ret = ad_sd_write_reg(&st->sd, AD7124_CHANNEL(chan->address), 2,
- st->channels[chan->address].ain | AD7124_CHANNEL_EN(0));
- if (ret < 0)
- return ret;
-
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
mutex_lock(&st->cfgs_lock);
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 638e2468efbf..f3088e8b4b8b 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -569,10 +569,16 @@ static int ad7173_disable_all(struct ad_sigma_delta *sd)
return 0;
}
+static int ad7173_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
+{
+ return ad_sd_write_reg(sd, AD7173_REG_CH(chan), 2, 0);
+}
+
static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
.set_channel = ad7173_set_channel,
.append_status = ad7173_append_status,
.disable_all = ad7173_disable_all,
+ .disable_one = ad7173_disable_one,
.set_mode = ad7173_set_mode,
.has_registers = true,
.addr_shift = 0,
@@ -668,11 +674,6 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- /* disable channel after single conversion */
- ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(chan->address), 2, 0);
- if (ret < 0)
- return ret;
-
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
if (chan->type == IIO_TEMP) {
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 97a05f325df7..ec34b3d1336f 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -321,6 +321,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
sigma_delta->keep_cs_asserted = false;
ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
+ ad_sigma_delta_disable_one(sigma_delta, chan->address);
sigma_delta->bus_locked = false;
spi_bus_unlock(sigma_delta->spi->controller);
iio_device_release_direct_mode(indio_dev);
@@ -671,6 +672,11 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
dev_err(&spi->dev, "ad_sigma_delta_info lacks disable_all().\n");
return -EINVAL;
}
+
+ if (!info->disable_one) {
+ dev_err(&spi->dev, "ad_sigma_delta_info lacks disable_one().\n");
+ return -EINVAL;
+ }
}
if (info->irq_line)
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 383614ebd760..f8c1d2505940 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -37,6 +37,10 @@ struct iio_dev;
* @append_status: Will be called to enable status append at the end of the sample, may be NULL.
* @set_mode: Will be called to select the current mode, may be NULL.
* @disable_all: Will be called to disable all channels, may be NULL.
+ * @disable_one: Will be called to disable a single channel after
+ * ad_sigma_delta_single_conversion(), may be NULL.
+ * Usage of this callback expects iio_chan_spec.address to contain
+ * the value required for the driver to identify the channel.
* @postprocess_sample: Is called for each sampled data word, can be used to
* modify or drop the sample data, it, may be NULL.
* @has_registers: true if the device has writable and readable registers, false
@@ -55,6 +59,7 @@ struct ad_sigma_delta_info {
int (*append_status)(struct ad_sigma_delta *, bool append);
int (*set_mode)(struct ad_sigma_delta *, enum ad_sigma_delta_mode mode);
int (*disable_all)(struct ad_sigma_delta *);
+ int (*disable_one)(struct ad_sigma_delta *, unsigned int chan);
int (*postprocess_sample)(struct ad_sigma_delta *, unsigned int raw_sample);
bool has_registers;
unsigned int addr_shift;
@@ -140,6 +145,15 @@ static inline int ad_sigma_delta_disable_all(struct ad_sigma_delta *sd)
return 0;
}
+static inline int ad_sigma_delta_disable_one(struct ad_sigma_delta *sd,
+ unsigned int chan)
+{
+ if (sd->info->disable_one)
+ return sd->info->disable_one(sd, chan);
+
+ return 0;
+}
+
static inline int ad_sigma_delta_set_mode(struct ad_sigma_delta *sd,
unsigned int mode)
{
--
2.43.0
On Thu, 2024-06-06 at 19:07 +0300, Dumitru Ceclan via B4 Relay wrote:
> From: Dumitru Ceclan <[email protected]>
>
> Sigma delta ADCs with a sequencer need to disable the previously enabled
> channel when reading using ad_sigma_delta_single_conversion(). This was
> done manually in drivers for devices with sequencers.
>
> This patch implements handling of single channel disabling after a
> single conversion.
>
> Signed-off-by: Dumitru Ceclan <[email protected]>
> ---
You could have this done in separate patches... Oh well, this is simple enough
that I don't care much.
Reviewed-by: Nuno Sa <[email protected]>
On 07/06/2024 12:02, Nuno Sá wrote:
> On Thu, 2024-06-06 at 19:07 +0300, Dumitru Ceclan via B4 Relay wrote:
>> From: Dumitru Ceclan <[email protected]>
>>
>> Sigma delta ADCs with a sequencer need to disable the previously enabled
>> channel when reading using ad_sigma_delta_single_conversion(). This was
>> done manually in drivers for devices with sequencers.
>>
>> This patch implements handling of single channel disabling after a
>> single conversion.
>>
>> Signed-off-by: Dumitru Ceclan <[email protected]>
>> ---
>
> You could have this done in separate patches... Oh well, this is simple enough
> that I don't care much.
>
> Reviewed-by: Nuno Sa <[email protected]>
>
Separate patches would break driver functionality then fix it.
The drivers would not probe as disable_one() callback is missing.
This would have been alright?
On Fri, 2024-06-07 at 12:29 +0300, Ceclan, Dumitru wrote:
> On 07/06/2024 12:02, Nuno Sá wrote:
> > On Thu, 2024-06-06 at 19:07 +0300, Dumitru Ceclan via B4 Relay wrote:
> > > From: Dumitru Ceclan <[email protected]>
> > >
> > > Sigma delta ADCs with a sequencer need to disable the previously enabled
> > > channel when reading using ad_sigma_delta_single_conversion(). This was
> > > done manually in drivers for devices with sequencers.
> > >
> > > This patch implements handling of single channel disabling after a
> > > single conversion.
> > >
> > > Signed-off-by: Dumitru Ceclan <[email protected]>
> > > ---
> >
> > You could have this done in separate patches... Oh well, this is simple
> > enough
> > that I don't care much.
> >
> > Reviewed-by: Nuno Sa <[email protected]>
> >
>
> Separate patches would break driver functionality then fix it.
> The drivers would not probe as disable_one() callback is missing.
>
> This would have been alright?
>
No, it's not... I mean, you could also only enforce the disable_one() presence
after updating all users but I agree this is simple enough to go in one patch.
- Nuno Sá