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.
+
+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>
--
2.48.1On 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.
> > +.. 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
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
On Mon, Mar 10, 2025 at 07:54:16PM +0000, Jonathan Cameron wrote: > 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. > If doing buffer captures with oversampling enabled, both sampling frequencies have an impact: e.g., oversampling: 4 sample_rate: 2MHz PWM sampling frequency: 500KHz PWM trigger out (CNV) | | | | | ADC conversion ++++ ++++ ++++ ++++ ++++ ADC data ready (GP) * * * * * For monitor mode, it will constantly be doing conversion to check for threshold crossings, at the defined sample_rate. I like the idea of having the device's sample_rate as conversion_frequency. > > > > > > + > > > > +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. > The gpio asserts when the threshold is crossed, and desserts when it is back in bounds. The interrupt controller should be configured to detecting rising edges, to not call multiple irq_handlers for the same crossing. If the interrupt controller is set to trigger on level, multiple irq handler calls will occur before being able to access the device register to disable the GPIO. Jorge
On 3/14/25 1:13 PM, Jorge Marques wrote:
> On Mon, Mar 10, 2025 at 07:54:16PM +0000, Jonathan Cameron wrote:
>> 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.
>>
>
> If doing buffer captures with oversampling enabled, both sampling
> frequencies have an impact:
>
> e.g.,
> oversampling: 4
> sample_rate: 2MHz
> PWM sampling frequency: 500KHz
>
> PWM trigger out (CNV) | | | | |
> ADC conversion ++++ ++++ ++++ ++++ ++++
> ADC data ready (GP) * * * * *
>
> For monitor mode, it will constantly be doing conversion to check for
> threshold crossings, at the defined sample_rate.
>
> I like the idea of having the device's sample_rate as
> conversion_frequency.
In addition to what makes sense for this chip, we should also consider what
makes sense other chips with similar features. For example, I am working on
ad7606c which has control for the oversampling burst frequency (frequency of
"+" in the diagram above). So it would make sense to have a standard attribute
that would work for both chips.
On ad4052, just because we have a single register that controls two different
functions doesn't mean we have to be limited to a single attribute that controls
that register.
So I would create the events/sampling_frequency{,_available} attributes like
Jonathan suggested for controlling the sampling frequency in monitor mode and
introduce new oversampling_burst_frequency{,_available} attributes for
controlling the conversion frequency when oversampling. When an attribute is
written, we can cache the requested value in the state struct instead of
writing it directly to the register on the ADC if we want the attributes to be
independent. Then only write the register when we enable monitor mode or when
we start reading samples with oversampling enabled.
Sure, it is more work to implement it in the driver this way, but that shouldn't
be an an excuse to do things in a way that isn't compatible with other ADCs.
On Fri, Mar 14, 2025 at 01:56:32PM -0500, David Lechner wrote:
> On 3/14/25 1:13 PM, Jorge Marques wrote:
> > On Mon, Mar 10, 2025 at 07:54:16PM +0000, Jonathan Cameron wrote:
> >> 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.
> >>
> >
> > If doing buffer captures with oversampling enabled, both sampling
> > frequencies have an impact:
> >
> > e.g.,
> > oversampling: 4
> > sample_rate: 2MHz
> > PWM sampling frequency: 500KHz
> >
> > PWM trigger out (CNV) | | | | |
> > ADC conversion ++++ ++++ ++++ ++++ ++++
> > ADC data ready (GP) * * * * *
> >
> > For monitor mode, it will constantly be doing conversion to check for
> > threshold crossings, at the defined sample_rate.
> >
> > I like the idea of having the device's sample_rate as
> > conversion_frequency.
>
> In addition to what makes sense for this chip, we should also consider what
> makes sense other chips with similar features. For example, I am working on
> ad7606c which has control for the oversampling burst frequency (frequency of
> "+" in the diagram above). So it would make sense to have a standard attribute
> that would work for both chips.
>
> On ad4052, just because we have a single register that controls two different
> functions doesn't mean we have to be limited to a single attribute that controls
> that register.
>
I looked into the ad7606c driver and summarized below to organize our
ideas:
PADDING OVERSAMPLING
--------------------
Delay between conversions:
OS_CLOCK(Hz) = 1 / (1+OS_PAD/16)
OS_CLOCK: internal clock, reg
0x08 OVERSAMPLING
OS_PAD[7:4]: Extends the internal oversampling period allowing
evenly spaced sampling between CONVST rising edges,
from 0 to 15
OS_RATIO[3:0]: from off(1) to 256
Therefore, OS_CLOCK range is therefore 1Hz .. 0.516Hz
(1) from previous discussion, iio oversampling 1 equals off.
EXTERNAL OVERSAMPLING CLOCK
---------------------------
Use CONVST as the external trigger for
each conversion
On AD4052 family:
BURST AVERAGING MODE
--------------------
Delay between conversions
Total latency:
(AVG_WIN_LEN-1)/FS_BURST_AUTO + t_CONV
0x23 AVG_CONFIG
AVG_WIN_LEN[3:0]: Averaging ratio/number of samples
0x27 TIMER_CONFIG
FS_BURST_AUTO[7:4]: from 111Hz to 2 MHz, internal sample rate
AVERAGING MODE
--------------
Use CONVST as the external trigger for
each conversion
So, we can say that
PADDING OVERSAMPLING == BURST AVERAGING MODE, and
EXTERNAL OVERSAMPLING CLOCK == AVERAGING MODE
> So I would create the events/sampling_frequency{,_available} attributes like
> Jonathan suggested for controlling the sampling frequency in monitor mode and
> introduce new oversampling_burst_frequency{,_available} attributes for
> controlling the conversion frequency when oversampling. When an attribute is
> written, we can cache the requested value in the state struct instead of
> writing it directly to the register on the ADC if we want the attributes to be
> independent. Then only write the register when we enable monitor mode or when
> we start reading samples with oversampling enabled.
>
> Sure, it is more work to implement it in the driver this way, but that shouldn't
> be an an excuse to do things in a way that isn't compatible with other ADCs.
>
I am alright with that and will follow the suggestion of having the
values independent through cache.
So, two new attributes will be implemented:
* oversampling_[burst_]frequency{,_available} (new ABI required)
* events/sampling_frequency{,_available}
And I will drop conversion_frequency (early sample_rate) attribute.
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
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
On Mon, Mar 10, 2025 at 07:56:29PM +0000, Jonathan Cameron wrote: > 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: > > >>> + * - ``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! > I get the idea but maybe the datasheet sample rate as conversion_frequency and stored as a channel attribute (iio_chan_spec.ext_info) is clear enough. The datasheet sample rate affects both the burst averaging mode (oversampling) and monitor mode (threshold events). The max1363 stores as an event attribute (iio_info.event_attr) and requires iio/sysfs.h include. A last option is to store as a general purpose device attribute (iio_info.attrs). As a channel attribute, the driver logic is slightly simpler by using the macros. Jorge
On Fri, 14 Mar 2025 18:34:46 +0100 Jorge Marques <gastmaier@gmail.com> wrote: > On Mon, Mar 10, 2025 at 07:56:29PM +0000, Jonathan Cameron wrote: > > 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: > > > >>> + * - ``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! > > > > I get the idea but maybe the datasheet sample rate as conversion_frequency > and stored as a channel attribute (iio_chan_spec.ext_info) is clear enough. It's not standard ABI. So no standard userspace code will be aware of it. we can add to standard ABI, but only if we can't support a feature with what is already defined. Event then we need to keep it consistent with existing ABI. Note that the first step on any ABI is to write documentation for it in Documentation/ABI/testing/sysfs-bus-iio-* That can help people understand what is being proposed and allows discussion of how to generalize any new ABI to be useful across many drivers. > > The datasheet sample rate affects both the burst averaging mode (oversampling) and > monitor mode (threshold events). True, but are both occurring at the same time? My reading of the situation was that they weren't but I could be wrong. If they aren't then just rewrite the register to a cached value when you change mode. > > The max1363 stores as an event attribute (iio_info.event_attr) and requires iio/sysfs.h include. > A last option is to store as a general purpose device attribute (iio_info.attrs). > As a channel attribute, the driver logic is slightly simpler by using the macros. Agreed that non standard event attributes are a bit trickier to deal with. They have never been common enough for us to fix that. However the ABI exists and is documented, so that's almost certainly the way to go. Jonathan > > Jorge
© 2016 - 2026 Red Hat, Inc.