All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer()
@ 2010-09-07 15:37 Mark Brown
  2010-09-07 15:37 ` [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time " Mark Brown
  2010-09-08  1:11 ` [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling " Jassi Brar
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Brown @ 2010-09-07 15:37 UTC (permalink / raw)
  To: Grant Likely, David Brownell, Jassi Brar
  Cc: spi-devel-general, linux-kernel, Mark Brown

In wait_for_xfer() for PIO transfer we are using val as both a
counter variable to track the number of spins we've waited for
completion and the value we read from the controller, causing
us to fail to ever actually notice the timeout. Fix this by using
a separate value to hold the register readback.

Also warn when we hit the timeout.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/spi/spi_s3c64xx.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 6e48ea9..03b28e4 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -321,7 +321,7 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 {
 	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	void __iomem *regs = sdd->regs;
-	unsigned long val;
+	unsigned long val, reg;
 	int ms;
 
 	/* millisecs to xfer 'len' bytes @ 'cur_speed' */
@@ -333,13 +333,16 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 		val = wait_for_completion_timeout(&sdd->xfer_completion, val);
 	} else {
 		val = msecs_to_loops(ms);
+
 		do {
-			val = readl(regs + S3C64XX_SPI_STATUS);
-		} while (RX_FIFO_LVL(val, sci) < xfer->len && --val);
+			reg = readl(regs + S3C64XX_SPI_STATUS);
+		} while (RX_FIFO_LVL(reg, sci) < xfer->len && --val);
 	}
 
