| Message ID | 20180212124532.25776-3-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Convert GPIO SPI to use descriptors only | expand |
On Wed, Feb 14, 2018 at 7:53 PM, Trent Piepho <tpiepho@impinj.com> wrote: > On Wed, 2018-02-14 at 16:01 +0000, Mark Brown wrote: >> On Mon, Feb 12, 2018 at 01:45:31PM +0100, Linus Walleij wrote: >> >> > The non-generic bitbang was a feature where a platform could optimize >> > SPI bit-banging by inlining the routines to hammer GPIO lines into >> > the GPIO bitbanging driver as direct register writes using a custom >> > set of GPIO library calls. >> > It does not work with multiplatform concepts, violates everything >> > about how GPIO is made generic and is just generally a bad idea, >> > even on legacy system. Also there is no single user in the entire >> > kernel (for good reasons). >> >> It's not expected that users should go upstream and it doesn't seem >> helpful to delete without replacement. The original idea was to allow >> things like setting multiple GPIOs at once if there were calls for that, >> now that gpiolib has support for that we should at least convert to it >> before removing the hook. > > This ability really does make a difference. I wrote a bit-banged JTAG > driver a powerpc embedded device. Using the int based gpiolib, I could > get about a 1 MHz clock. By allowing one to define a wrapper that > could take compiled in gpio settings, I could get that clock up to > something like 20 MHz. The device had to load an FPGA image via JTAG > when it started, so this was something like 5 seconds of boot time that > would become almost two minutes without the ability to do GPIOs really > fast. That's the difference between a product appearing to customers > as quick compared to what they expect vs pathetic. > > Of course this never made it into the upstream kernel. It only worked > on that specific product. > > The biggest win was due to locking. gpiolib needs each gpio set/get to > be atomic vs other gpio consumers, so you end up with locks around each > call. This is huge overhead when a gpio only needs a single machine > instruction. I could tell the wrapper to lock the gpios at the start > of a JTAG op and unlock after and reduce this overhead by orders of > magnitude. I'm dropping this patch for now, we could revisit it and look at the new gpiolib API for driving multiple lines with single register writes. I remember I actually pushed SPI as an example I wanted to see converted when we implemented that API :/ Yours, Linus Walleij -- 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
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c index b85a93cad44a..2e5f092813f1 100644 --- a/drivers/spi/spi-gpio.c +++ b/drivers/spi/spi-gpio.c @@ -53,34 +53,8 @@ struct spi_gpio { /*----------------------------------------------------------------------*/ -/* - * Because the overhead of going through four GPIO procedure calls - * per transferred bit can make performance a problem, this code - * is set up so that you can use it in either of two ways: - * - * - The slow generic way: set up platform_data to hold the GPIO - * numbers used for MISO/MOSI/SCK, and issue procedure calls for - * each of them. This driver can handle several such busses. - * - * - The quicker inlined way: only helps with platform GPIO code - * that inlines operations for constant GPIOs. This can give - * you tight (fast!) inner loops, but each such bus needs a - * new driver. You'll define a new C file, with Makefile and - * Kconfig support; the C code can be a total of six lines: - * - * #define DRIVER_NAME "myboard_spi2" - * #define SPI_MISO_GPIO 119 - * #define SPI_MOSI_GPIO 120 - * #define SPI_SCK_GPIO 121 - * #define SPI_N_CHIPSEL 4 - * #include "spi-gpio.c" - */ - #ifndef DRIVER_NAME #define DRIVER_NAME "spi_gpio" - -#define GENERIC_BITBANG /* vs tight inlines */ - #endif /*----------------------------------------------------------------------*/ @@ -362,10 +336,8 @@ static int spi_gpio_probe(struct platform_device *pdev) use_of = 1; pdata = dev_get_platdata(&pdev->dev); -#ifdef GENERIC_BITBANG if (!pdata || (!use_of && !pdata->num_chipselect)) return -ENODEV; -#endif master = spi_alloc_master(&pdev->dev, sizeof(*spi_gpio)); if (!master)
The non-generic bitbang was a feature where a platform could optimize SPI bit-banging by inlining the routines to hammer GPIO lines into the GPIO bitbanging driver as direct register writes using a custom set of GPIO library calls. It does not work with multiplatform concepts, violates everything about how GPIO is made generic and is just generally a bad idea, even on legacy system. Also there is no single user in the entire kernel (for good reasons). Delete the remnants of this optimization. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/spi/spi-gpio.c | 28 ---------------------------- 1 file changed, 28 deletions(-) -- 2.14.3 -- 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