[PATCH] mmc: mmci: Fixup error handling for dma

[PATCH] mmc: mmci: Fixup error handling for dma

[Ulf Hansson]

When getting a cmd irq during an ongoing data transfer
with dma, the dma job were never terminated. This is now
corrected.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
---
 drivers/mmc/host/mmci.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e900f51..62ad649 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 
 	if (!cmd->data || cmd->error) {
-		if (host->data)
+		if (host->data) {
+			/* Terminate the DMA transfer */
+			if (dma_inprogress(host))
+				mmci_dma_data_error(host);
 			mmci_stop_data(host);
+		}
 		mmci_request_end(host, cmd->mrq);
 	} else if (!(cmd->data->flags & MMC_DATA_READ)) {
 		mmci_start_data(host, cmd->data);
-- 
1.7.5.4

[PATCH] mmc: mmci: Fixup error handling for dma

[Ulf Hansson]

When getting a cmd irq during an ongoing data transfer
with dma, the dma job were never terminated. This is now
corrected.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Signed-off-by: Per Forlin <per.forlin@stericsson.com>
---
 drivers/mmc/host/mmci.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e900f51..62ad649 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	}
 
 	if (!cmd->data || cmd->error) {
-		if (host->data)
+		if (host->data) {
+			/* Terminate the DMA transfer */
+			if (dma_inprogress(host))
+				mmci_dma_data_error(host);
 			mmci_stop_data(host);
+		}
 		mmci_request_end(host, cmd->mrq);
 	} else if (!(cmd->data->flags & MMC_DATA_READ)) {
 		mmci_start_data(host, cmd->data);
-- 
1.7.5.4

Re: [PATCH] mmc: mmci: Fixup error handling for dma

[Linus Walleij]

On Tue, Nov 29, 2011 at 3:51 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:

> When getting a cmd irq during an ongoing data transfer
> with dma, the dma job were never terminated. This is now
> corrected.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij

[PATCH] mmc: mmci: Fixup error handling for dma

[Linus Walleij]

On Tue, Nov 29, 2011 at 3:51 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:

> When getting a cmd irq during an ongoing data transfer
> with dma, the dma job were never terminated. This is now
> corrected.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij

Re: [PATCH] mmc: mmci: Fixup error handling for dma

[Russell King - ARM Linux]

On Tue, Nov 29, 2011 at 03:51:26PM +0100, Ulf Hansson wrote:
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index e900f51..62ad649 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>  	}
>  
>  	if (!cmd->data || cmd->error) {
> -		if (host->data)
> +		if (host->data) {
> +			/* Terminate the DMA transfer */
> +			if (dma_inprogress(host))
> +				mmci_dma_data_error(host);

Doesn't this leave the DMA buffers mapped?

[PATCH] mmc: mmci: Fixup error handling for dma

[Russell King - ARM Linux]

On Tue, Nov 29, 2011 at 03:51:26PM +0100, Ulf Hansson wrote:
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index e900f51..62ad649 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -761,8 +761,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>  	}
>  
>  	if (!cmd->data || cmd->error) {
> -		if (host->data)
> +		if (host->data) {
> +			/* Terminate the DMA transfer */
> +			if (dma_inprogress(host))
> +				mmci_dma_data_error(host);

Doesn't this leave the DMA buffers mapped?

Re: [PATCH] mmc: mmci: Fixup error handling for dma

[Linus Walleij]

On Wed, Nov 30, 2011 at 11:21 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

>>       if (!cmd->data || cmd->error) {
>> -             if (host->data)
>> +             if (host->data) {
>> +                     /* Terminate the DMA transfer */
>> +                     if (dma_inprogress(host))
>> +                             mmci_dma_data_error(host);
>
> Doesn't this leave the DMA buffers mapped?

Yep it needs an mmci_dma_unmap() call too AFAICT,
Ulf will you update?

Yours,
Linus Walleij

[PATCH] mmc: mmci: Fixup error handling for dma

[Linus Walleij]

On Wed, Nov 30, 2011 at 11:21 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

>> ? ? ? if (!cmd->data || cmd->error) {
>> - ? ? ? ? ? ? if (host->data)
>> + ? ? ? ? ? ? if (host->data) {
>> + ? ? ? ? ? ? ? ? ? ? /* Terminate the DMA transfer */
>> + ? ? ? ? ? ? ? ? ? ? if (dma_inprogress(host))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? mmci_dma_data_error(host);
>
> Doesn't this leave the DMA buffers mapped?

Yep it needs an mmci_dma_unmap() call too AFAICT,
Ulf will you update?

Yours,
Linus Walleij

Re: [PATCH] mmc: mmci: Fixup error handling for dma

[Ulf Hansson]

Linus Walleij wrote:
> On Wed, Nov 30, 2011 at 11:21 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> 
>>>       if (!cmd->data || cmd->error) {
>>> -             if (host->data)
>>> +             if (host->data) {
>>> +                     /* Terminate the DMA transfer */
>>> +                     if (dma_inprogress(host))
>>> +                             mmci_dma_data_error(host);
>> Doesn't this leave the DMA buffers mapped?

Correct, but it will be handled in post_request function.

> 
> Yep it needs an mmci_dma_unmap() call too AFAICT,
> Ulf will you update?

Actually it's needs only dma_unmap_sg, but as stated this should be 
handled in mmci_post_request.

By, the way - I spoke to Per Förlin who implemented the pre_request and 
post_request functions; we will soon push some clean-up patches for 
improved error handling and how to manage the cookie better. Right now 
it is kind of hard to follow the error path, I think. We hope this shall 
be improved then.

Although it should have nothing to do with this patch directly, since 
this is a bugfix only.

BR
Ulf Hansson

[PATCH] mmc: mmci: Fixup error handling for dma

[Ulf Hansson]

Linus Walleij wrote:
> On Wed, Nov 30, 2011 at 11:21 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> 
>>>       if (!cmd->data || cmd->error) {
>>> -             if (host->data)
>>> +             if (host->data) {
>>> +                     /* Terminate the DMA transfer */
>>> +                     if (dma_inprogress(host))
>>> +                             mmci_dma_data_error(host);
>> Doesn't this leave the DMA buffers mapped?

Correct, but it will be handled in post_request function.

> 
> Yep it needs an mmci_dma_unmap() call too AFAICT,
> Ulf will you update?

Actually it's needs only dma_unmap_sg, but as stated this should be 
handled in mmci_post_request.

By, the way - I spoke to Per F?rlin who implemented the pre_request and 
post_request functions; we will soon push some clean-up patches for 
improved error handling and how to manage the cookie better. Right now 
it is kind of hard to follow the error path, I think. We hope this shall 
be improved then.

Although it should have nothing to do with this patch directly, since 
this is a bugfix only.

BR
Ulf Hansson