Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 22 Jun 2019 09:37:11 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Mircea Caprioru <mircea.caprioru@analog.com>
Cc:     <Michael.Hennerich@analog.com>, <stefan.popa@analog.com>,
        <lars@metafoo.de>, <gregkh@linuxfoundation.org>,
        <linux-kernel@vger.kernel.org>, <linux-iio@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <robh+dt@kernel.org>
Subject: Re: [PATCH V2 2/4] iio: adc: ad7124: Add buffered input support
Message-ID: <20190622093654.61e31fc5@archlinux>
In-Reply-To: <20190621121344.24917-2-mircea.caprioru@analog.com>
References: <20190621121344.24917-1-mircea.caprioru@analog.com>
        <20190621121344.24917-2-mircea.caprioru@analog.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 21 Jun 2019 15:13:42 +0300
Mircea Caprioru <mircea.caprioru@analog.com> wrote:

> This patch adds the option to enable the buffered mode for positive and
> negative inputs. Each option can be enabled independently.
> 
> In buffered mode, the input channel feeds into a high impedance input stage
> of the buffer amplifier. Therefore, the input can tolerate significant
> source impedances and is tailored for direct connection to external
> resistive type sensors such as strain gages or RTDs.
> 
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
Hi Mircea,

This (and the other 2) looks good to me, but I won't
pick them up until the binding patch is split up.  That
way we won't end up with the driver being out of sync with
the binding docs, even for a brief time.

Thanks,

Jonathan

> ---
> 
> Changelog v2:
> - nothing changed here
> 
>  drivers/iio/adc/ad7124.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 810234db9c0d..ab52c5e9ecb1 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -61,6 +61,8 @@
>  #define AD7124_CONFIG_REF_SEL(x)	FIELD_PREP(AD7124_CONFIG_REF_SEL_MSK, x)
>  #define AD7124_CONFIG_PGA_MSK		GENMASK(2, 0)
>  #define AD7124_CONFIG_PGA(x)		FIELD_PREP(AD7124_CONFIG_PGA_MSK, x)
> +#define AD7124_CONFIG_IN_BUFF_MSK	GENMASK(7, 6)
> +#define AD7124_CONFIG_IN_BUFF(x)	FIELD_PREP(AD7124_CONFIG_IN_BUFF_MSK, x)
>  
>  /* AD7124_FILTER_X */
>  #define AD7124_FILTER_FS_MSK		GENMASK(10, 0)
> @@ -108,6 +110,8 @@ struct ad7124_chip_info {
>  struct ad7124_channel_config {
>  	enum ad7124_ref_sel refsel;
>  	bool bipolar;
> +	bool buf_positive;
> +	bool buf_negative;
>  	unsigned int ain;
>  	unsigned int vref_mv;
>  	unsigned int pga_bits;
> @@ -473,6 +477,11 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
>  		else
>  			st->channel_config[channel].refsel = tmp;
>  
> +		st->channel_config[channel].buf_positive =
> +			of_property_read_bool(child, "adi,buffered-positive");
> +		st->channel_config[channel].buf_negative =
> +			of_property_read_bool(child, "adi,buffered-negative");
> +
>  		*chan = ad7124_channel_template;
>  		chan->address = channel;
>  		chan->scan_index = channel;
> @@ -492,7 +501,7 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
>  static int ad7124_setup(struct ad7124_state *st)
>  {
>  	unsigned int val, fclk, power_mode;
> -	int i, ret;
> +	int i, ret, tmp;
>  
>  	fclk = clk_get_rate(st->mclk);
>  	if (!fclk)
> @@ -525,8 +534,12 @@ static int ad7124_setup(struct ad7124_state *st)
>  		if (ret < 0)
>  			return ret;
>  
> +		tmp = (st->channel_config[i].buf_positive << 1)  +
> +			st->channel_config[i].buf_negative;
> +
>  		val = AD7124_CONFIG_BIPOLAR(st->channel_config[i].bipolar) |
> -		      AD7124_CONFIG_REF_SEL(st->channel_config[i].refsel);
> +		      AD7124_CONFIG_REF_SEL(st->channel_config[i].refsel) |
> +		      AD7124_CONFIG_IN_BUFF(tmp);
>  		ret = ad_sd_write_reg(&st->sd, AD7124_CONFIG(i), 2, val);
>  		if (ret < 0)
>  			return ret;