[2/2] mmc: mmci: Clarify comments in the code about busy detection
diff mbox series

Message ID 20190715164230.27348-1-ulf.hansson@linaro.org
State New
Headers show
Series
  • [1/2] mmc: mmci: Drop re-read of MMCISTATUS for busy status
Related show

Commit Message

Ulf Hansson July 15, 2019, 4:42 p.m. UTC
The code dealing with busy detection is somewhat complicated. In a way to
make it a bit clearer, let's try to clarify the comments in the code about
it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/mmci.c | 43 ++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

Comments

Linus Walleij July 16, 2019, 9:28 p.m. UTC | #1
On Mon, Jul 15, 2019 at 6:42 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:

> The code dealing with busy detection is somewhat complicated. In a way to
> make it a bit clearer, let's try to clarify the comments in the code about
> it.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Uh oh there seems to have been some refactoring of the code that
it was trying to document. Stale docs.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Patch
diff mbox series

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 5f35afc4dbf9..94e7ba368cca 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1222,19 +1222,30 @@  mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	      (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND)))
 		return;
 
-	/*
-	 * ST Micro variant: handle busy detection.
-	 */
+	/* Handle busy detection on DAT0 if the variant supports it. */
 	if (busy_resp && host->variant->busy_detect) {
 
-		/* We are busy with a command, return */
+		/*
+		 * If there is a command in-progress that has been successfully
+		 * sent, then bail out if busy status is set and wait for the
+		 * busy end IRQ.
+		 */
 		if (host->busy_status &&
 		    (status & host->variant->busy_detect_flag))
 			return;
 
 		/*
 		 * Before unmasking for the busy end IRQ, confirm that the
-		 * command was sent successfully.
+		 * command was sent successfully. To keep track of having a
+		 * command in-progress, while waiting for busy signaling to end,
+		 * store the status in host->busy_status.
+		 *
+		 * Note that, since the HW seems to be triggering an IRQ on both
+		 * edges while monitoring DAT0 for busy completion, but the same
+		 * status bit in MMCISTATUS is used to monitor both start and
+		 * end of busy detection, special care must be taken to make
+		 * sure that both start and end interrupts are always cleared
+		 * one after the other.
 		 */
 		if (!host->busy_status &&
 		    !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
@@ -1248,19 +1259,17 @@  mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 			writel(readl(base + MMCIMASK0) |
 			       host->variant->busy_detect_mask,
 			       base + MMCIMASK0);
-			/*
-			 * Now cache the last response status code (until
-			 * the busy bit goes low), and return.
-			 */
+
 			host->busy_status =
 				status & (MCI_CMDSENT|MCI_CMDRESPEND);
 			return;
 		}
 
 		/*
-		 * At this point we are not busy with a command, we have
-		 * not received a new busy request, clear and mask the busy
-		 * end IRQ and fall through to process the IRQ.
+		 * If there is a command in-progress that has been successfully
+		 * sent and the busy status 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) {
 
@@ -1506,14 +1515,8 @@  static irqreturn_t mmci_irq(int irq, void *dev_id)
 		}
 
 		/*
-		 * We intentionally clear the MCI_ST_CARDBUSY IRQ (if it's
-		 * enabled) in mmci_cmd_irq() function where ST Micro busy
-		 * detection variant is handled. Considering the HW seems to be
-		 * triggering the IRQ on both edges while monitoring DAT0 for
-		 * busy completion and that same status bit is used to monitor
-		 * start and end of busy detection, special care must be taken
-		 * to make sure that both start and end interrupts are always
-		 * cleared one after the other.
+		 * Busy detection is managed by mmci_cmd_irq(), including to
+		 * clear the corresponding IRQ.
 		 */
 		status &= readl(host->base + MMCIMASK0);
 		if (host->variant->busy_detect)