Re: [PATCH 3/5] iio: xilinx-xadc: Fix sequencer configuration for aux channels in simultaneous mode

[Jonathan Cameron <jic23@kernel.org>]

On Fri,  3 Apr 2020 15:27:15 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The XADC has two internal ADCs. Depending on the mode it is operating in
> either one or both of them are used. The device manual calls this
> continuous (one ADC) and simultaneous (both ADCs) mode.
> 
> The meaning of the sequencing register for the aux channels changes
> depending on the mode.
> 
> In continuous mode each bit corresponds to one of the 16 aux channels. And
> the single ADC will convert them one by one in order.
> 
> In simultaneous mode the aux channels are split into two groups the first 8
> channels are assigned to the first ADC and the other 8 channels to the
> second ADC. The upper 8 bits of the sequencing register are unused and the
> lower 8 bits control both ADCs. This means a bit needs to be set if either
> the corresponding channel from the first group or the second group (or
> both) are set.
> 
> Currently the driver does not have the special handling required for
> simultaneous mode. Add it.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Looks fine, but sounds like a fix, so when was the problem introduced?

Thanks,

Jonathan

> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index 04a2a609ced4..4acababda4d5 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -798,6 +798,16 @@ static int xadc_preenable(struct iio_dev *indio_dev)
>  	if (ret)
>  		goto err;
>  
> +	/*
> +	 * In simultaneous mode the upper and lower aux channels are samples at
> +	 * the same time. In this mode the upper 8 bits in the sequencer
> +	 * register are don't care and the lower 8 bits control two channels
> +	 * each. As such we must set the bit if either the channel in the lower
> +	 * group or the upper group is enabled.
> +	 */
> +	if (seq_mode == XADC_CONF1_SEQ_SIMULTANEOUS)
> +		scan_mask = ((scan_mask >> 8) | scan_mask) & 0xff0000;
> +
>  	ret = xadc_write_adc_reg(xadc, XADC_REG_SEQ(1), scan_mask >> 16);
>  	if (ret)
>  		goto err;