From mboxrd@z Thu Jan 1 00:00:00 1970 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 65E92C00140 for ; Thu, 18 Aug 2022 06:21:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Subject:From:References:Cc: To:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BFcIEk8VyB/CcWp8OLUXDvADmCWpHLDeeb9SmAhYDH4=; b=Ib/ITVbhi0pfVZ F/V9fD8df+jpec9h9dmxXzm+gybQWwDV+FaMe2Fg0WIix1C6+VUx5/U0qbc6qXwLOY0fIyxm90PRQ plycnQRkGFGqnTFvid8heErXaIBLaRgU95u3UvfsVIpjzCx6JUyOiikjdCTQ/mO+NMs1GTK3Wh9h6 U134Y6z8BiPL/bAV6i0ILReLrtRgyCU6HdKeNXbN5dzO0hTEtWNchAitA7lzMpj/uFZCOvzHq299r rKmNjsu3m8o/LMDT/bF0l6H02+wOJbqm3w61dlMtmfedX2RNp1ooid9iYtqB3eeiAYA5M7NYo1V9c mv8H12tHKcpGgpEJ+kQw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oOYts-00GxjJ-DY; Thu, 18 Aug 2022 06:21:04 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oOYtg-00GxSG-Qt; Thu, 18 Aug 2022 06:20:54 +0000 Received: by mail-ej1-x632.google.com with SMTP id uj29so1491112ejc.0; Wed, 17 Aug 2022 23:20:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:from:to:cc; bh=cE40KzOAHgSsI3+yA6l4Fbr52ebb6r45dUqZKpm5LLw=; b=drWEerl4127r+Ht3QRFCKOsppxqYE139Q+6U3XMYkgO0HjWQDgW+xcyEZKECC1Umsf +n7yrVzExv6BST+ZK27Luoho33R5fDy7wD0u6Ptr8JToK/LGRScrrjqxRXMrGdGSbBqa iSHSiGUZCzSR5SbN3BzDIqMaVJceOtnYdiVH5Mxel62afJedDhS5iIvjMdsuWXrLheb8 TwyBbzXFCCcrcqL9IukaqNTFK/g2pUYu7fUBXf0IJuG9KhWspNh6AJi70UNG3XYGOZxX 3E+plkYWJ4e3+lWFC084EkEdPTaPDCufKhc6BdbU9hKMdhIKnph3Vp3PcDOaDYYPzw+j GNVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=cE40KzOAHgSsI3+yA6l4Fbr52ebb6r45dUqZKpm5LLw=; b=CaMXdxWxv/aYzpDHAQZ5ag5NyOn/Djsk13BDTXm0twHrIXXfqW/zC8oGiDCdgWXimW Y3INSF7oMFIqYNfgayeSvlnc6OFKhdoDaKWQssc2Pn9HFnrdpdJw3GEjx3zy9z93VJCT FH6eQMC9rMORwF9LqPRmCbd6aTDPwkfIhAnWLP70nefSmnTJ3hdfU2HTcdwp6C97u78c VrH1+S0+qTYrqb30B0YiDzKVRJy0ST4zDu+OYEJb2SRNu5FSlITeiIZ4eenBb8KaPMQR O1NZ0NspOyMXgt7WRlUZTulA8nCvObkXQ5jTIEDMTyK2ql1nPkv2FAUTKMxObxssw4Xk flAw== X-Gm-Message-State: ACgBeo02Rdt80HZpoiizL6DU/RYRnn6QodLkKT01iKrKRTDCEHp/xCnc K90+XKjhjFhTP4800UJhbU4= X-Google-Smtp-Source: AA6agR5SM1kCR6ZA2xPD4obyZAHIbd6m040hxAO9SVHTySXbjP5sn43H3Itv6iCHvcRMyP4/1SxbAg== X-Received: by 2002:a17:907:1c89:b0:734:d05c:582e with SMTP id nb9-20020a1709071c8900b00734d05c582emr988364ejc.282.1660803650447; Wed, 17 Aug 2022 23:20:50 -0700 (PDT) Received: from ?IPV6:2a01:c23:bdc7:a00:a1fc:6cca:3686:2302? (dynamic-2a01-0c23-bdc7-0a00-a1fc-6cca-3686-2302.c23.pool.telefonica.de. [2a01:c23:bdc7:a00:a1fc:6cca:3686:2302]) by smtp.googlemail.com with ESMTPSA id x19-20020aa7d6d3000000b00445e037345csm534784edr.14.2022.08.17.23.20.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Aug 2022 23:20:49 -0700 (PDT) Message-ID: Date: Thu, 18 Aug 2022 08:20:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Content-Language: en-US To: Ulf Hansson Cc: Neil Armstrong , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , "linux-mmc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Amlogic Meson..." References: From: Heiner Kallweit Subject: Re: [PATCH] mmc: meson-gx: add SDIO interrupt support In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220817_232052_937009_AAFDB104 X-CRM114-Status: GOOD ( 23.53 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org On 15.08.2022 20:20, Ulf Hansson wrote: > On Sun, 14 Aug 2022 at 23:44, Heiner Kallweit wrote: >> >> This adds SDIO interrupt support. >> Successfully tested on a S905X4-based system with a BRCM4334 >> SDIO wifi module (brcmfmac driver). >> >> Signed-off-by: Heiner Kallweit >> --- >> drivers/mmc/host/meson-gx-mmc.c | 45 +++++++++++++++++++++++++-------- >> 1 file changed, 34 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >> index 2f08d442e..e8d53fcdd 100644 >> --- a/drivers/mmc/host/meson-gx-mmc.c >> +++ b/drivers/mmc/host/meson-gx-mmc.c >> @@ -41,14 +41,17 @@ >> #define CLK_V2_TX_DELAY_MASK GENMASK(19, 16) >> #define CLK_V2_RX_DELAY_MASK GENMASK(23, 20) >> #define CLK_V2_ALWAYS_ON BIT(24) >> +#define CLK_V2_IRQ_SDIO_SLEEP BIT(29) >> >> #define CLK_V3_TX_DELAY_MASK GENMASK(21, 16) >> #define CLK_V3_RX_DELAY_MASK GENMASK(27, 22) >> #define CLK_V3_ALWAYS_ON BIT(28) >> +#define CLK_V3_IRQ_SDIO_SLEEP BIT(29) >> >> #define CLK_TX_DELAY_MASK(h) (h->data->tx_delay_mask) >> #define CLK_RX_DELAY_MASK(h) (h->data->rx_delay_mask) >> #define CLK_ALWAYS_ON(h) (h->data->always_on) >> +#define CLK_IRQ_SDIO_SLEEP(h) (h->data->irq_sdio_sleep) >> >> #define SD_EMMC_DELAY 0x4 >> #define SD_EMMC_ADJUST 0x8 >> @@ -100,9 +103,6 @@ >> #define IRQ_END_OF_CHAIN BIT(13) >> #define IRQ_RESP_STATUS BIT(14) >> #define IRQ_SDIO BIT(15) >> -#define IRQ_EN_MASK \ >> - (IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN | IRQ_RESP_STATUS |\ >> - IRQ_SDIO) >> >> #define SD_EMMC_CMD_CFG 0x50 >> #define SD_EMMC_CMD_ARG 0x54 >> @@ -136,6 +136,7 @@ struct meson_mmc_data { >> unsigned int rx_delay_mask; >> unsigned int always_on; >> unsigned int adjust; >> + unsigned int irq_sdio_sleep; >> }; >> >> struct sd_emmc_desc { >> @@ -431,6 +432,7 @@ static int meson_mmc_clk_init(struct meson_host *host) >> clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, CLK_PHASE_180); >> clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, CLK_PHASE_0); >> clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, CLK_PHASE_0); >> + clk_reg |= CLK_IRQ_SDIO_SLEEP(host); >> writel(clk_reg, host->regs + SD_EMMC_CLOCK); >> >> /* get the mux parents */ >> @@ -933,7 +935,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id) >> { >> struct meson_host *host = dev_id; >> struct mmc_command *cmd; >> - struct mmc_data *data; >> u32 irq_en, status, raw_status; >> irqreturn_t ret = IRQ_NONE; >> >> @@ -948,14 +949,24 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id) >> return IRQ_NONE; >> } >> >> - if (WARN_ON(!host) || WARN_ON(!host->cmd)) >> + if (WARN_ON(!host)) >> return IRQ_NONE; >> >> /* ack all raised interrupts */ >> writel(status, host->regs + SD_EMMC_STATUS); >> >> cmd = host->cmd; >> - data = cmd->data; >> + >> + if (status & IRQ_SDIO) { >> + mmc_signal_sdio_irq(host->mmc); > > This is the legacy interface for supporting SDIO irqs. I am planning > to remove it, sooner or later. > > Please convert into using sdio_signal_irq() instead. Note that, using > sdio_signal_irq() means you need to implement support for > MMC_CAP2_SDIO_IRQ_NOTHREAD, which also includes to implement the > ->ack_sdio_irq() callback. > > There are other host drivers to be inspired from, but don't hesitate > to ask if there is something unclear. > One more question came to my mind: Typically host drivers disable the SDIO interrupt source before calling sdio_signal_irq(), and re-enable it in ->ack_sdio_irq(). In sdio_run_irqs() we have the following: if (!host->sdio_irq_pending) host->ops->ack_sdio_irq(host); In the middle of this code the host can't actively trigger a SDIO interrupt because the interrupt source is still disabled. But some other host interrupt could fire with also the SDIO interrupt source bit set. Then the hard irq handler would disable the SDIO interrupt source, and directly after this ->ack_sdio_irq() would re-enable it. This looks racy to me and we may need some protection. Do you share this view or do I miss something? > [...] > > Kind regards > Uffe _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic