linux-kernel - Re: [PATCH v2 4/4] iio: adc: ad7405: add ad7405 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] [day] [month] [year] [list]

Message-ID: <20250511160233.66bd8e71@jic23-huawei>
Date: Sun, 11 May 2025 16:02:33 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Pop Ioan Daniel <pop.ioan-daniel@...log.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, "David Lechner" <dlechner@...libre.com>,
 Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko
 <andy@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Sergiu Cuciurean
 <sergiu.cuciurean@...log.com>, "Dragos Bogdan" <dragos.bogdan@...log.com>,
 Antoniu Miclaus <antoniu.miclaus@...log.com>, Olivier Moysan
 <olivier.moysan@...s.st.com>, Javier Carrasco
 <javier.carrasco.cruz@...il.com>, Matti Vaittinen
 <mazziesaccount@...il.com>, Tobias Sperling <tobias.sperling@...ting.com>,
 Alisa-Dariana Roman <alisadariana@...il.com>, Marcelo Schmitt
 <marcelo.schmitt@...log.com>, Matteo Martelli <matteomartelli3@...il.com>,
 <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] iio: adc: ad7405: add ad7405 driver

On Thu, 8 May 2025 15:30:57 +0300
Pop Ioan Daniel <pop.ioan-daniel@...log.com> wrote:

> Add support for the AD7405/ADUM770x, a high performance isolated ADC,
> 1-channel, 16-bit with a second-order Σ-Δ modulator that converts an
> analog input signal into a high speed, single-bit data stream.
> 
> Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@...log.com>
Hi,

Just a few comments inline.  In particular I'd avoid the gcc specific
behaviour unless we have it documented somewhere in the kernel that
static flexible array instantiation is allowed.

Doesn't make sense here anyway just make it 1 element.

Jonathan

> diff --git a/drivers/iio/adc/ad7405.c b/drivers/iio/adc/ad7405.c
> new file mode 100644
> index 000000000000..5fe36ce61819
> --- /dev/null
> +++ b/drivers/iio/adc/ad7405.c
> @@ -0,0 +1,264 @@

> +
> +struct ad7405_chip_info {
> +	const char *name;
> +	unsigned int max_rate;
> +	struct iio_chan_spec channel[];
See below.

> +};

> +static int ad7405_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long info)
> +{
> +	struct ad7405_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +			*vals = st->sample_frequency_tbl;
1 tab only.
> +			*length = ARRAY_SIZE(st->sample_frequency_tbl);
> +			*type = IIO_VAL_INT;
> +			return IIO_AVAIL_LIST;
> +	default:
> +			return -EINVAL;
> +	}
> +}

> +
> +static const struct ad7405_chip_info ad7405_chip_info = {
> +		.name = "AD7405",
> +		.channel = {
> +			AD7405_IIO_CHANNEL,
> +		},
> +};
> +
> +static const struct ad7405_chip_info adum7701_chip_info = {
> +		.name = "ADUM7701",

Convention here is 1 tab indent only.

> +		.channel = {
This will look nicer without the array of 1 thing going on.

Interesting corner of the c spec to use a flexible array member
for this. Definitely don't do that as I hate reading that spec to
check corners like this.  On this occasion I looked and could
not find an answer.  There is a gcc extension that makes this work
though.

> +			AD7405_IIO_CHANNEL,
> +		},
> +};