linux-kernel - Re: [PATCH 1/4] mmc: mmci: avoid fake busy polling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <15ce68e6-136f-7e9b-553c-968a0653a9b8@st.com>
Date:   Wed, 6 Mar 2019 11:08:57 +0100
From:   Ludovic BARRE <ludovic.barre@...com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     Rob Herring <robh+dt@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH 1/4] mmc: mmci: avoid fake busy polling

Hi Ulf

I could have a snowball board, I don't remember if it a ux500v1 or v2?
This board has been removed of u-boot mainline, do you have a specific
repository for snowball or what branch do you advise?
do you have a snowball wiki (up-to-date) for flashing...

BR
Ludo
On 3/6/19 10:49 AM, Ulf Hansson wrote:
> On Wed, 6 Mar 2019 at 10:04, Ludovic BARRE <ludovic.barre@...com> wrote:
>>
>> Hi Ulf
>>
>> On 3/6/19 10:00 AM, Ulf Hansson wrote:
>>> On Tue, 5 Mar 2019 at 17:10, Ludovic Barre <ludovic.Barre@...com> wrote:
>>>>
>>>> From: Ludovic Barre <ludovic.barre@...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.
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@...com>
>>>
>>> Before I review this, can you tell what HW you have tested this on?
>>
>> I tested on stm32mp157c (stm32_sdmmc variant)
> 
> Okay, I see. So we need to get this tested for the ux500v2 variant as
> well. I try to get some time to do that, soon.
> 
> However it seems like you could benefit from having one of those
> boards yourself. It would speed up the process, as you wouldn't have
> to rely on me doing the test. :-) Is there a chance of you could dig
> up some of these old boards from somewhere?
> 
> Kind regards
> Uffe
> 
>>
>>>
>>> Kind regards
>>> Uffe
>>>
>>>> ---
>>>>    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)) {
>>>>
>>>> @@ -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;
>>>> +
>>>> +               if (host->variant->busy_detect &&
>>>> +                   (!busy_resp || host->busy_status))
>>>>                           status &= ~host->variant->busy_detect_flag;
>>>>
>>>>                   ret = 1;
>>>> --
>>>> 2.7.4
>>>>