Skip to content

Fix ADAQ4216 data capture #2861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Fix ADAQ4216 data capture #2861

wants to merge 16 commits into from

Conversation

machschmitt
Copy link
Contributor

@machschmitt machschmitt commented Jul 24, 2025 •
edited
Loading

This fixes ADAQ4216 data capture

  • Update device tree for ADAQ4216.
  • A couple of minor fixes to ADAQ4216 support.
  • Update to pwm-trigger sources.
  • Extend SPI offload trigger support by adding an end_cycle_offset_ns field to struct spi_offload_trigger_periodic.
  • Add a hacky IIO buffer type.

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have tested the changes on the relevant hardware
  • I have updated the documentation outside this repo accordingly (if there is the case)

@machschmitt machschmitt force-pushed the adaq4216-data-capture-fix branch 2 times, most recently from c9ac8fd to dd952d3 Compare July 25, 2025 14:33
@machschmitt
Copy link
Contributor Author

Change log v1 -> v2

  • Fixed checkpatch warnings:
    • Added commit descriptions where needed
    • Aligned function parameters

No functional change.

@gastmaier
Copy link
Contributor

Can you also fix the ci caught warnings? e.g.,
on ad4630_get_avg_frame_len if regmap fails, it goes to out, and always returns 0; causing *val = temp; to execute, with temp uninitialized (due to the goto + return 0)

@machschmitt machschmitt force-pushed the adaq4216-data-capture-fix branch from dd952d3 to b27cc6d Compare July 30, 2025 21:32
@machschmitt
Copy link
Contributor Author

Change log v2 -> v3

  • Added a comment explaining the need of a special IIO buffer.
  • Add a patch fixing possible use of uninitialized variable due to previous goto + return 0 combination.

@machschmitt machschmitt force-pushed the adaq4216-data-capture-fix branch from cc8e0e0 to 65d4fa4 Compare August 4, 2025 15:07
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Update to new/current SPI-Engine
812fd7b 
Switch from old/temporary SPI-Engine compatible to upstream consolidated
SPI-Engine compatible string.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Update to use trigger-sources
3e9b88f 
The ad4630 driver was updated and is now using the SPI offload trigger
infrastructure which relies on a pwm-trigger being declared in device tree.
Update ADAQ4216 device tree to work with the recent SPI offload trigger
abstraction and infrastructure.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Drop leftover clock props in ADC…
0af26f8 
… node

The clock properties are no longer used after the update to use
trigger-sources. Drop leftover clock properties in ADC node.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Increase #pwm-cells to 3
8440dde 
Increase the number of pwm-cells to 3, following recent changes made to
zynq-zed-adv7511-ad4630-24.dts.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Make similar to ad4630-24 dts
ee9c8ce 
Move properties around and rename PWM phandle (axi_pwm_gen -> adc_trigger)
to make ADAQ4216 dts similar to AD4630-24 as they should be.
No functional change.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
arm: dts: zynq-zed-adv7511-adaq4216: Drop unnecessary num-cs
08ad9cb 
Remove unneeded num-cs property from SPI-Engine device node.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Update ADAQ channel storage bits to match the DMA w…
f4bcf0e 
…ord size

Set ADAQ channel storage bit to 32 since that was chosen to be the DMA word
size. While at it, align channel macro arguments to comply with Linux code
style and avoid checkpatch warnings.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Remove sampling frequency available attribute
f4bf45c 
With SPI-Engine offload module, PWMs, and trigger-sources, the sampling
frequency can be set from a wide range of values that cannot be
displayed as a list of available items. Remove the sampling frequency
available attribute so the user is not constrained when specifying the
desired sampling frequency.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Adjust ADAQ4216 output modes mask
e5be754 
ADAQ4216 doesn't use BIT(2) of OUT_DATA_MD field in mode register.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: buffer: Add industrialio-buffer-dmaengine-filtered
c3cf5b5 
This is a hack to discard ADAQ4216 second channel data.

The ad4630_fmc HDL project was designed for ADCs with two channels and
always streams two data channels to DMA (even when the ADC has only one
physical channel). Though, if the ADC has only one physical channel, the
data that would come from the second ADC channel comes in as noise and has
to be discarded. Because of that, only half of data collected by DMA blocks
shall be passed to IIO buffers when using single-channel ADCs such as
ADAQ4216.

Because only half of the data pushed to DMA is used, the filtered dmaengine
buffer variant allocates blocks that are two times the usual size so the
IIO buffer can still be filled with the data from one DMA block. Otherwise,
a data glitch occurs right in the middle of the IIO buffer.

Add a separate dmaengine buffer variant for filtering data from ADAQ4216
and similar single-channel ADCs supported by the ad4630.c driver.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Use filtered DMA Engine buffer for single-channel ADCs
3937ab6 
Use the new filtered DMA buffer variant to avoid pushing noise to IIO
buffers when single-channel ADCs are used.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Fix possible use of uninitialized variable
69ae08b 
Previously, due to return 0 after goto jump, '*val = temp;' could execute
with temp uninitialized if regmap_read() failed in
ad4630_get_avg_frame_len(). Prevent that by returning the actual error code
if regmap_read() fails in ad4630_get_avg_frame_len().

Suggested-by: Jorge Marques <[email protected]>
Fixes: 5bc31c5 ("iio: adc: ad4630: Use oversampling iio attribute for average mode")
Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Drop lower bound average mode check
b53b2e7 
The average mode value is never negative so there is no need to check if it
is lower than zero.

This fixes a 'comparison of unsigned expression in '< 0' is always false'
build warn.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Drop lower bound PGA value check
4781720 
The PGA gain value is stored in an unsigned variable so it's value can
never be a negative number. Thus, drop the lower bound check for PGA gain
values as that is not needed.

This fixes a 'comparison of unsigned expression in '< 0' is always false'
build warn.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Drop lower bound calibscale value check
69dcfc6 
The calibration scale value is stored in an unsigned variable so it's
value can never be a negative number. Thus, drop the lower bound check
for the calibration scale values as that is not needed.

This fixes a 'comparison of unsigned expression in '< 0' is always false'
build warn.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt
iio: adc: ad4630: Use right shift operator for scale calculation
f71869b 
The ADC input range value is never negative and so there is no need to use
shift_right() to calculate the scale. Make direct use of right shift
operator for scale calculation.

This fixes 'comparison of unsigned expression in ‘< 0’ is always false'
build warn.

Signed-off-by: Marcelo Schmitt <[email protected]>
@machschmitt machschmitt force-pushed the adaq4216-data-capture-fix branch from 65d4fa4 to f71869b Compare August 6, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nunojsa nunojsa Awaiting requested review from nunojsa nunojsa will be requested when the pull request is marked ready for review nunojsa is a code owner

@mhennerich mhennerich Awaiting requested review from mhennerich mhennerich will be requested when the pull request is marked ready for review mhennerich is a code owner

@dbogdan dbogdan Awaiting requested review from dbogdan dbogdan will be requested when the pull request is marked ready for review dbogdan is a code owner

@btogorean btogorean Awaiting requested review from btogorean btogorean will be requested when the pull request is marked ready for review btogorean is a code owner

@chegbeli chegbeli Awaiting requested review from chegbeli chegbeli will be requested when the pull request is marked ready for review chegbeli is a code owner

@buha buha Awaiting requested review from buha buha will be requested when the pull request is marked ready for review buha is a code owner

@amiclaus amiclaus Awaiting requested review from amiclaus amiclaus will be requested when the pull request is marked ready for review amiclaus is a code owner

@rbolboac rbolboac Awaiting requested review from rbolboac rbolboac will be requested when the pull request is marked ready for review rbolboac is a code owner

@danmois danmois Awaiting requested review from danmois danmois will be requested when the pull request is marked ready for review danmois is a code owner

@CiprianRegus CiprianRegus Awaiting requested review from CiprianRegus CiprianRegus will be requested when the pull request is marked ready for review CiprianRegus is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@machschmitt @gastmaier