drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
xadc_postdisable() unconditionally sets the sequencer to continuous
mode. For dual external multiplexer configurations this is incorrect:
simultaneous sampling mode is required so that ADC-A samples through
the mux on VAUX[0-7] while ADC-B simultaneously samples through the
mux on VAUX[8-15]. In continuous mode only ADC-A is active, so
VAUX[8-15] channels return incorrect data.
Since postdisable is also called from xadc_probe() to set the initial
idle state, the wrong sequencer mode is active from the moment the
driver loads.
The preenable path already uses xadc_get_seq_mode() which returns
SIMULTANEOUS for dual mux. Fix postdisable to do the same.
Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Christofer Jonason <christofer.jonason@guidelinegeo.com>
---
Changes in v2:
- Align continuation line to opening parenthesis (Andy)
drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index e257c1b94..3980dfacb 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -817,6 +817,7 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
{
struct xadc *xadc = iio_priv(indio_dev);
unsigned long scan_mask;
+ int seq_mode;
int ret;
int i;
@@ -824,6 +825,12 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
for (i = 0; i < indio_dev->num_channels; i++)
scan_mask |= BIT(indio_dev->channels[i].scan_index);
+ /*
+ * Use the correct sequencer mode for the idle state: simultaneous
+ * mode for dual external mux configurations, continuous otherwise.
+ */
+ seq_mode = xadc_get_seq_mode(xadc, scan_mask);
+
/* Enable all channels and calibration */
ret = xadc_write_adc_reg(xadc, XADC_REG_SEQ(0), scan_mask & 0xffff);
if (ret)
@@ -834,11 +841,11 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
return ret;
ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_SEQ_MASK,
- XADC_CONF1_SEQ_CONTINUOUS);
+ seq_mode);
if (ret)
return ret;
- return xadc_power_adc_b(xadc, XADC_CONF1_SEQ_CONTINUOUS);
+ return xadc_power_adc_b(xadc, seq_mode);
}
static int xadc_preenable(struct iio_dev *indio_dev)
--
2.47.3
On Wed, 4 Mar 2026 10:07:27 +0100
Christofer Jonason <christofer.jonason@guidelinegeo.com> wrote:
> xadc_postdisable() unconditionally sets the sequencer to continuous
> mode. For dual external multiplexer configurations this is incorrect:
> simultaneous sampling mode is required so that ADC-A samples through
> the mux on VAUX[0-7] while ADC-B simultaneously samples through the
> mux on VAUX[8-15]. In continuous mode only ADC-A is active, so
> VAUX[8-15] channels return incorrect data.
>
> Since postdisable is also called from xadc_probe() to set the initial
> idle state, the wrong sequencer mode is active from the moment the
> driver loads.
>
> The preenable path already uses xadc_get_seq_mode() which returns
> SIMULTANEOUS for dual mux. Fix postdisable to do the same.
>
> Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christofer Jonason <christofer.jonason@guidelinegeo.com>
I'll leave this on list for a little longer as I'd really like a confirmation
of this one from the AMD Xilinx folk.
Thanks,
Jonathan
> ---
> Changes in v2:
> - Align continuation line to opening parenthesis (Andy)
> drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index e257c1b94..3980dfacb 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -817,6 +817,7 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> {
> struct xadc *xadc = iio_priv(indio_dev);
> unsigned long scan_mask;
> + int seq_mode;
> int ret;
> int i;
>
> @@ -824,6 +825,12 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> for (i = 0; i < indio_dev->num_channels; i++)
> scan_mask |= BIT(indio_dev->channels[i].scan_index);
>
> + /*
> + * Use the correct sequencer mode for the idle state: simultaneous
> + * mode for dual external mux configurations, continuous otherwise.
> + */
> + seq_mode = xadc_get_seq_mode(xadc, scan_mask);
> +
> /* Enable all channels and calibration */
> ret = xadc_write_adc_reg(xadc, XADC_REG_SEQ(0), scan_mask & 0xffff);
> if (ret)
> @@ -834,11 +841,11 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> return ret;
>
> ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_SEQ_MASK,
> - XADC_CONF1_SEQ_CONTINUOUS);
> + seq_mode);
> if (ret)
> return ret;
>
> - return xadc_power_adc_b(xadc, XADC_CONF1_SEQ_CONTINUOUS);
> + return xadc_power_adc_b(xadc, seq_mode);
> }
>
> static int xadc_preenable(struct iio_dev *indio_dev)
+Salih, Conall,
On 3/7/26 13:41, Jonathan Cameron wrote:
> On Wed, 4 Mar 2026 10:07:27 +0100
> Christofer Jonason <christofer.jonason@guidelinegeo.com> wrote:
>
>> xadc_postdisable() unconditionally sets the sequencer to continuous
>> mode. For dual external multiplexer configurations this is incorrect:
>> simultaneous sampling mode is required so that ADC-A samples through
>> the mux on VAUX[0-7] while ADC-B simultaneously samples through the
>> mux on VAUX[8-15]. In continuous mode only ADC-A is active, so
>> VAUX[8-15] channels return incorrect data.
>>
>> Since postdisable is also called from xadc_probe() to set the initial
>> idle state, the wrong sequencer mode is active from the moment the
>> driver loads.
>>
>> The preenable path already uses xadc_get_seq_mode() which returns
>> SIMULTANEOUS for dual mux. Fix postdisable to do the same.
>>
>> Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christofer Jonason <christofer.jonason@guidelinegeo.com>
>
> I'll leave this on list for a little longer as I'd really like a confirmation
> of this one from the AMD Xilinx folk.
Salih/Conall: Please look at this patch and provide your comment or tag.
Thanks,
Michal
On Wed, Mar 04, 2026 at 10:07:27AM +0100, Christofer Jonason wrote: > xadc_postdisable() unconditionally sets the sequencer to continuous > mode. For dual external multiplexer configurations this is incorrect: > simultaneous sampling mode is required so that ADC-A samples through > the mux on VAUX[0-7] while ADC-B simultaneously samples through the > mux on VAUX[8-15]. In continuous mode only ADC-A is active, so > VAUX[8-15] channels return incorrect data. > > Since postdisable is also called from xadc_probe() to set the initial > idle state, the wrong sequencer mode is active from the moment the > driver loads. > > The preenable path already uses xadc_get_seq_mode() which returns > SIMULTANEOUS for dual mux. Fix postdisable to do the same. Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko
On Wed, 2026-03-04 at 10:07 +0100, Christofer Jonason wrote:
> xadc_postdisable() unconditionally sets the sequencer to continuous
> mode. For dual external multiplexer configurations this is incorrect:
> simultaneous sampling mode is required so that ADC-A samples through
> the mux on VAUX[0-7] while ADC-B simultaneously samples through the
> mux on VAUX[8-15]. In continuous mode only ADC-A is active, so
> VAUX[8-15] channels return incorrect data.
>
> Since postdisable is also called from xadc_probe() to set the initial
> idle state, the wrong sequencer mode is active from the moment the
> driver loads.
>
> The preenable path already uses xadc_get_seq_mode() which returns
> SIMULTANEOUS for dual mux. Fix postdisable to do the same.
>
> Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christofer Jonason <christofer.jonason@guidelinegeo.com>
> ---
From a code standpoint:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Changes in v2:
> - Align continuation line to opening parenthesis (Andy)
> drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index e257c1b94..3980dfacb 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -817,6 +817,7 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> {
> struct xadc *xadc = iio_priv(indio_dev);
> unsigned long scan_mask;
> + int seq_mode;
> int ret;
> int i;
>
> @@ -824,6 +825,12 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> for (i = 0; i < indio_dev->num_channels; i++)
> scan_mask |= BIT(indio_dev->channels[i].scan_index);
>
> + /*
> + * Use the correct sequencer mode for the idle state: simultaneous
> + * mode for dual external mux configurations, continuous otherwise.
> + */
> + seq_mode = xadc_get_seq_mode(xadc, scan_mask);
> +
> /* Enable all channels and calibration */
> ret = xadc_write_adc_reg(xadc, XADC_REG_SEQ(0), scan_mask & 0xffff);
> if (ret)
> @@ -834,11 +841,11 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
> return ret;
>
> ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_SEQ_MASK,
> - XADC_CONF1_SEQ_CONTINUOUS);
> + seq_mode);
> if (ret)
> return ret;
>
> - return xadc_power_adc_b(xadc, XADC_CONF1_SEQ_CONTINUOUS);
> + return xadc_power_adc_b(xadc, seq_mode);
> }
>
> static int xadc_preenable(struct iio_dev *indio_dev)
© 2016 - 2026 Red Hat, Inc.