| Message ID | 1428922770-16955-1-git-send-email-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 13, 2015 at 12:59:30PM +0200, Michael Grzeschik wrote: > The current implementation of bitbang_txrx_be_cpha0 and > bitbang_txrx_be_cpha1 always call setmosi. That runs into several > unnecessary calls into the gpiolib when the level of the GPIO actually > has not to be changed. > > This patch changes the routines to remember the last GPIO level > and only calls setmosi if an change has to be made. This > way it improves the transfer throughput. Do you have any benchmarking or anything showing that this offers a practical benefit? Generally a bitbanging system would be using memory mapped GPIOs for the bus so we're talking about replacing a memory access with some shifts and logic operations here and while I can believe that something that runs in cache is going to be an overall win over going out to a device it's not quite so clear cut as it could be.
On April 18, 2015 2:09:26 PM GMT+02:00, Mark Brown <broonie@kernel.org> wrote: >On Mon, Apr 13, 2015 at 12:59:30PM +0200, Michael Grzeschik wrote: >> The current implementation of bitbang_txrx_be_cpha0 and >> bitbang_txrx_be_cpha1 always call setmosi. That runs into several >> unnecessary calls into the gpiolib when the level of the GPIO >actually >> has not to be changed. >> >> This patch changes the routines to remember the last GPIO level >> and only calls setmosi if an change has to be made. This >> way it improves the transfer throughput. > >Do you have any benchmarking or anything showing that this offers a >practical benefit? Generally a bitbanging system would be using memory >mapped GPIOs for the bus so we're talking about replacing a memory >access with some shifts and logic operations here and while I can >believe that something that runs in cache is going to be an overall win >over going out to a device it's not quite so clear cut as it could be. IIRC we had a reduction of about 30% when programming an FPGA via SPI. But Michael can give you the exact numbers. Marc -- 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 Sat, Apr 18, 2015 at 03:18:49PM +0200, Marc Kleine-Budde wrote: > IIRC we had a reduction of about 30% when programming an FPGA via SPI. > But Michael can give you the exact numbers. OK, that's definitely worth writing home about - putting that in the commit message would have helped a lot! I've applied the patch.
diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h index c616e41..58606bf 100644 --- a/drivers/spi/spi-bitbang-txrx.h +++ b/drivers/spi/spi-bitbang-txrx.h @@ -49,12 +49,17 @@ bitbang_txrx_be_cpha0(struct spi_device *spi, { /* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */ + bool oldbit = !(word & (1 << 31)); /* clock starts at inactive polarity */ for (word <<= (32 - bits); likely(bits); bits--) { /* setup MSB (to slave) on trailing edge */ - if ((flags & SPI_MASTER_NO_TX) == 0) - setmosi(spi, word & (1 << 31)); + if ((flags & SPI_MASTER_NO_TX) == 0) { + if (!!(word & (1 << 31)) != oldbit) { + setmosi(spi, word & (1 << 31)); + oldbit = word & (1 << 31); + } + } spidelay(nsecs); /* T(setup) */ setsck(spi, !cpol); @@ -76,13 +81,18 @@ bitbang_txrx_be_cpha1(struct spi_device *spi, { /* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */ + bool oldbit = !(word & (1 << 31)); /* clock starts at inactive polarity */ for (word <<= (32 - bits); likely(bits); bits--) { /* setup MSB (to slave) on leading edge */ setsck(spi, !cpol); - if ((flags & SPI_MASTER_NO_TX) == 0) - setmosi(spi, word & (1 << 31)); + if ((flags & SPI_MASTER_NO_TX) == 0) { + if (!!(word & (1 << 31)) != oldbit) { + setmosi(spi, word & (1 << 31)); + oldbit = word & (1 << 31); + } + } spidelay(nsecs); /* T(setup) */ setsck(spi, cpol);
The current implementation of bitbang_txrx_be_cpha0 and bitbang_txrx_be_cpha1 always call setmosi. That runs into several unnecessary calls into the gpiolib when the level of the GPIO actually has not to be changed. This patch changes the routines to remember the last GPIO level and only calls setmosi if an change has to be made. This way it improves the transfer throughput. Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> --- v2: - added missing braces v3: - fixed initial value of oldbit - added !! to condition for bool comparison drivers/spi/spi-bitbang-txrx.h | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)