spi: fix client driver breakages when using GPIO descriptors

Commit f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs")
introduced the optional use of GPIO descriptors for chip selects.

A side-effect of this change: when a SPI bus uses GPIO descriptors,
all its client devices have SPI_CS_HIGH set in spi->mode. This flag is
required for the SPI bus to operate correctly.

This unfortunately breaks many client drivers, which use the following
pattern to configure their underlying SPI bus:

static int client_device_probe(struct spi_device *spi)
{
	...
	spi->mode = SPI_MODE_0;
	spi->bits_per_word = 8;
	err = spi_setup(spi);
	..
}

In short, many client drivers overwrite the SPI_CS_HIGH bit in
spi->mode, and break the underlying SPI bus driver.

This is especially true for Freescale/NXP imx ecspi, where large
numbers of spi client drivers now no longer work.

Proposed fix:
-------------
When using gpio descriptors, depend on gpiolib to handle CS polarity.
Existing quirks in gpiolib ensure that this is handled correctly.

Existing gpiolib behaviour will force the polarity of any chip-select
gpiod to active-high (if 'spi-active-high' devicetree prop present) or
active-low (if 'spi-active-high' absent). Irrespective of whether
the gpio is marked GPIO_ACTIVE_[HIGH|LOW] in the devicetree.

Loose ends:
-----------
If this fix is applied:
- is commit 138c9c32f090
  ("spi: spidev: Fix CS polarity if GPIO descriptors are used")
  still necessary / correct ?

Fixes: f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs")
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20201106150706.29089-1-TheSven73@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 0cab239d8e7fc00983084107c7a74f5f0367af74..7566482c052c8f0ddea9e7115d6504171b51b0aa 100644 (file)
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -812,18 +812,16 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
                enable = !enable;
 
        if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio)) {
-               /*
-                * Honour the SPI_NO_CS flag and invert the enable line, as
-                * active low is default for SPI. Execution paths that handle
-                * polarity inversion in gpiolib (such as device tree) will
-                * enforce active high using the SPI_CS_HIGH resulting in a
-                * double inversion through the code above.
-                */
                if (!(spi->mode & SPI_NO_CS)) {
                        if (spi->cs_gpiod)
+                               /* polarity handled by gpiolib */
                                gpiod_set_value_cansleep(spi->cs_gpiod,
-                                                        !enable);
+                                                        enable1);
                        else
+                               /*
+                                * invert the enable line, as active low is
+                                * default for SPI.
+                                */
                                gpio_set_value_cansleep(spi->cs_gpio, !enable);
                }
                /* Some SPI masters need both GPIO CS & slave_select */
@@ -1992,15 +1990,6 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
        }
        spi->chip_select = value;
 
-       /*
-        * For descriptors associated with the device, polarity inversion is
-        * handled in the gpiolib, so all gpio chip selects are "active high"
-        * in the logical sense, the gpiolib will invert the line if need be.
-        */
-       if ((ctlr->use_gpio_descriptors) && ctlr->cs_gpiods &&
-           ctlr->cs_gpiods[spi->chip_select])
-               spi->mode |= SPI_CS_HIGH;
-
        /* Device speed */
        if (!of_property_read_u32(nc, "spi-max-frequency", &value))
                spi->max_speed_hz = value;