Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

[Russell King - ARM Linux]

I'm not sure who's responsible for this, but the nonblocking MMC stuff is
broken on OMAPs HSMMC:

mmcblk0: error -84 transferring data, sector 149201, nr 64, cmd response 0x900, card status 0xb00                                                               
mmcblk0: retrying using single block read                                       
------------[ cut here ]------------                                            
WARNING: at /home/rmk/git/linux-2.6-rmk/lib/dma-debug.c:811 check_unmap+0x1ac/0x764()
omap_hsmmc omap_hsmmc.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x0000000080933000] [size=20480 bytes]
Modules linked in:                                                              
Backtrace:                                                                      
[<c0017874>] (dump_backtrace+0x0/0x10c) from [<c02ce8ac>] (dump_stack+0x18/0x1c)
 r7:c1abfcb8 r6:c0186248 r5:c037de51 r4:0000032b                                
[<c02ce894>] (dump_stack+0x0/0x1c) from [<c0039ed4>] (warn_slowpath_common+0x58/0x70)
[<c0039e7c>] (warn_slowpath_common+0x0/0x70) from [<c0039f90>] (warn_slowpath_fmt+0x38/0x40)                                                                    
 r8:c1abfd50 r7:00000000 r6:00005000 r5:00000000 r4:80933000                    
[<c0039f58>] (warn_slowpath_fmt+0x0/0x40) from [<c0186248>] (check_unmap+0x1ac/0x764)                                                                           
 r3:c0367d55 r2:c037e24d                                                        
[<c018609c>] (check_unmap+0x0/0x764) from [<c0186978>] (debug_dma_unmap_sg+0x100/0x134)                                                                         
[<c0186878>] (debug_dma_unmap_sg+0x0/0x134) from [<c0019770>] (dma_unmap_sg+0x24/0x7c)                                                                          
[<c001974c>] (dma_unmap_sg+0x0/0x7c) from [<c0207220>] (omap_hsmmc_post_req+0x48/0x54)                                                                          
[<c02071d8>] (omap_hsmmc_post_req+0x0/0x54) from [<c01fb644>] (mmc_start_req+0x9c/0x128)                                                                        
 r4:c1a76000                                                                    
[<c01fb5a8>] (mmc_start_req+0x0/0x128) from [<c02049fc>] (mmc_blk_issue_rw_rq+0x80/0x460)                                                                       
 r8:c1ab5c00 r7:00000001 r6:c1ab5824 r5:c1ab5824 r4:c1ab5c00                    
[<c020497c>] (mmc_blk_issue_rw_rq+0x0/0x460) from [<c02050a8>] (mmc_blk_issue_rq+0x2cc/0x2fc)                                                                   
[<c0204ddc>] (mmc_blk_issue_rq+0x0/0x2fc) from [<c02056c4>] (mmc_queue_thread+0xa0/0x104)                                                                       
[<c0205624>] (mmc_queue_thread+0x0/0x104) from [<c0054f8c>] (kthread+0x88/0x90) 
[<c0054f04>] (kthread+0x0/0x90) from [<c003d9a4>] (do_exit+0x0/0x618)           
 r7:00000013 r6:c003d9a4 r5:c0054f04 r4:c1a7bc7c                                
---[ end trace 3314ad56daf5d14f ]---

Luckily thinks continue to work, but clearly releasing DMA mappings which
drivers don't own is bad news.  Unfortunately, I don't have the bandwidth
to be able to investigate this at present - well, I could do but then I'd
have to drop working on the OMAP4 vs generic suspend/resume code and learn
about something I've no current clue about.

Can someone please investigate and fix whatever is broken.

Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

[Santosh]

+ Balaji,

