Re: [PATCH] spi: spidev: Fix CS polarity if GPIO descriptors are used

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Feb 18, 2020 at 1:08 PM Lukas Wunner <lukas@xxxxxxxxx> wrote:

> Commit f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs")
> amended of_spi_parse_dt() to always set SPI_CS_HIGH for SPI slaves whose
> Chip Select is defined by a "cs-gpios" devicetree property.
>
> This change broke userspace applications which issue an SPI_IOC_WR_MODE
> ioctl() to an spidev:  Chip Select polarity will be incorrect unless the
> application is changed to set SPI_CS_HIGH.  And once changed, it will be
> incompatible with kernels not containing the commit.
>
> Fix by setting SPI_CS_HIGH in spidev_ioctl() (under the same conditions
> as in of_spi_parse_dt()).
>
> Fixes: f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs")
> Reported-by: Simon Han <z.han@xxxxxxxxxx>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v5.1+
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
(...)

Nit: I would also insert a comment in the code to tell what is going on.

> +                       if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
> +                           ctlr->cs_gpiods[spi->chip_select])
> +                               tmp |= SPI_CS_HIGH;

Should this be tmp ^= SPI_CS_HIGH?

If the device tree node for cs-gpios is actually active high, which
happens, then you probably want the opposite of what was
requested, right?

Yours,
Linus Walleij