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@ee.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);
+ 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");
+
spi_set_drvdata(spi, m41t93);
m41t93->rtc = devm_rtc_device_register(&spi->dev, m41t93_driver.driver.name,
--
2.34.1On 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@ee.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
On Wed, Sep 03, 2025 at 04:47:23PM +0200, Alexandre Belloni wrote:
> 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@ee.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.
Okay.
I did not get the intent of original author to read WDAY register
and check bits 3 to 7 to be 0 (as per datasheet, these are reserved bits
and it does not explicitly say these are 0). I assume it is checking if
device is connected, but IMO this check can be skipped altogether.
Anaways, I would like to take this seperately and focus this series only
on features, hence will skip this patch in v2.
>
> > + 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);
motivation for this change was - I was hitting this with res = 0x77
ocassionally after soft reboot on my setup (am62x SK + m41t93 chip on spi0).
I will confirm if it is any setup issue before taking up this code fix
seperately.
> > + 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.
Sure. will remove this.
>
> > 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
Thanks for the review,
Regards,
Akhilesh
© 2016 - 2025 Red Hat, Inc.