diff mbox series

[3/4] docs: iio: new docs for ad4052 driver

Message ID 20250306-iio-driver-ad4052-v1-3-2badad30116c@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for AD4052 device family | expand

Commit Message

Jorge Marques March 6, 2025, 2:03 p.m. UTC
This adds a new page to document how to use the ad4052 ADC driver.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 Documentation/iio/ad4052.rst | 93 ++++++++++++++++++++++++++++++++++++++++++++
 MAINTAINERS                  |  1 +
 2 files changed, 94 insertions(+)

Comments

David Lechner March 7, 2025, 10:52 a.m. UTC | #1
On Thu, Mar 6, 2025 at 3:04 PM Jorge Marques <jorge.marques@analog.com> wrote:
>
> This adds a new page to document how to use the ad4052 ADC driver.
>
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  Documentation/iio/ad4052.rst | 93 ++++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS                  |  1 +
>  2 files changed, 94 insertions(+)
>
> diff --git a/Documentation/iio/ad4052.rst b/Documentation/iio/ad4052.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..cf0cbd60d0a48ea52f74ea02fde659fcdba61aa1
> --- /dev/null
> +++ b/Documentation/iio/ad4052.rst
> @@ -0,0 +1,93 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +AD4052 driver
> +=============
> +
> +ADC driver for Analog Devices Inc. AD4052 and similar devices.
> +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
> +   * - ``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.
> +   * - ``sample_rate``
> +     - Device internal sample rate used in the burst averaging mode.
> +   * - ``sample_rate_available``
> +     - List of available sample rates.

Why not using the standard sampling_frequency[_available] attributes?

> +
> +Threshold events
> +================
> +
> +The ADC supports a monitoring mode to raise threshold events.
> +The driver supports a single interrupt for both rising and falling
> +readings.
> +
> +During monitor mode, the device is busy since other transactions
> +require to put the device in configuration mode first.

This isn't so clear to me. Is this saying that events do not work
while doing a buffered read? Do you need to do need to read the
in_voltage0_raw input to trigger an event?

> +
> +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

This diagram show a PWM connected to the CNV pin on the ADC, but I
didn't see a pwms property in the DT bindings to describe this.

> +
> +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.
> +
> +The scan type is different when the buffer with offload support is enabled.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fef8adaee888d59e1aa3b3592dda5a8bea0b7677..312b2cf94b8f06298b1cbe5975ee32e2cf9a74d8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1322,6 +1322,7 @@ M:        Jorge Marques <jorge.marques@analog.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@analog.com>
>
> --
> 2.48.1
>

I didn't have time to read the full datasheet or look at the driver
code yet, but can do that next week.
Jorge Marques March 9, 2025, 8:49 p.m. UTC | #2
> > +.. list-table:: Driver attributes
> > +   :header-rows: 1
> > +
> > +   * - Attribute
> > +     - Description
> > +   * - ``in_voltage0_raw``
> > +     - Raw ADC voltage value
> > +   * - ``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.
> > +   * - ``sample_rate``
> > +     - Device internal sample rate used in the burst averaging mode.
> > +   * - ``sample_rate_available``
> > +     - List of available sample rates.
> 
> Why not using the standard sampling_frequency[_available] attributes?
Because sampling_frequency is the sampling frequency for the pwm trigger
during buffer readings.
sample_rate is the internal device clock used during monitor and burst
averaging modes.

> > +
> > +Threshold events
> > +================
> > +
> > +The ADC supports a monitoring mode to raise threshold events.
> > +The driver supports a single interrupt for both rising and falling
> > +readings.
> > +
> > +During monitor mode, the device is busy since other transactions
> > +require to put the device in configuration mode first.
> 
> This isn't so clear to me. Is this saying that events do not work
> while doing a buffered read? Do you need to do need to read the
> in_voltage0_raw input to trigger an event?
> 
No, the device monitor mode and trigger mode autonomously samples using the
internal clock set with the sample rate property.
I rephrased that to:

 The feature is enabled/disabled by setting ``thresh_either_en``.
 During monitor mode, the device continuously operate in autonomous mode until
 put back in configuration mode, due to this, the device returns busy until the
 feature is disabled.

The reasoning is that during configuration mode no ADC
conversion is done, including if the previous mode was autonomous.
If instead of return busy the driver hided this and resumed monitor mode
after the access, a hidden (to the user) monitoring down-time would and
thresholds crossings could be lost, undermining the feature.

> > +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
> 
> This diagram show a PWM connected to the CNV pin on the ADC, but I
> didn't see a pwms property in the DT bindings to describe this.
> 
It is not clear to me where the pwm property should be in the DT
bindings, since the PWM node now belongs to the offload-capable SPI controller.

> I didn't have time to read the full datasheet or look at the driver
> code yet, but can do that next week.
Ok, thank you for the review

