Message-ID: <nrffdb34ldh24kjphvebdnc2p466xbmay4pm6pe3nen2wftycv@uyxqjovmm726>
Date: Tue, 29 Apr 2025 15:49:02 +0200
From: Jorge Marques <gastmaier@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Jorge Marques <jorge.marques@...log.com>, 
	Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael Hennerich <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jonathan Corbet <corbet@....net>, Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, Uwe Kleine-König <ukleinek@...nel.org>, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2 4/5] docs: iio: new docs for ad4052 driver

On Fri, Apr 25, 2025 at 04:44:20PM -0500, David Lechner wrote:
> On 4/22/25 6:34 AM, Jorge Marques wrote:
> > This adds a new page to document how to use the ad4052 ADC driver.
> > 
> > Signed-off-by: Jorge Marques <jorge.marques@...log.com>
> > ---
> >  Documentation/iio/ad4052.rst | 95 ++++++++++++++++++++++++++++++++++++++++++++
> 
> Also need to update the table of contents in Documentation/iio/index.rst,
> otherwise this page won't be build (and will cause a build error).
> 
> You can run `make htmldocs SPHINXDIRS=iio` to speed things up and only build
> the iio directory to make sure you have it right.
> 
> More info: https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html
> 

Yes, sorry.

> >  MAINTAINERS                  |  1 +
> >  2 files changed, 96 insertions(+)
> > 
> > diff --git a/Documentation/iio/ad4052.rst b/Documentation/iio/ad4052.rst
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..410aaa437ed5fea6a2924d374fa5f816f5754e22
> > --- /dev/null
> > +++ b/Documentation/iio/ad4052.rst
> > @@ -0,0 +1,95 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +=============
> > +AD4052 driver
> > +=============
> > +
> > +ADC driver for Analog Devices Inc. AD4052 and similar devices.
> 
> Please don't put newline after every period. Here and throughout the document.
> It makes it harder to read.
> 

Ack, that was a personal touch that at the time was easier for me to
read ahah. Luckly, vim's `gqq` can flawlessly convert.

> > +The module name is ``ad4052``.
> > +
> > +Supported devices
> > +=================
> > +
> > +The following chips are supported by this driver:
> > +
> > +* `AD4050 <https://www.analog.com/AD4050>`_
> > +* `AD4052 <https://www.analog.com/AD4052>`_
> > +* `AD4056 <https://www.analog.com/AD4056>`_
> > +* `AD4058 <https://www.analog.com/AD4058>`_
> > +
> > +Wiring modes
> > +============
> > +
> > +The ADC uses SPI 4-wire mode, and contain two programmable GPIOs and
> > +a CNV pin.
> > +
> > +The CNV pin is exposed as the ``cnv-gpios`` and triggers a ADC conversion.
> > +GP1 is ADC conversion ready signal and GP0 Threshold event interrupt, both
> > +exposed as interrupts.
> > +
> > +Omit ``cnv-gpios`` and tie CNV and CS together to use the rising edge
> > +of the CS as the CNV signal.
> > +
> > +Device attributes
> > +=================
> > +
> > +The ADC contain only one channels, and the following attributes:
> > +
> > +.. list-table:: Driver attributes
> > +   :header-rows: 1
> > +
> > +   * - Attribute
> > +     - Description
> > +   * - ``in_voltage0_raw``
> > +     - Raw ADC voltage value
> 
> No scale attribute? How do we convert raw to millivolts?
> 

I will add here and to the driver.

> > +   * - ``in_voltage0_oversampling_ratio``
> > +     - Enable the device's burst averaging mode to over sample using
> > +       the internal sample rate.
> > +   * - ``in_voltage0_oversampling_ratio_available``
> > +     - List of available oversampling values. Value 0 disable the burst
> > +       averaging mode.
> 
> Typically 1 means no oversampling, not zero. (It is a ratio, divide by 1 is the
> same as doing nothing, but divide by 0 is undefined.)
> 

Outdated doc, fixed.

> > +   * - ``conversion_frequency``
> 
> Needs to be updated to ``oversampling_frequency``.
> 

Ack.

> > +     - Device internal sample rate used in the burst averaging mode.
> > +   * - ``conversion_frequency_available``
> > +     - List of available sample rates.
> > +
> > +Threshold events
> > +================
> > +
> > +The ADC supports a monitoring mode to raise threshold events.
> > +The driver supports a single interrupt for both rising and falling
> > +readings.
> > +
> > +The feature is enabled/disabled by setting ``thresh_either_en``.
> > +During monitor mode, the device continuously operates in autonomous mode until
> > +put back in configuration mode, due to this, the device returns busy until the
> > +feature is disabled.
> 
> Probably worth mentioning the ``events/sampling_frequency`` and
> ``sampling_frequency_available`` attributes since we've mentioned all of the
> other attributes.
> 

Ack.

> > +
> > +Low-power mode
> > +==============
> > +
> > +The device enters low-power mode on idle to save power.
> > +Enabling an event puts the device out of the low-power since the ADC
> > +autonomously samples to assert the event condition.
> > +
> > +SPI offload support
> > +===================
> > +
> > +To be able to achieve the maximum sample rate, the driver can be used with the
> > +`AXI SPI Engine`_ to provide SPI offload support.
> > +
> > +.. _AXI SPI Engine: http://analogdevicesinc.github.io/hdl/projects/ad4052_ardz/index.html
> > +
> > +When SPI offload is being used, additional attributes are present:
> > +
> > +.. list-table:: Additional attributes
> > +   :header-rows: 1
> > +
> > +   * - Attribute
> > +     - Description
> > +   * - ``in_voltage0_sampling_frequency``
> > +     - Set the sampling frequency.
> > +   * - ``in_voltage0_sampling_frequency_available``
> > +     - Get the sampling frequency range.
> 
> In the driver, this is currently info_mask_shared_by_type, so would be
> ``in_voltage_sampling_frequency``. And there currently isn't 
> ``in_voltage_sampling_frequency_available`` in the driver, so it needs to be
> added in the driver (or removed here).

Removed here, it is constrained by the PWM trigger.

> 
> > +
> > +The scan type is different when the buffer with offload support is enabled.
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 81fbe7176475c48eae03ab04115d4ef5b6299fac..04aa8db44bee418382a2e74cb6b1d03a810bd781 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1334,6 +1334,7 @@ M:	Jorge Marques <jorge.marques@...log.com>
> >  S:	Supported
> >  W:	https://ez.analog.com/linux-software-drivers
> >  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4052.yaml
> > +F:	Documentation/iio/ad4052.rst
> >  
> >  ANALOG DEVICES INC AD4130 DRIVER
> >  M:	Cosmin Tanislav <cosmin.tanislav@...log.com>
> > 
> 

Sorry about this submission, this file was indeed not updated between
version.

Regards,
Jorge

Powered by blists - more mailing lists