Re: [PATCH 5/7] rtc: m41t93: fix device connection/detection logic during probe

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

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.

> +	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