From patchwork Sat Apr 8 22:00:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 13205844 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F697C77B74 for ; Sat, 8 Apr 2023 22:00:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229456AbjDHWAn (ORCPT ); Sat, 8 Apr 2023 18:00:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48646 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229501AbjDHWAm (ORCPT ); Sat, 8 Apr 2023 18:00:42 -0400 Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8D2B455A1 for ; Sat, 8 Apr 2023 15:00:40 -0700 (PDT) Received: by mail-lf1-x129.google.com with SMTP id r27so2339895lfe.0 for ; Sat, 08 Apr 2023 15:00:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680991239; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=zqZGMFnsNYppNHEjJU3Id3G9gHyKFj8hUL8QQ14wDPc=; b=YPFnb73KNc8nQxfZWba9/TEkDqT3+t4nLQC/I6bezWUX0VU2Sp9CeGSLoYCAh2ZsnX ITTEylM/6Gr22tQUVOPwH77uY+wAHQVhm/DC01nxFyVxw4BAzHrCtX+vaYN6dPY0OSNw atHWfZNWyBSjnRAY+plc4lMc40jTQB3/UO9Yz734wk9gr20qj/vhU2ZGR7UpTScb/q0D WGKDhdRGXxfRxlTKhHIhl5iSz7hGyafqpAM5PAPgabLyGyGyueoSLiTwIwQTrYFvADK0 eBod3PhN8eS9gn+NtjDLIq3d+bus7ipuM16tm2i7m5SdQnjV1NFme8RzBMnPTosuz45G /ToQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680991239; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=zqZGMFnsNYppNHEjJU3Id3G9gHyKFj8hUL8QQ14wDPc=; b=cb8phaRh9fDBOWr+lHfA2iLoRhuRkheqKTL9sOr9W70Xku5mujH+lMblAhdMmwESHV e5OIhh5uy3zfre+coZU4CuxdWWW5tFUDQpzmMzXMXU0tDQeGEE1wG95xxjvPtyg2aNk9 ZOzKOU7yZMfEY5GtmGAmFIaNBwORE1y6vTiFQ+kUfmQP4bOExmjKZvUoStt3gpJMC/Xe 6PRfcnRYwhcN4B4tpftLJ6KVdkSOWd+UDPtwtwLL6hAyDjfV4VE7/RESWvjWyd1QkrYv 08IQqooYw0StjDb6jrJ0RK/milcXsFpsKtkKf55ecp66dUOphqQrqqCjZ1+KKdbz/ziQ t8iQ== X-Gm-Message-State: AAQBX9dZXsYJnublrWAAKd6TNARmh1lXiLgAk4rEc+//aE5y2EowcEhD /HrSXH+0tLS3NSBTfMBotwR/dg== X-Google-Smtp-Source: AKy350bytppW9j4EnEtNwAXVAYuyk33SR51sQnA0WepKJf/jWMOIsFfAdaDWj6ulifvWmWfBKUsSWA== X-Received: by 2002:ac2:554e:0:b0:4b4:8f01:f8b1 with SMTP id l14-20020ac2554e000000b004b48f01f8b1mr1779726lfk.31.1680991238877; Sat, 08 Apr 2023 15:00:38 -0700 (PDT) Received: from [192.168.1.2] (c-05d8225c.014-348-6c756e10.bbcust.telenor.se. [92.34.216.5]) by smtp.gmail.com with ESMTPSA id v9-20020a056512048900b004e9cad1cd7csm1328904lfq.229.2023.04.08.15.00.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Apr 2023 15:00:38 -0700 (PDT) From: Linus Walleij Date: Sun, 09 Apr 2023 00:00:26 +0200 Subject: [PATCH v2 05/12] mmc: mmci: Make busy complete state machine explicit MIME-Version: 1.0 Message-Id: <20230405-pl180-busydetect-fix-v2-5-eeb10323b546@linaro.org> References: <20230405-pl180-busydetect-fix-v2-0-eeb10323b546@linaro.org> In-Reply-To: <20230405-pl180-busydetect-fix-v2-0-eeb10323b546@linaro.org> To: Yann Gautier , Stefan Hansson , Ulf Hansson , Maxime Coquelin , Alexandre Torgue Cc: linux-mmc@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, Linus Walleij X-Mailer: b4 0.12.0 Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org This refactors the ->busy_complete() callback currently only used by Ux500 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 four states: MMCI_BUSY_NOT_STARTED, MMCI_BUSY_WAITING_FOR_IRQS, MMCI_BUSY_START_IRQ and MMCI_BUSY_END_IRQ. The 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 dev_err() prints for unexpected states. Augment the STM32 driver with similar states for completeness. 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, this happens: [127220.662719] mmci-pl18x 80005000.mmc: lost busy status when waiting for busy end 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 --- ChangeLog v1->v2: - No changes --- drivers/mmc/host/mmci.c | 55 +++++++++++++++++++++++++++++++------ drivers/mmc/host/mmci.h | 16 +++++++++++ drivers/mmc/host/mmci_stm32_sdmmc.c | 6 +++- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 7d42625f2356..887b83e392a4 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -670,6 +670,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; } @@ -687,7 +688,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_IDLE) { status = readl(base + MMCISTATUS); if (status & host->variant->busy_detect_flag) { writel(readl(base + MMCIMASK0) | @@ -695,6 +696,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_IRQS; return false; } } @@ -710,11 +712,40 @@ 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_IRQS) { + 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_START_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 (host->busy_state == MMCI_BUSY_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_END_IRQ; + return false; + } else { + dev_dbg(mmc_dev(host->mmc), + "lost busy status when waiting for busy end 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; + } } /* @@ -723,11 +754,18 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) * 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); + if (host->busy_state == MMCI_BUSY_END_IRQ) { + if (status & host->variant->busy_detect_flag) { + /* We should just get two IRQs for busy detect */ + dev_err(mmc_dev(host->mmc), "spurious busy detect IRQ\n"); + return false; + } + 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; } @@ -1258,6 +1296,7 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) } if (cmd->flags & MMC_RSP_BUSY) { + host->busy_state = MMCI_BUSY_IDLE; host->busy_status = 0; if (host->variant->busy_timeout) { diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index e1a9b96a3396..82f3850325c8 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -261,6 +261,21 @@ 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_IDLE, + MMCI_BUSY_WAITING_FOR_IRQS, + MMCI_BUSY_START_IRQ, + MMCI_BUSY_END_IRQ, + MMCI_BUSY_DONE, +}; + /** * struct variant_data - MMCI variant-specific quirks * @clkreg: default value for MCICLOCK register @@ -409,6 +424,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; diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 60bca78a72b1..24831a1092b2 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -393,8 +393,10 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) busy_d0 = sdmmc_status & MCI_STM32_BUSYD0; /* complete if there is an error or busy_d0end */ - if ((status & err_msk) || busy_d0end) + if ((status & err_msk) || busy_d0end) { + host->busy_state = MMCI_BUSY_DONE; goto complete; + } /* * On response the busy signaling is reflected in the BUSYD0 flag. @@ -408,6 +410,7 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); } + host->busy_state = MMCI_BUSY_END_IRQ; return false; } @@ -416,6 +419,7 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) writel_relaxed(mask & ~host->variant->busy_detect_mask, base + MMCIMASK0); host->busy_status = 0; + host->busy_state = MMCI_BUSY_DONE; } writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR);