Bug 208059 - SPI chipselect gets wrong polarity with spi-gpio
Summary: SPI chipselect gets wrong polarity with spi-gpio
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-04 18:19 UTC by kernel
Modified: 2020-06-04 18:19 UTC (History)
1 user (show)

See Also:
Kernel Version: 5.4.44
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments

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

Note You need to log in before you can comment on or make changes to this bug.