himci: one place to give the transfer back

[widgetii]

The follow-up promised in #57/#58/#59.

himci_setup_data() maps the scatterlist and takes host->data.
himci_data_done() is the only thing that returns either, and it runs only on
the path where the data phase actually completed. Every other way out of
himci_request() with a transfer set up left the buffer mapped for the
device and host->data pointing at a request about to be completed:

• a data size larger than the descriptor table can hold
• a set-block-count command that could not be sent, or timed out
• the command itself failing to go out
• the command completing with an error, on both the tuning and the ordinary
  branch

himci_finish_request() clears host->data without unmapping, which is
exactly why none of this showed up — the pointer looked tidy afterwards and the
mapping was gone for good.

Why at request_end rather than at each site

Six goto request_end sites can reach it with a transfer outstanding. Fixing
them one at a time is how the seventh gets forgotten. Unwinding once at
request_end also subsumes the explicit himci_data_done() call the
FIFO-reset fix needed, so there is one mechanism instead of two — that call is
removed here.

host->data is already NULL when the data phase completed normally, so a
healthy transfer is untouched. That is also the argument that this is safe:
for a working card the new block never runs.

The error assignment

himci_data_done() reports a full transfer when it finds no error set, and
nothing moved on any of these paths — so the error is filled in first. Taking
the command's error where there is one keeps the pair coherent for the core,
which previously saw a data phase with no error and no bytes transferred.

Nothing reads data_error_count — it is incremented at request_end and reset
in the probe — so counting these does not change recovery behaviour. I checked
that before letting the assignment widen what increments it.

Testing

Compile-tested against the board kernel config with CONFIG_HIMCI=y
(arm-openipc-linux toolchain, make drivers/mmc/host/himci/himci.o), no new
warnings.

Not exercised, and I would rather say so than imply otherwise. These are
error paths that need a card failing in a particular way. The test board has no
serial console and no altbootcmd, so flashing a kernel onto it to reach them
is not a trade worth making — the same reasoning as #57. What can be said is
that the healthy path does not enter the new block at all.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

himci: Centralize DMA transfer cleanup on request errors

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Centralizes outstanding DMA transfer cleanup at himci_request()'s shared exit.
• Propagates command failures to data requests before unmapping incomplete transfers.
• Prevents leaked scatterlist mappings and stale host->data references on error paths.

Diagram

graph TD
  A["MMC Request"] --> B["Setup Data"] --> C["Execute Transfer"] --> D{"Data Pending?"}
  B -->|setup exit| D
  D -->|yes| E["Ensure Error"] --> F["Data Done"] --> G["Finish Request"]
  D -->|no| G

Loading

High-Level Assessment

The centralized guarded unwind at request_end is the preferred approach. Per-exit cleanup was considered but would duplicate ownership logic across multiple failure branches and remain vulnerable to future omissions; host->data already provides an effective indicator that normal completion has released the transfer.

Files changed (1) +30 / -10

Bug fix (1) +30 / -10

himci.cUnwind outstanding DMA transfers at the shared request exit +30/-10

Unwind outstanding DMA transfers at the shared request exit

• Adds guarded cleanup at 'request_end' to assign an appropriate data error, unmap the scatterlist through 'himci_data_done()', and clear 'host->data' before completing the request. Removes the FIFO-reset branch's dedicated cleanup because all failure paths now use the centralized unwind.

drivers/mmc/host/himci/himci.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