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: <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)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