linux-kernel - Re: [PATCH v12 2/2] iio: adc: ad7173: add AD7173 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240121155041.3fc1a85d@jic23-huawei>
Date: Sun, 21 Jan 2024 15:50:41 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Dumitru Ceclan <mitrutzceclan@...il.com>
Cc: linus.walleij@...aro.org, brgl@...ev.pl, andy@...nel.org,
 linux-gpio@...r.kernel.org, Lars-Peter Clausen <lars@...afoo.de>, Rob
 Herring <robh+dt@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
 Michael Walle <michael@...le.cc>, Andy Shevchenko
 <andy.shevchenko@...il.com>, Arnd Bergmann <arnd@...db.de>, ChiaEn Wu
 <chiaen_wu@...htek.com>, Niklas Schnelle <schnelle@...ux.ibm.com>, Leonard
 Göhrs <l.goehrs@...gutronix.de>, Mike Looijmans
 <mike.looijmans@...ic.nl>, Haibo Chen <haibo.chen@....com>, Hugo Villeneuve
 <hvilleneuve@...onoff.com>, David Lechner <dlechner@...libre.com>, Ceclan
 Dumitru <dumitru.ceclan@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v12 2/2] iio: adc: ad7173: add AD7173 driver

On Thu, 18 Jan 2024 14:49:23 +0200
Dumitru Ceclan <mitrutzceclan@...il.com> 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.
> 
> Reviewed-by: Andy Shevchenko <andy@...nel.org>
> Reviewed-by: Michael Walle <michael@...le.cc> # for gpio-regmap
> Signed-off-by: Dumitru Ceclan <mitrutzceclan@...il.com>

One comment following through from adding the named irq support to the
binding.  The driver needs to check that it has the right one
as perhaps only the error one is wired, or it is the first one specified
in the binding.


> +static int ad7173_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct ad7173_state *st;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return -ENODEV;
> +
> +	ida_init(&st->cfg_slots_status);
> +	ret = devm_add_action_or_reset(dev, ad7173_ida_destroy, st);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->name = st->info->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ad7173_info;
> +
> +	spi->mode = SPI_MODE_3;
> +
> +	ad7173_sigma_delta_info.num_slots = st->info->num_configs;
> +	ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7173_fw_parse_channel_config(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_ad_sd_setup_buffer_and_trigger(dev, indio_dev);

If the error interrupt is provided either first, or as the only interrupt
this is going to use the wrong one.

Probably need to have a variant of that which takes an explicit irq so that
figuring out which irq is relevant becomes a driver problem rather than the
library having a go based on spi->irq.


> +	if (ret)
> +		return ret;
> +
> +	ret = ad7173_setup(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	if (IS_ENABLED(CONFIG_GPIOLIB))
> +		return ad7173_gpio_init(st);
> +
> +	return 0;
> +}