[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025090314472377b79cdf@mail.local>
Date: Wed, 3 Sep 2025 16:47:23 +0200
From: Alexandre Belloni <alexandre.belloni@...tlin.com>
To: Akhilesh Patil <akhilesh@...iitb.ac.in>
Cc: krzk+dt@...nel.org, robh@...nel.org, conor+dt@...nel.org,
skhan@...uxfoundation.org, linux-rtc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
akhileshpatilvnit@...il.com
Subject: Re: [PATCH 5/7] rtc: m41t93: fix device connection/detection logic
during probe
On 03/09/2025 19:57:21+0530, Akhilesh Patil wrote:
> Fix the incorrect assumption about WDAY register (0x4) bits 3 to 7
> being 0 after initial power-on to test response from device during probe
>
> Do not expect these bits to be 0 after power on as datasheet does not
> explicitly mention these power on defaults but recommends software to
> clear these bits during operation. Refer section 3.15 for initial
> power-on default bits.
>
> Fix the random probe failures after power on by removing this condition
> check. Add alternate response check logic which performs write, read,
> compare check on device SRAM register to check device connection.
>
> Signed-off-by: Akhilesh Patil <akhilesh@...iitb.ac.in>
> ---
> drivers/rtc/rtc-m41t93.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/rtc/rtc-m41t93.c b/drivers/rtc/rtc-m41t93.c
> index 8cc179e08a4a..902797070246 100644
> --- a/drivers/rtc/rtc-m41t93.c
> +++ b/drivers/rtc/rtc-m41t93.c
> @@ -30,6 +30,7 @@
> #define M41T93_BIT_A1IE BIT(7)
> #define M41T93_BIT_ABE BIT(5)
> #define M41T93_FLAG_AF1 BIT(6)
> +#define M41T93_SRAM_BASE 0x19
>
>
> #define M41T93_REG_ALM_HOUR_HT 0xc
> @@ -290,17 +291,25 @@ static int m41t93_probe(struct spi_device *spi)
> return PTR_ERR(m41t93->regmap);
> }
>
> - ret = regmap_read(m41t93->regmap, M41T93_REG_WDAY, &res);
> - if (ret < 0) {
> + ret = regmap_write(m41t93->regmap, M41T93_SRAM_BASE, 0xA5);
Nope, probe is not called at RTC power on but when linux starts. The
whole point of the RTC is to survive Linux. Writing to this register is
breaking functionnality.
> + if (ret) {
> dev_err(&spi->dev, "IO error\n");
> return -EIO;
> }
>
> - if (res < 0 || (res & 0xf8) != 0) {
> - dev_err(&spi->dev, "not found 0x%x.\n", res);
> + ret = regmap_read(m41t93->regmap, M41T93_SRAM_BASE, &res);
> + if (ret) {
> + dev_err(&spi->dev, "IO error\n");
> + return -EIO;
> + }
> +
> + if (res != 0xA5) {
> + dev_err(&spi->dev, "No valid response from device 0x%x.\n", res);
> return -ENODEV;
> }
>
> + dev_notice(&spi->dev, "m41t93 device response success\n");
> +
This is too verbose.
> spi_set_drvdata(spi, m41t93);
>
> m41t93->rtc = devm_rtc_device_register(&spi->dev, m41t93_driver.driver.name,
> --
> 2.34.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists