[PATCH] spi: stm32_spi: Fix GPIO chipselect polarity handling

[Marek Vasut]

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 at denx.de>
>> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> Cc: Patrice Chotard <patrice.chotard at 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.