Koror ram capture support by stefpopa · Pull Request #3448 · analogdevicesinc/linux

stefpopa · 2026-07-21T08:02:52Z

PR Description

Add debugfs interface for quick single-shot RX/ORx RAM captures
Add full IIO device (adrv904x-ramc) with I/Q channel support and buffer infrastructure

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 compiled my changes, including the documentation
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly
I have provided links for the relevant upstream lore

github-actions · 2026-07-21T14:41:47Z

LLM review

This series adds a debugfs interface and a dedicated IIO device (adrv904x-ramc) for
triggering RX/ORx capture-RAM snapshots on the ADRV904X transceiver.

run: 29838719279

`dff75231057fb` - Add IIO device for RX/ORx RAM capture

Build failure: adrv904x_ramc_probe() calls devm_iio_kfifo_buffer_setup_ext(), but
ADRV904X in drivers/iio/trx-rf/Kconfig only selects CF_AXI_ADC (which pulls in
IIO_BUFFER/IIO_BUFFER_HW_CONSUMER/IIO_BUFFER_DMAENGINE, not IIO_KFIFO_BUF). Reproduced
with set_arch gcc_arm + auto_set_kconfig + make:

ld: vmlinux.o: in function `adrv904x_ramc_probe':
undefined reference to `devm_iio_kfifo_buffer_setup_ext'

Adding select IIO_KFIFO_BUF fixes the link, and a full ARCH=arm build then succeeds.

Data-correctness bug: adrv904x_ramc_capture_and_push() always splits the capture
buffer into two equal halves (offset = word_count / 2) to demux I/Q data. That matches the
RX/DPD capture RAM layout (adrv904x_datainterface.c: bankSize = wordCount >> 1, comment
"dpd capture memory 2 banks 8k each"), but the ORx capture RAM uses a different, fixed
layout (comment "ORx capture ram 3 banks 4k each": up to three fixed 4K-word banks depending
on the requested size). The halved split is bank-aligned for ORx only when
word_count == 8K; for every other ORx-valid sample_count (32-4096, or 12288) the demux
mixes data from the wrong physical banks and silently returns corrupted I/Q data on the
ORX0/ORX1 channels, without any error being reported to userspace.

CI warnings

The gcc_fanalyzer/clang_analyzer warnings for the dead-store struct device *dev in
adrv904x.c (lines ~1599-1948) all trace back to commit cb241a9d869ab, which predates this
review range, so they are pre-existing and not introduced by this series.

Verification

Built with set_arch gcc_arm + auto_set_kconfig + make -j$(nproc) -C . O=build;
reproduced the devm_iio_kfifo_buffer_setup_ext link error, then confirmed a
select IIO_KFIFO_BUF fix produces a complete arch/arm/boot/zImage.
Read adrv904x_datainterface.c's private adrv904x_RxOrxDataCaptureRead() to confirm the
RX/DPD vs. ORx bank-layout difference (no ADRV904X hardware available to reproduce the
ORx corruption at runtime).
checkpatch.pl is clean on both commits, aside from the generic "does MAINTAINERS need
updating?" notice, which is not actionable (this driver has no MAINTAINERS entry at all).
Built the two touched files with W=1: no new warnings introduced by this series.

Suggested patches

Two --fixup=dff75231057fb commits, verified to git apply --check cleanly and to build
successfully for ARCH=arm:

select IIO_KFIFO_BUF in drivers/iio/trx-rf/Kconfig.
Restrict ORx sample_count to 8K (the only value for which the current 2-bank demux is
correct) at sysfs-store time and defensively inside the capture path, instead of silently
returning garbled data. Proper multi-bank ORx demux is left to the driver author to
implement and validate against real hardware.

Apply the suggested patches with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/linux 29838719279

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"      -o ~/.local/bin/apply-patches.sh &&   grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

nunojsa

As said, not sure I'm convinced about the debugfs added value but if you choose to keep it and, as a nit, swap the patches. Make the real feature (the iio buffer) be the first patch and the debug thing a follow up. Makes more sense to me

nunojsa · 2026-08-04T12:37:38Z

