[PATCH 0/5] Rewrite GPIO SPI to use descriptors

[PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Linus Walleij]

Just as it says, this rewrites the GPIO bit-banged SPI driver to
just use GPIO descriptors and pushes the configuration of the GPIO
lines down into devices trees and board files.

As there is a patch to the GPIO core, I can provide a tag to
be pulled from the GPIO tree so I can pull the first patch
into the GPIO devel branch for next as well.

There is a testing branch based on v4.15-rc1 already if people
just want to pull it in and test it from there:

https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=gpio-descriptors-spi

Please test it if you can.

Linus Walleij (5):
  gpio: of: Support SPI nonstandard GPIO properties
  spi: spi-gpio: Rewrite to use GPIO descriptors
  spi: spi-gpio: Augment device tree bindings
  spi: spi-gpio: Make optional chipselect handling more explicit
  spi: spi-gpio: Delete references to non-GENERIC_BITBANG

 Documentation/devicetree/bindings/spi/spi-gpio.txt |  24 +-
 arch/arm/mach-pxa/cm-x300.c                        |  21 +-
 arch/arm/mach-pxa/raumfeld.c                       |  26 +-
 arch/arm/mach-s3c24xx/mach-jive.c                  |  55 ++--
 arch/arm/mach-s3c24xx/mach-qt2410.c                |  26 +-
 arch/arm/mach-s3c64xx/mach-smartq.c                |  22 +-
 arch/mips/alchemy/devboards/db1000.c               |  24 +-
 arch/mips/jz4740/board-qi_lb60.c                   |  26 +-
 drivers/gpio/gpiolib-of.c                          |  36 +++
 drivers/misc/eeprom/digsy_mtc_eeprom.c             |  29 +-
 drivers/spi/spi-gpio.c                             | 293 ++++++---------------
 include/linux/spi/spi_gpio.h                       |  49 +---
 12 files changed, 311 insertions(+), 320 deletions(-)

-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] gpio: of: Support SPI nonstandard GPIO properties

[Linus Walleij]

Before it was clearly established that all GPIO properties in the
device tree shall be named "foo-gpios" (with the deprecated variant
"foo-gpio" for single lines) we unfortunately merged a few bindings
which named the lines "gpio-foo" instead.

This is most prominent in the GPIO SPI driver in Linux which names
the lines "gpio-sck", "gpio-mosi" and "gpio-miso".

As we want to switch the GPIO SPI driver to using descriptors, we
need devm_gpiod_get() to return something reasonable when looking
up these in the device tree.

Put in a special #ifdef:ed kludge to do this special lookup only
for the SPI case and gets compiled out if we're not enabling SPI.
If we have more oddly defined legacy GPIOs like this, they can be
handled in a similar manner.

Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/gpio/gpiolib-of.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index e0d59e61b52f..0f2d096c78a3 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -117,6 +117,37 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
 }
 EXPORT_SYMBOL(of_get_named_gpio_flags);
 
+/*
+ * The SPI GPIO bindings happened before we managed to establish that GPIO
+ * properties should be named "foo-gpios" so we have this special kludge for
+ * them.
+ */
+#ifdef CONFIG_SPI_MASTER
+static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
+					  enum of_gpio_flags *of_flags)
+{
+	char prop_name[32]; /* 32 is max size of property name */
+	struct device_node *np = dev->of_node;
+	struct gpio_desc *desc;
+
+	/* Allow this specifically for "spi-gpio" devices */
+	if (!of_device_is_compatible(np, "spi-gpio") || !con_id)
+		return ERR_PTR(-ENOENT);
+
+	/* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
+	snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
+
+	desc = of_get_named_gpiod_flags(np, prop_name, 0, of_flags);
+	return desc;
+}
+#else
+static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
+					  enum of_gpio_flags *of_flags)
+{
+	return ERR_PTR(-ENOENT);
+}
+#endif
+
 struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			       unsigned int idx,
 			       enum gpio_lookup_flags *flags)
@@ -126,6 +157,7 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 	struct gpio_desc *desc;
 	unsigned int i;
 
+	/* Try GPIO property "foo-gpios" and "foo-gpio" */
 	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
 		if (con_id)
 			snprintf(prop_name, sizeof(prop_name), "%s-%s", con_id,
@@ -140,6 +172,10 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			break;
 	}
 
+	/* Special handling for SPI GPIOs if used */
+	if (IS_ERR(desc))
+		desc = of_find_spi_gpio(dev, con_id, &of_flags);
+
 	if (IS_ERR(desc))
 		return desc;
 
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] gpio: of: Support SPI nonstandard GPIO properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

On Mon, Jan 01, 2018 at 02:37:45PM +0100, Linus Walleij wrote:

> Before it was clearly established that all GPIO properties in the
> device tree shall be named "foo-gpios" (with the deprecated variant
> "foo-gpio" for single lines) we unfortunately merged a few bindings
> which named the lines "gpio-foo" instead.

Can we get this merged during the upcoming merge window to make the rest
of the series easier to handle please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/5] gpio: of: Support SPI nonstandard GPIO properties

[Rob Herring]

On Mon, Jan 1, 2018 at 7:37 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Before it was clearly established that all GPIO properties in the
> device tree shall be named "foo-gpios" (with the deprecated variant
> "foo-gpio" for single lines) we unfortunately merged a few bindings
> which named the lines "gpio-foo" instead.
>
> This is most prominent in the GPIO SPI driver in Linux which names
> the lines "gpio-sck", "gpio-mosi" and "gpio-miso".
>
> As we want to switch the GPIO SPI driver to using descriptors, we
> need devm_gpiod_get() to return something reasonable when looking
> up these in the device tree.
>
> Put in a special #ifdef:ed kludge to do this special lookup only
> for the SPI case and gets compiled out if we're not enabling SPI.
> If we have more oddly defined legacy GPIOs like this, they can be
> handled in a similar manner.
>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/gpio/gpiolib-of.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index e0d59e61b52f..0f2d096c78a3 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -117,6 +117,37 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
>  }
>  EXPORT_SYMBOL(of_get_named_gpio_flags);
>
> +/*
> + * The SPI GPIO bindings happened before we managed to establish that GPIO
> + * properties should be named "foo-gpios" so we have this special kludge for
> + * them.
> + */
> +#ifdef CONFIG_SPI_MASTER
> +static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
> +                                         enum of_gpio_flags *of_flags)
> +{
> +       char prop_name[32]; /* 32 is max size of property name */
> +       struct device_node *np = dev->of_node;
> +       struct gpio_desc *desc;

Instead of the ifdef:

if (!IS_ENABLED(CONFIG_SPI_MASTER))
  return ERR_PTR(-ENOENT);

Or merge the condition with the if below.

> +
> +       /* Allow this specifically for "spi-gpio" devices */
> +       if (!of_device_is_compatible(np, "spi-gpio") || !con_id)
> +               return ERR_PTR(-ENOENT);
> +
> +       /* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
> +       snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
> +
> +       desc = of_get_named_gpiod_flags(np, prop_name, 0, of_flags);
> +       return desc;
> +}
> +#else
> +static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
> +                                         enum of_gpio_flags *of_flags)
> +{
> +       return ERR_PTR(-ENOENT);
> +}
> +#endif
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] gpio: of: Support SPI nonstandard GPIO properties

[Linus Walleij]

On Wed, Jan 3, 2018 at 4:23 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Jan 1, 2018 at 7:37 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

>> +/*
>> + * The SPI GPIO bindings happened before we managed to establish that GPIO
>> + * properties should be named "foo-gpios" so we have this special kludge for
>> + * them.
>> + */
>> +#ifdef CONFIG_SPI_MASTER
>> +static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
>> +                                         enum of_gpio_flags *of_flags)
>> +{
>> +       char prop_name[32]; /* 32 is max size of property name */
>> +       struct device_node *np = dev->of_node;
>> +       struct gpio_desc *desc;
>
> Instead of the ifdef:
>
> if (!IS_ENABLED(CONFIG_SPI_MASTER))
>   return ERR_PTR(-ENOENT);
>
> Or merge the condition with the if below.

I was consciously exploting the fact the CONFIG_SPI_MASTER is bool,
but I guess that can be fragile if someone decides to modularize it so, OK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] gpio: of: Support SPI nonstandard GPIO properties

[Andy Shevchenko]

On Thu, Jan 4, 2018 at 10:24 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Jan 3, 2018 at 4:23 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

>> Instead of the ifdef:
>>
>> if (!IS_ENABLED(CONFIG_SPI_MASTER))
>>   return ERR_PTR(-ENOENT);
>>
>> Or merge the condition with the if below.
>
> I was consciously exploting the fact the CONFIG_SPI_MASTER is bool,
> but I guess that can be fragile if someone decides to modularize it so, OK.

