linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "gpio/spi: Fix spi-gpio regression on active high CS"
@ 2019-07-15 20:45 Linus Walleij
       [not found] ` <20190716012712.BC9F22173C@mail.kernel.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2019-07-15 20:45 UTC (permalink / raw)
  To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij, linux-spi, stable

This reverts commit fbbf145a0e0a0177e089c52275fbfa55763e7d1d.

It seems I was misguided in my fixup, which was working at the
time but did not work on the final v5.2.

The patch tried to avoid a quirk the gpiolib code not to treat
"spi-gpio" CS gpios "special" by enforcing them to be active
low, in the belief that since the "spi-gpio" driver was
parsing the device tree on its own, it did not care to inspect
the "spi-cs-high" attribute on the device nodes.

That's wrong. The SPI core was inspecting them inside the
of_spi_parse_dt() funtion and setting SPI_CS_HIGH on the
nodes, and the driver inspected this flag when driving the
line.

As of now, the core handles the GPIO and it will consistently
set the GPIO descriptor to 1 to enable CS, strictly requireing
the gpiolib to invert it. And the gpiolib should indeed
enforce active low on the CS line.

Device trees should of course put the right flag on the GPIO
handles, but it used to not matter. If we don't enforce active
low on "gpio-gpio" we may run into ABI backward compatibility
issues, so revert this.

Cc: linux-spi@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I am sorry that this at one point fixed a problem for me, it
doesn't anymore and I don't know why it ever did. :(
---
 drivers/gpio/gpiolib-of.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index f974075ff00e..a8f02f551d6b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -118,15 +118,8 @@ static void of_gpio_flags_quirks(struct device_node *np,
 	 * Legacy handling of SPI active high chip select. If we have a
 	 * property named "cs-gpios" we need to inspect the child node
 	 * to determine if the flags should have inverted semantics.
-	 *
-	 * This does not apply to an SPI device named "spi-gpio", because
-	 * these have traditionally obtained their own GPIOs by parsing
-	 * the device tree directly and did not respect any "spi-cs-high"
-	 * property on the SPI bus children.
 	 */
-	if (IS_ENABLED(CONFIG_SPI_MASTER) &&
-	    !strcmp(propname, "cs-gpios") &&
-	    !of_device_is_compatible(np, "spi-gpio") &&
+	if (IS_ENABLED(CONFIG_SPI_MASTER) && !strcmp(propname, "cs-gpios") &&
 	    of_property_read_bool(np, "cs-gpios")) {
 		struct device_node *child;
 		u32 cs;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Revert "gpio/spi: Fix spi-gpio regression on active high CS"
       [not found] ` <20190716012712.BC9F22173C@mail.kernel.org>
@ 2019-07-16  8:56   ` Linus Walleij
  0 siblings, 0 replies; 2+ messages in thread
From: Linus Walleij @ 2019-07-16  8:56 UTC (permalink / raw)
  To: Sasha Levin
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-spi, stable

On Tue, Jul 16, 2019 at 3:27 AM Sasha Levin <sashal@kernel.org> wrote:

> v5.2.1: Build OK!
> v5.1.18: Failed to apply! Possible dependencies:
(...)
> How should we proceed with this patch?

Only apply it to 5.2.y once upstream.

Thanks!
Linus Walleij

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-07-16  8:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 20:45 [PATCH] Revert "gpio/spi: Fix spi-gpio regression on active high CS" Linus Walleij
     [not found] ` <20190716012712.BC9F22173C@mail.kernel.org>
2019-07-16  8:56   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).