Received-SPF: pass (google.com: domain of linux-kernel+bounces-76658-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 22 Feb 2024 16:02:10 +0200
From: Andy Shevchenko <andy@kernel.org>
To: Dumitru Ceclan <mitrutzceclan@gmail.com>
Cc: linus.walleij@linaro.org, brgl@bgdev.pl, linux-gpio@vger.kernel.org,
	Lars-Peter Clausen <lars@metafoo.de>,
	Jonathan Cameron <jic23@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Michael Walle <michael@walle.cc>, Arnd Bergmann <arnd@arndb.de>,
	ChiaEn Wu <chiaen_wu@richtek.com>,
	Niklas Schnelle <schnelle@linux.ibm.com>,
	Leonard =?iso-8859-1?Q?G=F6hrs?= <l.goehrs@pengutronix.de>,
	Mike Looijmans <mike.looijmans@topic.nl>,
	Haibo Chen <haibo.chen@nxp.com>,
	Hugo Villeneuve <hvilleneuve@dimonoff.com>,
	David Lechner <dlechner@baylibre.com>,
	Ceclan Dumitru <dumitru.ceclan@analog.com>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Nuno Sa <nuno.sa@analog.com>
Subject: Re: [PATCH v14 3/3] iio: adc: ad7173: add AD7173 driver
Message-ID: <ZddT4mmxf3W55ea8@smile.fi.intel.com>
References: <20240222110817.29670-1-mitrutzceclan@gmail.com>
 <20240222110817.29670-3-mitrutzceclan@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240222110817.29670-3-mitrutzceclan@gmail.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Thu, Feb 22, 2024 at 01:07:43PM +0200, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel
> applications or higher speed multiplexed applications. The Sigma-Delta
> ADC is intended primarily for measurement of signals close to DC but also
> delivers outstanding performance with input bandwidths out to ~10kHz.

A couple more comments which Jonathan might address when applying,
up to him.

..

With

	struct device *dev = &st->sd.spi->dev;

the below will be neater.


> +	struct ad7173_state *st = iio_priv(indio_dev);
> +	u8 buf[AD7173_RESET_LENGTH];
> +	unsigned int id;
> +	int ret;

..

> +	if (id != st->info->id)
> +		dev_warn(&st->sd.spi->dev,
> +			 "Unexpected device id: 0x%04X, expected: 0x%04X\n",
> +			 id, st->info->id);

(like here)

..

> +	st->config_usage_counter = 0;
> +	st->config_cnts = devm_kcalloc(&st->sd.spi->dev, st->info->num_configs,
> +				       sizeof(u64), GFP_KERNEL);

sizeof(*st->config_cnts) ?

(or here)

> +	if (!st->config_cnts)
> +		return -ENOMEM;

..

> +	ret = fwnode_property_match_property_string(dev_fwnode(dev),


device_property_match_property_string()

> +						    "clock-names",
> +						    ad7173_clk_sel,
> +						    ARRAY_SIZE(ad7173_clk_sel));

..

> +	if (num_channels == 0)
> +		return dev_err_probe(dev, -EINVAL, "No channels specified\n");

-ENODATA?

-- 
With Best Regards,
Andy Shevchenko