All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Mark Brown <broonie@kernel.org>, linux-spi@vger.kernel.org
Cc: linux-gpio@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Andrey Smirnov <andrew.smirnov@gmail.com>
Subject: [PATCH] spi: gpio: Use core CS GPIO handling for everything
Date: Tue, 16 Jul 2019 23:24:13 +0200	[thread overview]
Message-ID: <20190716212413.30974-1-linus.walleij@linaro.org> (raw)

The SPI core can handle all CS GPIOs without any problem.
As can be seen from the code in spi_get_gpio_descs()
the core will just get a bunch of chip selects from
the device.

When using GPIO descriptors from a board file, the
process is not any different.

However we need to be carefule about one thing:
devm_gpiod_get_index_optional() passes GPIOD_OUT_LOW
in the core and GPIOD_OUT_HIGH in the spi-gpio driver.
This is because the driver assumes the polarity
inversion is not handled by the core.

We need to revisit commit 9b00bc7b901f
("spi: spi-gpio: Rewrite to use GPIO descriptors")
and make sure all passed descriptors are flagged as
active low for the core, because now they are passed
in as active high and inverted by the driver instead.
The device tree core will enforce active low on
CS gpios, but board files are not that forgiving.

Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-pxa/cm-x300.c            |  2 +-
 arch/arm/mach-s3c24xx/mach-jive.c      |  4 +-
 arch/arm/mach-s3c24xx/mach-qt2410.c    |  2 +-
 arch/arm/mach-s3c64xx/mach-smartq.c    |  2 +-
 arch/mips/alchemy/devboards/db1000.c   |  2 +-
 arch/mips/jz4740/board-qi_lb60.c       |  2 +-
 drivers/misc/eeprom/digsy_mtc_eeprom.c |  2 +-
 drivers/spi/spi-gpio.c                 | 59 ++------------------------
 8 files changed, 12 insertions(+), 63 deletions(-)

diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
index 425855f456f2..60ebc5f4041e 100644
--- a/arch/arm/mach-pxa/cm-x300.c
+++ b/arch/arm/mach-pxa/cm-x300.c
@@ -362,7 +362,7 @@ static struct gpiod_lookup_table cm_x300_spi_gpiod_table = {
 		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_DOUT,
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("gpio-pxa", GPIO_LCD_CS,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/arch/arm/mach-s3c24xx/mach-jive.c b/arch/arm/mach-s3c24xx/mach-jive.c
index 885e8f12e4b9..66c38472111d 100644
--- a/arch/arm/mach-s3c24xx/mach-jive.c
+++ b/arch/arm/mach-s3c24xx/mach-jive.c
@@ -406,7 +406,7 @@ static struct gpiod_lookup_table jive_lcdspi_gpiod_table = {
 		GPIO_LOOKUP("GPIOB", 8,
 			    "mosi", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("GPIOB", 7,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
@@ -431,7 +431,7 @@ static struct gpiod_lookup_table jive_wm8750_gpiod_table = {
 		GPIO_LOOKUP("GPIOB", 9,
 			    "mosi", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("GPIOH", 10,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/arch/arm/mach-s3c24xx/mach-qt2410.c b/arch/arm/mach-s3c24xx/mach-qt2410.c
index 5d48e5b6e738..61fbd3694257 100644
--- a/arch/arm/mach-s3c24xx/mach-qt2410.c
+++ b/arch/arm/mach-s3c24xx/mach-qt2410.c
@@ -214,7 +214,7 @@ static struct gpiod_lookup_table qt2410_spi_gpiod_table = {
 		GPIO_LOOKUP("GPIOG", 5,
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("GPIOB", 5,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c b/arch/arm/mach-s3c64xx/mach-smartq.c
index 951208f168e7..f81a85c75247 100644
--- a/arch/arm/mach-s3c64xx/mach-smartq.c
+++ b/arch/arm/mach-s3c64xx/mach-smartq.c
@@ -225,7 +225,7 @@ static struct gpiod_lookup_table smartq_lcd_control_gpiod_table = {
 		GPIO_LOOKUP("GPIOM", 3,
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("GPIOM", 0,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/arch/mips/alchemy/devboards/db1000.c b/arch/mips/alchemy/devboards/db1000.c
index 2c52ee27b4f2..aa3b5b3d81aa 100644
--- a/arch/mips/alchemy/devboards/db1000.c
+++ b/arch/mips/alchemy/devboards/db1000.c
@@ -424,7 +424,7 @@ static struct gpiod_lookup_table db1100_spi_gpiod_table = {
 		GPIO_LOOKUP("alchemy-gpio2", 7,
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("alchemy-gpio2", 10,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index 071e9d94eea7..9ea53398c0f1 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -328,7 +328,7 @@ static struct gpiod_lookup_table qi_lb60_spigpio_gpio_table = {
 		GPIO_LOOKUP("GPIOC", 22,
 			    "mosi", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("GPIOC", 21,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/drivers/misc/eeprom/digsy_mtc_eeprom.c b/drivers/misc/eeprom/digsy_mtc_eeprom.c
index f1f766b70965..c058eaff0385 100644
--- a/drivers/misc/eeprom/digsy_mtc_eeprom.c
+++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c
@@ -69,7 +69,7 @@ static struct gpiod_lookup_table eeprom_spi_gpiod_table = {
 		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_DO,
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_CS,
-			    "cs", GPIO_ACTIVE_HIGH),
+			    "cs", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 9eb82150666e..4309832221b1 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -35,7 +35,6 @@ struct spi_gpio {
 	struct gpio_desc		*sck;
 	struct gpio_desc		*miso;
 	struct gpio_desc		*mosi;
-	struct gpio_desc		**cs_gpios;
 };
 
 /*----------------------------------------------------------------------*/
@@ -203,37 +202,6 @@ static void spi_gpio_chipselect(struct spi_device *spi, int is_active)
 	/* set initial clock line level */
 	if (is_active)
 		gpiod_set_value_cansleep(spi_gpio->sck, spi->mode & SPI_CPOL);
-
-	/* Drive chip select line, if we have one */
-	if (spi_gpio->cs_gpios) {
-		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)
-{
-	struct gpio_desc	*cs;
-	int			status = 0;
-	struct spi_gpio		*spi_gpio = spi_to_spi_gpio(spi);
-
-	/*
-	 * The CS GPIOs have already been
-	 * initialized from the descriptor lookup.
-	 */
-	if (spi_gpio->cs_gpios) {
-		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);
-
-	return status;
 }
 
 static int spi_gpio_set_direction(struct spi_device *spi, bool output)
@@ -264,11 +232,6 @@ static int spi_gpio_set_direction(struct spi_device *spi, bool output)
 	return 0;
 }
 
-static void spi_gpio_cleanup(struct spi_device *spi)
-{
-	spi_bitbang_cleanup(spi);
-}
-
 /*
  * 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
@@ -324,8 +287,6 @@ static int spi_gpio_probe_pdata(struct platform_device *pdev,
 {
 	struct device *dev = &pdev->dev;
 	struct spi_gpio_platform_data *pdata = dev_get_platdata(dev);
-	struct spi_gpio *spi_gpio = spi_master_get_devdata(master);
-	int i;
 
 #ifdef GENERIC_BITBANG
 	if (!pdata || !pdata->num_chipselect)
@@ -335,20 +296,8 @@ static int spi_gpio_probe_pdata(struct platform_device *pdev,
 	 * The master needs to think there is a chipselect even if not
 	 * connected
 	 */
-	master->num_chipselect = pdata->num_chipselect ?: 1;
-
-	spi_gpio->cs_gpios = devm_kcalloc(dev, master->num_chipselect,
-					  sizeof(*spi_gpio->cs_gpios),
-					  GFP_KERNEL);
-	if (!spi_gpio->cs_gpios)
-		return -ENOMEM;
-
-	for (i = 0; i < master->num_chipselect; 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]);
-	}
+	master->num_chipselect = pdata->num_chipselect ? : 1;
+	master->use_gpio_descriptors = true;
 
 	return 0;
 }
@@ -405,8 +354,8 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	}
 
 	master->bus_num = pdev->id;
-	master->setup = spi_gpio_setup;
-	master->cleanup = spi_gpio_cleanup;
+	master->setup = spi_bitbang_setup;
+	master->cleanup = spi_bitbang_cleanup;
 
 	bb = &spi_gpio->bitbang;
 	bb->master = master;
-- 
2.21.0


                 reply	other threads:[~2019-07-16 21:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190716212413.30974-1-linus.walleij@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=andrew.smirnov@gmail.com \
    --cc=broonie@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.