blob: 1be76dfd2e43bbbc2571a1cd70ceb2d57dd87c6a [file] [log] [blame]
Return-Path: <SRS0=CqLn=S3=lists.infradead.org=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level:
X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,
SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
by smtp.lore.kernel.org (Postfix) with ESMTP id D95C0C43219
for <infradead-linux-arm-kernel@archiver.kernel.org>; Thu, 25 Apr 2019 14:09:49 +0000 (UTC)
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 mail.kernel.org (Postfix) with ESMTPS id C11A920644
for <infradead-linux-arm-kernel@archiver.kernel.org>; Thu, 25 Apr 2019 14:09:49 +0000 (UTC)
Authentication-Results: mail.kernel.org;
dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="P2juJ3+8";
dkim=fail reason="signature verification failed" (2048-bit key) header.d=st.com header.i=@st.com header.b="Q0NOgg/W"
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C11A920644
Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=st.com
Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type:
Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive:
List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From:
References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date:
Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner;
bh=su1w7GV6G2sWiCkguTBi2Vfn8UgMUCmysKzSGV4qmtA=; b=P2juJ3+8GOMjDODVtKmSoDeb5
rSP/7K27mEeO9BeUUSpfb6amqOEpwgPR9V+DhAdoPu2DlR1AxkoIP9mzStSXrzmyEdRWcOS/T9wGd
3CGGPcYZhRDDev7uZfMO/ai7fnl/i3/LNuKH6dIIjQeR2ws2YebHOxkP3n+BC9SZH5RWv3NhBGiVr
gdEcc07h7POgzcywsW+lK+cKkM8VdMjY6lEIaROIz/mMsar9AtuLyahgjjlVW6oUHOF58I/dYt+C3
sUzYbqndIUhwlsToo0IWjT2MdcsvFBcRhEQuqIjbGKaPT87MuuI1Z4mouZLpg2M4/uKtGDvq5fwGd
RUTeGF5NQ==;
Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org)
by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux))
id 1hJf4P-00051C-0g; Thu, 25 Apr 2019 14:09:49 +0000
Received: from mx07-00178001.pphosted.com ([62.209.51.94])
by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux))
id 1hJf4K-0004zv-Qq
for linux-arm-kernel@lists.infradead.org; Thu, 25 Apr 2019 14:09:46 +0000
Received: from pps.filterd (m0046037.ppops.net [127.0.0.1])
by mx07-00178001.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id
x3PE6pwi031045; Thu, 25 Apr 2019 16:09:37 +0200
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com;
h=subject : to : cc :
references : from : message-id : date : mime-version : in-reply-to :
content-type : content-transfer-encoding; s=STMicroelectronics;
bh=5gU58BSfd/u6NaJSnx/oRSYTlpw/4kusuaMrKHoGSOc=;
b=Q0NOgg/WDhdp6G0dfkiY3OYqV0Xp9HiRvi2nAnG0Cahl9PA1C5jbvDqNPOv5QMePIfgB
2MwjKmMPX2XiG7/FITQy3LVSiqn3Q2cV1KqjqeWii67mTlr4GWQTOqWtAn7v3xLp8J2c
vkAV15Gk5ljIQ74QG9LWWMWxFYb9LEbUgmQPmFJp97gHG1T4FAbVilqtDwAJPqM1FGnl
koHxk2bpjJsV6Bzjv9JZzRcSzhXsMongkBq/jQrtnDAjH+f0GojYNSBr2OeFsaet2fzM
F88qZnpNhCXPtkc4qWnc/wyt2x/VK+5jTG22miwH46akPmAduBAyiFXk2ueavKZAnreN Gg==
Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])
by mx07-00178001.pphosted.com with ESMTP id 2s3bbv8yy8-1
(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);
Thu, 25 Apr 2019 16:09:37 +0200
Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])
by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 03BC431;
Thu, 25 Apr 2019 14:09:37 +0000 (GMT)
Received: from Webmail-eu.st.com (sfhdag6node1.st.com [10.75.127.16])
by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id C993C27DF;
Thu, 25 Apr 2019 14:09:36 +0000 (GMT)
Received: from [10.48.0.237] (10.75.127.47) by SFHDAG6NODE1.st.com
(10.75.127.16) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Thu, 25 Apr
2019 16:09:36 +0200
Subject: Re: [PATCH 1/4] mmc: mmci: avoid fake busy polling
To: Ulf Hansson <ulf.hansson@linaro.org>
References: <1551802205-32188-1-git-send-email-ludovic.Barre@st.com>
<1551802205-32188-2-git-send-email-ludovic.Barre@st.com>
<CAPDyKFoZuQ+hshZpj-qjchKf7enW4ChPd=r_QhK2xtuJcSvqxQ@mail.gmail.com>
<fd57fd63-093e-dd23-5ca4-6bd4f99ecda9@st.com>
<CAPDyKFrmTX6w1ZgwBkEmieCQ5swUQx_O864mHofhNsz3LFC32A@mail.gmail.com>
From: Ludovic BARRE <ludovic.barre@st.com>
Message-ID: <30eae958-fd66-96a2-52a2-661c0646a302@st.com>
Date: Thu, 25 Apr 2019 16:09:35 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <CAPDyKFrmTX6w1ZgwBkEmieCQ5swUQx_O864mHofhNsz3LFC32A@mail.gmail.com>
Content-Language: en-GB
X-Originating-IP: [10.75.127.47]
X-ClientProxiedBy: SFHDAG7NODE3.st.com (10.75.127.21) To SFHDAG6NODE1.st.com
(10.75.127.16)
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, ,
definitions=2019-04-25_11:, , signatures=0
X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3
X-CRM114-CacheID: sfid-20190425_070945_342427_8BA5D2CE
X-CRM114-Status: GOOD ( 27.71 )
X-BeenThere: linux-arm-kernel@lists.infradead.org
X-Mailman-Version: 2.1.21
Precedence: list
List-Id: <linux-arm-kernel.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>,
<mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
List-Post: <mailto:linux-arm-kernel@lists.infradead.org>
List-Help: <mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
<mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>
Cc: DTML <devicetree@vger.kernel.org>,
Alexandre Torgue <alexandre.torgue@st.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
linux-stm32@st-md-mailman.stormreply.com,
Linux ARM <linux-arm-kernel@lists.infradead.org>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org>
Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org
On 4/25/19 12:08 PM, Ulf Hansson wrote:
> On Thu, 25 Apr 2019 at 11:22, Ludovic BARRE <ludovic.barre@st.com> wrote:
>>
>> hi Ulf
>>
>> On 4/23/19 3:39 PM, Ulf Hansson wrote:
>>> On Tue, 5 Mar 2019 at 17:10, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>>>
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> The busy status bit could occurred even if no busy response is
>>>> expected (example cmd11). On sdmmc variant, the busy_detect_flag
>>>> reflects inverted value of d0 state, it's sampled at the end of a
>>>> CMD response and a second time 2 clk cycles after the CMD response.
>>>> To avoid a fake busy, the busy status could be checked and polled
>>>> only if the command has RSP_BUSY flag.
>>>
>>> I would appreciate a better explanation of what this patch really changes.
>>>
>>> The above is giving some background to the behavior of sdmmc variant,
>>> but at this point that variant doesn't even have the
>>> ->variant->busy_detect flag set.
>>>
>>
>> Yes, I will try to explain more and focus on common behavior.
>>
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>> drivers/mmc/host/mmci.c | 19 +++++++++++++------
>>>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>>>> index 387ff14..4901b73 100644
>>>> --- a/drivers/mmc/host/mmci.c
>>>> +++ b/drivers/mmc/host/mmci.c
>>>> @@ -1220,12 +1220,13 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>>>> unsigned int status)
>>>> {
>>>> void __iomem *base = host->base;
>>>> - bool sbc;
>>>> + bool sbc, busy_resp;
>>>>
>>>> if (!cmd)
>>>> return;
>>>>
>>>> sbc = (cmd == host->mrq->sbc);
>>>> + busy_resp = !!(cmd->flags & MMC_RSP_BUSY);
>>>>
>>>> /*
>>>> * We need to be one of these interrupts to be considered worth
>>>> @@ -1239,8 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>>>> /*
>>>> * ST Micro variant: handle busy detection.
>>>> */
>>>> - if (host->variant->busy_detect) {
>>>> - bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY);
>>>> + if (busy_resp && host->variant->busy_detect) {
>>>>
>>>> /* We are busy with a command, return */
>>>> if (host->busy_status &&
>>>> @@ -1253,7 +1253,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>>>> * that the special busy status bit is still set before
>>>> * proceeding.
>>>> */
>>>> - if (!host->busy_status && busy_resp &&
>>>> + if (!host->busy_status &&
>>>> !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
>>>> (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
>>>
>>> All the changes above makes perfect sense to me, but looks more like a
>>> cleanup of the code, rather than actually changing the behavior.
>>
>> yes, few changing (this just avoid to enter in
>> "if (host->variant->busy_detect)") at each time.
>> I could move this part in cleanup patch (before this patch)
>
> Sounds good to me!
>
>>
>>>
>>>>
>>>> @@ -1508,6 +1508,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
>>>> {
>>>> struct mmci_host *host = dev_id;
>>>> u32 status;
>>>> + bool busy_resp;
>>>> int ret = 0;
>>>>
>>>> spin_lock(&host->lock);
>>>> @@ -1550,9 +1551,15 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
>>>> }
>>>>
>>>> /*
>>>> - * Don't poll for busy completion in irq context.
>>>> + * Don't poll for:
>>>> + * -busy completion in irq context.
>>>> + * -no busy response expected.
>>>> */
>>>> - if (host->variant->busy_detect && host->busy_status)
>>>> + busy_resp = host->cmd ?
>>>> + !!(host->cmd->flags & MMC_RSP_BUSY) : false;
>>>
>>> This doesn't make sense to me, but I may be missing something.
>>>
>>> host->busy_status is being updated by mmci_cmd_irq() and only when
>>> MMC_RSP_BUSY is set for the command in flight. In other words,
>>> checking for MMC_RSP_BUSY here as well is redundant. No?
>>
>> In mmci_irq the "do while" loops until the status is totally cleared.
>>
>> Today (for variant with busy_detect option), the status busy_detect_flag
>> is excluded only while busy_status period (command with MMC_RSP_BUSY and
>> while busy line is low => "busy_status=1")
>>
>> On SDMMC variant I noticed that busy_detect_flag status could be enabled
>> even if the command is not MMC_RSP_BUSY, for example sdmmc variant stay
>> in loop while cmd11 voltage switch.
>
> Right, I see.
>
>>
>> So I wish extend host->variant->busy_detect_flag exclusion for all
>> commands which is not a MMC_RSP_BUSY. I suppose that other variants
>> could have the same behavior, and else there is no impact, normally.
>
> I am guessing this is because the variant->busy_dpsm_flag has been set
> in the datactrl register, which is needed for mmci_card_busy().
>
> That said, I am kind of wondering if we ever should need repeat the
> while loop if 'status' contains the bit for
> host->variant->busy_detect_flag. I mean we have already called
> mmci_cmd_irq() to handle it.
>
> So, couldn't we just always do:
>
> if (host->variant->busy_detect_flag)
> status &= ~host->variant->busy_detect_flag;
>
> No?
yes that make sense, I launched tests on sdmmc and it's ok.
I think, that we could take on this solution.
If it's ok for you, I resend a series with all modifications.
Regards
Ludo
>
>>
>>>
>>>> +
>>>> + if (host->variant->busy_detect &&
>>>> + (!busy_resp || host->busy_status))
>>>> status &= ~host->variant->busy_detect_flag;
>>>>
>>>> ret = 1;
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> Kind regards
>>> Uffe
>>>
>
> Kind regards
> Uffe
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel