> On 18.04.2015, at 14:27, Mark Brown <[email protected]> wrote:
> That's probably fine, though the 40ms is a bit on the long side. The
> 30s timeout could use pulling in too, that's going to fail very badly if
> anything does go wronng.
Anything below 2 jiffies will give enough false positives during a kernel
recompile - the original code has had 2 jiffies as the effective minimum,
because the calculated delivery-time of max 30us is still orders of magnitudes
smaller than a single jiffy, but a reschedule can happen, which would be the
main reason why we have had triggered timeouts before.
SO IMO anything shorter than 2-3 jifies would need to use a new framework to
get access to high-resolution timers - and I do not know how to do that.
We can implement it as 3 jiffies or 30ms if that seems more acceptable.
Would look something like this:
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 37875cf..1bbfccd 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -68,8 +68,7 @@
#define BCM2835_SPI_CS_CS_10 0x00000002
#define BCM2835_SPI_CS_CS_01 0x00000001
-#define BCM2835_SPI_POLLING_LIMIT_US 30
-#define BCM2835_SPI_TIMEOUT_MS 30000
+#define BCM2835_SPI_POLLING_LIMIT_MS 30
#define BCM2835_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH \
| SPI_NO_CS | SPI_3WIRE)
@@ -164,8 +163,9 @@ static int bcm2835_spi_transfer_one_poll(struct spi_master
*master,
unsigned long xfer_time_us)
{
struct bcm2835_spi *bs = spi_master_get_devdata(master);
- /* set timeout to 1 second of maximum polling */
- unsigned long timeout = jiffies + HZ;
+ /* set timeout and then fall back to interrupts */
+ unsigned long timeout = jiffies +
+ HZ * BCM2835_SPI_POLLING_LIMIT_MS / 1000;
/* enable HW block without interrupts */
bcm2835_wr(bs, BCM2835_SPI_CS, cs | BCM2835_SPI_CS_TA);
@@ -177,13 +177,12 @@ static int bcm2835_spi_transfer_one_poll(struct
spi_master *master,
/* fill in tx fifo as much as possible */
bcm2835_wr_fifo(bs);
/* if we still expect some data after the read,
- * check for a possible timeout
+ * check for a possible timeout and if we reach that
+ * then fallback to interrupt mode...
*/
if (bs->rx_len && time_after(jiffies, timeout)) {
- /* Transfer complete - reset SPI HW */
- bcm2835_spi_reset_hw(master);
- /* and return timeout */
- return -ETIMEDOUT;
+ return bcm2835_spi_transfer_one_irq(master, spi,
+ tfr,cs);
}
}
I will need to test it and then I will submit it as a patch against
the 1s timeout patch (so what is already in for-next).
Martin--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
