* [PATCH] rtc: ds1343: Do not hardcode SPI mode flags
@ 2019-12-04 22:57 Linus Walleij
2019-12-10 16:49 ` Alexandre Belloni
0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2019-12-04 22:57 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni; +Cc: linux-rtc, Linus Walleij, Mark Brown
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>
---
drivers/rtc/rtc-ds1343.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-ds1343.c b/drivers/rtc/rtc-ds1343.c
index fa6de31d5793..105a2fd93583 100644
--- a/drivers/rtc/rtc-ds1343.c
+++ b/drivers/rtc/rtc-ds1343.c
@@ -484,9 +484,12 @@ static int ds1343_probe(struct spi_device *spi)
mutex_init(&priv->mutex);
/* 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)
--
2.23.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] rtc: ds1343: Do not hardcode SPI mode flags
2019-12-04 22:57 [PATCH] rtc: ds1343: Do not hardcode SPI mode flags Linus Walleij
@ 2019-12-10 16:49 ` Alexandre Belloni
0 siblings, 0 replies; 2+ messages in thread
From: Alexandre Belloni @ 2019-12-10 16:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: Alessandro Zummo, linux-rtc, Mark Brown
On 04/12/2019 23:57:31+0100, Linus Walleij wrote:
> 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>
> ---
> drivers/rtc/rtc-ds1343.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-10 16:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 22:57 [PATCH] rtc: ds1343: Do not hardcode SPI mode flags Linus Walleij
2019-12-10 16:49 ` Alexandre Belloni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).