From 51e19c7ef2b02e431dda26ee8d4183c88d14b7ad Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:35:05 +0300 Subject: [PATCH] himci: one place to give the transfer back The follow-up promised when the FIFO-reset timeout was fixed. 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 that was 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. Unwinding at request_end rather than at each site means the next path added cannot forget it, and it subsumes the explicit call the FIFO-reset fix needed, so there is one mechanism instead of two. host->data is already NULL when the data phase completed normally, so a healthy transfer is untouched -- which is also why this changes nothing for a working card. The error is filled in first because himci_data_done() reports a full transfer when it finds none set, and nothing moved on any of these paths. 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. Nothing reads data_error_count -- it is incremented here and reset in the probe -- so counting these does not change recovery behaviour. 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: these are error paths that need a card that fails in a particular way, and the test board has no serial console or altbootcmd, so flashing a kernel onto it to reach them is not a trade worth making. --- drivers/mmc/host/himci/himci.c | 40 +++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/himci/himci.c b/drivers/mmc/host/himci/himci.c index 8ccceb212a..575dbc6850 100644 --- a/drivers/mmc/host/himci/himci.c +++ b/drivers/mmc/host/himci/himci.c @@ -1045,18 +1045,11 @@ static void himci_request(struct mmc_host *mmc, struct mmc_request *mrq) * back. * * The engine is not running yet -- himci_idma_start() - * is below this point -- but himci_setup_data() has - * already mapped the scatterlist and taken - * host->data, and himci_data_done() is the only thing - * that gives either back. Without it the buffer goes - * back to the core still mapped for the device, and - * host->data is left pointing at a request that has - * been completed. Called with no status bits set, so - * it keeps the error above rather than deciding its - * own. + * is below this point -- and the mapping + * himci_setup_data() took is given back by the unwind + * at request_end, which every path here shares. */ mrq->data->error = -ETIMEDOUT; - himci_data_done(host, 0); goto request_end; } } while (tmp_reg & FIFO_RESET); @@ -1166,6 +1159,33 @@ static void himci_request(struct mmc_host *mmc, struct mmc_request *mrq) } request_end: + /* One place to give the transfer back. + * + * himci_setup_data() maps the scatterlist and takes host->data, and + * himci_data_done() is the only thing that returns either. Several + * paths reach here with that still outstanding -- a set-block-count + * that failed, a command that could not be sent, a command that + * completed with an error, a data size the descriptor table cannot + * hold -- and every one of them left the buffer mapped for the device + * for the life of the camera. himci_finish_request() below clears + * host->data without unmapping, which is what kept it invisible. + * + * Done here rather than at each site so the next path added cannot + * forget it. host->data is already NULL when the transfer completed + * normally, so nothing is unwound twice. + * + * The error is filled in first because himci_data_done() reports a + * full transfer when it finds none set, and nothing moved on any of + * these paths. + */ + if (host->data) { + if (!host->data->error) { + host->data->error = + mrq->cmd->error ? mrq->cmd->error : -EIO; + } + himci_data_done(host, 0); + } + /* clear MMC host intr */ spin_lock_irqsave(&host->lock, flags); himci_writel(ALL_SD_INT_CLR, host->base + MCI_RINTSTS);