Received-SPF: pass (google.com: domain of linux-kernel+bounces-183167-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sun, 19 May 2024 18:13:49 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Dumitru Ceclan via B4 Relay
 <devnull+dumitru.ceclan.analog.com@kernel.org>
Cc: dumitru.ceclan@analog.com, Lars-Peter Clausen <lars@metafoo.de>, Michael
 Hennerich <Michael.Hennerich@analog.com>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, David Lechner <dlechner@baylibre.com>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, Dumitru Ceclan <mitrutzceclan@gmail.com>
Subject: Re: [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices
Message-ID: <20240519181349.0209271c@jic23-huawei>
In-Reply-To: <20240514-ad4111-v2-8-29be6a55efb5@analog.com>
References: <20240514-ad4111-v2-0-29be6a55efb5@analog.com>
	<20240514-ad4111-v2-8-29be6a55efb5@analog.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 14 May 2024 10:22:53 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:

> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> 
> Add support for AD4111/AD4112/AD4114/AD4115/AD4116.
> 
> The AD411X family encompasses a series of low power, low noise, 24-bit,
> sigma-delta analog-to-digital converters that offer a versatile range of
> specifications.
> 
> This family of ADCs integrates an analog front end suitable for processing
> both fully differential and single-ended, bipolar voltage inputs
> addressing a wide array of industrial and instrumentation requirements.
> 
> - All ADCs have inputs with a precision voltage divider with a division
>   ratio of 10.
> - AD4116 has 5 low level inputs without a voltage divider.
> - AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
>   shunt resistor.
> 
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
A couple of trivial things inline. Otherwise lgtm

> @@ -1058,17 +1296,26 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan->scan_index = chan_index;
>  		chan->channel = ain[0];
>  		chan->channel2 = ain[1];
> -		chan->differential = true;
> -
>  		chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
>  		chan_st_priv->chan_reg = chan_index;
> -		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
>  		chan_st_priv->cfg.odr = 0;
> -
>  		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
> +
>  		if (chan_st_priv->cfg.bipolar)
>  			chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
>  
> +		if (is_current_chan)
> +			chan->type = IIO_CURRENT;
> +		else
> +			chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> +
> +		ret = fwnode_property_match_property_string(child,
> +							    "adi,channel-type",
> +							    ad7173_channel_types,
> +							    ARRAY_SIZE(ad7173_channel_types));
> +		chan->differential = (ret < 0 || ret == AD7173_CHAN_DIFFERENTIAL)
> +					? true : false;

That's not a boolean. So better to set 1 or 0.


> +MODULE_DESCRIPTION("Analog Devices AD717x and AD411x ADC driver");

I'm scared of wildcards even in descriptive fields as they often end up covering
something unintended. I'd just go with 'and similar'.

>  MODULE_LICENSE("GPL");
>