2008-10-03 15:12:07

by Haavard Skinnemoen

[permalink] [raw]
Subject: [PATCH] atmel-mci: Initialize BLKR before sending data transfer command

The atmel-mci driver sometimes fails data transfers like this:

mmcblk0: error -5 transferring data
end_request: I/O error, dev mmcblk0, sector 2749769
end_request: I/O error, dev mmcblk0, sector 2749777

It turns out that this might be caused by the BLKR register (which
contains the block size and the number of blocks being transfered) being
initialized too late. This patch moves the initialization of BLKR so
that it contains the correct value before the block transfer command is
sent.

This error is difficult to reproduce, but if you insert a long delay
(mdelay(10) or thereabouts) between the calls to atmci_start_command()
and atmci_submit_data(), all transfers seem to fail without this patch,
while I haven't seen any failures with this patch.

Reported-by: Hein_Tibosch <[email protected]>
Signed-off-by: Haavard Skinnemoen <[email protected]>
---
drivers/mmc/host/atmel-mci.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 917035e..0000896 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -426,8 +426,6 @@ static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
host->sg = NULL;
host->data = data;

- mci_writel(host, BLKR, MCI_BCNT(data->blocks)
- | MCI_BLKLEN(data->blksz));
dev_vdbg(&mmc->class_dev, "BLKR=0x%08x\n",
MCI_BCNT(data->blocks) | MCI_BLKLEN(data->blksz));

@@ -483,6 +481,10 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
if (data->blocks > 1 && data->blksz & 3)
goto fail;
atmci_set_timeout(host, data);
+
+ /* Must set block count/size before sending command */
+ mci_writel(host, BLKR, MCI_BCNT(data->blocks)
+ | MCI_BLKLEN(data->blksz));
}

iflags = MCI_CMDRDY;
--
1.5.6.5


2008-10-06 17:45:57

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] atmel-mci: Initialize BLKR before sending data transfer command

Linus, any chance of getting this into .27? It has been seen in the
wild and causes I/O errors (which can lead to data loss).

If so, you can add:

Signed-off-by: Pierre Ossman <[email protected]>

On Fri, 3 Oct 2008 17:07:38 +0200
Haavard Skinnemoen <[email protected]> wrote:

> The atmel-mci driver sometimes fails data transfers like this:
>
> mmcblk0: error -5 transferring data
> end_request: I/O error, dev mmcblk0, sector 2749769
> end_request: I/O error, dev mmcblk0, sector 2749777
>
> It turns out that this might be caused by the BLKR register (which
> contains the block size and the number of blocks being transfered) being
> initialized too late. This patch moves the initialization of BLKR so
> that it contains the correct value before the block transfer command is
> sent.
>
> This error is difficult to reproduce, but if you insert a long delay
> (mdelay(10) or thereabouts) between the calls to atmci_start_command()
> and atmci_submit_data(), all transfers seem to fail without this patch,
> while I haven't seen any failures with this patch.
>
> Reported-by: Hein_Tibosch <[email protected]>
> Signed-off-by: Haavard Skinnemoen <[email protected]>
> ---
> drivers/mmc/host/atmel-mci.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index 917035e..0000896 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -426,8 +426,6 @@ static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
> host->sg = NULL;
> host->data = data;
>
> - mci_writel(host, BLKR, MCI_BCNT(data->blocks)
> - | MCI_BLKLEN(data->blksz));
> dev_vdbg(&mmc->class_dev, "BLKR=0x%08x\n",
> MCI_BCNT(data->blocks) | MCI_BLKLEN(data->blksz));
>
> @@ -483,6 +481,10 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> if (data->blocks > 1 && data->blksz & 3)
> goto fail;
> atmci_set_timeout(host, data);
> +
> + /* Must set block count/size before sending command */
> + mci_writel(host, BLKR, MCI_BCNT(data->blocks)
> + | MCI_BLKLEN(data->blksz));
> }
>
> iflags = MCI_CMDRDY;


--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.


Attachments:
signature.asc (197.00 B)

2008-10-06 17:52:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] atmel-mci: Initialize BLKR before sending data transfer command



On Mon, 6 Oct 2008, Pierre Ossman wrote:
>
> Linus, any chance of getting this into .27? It has been seen in the
> wild and causes I/O errors (which can lead to data loss).
>
> If so, you can add:
>
> Signed-off-by: Pierre Ossman <[email protected]>

Please just send the whole patch. I don't want to have to go searching for
it somewhere else, or edit it for whitespace and quoting damage from you
quoting it. Just add

From: Haavard Skinnemoen <[email protected]>

at the top of the body to make attribution come out right (even though you
send it to me), and add your own Signed-off-by: at the end. That way I
have many fewer chances of screwing up, when I don't have to try to follow
the references list back to another mailbox or off to some archive site
(since I filter kernel mailing list email to another mailbox that I don't
archive and that I age aggressively to keep it recent and relevant).

Linus

2008-10-06 18:10:24

by Pierre Ossman

[permalink] [raw]
Subject: [PATCH] atmel-mci: Initialize BLKR before sending data transfer command

From: Haavard Skinnemoen <[email protected]>

The atmel-mci driver sometimes fails data transfers like this:

mmcblk0: error -5 transferring data
end_request: I/O error, dev mmcblk0, sector 2749769
end_request: I/O error, dev mmcblk0, sector 2749777

It turns out that this might be caused by the BLKR register (which
contains the block size and the number of blocks being transfered) being
initialized too late. This patch moves the initialization of BLKR so
that it contains the correct value before the block transfer command is
sent.

This error is difficult to reproduce, but if you insert a long delay
(mdelay(10) or thereabouts) between the calls to atmci_start_command()
and atmci_submit_data(), all transfers seem to fail without this patch,
while I haven't seen any failures with this patch.

Reported-by: Hein_Tibosch <[email protected]>
Signed-off-by: Haavard Skinnemoen <[email protected]>
Signed-off-by: Pierre Ossman <[email protected]>
---
drivers/mmc/host/atmel-mci.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 917035e..0000896 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -426,8 +426,6 @@ static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
host->sg = NULL;
host->data = data;

- mci_writel(host, BLKR, MCI_BCNT(data->blocks)
- | MCI_BLKLEN(data->blksz));
dev_vdbg(&mmc->class_dev, "BLKR=0x%08x\n",
MCI_BCNT(data->blocks) | MCI_BLKLEN(data->blksz));

@@ -483,6 +481,10 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
if (data->blocks > 1 && data->blksz & 3)
goto fail;
atmci_set_timeout(host, data);
+
+ /* Must set block count/size before sending command */
+ mci_writel(host, BLKR, MCI_BCNT(data->blocks)
+ | MCI_BLKLEN(data->blksz));
}

iflags = MCI_CMDRDY;


--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.


Attachments:
signature.asc (197.00 B)