On Friday 26 August 2011 09:49 PM, Russell King - ARM Linux wrote:
> I'm not sure who's responsible for this, but the nonblocking MMC stuff is
> broken on OMAPs HSMMC:
>
> mmcblk0: error -84 transferring data, sector 149201, nr 64, cmd response 0x900, card status 0xb00
> mmcblk0: retrying using single block read
> ------------[ cut here ]------------
> WARNING: at /home/rmk/git/linux-2.6-rmk/lib/dma-debug.c:811 check_unmap+0x1ac/0x764()
> omap_hsmmc omap_hsmmc.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x0000000080933000] [size=20480 bytes]
> Modules linked in:
> Backtrace:
> [<c0017874>] (dump_backtrace+0x0/0x10c) from [<c02ce8ac>] (dump_stack+0x18/0x1c)
>   r7:c1abfcb8 r6:c0186248 r5:c037de51 r4:0000032b
> [<c02ce894>] (dump_stack+0x0/0x1c) from [<c0039ed4>] (warn_slowpath_common+0x58/0x70)
> [<c0039e7c>] (warn_slowpath_common+0x0/0x70) from [<c0039f90>] (warn_slowpath_fmt+0x38/0x40)
>   r8:c1abfd50 r7:00000000 r6:00005000 r5:00000000 r4:80933000
> [<c0039f58>] (warn_slowpath_fmt+0x0/0x40) from [<c0186248>] (check_unmap+0x1ac/0x764)
>   r3:c0367d55 r2:c037e24d
> [<c018609c>] (check_unmap+0x0/0x764) from [<c0186978>] (debug_dma_unmap_sg+0x100/0x134)
> [<c0186878>] (debug_dma_unmap_sg+0x0/0x134) from [<c0019770>] (dma_unmap_sg+0x24/0x7c)
> [<c001974c>] (dma_unmap_sg+0x0/0x7c) from [<c0207220>] (omap_hsmmc_post_req+0x48/0x54)
> [<c02071d8>] (omap_hsmmc_post_req+0x0/0x54) from [<c01fb644>] (mmc_start_req+0x9c/0x128)
>   r4:c1a76000
> [<c01fb5a8>] (mmc_start_req+0x0/0x128) from [<c02049fc>] (mmc_blk_issue_rw_rq+0x80/0x460)
>   r8:c1ab5c00 r7:00000001 r6:c1ab5824 r5:c1ab5824 r4:c1ab5c00
> [<c020497c>] (mmc_blk_issue_rw_rq+0x0/0x460) from [<c02050a8>] (mmc_blk_issue_rq+0x2cc/0x2fc)
> [<c0204ddc>] (mmc_blk_issue_rq+0x0/0x2fc) from [<c02056c4>] (mmc_queue_thread+0xa0/0x104)
> [<c0205624>] (mmc_queue_thread+0x0/0x104) from [<c0054f8c>] (kthread+0x88/0x90)
> [<c0054f04>] (kthread+0x0/0x90) from [<c003d9a4>] (do_exit+0x0/0x618)
>   r7:00000013 r6:c003d9a4 r5:c0054f04 r4:c1a7bc7c
> ---[ end trace 3314ad56daf5d14f ]---
>
> Luckily thinks continue to work, but clearly releasing DMA mappings which
> drivers don't own is bad news.  Unfortunately, I don't have the bandwidth
> to be able to investigate this at present - well, I could do but then I'd
> have to drop working on the OMAP4 vs generic suspend/resume code and learn
> about something I've no current clue about.
>
Please continue your help on generic suspend.

> Can someone please investigate and fix whatever is broken.
>
Will find somebody to look at this issue.

Regards
Santosh

Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

[Per Forlin]

