| Message ID | 1438155250-14476-1-git-send-email-kernel@martin.sperl.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0122a5183088e3117bb9c8fbe248914efb502f3f |
| Headers | show |
On Wed, Jul 29, 2015 at 07:34:10AM +0000, kernel@martin.sperl.org wrote: > From: Martin Sperl <kernel@martin.sperl.org> > > This resulted in the use of polling mode when other approaches > (dma or interrupts) would have been more appropriate. Please describe the actual issue and how it's being fixed in the commit message. In this case the fix appears to be converting the timeout variable to a long long which will make things better but obviously isn't a complete fix for overflows - presumably the ideal thing here is to handle saturation better?
> On 29.07.2015, at 16:03, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Jul 29, 2015 at 07:34:10AM +0000, kernel@martin.sperl.org wrote: >> From: Martin Sperl <kernel@martin.sperl.org> >> >> This resulted in the use of polling mode when other approaches >> (dma or interrupts) would have been more appropriate. > > Please describe the actual issue and how it's being fixed in the commit > message. In this case the fix appears to be converting the timeout > variable to a long long which will make things better but obviously > isn't a complete fix for overflows - presumably the ideal thing here is > to handle saturation better? Will change the description of the commit and resubmit. As for saturation I guess that nobody will ever run a spi-transfer of len=2049638230413 bytes (at which point it would wrap) which: a) would take 4679 days to reach at 125MHz (the max frequency of the bcm2835 SPI HW limit) b) would not fit spi_transfer->len (defined as unsigned) So handling saturation is not an issue when using "long long” in the context of this device. Martin-- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark! > On 29.07.2015, at 16:31, Martin Sperl <kernel@martin.sperl.org> wrote: > > Will change the description of the commit and resubmit. Now that you have applied the patch already, do I need to resubmit the commit with a changed description? Martin -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 29, 2015 at 04:35:03PM +0200, Martin Sperl wrote: > Now that you have applied the patch already, do I need to > resubmit the commit with a changed description? No, it's fine.
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index c9357bb..e7874a6 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -480,7 +480,7 @@ static int bcm2835_spi_transfer_one_poll(struct spi_master *master, struct spi_device *spi, struct spi_transfer *tfr, u32 cs, - unsigned long xfer_time_us) + unsigned long long xfer_time_us) { struct bcm2835_spi *bs = spi_master_get_devdata(master); unsigned long timeout; @@ -531,7 +531,8 @@ static int bcm2835_spi_transfer_one(struct spi_master *master, { struct bcm2835_spi *bs = spi_master_get_devdata(master); unsigned long spi_hz, clk_hz, cdiv; - unsigned long spi_used_hz, xfer_time_us; + unsigned long spi_used_hz; + unsigned long long xfer_time_us; u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS); /* set clock */ @@ -573,9 +574,10 @@ static int bcm2835_spi_transfer_one(struct spi_master *master, bs->rx_len = tfr->len; /* calculate the estimated time in us the transfer runs */ - xfer_time_us = tfr->len + xfer_time_us = (unsigned long long)tfr->len * 9 /* clocks/byte - SPI-HW waits 1 clock after each byte */ - * 1000000 / spi_used_hz; + * 1000000; + do_div(xfer_time_us, spi_used_hz); /* for short requests run polling*/ if (xfer_time_us <= BCM2835_SPI_POLLING_LIMIT_US)