linux-kernel - Re: [PATCH v2] dt-bindings:iio:adc:st,spear600-adc: txt to yaml format conversion.

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: <20250506-equivocal-snooper-8a7d1ce931c8@spud>
Date: Tue, 6 May 2025 13:50:29 +0100
From: Conor Dooley <conor@...nel.org>
To: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org, jic23@...nel.org,
	krzk+dt@...nel.org, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, robh@...nel.org,
	~lkcamp/patches@...ts.sr.ht
Subject: Re: [PATCH v2] dt-bindings:iio:adc:st,spear600-adc: txt to yaml
 format conversion.

On Sat, May 03, 2025 at 03:44:12PM -0300, Rodrigo Gobbi wrote:
> > Is 0 the default here or 1? "Single data conversion" sounds more like 1
> > sample than 0, and the default of 0 is below the minimum of 1. What's
> > going on there?
> 
> Good point, after I`ve submitted the patch I was double checking it and noticed 
> that too. It`s stange because the public datasheet mentions "Programmable averaging of results 
> from 1 (No averaging) up to 128". Meanwhile, the spear_adc.c driver at probe
> stated the following:
> 
> 	/*
> 	 * Optional avg_samples defaults to 0, resulting in single data
> 	 * conversion
> 	 */
> 	device_property_read_u32(dev, "average-samples", &st->avg_samples);
> 
> Since avg_samples is inside 
> 
> 	struct spear_adc_state *st;
> 
> which is allocated with devm_iio_device_alloc() (which uses the kzalloc/zero filling the priv data):
> 
> 	indio_dev = devm_iio_device_alloc(dev, sizeof(struct spear_adc_state));
> 	if (!indio_dev)
> 		return dev_err_probe(dev, -ENOMEM,
> 				     "failed allocating iio device\n");
> 
> 	st = iio_priv(indio_dev);
> 
> ...matches the driver comment meaning the default is actually "0", single data, but it does
> not match the public datasheet in my understanding. Since I don`t have access to a more
> detailed datasheet, I chose to describe "1" as a minimum value, but I agree it is weird.
> Maybe we could drop the minimum constraint in this case (go with default and max)?
> Tks and regards.

Sounds like it's a 4-bit register where the samples is (1 + written value),
and the property is expected to be written directly to the register.
I'd then expect the property to be min 0, default 0, max 127. If you
write 128 to the register, you'll accidentally set the external vref
bit. I'd maybe go as far as &ing the value to make sure out of range
stuff is not permitted?

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)