mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Jeff C. Jensen via B4 Relay" <devnull+kernel.nullptr.com@kernel.org>
To: Vinod Koul <vkoul@kernel.org>, dmaengine@vger.kernel.org
Cc: Frank Li <Frank.Li@kernel.org>,
	Michal Simek <michal.simek@amd.com>,
	 Suraj Gupta <suraj.gupta2@amd.com>,
	 Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>,
	 Alex Bereza <alex@bereza.email>,
	 Folker Schwesinger <dev@folker-schwesinger.de>,
	 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	 "Jeff C. Jensen" <kernel@nullptr.com>
Subject: [PATCH 0/5] dmaengine: xilinx_dma: Fix AXI DMA descriptor ring ordering and failed resets
Date: Thu, 22 Oct 2026 15:11:29 -0700	[thread overview]
Message-ID: <20261022-xilinx-dma-work-v1-0-20faea2e7a80@nullptr.com> (raw)

This series fixes descriptor handling problems in the AXI DMA driver
that I ran into while streaming from programmable logic into a single
S2MM channel on a Zynq-7020.

Patches 1-4 are fixes. Patch 5 is an RFC and is not meant to be applied
with them; patches 1-4 do not depend on it.

Patches 1 and 2 are small fixes to the preparation paths. A transfer
whose entries are all zero length built a descriptor with no segments,
and the cyclic period count could be truncated on 64-bit builds.

Patch 3 is the main change. The driver hands out segments from a free
list, while the engine follows the descriptor ring in pool order, so the
two must stay in step. Commit cee9c863ee68 fixed one way they drift
apart. The Sashiko review of that commit [1] pointed out three more, and
patch 3 removes them all by allocating consecutive ring positions
instead of free list entries.

Patch 4 handles a reset that does not complete. Today the terminate path
frees the descriptors anyway, although the engine may still be using
them. With this patch the engine is withheld instead: its channels keep
their descriptors and take no new work.

Patch 5 lets a withheld channel recover: the next terminate resets the
engine, and synchronize reclaims the descriptors once the completion
tasklet has been killed. My question for this RFC is whether a completed
engine reset, followed by synchronize, is an acceptable point to reclaim
the descriptors of a withheld channel, given that the driver instance
manages no other channel of that engine.

The series is based on v7.3-rc4, the dmaengine fixes branch at
93f51579e7df, and builds on cee9c863ee68 and 7ed1e3070c9b, both of which
are in v7.3-rc4.

Testing
=======

Every patch builds for arm (multi_v7_defconfig) and arm64 (defconfig),
with the driver built in and as a module, with W=1 and sparse, and
passes checkpatch --strict. I tested the series on hardware as a
backport to the AMD vendor kernel, described below; the patches as
posted, against v7.3-rc4, are build-tested only.

The backport ran on the AMD 6.12.40 vendor kernel together with
aa99c4d1d63b, cee9c863ee68 and 7ed1e3070c9b, on a Zynq-7020 with one AXI
DMA v7.1 S2MM channel: scatter-gather, 32-bit, no DRE and the
512-segment pool. The vendor kernel has no dma_vec path and lacks
607869003479, so the dma_vec path and the transfer chaining that commit
adds are build-tested only with these changes.

 - A test client streamed 640128-byte packets at 80 per second from a
   pattern generator, refilled the channel from the completion callback
   and from a thread on the other CPU, and checked every sample. Two 60
   s runs passed, each running out of segments about 15000 times. Cases
   for pool capacity, a zero-length scatterlist, the buffer-length
   boundary and cyclic followed by slave-sg also passed.

 - Our production client streamed nine sessions, more than 160000
   packets, with every sample checked on the receiving host.

 - Failed resets were injected with fail_function on
   xilinx_dma_chan_reset(), through an ALLOW_ERROR_INJECTION() added for
   the test only. Withholding after one and two failed resets, an
   asynchronous terminate, 20 withhold and recover cycles, a failed
   recovery and streaming after recovery all passed, and a withheld
   channel released by its client kept its memory and refused the next
   allocation. The halt before each skipped reset completed, so this
   tests the state handling of patches 4 and 5 with a stopped engine,
   not an engine that cannot be stopped.