On 26 August 2011 18:28, Santosh <santosh.shilimkar@ti.com> wrote:
> + Balaji,
>
> On Friday 26 August 2011 09:49 PM, Russell King - ARM Linux wrote:
>>
>> I'm not sure who's responsible for this, but the nonblocking MMC stuff is
>> broken on OMAPs HSMMC:
>>
>> mmcblk0: error -84 transferring data, sector 149201, nr 64, cmd response
>> 0x900, card status 0xb00
>> mmcblk0: retrying using single block read
>> ------------[ cut here ]------------
>> WARNING: at /home/rmk/git/linux-2.6-rmk/lib/dma-debug.c:811
>> check_unmap+0x1ac/0x764()
>> omap_hsmmc omap_hsmmc.0: DMA-API: device driver tries to free DMA memory
>> it has not allocated [device address=0x0000000080933000] [size=20480 bytes]
>> Modules linked in:
>> Backtrace:
>> [<c0017874>] (dump_backtrace+0x0/0x10c) from [<c02ce8ac>]
>> (dump_stack+0x18/0x1c)
>>  r7:c1abfcb8 r6:c0186248 r5:c037de51 r4:0000032b
>> [<c02ce894>] (dump_stack+0x0/0x1c) from [<c0039ed4>]
>> (warn_slowpath_common+0x58/0x70)
>> [<c0039e7c>] (warn_slowpath_common+0x0/0x70) from [<c0039f90>]
>> (warn_slowpath_fmt+0x38/0x40)
>>  r8:c1abfd50 r7:00000000 r6:00005000 r5:00000000 r4:80933000
>> [<c0039f58>] (warn_slowpath_fmt+0x0/0x40) from [<c0186248>]
>> (check_unmap+0x1ac/0x764)
>>  r3:c0367d55 r2:c037e24d
>> [<c018609c>] (check_unmap+0x0/0x764) from [<c0186978>]
>> (debug_dma_unmap_sg+0x100/0x134)
>> [<c0186878>] (debug_dma_unmap_sg+0x0/0x134) from [<c0019770>]
>> (dma_unmap_sg+0x24/0x7c)
>> [<c001974c>] (dma_unmap_sg+0x0/0x7c) from [<c0207220>]
>> (omap_hsmmc_post_req+0x48/0x54)
>> [<c02071d8>] (omap_hsmmc_post_req+0x0/0x54) from [<c01fb644>]
>> (mmc_start_req+0x9c/0x128)
>>  r4:c1a76000
>> [<c01fb5a8>] (mmc_start_req+0x0/0x128) from [<c02049fc>]
>> (mmc_blk_issue_rw_rq+0x80/0x460)
>>  r8:c1ab5c00 r7:00000001 r6:c1ab5824 r5:c1ab5824 r4:c1ab5c00
>> [<c020497c>] (mmc_blk_issue_rw_rq+0x0/0x460) from [<c02050a8>]
>> (mmc_blk_issue_rq+0x2cc/0x2fc)
>> [<c0204ddc>] (mmc_blk_issue_rq+0x0/0x2fc) from [<c02056c4>]
>> (mmc_queue_thread+0xa0/0x104)
>> [<c0205624>] (mmc_queue_thread+0x0/0x104) from [<c0054f8c>]
>> (kthread+0x88/0x90)
>> [<c0054f04>] (kthread+0x0/0x90) from [<c003d9a4>] (do_exit+0x0/0x618)
>>  r7:00000013 r6:c003d9a4 r5:c0054f04 r4:c1a7bc7c
>> ---[ end trace 3314ad56daf5d14f ]---
>>
>> Luckily thinks continue to work, but clearly releasing DMA mappings which
>> drivers don't own is bad news.  Unfortunately, I don't have the bandwidth
>> to be able to investigate this at present - well, I could do but then I'd
>> have to drop working on the OMAP4 vs generic suspend/resume code and learn
>> about something I've no current clue about.
>>
> Please continue your help on generic suspend.
>
>> Can someone please investigate and fix whatever is broken.
>>
> Will find somebody to look at this issue.
>
I had a look at this and my guess is that the same mapped DMA memory
is unmapped twice. First the memory is unmapped in
omap_hsmmc_dma_cleanup() due to the mmc error, then later in
omap_hsmmc_post_req(). Here is a patch to resolve that. I haven't had
the chance to test it yet though.

---
 drivers/mmc/host/omap_hsmmc.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 21e4a79..7f40767 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1011,6 +1011,7 @@ static void omap_hsmmc_dma_cleanup(struct
omap_hsmmc_host *host, int errno)
                        host->data->sg_len,
                        omap_hsmmc_get_dma_dir(host, host->data));
                omap_free_dma(dma_ch);