-	if (!val)
+	if (!val) {
+		dev_warn(&sdd->pdev->dev, "Transfer timeout\n");
 		return -EIO;
+	}
 
 	if (dma_mode) {
 		u32 status;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time in wait_for_xfer()
  2010-09-07 15:37 [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer() Mark Brown
@ 2010-09-07 15:37 ` Mark Brown
  2010-09-08  1:44   ` Jassi Brar
  2010-09-08  1:11 ` [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling " Jassi Brar
  1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2010-09-07 15:37 UTC (permalink / raw)
  To: Grant Likely, David Brownell, Jassi Brar
  Cc: spi-devel-general, linux-kernel, Mark Brown

For small transfers at high speeds the expected transfer time can easily
be well under 1ms, causing the delay in wait_for_xfer() to be only the
dead reckoning fudge factor of 5ms currently included. Experiments on
some of my systems shows that this is marginal for some transfers so
double it to 10ms.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/spi/spi_s3c64xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 03b28e4..4abb441 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -326,7 +326,7 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 
 	/* millisecs to xfer 'len' bytes @ 'cur_speed' */
 	ms = xfer->len * 8 * 1000 / sdd->cur_speed;
-	ms += 5; /* some tolerance */
+	ms += 10; /* some tolerance */
 
 	if (dma_mode) {
 		val = msecs_to_jiffies(ms) + 10;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer()
  2010-09-07 15:37 [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer() Mark Brown
  2010-09-07 15:37 ` [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time " Mark Brown
@ 2010-09-08  1:11 ` Jassi Brar
  2010-09-08  9:06   ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2010-09-08  1:11 UTC (permalink / raw)
  To: Mark Brown
  Cc: Grant Likely, David Brownell, Jassi Brar, spi-devel-general,
	linux-kernel

On Wed, Sep 8, 2010 at 12:37 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> In wait_for_xfer() for PIO transfer we are using val as both a
> counter variable to track the number of spins we've waited for
> completion and the value we read from the controller, causing
> us to fail to ever actually notice the timeout. Fix this by using
> a separate value to hold the register readback.
>
> Also warn when we hit the timeout.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/spi/spi_s3c64xx.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 6e48ea9..03b28e4 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -321,7 +321,7 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>  {
>        struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
>        void __iomem *regs = sdd->regs;
> -       unsigned long val;
> +       unsigned long val, reg;
>        int ms;
>
>        /* millisecs to xfer 'len' bytes @ 'cur_speed' */
> @@ -333,13 +333,16 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>                val = wait_for_completion_timeout(&sdd->xfer_completion, val);
>        } else {
>                val = msecs_to_loops(ms);
> +
>                do {
> -                       val = readl(regs + S3C64XX_SPI_STATUS);
> -               } while (RX_FIFO_LVL(val, sci) < xfer->len && --val);
> +                       reg = readl(regs + S3C64XX_SPI_STATUS);
> +               } while (RX_FIFO_LVL(reg, sci) < xfer->len && --val);
>        }
>
> -       if (!val)
> +       if (!val) {
> +               dev_warn(&sdd->pdev->dev, "Transfer timeout\n");
>                return -EIO;
> +       }

I have already submitted a patch a few days ago
https://patchwork.kernel.org/patch/151941/
(It's strange that Grant's id isn't there in the CC list, despite my writing it)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time in wait_for_xfer()
  2010-09-07 15:37 ` [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time " Mark Brown
@ 2010-09-08  1:44   ` Jassi Brar
  2010-09-08 16:00     ` Grant Likely
  0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2010-09-08  1:44 UTC (permalink / raw)
  To: Mark Brown
  Cc: Grant Likely, David Brownell, Jassi Brar, spi-devel-general,
	linux-kernel

On Wed, Sep 8, 2010 at 12:37 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> For small transfers at high speeds the expected transfer time can easily
> be well under 1ms, causing the delay in wait_for_xfer() to be only the
> dead reckoning fudge factor of 5ms currently included. Experiments on
> some of my systems shows that this is marginal for some transfers so
> double it to 10ms.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/spi/spi_s3c64xx.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 03b28e4..4abb441 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -326,7 +326,7 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>
>        /* millisecs to xfer 'len' bytes @ 'cur_speed' */
>        ms = xfer->len * 8 * 1000 / sdd->cur_speed;
> -       ms += 5; /* some tolerance */
> +       ms += 10; /* some tolerance */
>
>        if (dma_mode) {
>                val = msecs_to_jiffies(ms) + 10;

Acked-by: Jassi Brar <jassi.brar@samsung.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer()
  2010-09-08  1:11 ` [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling " Jassi Brar
@ 2010-09-08  9:06   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2010-09-08  9:06 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Grant Likely, David Brownell, Jassi Brar, spi-devel-general,
	linux-kernel

On Wed, Sep 08, 2010 at 10:11:59AM +0900, Jassi Brar wrote:

> I have already submitted a patch a few days ago
> https://patchwork.kernel.org/patch/151941/
> (It's strange that Grant's id isn't there in the CC list, despite my writing it)

Ah, excellent.  Now I see patchwork there look to be some other things
sitting in patchwork which might be handy for me as well.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time in wait_for_xfer()
  2010-09-08  1:44   ` Jassi Brar
@ 2010-09-08 16:00     ` Grant Likely
  0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-09-08 16:00 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Mark Brown, David Brownell, Jassi Brar, spi-devel-general, linux-kernel

On Wed, Sep 08, 2010 at 10:44:26AM +0900, Jassi Brar wrote:
> On Wed, Sep 8, 2010 at 12:37 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > For small transfers at high speeds the expected transfer time can easily
> > be well under 1ms, causing the delay in wait_for_xfer() to be only the
> > dead reckoning fudge factor of 5ms currently included. Experiments on
> > some of my systems shows that this is marginal for some transfers so
> > double it to 10ms.
> >
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > ---
> >  drivers/spi/spi_s3c64xx.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> > index 03b28e4..4abb441 100644
> > --- a/drivers/spi/spi_s3c64xx.c
> > +++ b/drivers/spi/spi_s3c64xx.c
> > @@ -326,7 +326,7 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
> >
> >        /* millisecs to xfer 'len' bytes @ 'cur_speed' */
> >        ms = xfer->len * 8 * 1000 / sdd->cur_speed;
> > -       ms += 5; /* some tolerance */
> > +       ms += 10; /* some tolerance */
> >
> >        if (dma_mode) {
> >                val = msecs_to_jiffies(ms) + 10;
> 
> Acked-by: Jassi Brar <jassi.brar@samsung.com>

Applied, thanks.

g.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-09-08 16:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-07 15:37 [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling in wait_for_xfer() Mark Brown
2010-09-07 15:37 ` [PATCH 2/2] spi/spi_s3c64xx: Increase dead reckoning time " Mark Brown
2010-09-08  1:44   ` Jassi Brar
2010-09-08 16:00     ` Grant Likely
2010-09-08  1:11 ` [PATCH 1/2] spi/spi_s3c64xx: Fix timeout handling " Jassi Brar
2010-09-08  9:06   ` Mark Brown

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.