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

[Ludovic BARRE]

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@xxxxxx> 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@xxxxxx> wrote:
> > > > From: Ludovic Barre <ludovic.barre@xxxxxx>
> > > >
> > > > 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@xxxxxx>
> > >
> > > 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