Jorge
David Lechner March 10, 2025, 2:31 p.m. UTC | #3
On 3/9/25 3:49 PM, Jorge Marques wrote:
>>> +.. list-table:: Driver attributes
>>> +   :header-rows: 1
>>> +
>>> +   * - Attribute
>>> +     - Description
>>> +   * - ``in_voltage0_raw``
>>> +     - Raw ADC voltage value
>>> +   * - ``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.
>>> +   * - ``sample_rate``
>>> +     - Device internal sample rate used in the burst averaging mode.
>>> +   * - ``sample_rate_available``
>>> +     - List of available sample rates.
>>
>> Why not using the standard sampling_frequency[_available] attributes?
> Because sampling_frequency is the sampling frequency for the pwm trigger
> during buffer readings.
> sample_rate is the internal device clock used during monitor and burst
> averaging modes.

I haven't done a chips with a monitor mode yet where we aren't reading
the samples, so hopefully Jonathan will chime in here on the usual way
to handle that.

For the burst averaging mode, I understand the need for a separate attribute
now. I would suggest to call this the conversion_frequency rather than
sampling_rate since IIO already defines "sampling" to be the data read
from the chip to Linux even if it is an averaged value, it still counts
as one sample.

> 
>>> +
>>> +Threshold events
>>> +================
>>> +
>>> +The ADC supports a monitoring mode to raise threshold events.
>>> +The driver supports a single interrupt for both rising and falling
>>> +readings.
>>> +
>>> +During monitor mode, the device is busy since other transactions
>>> +require to put the device in configuration mode first.
>>
>> This isn't so clear to me. Is this saying that events do not work
>> while doing a buffered read? Do you need to do need to read the
>> in_voltage0_raw input to trigger an event?
>>
> No, the device monitor mode and trigger mode autonomously samples using the
> internal clock set with the sample rate property.
> I rephrased that to:
> 
>  The feature is enabled/disabled by setting ``thresh_either_en``.
>  During monitor mode, the device continuously operate in autonomous mode until
>  put back in configuration mode, due to this, the device returns busy until the
>  feature is disabled.

This is better, thanks.

> 
> The reasoning is that during configuration mode no ADC
> conversion is done, including if the previous mode was autonomous.
> If instead of return busy the driver hided this and resumed monitor mode
> after the access, a hidden (to the user) monitoring down-time would and
> thresholds crossings could be lost, undermining the feature.
> 
>>> +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
>>
>> This diagram show a PWM connected to the CNV pin on the ADC, but I
>> didn't see a pwms property in the DT bindings to describe this.
>>
> It is not clear to me where the pwm property should be in the DT
> bindings, since the PWM node now belongs to the offload-capable SPI controller.

If the PWM output is connected to the CNV pin of the ADC, then the PWM
consumer property belongs in the ADC devicetree node.

If the PWM output is connected directly to the SPI offload trigger input
then then it should use the trigger-sources bindings in the SPI controller.

From the diagram in the link you gave, it seems clear to me that the
PWM is connected to the CNV pin on the ADC and GP1 is connected to the
SPI offload trigger input on the SPI controller.

So the ADC node gets pwms and #trigger-source-cells properties and the
SPI controller gets a trigger-sources property that is a phandle to the
ADC node.

> 
>> I didn't have time to read the full datasheet or look at the driver
>> code yet, but can do that next week.
> Ok, thank you for the review
> 
> Jorge
Jonathan Cameron March 10, 2025, 7:54 p.m. UTC | #4
On Sun, 9 Mar 2025 21:49:24 +0100
Jorge Marques <gastmaier@gmail.com> wrote:

> > > +.. list-table:: Driver attributes
> > > +   :header-rows: 1
> > > +
> > > +   * - Attribute
> > > +     - Description
> > > +   * - ``in_voltage0_raw``
> > > +     - Raw ADC voltage value
> > > +   * - ``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.
> > > +   * - ``sample_rate``
> > > +     - Device internal sample rate used in the burst averaging mode.
> > > +   * - ``sample_rate_available``
> > > +     - List of available sample rates.  
> > 
> > Why not using the standard sampling_frequency[_available] attributes?  
> Because sampling_frequency is the sampling frequency for the pwm trigger
> during buffer readings.
> sample_rate is the internal device clock used during monitor and burst
> averaging modes.

For an ABI that is very vague and the two use cases seem to be logically
quite different.

Seems that for each trigger we have an oversampling ratio controlled number
of samples at this rate. It is unusual to be able to control oversampling
rate separately from the trigger clock, hence the lack of ABI.  If
we add something new for this it should something relating to oversampling.
oversampling_frequency perhaps.

For monitor mode, it is tied to the sampling frequency for most devices.
But there are exceptions.  E.g. the max1363. Trick is to make it an event
ABI property and hence under events/ rather than in the root directory.

In this case you'll have to store two values and write the appropriate
one into the register to suit a given operating mode.


> 
> > > +
> > > +Threshold events
> > > +================
> > > +
> > > +The ADC supports a monitoring mode to raise threshold events.
> > > +The driver supports a single interrupt for both rising and falling
> > > +readings.
> > > +
> > > +During monitor mode, the device is busy since other transactions
> > > +require to put the device in configuration mode first.  
> > 
> > This isn't so clear to me. Is this saying that events do not work
> > while doing a buffered read? Do you need to do need to read the
> > in_voltage0_raw input to trigger an event?
> >   
> No, the device monitor mode and trigger mode autonomously samples using the
> internal clock set with the sample rate property.
> I rephrased that to:
> 
>  The feature is enabled/disabled by setting ``thresh_either_en``.
>  During monitor mode, the device continuously operate in autonomous mode until
>  put back in configuration mode, due to this, the device returns busy until the
>  feature is disabled.
> 
> The reasoning is that during configuration mode no ADC
> conversion is done, including if the previous mode was autonomous.
> If instead of return busy the driver hided this and resumed monitor mode
> after the access, a hidden (to the user) monitoring down-time would and
> thresholds crossings could be lost, undermining the feature.

hmm. This is a trade off between usability and precise matching of expectations.
From your description does monitor mode only trigger if the threshold is
crossed or is it a level comparison?  If it's level I'd consider the
option of brief disabling.  Unlikely to be a problem interrupting things
in vast majority of usecases. Documentation can then express this issue.

> 
> > > +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  
> > 
> > This diagram show a PWM connected to the CNV pin on the ADC, but I
> > didn't see a pwms property in the DT bindings to describe this.
> >   
> It is not clear to me where the pwm property should be in the DT
> bindings, since the PWM node now belongs to the offload-capable SPI controller.
> 
> > I didn't have time to read the full datasheet or look at the driver
> > code yet, but can do that next week.  
> Ok, thank you for the review
> 
> Jorge
Jonathan Cameron March 10, 2025, 7:56 p.m. UTC | #5
On Mon, 10 Mar 2025 09:31:45 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/9/25 3:49 PM, Jorge Marques wrote:
> >>> +.. list-table:: Driver attributes
> >>> +   :header-rows: 1
> >>> +
> >>> +   * - Attribute
> >>> +     - Description
> >>> +   * - ``in_voltage0_raw``
> >>> +     - Raw ADC voltage value
> >>> +   * - ``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.
> >>> +   * - ``sample_rate``
> >>> +     - Device internal sample rate used in the burst averaging mode.
> >>> +   * - ``sample_rate_available``
> >>> +     - List of available sample rates.  
> >>
> >> Why not using the standard sampling_frequency[_available] attributes?  
> > Because sampling_frequency is the sampling frequency for the pwm trigger
> > during buffer readings.
> > sample_rate is the internal device clock used during monitor and burst
> > averaging modes.  
> 
> I haven't done a chips with a monitor mode yet where we aren't reading
> the samples, so hopefully Jonathan will chime in here on the usual way
> to handle that.
> 
> For the burst averaging mode, I understand the need for a separate attribute
> now. I would suggest to call this the conversion_frequency rather than
> sampling_rate since IIO already defines "sampling" to be the data read
> from the chip to Linux even if it is an averaged value, it still counts
> as one sample.

I should have read on.  I'd like this more closely associated with oversampling.
As per other reply we use sampling_frequency in the events directory for
the monitoring frequency case.  One of our very first drivers did this
(max1363) so it's been in the ABI a long time!

Jonathan
diff mbox series

Patch

diff --git a/Documentation/iio/ad4052.rst b/Documentation/iio/ad4052.rst
new file mode 100644
index 0000000000000000000000000000000000000000..cf0cbd60d0a48ea52f74ea02fde659fcdba61aa1
--- /dev/null
+++ b/Documentation/iio/ad4052.rst
@@ -0,0 +1,93 @@ 
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+AD4052 driver
+=============
+
+ADC driver for Analog Devices Inc. AD4052 and similar devices.
+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
+   * - ``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.
+   * - ``sample_rate``
+     - Device internal sample rate used in the burst averaging mode.
+   * - ``sample_rate_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.
+
+During monitor mode, the device is busy since other transactions
+require to put the device in configuration mode first.
+
+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.
+
+The scan type is different when the buffer with offload support is enabled.
diff --git a/MAINTAINERS b/MAINTAINERS
index fef8adaee888d59e1aa3b3592dda5a8bea0b7677..312b2cf94b8f06298b1cbe5975ee32e2cf9a74d8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1322,6 +1322,7 @@  M:	Jorge Marques <jorge.marques@analog.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@analog.com>