Mailing List Archive: Linux: Kernel

[PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout

Index | Next | Previous | View Threaded

dilinger at queued

Jun 23, 2008, 7:13 AM

Post #1 of 7 (2626 views)
Permalink

[PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout

The CaFe chip has a hardware bug that ends up with us getting a timeout
value that's too small, causing the following sorts of problems:

[ 60.525138] mmcblk0: error -110 transferring data
[ 60.531477] end_request: I/O error, dev mmcblk0, sector 1484353
[ 60.533371] Buffer I/O error on device mmcblk0p2, logical block 181632
[ 60.533371] lost page write due to I/O error on mmcblk0p2

Presumably this is an off-by-one error in the hardware. Incrementing
the timeout count value that we stuff into the TIMEOUT_CONTROL register
gets us a value that works. This bug was originally discovered by
Pierre Ossman, I believe.

[.thanks to Robert Millan for proving that this was still a problem]

Signed-off-by: Andres Salomon <dilinger [at] debian>
---
drivers/mmc/host/sdhci.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5b74c8c..2b3f06a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -57,6 +57,8 @@ static unsigned int debug_quirks = 0;
#define SDHCI_QUIRK_RESET_AFTER_REQUEST (1<<8)
/* Controller needs voltage and power writes to happen separately */
#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1<<9)
+/* Controller has an off-by-one issue with timeout value */
+#define SDHCI_QUIRK_INCR_TIMEOUT_CONTROL (1<<10)

static const struct pci_device_id pci_ids[] __devinitdata = {
{
@@ -134,7 +136,8 @@ static const struct pci_device_id pci_ids[] __devinitdata = {
.device = PCI_DEVICE_ID_MARVELL_CAFE_SD,
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
- .driver_data = SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER,
+ .driver_data = SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
+ SDHCI_QUIRK_INCR_TIMEOUT_CONTROL,
},

{
@@ -479,6 +482,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
break;
}

+ /*
+ * Compensate for an off-by-one error in the CaFe hardware; otherwise,
+ * a too-small count gives us interrupt timeouts.
+ */
+ if ((host->chip->quirks & SDHCI_QUIRK_INCR_TIMEOUT_CONTROL))
+ count++;
+
if (count >= 0xF) {
printk(KERN_WARNING "%s: Too large timeout requested!\n",
mmc_hostname(host->mmc));
--
1.5.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

akpm at linux-foundation

Jun 23, 2008, 5:04 PM

Post #2 of 7 (2540 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

On Mon, 23 Jun 2008 10:13:52 -0400
Andres Salomon <dilinger [at] queued> wrote:

>
> The CaFe chip has a hardware bug that ends up with us getting a timeout
> value that's too small, causing the following sorts of problems:
>
> [ 60.525138] mmcblk0: error -110 transferring data
> [ 60.531477] end_request: I/O error, dev mmcblk0, sector 1484353
> [ 60.533371] Buffer I/O error on device mmcblk0p2, logical block 181632
> [ 60.533371] lost page write due to I/O error on mmcblk0p2
>
> Presumably this is an off-by-one error in the hardware. Incrementing
> the timeout count value that we stuff into the TIMEOUT_CONTROL register
> gets us a value that works. This bug was originally discovered by
> Pierre Ossman, I believe.
>
> [.thanks to Robert Millan for proving that this was still a problem]
>
> Signed-off-by: Andres Salomon <dilinger [at] debian>
> ---
> drivers/mmc/host/sdhci.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5b74c8c..2b3f06a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -57,6 +57,8 @@ static unsigned int debug_quirks = 0;
> #define SDHCI_QUIRK_RESET_AFTER_REQUEST (1<<8)
> /* Controller needs voltage and power writes to happen separately */
> #define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1<<9)
> +/* Controller has an off-by-one issue with timeout value */
> +#define SDHCI_QUIRK_INCR_TIMEOUT_CONTROL (1<<10)
>
> static const struct pci_device_id pci_ids[] __devinitdata = {
> {
> @@ -134,7 +136,8 @@ static const struct pci_device_id pci_ids[] __devinitdata = {
> .device = PCI_DEVICE_ID_MARVELL_CAFE_SD,
> .subvendor = PCI_ANY_ID,
> .subdevice = PCI_ANY_ID,
> - .driver_data = SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER,
> + .driver_data = SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> + SDHCI_QUIRK_INCR_TIMEOUT_CONTROL,
> },
>
> {
> @@ -479,6 +482,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
> break;
> }
>
> + /*
> + * Compensate for an off-by-one error in the CaFe hardware; otherwise,
> + * a too-small count gives us interrupt timeouts.
> + */
> + if ((host->chip->quirks & SDHCI_QUIRK_INCR_TIMEOUT_CONTROL))
> + count++;
> +
> if (count >= 0xF) {
> printk(KERN_WARNING "%s: Too large timeout requested!\n",
> mmc_hostname(host->mmc));

This is needed in 2.6.26, I assume?

If so, I can merge it unless Pierre has objections?

And it will cause conflicts with overlapping changes in linux-next.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

akpm at linux-foundation

Jun 23, 2008, 5:08 PM

Post #3 of 7 (2552 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

On Mon, 23 Jun 2008 17:04:49 -0700
Andrew Morton <akpm [at] linux-foundation> wrote:

> And it will cause conflicts with overlapping changes in linux-next.

oops, I lied. The problem was that it secretly depended upon
olpc-sdhci-add-quirk-for-the-marvell-cafes-vdd-powerup-issue.patch

So if we want to fix thsi issue in 2.6.26 we need to merge both

olpc-sdhci-add-quirk-for-the-marvell-cafes-vdd-powerup-issue.patch

and

olpc-sdhci-add-quirk-for-the-marvell-cafes-interrupt-timeout.patch

yes?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

dilinger at queued

Jun 23, 2008, 6:38 PM

Post #4 of 7 (2543 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

On Mon, 23 Jun 2008 17:08:50 -0700
Andrew Morton <akpm [at] linux-foundation> wrote:

> On Mon, 23 Jun 2008 17:04:49 -0700
> Andrew Morton <akpm [at] linux-foundation> wrote:
>
> > And it will cause conflicts with overlapping changes in linux-next.
>
>
> oops, I lied. The problem was that it secretly depended upon
> olpc-sdhci-add-quirk-for-the-marvell-cafes-vdd-powerup-issue.patch
>
> So if we want to fix thsi issue in 2.6.26 we need to merge both
>
> olpc-sdhci-add-quirk-for-the-marvell-cafes-vdd-powerup-issue.patch
>
> and
>
> olpc-sdhci-add-quirk-for-the-marvell-cafes-interrupt-timeout.patch
>
> yes?

Correct. I originally wasn't going to send the interrupt-timeout
patch (but was shown that the bug still existed), which is why the two
patches weren't sent as a series. Sorry!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

dilinger at queued

Jun 23, 2008, 6:40 PM

Post #5 of 7 (2556 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

On Mon, 23 Jun 2008 17:04:49 -0700
Andrew Morton <akpm [at] linux-foundation> wrote:

> On Mon, 23 Jun 2008 10:13:52 -0400
> Andres Salomon <dilinger [at] queued> wrote:
>
> >
> > The CaFe chip has a hardware bug that ends up with us getting a
> > timeout value that's too small, causing the following sorts of
> > problems:
> >
> > [ 60.525138] mmcblk0: error -110 transferring data
> > [ 60.531477] end_request: I/O error, dev mmcblk0, sector 1484353
> > [ 60.533371] Buffer I/O error on device mmcblk0p2, logical block
> > 181632 [ 60.533371] lost page write due to I/O error on mmcblk0p2
> >
[...]
>
> This is needed in 2.6.26, I assume?
>

Yes, please.

> If so, I can merge it unless Pierre has objections?
>
> And it will cause conflicts with overlapping changes in linux-next.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

drzeus at drzeus

Jun 27, 2008, 10:30 AM

Post #6 of 7 (2518 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

Attachments:

signature.asc (0.19 KB)

dilinger at queued

Jun 27, 2008, 10:42 AM

Post #7 of 7 (2514 views)
Permalink

Re: [PATCH] [OLPC] sdhci: add quirk for the Marvell CaFe's interrupt timeout [In reply to]

On Fri, 27 Jun 2008 19:30:01 +0200
Pierre Ossman <drzeus [at] drzeus> wrote:

> On Mon, 23 Jun 2008 10:13:52 -0400
> Andres Salomon <dilinger [at] queued> wrote:
>
> >
> > The CaFe chip has a hardware bug that ends up with us getting a
> > timeout value that's too small, causing the following sorts of
> > problems:
> >
> > [ 60.525138] mmcblk0: error -110 transferring data
> > [ 60.531477] end_request: I/O error, dev mmcblk0, sector 1484353
> > [ 60.533371] Buffer I/O error on device mmcblk0p2, logical block
> > 181632 [ 60.533371] lost page write due to I/O error on mmcblk0p2
> >
> > Presumably this is an off-by-one error in the hardware.
> > Incrementing the timeout count value that we stuff into the
> > TIMEOUT_CONTROL register gets us a value that works. This bug was
> > originally discovered by Pierre Ossman, I believe.
> >
> > [.thanks to Robert Millan for proving that this was still a problem]
> >
> > Signed-off-by: Andres Salomon <dilinger [at] debian>
>
> Hmm... I'm not entirely sure about the specifics of the workaround
> here. It's likely that we'll have an off-by-minus-one in another
> controller, and off-by-two in a third.
>
> Perhaps we should just have "broken timeout" and set the timeout to
> 0xE. It doesn't cause any side-effects except that the user will have
> to wait slightly longer for requests to fail if the card has decided
> to crap out.
>

That would be fine. OFW actually just hardcodes the timeout to 0xc,
with Mitch citing the same logic. Just setting it to the upper bound
would certainly make it more applicable hardware other than the cafe.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads