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 failed transfer cleanup

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Centralizes outstanding DMA transfer cleanup at the shared request exit.
• Preserves command errors, otherwise assigning EIO before reporting zero transferred bytes.
• Prevents scatterlist mapping leaks and stale host data on failed requests.

Diagram

graph TD
  REQ["MMC request"] --> SETUP["Map transfer"] --> PATH{"Data completed?"}
  PATH -->|Yes| DONE["Normal data done"] --> END["Request end"]
  PATH -->|No| END
  END --> CHECK{"Transfer outstanding?"}
  CHECK -->|Yes| UNWIND["Set error and unmap"] --> FINISH["Complete request"]
  CHECK -->|No| FINISH

Loading

High-Level Assessment

The centralized request-end unwind is the best approach because every failure path already converges there and host->data records whether cleanup remains necessary. Per-site cleanup was considered but would duplicate logic and remain vulnerable to missed current or future exits.

Files changed (1) +30 / -10

Bug fix (1) +30 / -10

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

Unwind outstanding DMA transfers at the common request exit

• Adds guarded cleanup at request_end to assign an appropriate data error, unmap the scatterlist, and clear host->data before request completion. Removes the FIFO-timeout-specific cleanup because the common unwind now covers it and every other early exit.

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 ↗