Unverified Commit 766c6b63 authored by Sven Van Asbroeck's avatar Sven Van Asbroeck Committed by Mark Brown
Browse files

spi: fix client driver breakages when using GPIO descriptors

Commit f3186dd8 ("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 138c9c32
  ("spi: spidev: Fix CS polarity if GPIO descriptors are used")
  still necessary / correct ?

Fixes: f3186dd8

 ("spi: Optionally use GPIO descriptors for CS GPIOs")
Signed-off-by: Sven Van Asbroeck's avatarSven Van Asbroeck <thesven73@gmail.com>
Acked-by: default avatarLinus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20201106150706.29089-1-TheSven73@gmail.com

Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent bc7f2cd7
......@@ -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;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment