Commit 7be5ac5f authored by Linus Walleij's avatar Linus Walleij Committed by Ulf Hansson
Browse files

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: default avatarLinus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20230405-pl180-busydetect-fix-v7-5-69a7164f2a61@linaro.org


Signed-off-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
parent e1a2485c

drivers/mmc/host/mmci.c

+31 −18
Original line number Diff line number Diff line
@@ -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)) {

	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;

	}



	/*

	 * 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) {

		} 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 (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;

drivers/mmc/host/mmci.h

+14 −0
Original line number Diff line number Diff line
@@ -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;