linux-kernel - Re: [PATCH v5 6/7] iio: adc: Add support for AD4000

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]

Date: Thu, 27 Jun 2024 14:09:59 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, broonie@...nel.org,
	lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
	robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
	conor+dt@...nel.org, nuno.sa@...log.com, dlechner@...libre.com,
	corbet@....net, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-spi@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 6/7] iio: adc: Add support for AD4000

On 06/26, Nuno Sá wrote:
> On Wed, 2024-06-26 at 10:17 -0300, Marcelo Schmitt wrote:
> > On 06/26, Nuno Sá wrote:
> > > On Tue, 2024-06-25 at 18:55 -0300, Marcelo Schmitt wrote:
> > > > Add support for AD4000 series of low noise, low power, high speed,
> > > > successive approximation register (SAR) ADCs.
> > > > 
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> > > > ---
...
> > > > +	ret = devm_regulator_get_enable(dev, "vdd");
> > > > +	if (ret)
> > > > +		return dev_err_probe(dev, ret, "Failed to enable VDD
> > > > supply\n");
> > > > +
> > > > +	ret = devm_regulator_get_enable(dev, "vio");
> > > > +	if (ret)
> > > > +		return dev_err_probe(dev, ret, "Failed to enable VIO
> > > > supply\n");
> > > 
> > > devm_regulator_bulk_get_enable()? Do we have any ordering constrains?
> > 
> > No ordering constraints, but vdd and vio are optional while ref is required
> > and
> > we need to get the voltage of ref.
> > devm_regulator_bulk_get_enable_read_voltage()? and discard vdd and vio
> > voltages?
> 
> Hmmm, vdd and vio do not look like optional to me :). Anyways I meant
> devm_regulator_bulk_get_enable() only for vdd and vio and still treat ref
> separately.
> 

I've mistaken these supplies with supplies for a different device.
Yes, vdd and vio are required and devm_regulator_bulk_get_enable() is useful
to init them.

> > 
> > > 
> > > > +
...
> > > > +		/*
> > > > +		 * In "3-wire mode", the ADC SDI line must be kept high
> > > > when
> > > > +		 * data is not being clocked out of the controller.
> > > > +		 * Request the SPI controller to make MOSI idle high.
> > > > +		 */
> > > > +		spi->mode |= SPI_MOSI_IDLE_HIGH;
> > > > +		ret = spi_setup(spi);
> > > > +		if (ret < 0)
> > > > +			return ret;
> > > > +
> > > > +		ret = ad4000_prepare_3wire_mode_message(st, indio_dev-
> > > > > channels);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +
> > > > +		ret = ad4000_config(st);
> > > > +		if (ret < 0)
> > > > +			dev_warn(dev, "Failed to config device\n");
> > > > +
> > > 
> > > Should this be a warning? Very suspicious :)
> > 
> > This devices have some many possible wiring configurations.
> > I didn't want to fail just because reg access fail.
> > Maybe ADC SDI was wired to VIO but dt don't have adi,sdi-pin = "high".
> > Reg access will fail but sample read should work.
> 
> Well, to me that really is a configuration failure and we should treat it as
> such. If we are in the so called "reg_access_info" which I read as "we can
> access registers", failing to do so should be treated as an error. 
> 
> So, setting scale would also fail and we then have a broken interface :)

Drat, that's right. 
Okay, will make probe fail if config fails.

Thanks,
Marcelo

> 
> - Nuno Sá
> >