From: Simon Horman <horms@verge.net.au>
To: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: spi-rspi I/O errors
Date: Wed, 08 Jan 2014 00:28:46 +0000 [thread overview]
Message-ID: <20140108002846.GF5136@verge.net.au> (raw)
In-Reply-To: <alpine.DEB.2.02.1401072116370.27579-97SZ98TBZzA1xEWliksxXw@public.gmane.org>
On Tue, Jan 07, 2014 at 09:27:18PM +0100, Geert Uytterhoeven wrote:
> I was regularly getting I/O errors when using the Renesas RSPI/QSPI
> driver on r8a7791:
>
> m25p80 spi0.0: error -110 reading SR
>
> Until I applied the following patch, which re-reads RSPI_SPSR on a time-out,
> and continues if the condition has become true:
>
> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
> index 4b31d89e8568..e63e30c500da 100644
> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -442,8 +442,13 @@ static int rspi_wait_for_interrupt(struct rspi_data *rspi, u8 wait_mask,
> rspi->spsr = rspi_read8(rspi, RSPI_SPSR);
> rspi_enable_irq(rspi, enable_bit);
> ret = wait_event_timeout(rspi->wait, rspi->spsr & wait_mask, HZ);
> - if (ret = 0 && !(rspi->spsr & wait_mask))
> - return -ETIMEDOUT;
> + if (ret = 0 && !(rspi->spsr & wait_mask)) {
> + u8 spsr = rspi_read8(rspi, RSPI_SPSR);
> + printk("*** rspi->spsr = 0x%02x, real spsr = 0x%02x, wait_mask = 0x%02x ***\n",
> + rspi->spsr, spsr, wait_mask);
> + if (!(spsr & wait_mask))
> + return -ETIMEDOUT;
I'm entirely unsure if this is the right approach,
but if it is should rspi->spsr = spsr go somewhere around here?
> + }
>
> return 0;
> }
>
> Now it prints from time to time:
>
> *** rspi->spsr = 0x20, real spsr = 0xa0, wait_mask = 0x80 ***
>
> which shows that rspi->spsr (as set from the interrupt handler) didn't
> have bit 7 set, while RSPI_SPSR does have bit 7 set.
>
> So this looks like a race condition in the interrupt handling.
>
> I didn't notice any data corruption after the patch.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
To: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: spi-rspi I/O errors
Date: Wed, 8 Jan 2014 09:28:46 +0900 [thread overview]
Message-ID: <20140108002846.GF5136@verge.net.au> (raw)
In-Reply-To: <alpine.DEB.2.02.1401072116370.27579-97SZ98TBZzA1xEWliksxXw@public.gmane.org>
On Tue, Jan 07, 2014 at 09:27:18PM +0100, Geert Uytterhoeven wrote:
> I was regularly getting I/O errors when using the Renesas RSPI/QSPI
> driver on r8a7791:
>
> m25p80 spi0.0: error -110 reading SR
>
> Until I applied the following patch, which re-reads RSPI_SPSR on a time-out,
> and continues if the condition has become true:
>
> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
> index 4b31d89e8568..e63e30c500da 100644
> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -442,8 +442,13 @@ static int rspi_wait_for_interrupt(struct rspi_data *rspi, u8 wait_mask,
> rspi->spsr = rspi_read8(rspi, RSPI_SPSR);
> rspi_enable_irq(rspi, enable_bit);
> ret = wait_event_timeout(rspi->wait, rspi->spsr & wait_mask, HZ);
> - if (ret == 0 && !(rspi->spsr & wait_mask))
> - return -ETIMEDOUT;
> + if (ret == 0 && !(rspi->spsr & wait_mask)) {
> + u8 spsr = rspi_read8(rspi, RSPI_SPSR);
> + printk("*** rspi->spsr = 0x%02x, real spsr = 0x%02x, wait_mask = 0x%02x ***\n",
> + rspi->spsr, spsr, wait_mask);
> + if (!(spsr & wait_mask))
> + return -ETIMEDOUT;
I'm entirely unsure if this is the right approach,
but if it is should rspi->spsr = spsr go somewhere around here?
> + }
>
> return 0;
> }
>
> Now it prints from time to time:
>
> *** rspi->spsr = 0x20, real spsr = 0xa0, wait_mask = 0x80 ***
>
> which shows that rspi->spsr (as set from the interrupt handler) didn't
> have bit 7 set, while RSPI_SPSR does have bit 7 set.
>
> So this looks like a race condition in the interrupt handling.
>
> I didn't notice any data corruption after the patch.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-01-08 0:28 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-07 20:27 spi-rspi I/O errors Geert Uytterhoeven
2014-01-07 20:27 ` Geert Uytterhoeven
2014-01-08 0:27 ` Laurent Pinchart
2014-01-08 0:27 ` Laurent Pinchart
2014-01-08 8:28 ` Geert Uytterhoeven
2014-01-08 8:28 ` Geert Uytterhoeven
[not found] ` <CAMuHMdWDwF+KcNPrKgjvH6w7Uxn-iON+nQz_+Z_RTU_yV7SVog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-08 8:41 ` Laurent Pinchart
2014-01-08 8:41 ` Laurent Pinchart
2014-01-08 13:13 ` Geert Uytterhoeven
2014-01-08 13:13 ` Geert Uytterhoeven
[not found] ` <CAMuHMdVzFeymxW2CZUyKgZ-fxsektE1EpLQksfSzoekj2ex53Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-09 3:26 ` Magnus Damm
2014-01-09 3:26 ` Magnus Damm
[not found] ` <CANqRtoRj_CybzDY3h1Caz=G=R7R-GVBxJ86xr5isWnWn1mDb2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-09 9:48 ` Geert Uytterhoeven
2014-01-09 9:48 ` Geert Uytterhoeven
2014-01-09 18:13 ` Laurent Pinchart
2014-01-09 18:13 ` Laurent Pinchart
2014-01-09 18:14 ` Laurent Pinchart
2014-01-09 18:14 ` Laurent Pinchart
2014-01-10 9:20 ` Geert Uytterhoeven
2014-01-10 9:20 ` Geert Uytterhoeven
2014-01-10 9:20 ` Geert Uytterhoeven
2014-01-13 10:19 ` Geert Uytterhoeven
2014-01-13 10:19 ` Geert Uytterhoeven
2014-01-13 10:19 ` Geert Uytterhoeven
[not found] ` <alpine.DEB.2.02.1401072116370.27579-97SZ98TBZzA1xEWliksxXw@public.gmane.org>
2014-01-08 0:28 ` Simon Horman [this message]
2014-01-08 0:28 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140108002846.GF5136@verge.net.au \
--to=horms@verge.net.au \
--cc=geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org \
--cc=linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.