Bug 208059

Summary: SPI chipselect gets wrong polarity with spi-gpio
Product: Drivers Reporter: kernel
Component: OtherAssignee: drivers_other
Status: NEW ---    
Severity: normal CC: triad
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.4.44 Subsystem:
Regression: Yes Bisected commit-id:

Description kernel 2020-06-04 18:19:29 UTC
I've some boards with a ks8995 switch attached to spi-gpio.  As usual, the chip select line has active low polarity.  After updating my kernel from 5.1 to 5.4 LTS, the CS line is inverted (i.e. behaves as though it were configured active high).

Looks like this regression is due to commit f3186dd876697e696d07136623d5cf0a6fb0bc0f. [1]

0. device is registered via dt (of_register_spi_device())

1. of_spi_parse_dt() forces the SPI_CS_HIGH flag on spi->mode (because ctlr->use_gpio_descriptors).

2. when CS_HIGH is set, spi_set_cs() treats its argument as a logical value, and lets gpiolib invert it if the pin is really configured active low

3. unfortunately, ks8995_probe() overrides spi->mode and the CS_HIGH flag is lost.  Now spi_set_cs() treats its argument as an active low signal but passes it to gpiod_set_value() as usual, and gpiolib will invert the value...

I'm not sure what's the intended design here.

Maybe spi peripheral drivers shouldn't override mode (but grepping around shows that it is a very common thing to do).  Maybe the logic that forces SPI_CS_HIGH should move from of_spi_parse_dt() to spi_setup() so the flag will be set again when the peripheral driver calls spi_setup().  Maybe this particular application of the flag should be changed to use an internal variable (since it's not a property or configuration of the spi peripheral device but an implementation detail of the spi device layer?).  Maybe spi_set_cs() should not rely on the flag and just unconditionally convert the argument to logical value if it's going to pass it to gpiolib?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/spi/spi.c?id=f3186dd876697e696d07136623d5cf0a6fb0bc0f