Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs

[Evgeniy Didin <Evgeniy.Didin@synopsys.com>]

Hello Ulf,

On Thu, 2018-03-15 at 10:57 +0100, Ulf Hansson wrote:
> On 14 March 2018 at 20:30, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
> > It was found that in IDMAC mode after soft-reset driver switches
> > to PIO mode.
> > 
> > That's what happens in case of DTO timeout overflow calculation failure:
> > 1. soft-reset is called
> > 2. driver restarts dma
> > 3. descriptors states are checked, one of descriptor is owned by the IDMAC.
> > 4. driver can't use DMA and then switches to PIO mode.
> > 
> > Failure was already fixed in:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dmmc_msg48125.html&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=vQk-RIbjwN0zvlwiMSpq3LYUTNf7Gqc4ujhosYI
> > TtAw&m=6rPWpKUYQD3kY-2OEikJUWyEJvwKJljWHFC8rd2TCak&s=JjX0Dx8-eSyW2cATMsnG1eAzrKgoDkS1bcS5XYrVtlE&e=.
> 
> Evgeniy,
> 
> It seems like I should squash this fix into the above commit? Makes sense?
IMHO it is not a good idea, because this fix is orthogonal to the above commit.
Debugging drto overflow issue we found this strange behavior.
This might be general issue of soft-reset. So I think this should be separate patch.
Also if these patches are independent, it will be easier to backport them.

Best regards,
Evgeniy Didin

> Kind regards
> Uffe
> 
> > 
> > Behaviour while soft-reset is not something we except or
> > even want to happen. So we switch from dw_mci_idmac_reset
> > to dw_mci_idmac_init, so descriptors are cleaned before starting dma.
> > 
> > And while at it explicitly zero des0 which otherwise might
> > contain garbage as being allocated by dmam_alloc_coherent().
> > 
> > Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > Cc: Shawn Lin <shawn.lin@rock-chips.com>
> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > Cc: linux-snps-arc@lists.infradead.org
> > ---
> >  drivers/mmc/host/dw_mmc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 0aa39975f33b..26f2ac107702 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -558,6 +558,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
> >                                         (sizeof(struct idmac_desc_64addr) *
> >                                                         (i + 1))) >> 32;
> >                         /* Initialize reserved and buffer size fields to "0" */
> > +                       p->des0 = 0;
> >                         p->des1 = 0;
> >                         p->des2 = 0;
> >                         p->des3 = 0;
> > @@ -580,6 +581,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
> >                      i++, p++) {
> >                         p->des3 = cpu_to_le32(host->sg_dma +
> >                                         (sizeof(struct idmac_desc) * (i + 1)));
> > +                       p->des0 = 0;
> >                         p->des1 = 0;
> >                 }
> > 
> > @@ -1795,8 +1797,8 @@ static bool dw_mci_reset(struct dw_mci *host)
> >         }
> > 
> >         if (host->use_dma == TRANS_MODE_IDMAC)
> > -               /* It is also recommended that we reset and reprogram idmac */
> > -               dw_mci_idmac_reset(host);
> > +               /* It is also required that we reinit idmac */
> > +               dw_mci_idmac_init(host);
> > 
> >         ret = true;
> > 
> > --
> > 2.11.0
> > 
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc