[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zn2c531lfpBErYKb@debian-BULLSEYE-live-builder-AMD64>
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á
> >
Powered by blists - more mailing lists