mbox series

[0/3] mmc: tmio and renesas_sdhi_internal_dmac: fix dma unmapping

Message ID 1590044466-28372-1-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
Headers show
Series mmc: tmio and renesas_sdhi_internal_dmac: fix dma unmapping | expand

Message

Yoshihiro Shimoda May 21, 2020, 7:01 a.m. UTC
This patch series is based on mmc.git / next branch.

I guess we should add such a code into renesas_sdhi_sys_dmac and
uniphier-sd drivers too. But, for now, I fixed
renesas_sdhi_internal_dmac only.

Yoshihiro Shimoda (3):
  mmc: tmio: core: Add end operation into tmio_mmc_dma_ops
  mmc: renesas_sdhi_internal_dmac: clean up the code for dma complete
  mmc: renesas_sdhi_internal_dmac: Fix dma unmapping in error cases

 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 28 +++++++++++++++++++++++----
 drivers/mmc/host/tmio_mmc.h                   |  3 +++
 drivers/mmc/host/tmio_mmc_core.c              |  8 ++++++++
 3 files changed, 35 insertions(+), 4 deletions(-)

Comments

Yoshihiro Shimoda May 21, 2020, 7:44 a.m. UTC | #1
Hi again,

> From: Yoshihiro Shimoda, Sent: Thursday, May 21, 2020 4:01 PM
> To: ulf.hansson@linaro.org; wsa+renesas@sang-engineering.com
> Cc: linux-mmc@vger.kernel.org; linux-renesas-soc@vger.kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Subject: [PATCH 0/3] mmc: tmio and renesas_sdhi_internal_dmac: fix dma unmapping
> 
> This patch series is based on mmc.git / next branch.

Note that this patch series is tested by using additional debug code [1],
because there is difficult to reproduce this issue. Before apply patch,
When I enabled CONFIG_DMA_API_DEBUG and CONFIG_DMA_API_DEBUG_SG,
I observed lacking dma unmapping on /sys/kernel/debug/dma-api/dump.
And then I confirmed the patch can fix the issue.

---
[1]
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index adc2bf7..1df00f6 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -192,6 +192,7 @@ struct tmio_mmc_host {
 	void (*hs400_complete)(struct tmio_mmc_host *host);
 
 	const struct tmio_mmc_dma_ops *dma_ops;
+	int debug;
 };
 
 struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 946fb01..f8fe905 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -552,7 +552,8 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
 		cmd->resp[0] = cmd->resp[3];
 	}
 
-	if (stat & TMIO_STAT_CMDTIMEOUT)
+	host->debug++;
+	if (stat & TMIO_STAT_CMDTIMEOUT || !(host->debug & 0xff))
 		cmd->error = -ETIMEDOUT;
 	else if ((stat & TMIO_STAT_CRCFAIL && cmd->flags & MMC_RSP_CRC) ||
 		 stat & TMIO_STAT_STOPBIT_ERR ||
Wolfram Sang June 2, 2020, 12:51 p.m. UTC | #2
Hi Shimoda-san,

thanks for the patches and for providing a test case. I was not able to
reproduce the issue, though. I'll explain...

> Note that this patch series is tested by using additional debug code [1],
> because there is difficult to reproduce this issue. Before apply patch,
> When I enabled CONFIG_DMA_API_DEBUG and CONFIG_DMA_API_DEBUG_SG,
> I observed lacking dma unmapping on /sys/kernel/debug/dma-api/dump.
> And then I confirmed the patch can fix the issue.

So, I have this debug patch applied on top of mmc/next. I have the above
CONFIG_ symbols enabled. I have _not_ applied your three patches which
fix the issue. I mounted the eMMC and read a large file. I see the
injected timeouts happening:

[   94.079560] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   94.088668] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   94.097727] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   94.106768] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   94.115848] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   99.300589] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)

But I do not see any output from the DMA debug system about a missing
unmapping. I expected that, though, because your fixes are not applied.
The testfile could even be correctly checksummed after reading, just
awfully slow, of course.

Am I missing something?

All the best,

   Wolfram
Yoshihiro Shimoda June 4, 2020, 2:52 a.m. UTC | #3
Hi Wolfram-san,

> From: wsa+renesas@sang-engineering.com, Sent: Tuesday, June 2, 2020 9:52 PM
> 
> Hi Shimoda-san,
> 
> thanks for the patches and for providing a test case. I was not able to
> reproduce the issue, though. I'll explain...

Thank you for trying to reproduce the issue!

> > Note that this patch series is tested by using additional debug code [1],
> > because there is difficult to reproduce this issue. Before apply patch,
> > When I enabled CONFIG_DMA_API_DEBUG and CONFIG_DMA_API_DEBUG_SG,
> > I observed lacking dma unmapping on /sys/kernel/debug/dma-api/dump.
> > And then I confirmed the patch can fix the issue.
> 
> So, I have this debug patch applied on top of mmc/next. I have the above
> CONFIG_ symbols enabled. I have _not_ applied your three patches which
> fix the issue. I mounted the eMMC and read a large file. I see the
> injected timeouts happening:
> 
> [   94.079560] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> [   94.088668] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> [   94.097727] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> [   94.106768] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> [   94.115848] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> [   99.300589] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
> 
> But I do not see any output from the DMA debug system about a missing
> unmapping. I expected that, though, because your fixes are not applied.
> The testfile could even be correctly checksummed after reading, just
> awfully slow, of course.

I'm sorry, I should have shared my log. My test case is:
- Use dd command as background.
- Read /sys/kernel/debug/dma-api/dump while the dd command is running.
--> "ee140000.sd" lines are increased gradually.

---- Log ---
~ # dd if=/dev/mmcblk1 of=/dev/null bs=512k &
~ # grep sd /sys/kernel/debug/dma-api/dump
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8274 P=736a8a000 N=736a8a D=740a4000 L=3000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8603 P=736a51000 N=736a51 D=74337000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
~ # [   56.797275] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
[   56.857055] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   56.866231] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   56.875330] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   56.884313] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   56.893334] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   61.917228] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
[   61.967599] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   61.976722] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   61.985726] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   61.994847] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   62.003772] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle

~ # 
~ # [   67.037259] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
[   67.098616] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   67.107610] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   67.116595] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   67.125591] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   67.134819] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle

~ # 
~ # grep sd /sys/kernel/debug/dma-api/dump
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8274 P=736a8a000 N=736a8a D=740a4000 L=3000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8603 P=736a51000 N=736a51 D=74337000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8807 P=737c2c000 N=737c2c D=744ce000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8993 P=736b80000 N=736b80 D=74642000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 9251 P=73665f000 N=73665f D=74847000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
~ # [   72.157278] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
[   72.217556] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   72.226595] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   72.235577] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   72.244558] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   72.253580] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   77.277235] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
[   77.339935] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   77.348953] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   77.358283] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   77.367271] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
[   77.376215] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle

~ # 
~ # grep sd /sys/kernel/debug/dma-api/dump
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8274 P=736a8a000 N=736a8a D=740a4000 L=3000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8603 P=736a51000 N=736a51 D=74337000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8807 P=737c2c000 N=737c2c D=744ce000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 8993 P=736b80000 N=736b80 D=74642000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 9251 P=73665f000 N=73665f D=74847000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 9470 P=739af2000 N=739af2 D=749fc000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
ee140000.sd renesas_sdhi_internal_dmac scather-gather idx 9794 P=736867000 N=736867 D=74c85000 L=1000 DMA_FROM_DEVICE dma map error check not applicable
----

Best regards,
Yoshihiro Shimoda
Ulf Hansson July 6, 2020, 1:16 p.m. UTC | #4
On Thu, 4 Jun 2020 at 04:52, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>
> Hi Wolfram-san,
>
> > From: wsa+renesas@sang-engineering.com, Sent: Tuesday, June 2, 2020 9:52 PM
> >
> > Hi Shimoda-san,
> >
> > thanks for the patches and for providing a test case. I was not able to
> > reproduce the issue, though. I'll explain...
>
> Thank you for trying to reproduce the issue!
>
> > > Note that this patch series is tested by using additional debug code [1],
> > > because there is difficult to reproduce this issue. Before apply patch,
> > > When I enabled CONFIG_DMA_API_DEBUG and CONFIG_DMA_API_DEBUG_SG,
> > > I observed lacking dma unmapping on /sys/kernel/debug/dma-api/dump.
> > > And then I confirmed the patch can fix the issue.
> >
> > So, I have this debug patch applied on top of mmc/next. I have the above
> > CONFIG_ symbols enabled. I have _not_ applied your three patches which
> > fix the issue. I mounted the eMMC and read a large file. I see the
> > injected timeouts happening:
> >
> > [   94.079560] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> > [   94.088668] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> > [   94.097727] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> > [   94.106768] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> > [   94.115848] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for SD bus idle
> > [   99.300589] renesas_sdhi_internal_dmac ee140000.sd: timeout waiting for hardware interrupt (CMD13)
> >
> > But I do not see any output from the DMA debug system about a missing
> > unmapping. I expected that, though, because your fixes are not applied.
> > The testfile could even be correctly checksummed after reading, just
> > awfully slow, of course.
>
> I'm sorry, I should have shared my log. My test case is:
> - Use dd command as background.
> - Read /sys/kernel/debug/dma-api/dump while the dd command is running.
> --> "ee140000.sd" lines are increased gradually.

[...]

Just wanted to check if this is ready to go or more tests are needed?

Kind regards
Uffe
Wolfram Sang July 6, 2020, 2 p.m. UTC | #5
> Just wanted to check if this is ready to go or more tests are needed?

From my tests, this patch series fixes the issue. I'd just like to avoid
the extra callback. However, my tries to do that failed so far. And now
I'll be away for two weeks.

Dunno, maybe we merge the series and if I come up with something else
that works, we can add it incrementally?
Ulf Hansson July 6, 2020, 2:49 p.m. UTC | #6
On Mon, 6 Jul 2020 at 16:00, wsa+renesas@sang-engineering.com
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > Just wanted to check if this is ready to go or more tests are needed?
>
> From my tests, this patch series fixes the issue. I'd just like to avoid
> the extra callback. However, my tries to do that failed so far. And now
> I'll be away for two weeks.
>
> Dunno, maybe we merge the series and if I come up with something else
> that works, we can add it incrementally?

Sounds reasonable to me.

So, applied for next, thanks!

Kind regards
Uffe
Wolfram Sang July 6, 2020, 2:51 p.m. UTC | #7
On Mon, Jul 06, 2020 at 04:49:20PM +0200, Ulf Hansson wrote:
> On Mon, 6 Jul 2020 at 16:00, wsa+renesas@sang-engineering.com
> <wsa+renesas@sang-engineering.com> wrote:
> >
> >
> > > Just wanted to check if this is ready to go or more tests are needed?
> >
> > From my tests, this patch series fixes the issue. I'd just like to avoid
> > the extra callback. However, my tries to do that failed so far. And now
> > I'll be away for two weeks.
> >
> > Dunno, maybe we merge the series and if I come up with something else
> > that works, we can add it incrementally?
> 
> Sounds reasonable to me.
> 
> So, applied for next, thanks!

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>