+               data->host_cookie = 0;
        }
        host->data = NULL;
 }
@@ -1576,8 +1577,10 @@ static void omap_hsmmc_post_req(struct mmc_host
*mmc, struct mmc_request *mrq,
        struct mmc_data *data = mrq->data;

        if (host->use_dma) {
-               dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
-                            omap_hsmmc_get_dma_dir(host, data));
+               if (data->host_cookie)
+                       dma_unmap_sg(mmc_dev(host->mmc), data->sg,
+                                    data->sg_len,
+                                    omap_hsmmc_get_dma_dir(host, data));
                data->host_cookie = 0;


I also checked the mmci code for this same issue and mmci doesn't have
the same bug.
MMCI works fine in 3.1-rc2 even though the code is "wrong". I propose
this change.
@@ -529,7 +529,7 @@ static void mmci_post_request(struct mmc_host
*mmc, struct mmc_request *mrq,
        if (chan) {
                if (err)
                        dmaengine_terminate_all(chan);
-               if (err || data->host_cookie)
+               if (data->host_cookie)

I'll post these patches as soon as I have managed to test them or
sooner if I get a tested-by.

Regards,
Per

Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

[Russell King - ARM Linux]

On Sun, Aug 28, 2011 at 12:50:54PM +0200, Per Forlin wrote:
> On 26 August 2011 18:28, Santosh <santosh.shilimkar@ti.com> wrote:
> > + Balaji,
> >
> > On Friday 26 August 2011 09:49 PM, Russell King - ARM Linux wrote:
> >>
> >> I'm not sure who's responsible for this, but the nonblocking MMC stuff is
> >> broken on OMAPs HSMMC:
> >>
> >> mmcblk0: error -84 transferring data, sector 149201, nr 64, cmd response
> >> 0x900, card status 0xb00
> >> mmcblk0: retrying using single block read
> >> ------------[ cut here ]------------
> >> WARNING: at /home/rmk/git/linux-2.6-rmk/lib/dma-debug.c:811
> >> check_unmap+0x1ac/0x764()
> >> omap_hsmmc omap_hsmmc.0: DMA-API: device driver tries to free DMA memory
> >> it has not allocated [device address=0x0000000080933000] [size=20480 bytes]
> >> Modules linked in:
> >> Backtrace:
> >> [<c0017874>] (dump_backtrace+0x0/0x10c) from [<c02ce8ac>]
> >> (dump_stack+0x18/0x1c)
> >>  r7:c1abfcb8 r6:c0186248 r5:c037de51 r4:0000032b
> >> [<c02ce894>] (dump_stack+0x0/0x1c) from [<c0039ed4>]
> >> (warn_slowpath_common+0x58/0x70)
> >> [<c0039e7c>] (warn_slowpath_common+0x0/0x70) from [<c0039f90>]
> >> (warn_slowpath_fmt+0x38/0x40)
> >>  r8:c1abfd50 r7:00000000 r6:00005000 r5:00000000 r4:80933000
> >> [<c0039f58>] (warn_slowpath_fmt+0x0/0x40) from [<c0186248>]
> >> (check_unmap+0x1ac/0x764)
> >>  r3:c0367d55 r2:c037e24d
> >> [<c018609c>] (check_unmap+0x0/0x764) from [<c0186978>]
> >> (debug_dma_unmap_sg+0x100/0x134)
> >> [<c0186878>] (debug_dma_unmap_sg+0x0/0x134) from [<c0019770>]
> >> (dma_unmap_sg+0x24/0x7c)
> >> [<c001974c>] (dma_unmap_sg+0x0/0x7c) from [<c0207220>]
> >> (omap_hsmmc_post_req+0x48/0x54)
> >> [<c02071d8>] (omap_hsmmc_post_req+0x0/0x54) from [<c01fb644>]
> >> (mmc_start_req+0x9c/0x128)
> >>  r4:c1a76000
> >> [<c01fb5a8>] (mmc_start_req+0x0/0x128) from [<c02049fc>]
> >> (mmc_blk_issue_rw_rq+0x80/0x460)
> >>  r8:c1ab5c00 r7:00000001 r6:c1ab5824 r5:c1ab5824 r4:c1ab5c00
> >> [<c020497c>] (mmc_blk_issue_rw_rq+0x0/0x460) from [<c02050a8>]
> >> (mmc_blk_issue_rq+0x2cc/0x2fc)
> >> [<c0204ddc>] (mmc_blk_issue_rq+0x0/0x2fc) from [<c02056c4>]
> >> (mmc_queue_thread+0xa0/0x104)
> >> [<c0205624>] (mmc_queue_thread+0x0/0x104) from [<c0054f8c>]
> >> (kthread+0x88/0x90)
> >> [<c0054f04>] (kthread+0x0/0x90) from [<c003d9a4>] (do_exit+0x0/0x618)
> >>  r7:00000013 r6:c003d9a4 r5:c0054f04 r4:c1a7bc7c
> >> ---[ end trace 3314ad56daf5d14f ]---
> >>
> >> Luckily thinks continue to work, but clearly releasing DMA mappings which
> >> drivers don't own is bad news.  Unfortunately, I don't have the bandwidth
> >> to be able to investigate this at present - well, I could do but then I'd
> >> have to drop working on the OMAP4 vs generic suspend/resume code and learn
> >> about something I've no current clue about.
> >>
> > Please continue your help on generic suspend.
> >
> >> Can someone please investigate and fix whatever is broken.
> >>
> > Will find somebody to look at this issue.
> >
> I had a look at this and my guess is that the same mapped DMA memory
> is unmapped twice. First the memory is unmapped in
> omap_hsmmc_dma_cleanup() due to the mmc error, then later in
> omap_hsmmc_post_req(). Here is a patch to resolve that. I haven't had
> the chance to test it yet though.
> 
> ---
>  drivers/mmc/host/omap_hsmmc.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 21e4a79..7f40767 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1011,6 +1011,7 @@ static void omap_hsmmc_dma_cleanup(struct
> omap_hsmmc_host *host, int errno)
>                         host->data->sg_len,
>                         omap_hsmmc_get_dma_dir(host, host->data));
>                 omap_free_dma(dma_ch);
> +               data->host_cookie = 0;
>         }
>         host->data = NULL;
>  }
> @@ -1576,8 +1577,10 @@ static void omap_hsmmc_post_req(struct mmc_host
> *mmc, struct mmc_request *mrq,
>         struct mmc_data *data = mrq->data;
> 
>         if (host->use_dma) {
> -               dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> -                            omap_hsmmc_get_dma_dir(host, data));
> +               if (data->host_cookie)
> +                       dma_unmap_sg(mmc_dev(host->mmc), data->sg,
> +                                    data->sg_len,
> +                                    omap_hsmmc_get_dma_dir(host, data));
>                 data->host_cookie = 0;
> 
> 
> I also checked the mmci code for this same issue and mmci doesn't have
> the same bug.
> MMCI works fine in 3.1-rc2 even though the code is "wrong". I propose
> this change.
> @@ -529,7 +529,7 @@ static void mmci_post_request(struct mmc_host
> *mmc, struct mmc_request *mrq,
>         if (chan) {
>                 if (err)
>                         dmaengine_terminate_all(chan);
> -               if (err || data->host_cookie)
> +               if (data->host_cookie)
> 
> I'll post these patches as soon as I have managed to test them or
> sooner if I get a tested-by.

Looking at MMCI, are you sure all those DMA engine terminate calls are
correct?

For example, why are we terminating all DMA engine transfers in the
post request function?  Surely that should've already been taken care
of by the driver before it told the core that the request was completed.
If the post request function is called for the previous request, meanwhile
the next request is in flight, terminating the DMA engine transfer will
cause invalid state in the driver.

Basically, this looks completely buggered.  I think you need to review
your changes with a thought to what happens logically with DMA and the
MMC host drivers whenever an error occurs.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/12] use nonblock mmc requests to minimize latency

[Per Forlin]

On 28 August 2011 13:11, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Aug 28, 2011 at 12:50:54PM +0200, Per Forlin wrote:
>> On 26 August 2011 18:28, Santosh <santosh.shilimkar@ti.com> wrote:
>> > + Balaji,
>> >
>> > On Friday 26 August 2011 09:49 PM, Russell King - ARM Linux wrote:
>> >>
>> >> I'm not sure who's responsible for this, but the nonblocking MMC stuff is
>> >> broken on OMAPs HSMMC:
>> >>
>> >> mmcblk0: error -84 transferring data, sector 149201, nr 64, cmd response
>> >> 0x900, card status 0xb00
>> >> mmcblk0: retrying using single block read
>> >> ------------[ cut here ]------------
>> >> WARNING: at /home/rmk/git/linux-2.6-rmk/lib/dma-debug.c:811
>> >> check_unmap+0x1ac/0x764()
>> >> omap_hsmmc omap_hsmmc.0: DMA-API: device driver tries to free DMA memory
>> >> it has not allocated [device address=0x0000000080933000] [size=20480 bytes]
>> >> Modules linked in:
>> >> Backtrace:
>> >> [<c0017874>] (dump_backtrace+0x0/0x10c) from [<c02ce8ac>]
>> >> (dump_stack+0x18/0x1c)
>> >>  r7:c1abfcb8 r6:c0186248 r5:c037de51 r4:0000032b
>> >> [<c02ce894>] (dump_stack+0x0/0x1c) from [<c0039ed4>]
>> >> (warn_slowpath_common+0x58/0x70)
>> >> [<c0039e7c>] (warn_slowpath_common+0x0/0x70) from [<c0039f90>]
>> >> (warn_slowpath_fmt+0x38/0x40)
>> >>  r8:c1abfd50 r7:00000000 r6:00005000 r5:00000000 r4:80933000
>> >> [<c0039f58>] (warn_slowpath_fmt+0x0/0x40) from [<c0186248>]
>> >> (check_unmap+0x1ac/0x764)
>> >>  r3:c0367d55 r2:c037e24d
>> >> [<c018609c>] (check_unmap+0x0/0x764) from [<c0186978>]
>> >> (debug_dma_unmap_sg+0x100/0x134)
>> >> [<c0186878>] (debug_dma_unmap_sg+0x0/0x134) from [<c0019770>]
>> >> (dma_unmap_sg+0x24/0x7c)
>> >> [<c001974c>] (dma_unmap_sg+0x0/0x7c) from [<c0207220>]
>> >> (omap_hsmmc_post_req+0x48/0x54)
>> >> [<c02071d8>] (omap_hsmmc_post_req+0x0/0x54) from [<c01fb644>]
>> >> (mmc_start_req+0x9c/0x128)
>> >>  r4:c1a76000
>> >> [<c01fb5a8>] (mmc_start_req+0x0/0x128) from [<c02049fc>]
>> >> (mmc_blk_issue_rw_rq+0x80/0x460)
>> >>  r8:c1ab5c00 r7:00000001 r6:c1ab5824 r5:c1ab5824 r4:c1ab5c00
>> >> [<c020497c>] (mmc_blk_issue_rw_rq+0x0/0x460) from [<c02050a8>]
>> >> (mmc_blk_issue_rq+0x2cc/0x2fc)
>> >> [<c0204ddc>] (mmc_blk_issue_rq+0x0/0x2fc) from [<c02056c4>]
>> >> (mmc_queue_thread+0xa0/0x104)
>> >> [<c0205624>] (mmc_queue_thread+0x0/0x104) from [<c0054f8c>]
>> >> (kthread+0x88/0x90)
>> >> [<c0054f04>] (kthread+0x0/0x90) from [<c003d9a4>] (do_exit+0x0/0x618)
>> >>  r7:00000013 r6:c003d9a4 r5:c0054f04 r4:c1a7bc7c
>> >> ---[ end trace 3314ad56daf5d14f ]---
>> >>
>> >> Luckily thinks continue to work, but clearly releasing DMA mappings which
>> >> drivers don't own is bad news.  Unfortunately, I don't have the bandwidth
>> >> to be able to investigate this at present - well, I could do but then I'd
>> >> have to drop working on the OMAP4 vs generic suspend/resume code and learn
>> >> about something I've no current clue about.
>> >>
>> > Please continue your help on generic suspend.
>> >
>> >> Can someone please investigate and fix whatever is broken.
>> >>
>> > Will find somebody to look at this issue.
>> >
>> I had a look at this and my guess is that the same mapped DMA memory
>> is unmapped twice. First the memory is unmapped in
>> omap_hsmmc_dma_cleanup() due to the mmc error, then later in
>> omap_hsmmc_post_req(). Here is a patch to resolve that. I haven't had
>> the chance to test it yet though.
>>
>> ---
>>  drivers/mmc/host/omap_hsmmc.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 21e4a79..7f40767 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -1011,6 +1011,7 @@ static void omap_hsmmc_dma_cleanup(struct
>> omap_hsmmc_host *host, int errno)
>>                         host->data->sg_len,
>>                         omap_hsmmc_get_dma_dir(host, host->data));
>>                 omap_free_dma(dma_ch);
>> +               data->host_cookie = 0;
>>         }
>>         host->data = NULL;
>>  }
>> @@ -1576,8 +1577,10 @@ static void omap_hsmmc_post_req(struct mmc_host
>> *mmc, struct mmc_request *mrq,
>>         struct mmc_data *data = mrq->data;
>>
>>         if (host->use_dma) {
>> -               dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
>> -                            omap_hsmmc_get_dma_dir(host, data));
>> +               if (data->host_cookie)
>> +                       dma_unmap_sg(mmc_dev(host->mmc), data->sg,
>> +                                    data->sg_len,
>> +                                    omap_hsmmc_get_dma_dir(host, data));
>>                 data->host_cookie = 0;
>>
>>
>> I also checked the mmci code for this same issue and mmci doesn't have
>> the same bug.
>> MMCI works fine in 3.1-rc2 even though the code is "wrong". I propose
>> this change.
>> @@ -529,7 +529,7 @@ static void mmci_post_request(struct mmc_host
>> *mmc, struct mmc_request *mrq,
>>         if (chan) {
>>                 if (err)
>>                         dmaengine_terminate_all(chan);
>> -               if (err || data->host_cookie)
>> +               if (data->host_cookie)
>>
>> I'll post these patches as soon as I have managed to test them or
>> sooner if I get a tested-by.
>
> Looking at MMCI, are you sure all those DMA engine terminate calls are
> correct?
>
I should probably do so together with adding better descriptions on
what happens and what the intention is.

> For example, why are we terminating all DMA engine transfers in the
> post request function?
The reason for calling DMA terminate here is to handle the case of
cleaning up an MMC request that has only been prepared in
mmci_pre_request(), and not been passed to mmci_request().
The if (err) is only true if that scenario has happened. The variable
err may need to be better defined. I can think of three states:
1. err == no error
2. err == mmc request failed
3. err == mmc request is prepared but not started
Currently "err" is only set if #3 occurs.

The parts to clean up in post_request() if #3 happens (err == true):
in mmci: DMA mapped memory and prepared DMA descriptors
in omap_hsmmc: DMA mapped memory

Now when I think more about I need to check the DMA drivers more
closely if dma_terminate_all() will free resources for prepared but
not submitted descriptors. I will have to add this support to the dma
drivers if it's missing.

> of by the driver before it told the core that the request was completed.
> If the post request function is called for the previous request, meanwhile
> the next request is in flight, terminating the DMA engine transfer will
> cause invalid state in the driver.
>
> Basically, this looks completely buggered.  I think you need to review
> your changes with a thought to what happens logically with DMA and the
> MMC host drivers whenever an error occurs.
>
I agree, I need to look more closely on the error handling.

Regards,
Per