Hello,
On 10/07/2024 18:52:07+0100, Ian Abbott wrote:
> Commit 3b52093dc917 ("rtc: ds1343: Do not hardcode SPI mode flags")
> bit-flips (^=) the existing SPI_CS_HIGH setting in the SPI mode during
> device probe. This will set it to the wrong value if the spi-cs-high
> property has been set in the devicetree node. Just force it to be set
> active high and get rid of some commentary that attempted to explain why
> flipping the bit was the correct choice.
>
> Fixes: 3b52093dc917 ("rtc: ds1343: Do not hardcode SPI mode flags")
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.6+
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> drivers/rtc/rtc-ds1343.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds1343.c b/drivers/rtc/rtc-ds1343.c
> index ed5a6ba89a3e..484b5756b55c 100644
> --- a/drivers/rtc/rtc-ds1343.c
> +++ b/drivers/rtc/rtc-ds1343.c
> @@ -361,13 +361,10 @@ static int ds1343_probe(struct spi_device *spi)
> if (!priv)
> return -ENOMEM;
>
> - /* RTC DS1347 works in spi mode 3 and
> - * 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.
> + /*
> + * RTC DS1347 works in spi mode 3 and its chip select is active high.
> */
> - spi->mode |= SPI_MODE_3;
> - spi->mode ^= SPI_CS_HIGH;
> + spi->mode |= SPI_MODE_3 | SPI_CS_HIGH;
Linus being the gpio maintainer and Mark being the SPI maintainer, I'm
pretty sure this was correct at the time.
Are you sure you are not missing an active high/low flag on a gpio
definition?
> spi->bits_per_word = 8;
> res = spi_setup(spi);
> if (res)
> --
> 2.43.0
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