+
+	ret = devm_add_action(&spi->dev, adrv904x_free_capture_data, phy);
+	if (ret)
+		return ret;


This should likely be in adrv904x_ramc_probe(). Another oddity is why aren't we using devm_add_action_or_reset()?

rx_capture_data is allocated by debugfs (DBGFS_RX_DATA_CAPTURE), not by RAMC - so the cleanup belongs here after adrv904x_register_debugfs(), not in adrv904x_ramc_probe().
Changed to devm_add_action_or_reset().

nunojsa · 2026-08-04T12:40:06Z

+
+	/* Scan buffer: I + Q + timestamp */
+	u8 scan_data[ALIGN(2 * sizeof(s32), sizeof(s64)) + sizeof(s64)]
+		__aligned(IIO_DMA_MINALIGN);


Needs to the last in the struct! Also please check if we have this helpers:

https://elixir.bootlin.com/linux/v7.2-rc5/source/include/linux/iio/iio.h#L935

If not, might be worth it to backport them (if easy enough)

nunojsa · 2026-08-04T12:40:34Z

+ * re-enable the buffer (or use iio_readdev which handles this automatically).
+ */
+
+#include "adrv904x.h"


This should go after the "system" includes

nunojsa · 2026-08-04T12:45:34Z

+		.write = adrv904x_ramc_sample_count_store,
+		.shared = IIO_SHARED_BY_ALL,
+	},
+	{},


add a space and not need for comma here

nunojsa · 2026-08-04T12:48:48Z

+			i_val = sign_extend_28bit(bank1[2 * i + 1]);
+			scan_s32[0] = i_val;
+			scan_s32[1] = q_val;
+			iio_push_to_buffers_with_timestamp(st->indio_dev,


https://elixir.bootlin.com/linux/v7.2-rc5/source/include/linux/iio/buffer.h#L29

nunojsa · 2026-08-04T12:50:57Z

+		dev_err(st->dev, "Failed to capture channel 0x%lx: %d\n",
+			active_channel, ret);
+		return -EIO;
+	}


Do we need to lock the whole thing or just adi_adrv904x_RxOrxDataCaptureStart(). This also makes me wonder if we shouldn't just use the device lock for the the whole ram device

Lock scope reduced to just the API call..

We use a separate st->lock for RAMC state to avoid blocking other phy operations while changing sample_count. But using phy->lock for everything would be simpler - let me know if you prefer that.

nunojsa · 2026-08-04T12:54:34Z

+		/* Allocate buffer: up to 12 chars per value (including newline) */
+		buf_size = phy->rx_capture_len * 12 + 1;
+		out_buf = kvzalloc(buf_size, GFP_KERNEL);
+		if (!out_buf)


Use __free(kvfree) and avoid some leaks. Also what sizes are talking about here? IIRC, kvalloc() is for things in the megabyte range

nunojsa · 2026-08-04T12:55:02Z

+		result = simple_read_from_buffer(userbuf, count, ppos, out_buf, pos);
+		kvfree(out_buf);
+		return result;
+	}


I would also prefer to move this into an helper function

nunojsa · 2026-08-04T13:01:59Z

+		}
+
+		kvfree(phy->rx_capture_data);
+		phy->rx_capture_data = capture_buf;


So here we have phy->rx_capture_data. So the action in the other patch is just wrong (needs to be in the same patch as debugfs). Also not sure if all of this handling in terms of allocation makes the debugfs thing worth it! What's the big advantaged when compared with using IIO buffering?

Another side note is maybe kvrealloc() would also make this simpler?

Add a dedicated IIO device (adrv904x-ramc) for RX/ORx data capture with proper I/Q channel representation and standard IIO buffer interface. Features: - 10 channel pairs (RX0-7, ORX0-1) with I/Q modifiers - Channels appear as voltage0_i/voltage0_q through voltage9_i/voltage9_q - Hardware constraint enforced via available_scan_masks: only one I/Q pair can be captured at a time - Single-shot capture: enabling the buffer triggers one capture that fills the buffer, then stops. This matches the hardware behavior (capture RAM is a snapshot mechanism, not continuous streaming) - Configurable capture size via sample_count attribute (32-32768) - 28-bit two's complement data using sign_extend32() - Bank interleaving and Q negation per hardware specification The single-shot behavior is intentional: continuous RX data is available through the JESD streaming interface (axi-adrv904x-rx-hpc). RAM capture is designed for debugging, calibration verification, and signal analysis where point-in-time snapshots are needed. Usage with iio_readdev: iio_readdev -s 1024 adrv904x-ramc voltage0_i voltage0_q > capture.bin Signed-off-by: Stefan Popa <stefan.popa@analog.com>

Add a debugfs interface for triggering single-shot RX/ORx data captures from the ADRV904x capture RAM. This provides a simple mechanism for debugging and signal analysis without requiring full IIO buffer setup. The rx_data_capture debugfs file accepts channel and capture length: echo "<channel> <length>" > rx_data_capture cat rx_data_capture Channel mapping: 0-7 for RX0-RX7, 8-9 for ORX0-ORX1. Capture length must be a hardware-supported size (32 to 32768 samples). ORX channels are limited to 12288 samples maximum. The captured data is stored as raw 32-bit values (one per line) and contains interleaved I/Q samples in hardware format. Memory allocation is performed before taking the device mutex to avoid sleeping with locks held. Signed-off-by: Stefan Popa <stefan.popa@analog.com>

stefpopa · 2026-08-07T09:48:38Z

changelog v2:

Swapped patch order (IIO device first, debugfs second)
Moved devm_add_action_or_reset() and adrv904x_free_capture_data() to debugfs patch
Used devm_add_action_or_reset() instead of devm_add_action()
Used IIO_DECLARE_DMA_BUFFER_WITH_TS() macro for scan buffer
Used iio_push_to_buffers_with_ts() instead of iio_push_to_buffers_with_timestamp()
Reduced phy->lock scope to API call only
Used __free(kfree) with no_free_ptr() for capture buffer in debugfs
Extracted adrv904x_debugfs_read_rx_capture() helper function
Changed from kvcalloc/kvfree to kcalloc/kfree (max 128KB allocation)
Fixed include order (system includes before local)
Fixed ext_info terminator style ({ } with space)

stefpopa · 2026-08-28T07:43:08Z

@nunojsa any remarks on v2? Thanks!

stefpopa · 2026-09-16T07:23:16Z

@nunojsa when you get the chance, could you please take a look on v2?

stefpopa requested review from CiprianRegus, amiclaus, btogorean, buha, chegbeli, danmois, dbogdan, machschmitt, mhennerich, nunojsa and rbolboac as code owners July 21, 2026 08:02

gastmaier added the llm review Request a review from a LLM Reviewer label Jul 21, 2026

stefpopa force-pushed the staging/koror_ram_capture branch from dff7523 to 2b7e61a Compare July 22, 2026 09:16

stefpopa force-pushed the staging/koror_ram_capture branch 2 times, most recently from 104691a to f066077 Compare July 31, 2026 06:12

nunojsa reviewed Aug 4, 2026

View reviewed changes

nunojsa mentioned this pull request Aug 5, 2026

Staging/adrv903x ram capture #3445

Open

6 of 8 tasks

stefpopa force-pushed the staging/koror_ram_capture branch 2 times, most recently from f931c3f to 3ac56dd Compare August 7, 2026 08:56

stefpopa added 2 commits August 7, 2026 11:59

stefpopa force-pushed the staging/koror_ram_capture branch from 3ac56dd to 720ecd7 Compare August 7, 2026 09:30

Conversation

stefpopa commented Jul 21, 2026

PR Description

PR Type

PR Checklist

Uh oh!

github-actions Bot commented Jul 21, 2026

LLM review

dff75231057fb - Add IIO device for RX/ORx RAM capture

CI warnings

Verification

Suggested patches

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefpopa commented Aug 7, 2026

Uh oh!

stefpopa commented Aug 28, 2026

Uh oh!

stefpopa commented Sep 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`dff75231057fb` - Add IIO device for RX/ORx RAM capture