To reproduce what patch 3 fixes: without the series, the test client ran
for 10 minutes without an error, both on the vendor kernel with the
three backports and on the vendor kernel with aa99c4d1d63b alone. The
windows are small, so I added a test-only 500 us busy-wait between the
completion callback and the free in xilinx_dma_chan_desc_cleanup(). The
test client terminates the channel once at the start of each run while
packets keep arriving. On the vendor kernel with the backports, all
three 20 s runs failed, one with a DMA internal error and two with the
channel stalled. With patches 1-3 added, the same three runs passed, and
so did one with a 2000 us busy-wait. I could not reproduce the
partial-preparation case: the test client serializes its preparations
against its own callback, which leaves that window open only between the
callback and the free.

Not tested: MM2S, two-channel controllers, CDMA, VDMA and MCDMA, and an
engine that really cannot be stopped.

Known limitations
=================

 - The AXI MCDMA pool keeps its free list, so the terminate and
   partial-preparation problems remain there. My design does not use AXI
   MCDMA, and I did not test it. Extending the reservation to MCDMA
   would be possible future work.

 - xilinx_vdma_channel_set_config() with .reset set reports a failed
   reset without withholding the engine. A terminate that follows resets
   again, and withholds the engine if that reset fails too.

 - The other two issues in the Sashiko review, the chan->terminating
   race with submit and dma_run_dependencies() under chan->lock, are not
   addressed here.

This series was developed with the help of Claude Code, an LLM coding
assistant; I reviewed all of the code and text.

[1] https://lore.kernel.org/all/20260817094004.A74621F000E9@smtp.kernel.org/

---
Jeff C. Jensen (5):
      dmaengine: xilinx_dma: Reject transfers that map no data
      dmaengine: xilinx_dma: Fix cyclic period count truncation
      dmaengine: xilinx_dma: Allocate AXI DMA segments in ring order
      dmaengine: xilinx_dma: Withhold an AXI DMA engine whose reset did not complete
      dmaengine: xilinx_dma: Recover a withheld AXI DMA channel

 drivers/dma/xilinx/xilinx_dma.c | 533 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 466 insertions(+), 67 deletions(-)
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20261022-xilinx-dma-work-4f2fad787d15

Best regards,
-- 
Jeff C. Jensen <kernel@nullptr.com>



             reply	other threads:[~2026-10-22 22:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 22:11 Jeff C. Jensen via B4 Relay [this message]
2026-10-22 22:11 ` [PATCH 1/5] dmaengine: xilinx_dma: Reject transfers that map no data Jeff C. Jensen via B4 Relay
2026-10-22 22:11 ` [PATCH 2/5] dmaengine: xilinx_dma: Fix cyclic period count truncation Jeff C. Jensen via B4 Relay
2026-10-22 22:11 ` [PATCH 3/5] dmaengine: xilinx_dma: Allocate AXI DMA segments in ring order Jeff C. Jensen via B4 Relay
2026-10-22 22:11 ` [PATCH 4/5] dmaengine: xilinx_dma: Withhold an AXI DMA engine whose reset did not complete Jeff C. Jensen via B4 Relay
2026-10-22 22:11 ` [PATCH 5/5] dmaengine: xilinx_dma: Recover a withheld AXI DMA channel Jeff C. Jensen via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022-xilinx-dma-work-v1-0-20faea2e7a80@nullptr.com \
    --to=devnull+kernel.nullptr.com@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=alex@bereza.email \
    --cc=dev@folker-schwesinger.de \
    --cc=dmaengine@vger.kernel.org \
    --cc=kernel@nullptr.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=radhey.shyam.pandey@amd.com \
    --cc=suraj.gupta2@amd.com \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®