[PATCH] spi: stm32_spi: Fix GPIO chipselect polarity handling
The GPIO chipselect signal polarity is handled by the GPIO core code, the driver code is only responsible for asserting and deasserting the GPIO. Do not invert the GPIO polarity in the driver code.
For example, In case CS GPIO is active low, then the DT must contain GPIO_ACTIVE_LOW flag and the GPIO core code would set the GPIO accordingly.
Signed-off-by: Marek Vasut marex@denx.de Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Patrice Chotard patrice.chotard@foss.st.com --- drivers/spi/stm32_spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c index fe5419e8518..0cb24546ca9 100644 --- a/drivers/spi/stm32_spi.c +++ b/drivers/spi/stm32_spi.c @@ -434,7 +434,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen,
slave_plat = dev_get_parent_plat(slave); if (flags & SPI_XFER_BEGIN) - stm32_spi_set_cs(bus, slave_plat->cs, false); + stm32_spi_set_cs(bus, slave_plat->cs, true);
/* Be sure to have data in fifo before starting data transfer */ if (priv->tx_buf) @@ -485,7 +485,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen, stm32_spi_stopxfer(bus);
if (flags & SPI_XFER_END) - stm32_spi_set_cs(bus, slave_plat->cs, true); + stm32_spi_set_cs(bus, slave_plat->cs, false);
return xfer_status; }
Hi Marek
On 8/23/22 19:07, Marek Vasut wrote:
The GPIO chipselect signal polarity is handled by the GPIO core code, the driver code is only responsible for asserting and deasserting the GPIO. Do not invert the GPIO polarity in the driver code.
For example, In case CS GPIO is active low, then the DT must contain GPIO_ACTIVE_LOW flag and the GPIO core code would set the GPIO accordingly.
Signed-off-by: Marek Vasut marex@denx.de Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Patrice Chotard patrice.chotard@foss.st.com
drivers/spi/stm32_spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c index fe5419e8518..0cb24546ca9 100644 --- a/drivers/spi/stm32_spi.c +++ b/drivers/spi/stm32_spi.c @@ -434,7 +434,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen,
slave_plat = dev_get_parent_plat(slave); if (flags & SPI_XFER_BEGIN)
stm32_spi_set_cs(bus, slave_plat->cs, false);
stm32_spi_set_cs(bus, slave_plat->cs, true);/* Be sure to have data in fifo before starting data transfer */ if (priv->tx_buf)
@@ -485,7 +485,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen, stm32_spi_stopxfer(bus);
if (flags & SPI_XFER_END)
stm32_spi_set_cs(bus, slave_plat->cs, true);
stm32_spi_set_cs(bus, slave_plat->cs, false);return xfer_status;
}
It's a bit confusing as there is also the spi property "spi-cs-high" which also invert the polarity in stm32_spi_set_cs() :
if (priv->cs_high) enable = !enable;
return dm_gpio_set_value(&plat->cs_gpios[cs], enable ? 1 : 0);
It's seems that chip select polarity can be managed at two different places.
Patrice
On 8/24/22 16:04, Patrice CHOTARD wrote:
Hi Marek
Hi,
On 8/23/22 19:07, Marek Vasut wrote:
The GPIO chipselect signal polarity is handled by the GPIO core code, the driver code is only responsible for asserting and deasserting the GPIO. Do not invert the GPIO polarity in the driver code.
For example, In case CS GPIO is active low, then the DT must contain GPIO_ACTIVE_LOW flag and the GPIO core code would set the GPIO accordingly.
Signed-off-by: Marek Vasut marex@denx.de Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Patrice Chotard patrice.chotard@foss.st.com
drivers/spi/stm32_spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/stm32_spi.c b/drivers/spi/stm32_spi.c index fe5419e8518..0cb24546ca9 100644 --- a/drivers/spi/stm32_spi.c +++ b/drivers/spi/stm32_spi.c @@ -434,7 +434,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen,
slave_plat = dev_get_parent_plat(slave); if (flags & SPI_XFER_BEGIN)
stm32_spi_set_cs(bus, slave_plat->cs, false);
stm32_spi_set_cs(bus, slave_plat->cs, true);/* Be sure to have data in fifo before starting data transfer */ if (priv->tx_buf)
@@ -485,7 +485,7 @@ static int stm32_spi_xfer(struct udevice *slave, unsigned int bitlen, stm32_spi_stopxfer(bus);
if (flags & SPI_XFER_END)
stm32_spi_set_cs(bus, slave_plat->cs, true);
stm32_spi_set_cs(bus, slave_plat->cs, false);return xfer_status; }
It's a bit confusing as there is also the spi property "spi-cs-high" which also invert the polarity in stm32_spi_set_cs() :
if (priv->cs_high) enable = !enable;
return dm_gpio_set_value(&plat->cs_gpios[cs], enable ? 1 : 0);
It's seems that chip select polarity can be managed at two different places.
Urgh, all right, drop this patch for now. This clearly needs closer look.
Also, as a bonus, 'sspi' command allows us to override the bus flags, so that's place number 3 where things can go confusing.
participants (2)
-
Marek Vasut -
Patrice CHOTARD