[PATCH 1/9] spi: sunxi: fix transfer timeout

Subsystems: spi subsystem, the rest

STALE4025d

7 messages, 3 authors, 2015-08-20 · open the first message on its own page

[PATCH 1/9] spi: sunxi: fix transfer timeout

From: Michal Suchanek <hidden>
Date: 2015-08-20 14:19:45

The trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over
1MHz SPI bus takes way longer than that. Calculate the timeout from the
actual time the transfer is supposed to take and multiply by 2 for good
measure.

Signed-off-by: Michal Suchanek <redacted>
---
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index fbb0a4d..48532ec 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -170,6 +170,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
+	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
 	u32 reg;
@@ -279,9 +280,16 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
 
+	tx_time = max_t(int, tfr->len * 8 * 2 / (speed / 1000), 100);
+	start = jiffies;
 	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(1000));
+					      msecs_to_jiffies(tx_time));
+	end = jiffies;
 	if (!timeout) {
+		dev_warn(&master->dev,
+			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+			 dev_name(&spi->dev), tfr->len, speed,
+			 jiffies_to_msecs(end - start), tx_time);
 		ret = -ETIMEDOUT;
 		goto out;
 	}
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index ac48f59..3d0f66c 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -162,6 +162,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	unsigned int mclk_rate, div, timeout;
 	unsigned int tx_len = 0;
 	int ret = 0;
+	unsigned int start, end, tx_time;
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
@@ -269,9 +270,16 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
 
+	tx_time = max_t(int, tfr->len * 8 * 2 / (speed / 1000), 100);
+	start = jiffies;
 	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(1000));
+					      msecs_to_jiffies(tx_time));
+	end = jiffies;
 	if (!timeout) {
+		dev_warn(&master->dev,
+			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+			 dev_name(&spi->dev), tfr->len, speed,
+			 jiffies_to_msecs(end - start), tx_time);
 		ret = -ETIMEDOUT;
 		goto out;
 	}
-- 
2.1.4

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Maxime Ripard <hidden>
Date: 2015-08-20 14:43:15

On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote:
quoted hunk
The trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over
1MHz SPI bus takes way longer than that. Calculate the timeout from the
actual time the transfer is supposed to take and multiply by 2 for good
measure.

Signed-off-by: Michal Suchanek <redacted>
---
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index fbb0a4d..48532ec 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -170,6 +170,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
+	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
 	u32 reg;
@@ -279,9 +280,16 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
 
+	tx_time = max_t(int, tfr->len * 8 * 2 / (speed / 1000), 100);
+	start = jiffies;
 	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(1000));
+					      msecs_to_jiffies(tx_time));
+	end = jiffies;
 	if (!timeout) {
+		dev_warn(&master->dev,
+			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+			 dev_name(&spi->dev), tfr->len, speed,
+			 jiffies_to_msecs(end - start), tx_time);
Timeout already contains the number of jiffies elapsed (and I'm not
sure anyone reading that message would get that the last number is
actually the maximum timeout).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Mark Brown <broonie@kernel.org>
Date: 2015-08-20 18:42:20

On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote:
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
Are we *sure* we can't work on merging these drivers :(

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Maxime Ripard <hidden>
Date: 2015-08-20 19:34:40

On Thu, Aug 20, 2015 at 11:41:32AM -0700, Mark Brown wrote:
On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote:
quoted
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
Are we *sure* we can't work on merging these drivers :(
Those are two different IPs, that don't really share anything but
their author...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Mark Brown <broonie@kernel.org>
Date: 2015-08-20 21:09:07

On Thu, Aug 20, 2015 at 09:34:33PM +0200, Maxime Ripard wrote:
On Thu, Aug 20, 2015 at 11:41:32AM -0700, Mark Brown wrote:
quoted
On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote:
quoted
quoted
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
quoted
Are we *sure* we can't work on merging these drivers :(
Those are two different IPs, that don't really share anything but
their author...
I seem to be seeing a number of changes like this one which make
apparently very similar modifications to both.  Perhaps there is more
core usage that should be happening instead?

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Maxime Ripard <hidden>
Date: 2015-08-20 21:18:55

On Thu, Aug 20, 2015 at 02:08:30PM -0700, Mark Brown wrote:
On Thu, Aug 20, 2015 at 09:34:33PM +0200, Maxime Ripard wrote:
quoted
On Thu, Aug 20, 2015 at 11:41:32AM -0700, Mark Brown wrote:
quoted
On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote:
quoted
quoted
quoted
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
quoted
quoted
Are we *sure* we can't work on merging these drivers :(
quoted
Those are two different IPs, that don't really share anything but
their author...
I seem to be seeing a number of changes like this one which make
apparently very similar modifications to both.  Perhaps there is more
core usage that should be happening instead?
Yeah, because I wrote the two at the same time, and they share the
same flaws. But that doesn't really mean that you can share anything
at the driver level. And I'm not really sure that we can do much more
at the framework level either, except maybe handling the timeout
directly (but then the drivers would have to handle the recovering
after a timeout too).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/9] spi: sunxi: fix transfer timeout

From: Mark Brown <broonie@kernel.org>
Date: 2015-08-20 21:29:33

On Thu, Aug 20, 2015 at 11:18:50PM +0200, Maxime Ripard wrote:
at the driver level. And I'm not really sure that we can do much more
at the framework level either, except maybe handling the timeout
directly (but then the drivers would have to handle the recovering
after a timeout too).
We have handle_err() for that sort of cleanup.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help