Re: [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 19 Jan 2022 19:02:46 -0600
Robert Hancock <robert.hancock@calian.com> wrote:

> Some of the AMS channels need to be read by switching into single-channel
> mode from the normal polling sequence. There was a logic issue in this
> switching code that could cause the first read of these channels to read
> back as zero.
> 
> It appears that the sequencer should be set back to default mode before
> changing the channel selection, and the channel should be set before
> switching the sequencer back into single-channel mode.
> 
> Also, write 1 to the EOC bit in the status register to clear it before
> waiting for it to become set, so that we actually wait for a new
> conversion to complete, and don't proceed based on a previous conversion
> completing.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Looking for an Ack from Anand or someone else familiar with this device.

Thanks,

Jonathan


> ---
>  drivers/iio/adc/xilinx-ams.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index 199027c93cdc..7bf097fa10cb 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -530,14 +530,18 @@ static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
>  		return -EINVAL;
>  	}
>  
> -	/* set single channel, sequencer off mode */
> +	/* put sysmon in a soft reset to change the sequence */
>  	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> -			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
> +			  AMS_CONF1_SEQ_DEFAULT);
>  
>  	/* write the channel number */
>  	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
>  			  channel_num);
>  
> +	/* set single channel, sequencer off mode */
> +	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
> +
>  	return 0;
>  }
>  
> @@ -551,6 +555,8 @@ static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
>  	if (ret)
>  		return ret;
>  
> +	/* clear end-of-conversion flag, wait for next conversion to complete */
> +	writel(expect, ams->base + AMS_ISR_1);
>  	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, (reg & expect),
>  				 AMS_INIT_POLL_TIME_US, AMS_INIT_TIMEOUT_US);
>  	if (ret)