[PATCH v2 0/5] spi-gpio updates

[PATCH v2 0/5] spi-gpio updates

[Sascha Hauer]

Our spio-gpio driver currently only supports 8bit word width. This
series updates the driver to support arbitrary word widths up to 32bit.
Also we get rid of the deprecated GPIO binding that the driver uses.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Changes in v2:
- use ndelay to implement spi_delay()
- extract initial CS setting to extra patch
- add patch to convert to gpiod API
- update sandbox-libftdi-example.dtsi to use new GPIO binding
- Link to v1: https://lore.barebox.org/20240925-spi-gpio-v1-0-47f6285b3bf1@pengutronix.de

---
Sascha Hauer (5):
      spi: spi-gpio: actually delay in spidelay()
      spi: spi-gpio: support different word widths
      spi: spi-gpio: switch to new gpio binding
      spi: spi-gpio: deassert CS during setup
      spi: spi-gpio: switch to gpiod api

 arch/arm/dts/stm32mp151-mect1s.dts            |   6 +-
 arch/sandbox/dts/sandbox-libftdi-example.dtsi |   6 +-
 drivers/spi/gpio_spi.c                        | 181 +++++++++++++-------------
 include/spi/spi_gpio.h                        |  27 ----
 4 files changed, 95 insertions(+), 125 deletions(-)
---
base-commit: 419ea9350aa083d4a2806a70132129a49a5ecf95
change-id: 20240925-spi-gpio-4e65be1f7b90

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH v2 1/5] spi: spi-gpio: actually delay in spidelay()

[Sascha Hauer]

spidelay() currently only is a no-op dummy function. Actually delay in
this function to avoid spi-gpio being faster than specified.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/gpio_spi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/gpio_spi.c b/drivers/spi/gpio_spi.c
index e5664df3fe..dc9f46ca45 100644
--- a/drivers/spi/gpio_spi.c
+++ b/drivers/spi/gpio_spi.c
@@ -48,7 +48,10 @@ static inline int getmiso(const struct spi_device *spi)
 	return !!gpio_get_value(priv->data->miso);
 }
 
-#define spidelay(nsecs) do { } while (0)
+static inline void spidelay(unsigned int nsecs)
+{
+	ndelay(nsecs);
+}
 
 #include "spi-bitbang-txrx.h"
 

-- 
2.39.5

[PATCH v2 2/5] spi: spi-gpio: support different word widths

[Sascha Hauer]

The spi-gpio driver only supports 8bit word width. Add support for
arbitrary word widths up to 32bit.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/gpio_spi.c | 57 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/gpio_spi.c b/drivers/spi/gpio_spi.c
index dc9f46ca45..d23abf3dfb 100644
--- a/drivers/spi/gpio_spi.c
+++ b/drivers/spi/gpio_spi.c
@@ -78,7 +78,7 @@ static inline u32 gpio_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
 		return bitbang_txrx_be_cpha0(spi, nsecs, cpol, word, bits);
 }
 
-static int gpio_spi_transfer_one(struct spi_device *spi, struct spi_transfer *t)
+static int gpio_spi_transfer_one_u8(struct spi_device *spi, struct spi_transfer *t)
 {
 	bool read = (t->rx_buf) ? true : false;
 	u32 word = 0;
@@ -87,14 +87,59 @@ static int gpio_spi_transfer_one(struct spi_device *spi, struct spi_transfer *t)
 	for (n = 0; n < t->len; n++) {
 		if (!read)
 			word = ((const u8 *)t->tx_buf)[n];
-		word = gpio_spi_txrx_word(spi, 0, word, 8);
+		word = gpio_spi_txrx_word(spi, 0, word, spi->bits_per_word);
+		if (read)
+			((u8 *)t->rx_buf)[n] = word;
+	}
+
+	return 0;
+}
+
+static int gpio_spi_transfer_one_u16(struct spi_device *spi, struct spi_transfer *t)
+{
+	bool read = (t->rx_buf) ? true : false;
+	u32 word = 0;
+	int n;
+
+	for (n = 0; n < t->len / 2; n++) {
+		if (!read)
+			word = ((const u16 *)t->tx_buf)[n];
+		word = gpio_spi_txrx_word(spi, 0, word, spi->bits_per_word);
 		if (read)
-			((u8 *)t->rx_buf)[n] = word & 0xff;
+			((u16 *)t->rx_buf)[n] = word;
 	}
 
 	return 0;
 }
 
+static int gpio_spi_transfer_one_u32(struct spi_device *spi, struct spi_transfer *t)
+{
+	bool read = (t->rx_buf) ? true : false;
+	u32 word = 0;
+	int n;
+
+	for (n = 0; n < t->len / 4; n++) {
+		if (!read)
+			word = ((const u32 *)t->tx_buf)[n];
+		word = gpio_spi_txrx_word(spi, 0, word, spi->bits_per_word);
+		if (read)
+			((u32 *)t->rx_buf)[n] = word;
+	}
+
+	return 0;
+}
+
+static int gpio_spi_transfer_one(struct spi_device *spi, struct spi_transfer *t)
+{
+	if (spi->bits_per_word <= 8)
+		return gpio_spi_transfer_one_u8(spi, t);
+	if (spi->bits_per_word <= 16)
+		return gpio_spi_transfer_one_u16(spi, t);
+	if (spi->bits_per_word <= 32)
+		return gpio_spi_transfer_one_u32(spi, t);
+	return -EINVAL;
+}
+
 static int gpio_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 {
 	struct spi_transfer *t;
@@ -120,12 +165,6 @@ static int gpio_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 
 static int gpio_spi_setup(struct spi_device *spi)
 {
-	if (spi->bits_per_word != 8) {
-		dev_err(spi->master->dev, "master does not support %d bits per word\n",
-			spi->bits_per_word);
-		return -EINVAL;
-	}
-
 	return 0;
 }
 

-- 
2.39.5

[PATCH v2 3/5] spi: spi-gpio: switch to new gpio binding

[Sascha Hauer]

The old deprecated device tree binding for the spi-gpio driver uses
"gpio-sck", "gpio-mosi" and "gpio-miso" to specify the GPIOs. Switch to
the new binding which uses the standard GPIO property names.

The old binding is still used in some device trees, but none of the SoCs
using them is actually supported in barebox, so do not bother to keep a
fallback to the old binding. The one in-tree user of the old binding is
converted in this patch.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/dts/stm32mp151-mect1s.dts            | 6 +++---
 arch/sandbox/dts/sandbox-libftdi-example.dtsi | 6 +++---
 drivers/spi/gpio_spi.c                        | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/dts/stm32mp151-mect1s.dts b/arch/arm/dts/stm32mp151-mect1s.dts
index 4d0e31d189..b0dc1cfaa8 100644
--- a/arch/arm/dts/stm32mp151-mect1s.dts
+++ b/arch/arm/dts/stm32mp151-mect1s.dts
@@ -65,9 +65,9 @@ led-1 {
 
 	spi_gpio: spi-gpio-0 {
 		compatible = "spi-gpio";
-		gpio-sck = <&gpioi 1 GPIO_ACTIVE_HIGH>;
-		gpio-mosi = <&gpioi 3 GPIO_ACTIVE_HIGH>;
-		gpio-miso = <&gpioi 2 GPIO_ACTIVE_HIGH>;
+		sck-gpios = <&gpioi 1 GPIO_ACTIVE_HIGH>;
+		mosi-gpios = <&gpioi 3 GPIO_ACTIVE_HIGH>;
+		miso-gpios = <&gpioi 2 GPIO_ACTIVE_HIGH>;
 		cs-gpios = <&gpioj 3 GPIO_ACTIVE_LOW>;
 		num-chipselects = <1>;
 		#address-cells = <1>;
diff --git a/arch/sandbox/dts/sandbox-libftdi-example.dtsi b/arch/sandbox/dts/sandbox-libftdi-example.dtsi
index c1c3074097..0a1bb0bb82 100644
--- a/arch/sandbox/dts/sandbox-libftdi-example.dtsi
+++ b/arch/sandbox/dts/sandbox-libftdi-example.dtsi
@@ -25,9 +25,9 @@ spi0: spi0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		gpio-sck  = <&gpio0 0 GPIO_ACTIVE_HIGH>;
-		gpio-mosi = <&gpio0 1 GPIO_ACTIVE_HIGH>;
-		gpio-miso = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+		sck-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+		mosi-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+		miso-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
 		cs-gpios  = <&gpio0 3 GPIO_ACTIVE_HIGH>;
 
 		num-chipselects = <1>;
diff --git a/drivers/spi/gpio_spi.c b/drivers/spi/gpio_spi.c
index d23abf3dfb..417e8db30e 100644
--- a/drivers/spi/gpio_spi.c
+++ b/drivers/spi/gpio_spi.c
@@ -177,7 +177,7 @@ static int gpio_spi_of_probe(struct device *dev)
 	if (!IS_ENABLED(CONFIG_OFDEVICE) || dev->platform_data)
 		return 0;
 
-	sck = of_get_named_gpio(np, "gpio-sck", 0);
+	sck = of_get_named_gpio(np, "sck-gpios", 0);
 	if (!gpio_is_valid(sck))
 		return dev_err_probe(dev, sck < 0 ? sck : -EINVAL,
 				     "missing mandatory SCK gpio\n");
@@ -186,11 +186,11 @@ static int gpio_spi_of_probe(struct device *dev)
 	pdata->sck = sck;
 	pdata->num_cs = MAX_CHIPSELECT;
 
-	pdata->miso = of_get_named_gpio(np, "gpio-miso", 0);
+	pdata->miso = of_get_named_gpio(np, "miso-gpios", 0);
 	if (!gpio_is_valid(pdata->miso))
 		pdata->miso = SPI_GPIO_NO_MISO;
 
-	pdata->mosi = of_get_named_gpio(np, "gpio-mosi", 0);
+	pdata->mosi = of_get_named_gpio(np, "mosi-gpios", 0);
 	if (!gpio_is_valid(pdata->mosi))
 		pdata->mosi = SPI_GPIO_NO_MOSI;
 

-- 
2.39.5

[PATCH v2 4/5] spi: spi-gpio: deassert CS during setup

[Sascha Hauer]

The initial CS gpio state is unknown. When it's asserted the first
transfer might fail. Deassert the CS during setup to make sure the
slave gets a fresh start.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/gpio_spi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/gpio_spi.c b/drivers/spi/gpio_spi.c
index 417e8db30e..36e6204283 100644
--- a/drivers/spi/gpio_spi.c
+++ b/drivers/spi/gpio_spi.c
@@ -165,6 +165,7 @@ static int gpio_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 
 static int gpio_spi_setup(struct spi_device *spi)
 {
+	gpio_spi_set_cs(spi, 0);
 	return 0;
 }
 

-- 
2.39.5

[PATCH v2 5/5] spi: spi-gpio: switch to gpiod api

[Sascha Hauer]

Switch the driver to the gpiod api. While at it, remove the unused
platform_data support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/gpio_spi.c | 118 +++++++++++++++----------------------------------
 include/spi/spi_gpio.h |  27 -----------
 2 files changed, 36 insertions(+), 109 deletions(-)

diff --git a/drivers/spi/gpio_spi.c b/drivers/spi/gpio_spi.c
index 36e6204283..6062bad98a 100644
--- a/drivers/spi/gpio_spi.c
+++ b/drivers/spi/gpio_spi.c
@@ -12,16 +12,19 @@
 #include <driver.h>
 #include <errno.h>
 #include <gpio.h>
+#include <linux/gpio/consumer.h>
 #include <init.h>
 #include <io.h>
 #include <malloc.h>
 #include <of_gpio.h>
 #include <spi/spi.h>
-#include <spi/spi_gpio.h>
 
 struct gpio_spi {
 	struct spi_master master;
-	struct gpio_spi_pdata *data;
+	struct gpio_desc *sck;
+	struct gpio_desc *mosi;
+	struct gpio_desc *miso;
+	struct gpio_descs *cs;
 };
 
 #define priv_from_spi_device(s)	container_of(s->master, struct gpio_spi, master)
@@ -29,23 +32,28 @@ struct gpio_spi {
 static inline void setsck(const struct spi_device *spi, int is_on)
 {
 	struct gpio_spi *priv = priv_from_spi_device(spi);
-	gpio_set_value(priv->data->sck, is_on);
+
+	gpiod_set_value(priv->sck, is_on);
 }
 
 static inline void setmosi(const struct spi_device *spi, int is_on)
 {
 	struct gpio_spi *priv = priv_from_spi_device(spi);
-	if (!gpio_is_valid(priv->data->mosi))
+
+	if (!priv->mosi)
 		return;
-	gpio_set_value(priv->data->mosi, is_on);
+
+	gpiod_set_value(priv->mosi, is_on);
 }
 
 static inline int getmiso(const struct spi_device *spi)
 {
 	struct gpio_spi *priv = priv_from_spi_device(spi);
-	if (!gpio_is_valid(priv->data->miso))
+
+	if (!priv->miso)
 		return 1;
-	return !!gpio_get_value(priv->data->miso);
+
+	return !!gpiod_get_value(priv->miso);
 }
 
 static inline void spidelay(unsigned int nsecs)
@@ -59,11 +67,13 @@ static int gpio_spi_set_cs(struct spi_device *spi, bool en)
 {
 	struct gpio_spi *priv = priv_from_spi_device(spi);
 
-	if (!gpio_is_valid(priv->data->cs[spi->chip_select]))
-		return 0;
-
-	gpio_set_value(priv->data->cs[spi->chip_select],
-		       (spi->mode & SPI_CS_HIGH) ? en : !en);
+	/*
+	 * Use the raw variant here. Devices using active high chip select
+	 * either have the spi-cs-high property in the device tree or set
+	 * SPI_CS_HIGH in their driver.
+	 */
+	gpiod_set_raw_value(priv->cs->desc[spi->chip_select],
+			    (spi->mode & SPI_CS_HIGH) ? en : !en);
 
 	return 0;
 }
@@ -169,91 +179,35 @@ static int gpio_spi_setup(struct spi_device *spi)
 	return 0;
 }
 
-static int gpio_spi_of_probe(struct device *dev)
-{
-	struct device_node *np = dev->of_node;
-	struct gpio_spi_pdata *pdata;
-	int n, sck;
-
-	if (!IS_ENABLED(CONFIG_OFDEVICE) || dev->platform_data)
-		return 0;
-
-	sck = of_get_named_gpio(np, "sck-gpios", 0);
-	if (!gpio_is_valid(sck))
-		return dev_err_probe(dev, sck < 0 ? sck : -EINVAL,
-				     "missing mandatory SCK gpio\n");
-
-	pdata = xzalloc(sizeof(*pdata));
-	pdata->sck = sck;
-	pdata->num_cs = MAX_CHIPSELECT;
-
-	pdata->miso = of_get_named_gpio(np, "miso-gpios", 0);
-	if (!gpio_is_valid(pdata->miso))
-		pdata->miso = SPI_GPIO_NO_MISO;
-
-	pdata->mosi = of_get_named_gpio(np, "mosi-gpios", 0);
-	if (!gpio_is_valid(pdata->mosi))
-		pdata->mosi = SPI_GPIO_NO_MOSI;
-
-	for (n = 0; n < MAX_CHIPSELECT; n++) {
-		pdata->cs[n] = of_get_named_gpio(np, "cs-gpios", n);
-		if (!gpio_is_valid(pdata->cs[n]))
-		    pdata->cs[n] = SPI_GPIO_NO_CS;
-	}
-
-	dev->platform_data = pdata;
-
-	return 0;
-}
-
 static int gpio_spi_probe(struct device *dev)
 {
 	struct gpio_spi *priv;
-	struct gpio_spi_pdata *pdata;
 	struct spi_master *master;
-	int n, ret;
-
-	ret = gpio_spi_of_probe(dev);
-	if (ret)
-		return ret;
-	pdata = dev->platform_data;
 
-	ret = gpio_request_one(pdata->sck, GPIOF_DIR_OUT, "spi-sck");
-	if (ret)
-		return ret;
+	priv = xzalloc(sizeof(*priv));
 
-	if (pdata->miso != SPI_GPIO_NO_MISO) {
-		ret = gpio_request_one(pdata->miso, GPIOF_DIR_IN, "spi-miso");
-		if (ret)
-			return ret;
-	}
+	priv->sck = gpiod_get(dev, "sck", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->sck))
+		return dev_err_probe(dev, PTR_ERR(priv->sck), "No SCK GPIO\n");
 
-	if (pdata->mosi != SPI_GPIO_NO_MOSI) {
-		ret = gpio_request_one(pdata->mosi, GPIOF_DIR_OUT, "spi-mosi");
-		if (ret)
-			return ret;
-	}
+	priv->miso = gpiod_get_optional(dev, "miso", GPIOD_IN);
+	if (IS_ERR(priv->miso))
+		return dev_err_probe(dev, PTR_ERR(priv->miso), "No MISO GPIO\n");
 
-	for (n = 0; n < pdata->num_cs; n++) {
-		char *cs_name;
+	priv->mosi = gpiod_get_optional(dev, "mosi", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->mosi))
+		return dev_err_probe(dev, PTR_ERR(priv->mosi), "No MOSI GPIO\n");
 
-		if (!gpio_is_valid(pdata->cs[n]))
-			continue;
+	priv->cs = gpiod_get_array(dev, "cs", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->cs))
+		return dev_err_probe(dev, PTR_ERR(priv->cs), "No CS GPIOs\n");
 
-		cs_name = basprintf("spi-cs%d", n);
-		ret = gpio_request_one(pdata->cs[n], GPIOF_DIR_OUT, cs_name);
-		if (ret)
-			return ret;
-	}
-
-	priv = xzalloc(sizeof(*priv));
-	priv->data = pdata;
 	master = &priv->master;
 	master->dev = dev;
 	master->bus_num = dev->id;
 	master->setup = gpio_spi_setup;
 	master->transfer = gpio_spi_transfer;
-	master->num_chipselect = priv->data->num_cs;
+	master->num_chipselect = priv->cs->ndescs;
 
 	return spi_register_master(&priv->master);
 }
diff --git a/include/spi/spi_gpio.h b/include/spi/spi_gpio.h
deleted file mode 100644
index a18b7dbe07..0000000000
--- a/include/spi/spi_gpio.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * SPI master driver using generic bitbanged GPIO
- *
- * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
- *
- * Based on Linux driver
- *   Copyright (C) 2006,2008 David Brownell
- */
-
-#ifndef __SPI_GPIO_H
-#define __SPI_GPIO_H
-
-#define MAX_CHIPSELECT		4
-#define SPI_GPIO_NO_CS		(-1)
-#define SPI_GPIO_NO_MISO	(-1)
-#define SPI_GPIO_NO_MOSI	(-1)
-
-struct gpio_spi_pdata {
-	int sck;
-	int mosi;
-	int miso;
-	int cs[MAX_CHIPSELECT];
-	int num_cs;
-};
-
-#endif

-- 
2.39.5

Re: [PATCH v2 0/5] spi-gpio updates

[Sascha Hauer]

On Thu, 26 Sep 2024 13:21:36 +0200, Sascha Hauer wrote:
> Our spio-gpio driver currently only supports 8bit word width. This
> series updates the driver to support arbitrary word widths up to 32bit.
> Also we get rid of the deprecated GPIO binding that the driver uses.
> 
> 

Applied, thanks!

[1/5] spi: spi-gpio: actually delay in spidelay()
      https://git.pengutronix.de/cgit/barebox/commit/?id=105dd3824c50 (link may not be stable)
[2/5] spi: spi-gpio: support different word widths
      https://git.pengutronix.de/cgit/barebox/commit/?id=bd782e60aada (link may not be stable)
[3/5] spi: spi-gpio: switch to new gpio binding
      https://git.pengutronix.de/cgit/barebox/commit/?id=b0c73a574c2c (link may not be stable)
[4/5] spi: spi-gpio: deassert CS during setup
      https://git.pengutronix.de/cgit/barebox/commit/?id=ddb7dc177d0f (link may not be stable)
[5/5] spi: spi-gpio: switch to gpiod api
      https://git.pengutronix.de/cgit/barebox/commit/?id=6cb75a3f2f79 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>