| Message ID | 0e0b52774a48ddb71dc8095b66942451cd31ff7d.1440080122.git.hramrach@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 20, 2015 at 02:19:45PM -0000, Michal Suchanek wrote: > 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 <hramrach@gmail.com> > --- > 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
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 :(
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: > > > 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
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: > > 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 :( > 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?
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: > > 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: > > > > > 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... > > 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
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.
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; }
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 <hramrach@gmail.com> --- drivers/spi/spi-sun4i.c | 10 +++++++++- drivers/spi/spi-sun6i.c | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-)