Just a side note, we have IS_BUILTIN() for booleans.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] spi: spi-gpio: Rewrite to use GPIO descriptors

[Linus Walleij]

This converts the bit-banged GPIO SPI driver to looking up and
using GPIO descriptors to get a handle on GPIO lines for SCK,
MOSI, MISO and all CS lines.

All existing board files are converted in one go to keep it all
consistent. With these conversions I rarely find any interrim
steps that makes any sense.

Device tree probing and GPIO handling should work like before
also after this patch.

For board files, we stop using controller data to pass the GPIO
line for chip select, instead we pass this as a GPIO descriptor
lookup like everything else.

In some s3c24xx machines the names of the SPI devices were set to
"spi-gpio" rather than "spi_gpio" which can never have worked, I
fixed it working (I guess) as part of this patch set. Sometimes
I wonder how this code got upstream in the first place, it
obviously is not tested.

mach-s3c64xx/mach-smartq.c has the same problem and additionally
defines the *same* GPIO line for MOSI and MISO which is not going
to be accepted by gpiolib. As the lines were number 1,2,2 I assumed
it was a typo and use lines 1,2,3. A comment gives awat that line 0
is chip select though no actual SPI device is provided for the LCD
supposed to be on this bit-banged SPI bus. I left it intact instead
of just deleting the bus though.

Kill off board file code that try to initialize the SPI lines
to the same values that they will later be set by the spi_gpio
driver anyways. Given the huge number of weird things in these
board files I do not think this code is very tested or put in
with much afterthought anyways.

Cc: arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org # Request ACK
Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> # Request ACK
Cc: Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> # S3C stuff
Cc: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> # S3C stuff
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> # S3C Jive
Cc: Harald Welte <laforge-4Bgg8jF3iZdg9hUCZPvPmw@public.gmane.org> # S3C qt2410
Cc: Manuel Lauss <manuel.lauss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> # MIPS db1000
Cc: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org> # JZ4740
Cc: Anatolij Gustschin <agust-ynQEQJNshbs@public.gmane.org> # EEPROM hack
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
ARM SoC and MIPS maintainers: please ACK this so it can be merged
into the SPI tree.
---
 arch/arm/mach-pxa/cm-x300.c            |  21 ++-
 arch/arm/mach-pxa/raumfeld.c           |  26 +++-
 arch/arm/mach-s3c24xx/mach-jive.c      |  55 ++++---
 arch/arm/mach-s3c24xx/mach-qt2410.c    |  26 +++-
 arch/arm/mach-s3c64xx/mach-smartq.c    |  22 ++-
 arch/mips/alchemy/devboards/db1000.c   |  24 ++-
 arch/mips/jz4740/board-qi_lb60.c       |  26 +++-
 drivers/misc/eeprom/digsy_mtc_eeprom.c |  29 +++-
 drivers/spi/spi-gpio.c                 | 258 +++++++++++----------------------
 include/linux/spi/spi_gpio.h           |  49 +------
 10 files changed, 254 insertions(+), 282 deletions(-)

diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
index 868448d2cd82..5227331937d1 100644
--- a/arch/arm/mach-pxa/cm-x300.c
+++ b/arch/arm/mach-pxa/cm-x300.c
@@ -23,6 +23,7 @@
 #include <linux/clk.h>
 
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/dm9000.h>
 #include <linux/leds.h>
 #include <linux/platform_data/rtc-v3020.h>
@@ -343,9 +344,6 @@ static inline void cm_x300_init_bl(void) {}
 #define LCD_SPI_BUS_NUM	(1)
 
 static struct spi_gpio_platform_data cm_x300_spi_gpio_pdata = {
-	.sck		= GPIO_LCD_SCL,
-	.mosi		= GPIO_LCD_DIN,
-	.miso		= GPIO_LCD_DOUT,
 	.num_chipselect	= 1,
 };
 
@@ -357,6 +355,21 @@ static struct platform_device cm_x300_spi_gpio = {
 	},
 };
 
+static struct gpiod_lookup_table cm_x300_spi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_SCL,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_DIN,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_DOUT,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_CS,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct tdo24m_platform_data cm_x300_tdo24m_pdata = {
 	.model = TDO35S,
 };
@@ -367,7 +380,6 @@ static struct spi_board_info cm_x300_spi_devices[] __initdata = {
 		.max_speed_hz		= 1000000,
 		.bus_num		= LCD_SPI_BUS_NUM,
 		.chip_select		= 0,
-		.controller_data	= (void *) GPIO_LCD_CS,
 		.platform_data		= &cm_x300_tdo24m_pdata,
 	},
 };
@@ -376,6 +388,7 @@ static void __init cm_x300_init_spi(void)
 {
 	spi_register_board_info(cm_x300_spi_devices,
 				ARRAY_SIZE(cm_x300_spi_devices));
+	gpiod_add_lookup_table(&cm_x300_spi_gpiod_table);
 	platform_device_register(&cm_x300_spi_gpio);
 }
 #else
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 9d662fed03ec..07289595fe16 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -640,9 +640,6 @@ static void __init raumfeld_lcd_init(void)
  */
 
 static struct spi_gpio_platform_data raumfeld_spi_platform_data = {
-	.sck		= GPIO_SPI_CLK,
-	.mosi		= GPIO_SPI_MOSI,
-	.miso		= GPIO_SPI_MISO,
 	.num_chipselect	= 3,
 };
 
@@ -654,6 +651,25 @@ static struct platform_device raumfeld_spi_device = {
 	}
 };
 
+static struct gpiod_lookup_table raumfeld_spi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("gpio-0", GPIO_SPI_CLK,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-0", GPIO_SPI_MOSI,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-0", GPIO_SPI_MISO,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-0", GPIO_SPDIF_CS,
+				"cs", 0, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-0", GPIO_ACCEL_CS,
+				"cs", 1, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-0", GPIO_MCLK_DAC_CS,
+				"cs", 2, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct lis3lv02d_platform_data lis3_pdata = {
 	.click_flags 	= LIS3_CLICK_SINGLE_X |
 			  LIS3_CLICK_SINGLE_Y |
@@ -674,7 +690,6 @@ static struct lis3lv02d_platform_data lis3_pdata = {
 	.max_speed_hz	= 10000,		\
 	.bus_num	= 0,			\
 	.chip_select	= 0,			\
-	.controller_data = (void *) GPIO_SPDIF_CS,	\
 }
 
 #define SPI_LIS3	\
@@ -683,7 +698,6 @@ static struct lis3lv02d_platform_data lis3_pdata = {
 	.max_speed_hz	= 1000000,		\
 	.bus_num	= 0,			\
 	.chip_select	= 1,			\
-	.controller_data = (void *) GPIO_ACCEL_CS,	\
 	.platform_data	= &lis3_pdata,		\
 	.irq		= PXA_GPIO_TO_IRQ(GPIO_ACCEL_IRQ),	\
 }
@@ -694,7 +708,6 @@ static struct lis3lv02d_platform_data lis3_pdata = {
 	.max_speed_hz	= 1000000,		\
 	.bus_num	= 0,			\
 	.chip_select	= 2,			\
-	.controller_data = (void *) GPIO_MCLK_DAC_CS,	\
 }
 
 static struct spi_board_info connector_spi_devices[] __initdata = {
@@ -1060,6 +1073,7 @@ static void __init raumfeld_common_init(void)
 	else
 		gpio_direction_output(GPIO_SHUTDOWN_SUPPLY, 0);
 
+	gpiod_add_lookup_table(&raumfeld_spi_gpiod_table);
 	platform_add_devices(ARRAY_AND_SIZE(raumfeld_common_devices));
 	i2c_register_board_info(1, &raumfeld_pwri2c_board_info, 1);
 }
diff --git a/arch/arm/mach-s3c24xx/mach-jive.c b/arch/arm/mach-s3c24xx/mach-jive.c
index 17821976f769..7768243f0c47 100644
--- a/arch/arm/mach-s3c24xx/mach-jive.c
+++ b/arch/arm/mach-s3c24xx/mach-jive.c
@@ -17,6 +17,7 @@
 #include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/syscore_ops.h>
 #include <linux/serial_core.h>
 #include <linux/serial_s3c.h>
@@ -393,32 +394,53 @@ static struct ili9320_platdata jive_lcm_config = {
 /* LCD SPI support */
 
 static struct spi_gpio_platform_data jive_lcd_spi = {
-	.sck		= S3C2410_GPG(8),
-	.mosi		= S3C2410_GPB(8),
-	.miso		= SPI_GPIO_NO_MISO,
+	.num_chipselect	= 1,
 };
 
 static struct platform_device jive_device_lcdspi = {
-	.name		= "spi-gpio",
+	.name		= "spi_gpio",
 	.id		= 1,
 	.dev.platform_data = &jive_lcd_spi,
 };
 
+static struct gpiod_lookup_table jive_lcdspi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("GPIOG", 8,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOB", 8,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOB", 7,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
 
 /* WM8750 audio code SPI definition */
 
 static struct spi_gpio_platform_data jive_wm8750_spi = {
-	.sck		= S3C2410_GPB(4),
-	.mosi		= S3C2410_GPB(9),
-	.miso		= SPI_GPIO_NO_MISO,
+	.num_chipselect	= 1,
 };
 
 static struct platform_device jive_device_wm8750 = {
-	.name		= "spi-gpio",
+	.name		= "spi_gpio",
 	.id		= 2,
 	.dev.platform_data = &jive_wm8750_spi,
 };
 
+static struct gpiod_lookup_table jive_wm8750_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("GPIOB", 4,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOB", 9,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOH", 10,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 /* JIVE SPI devices. */
 
 static struct spi_board_info __initdata jive_spi_devs[] = {
@@ -429,14 +451,12 @@ static struct spi_board_info __initdata jive_spi_devs[] = {
 		.mode		= SPI_MODE_3,	/* CPOL=1, CPHA=1 */
 		.max_speed_hz	= 100000,
 		.platform_data	= &jive_lcm_config,
-		.controller_data = (void *)S3C2410_GPB(7),
 	}, {
 		.modalias	= "WM8750",
 		.bus_num	= 2,
 		.chip_select	= 0,
 		.mode		= SPI_MODE_0,	/* CPOL=0, CPHA=0 */
 		.max_speed_hz	= 100000,
-		.controller_data = (void *)S3C2410_GPH(10),
 	},
 };
 
@@ -624,25 +644,12 @@ static void __init jive_machine_init(void)
 	/** TODO - check that this is after the cmdline option! */
 	s3c_nand_set_platdata(&jive_nand_info);
 
-	/* initialise the spi */
-
 	gpio_request(S3C2410_GPG(13), "lcm reset");
 	gpio_direction_output(S3C2410_GPG(13), 0);
 
-	gpio_request(S3C2410_GPB(7), "jive spi");
-	gpio_direction_output(S3C2410_GPB(7), 1);
-
 	gpio_request_one(S3C2410_GPB(6), GPIOF_OUT_INIT_LOW, NULL);
 	gpio_free(S3C2410_GPB(6));
 
-	gpio_request_one(S3C2410_GPG(8), GPIOF_OUT_INIT_HIGH, NULL);
-	gpio_free(S3C2410_GPG(8));
-
-	/* initialise the WM8750 spi */
-
-	gpio_request(S3C2410_GPH(10), "jive wm8750 spi");
-	gpio_direction_output(S3C2410_GPH(10), 1);
-
 	/* Turn off suspend on both USB ports, and switch the
 	 * selectable USB port to USB device mode. */
 
@@ -660,6 +667,8 @@ static void __init jive_machine_init(void)
 
 	pm_power_off = jive_power_off;
 
+	gpiod_add_lookup_table(&jive_lcdspi_gpiod_table);
+	gpiod_add_lookup_table(&jive_wm8750_gpiod_table);
 	platform_add_devices(jive_devices, ARRAY_SIZE(jive_devices));
 }
 
diff --git a/arch/arm/mach-s3c24xx/mach-qt2410.c b/arch/arm/mach-s3c24xx/mach-qt2410.c
index 84e3a9c53184..6e588bb88974 100644
--- a/arch/arm/mach-s3c24xx/mach-qt2410.c
+++ b/arch/arm/mach-s3c24xx/mach-qt2410.c
@@ -28,6 +28,7 @@
 #include <linux/timer.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <linux/serial_core.h>
@@ -211,17 +212,30 @@ static struct platform_device qt2410_led = {
 /* SPI */
 
 static struct spi_gpio_platform_data spi_gpio_cfg = {
-	.sck		= S3C2410_GPG(7),
-	.mosi		= S3C2410_GPG(6),
-	.miso		= S3C2410_GPG(5),
+	.num_chipselect	= 1,
 };
 
 static struct platform_device qt2410_spi = {
-	.name		= "spi-gpio",
+	.name		= "spi_gpio",
 	.id		= 1,
 	.dev.platform_data = &spi_gpio_cfg,
 };
 
+static struct gpiod_lookup_table qt2410_spi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("GPIOG", 7,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOG", 6,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOG", 5,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOB", 5,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 /* Board devices */
 
 static struct platform_device *qt2410_devices[] __initdata = {
@@ -340,9 +354,7 @@ static void __init qt2410_machine_init(void)
 	s3c24xx_udc_set_platdata(&qt2410_udc_cfg);
 	s3c_i2c0_set_platdata(NULL);
 
-	WARN_ON(gpio_request(S3C2410_GPB(5), "spi cs"));
-	gpio_direction_output(S3C2410_GPB(5), 1);
-
+	gpiod_add_lookup_table(&qt2410_spi_gpiod_table);
 	platform_add_devices(qt2410_devices, ARRAY_SIZE(qt2410_devices));
 	s3c_pm_init();
 }
diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c b/arch/arm/mach-s3c64xx/mach-smartq.c
index e0e1a729ef98..34f9c469d24a 100644
--- a/arch/arm/mach-s3c64xx/mach-smartq.c
+++ b/arch/arm/mach-s3c64xx/mach-smartq.c
@@ -213,17 +213,30 @@ static int __init smartq_lcd_setup_gpio(void)
 
 /* GPM0 -> CS */
 static struct spi_gpio_platform_data smartq_lcd_control = {
-	.sck			= S3C64XX_GPM(1),
-	.mosi			= S3C64XX_GPM(2),
-	.miso			= S3C64XX_GPM(2),
+	.num_chipselect	= 1,
 };
 
 static struct platform_device smartq_lcd_control_device = {
-	.name			= "spi-gpio",
+	.name			= "spi_gpio",
 	.id			= 1,
 	.dev.platform_data	= &smartq_lcd_control,
 };
 
+static struct gpiod_lookup_table smartq_lcd_control_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("GPIOM", 1,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOM", 2,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOM", 3,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOM", 0,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static void smartq_lcd_power_set(struct plat_lcd_data *pd, unsigned int power)
 {
 	gpio_direction_output(S3C64XX_GPM(3), power);
@@ -411,6 +424,7 @@ void __init smartq_machine_init(void)
 	WARN_ON(smartq_wifi_init());
 
 	pwm_add_table(smartq_pwm_lookup, ARRAY_SIZE(smartq_pwm_lookup));
+	gpiod_add_lookup_table(&smartq_lcd_control_gpiod_table);
 	platform_add_devices(smartq_devices, ARRAY_SIZE(smartq_devices));
 
 	gpiod_add_lookup_table(&smartq_audio_gpios);
diff --git a/arch/mips/alchemy/devboards/db1000.c b/arch/mips/alchemy/devboards/db1000.c
index 433c4b9a9f0a..fd4dd6598097 100644
--- a/arch/mips/alchemy/devboards/db1000.c
+++ b/arch/mips/alchemy/devboards/db1000.c
@@ -22,6 +22,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/leds.h>
@@ -447,9 +448,6 @@ static struct ads7846_platform_data db1100_touch_pd = {
 };
 
 static struct spi_gpio_platform_data db1100_spictl_pd = {
-	.sck		= 209,
-	.mosi		= 208,
-	.miso		= 207,
 	.num_chipselect = 1,
 };
 
@@ -462,7 +460,6 @@ static struct spi_board_info db1100_spi_info[] __initdata = {
 		.mode		 = 0,
 		.irq		 = AU1100_GPIO21_INT,
 		.platform_data	 = &db1100_touch_pd,
-		.controller_data = (void *)210, /* for spi_gpio: CS# GPIO210 */
 	},
 };
 
@@ -474,6 +471,24 @@ static struct platform_device db1100_spi_dev = {
 	},
 };
 
+/*
+ * Alchemy GPIO 2 has its base at 200 so the GPIO lines
+ * 207 thru 210 are GPIOs at offset 7 thru 10 at this chip.
+ */
+static struct gpiod_lookup_table db1100_spi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("alchemy-gpio2", 9,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("alchemy-gpio2", 8,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("alchemy-gpio2", 7,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("alchemy-gpio2", 10,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
 
 static struct platform_device *db1x00_devs[] = {
 	&db1x00_codec_dev,
@@ -541,6 +556,7 @@ int __init db1000_dev_setup(void)
 			clk_put(p);
 
 		platform_add_devices(db1100_devs, ARRAY_SIZE(db1100_devs));
+		gpiod_add_lookup_table(&db1100_spi_gpiod_table);
 		platform_device_register(&db1100_spi_dev);
 	} else if (board == BCSR_WHOAMI_DB1000) {
 		c0 = AU1000_GPIO2_INT;
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index 6d7f97552200..96500b2867e3 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -313,25 +313,34 @@ static struct jz4740_fb_platform_data qi_lb60_fb_pdata = {
 	.pixclk_falling_edge = 1,
 };
 
-struct spi_gpio_platform_data spigpio_platform_data = {
-	.sck = JZ_GPIO_PORTC(23),
-	.mosi = JZ_GPIO_PORTC(22),
-	.miso = -1,
+struct spi_gpio_platform_data qi_lb60_spigpio_platform_data = {
 	.num_chipselect = 1,
 };
 
-static struct platform_device spigpio_device = {
+static struct platform_device qi_lb60_spigpio_device = {
 	.name = "spi_gpio",
 	.id   = 1,
 	.dev = {
-		.platform_data = &spigpio_platform_data,
+		.platform_data = &qi_lb60_spigpio_platform_data,
+	},
+};
+
+static struct gpiod_lookup_table qi_lb60_spigpio_gpio_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("GPIOC", 23,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOC", 22,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("GPIOC", 21,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
 	},
 };
 
 static struct spi_board_info qi_lb60_spi_board_info[] = {
 	{
 		.modalias = "ili8960",
-		.controller_data = (void *)JZ_GPIO_PORTC(21),
 		.chip_select = 0,
 		.bus_num = 1,
 		.max_speed_hz = 30 * 1000,
@@ -435,7 +444,7 @@ static struct platform_device *jz_platform_devices[] __initdata = {
 	&jz4740_mmc_device,
 	&jz4740_nand_device,
 	&qi_lb60_keypad,
-	&spigpio_device,
+	&qi_lb60_spigpio_device,
 	&jz4740_framebuffer_device,
 	&jz4740_pcm_device,
 	&jz4740_i2s_device,
@@ -489,6 +498,7 @@ static int __init qi_lb60_init_platform_devices(void)
 
 	gpiod_add_lookup_table(&qi_lb60_audio_gpio_table);
 	gpiod_add_lookup_table(&qi_lb60_nand_gpio_table);
+	gpiod_add_lookup_table(&qi_lb60_spigpio_gpio_table);
 
 	spi_register_board_info(qi_lb60_spi_board_info,
 				ARRAY_SIZE(qi_lb60_spi_board_info));
diff --git a/drivers/misc/eeprom/digsy_mtc_eeprom.c b/drivers/misc/eeprom/digsy_mtc_eeprom.c
index 66d9e1baeae5..747a8140de97 100644
--- a/drivers/misc/eeprom/digsy_mtc_eeprom.c
+++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c
@@ -7,9 +7,18 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
+ *
+ * FIXME: this driver is used on a device-tree probed platform: it
+ * should be defined as a bit-banged SPI device and probed from the device
+ * tree and not like this with static grabbing of a few numbered GPIO
+ * lines at random.
+ *
+ * Add proper SPI and EEPROM in arch/powerpc/boot/dts/digsy_mtc.dts
+ * and delete this driver.
  */
 
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
@@ -42,9 +51,6 @@ struct eeprom_93xx46_platform_data digsy_mtc_eeprom_data = {
 };
 
 static struct spi_gpio_platform_data eeprom_spi_gpio_data = {
-	.sck		= GPIO_EEPROM_CLK,
-	.mosi		= GPIO_EEPROM_DI,
-	.miso		= GPIO_EEPROM_DO,
 	.num_chipselect	= 1,
 };
 
@@ -56,6 +62,21 @@ static struct platform_device digsy_mtc_eeprom = {
 	},
 };
 
+static struct gpiod_lookup_table eeprom_spi_gpiod_table = {
+	.dev_id         = "spi_gpio",
+	.table          = {
+		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_CLK,
+			    "gpio-sck", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_DI,
+			    "gpio-mosi", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_DO,
+			    "gpio-miso", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_CS,
+			    "cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = {
 	{
 		.modalias		= "93xx46",
@@ -63,7 +84,6 @@ static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = {
 		.bus_num		= EE_SPI_BUS_NUM,
 		.chip_select		= 0,
 		.mode			= SPI_MODE_0,
-		.controller_data	= (void *)GPIO_EEPROM_CS,
 		.platform_data		= &digsy_mtc_eeprom_data,
 	},
 };
@@ -78,6 +98,7 @@ static int __init digsy_mtc_eeprom_devices_init(void)
 		pr_err("can't request gpio %d\n", GPIO_EEPROM_OE);
 		return ret;
 	}
+	gpiod_add_lookup_table(&eeprom_spi_gpiod_table);
 	spi_register_board_info(digsy_mtc_eeprom_info,
 				ARRAY_SIZE(digsy_mtc_eeprom_info));
 	return platform_device_register(&digsy_mtc_eeprom);
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 1c34c9314c8a..ab2cb9427481 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -2,6 +2,7 @@
  * SPI master driver using generic bitbanged GPIO
  *
  * Copyright (C) 2006,2008 David Brownell
+ * Copyright (C) 2017 Linus Walleij
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,10 +17,9 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
@@ -44,7 +44,11 @@ struct spi_gpio {
 	struct spi_bitbang		bitbang;
 	struct spi_gpio_platform_data	pdata;
 	struct platform_device		*pdev;
-	unsigned long			cs_gpios[0];
+	struct gpio_desc		*sck;
+	struct gpio_desc		*miso;
+	struct gpio_desc		*mosi;
+	/* Will be allocated times number of devices beyond end of struct */
+	struct gpio_desc		*cs_gpios[0];
 };
 
 /*----------------------------------------------------------------------*/
@@ -77,13 +81,6 @@ struct spi_gpio {
 
 #define GENERIC_BITBANG	/* vs tight inlines */
 
-/* all functions referencing these symbols must define pdata */
-#define SPI_MISO_GPIO	((pdata)->miso)
-#define SPI_MOSI_GPIO	((pdata)->mosi)
-#define SPI_SCK_GPIO	((pdata)->sck)
-
-#define SPI_N_CHIPSEL	((pdata)->num_chipselect)
-
 #endif
 
 /*----------------------------------------------------------------------*/
@@ -105,25 +102,27 @@ spi_to_pdata(const struct spi_device *spi)
 	return &spi_to_spi_gpio(spi)->pdata;
 }
 
-/* this is #defined to avoid unused-variable warnings when inlining */
-#define pdata		spi_to_pdata(spi)
-
+/* These helpers are in turn called by the bitbang inlines */
 static inline void setsck(const struct spi_device *spi, int is_on)
 {
-	gpio_set_value_cansleep(SPI_SCK_GPIO, is_on);
+	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
+
+	gpiod_set_value_cansleep(spi_gpio->sck, is_on);
 }
 
 static inline void setmosi(const struct spi_device *spi, int is_on)
 {
-	gpio_set_value_cansleep(SPI_MOSI_GPIO, is_on);
+	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
+
+	gpiod_set_value_cansleep(spi_gpio->mosi, is_on);
 }
 
 static inline int getmiso(const struct spi_device *spi)
 {
-	return !!gpio_get_value_cansleep(SPI_MISO_GPIO);
-}
+	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
 
-#undef pdata
+	return !!gpiod_get_value_cansleep(spi_gpio->miso);
+}
 
 /*
  * NOTE:  this clocks "as fast as we can".  It "should" be a function of the
@@ -216,123 +215,89 @@ static u32 spi_gpio_spec_txrx_word_mode3(struct spi_device *spi,
 static void spi_gpio_chipselect(struct spi_device *spi, int is_active)
 {
 	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
-	unsigned long cs = spi_gpio->cs_gpios[spi->chip_select];
+	struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select];
 
-	/* set initial clock polarity */
+	/* set initial clock line level */
 	if (is_active)
-		setsck(spi, spi->mode & SPI_CPOL);
+		gpiod_set_value_cansleep(spi_gpio->sck, spi->mode & SPI_CPOL);
 
-	if (cs != SPI_GPIO_NO_CHIPSELECT) {
-		/* SPI is normally active-low */
-		gpio_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active);
-	}
+	if (cs)
+		/* SPI chip selects are normally active-low */
+		gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active);
 }
 
 static int spi_gpio_setup(struct spi_device *spi)
 {
-	unsigned long		cs;
+	struct gpio_desc	*cs;
 	int			status = 0;
 	struct spi_gpio		*spi_gpio = spi_to_spi_gpio(spi);
-	struct device_node	*np = spi->master->dev.of_node;
-
-	if (np) {
-		/*
-		 * In DT environments, the CS GPIOs have already been
-		 * initialized from the "cs-gpios" property of the node.
-		 */
-		cs = spi_gpio->cs_gpios[spi->chip_select];
-	} else {
-		/*
-		 * ... otherwise, take it from spi->controller_data
-		 */
-		cs = (uintptr_t) spi->controller_data;
-	}
 
-	if (!spi->controller_state) {
-		if (cs != SPI_GPIO_NO_CHIPSELECT) {
-			status = gpio_request(cs, dev_name(&spi->dev));
-			if (status)
-				return status;
-			status = gpio_direction_output(cs,
-					!(spi->mode & SPI_CS_HIGH));
-		}
-	}
-	if (!status) {
-		/* in case it was initialized from static board data */
-		spi_gpio->cs_gpios[spi->chip_select] = cs;
+	/*
+	 * The CS GPIOs have already been
+	 * initialized from the descriptor lookup.
+	 */
+	cs = spi_gpio->cs_gpios[spi->chip_select];
+	if (!spi->controller_state && cs)
+		status = gpiod_direction_output(cs,
+						!(spi->mode & SPI_CS_HIGH));
+
+	if (!status)
 		status = spi_bitbang_setup(spi);
-	}
 
-	if (status) {
-		if (!spi->controller_state && cs != SPI_GPIO_NO_CHIPSELECT)
-			gpio_free(cs);
-	}
 	return status;
 }
 
 static void spi_gpio_cleanup(struct spi_device *spi)
 {
-	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
-	unsigned long cs = spi_gpio->cs_gpios[spi->chip_select];
-
-	if (cs != SPI_GPIO_NO_CHIPSELECT)
-		gpio_free(cs);
 	spi_bitbang_cleanup(spi);
 }
 
-static int spi_gpio_alloc(unsigned pin, const char *label, bool is_in)
-{
-	int value;
-
-	value = gpio_request(pin, label);
-	if (value == 0) {
-		if (is_in)
-			value = gpio_direction_input(pin);
-		else
-			value = gpio_direction_output(pin, 0);
-	}
-	return value;
-}
-
-static int spi_gpio_request(struct spi_gpio_platform_data *pdata,
-			    const char *label, u16 *res_flags)
+/*
+ * It can be convenient to use this driver with pins that have alternate
+ * functions associated with a "native" SPI controller if a driver for that
+ * controller is not available, or is missing important functionality.
+ *
+ * On platforms which can do so, configure MISO with a weak pullup unless
+ * there's an external pullup on that signal.  That saves power by avoiding
+ * floating signals.  (A weak pulldown would save power too, but many
+ * drivers expect to see all-ones data as the no slave "response".)
+ */
+static int spi_gpio_request(struct device *dev,
+			    struct spi_gpio *spi_gpio,
+			    unsigned int num_chipselects,
+			    u16 *mflags)
 {
-	int value;
+	int i;
 
-	/* NOTE:  SPI_*_GPIO symbols may reference "pdata" */
-
-	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI) {
-		value = spi_gpio_alloc(SPI_MOSI_GPIO, label, false);
-		if (value)
-			goto done;
-	} else {
+	spi_gpio->mosi = devm_gpiod_get_optional(dev, "mosi", GPIOD_OUT_LOW);
+	if (IS_ERR(spi_gpio->mosi))
+		return PTR_ERR(spi_gpio->mosi);
+	if (!spi_gpio->mosi)
 		/* HW configuration without MOSI pin */
-		*res_flags |= SPI_MASTER_NO_TX;
-	}
+		*mflags |= SPI_MASTER_NO_TX;
 
-	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO) {
-		value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
-		if (value)
-			goto free_mosi;
-	} else {
+	spi_gpio->miso = devm_gpiod_get_optional(dev, "miso", GPIOD_IN);
+	if (IS_ERR(spi_gpio->miso))
+		return PTR_ERR(spi_gpio->miso);
+	if (!spi_gpio->miso)
 		/* HW configuration without MISO pin */
-		*res_flags |= SPI_MASTER_NO_RX;
-	}
+		*mflags |= SPI_MASTER_NO_RX;
 
-	value = spi_gpio_alloc(SPI_SCK_GPIO, label, false);
-	if (value)
-		goto free_miso;
+	spi_gpio->sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);
+	if (IS_ERR(spi_gpio->mosi))
+		return PTR_ERR(spi_gpio->mosi);
 
-	goto done;
+	for (i = 0; i < num_chipselects; i++) {
+		spi_gpio->cs_gpios[i] = devm_gpiod_get_index(dev, "cs",
+							     i, GPIOD_OUT_HIGH);
+		if (IS_ERR(spi_gpio->cs_gpios[i]))
+			return PTR_ERR(spi_gpio->cs_gpios[i]);
+	}
+	/* Dummy chipselect line if the single device is not using chipselect */
+	if (!num_chipselects)
+		spi_gpio->cs_gpios[0] = NULL;
 
-free_miso:
-	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
-		gpio_free(SPI_MISO_GPIO);
-free_mosi:
-	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
-		gpio_free(SPI_MOSI_GPIO);
-done:
-	return value;
+	return 0;
 }
 
 #ifdef CONFIG_OF
@@ -358,26 +323,6 @@ static int spi_gpio_probe_dt(struct platform_device *pdev)
 	if (!pdata)
 		return -ENOMEM;
 
-	ret = of_get_named_gpio(np, "gpio-sck", 0);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "gpio-sck property not found\n");
-		goto error_free;
-	}
-	pdata->sck = ret;
-
-	ret = of_get_named_gpio(np, "gpio-miso", 0);
-	if (ret < 0) {
-		dev_info(&pdev->dev, "gpio-miso property not found, switching to no-rx mode\n");
-		pdata->miso = SPI_GPIO_NO_MISO;
-	} else
-		pdata->miso = ret;
-
-	ret = of_get_named_gpio(np, "gpio-mosi", 0);
-	if (ret < 0) {
-		dev_info(&pdev->dev, "gpio-mosi property not found, switching to no-tx mode\n");
-		pdata->mosi = SPI_GPIO_NO_MOSI;
-	} else
-		pdata->mosi = ret;
 
 	ret = of_property_read_u32(np, "num-chipselects", &tmp);
 	if (ret < 0) {
@@ -423,21 +368,16 @@ static int spi_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 #endif
 
-	if (use_of && !SPI_N_CHIPSEL)
+	if (use_of && !pdata->num_chipselect)
 		num_devices = 1;
 	else
-		num_devices = SPI_N_CHIPSEL;
-
-	status = spi_gpio_request(pdata, dev_name(&pdev->dev), &master_flags);
-	if (status < 0)
-		return status;
+		num_devices = pdata->num_chipselect;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(*spi_gpio) +
-					(sizeof(unsigned long) * num_devices));
-	if (!master) {
-		status = -ENOMEM;
-		goto gpio_free;
-	}
+				  (sizeof(struct gpio_desc *) * num_devices));
+	if (!master)
+		return -ENOMEM;
+
 	spi_gpio = spi_master_get_devdata(master);
 	platform_set_drvdata(pdev, spi_gpio);
 
@@ -445,6 +385,11 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	if (pdata)
 		spi_gpio->pdata = *pdata;
 
+	status = spi_gpio_request(&pdev->dev, spi_gpio,
+				  pdata->num_chipselect, &master_flags);
+	if (status)
+		return status;
+
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
 	master->flags = master_flags;
 	master->bus_num = pdev->id;
@@ -453,29 +398,6 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	master->cleanup = spi_gpio_cleanup;
 #ifdef CONFIG_OF
 	master->dev.of_node = pdev->dev.of_node;
-
-	if (use_of) {
-		int i;
-		struct device_node *np = pdev->dev.of_node;
-
-		/*
-		 * In DT environments, take the CS GPIO from the "cs-gpios"
-		 * property of the node.
-		 */
-
-		if (!SPI_N_CHIPSEL)
-			spi_gpio->cs_gpios[0] = SPI_GPIO_NO_CHIPSELECT;
-		else
-			for (i = 0; i < SPI_N_CHIPSEL; i++) {
-				status = of_get_named_gpio(np, "cs-gpios", i);
-				if (status < 0) {
-					dev_err(&pdev->dev,
-						"invalid cs-gpios property\n");
-					goto gpio_free;
-				}
-				spi_gpio->cs_gpios[i] = status;
-			}
-	}
 #endif
 
 	spi_gpio->bitbang.master = master;
@@ -496,15 +418,8 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	spi_gpio->bitbang.flags = SPI_CS_HIGH;
 
 	status = spi_bitbang_start(&spi_gpio->bitbang);
-	if (status < 0) {
-gpio_free:
-		if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
-			gpio_free(SPI_MISO_GPIO);
-		if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
-			gpio_free(SPI_MOSI_GPIO);
-		gpio_free(SPI_SCK_GPIO);
+	if (status)
 		spi_master_put(master);
-	}
 
 	return status;
 }
@@ -520,11 +435,6 @@ static int spi_gpio_remove(struct platform_device *pdev)
 	/* stop() unregisters child devices too */
 	spi_bitbang_stop(&spi_gpio->bitbang);
 
-	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
-		gpio_free(SPI_MISO_GPIO);
-	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
-		gpio_free(SPI_MOSI_GPIO);
-	gpio_free(SPI_SCK_GPIO);
 	spi_master_put(spi_gpio->bitbang.master);
 
 	return 0;
diff --git a/include/linux/spi/spi_gpio.h b/include/linux/spi/spi_gpio.h
index e7bd89a59cd1..9e7e83d8645b 100644
--- a/include/linux/spi/spi_gpio.h
+++ b/include/linux/spi/spi_gpio.h
@@ -8,64 +8,17 @@
  *   - id the same as the SPI bus number it implements
  *   - dev.platform data pointing to a struct spi_gpio_platform_data
  *
- * Or, see the driver code for information about speedups that are
- * possible on platforms that support inlined access for GPIOs (no
- * spi_gpio_platform_data is used).
- *
- * Use spi_board_info with these busses in the usual way, being sure
- * that the controller_data being the GPIO used for each device's
- * chipselect:
- *
- *	static struct spi_board_info ... [] = {
- *	...
- *		// this slave uses GPIO 42 for its chipselect
- *		.controller_data = (void *) 42,
- *	...
- *		// this one uses GPIO 86 for its chipselect
- *		.controller_data = (void *) 86,
- *	...
- *	};
- *
- * If chipselect is not used (there's only one device on the bus), assign
- * SPI_GPIO_NO_CHIPSELECT to the controller_data:
- *		.controller_data = (void *) SPI_GPIO_NO_CHIPSELECT;
- *
- * If the MISO or MOSI pin is not available then it should be set to
- * SPI_GPIO_NO_MISO or SPI_GPIO_NO_MOSI.
+ * Use spi_board_info with these busses in the usual way.
  *
  * If the bitbanged bus is later switched to a "native" controller,
  * that platform_device and controller_data should be removed.
  */
 
-#define SPI_GPIO_NO_CHIPSELECT		((unsigned long)-1l)
-#define SPI_GPIO_NO_MISO		((unsigned long)-1l)
-#define SPI_GPIO_NO_MOSI		((unsigned long)-1l)
-
 /**
  * struct spi_gpio_platform_data - parameter for bitbanged SPI master
- * @sck: number of the GPIO used for clock output
- * @mosi: number of the GPIO used for Master Output, Slave In (MOSI) data
- * @miso: number of the GPIO used for Master Input, Slave Output (MISO) data
  * @num_chipselect: how many slaves to allow
- *
- * All GPIO signals used with the SPI bus managed through this driver
- * (chipselects, MOSI, MISO, SCK) must be configured as GPIOs, instead
- * of some alternate function.
- *
- * It can be convenient to use this driver with pins that have alternate
- * functions associated with a "native" SPI controller if a driver for that
- * controller is not available, or is missing important functionality.
- *
- * On platforms which can do so, configure MISO with a weak pullup unless
- * there's an external pullup on that signal.  That saves power by avoiding
- * floating signals.  (A weak pulldown would save power too, but many
- * drivers expect to see all-ones data as the no slave "response".)
  */
 struct spi_gpio_platform_data {
-	unsigned	sck;
-	unsigned long	mosi;
-	unsigned long	miso;
-
 	u16		num_chipselect;
 };
 
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] spi: spi-gpio: Rewrite to use GPIO descriptors

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Mon, Jan 01, 2018 at 02:37:46PM +0100, Linus Walleij wrote:

> mach-s3c64xx/mach-smartq.c has the same problem and additionally
> defines the *same* GPIO line for MOSI and MISO which is not going
> to be accepted by gpiolib. As the lines were number 1,2,2 I assumed

You do get three wire SPI buses where one data line is used for both
MOSI and MISO so that might be intentional - IIRC it used to work fine
with a lot of SPI chips.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/5] spi: spi-gpio: Rewrite to use GPIO descriptors

[Olof Johansson]

On Mon, Jan 01, 2018 at 02:37:46PM +0100, Linus Walleij wrote:
> This converts the bit-banged GPIO SPI driver to looking up and
> using GPIO descriptors to get a handle on GPIO lines for SCK,
> MOSI, MISO and all CS lines.
> 
> All existing board files are converted in one go to keep it all
> consistent. With these conversions I rarely find any interrim
> steps that makes any sense.
> 
> Device tree probing and GPIO handling should work like before
> also after this patch.
> 
> For board files, we stop using controller data to pass the GPIO
> line for chip select, instead we pass this as a GPIO descriptor
> lookup like everything else.
> 
> In some s3c24xx machines the names of the SPI devices were set to
> "spi-gpio" rather than "spi_gpio" which can never have worked, I
> fixed it working (I guess) as part of this patch set. Sometimes
> I wonder how this code got upstream in the first place, it
> obviously is not tested.
> 
> mach-s3c64xx/mach-smartq.c has the same problem and additionally
> defines the *same* GPIO line for MOSI and MISO which is not going
> to be accepted by gpiolib. As the lines were number 1,2,2 I assumed
> it was a typo and use lines 1,2,3. A comment gives awat that line 0
> is chip select though no actual SPI device is provided for the LCD
> supposed to be on this bit-banged SPI bus. I left it intact instead
> of just deleting the bus though.
> 
> Kill off board file code that try to initialize the SPI lines
> to the same values that they will later be set by the spi_gpio
> driver anyways. Given the huge number of weird things in these
> board files I do not think this code is very tested or put in
> with much afterthought anyways.
> 
> Cc: arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org # Request ACK
> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> # Request ACK
> Cc: Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> # S3C stuff
> Cc: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> # S3C stuff
> Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> # S3C Jive
> Cc: Harald Welte <laforge-4Bgg8jF3iZdg9hUCZPvPmw@public.gmane.org> # S3C qt2410
> Cc: Manuel Lauss <manuel.lauss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> # MIPS db1000
> Cc: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org> # JZ4740
> Cc: Anatolij Gustschin <agust-ynQEQJNshbs@public.gmane.org> # EEPROM hack
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] spi: spi-gpio: Rewrite to use GPIO descriptors

[Andy Shevchenko]

On Mon, Jan 1, 2018 at 3:37 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> +static struct gpiod_lookup_table cm_x300_spi_gpiod_table = {
> +       .dev_id         = "spi_gpio",
> +       .table          = {
> +               GPIO_LOOKUP("gpio-pxa", GPIO_LCD_SCL,
> +                           "gpio-sck", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("gpio-pxa", GPIO_LCD_DIN,
> +                           "gpio-mosi", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("gpio-pxa", GPIO_LCD_DOUT,
> +                           "gpio-miso", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("gpio-pxa", GPIO_LCD_CS,
> +                           "cs", GPIO_ACTIVE_HIGH),
> +               { },
> +       },
> +};

...and why not to use new proper names from the beggining here?

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] spi: spi-gpio: Augment device tree bindings

[Linus Walleij]

After we augmented the core to handle "gpio-sck"/"sck-gpios",
"gpio-mosi"/"mosi-gpios", "gpio-miso"/"miso-gpios" alike,
deprecate the old binding and put the strict modern and
recommended binding practice into place as the default for
GPIO-based SPI.

Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/spi-gpio.txt | 24 ++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-gpio.txt b/Documentation/devicetree/bindings/spi/spi-gpio.txt
index a95603bcf6ff..52db562f17a4 100644
--- a/Documentation/devicetree/bindings/spi/spi-gpio.txt
+++ b/Documentation/devicetree/bindings/spi/spi-gpio.txt
@@ -1,18 +1,30 @@
 SPI-GPIO devicetree bindings
 
+This represents a group of 3-n GPIO lines used for bit-banged SPI on dedicated
+GPIO lines.
+
 Required properties:
 
  - compatible: should be set to "spi-gpio"
  - #address-cells: should be set to <0x1>
  - ranges
- - gpio-sck: GPIO spec for the SCK line to use
- - gpio-miso: GPIO spec for the MISO line to use
- - gpio-mosi: GPIO spec for the MOSI line to use
+ - sck-gpios: GPIO spec for the SCK line to use
+ - miso-gpios: GPIO spec for the MISO line to use
+ - mosi-gpios: GPIO spec for the MOSI line to use
  - cs-gpios: GPIOs to use for chipselect lines.
              Not needed if num-chipselects = <0>.
  - num-chipselects: Number of chipselect lines. Should be <0> if a single device
                     with no chip select is connected.
 
+Deprecated bindings:
+
+These legacy GPIO line bindings can alternatively be used to define the
+GPIO lines used, they should not be used in new device trees.
+
+ - gpio-sck: GPIO spec for the SCK line to use
+ - gpio-miso: GPIO spec for the MISO line to use
+ - gpio-mosi: GPIO spec for the MOSI line to use
+
 Example:
 
 	spi {
@@ -20,9 +32,9 @@ Example:
 		#address-cells = <0x1>;
 		ranges;
 
-		gpio-sck = <&gpio 95 0>;
-		gpio-miso = <&gpio 98 0>;
-		gpio-mosi = <&gpio 97 0>;
+		sck-gpios = <&gpio 95 0>;
+		miso-gpios = <&gpio 98 0>;
+		mosi-gpios = <&gpio 97 0>;
 		cs-gpios = <&gpio 125 0>;
 		num-chipselects = <1>;
 
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Applied "spi: spi-gpio: Augment device tree bindings" to the spi tree

[Mark Brown]

The patch

   spi: spi-gpio: Augment device tree bindings

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 77a060533c0468069d0b4a5d1968a42259763f08 Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Date: Mon, 12 Feb 2018 13:45:32 +0100
Subject: [PATCH] spi: spi-gpio: Augment device tree bindings

After we augmented the core to handle "gpio-sck"/"sck-gpios",
"gpio-mosi"/"mosi-gpios", "gpio-miso"/"miso-gpios" alike,
deprecate the old binding and put the strict modern and
recommended binding practice into place as the default for
GPIO-based SPI.

This reflects the similar change in I2C:
commit 7d29f509d2cf
("dt-bindings: i2c: i2c-gpio: Add support for named gpios")

Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/spi-gpio.txt | 24 ++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-gpio.txt b/Documentation/devicetree/bindings/spi/spi-gpio.txt
index a95603bcf6ff..52db562f17a4 100644
--- a/Documentation/devicetree/bindings/spi/spi-gpio.txt
+++ b/Documentation/devicetree/bindings/spi/spi-gpio.txt
@@ -1,18 +1,30 @@
 SPI-GPIO devicetree bindings
 
+This represents a group of 3-n GPIO lines used for bit-banged SPI on dedicated
+GPIO lines.
+
 Required properties:
 
  - compatible: should be set to "spi-gpio"
  - #address-cells: should be set to <0x1>
  - ranges
- - gpio-sck: GPIO spec for the SCK line to use
- - gpio-miso: GPIO spec for the MISO line to use
- - gpio-mosi: GPIO spec for the MOSI line to use
+ - sck-gpios: GPIO spec for the SCK line to use
+ - miso-gpios: GPIO spec for the MISO line to use
+ - mosi-gpios: GPIO spec for the MOSI line to use
  - cs-gpios: GPIOs to use for chipselect lines.
              Not needed if num-chipselects = <0>.
  - num-chipselects: Number of chipselect lines. Should be <0> if a single device
                     with no chip select is connected.
 
+Deprecated bindings:
+
+These legacy GPIO line bindings can alternatively be used to define the
+GPIO lines used, they should not be used in new device trees.
+
+ - gpio-sck: GPIO spec for the SCK line to use
+ - gpio-miso: GPIO spec for the MISO line to use
+ - gpio-mosi: GPIO spec for the MOSI line to use
+
 Example:
 
 	spi {
@@ -20,9 +32,9 @@ Example:
 		#address-cells = <0x1>;
 		ranges;
 
-		gpio-sck = <&gpio 95 0>;
-		gpio-miso = <&gpio 98 0>;
-		gpio-mosi = <&gpio 97 0>;
+		sck-gpios = <&gpio 95 0>;
+		miso-gpios = <&gpio 98 0>;
+		mosi-gpios = <&gpio 97 0>;
 		cs-gpios = <&gpio 125 0>;
 		num-chipselects = <1>;
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Linus Walleij]

I don't like the use of a NULL GPIO descriptor to handle the
"no chipselect connected" case, as it makes the code hard to read.
Use a clear bool "has_cs" that we use to explicitly handle the
case when no chip select is connected to it is clear to readers
how this is achieved.

When there is no chip select connected, we don't even allocate a
placeholder for the GPIO descriptor.

Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-gpio.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index ab2cb9427481..c12e588e54e7 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -47,6 +47,7 @@ struct spi_gpio {
 	struct gpio_desc		*sck;
 	struct gpio_desc		*miso;
 	struct gpio_desc		*mosi;
+	bool				has_cs;
 	/* Will be allocated times number of devices beyond end of struct */
 	struct gpio_desc		*cs_gpios[0];
 };
@@ -215,15 +216,18 @@ static u32 spi_gpio_spec_txrx_word_mode3(struct spi_device *spi,
 static void spi_gpio_chipselect(struct spi_device *spi, int is_active)
 {
 	struct spi_gpio *spi_gpio = spi_to_spi_gpio(spi);
-	struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select];
 
 	/* set initial clock line level */
 	if (is_active)
 		gpiod_set_value_cansleep(spi_gpio->sck, spi->mode & SPI_CPOL);
 
-	if (cs)
+	/* Drive chip select line, if we have one */
+	if (spi_gpio->has_cs) {
+		struct gpio_desc *cs = spi_gpio->cs_gpios[spi->chip_select];
+
 		/* SPI chip selects are normally active-low */
 		gpiod_set_value_cansleep(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active);
+	}
 }
 
 static int spi_gpio_setup(struct spi_device *spi)
@@ -293,9 +297,6 @@ static int spi_gpio_request(struct device *dev,
 		if (IS_ERR(spi_gpio->cs_gpios[i]))
 			return PTR_ERR(spi_gpio->cs_gpios[i]);
 	}
-	/* Dummy chipselect line if the single device is not using chipselect */
-	if (!num_chipselects)
-		spi_gpio->cs_gpios[0] = NULL;
 
 	return 0;
 }
@@ -354,7 +355,6 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	struct spi_gpio_platform_data	*pdata;
 	u16 master_flags = 0;
 	bool use_of = 0;
-	int num_devices;
 
 	status = spi_gpio_probe_dt(pdev);
 	if (status < 0)
@@ -368,19 +368,17 @@ static int spi_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 #endif
 
-	if (use_of && !pdata->num_chipselect)
-		num_devices = 1;
-	else
-		num_devices = pdata->num_chipselect;
-
 	master = spi_alloc_master(&pdev->dev, sizeof(*spi_gpio) +
-				  (sizeof(struct gpio_desc *) * num_devices));
+				  (sizeof(struct gpio_desc *) * pdata->num_chipselect));
 	if (!master)
 		return -ENOMEM;
 
 	spi_gpio = spi_master_get_devdata(master);
 	platform_set_drvdata(pdev, spi_gpio);
 
+	/* Determine if we have chip selects connected */
+	spi_gpio->has_cs = !!pdata->num_chipselect;
+
 	spi_gpio->pdev = pdev;
 	if (pdata)
 		spi_gpio->pdata = *pdata;
@@ -393,7 +391,8 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
 	master->flags = master_flags;
 	master->bus_num = pdev->id;
-	master->num_chipselect = num_devices;
+	/* The master needs to think there is a chipselect even if not connected */
+	master->num_chipselect = spi_gpio->has_cs ? pdata->num_chipselect : 1;
 	master->setup = spi_gpio_setup;
 	master->cleanup = spi_gpio_cleanup;
 #ifdef CONFIG_OF
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Geert Uytterhoeven]

Hi Linus,

On Mon, Jan 1, 2018 at 2:37 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> I don't like the use of a NULL GPIO descriptor to handle the
> "no chipselect connected" case, as it makes the code hard to read.
> Use a clear bool "has_cs" that we use to explicitly handle the
> case when no chip select is connected to it is clear to readers
> how this is achieved.

Hmm, having two indicators for the same feature (spi_gpio->has_cs == true and
spi_gpio->cs_gpios[0] = NULL), and the need to keep them in sync in the
future, also complicates things.

> When there is no chip select connected, we don't even allocate a
> placeholder for the GPIO descriptor.

That's good, although it may have no effect on actual memory consumption
due to slab granularity.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Linus Walleij]

On Wed, Jan 3, 2018 at 9:21 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:

> Hmm, having two indicators for the same feature (spi_gpio->has_cs == true and
> spi_gpio->cs_gpios[0] = NULL), and the need to keep them in sync in the
> future, also complicates things.

The patch does not do this.

It removes the latter and adds the former. That is why it removes the
assignment of NULL to cs_gpios[0].

>> When there is no chip select connected, we don't even allocate a
>> placeholder for the GPIO descriptor.
>
> That's good, although it may have no effect on actual memory consumption
> due to slab granularity.

No idea, I'm not doing it to save memory anyways.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Geert Uytterhoeven]

Hi Linus,

On Wed, Jan 3, 2018 at 9:58 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Jan 3, 2018 at 9:21 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>
>> Hmm, having two indicators for the same feature (spi_gpio->has_cs == true and
>> spi_gpio->cs_gpios[0] = NULL), and the need to keep them in sync in the
>> future, also complicates things.
>
> The patch does not do this.
>
> It removes the latter and adds the former. That is why it removes the
> assignment of NULL to cs_gpios[0].

So what is cs_gpios[0] if has_cs == true? Oh, it will point to unallocated
memory right after the structure...

See, that's the ambiguity of having two variables...
We do have NULL pointers and ERR_PTRs, so this ambiguity can (and IMHO
should) be avoided.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Linus Walleij]

On Wed, Jan 3, 2018 at 10:17 AM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Wed, Jan 3, 2018 at 9:58 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Jan 3, 2018 at 9:21 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>>
>>> Hmm, having two indicators for the same feature (spi_gpio->has_cs == true and
>>> spi_gpio->cs_gpios[0] = NULL), and the need to keep them in sync in the
>>> future, also complicates things.
>>
>> The patch does not do this.
>>
>> It removes the latter and adds the former. That is why it removes the
>> assignment of NULL to cs_gpios[0].
>
> So what is cs_gpios[0] if has_cs == true? Oh, it will point to unallocated
> memory right after the structure...

I think it is a problem with all dynamic arrays: they are prone to out-of-bounds
accesses.

That in turn comes from the following design pattern i the spi alloc_master()
helper:

        status = spi_gpio_request(pdata, dev_name(&pdev->dev), &master_flags);
        if (status < 0)
                return status;

        master = spi_alloc_master(&pdev->dev, sizeof(*spi_gpio) +
                                        (sizeof(unsigned long) * num_devices));
        if (!master) {
                status = -ENOMEM;
                goto gpio_free;
        }
        spi_gpio = spi_master_get_devdata(master);

So someone wanted to save a slab by using this instead of allocating
the array dynamically. It's not a very admirable coding style to begin
with. I doubt the system gains much from this.

If you agree I can rewrite it to have struct gpio_desc ** and
dynamically allocate that to the number of descriptors with devm_kzalloc()
instead, that is more in line with kernel patterns and this memory
optimization anyways seems a bit overdone anyways.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] spi: spi-gpio: Make optional chipselect handling more explicit

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Wed, Jan 03, 2018 at 10:40:52AM +0100, Linus Walleij wrote:

> So someone wanted to save a slab by using this instead of allocating
> the array dynamically. It's not a very admirable coding style to begin
> with. I doubt the system gains much from this.

Right, David was very focused on saving memory.

> If you agree I can rewrite it to have struct gpio_desc ** and
> dynamically allocate that to the number of descriptors with devm_kzalloc()
> instead, that is more in line with kernel patterns and this memory
> optimization anyways seems a bit overdone anyways.

Yes, that would be clearer - it's something that's good to fix anyway
rather than trying to contort around it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 5/5] spi: spi-gpio: Delete references to non-GENERIC_BITBANG

[Linus Walleij]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-gpio.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index c12e588e54e7..437b1cd4da71 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -54,34 +54,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
 
 /*----------------------------------------------------------------------*/
@@ -363,10 +337,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) +
 				  (sizeof(struct gpio_desc *) * pdata->num_chipselect));
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] spi: spi-gpio: Delete references to non-GENERIC_BITBANG

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Mon, Jan 01, 2018 at 02:37:49PM +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).

> Delete the remnants of this optimization.

What's the positive reason for removing this?  I'd not anticipate people
doing this upstream but it seems like a totally valid thing to do on
product.  It's just compiling out the gpiolib calls so it never looked
like too big a maintanence overhead.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Linus Walleij]

On Mon, Jan 1, 2018 at 2:37 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> Just as it says, this rewrites the GPIO bit-banged SPI driver to
> just use GPIO descriptors and pushes the configuration of the GPIO
> lines down into devices trees and board files.

I pulled this into my Gemini tree where I have a bit-banged SPI device
based on device tree, and everything works smoothly. So all DT-based
users of GPIO SPI should be working fine after this series.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Trent Piepho]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 537 bytes --]

On Mon, 2018-01-01 at 14:37 +0100, Linus Walleij wrote:
> Just as it says, this rewrites the GPIO bit-banged SPI driver to
> just use GPIO descriptors and pushes the configuration of the GPIO
> lines down into devices trees and board files.

Any before/after benchmark for speed or code size?

> 
>  12 files changed, 311 insertions(+), 320 deletions(-)
> 

Looks like code size is pretty even.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±²˜¢žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

Re: [PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Linus Walleij]

On Tue, Jan 2, 2018 at 7:42 PM, Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 2018-01-01 at 14:37 +0100, Linus Walleij wrote:
>> Just as it says, this rewrites the GPIO bit-banged SPI driver to
>> just use GPIO descriptors and pushes the configuration of the GPIO
>> lines down into devices trees and board files.
>
> Any before/after benchmark for speed or code size?

Not really, the main motivation for the change is neither performance
nor object code size, but to get rid of the global GPIO numbers, which
does not scale very well and makes stuff messy. Istead we aim
to make code deal with abstract GPIO handles known as
GPIO descriptors.

The motivation is the same as for moving IRQ numbers to IRQ
descriptors which has been going on for some time: it is easy when
you have just say 64 IRQs or GPIOs. The subsystems were designed
for that. As the number IRQ controllers and GPIO controllers on
a system grows, managing the number space and the hardware
descriptions becomes a pain.

I'd be surprised if either performance or object code size is affected
in a bad way though. I don't have any high-speed SPI devices I can test
really, and loopback would require some special hardware set-up I
don't have.

The last patch in the series remove remnants of an unused mechanism
that seemed to be for performance critical bitbanged SPI, that was never
put to use, I kind of assume noone is really doing performance critical
SPI over GPIO but I've been wrong before.

> Looks like code size is pretty even.

Number of lines yes, readability I would argue is better but it is a matter
of taste. I would hope some bytes have been pushed away from
the core and into the board files (etc) but I could be wrong.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

On Wed, Jan 03, 2018 at 12:17:07AM +0100, Linus Walleij wrote:

> I'd be surprised if either performance or object code size is affected
> in a bad way though. I don't have any high-speed SPI devices I can test
> really, and loopback would require some special hardware set-up I
> don't have.

You could just do something like spin on register reads on some device
or something?  Something like dumping a regmap via debugfs would do the
trick probably.  I too would be surprised if there were a substantial
hit but it'd be good to check, while they've never been fast equally the
slowness can make people more sensitive to performance (including impact
on the rest of the system).  One of the reasons people can end up with
the GPIO controller is discovering some hardware bug in a hardware
controller (like the issues a lot of the Freescale controllers have with
trying to deassert chip select on every word transferred).

> The last patch in the series remove remnants of an unused mechanism
> that seemed to be for performance critical bitbanged SPI, that was never
> put to use, I kind of assume noone is really doing performance critical
> SPI over GPIO but I've been wrong before.

It was never really possible to upstream users of that code, AFAICT it
was always for custom out of tree builds.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] Rewrite GPIO SPI to use descriptors

[Linus Walleij]

On Wed, Jan 3, 2018 at 5:24 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Jan 03, 2018 at 12:17:07AM +0100, Linus Walleij wrote:
>
>> I'd be surprised if either performance or object code size is affected
>> in a bad way though. I don't have any high-speed SPI devices I can test
>> really, and loopback would require some special hardware set-up I
>> don't have.
>
> You could just do something like spin on register reads on some device
> or something?  Something like dumping a regmap via debugfs would do the
> trick probably.

OK I tried this on my new Ilitek ILI9322 panel driver:

A script like this dumping the regmap 10000 times (it
has no caching obviously) on an otherwise idle system
in two iterations before and after the patches:

#!/bin/sh
for run in `seq 10000`
do
    cat /debug/regmap/spi0.0/registers > /dev/null
done

Before the patch:
time test.sh
real    3m 41.03s
user    0m 29.41s
sys     3m 7.22s

time test.sh
real    3m 44.24s
user    0m 32.31s
sys     3m 7.60s

After the patch:

time test.sh
real    3m 41.32s
user    0m 28.92s
sys     3m 8.08s

time test.sh
real    3m 39.92s
user    0m 30.20s
sys     3m 5.56s

So any performance differences seems to be in the error margin.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html