* [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
@ 2018-03-14 19:30 Evgeniy Didin
2018-03-15 9:57 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Evgeniy Didin @ 2018-03-14 19:30 UTC (permalink / raw)
To: linux-mmc
Cc: Jaehoon Chung, Ulf Hansson, Shawn Lin, Alexey Brodkin,
Andy Shevchenko, Evgeniy Didin, Jisheng Zhang, linux-snps-arc,
Eugeniy Paltsev
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://www.spinics.net/lists/linux-mmc/msg48125.html.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
2018-03-14 19:30 [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs Evgeniy Didin
@ 2018-03-15 9:57 ` Ulf Hansson
2018-03-15 13:18 ` Evgeniy Didin
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2018-03-15 9:57 UTC (permalink / raw)
To: Evgeniy Didin
Cc: Shawn Lin, Alexey Brodkin, linux-mmc@vger.kernel.org,
Jisheng Zhang, Jaehoon Chung, Andy Shevchenko, linux-snps-arc,
Eugeniy Paltsev
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://www.spinics.net/lists/linux-mmc/msg48125.html.
Evgeniy,
It seems like I should squash this fix into the above commit? Makes sense?
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
2018-03-15 9:57 ` Ulf Hansson
@ 2018-03-15 13:18 ` Evgeniy Didin
2018-03-15 13:39 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Evgeniy Didin @ 2018-03-15 13:18 UTC (permalink / raw)
To: ulf.hansson@linaro.org, Evgeniy.Didin@synopsys.com
Cc: shawn.lin@rock-chips.com, Alexey.Brodkin@synopsys.com,
linux-mmc@vger.kernel.org, Jisheng.Zhang@synaptics.com,
jh80.chung@samsung.com, andy.shevchenko@gmail.com,
linux-snps-arc@lists.infradead.org, Eugeniy.Paltsev@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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
2018-03-15 13:18 ` Evgeniy Didin
@ 2018-03-15 13:39 ` Ulf Hansson
2018-03-15 17:24 ` Evgeniy Didin
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2018-03-15 13:39 UTC (permalink / raw)
To: Evgeniy Didin
Cc: shawn.lin@rock-chips.com, Alexey.Brodkin@synopsys.com,
linux-mmc@vger.kernel.org, Jisheng.Zhang@synaptics.com,
jh80.chung@samsung.com, andy.shevchenko@gmail.com,
linux-snps-arc@lists.infradead.org, Eugeniy.Paltsev@synopsys.com
On 15 March 2018 at 14:18, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
> 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.
Okay! Thanks for clarifying.
> 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.
Should I add a stable tag then?
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
2018-03-15 13:39 ` Ulf Hansson
@ 2018-03-15 17:24 ` Evgeniy Didin
2018-03-16 7:41 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Evgeniy Didin @ 2018-03-15 17:24 UTC (permalink / raw)
To: ulf.hansson@linaro.org, Evgeniy.Didin@synopsys.com
Cc: shawn.lin@rock-chips.com, Alexey.Brodkin@synopsys.com,
linux-mmc@vger.kernel.org, Jisheng.Zhang@synaptics.com,
jh80.chung@samsung.com, andy.shevchenko@gmail.com,
linux-snps-arc@lists.infradead.org, Eugeniy.Paltsev@synopsys.com
On Thu, 2018-03-15 at 14:39 +0100, Ulf Hansson wrote:
> On 15 March 2018 at 14:18, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
> > 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-RIbjwN0zvlwiMSpq3LYUTNf7Gqc4ujh
> > > > osYI
> > > > 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.
>
> Okay! Thanks for clarifying.
>
> > 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.
>
> Should I add a stable tag then?
Sure. The issue can happen since v3.17, because starting with this version function dw_mci_reset was implemented.
Commit 3a33a94ce270 ("mmc: dw_mmc: change to use recommended reset procedure").
Best regards,
Evgeniy Didin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs
2018-03-15 17:24 ` Evgeniy Didin
@ 2018-03-16 7:41 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2018-03-16 7:41 UTC (permalink / raw)
To: Evgeniy Didin
Cc: shawn.lin@rock-chips.com, Alexey.Brodkin@synopsys.com,
linux-mmc@vger.kernel.org, Jisheng.Zhang@synaptics.com,
jh80.chung@samsung.com, andy.shevchenko@gmail.com,
linux-snps-arc@lists.infradead.org, Eugeniy.Paltsev@synopsys.com
On 15 March 2018 at 18:24, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
> On Thu, 2018-03-15 at 14:39 +0100, Ulf Hansson wrote:
>> On 15 March 2018 at 14:18, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
>> > 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-RIbjwN0zvlwiMSpq3LYUTNf7Gqc4ujh
>> > > > osYI
>> > > > 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.
>>
>> Okay! Thanks for clarifying.
>>
>> > 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.
>>
>> Should I add a stable tag then?
>
> Sure. The issue can happen since v3.17, because starting with this version function dw_mci_reset was implemented.
> Commit 3a33a94ce270 ("mmc: dw_mmc: change to use recommended reset procedure").
Alright. I tried applying it, and the first kernel version that seems
to work is 4.4. Thus I am adding that tag, if you or other people want
it for an earlier version, please to send a backported version to
@stable.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-16 7:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-14 19:30 [PATCH] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs Evgeniy Didin
2018-03-15 9:57 ` Ulf Hansson
2018-03-15 13:18 ` Evgeniy Didin
2018-03-15 13:39 ` Ulf Hansson
2018-03-15 17:24 ` Evgeniy Didin
2018-03-16 7:41 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox