From: Linus Walleij Date: Fri, 16 Jun 2023 20:42:23 +0000 (+0200) Subject: mmc: mmci: Make busy complete state machine explicit X-Git-Tag: v6.5-rc1~168^2~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7be5ac5f7a4890422920d6d91f94fff5d114d076;p=thirdparty%2Fkernel%2Flinux.git mmc: mmci: Make busy complete state machine explicit This refactors the ->busy_complete() callback currently only used by Ux500 and STM32 to handle busy detection on hardware where one and the same IRQ is fired whether we get a start or an end signal on busy detect. The code is currently using the cached status from the command IRQ in ->busy_status as a state to select what to do next: if this state is non-zero we are waiting for IRQs and if it is zero we treat the state as the starting point for a busy detect wait cycle. Make this explicit by creating a state machine where the ->busy_complete callback moves between three states. The Ux500 busy detect code currently assumes this order: we enable the busy detect IRQ, get a busy start IRQ, then a busy end IRQ, and then we clear and mask this IRQ and proceed. We insert debug prints for unexpected states. This works as before on most cards, however on a problematic card that is not working with busy detect, and which I have been debugging, the following happens a lot: [ 3.380554] mmci-pl18x 80005000.mmc: no busy signalling in time [ 3.387420] mmci-pl18x 80005000.mmc: no busy signalling in time [ 3.394561] mmci-pl18x 80005000.mmc: lost busy status when waiting for busy start IRQ This probably means that the busy detect start IRQ has already occurred when we start executing the ->busy_complete() callbacks, and the busy detect end IRQ is counted as the start IRQ, and this is what is causing the card to not be detected properly. Signed-off-by: Linus Walleij Link: https://lore.kernel.org/r/20230405-pl180-busydetect-fix-v7-5-69a7164f2a61@linaro.org Signed-off-by: Ulf Hansson --- diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9f92f39fec14d..4ab0c622dff32 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -680,6 +680,7 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) writel(host->variant->busy_detect_mask, base + MMCICLEAR); writel(readl(base + MMCIMASK0) & ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_state = MMCI_BUSY_DONE; host->busy_status = 0; return true; } @@ -697,7 +698,7 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status) { + if (host->busy_state == MMCI_BUSY_DONE) { status = readl(base + MMCISTATUS); if (status & host->variant->busy_detect_flag) { writel(readl(base + MMCIMASK0) | @@ -705,6 +706,7 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) base + MMCIMASK0); host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); + host->busy_state = MMCI_BUSY_WAITING_FOR_START_IRQ; return false; } } @@ -720,25 +722,34 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) * both the start and the end interrupts needs to be cleared, * one after the other. So, clear the busy start IRQ here. */ - if (host->busy_status && - (status & host->variant->busy_detect_flag)) { - host->busy_status |= status & (MCI_CMDSENT | MCI_CMDRESPEND); - writel(host->variant->busy_detect_mask, base + MMCICLEAR); - return false; + if (host->busy_state == MMCI_BUSY_WAITING_FOR_START_IRQ) { + if (status & host->variant->busy_detect_flag) { + host->busy_status |= status & (MCI_CMDSENT | MCI_CMDRESPEND); + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + host->busy_state = MMCI_BUSY_WAITING_FOR_END_IRQ; + return false; + } else { + dev_dbg(mmc_dev(host->mmc), + "lost busy status when waiting for busy start IRQ\n"); + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + writel(readl(base + MMCIMASK0) & + ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_state = MMCI_BUSY_DONE; + host->busy_status = 0; + return true; + } } - /* - * If there is a command in-progress that has been successfully - * sent and the busy bit isn't set, it means we have received - * the busy end IRQ. Clear and mask the IRQ, then continue to - * process the command. - */ - if (host->busy_status) { - writel(host->variant->busy_detect_mask, base + MMCICLEAR); - - writel(readl(base + MMCIMASK0) & - ~host->variant->busy_detect_mask, base + MMCIMASK0); - host->busy_status = 0; + if (host->busy_state == MMCI_BUSY_WAITING_FOR_END_IRQ) { + if (!(status & host->variant->busy_detect_flag)) { + host->busy_status |= status & (MCI_CMDSENT | MCI_CMDRESPEND); + host->busy_state = MMCI_BUSY_DONE; + return true; + } else { + dev_dbg(mmc_dev(host->mmc), + "busy status still asserted when handling busy end IRQ - will keep waiting\n"); + return false; + } } return true; @@ -1268,6 +1279,8 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) } host->busy_status = 0; + host->busy_state = MMCI_BUSY_DONE; + if (host->variant->busy_timeout && cmd->flags & MMC_RSP_BUSY) { if (!cmd->busy_timeout) cmd->busy_timeout = 10 * MSEC_PER_SEC; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index e1a9b96a3396f..12a7bbd3ce263 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -261,6 +261,19 @@ struct clk; struct dma_chan; struct mmci_host; +/** + * enum mmci_busy_state - enumerate the busy detect wait states + * + * This is used for the state machine waiting for different busy detect + * interrupts on hardware that fire a single IRQ for start and end of + * the busy detect phase on DAT0. + */ +enum mmci_busy_state { + MMCI_BUSY_WAITING_FOR_START_IRQ, + MMCI_BUSY_WAITING_FOR_END_IRQ, + MMCI_BUSY_DONE, +}; + /** * struct variant_data - MMCI variant-specific quirks * @clkreg: default value for MCICLOCK register @@ -409,6 +422,7 @@ struct mmci_host { u32 clk_reg; u32 clk_reg_add; u32 datactrl_reg; + enum mmci_busy_state busy_state; u32 busy_status; u32 mask1_reg; u8 vqmmc_enabled:1;