rtc: ds1343: Do not hardcode SPI mode flags
authorLinus Walleij <linus.walleij@linaro.org>
Wed, 4 Dec 2019 22:57:31 +0000 (23:57 +0100)
committerAlexandre Belloni <alexandre.belloni@bootlin.com>
Tue, 10 Dec 2019 16:47:16 +0000 (17:47 +0100)
The current use of mode flags to us SPI_MODE_3 and
SPI_CS_HIGH is fragile: it overwrites anything already
assigned by the SPI core. Change it thusly:

- Just |= the SPI_MODE_3 so we keep other flags
- Assign ^= SPI_CS_HIGH since we might be active high
  already, and that is usually the case with GPIOs used
  for chip select, even if they are in practice active low.

Add a comment clarifying why ^= SPI_CS_HIGH is the right
choice here.

Reported-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20191204225731.20306-1-linus.walleij@linaro.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
drivers/rtc/rtc-ds1343.c

index d21004a68ee08df0c3a5becde89c7e08139d5183..530e64442b920ad3832ca36313b9bf48033fc163 100644 (file)
@@ -365,9 +365,12 @@ static int ds1343_probe(struct spi_device *spi)
        priv->spi = spi;
 
        /* RTC DS1347 works in spi mode 3 and
-        * its chip select is active high
+        * its chip select is active high. Active high should be defined as
+        * "inverse polarity" as GPIO-based chip selects can be logically
+        * active high but inverted by the GPIO library.
         */
-       spi->mode = SPI_MODE_3 | SPI_CS_HIGH;
+       spi->mode |= SPI_MODE_3;
+       spi->mode ^= SPI_CS_HIGH;
        spi->bits_per_word = 8;
        res = spi_setup(spi);
        if (res)