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 DE50EC433FE for ; Fri, 21 Oct 2022 17:47:22 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: 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=VyRlpxm1Rc1OgqhUPzlZTniPwtL2qteD22RkBTj5oDU=; b=R2v8KZPlbXTfaK g6V4czOrppB4X4g4/9VZIR5dE2ajK1rbcJKamW1jD+gfykHaxGEPuU2qU/xMZYH1RyfVRbfd54SR3 Y3099suKav3PO6GbgV6ASoc8BjhmRikB4iojliPCL7oyoBVMEYItTcWOazblYtlieHmfHkFI61lek +EKeYj+JRmVKgO4niiQDzr8rrXYZT7LRIQ/tqDdyABYVqQ509R1y6eoN+P4inXOcnMk1Nfzha6EsM POeYGXSfUYfrDV6TdrLZXOe1xsXzpTU1EUQBDVD6+QhQACkljg9E00z7y3PKWD/QhPqUIyzSENEhk aFEz4yeoNKafNxozS6fA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1olw5p-009GtJ-IE; Fri, 21 Oct 2022 17:46:02 +0000 Received: from mail-qt1-x833.google.com ([2607:f8b0:4864:20::833]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1olw5f-009Gow-Kj for linux-arm-kernel@lists.infradead.org; Fri, 21 Oct 2022 17:45:53 +0000 Received: by mail-qt1-x833.google.com with SMTP id f22so2119439qto.3 for ; Fri, 21 Oct 2022 10:45:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=9jf1Or+O0WATniMBydkpibsCp44mV74kwc+1uNXS0e0=; b=MaUeJGs+YkNZIKBbZ8z/Z9S2HZlo6KAmfqilfv+d+s+Y3O9zurejymgbKOf7JSvPqO 0fVlj99M/nNWb/k86FK/5YdSrVsIK45/Uu7net3R/SKvCZ4BHFR+orBGxHdCbkfmm4Cp FUgVWZyPq+cJEXCG0EN5RBL6H1s6kAXhjA/2M8P4hheiqaYA1eLhV/hUv/2IqduQr1W0 9XbyIrp3u5e+AC5Wr41vwQ4TZa3FW5tE/PQxe7Irh01QJJYK5aJ8IFYAGvCcwwCDI07H GxgkE53xpRDM/OS16msJZEUFZd4QJcRApOUgUO3fAdgUXYy+6piNHX/NMCUNkakzBvxT NbFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9jf1Or+O0WATniMBydkpibsCp44mV74kwc+1uNXS0e0=; b=1Lfo6BdZdgZGxfDgmaDykDfYt16cBy1nFsFdwMpDkdCsY/dBXnuy+VpjAjCwT8maM2 fuQCcartDZLUVQ9OZBGoYir8t4Bn5D3pVBkuWD6FMXTcCBiulkkp5okzbDJbFZu+BiKN 1p6KY+nOaGd0U1mofX+Ro55y20rw1gGWk5Tx9xdmC2/b/yihouK7/ZXnyf7uxWyHJ7X+ Q72kn2A+W0Q60ec9iTh7FRWeZxSSBC/UIZHQJPnitX/zrUl5kB67pkvGV9P9NEaaqAJQ +A36DFP9jrNRdRwpFohT2lT1LLp483JizobFXdiXzxD4WWPYspd0PU67iDHSA+V4Zs2P kpCA== X-Gm-Message-State: ACrzQf1Hkxe5w8Aq8bnsQ3Hm6jqtPI71TSk6QMnYe5ykaL/EkWLPjgpC JuwACL1Xgru0giqt+3pr2Z0= X-Google-Smtp-Source: AMsMyM4geCAzB+hp+wq+emgUYQlwUd2aBYl5rvEbU1iMuZ+NC4ysO0kWjtTcWnCX/l/FutMzq+ybog== X-Received: by 2002:ac8:5b90:0:b0:39c:e9b9:c002 with SMTP id a16-20020ac85b90000000b0039ce9b9c002mr17475349qta.3.1666374346793; Fri, 21 Oct 2022 10:45:46 -0700 (PDT) Received: from [10.67.48.245] ([192.19.223.252]) by smtp.googlemail.com with ESMTPSA id l13-20020ac84a8d000000b00342f8d4d0basm8418189qtq.43.2022.10.21.10.45.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Oct 2022 10:45:46 -0700 (PDT) Message-ID: <9fe9c826-9b1a-0471-e30c-7fa949d2b08e@gmail.com> Date: Fri, 21 Oct 2022 10:45:36 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v2 2/7] mmc: sdhci-of-arasan: Fix SDHCI_RESET_ALL for CQHCI Content-Language: en-US To: Adrian Hunter , Brian Norris Cc: Ulf Hansson , Shawn Lin , Shawn Guo , Fabio Estevam , Haibo Chen , Broadcom internal kernel review list , NXP Linux Team , Pengutronix Kernel Team , Michal Simek , Faiz Abbas , linux-mmc@vger.kernel.org, Jonathan Hunter , Al Cooper , linux-arm-kernel@lists.infradead.org, Sowjanya Komatineni , linux-kernel@vger.kernel.org, Thierry Reding , Sascha Hauer , stable@vger.kernel.org References: <20221019215440.277643-1-briannorris@chromium.org> <20221019145246.v2.2.I29f6a2189e84e35ad89c1833793dca9e36c64297@changeid> <14efb3e6-96cf-f42e-16aa-c45001ec632e@gmail.com> <5f1adbf7-b477-914e-0a07-5c76532e85cd@intel.com> From: Florian Fainelli In-Reply-To: <5f1adbf7-b477-914e-0a07-5c76532e85cd@intel.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221021_104551_728393_6606F4ED X-CRM114-Status: GOOD ( 23.59 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10/19/22 23:29, Adrian Hunter wrote: > On 20/10/22 01:19, Brian Norris wrote: >> On Wed, Oct 19, 2022 at 02:59:39PM -0700, Florian Fainelli wrote: >>> On 10/19/22 14:54, Brian Norris wrote: >>>> The same bug was already found and fixed for two other drivers, in v5.7 >>>> and v5.9: >>>> >>>> 5cf583f1fb9c mmc: sdhci-msm: Deactivate CQE during SDHC reset >>>> df57d73276b8 mmc: sdhci-pci: Fix SDHCI_RESET_ALL for CQHCI for Intel GLK-based controllers >>>> >>>> The latter is especially prescient, saying "other drivers using CQHCI >>>> might benefit from a similar change, if they also have CQHCI reset by >>>> SDHCI_RESET_ALL." >> >>>> --- a/drivers/mmc/host/sdhci-of-arasan.c >>>> +++ b/drivers/mmc/host/sdhci-of-arasan.c >>>> @@ -366,6 +366,9 @@ static void sdhci_arasan_reset(struct sdhci_host *host, u8 mask) >>>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >>>> struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); >>>> + if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL)) >>>> + cqhci_deactivate(host->mmc); >>>> + >>>> sdhci_reset(host, mask); >>> >>> Cannot this be absorbed by sdhci_reset() that all of these drivers appear to >>> be utilizing since you have access to the host and the mask to make that >>> decision? >> >> It potentially could. >> >> I don't know if this is a specified SDHCI behavior that really belongs >> in the common helper, or if this is just a commonly-shared behavior. Per >> the comments I quote above ("if they also have CQHCI reset by >> SDHCI_RESET_ALL"), I chose to leave that as an implementation-specific >> behavior. >> >> I suppose it's not all that harmful to do this even if some SDHCI >> controller doesn't have the same behavior/quirk. >> >> I guess I also don't know if any SDHCI controllers will support command >> queueing (MMC_CAP2_CQE) via somethings *besides* CQHCI. I see >> CQE support in sdhci-sprd.c without CQHCI, although that driver doesn't >> set MMC_CAP2_CQE. > > SDHCI and CQHCI are separate modules and are not dependent, so they cannot > call into each other directly (and should not). A new CQE API would be > needed in mmc_cqe_ops e.g. (*cqe_notify_reset)(struct mmc_host *host), > and wrapped in mmc/host.h: > > static inline void mmc_cqe_notify_reset(struct mmc_host *host) > { > if (host->cqe_ops->cqe_notify_reset) > host->cqe_ops->cqe_notify_reset(host); > } > > Alternatively, you could make a new module for SDHCI/CQHCI helper functions, > although in this case there is so little code it could be static inline and > added in a new include file instead, say sdhci-cqhci.h e.g. > > #include "cqhci.h" > #include "sdhci.h" > > static inline void sdhci_cqhci_reset(struct sdhci_host *host, u8 mask) > { > if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL) && > host->mmc->cqe_private) > cqhci_deactivate(host->mmc); > sdhci_reset(host, mask); > } > I like the simplicity of the inline helper, especially towards backports. May suggest to name it sdhci_and_cqhci_reset() to illustrate that it does both, and does not apply specifically CQHCI that would be "embedded" into SDHCI, but your call here. -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel