diff mbox series

RFT: spi: bcm2835: Convert to use CS GPIO descriptors

Message ID 20190130092551.14856-1-linus.walleij@linaro.org
State New
Headers show
Series RFT: spi: bcm2835: Convert to use CS GPIO descriptors | expand

Commit Message

Linus Walleij Jan. 30, 2019, 9:25 a.m. UTC
This converts the BCM2835 SPI master driver to use GPIO
descriptors for chip select handling.

The BCM2835 driver was relying on the core to drive the
CS high/low so very small changes were needed for this
part. If it managed to request the CS from the device tree
node, all is pretty straight forward.

However for native GPIOs this driver has a quite unorthodox
loopback to request some GPIOs from the SoC GPIO chip by
looking it up from the device tree using gpiochip_find()
and then offseting hard into its numberspace. This has
been augmented a bit by using gpiochip_request_own_desc()
but this code really needs to be verified. If "native CS"
is actually an SoC GPIO, why is it even done this way?
Should this GPIO not just be defined in the device tree
like any other CS GPIO? I'm confused.

Cc: Lukas Wunner <lukas@wunner.de>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Chris Boot <bootc@bootc.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
I would very much appreciate if someone took this for
a ride on top of the SPI tree (or linux-next) and see
if all still works as expected.
---
 drivers/spi/spi-bcm2835.c | 42 ++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 16 deletions(-)

-- 
2.20.1
diff mbox series

Patch

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 35aebdfd3b4e..464cbfe70778 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -33,7 +33,8 @@ 
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h> /* FIXME: using chip internals */
 #include <linux/of_irq.h>
 #include <linux/spi/spi.h>
 
@@ -881,12 +882,17 @@  static int bcm2835_spi_setup(struct spi_device *spi)
 {
 	int err;
 	struct gpio_chip *chip;
+	enum gpiod_flags flags;
 	/*
 	 * sanity checking the native-chipselects
 	 */
 	if (spi->mode & SPI_NO_CS)
 		return 0;
-	if (gpio_is_valid(spi->cs_gpio))
+	/*
+	 * The SPI core has successfully requested the CS GPIO line from the
+	 * device tree, so we are done.
+	 */
+	if (spi->cs_gpiod)
 		return 0;
 	if (spi->chip_select > 1) {
 		/* error in the case of native CS requested with CS > 1
@@ -897,7 +903,15 @@  static int bcm2835_spi_setup(struct spi_device *spi)
 			"setup: only two native chip-selects are supported\n");
 		return -EINVAL;
 	}
-	/* now translate native cs to GPIO */
+
+	/*
+	 * Translate native CS to GPIO
+	 *
+	 * FIXME: poking around in the gpiolib internals like this is
+	 * not very good practice. Find a way to locate the real problem
+	 * and fix it. Why is the GPIO descriptor in spi->cs_gpiod
+	 * sometimes not assigned correctly? Erroneous device trees?
+	 */
 
 	/* get the gpio chip for the base */
 	chip = gpiochip_find("pinctrl-bcm2835", chip_match_name);
@@ -905,21 +919,16 @@  static int bcm2835_spi_setup(struct spi_device *spi)
 		return 0;
 
 	/* and calculate the real CS */
-	spi->cs_gpio = chip->base + 8 - spi->chip_select;
+	flags = (spi->mode & SPI_CS_HIGH) ? GPIOD_OUT_LOW : GPIOD_OUT_HIGH;
+	spi->cs_gpiod = gpiochip_request_own_desc(chip, 8 + spi->chip_select,
+						  DRV_NAME,
+						  flags);
+	if (IS_ERR(spi->cs_gpiod))
+		return PTR_ERR(spi->cs_gpiod);
 
 	/* and set up the "mode" and level */
-	dev_info(&spi->dev, "setting up native-CS%i as GPIO %i\n",
-		 spi->chip_select, spi->cs_gpio);
-
-	/* set up GPIO as output and pull to the correct level */
-	err = gpio_direction_output(spi->cs_gpio,
-				    (spi->mode & SPI_CS_HIGH) ? 0 : 1);
-	if (err) {
-		dev_err(&spi->dev,
-			"could not set CS%i gpio %i as output: %i",
-			spi->chip_select, spi->cs_gpio, err);
-		return err;
-	}
+	dev_info(&spi->dev, "FIXME: setting up native-CS%i as GPIO\n",
+		 spi->chip_select);
 
 	return 0;
 }
@@ -939,6 +948,7 @@  static int bcm2835_spi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, master);
 
+	master->use_gpio_descriptors = true;
 	master->mode_bits = BCM2835_SPI_MODE_BITS;
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->num_chipselect = 3;