* [PATCH] rtc-m41t62: kickstart ocillator upon failure @ 2025-01-16 6:26 nmydeen 2025-02-04 11:54 ` Corey Minyard 2025-04-01 9:04 ` Alexandre Belloni 0 siblings, 2 replies; 7+ messages in thread From: nmydeen @ 2025-01-16 6:26 UTC (permalink / raw) To: alexandre.belloni Cc: linux-rtc, linux-kernel, cminyard, A. Niyas Ahamed Mydeen From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> The ocillator on the m41t62 (and other chips of this type) needs a kickstart upon a failure; the RTC read routine will notice the oscillator failure and fail reads. This is added in the RTC write routine; this allows the system to know that the time in the RTC is accurate. This is following the procedure described in section 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> Reviewed-by: Corey Minyard <cminyard@mvista.com> --- drivers/rtc/rtc-m41t80.c | 70 ++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c index 1f58ae8b151e..77c21c91bae3 100644 --- a/drivers/rtc/rtc-m41t80.c +++ b/drivers/rtc/rtc-m41t80.c @@ -22,6 +22,7 @@ #include <linux/slab.h> #include <linux/mutex.h> #include <linux/string.h> +#include <linux/delay.h> #ifdef CONFIG_RTC_DRV_M41T80_WDT #include <linux/fs.h> #include <linux/ioctl.h> @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return flags; if (flags & M41T80_FLAGS_OF) { - dev_err(&client->dev, "Oscillator failure, data is invalid.\n"); + dev_err(&client->dev, "Oscillator failure, time may not be accurate, write time to RTC to fix it.\n"); return -EINVAL; } @@ -227,21 +228,60 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return 0; } -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *in_tm) { struct i2c_client *client = to_i2c_client(dev); struct m41t80_data *clientdata = i2c_get_clientdata(client); + struct rtc_time tm = *in_tm; unsigned char buf[8]; int err, flags; + time64_t time = 0; + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) + return flags; + if (flags & M41T80_FLAGS_OF) { + /* OF cannot be immediately reset: oscillator has to be restarted. */ + dev_warn(&client->dev, "OF bit is still set, kickstarting clock.\n"); + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't set ST bit\n"); + return err; + } + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, + flags & ~M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't clear ST bit\n"); + return err; + } + /* oscillator must run for 4sec before we attempt to reset OF bit */ + msleep(4000); + /* Clear the OF bit of Flags Register */ + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, + flags & ~M41T80_FLAGS_OF); + if (err < 0) { + dev_err(&client->dev, "Unable to write flags register\n"); + return err; + } + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) + return flags; + else if (flags & M41T80_FLAGS_OF) { + dev_err(&client->dev, "Can't clear the OF bit check battery\n"); + return err; + } + /* add 4sec of oscillator stablize time otherwise we are behind 4sec */ + time = rtc_tm_to_time64(&tm); + rtc_time64_to_tm(time+4, &tm); + } buf[M41T80_REG_SSEC] = 0; - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); - buf[M41T80_REG_WDAY] = tm->tm_wday; + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); + buf[M41T80_REG_WDAY] = tm.tm_wday; /* If the square wave output is controlled in the weekday register */ if (clientdata->features & M41T80_FEATURE_SQ_ALT) { @@ -261,18 +301,6 @@ static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) return err; } - /* Clear the OF bit of Flags Register */ - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); - if (flags < 0) - return flags; - - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, - flags & ~M41T80_FLAGS_OF); - if (err < 0) { - dev_err(&client->dev, "Unable to write flags register\n"); - return err; - } - return err; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] rtc-m41t62: kickstart ocillator upon failure 2025-01-16 6:26 [PATCH] rtc-m41t62: kickstart ocillator upon failure nmydeen @ 2025-02-04 11:54 ` Corey Minyard 2025-04-01 9:04 ` Alexandre Belloni 1 sibling, 0 replies; 7+ messages in thread From: Corey Minyard @ 2025-02-04 11:54 UTC (permalink / raw) To: Alexander Bigga, alexandre.belloni; +Cc: linux-rtc, linux-kernel, nmydeen [-- Attachment #1: Type: text/plain, Size: 5064 bytes --] Adding Alexander On Thu, Jan 16, 2025 at 11:56:41AM +0530, nmydeen@mvista.com wrote: > From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> > > The ocillator on the m41t62 (and other chips of this type) needs > a kickstart upon a failure; the RTC read routine will notice the > oscillator failure and fail reads. This is added in the RTC write > routine; this allows the system to know that the time in the RTC > is accurate. This is following the procedure described in section > 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" Any comments on this? I just saw that Alexander wasn't on the email, not sure if they are still involved. -corey > > Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> > Reviewed-by: Corey Minyard <cminyard@mvista.com> > --- > drivers/rtc/rtc-m41t80.c | 70 ++++++++++++++++++++++++++++------------ > 1 file changed, 49 insertions(+), 21 deletions(-) > > diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c > index 1f58ae8b151e..77c21c91bae3 100644 > --- a/drivers/rtc/rtc-m41t80.c > +++ b/drivers/rtc/rtc-m41t80.c > @@ -22,6 +22,7 @@ > #include <linux/slab.h> > #include <linux/mutex.h> > #include <linux/string.h> > +#include <linux/delay.h> > #ifdef CONFIG_RTC_DRV_M41T80_WDT > #include <linux/fs.h> > #include <linux/ioctl.h> > @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) > return flags; > > if (flags & M41T80_FLAGS_OF) { > - dev_err(&client->dev, "Oscillator failure, data is invalid.\n"); > + dev_err(&client->dev, "Oscillator failure, time may not be accurate, write time to RTC to fix it.\n"); > return -EINVAL; > } > > @@ -227,21 +228,60 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) > return 0; > } > > -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) > +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *in_tm) > { > struct i2c_client *client = to_i2c_client(dev); > struct m41t80_data *clientdata = i2c_get_clientdata(client); > + struct rtc_time tm = *in_tm; > unsigned char buf[8]; > int err, flags; > + time64_t time = 0; > > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > + if (flags < 0) > + return flags; > + if (flags & M41T80_FLAGS_OF) { > + /* OF cannot be immediately reset: oscillator has to be restarted. */ > + dev_warn(&client->dev, "OF bit is still set, kickstarting clock.\n"); > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, M41T80_SEC_ST); > + if (err < 0) { > + dev_err(&client->dev, "Can't set ST bit\n"); > + return err; > + } > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, > + flags & ~M41T80_SEC_ST); > + if (err < 0) { > + dev_err(&client->dev, "Can't clear ST bit\n"); > + return err; > + } > + /* oscillator must run for 4sec before we attempt to reset OF bit */ > + msleep(4000); > + /* Clear the OF bit of Flags Register */ > + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > + flags & ~M41T80_FLAGS_OF); > + if (err < 0) { > + dev_err(&client->dev, "Unable to write flags register\n"); > + return err; > + } > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > + if (flags < 0) > + return flags; > + else if (flags & M41T80_FLAGS_OF) { > + dev_err(&client->dev, "Can't clear the OF bit check battery\n"); > + return err; > + } > + /* add 4sec of oscillator stablize time otherwise we are behind 4sec */ > + time = rtc_tm_to_time64(&tm); > + rtc_time64_to_tm(time+4, &tm); > + } > buf[M41T80_REG_SSEC] = 0; > - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); > - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); > - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); > - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); > - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); > - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); > - buf[M41T80_REG_WDAY] = tm->tm_wday; > + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); > + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); > + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); > + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); > + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); > + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); > + buf[M41T80_REG_WDAY] = tm.tm_wday; > > /* If the square wave output is controlled in the weekday register */ > if (clientdata->features & M41T80_FEATURE_SQ_ALT) { > @@ -261,18 +301,6 @@ static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) > return err; > } > > - /* Clear the OF bit of Flags Register */ > - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > - if (flags < 0) > - return flags; > - > - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > - flags & ~M41T80_FLAGS_OF); > - if (err < 0) { > - dev_err(&client->dev, "Unable to write flags register\n"); > - return err; > - } > - > return err; > } > > -- > 2.34.1 > [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3365 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtc-m41t62: kickstart ocillator upon failure 2025-01-16 6:26 [PATCH] rtc-m41t62: kickstart ocillator upon failure nmydeen 2025-02-04 11:54 ` Corey Minyard @ 2025-04-01 9:04 ` Alexandre Belloni 2025-04-01 19:40 ` Niyas Mydeen 2025-04-02 12:05 ` nmydeen 1 sibling, 2 replies; 7+ messages in thread From: Alexandre Belloni @ 2025-04-01 9:04 UTC (permalink / raw) To: nmydeen; +Cc: linux-rtc, linux-kernel, cminyard Hello, On 16/01/2025 11:56:41+0530, nmydeen@mvista.com wrote: > From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> > > The ocillator on the m41t62 (and other chips of this type) needs > a kickstart upon a failure; the RTC read routine will notice the > oscillator failure and fail reads. This is added in the RTC write > routine; this allows the system to know that the time in the RTC > is accurate. This is following the procedure described in section > 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" > > Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> > Reviewed-by: Corey Minyard <cminyard@mvista.com> > --- > drivers/rtc/rtc-m41t80.c | 70 ++++++++++++++++++++++++++++------------ > 1 file changed, 49 insertions(+), 21 deletions(-) > > diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c > index 1f58ae8b151e..77c21c91bae3 100644 > --- a/drivers/rtc/rtc-m41t80.c > +++ b/drivers/rtc/rtc-m41t80.c > @@ -22,6 +22,7 @@ > #include <linux/slab.h> > #include <linux/mutex.h> > #include <linux/string.h> > +#include <linux/delay.h> > #ifdef CONFIG_RTC_DRV_M41T80_WDT > #include <linux/fs.h> > #include <linux/ioctl.h> > @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) > return flags; > > if (flags & M41T80_FLAGS_OF) { > - dev_err(&client->dev, "Oscillator failure, data is invalid.\n"); > + dev_err(&client->dev, "Oscillator failure, time may not be accurate, write time to RTC to fix it.\n"); > return -EINVAL; > } > > @@ -227,21 +228,60 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) > return 0; > } > > -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) > +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *in_tm) > { > struct i2c_client *client = to_i2c_client(dev); > struct m41t80_data *clientdata = i2c_get_clientdata(client); > + struct rtc_time tm = *in_tm; > unsigned char buf[8]; > int err, flags; > + time64_t time = 0; > > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > + if (flags < 0) > + return flags; > + if (flags & M41T80_FLAGS_OF) { > + /* OF cannot be immediately reset: oscillator has to be restarted. */ > + dev_warn(&client->dev, "OF bit is still set, kickstarting clock.\n"); > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, M41T80_SEC_ST); > + if (err < 0) { > + dev_err(&client->dev, "Can't set ST bit\n"); This is super verbose, please use dev_dbg or drop the dev_errs. The only user action after a failure would be to restart the operation anyway. > + return err; > + } > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, > + flags & ~M41T80_SEC_ST); > + if (err < 0) { > + dev_err(&client->dev, "Can't clear ST bit\n"); > + return err; > + } > + /* oscillator must run for 4sec before we attempt to reset OF bit */ > + msleep(4000); > + /* Clear the OF bit of Flags Register */ > + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > + flags & ~M41T80_FLAGS_OF); checkpatch --strict complains about some style issues, please fix those. > + if (err < 0) { > + dev_err(&client->dev, "Unable to write flags register\n"); > + return err; > + } > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > + if (flags < 0) > + return flags; > + else if (flags & M41T80_FLAGS_OF) { > + dev_err(&client->dev, "Can't clear the OF bit check battery\n"); > + return err; > + } > + /* add 4sec of oscillator stablize time otherwise we are behind 4sec */ > + time = rtc_tm_to_time64(&tm); > + rtc_time64_to_tm(time+4, &tm); > + } The main issue is that now, you have cleared OF so if any read/write to the RTC fails, you would return from the function without having set the time. So when OF is set, you should first add the 4s, then set the time, then kickstart the RTC. > buf[M41T80_REG_SSEC] = 0; > - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); > - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); > - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); > - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); > - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); > - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); > - buf[M41T80_REG_WDAY] = tm->tm_wday; > + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); > + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); > + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); > + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); > + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); > + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); > + buf[M41T80_REG_WDAY] = tm.tm_wday; > > /* If the square wave output is controlled in the weekday register */ > if (clientdata->features & M41T80_FEATURE_SQ_ALT) { > @@ -261,18 +301,6 @@ static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) > return err; > } > > - /* Clear the OF bit of Flags Register */ > - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > - if (flags < 0) > - return flags; > - > - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > - flags & ~M41T80_FLAGS_OF); > - if (err < 0) { > - dev_err(&client->dev, "Unable to write flags register\n"); > - return err; > - } > - > return err; > } > > -- > 2.34.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtc-m41t62: kickstart ocillator upon failure 2025-04-01 9:04 ` Alexandre Belloni @ 2025-04-01 19:40 ` Niyas Mydeen 2025-04-02 12:05 ` nmydeen 1 sibling, 0 replies; 7+ messages in thread From: Niyas Mydeen @ 2025-04-01 19:40 UTC (permalink / raw) To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, cminyard [-- Attachment #1.1: Type: text/plain, Size: 6856 bytes --] Hi Alexandre, Thanks for your comments, I have incorporated all your suggestions and attached here the updated version. Hope it meets your expectations. On Tue, Apr 1, 2025 at 2:34 PM Alexandre Belloni < alexandre.belloni@bootlin.com> wrote: > Hello, > > On 16/01/2025 11:56:41+0530, nmydeen@mvista.com wrote: > > From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> > > > > The ocillator on the m41t62 (and other chips of this type) needs > > a kickstart upon a failure; the RTC read routine will notice the > > oscillator failure and fail reads. This is added in the RTC write > > routine; this allows the system to know that the time in the RTC > > is accurate. This is following the procedure described in section > > 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" > > > > Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> > > Reviewed-by: Corey Minyard <cminyard@mvista.com> > > --- > > drivers/rtc/rtc-m41t80.c | 70 ++++++++++++++++++++++++++++------------ > > 1 file changed, 49 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c > > index 1f58ae8b151e..77c21c91bae3 100644 > > --- a/drivers/rtc/rtc-m41t80.c > > +++ b/drivers/rtc/rtc-m41t80.c > > @@ -22,6 +22,7 @@ > > #include <linux/slab.h> > > #include <linux/mutex.h> > > #include <linux/string.h> > > +#include <linux/delay.h> > > #ifdef CONFIG_RTC_DRV_M41T80_WDT > > #include <linux/fs.h> > > #include <linux/ioctl.h> > > @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, > struct rtc_time *tm) > > return flags; > > > > if (flags & M41T80_FLAGS_OF) { > > - dev_err(&client->dev, "Oscillator failure, data is > invalid.\n"); > > + dev_err(&client->dev, "Oscillator failure, time may not be > accurate, write time to RTC to fix it.\n"); > > return -EINVAL; > > } > > > > @@ -227,21 +228,60 @@ static int m41t80_rtc_read_time(struct device > *dev, struct rtc_time *tm) > > return 0; > > } > > > > -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) > > +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time > *in_tm) > > { > > struct i2c_client *client = to_i2c_client(dev); > > struct m41t80_data *clientdata = i2c_get_clientdata(client); > > + struct rtc_time tm = *in_tm; > > unsigned char buf[8]; > > int err, flags; > > + time64_t time = 0; > > > > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > > + if (flags < 0) > > + return flags; > > + if (flags & M41T80_FLAGS_OF) { > > + /* OF cannot be immediately reset: oscillator has to be > restarted. */ > > + dev_warn(&client->dev, "OF bit is still set, kickstarting > clock.\n"); > > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, > M41T80_SEC_ST); > > + if (err < 0) { > > + dev_err(&client->dev, "Can't set ST bit\n"); > > This is super verbose, please use dev_dbg or drop the dev_errs. The only > user action after a failure would be to restart the operation anyway. > > > + return err; > > + } > > + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, > > + flags & > ~M41T80_SEC_ST); > > + if (err < 0) { > > + dev_err(&client->dev, "Can't clear ST bit\n"); > > + return err; > > + } > > + /* oscillator must run for 4sec before we attempt to reset > OF bit */ > > + msleep(4000); > > + /* Clear the OF bit of Flags Register */ > > + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > > + flags & ~M41T80_FLAGS_OF); > > checkpatch --strict complains about some style issues, please fix those. > > > + if (err < 0) { > > + dev_err(&client->dev, "Unable to write flags > register\n"); > > + return err; > > + } > > + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > > + if (flags < 0) > > + return flags; > > + else if (flags & M41T80_FLAGS_OF) { > > + dev_err(&client->dev, "Can't clear the OF bit > check battery\n"); > > + return err; > > + } > > + /* add 4sec of oscillator stablize time otherwise we are > behind 4sec */ > > + time = rtc_tm_to_time64(&tm); > > + rtc_time64_to_tm(time+4, &tm); > > + } > > The main issue is that now, you have cleared OF so if any read/write to > the RTC fails, you would return from the function without having set the > time. So when OF is set, you should first add the 4s, then set the time, > then kickstart the RTC. > > > buf[M41T80_REG_SSEC] = 0; > > - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); > > - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); > > - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); > > - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); > > - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); > > - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); > > - buf[M41T80_REG_WDAY] = tm->tm_wday; > > + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); > > + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); > > + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); > > + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); > > + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); > > + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); > > + buf[M41T80_REG_WDAY] = tm.tm_wday; > > > > /* If the square wave output is controlled in the weekday register > */ > > if (clientdata->features & M41T80_FEATURE_SQ_ALT) { > > @@ -261,18 +301,6 @@ static int m41t80_rtc_set_time(struct device *dev, > struct rtc_time *tm) > > return err; > > } > > > > - /* Clear the OF bit of Flags Register */ > > - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); > > - if (flags < 0) > > - return flags; > > - > > - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, > > - flags & ~M41T80_FLAGS_OF); > > - if (err < 0) { > > - dev_err(&client->dev, "Unable to write flags register\n"); > > - return err; > > - } > > - > > return err; > > } > > > > -- > > 2.34.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > -- Regards, A. Mydeen. [-- Attachment #1.2: Type: text/html, Size: 9185 bytes --] [-- Attachment #2: 0001-rtc-m41t62-kickstart-ocillator-upon-failure.patch --] [-- Type: text/x-patch, Size: 4787 bytes --] From e1a294b30f7e48d0342087392ff584b4956faa05 Mon Sep 17 00:00:00 2001 From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> Date: Thu, 9 Jan 2025 16:33:00 +0530 Subject: [PATCH] rtc-m41t62: kickstart ocillator upon failure The ocillator on the m41t62 (and other chips of this type) needs a kickstart upon a failure; the RTC read routine will notice the oscillator failure and fail reads. This is added in the RTC write routine; this allows the system to know that the time in the RTC is accurate. This is following the procedure described in section 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> Reviewed-by: Corey Minyard <cminyard@mvista.com> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/rtc/rtc-m41t80.c | 68 ++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c index 1f58ae8b151e..7074d086f1c8 100644 --- a/drivers/rtc/rtc-m41t80.c +++ b/drivers/rtc/rtc-m41t80.c @@ -22,6 +22,7 @@ #include <linux/slab.h> #include <linux/mutex.h> #include <linux/string.h> +#include <linux/delay.h> #ifdef CONFIG_RTC_DRV_M41T80_WDT #include <linux/fs.h> #include <linux/ioctl.h> @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return flags; if (flags & M41T80_FLAGS_OF) { - dev_err(&client->dev, "Oscillator failure, data is invalid.\n"); + dev_err(&client->dev, "Oscillator failure, time may not be accurate, write time to RTC to fix it.\n"); return -EINVAL; } @@ -227,21 +228,31 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return 0; } -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *in_tm) { struct i2c_client *client = to_i2c_client(dev); struct m41t80_data *clientdata = i2c_get_clientdata(client); + struct rtc_time tm = *in_tm; unsigned char buf[8]; int err, flags; + time64_t time = 0; + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) + return flags; + if (flags & M41T80_FLAGS_OF) { + /* add 4sec of oscillator stablize time otherwise we are behind 4sec */ + time = rtc_tm_to_time64(&tm); + rtc_time64_to_tm(time + 4, &tm); + } buf[M41T80_REG_SSEC] = 0; - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); - buf[M41T80_REG_WDAY] = tm->tm_wday; + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); + buf[M41T80_REG_WDAY] = tm.tm_wday; /* If the square wave output is controlled in the weekday register */ if (clientdata->features & M41T80_FEATURE_SQ_ALT) { @@ -260,17 +271,34 @@ static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) dev_err(&client->dev, "Unable to write to date registers\n"); return err; } - - /* Clear the OF bit of Flags Register */ - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); - if (flags < 0) - return flags; - - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, - flags & ~M41T80_FLAGS_OF); - if (err < 0) { - dev_err(&client->dev, "Unable to write flags register\n"); - return err; + if (flags & M41T80_FLAGS_OF) { + /* OF cannot be immediately reset: oscillator has to be restarted. */ + dev_warn(&client->dev, "OF bit is still set, kickstarting clock.\n"); + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't set ST bit\n"); + return err; + } + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, flags & ~M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't clear ST bit\n"); + return err; + } + /* oscillator must run for 4sec before we attempt to reset OF bit */ + msleep(4000); + /* Clear the OF bit of Flags Register */ + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, flags & ~M41T80_FLAGS_OF); + if (err < 0) { + dev_err(&client->dev, "Unable to write flags register\n"); + return err; + } + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) { + return flags; + } else if (flags & M41T80_FLAGS_OF) { + dev_err(&client->dev, "Can't clear the OF bit check battery\n"); + return err; + } } return err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Re: [PATCH] rtc-m41t62: kickstart ocillator upon failure 2025-04-01 9:04 ` Alexandre Belloni 2025-04-01 19:40 ` Niyas Mydeen @ 2025-04-02 12:05 ` nmydeen 2025-04-02 12:05 ` [PATCH v2] " nmydeen 1 sibling, 1 reply; 7+ messages in thread From: nmydeen @ 2025-04-02 12:05 UTC (permalink / raw) To: alexandre.belloni; +Cc: linux-rtc, linux-kernel, cminyard Thanks, Hope it meets the expected ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] rtc-m41t62: kickstart ocillator upon failure 2025-04-02 12:05 ` nmydeen @ 2025-04-02 12:05 ` nmydeen 2025-05-25 22:24 ` Alexandre Belloni 0 siblings, 1 reply; 7+ messages in thread From: nmydeen @ 2025-04-02 12:05 UTC (permalink / raw) To: alexandre.belloni Cc: linux-rtc, linux-kernel, cminyard, A. Niyas Ahamed Mydeen From: "A. Niyas Ahamed Mydeen" <nmydeen@mvista.com> The ocillator on the m41t62 (and other chips of this type) needs a kickstart upon a failure; the RTC read routine will notice the oscillator failure and fail reads. This is added in the RTC write routine; this allows the system to know that the time in the RTC is accurate. This is following the procedure described in section 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" Signed-off-by: A. Niyas Ahamed Mydeen <nmydeen@mvista.com> Reviewed-by: Corey Minyard <cminyard@mvista.com> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/rtc/rtc-m41t80.c | 68 ++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c index 1f58ae8b151e..7074d086f1c8 100644 --- a/drivers/rtc/rtc-m41t80.c +++ b/drivers/rtc/rtc-m41t80.c @@ -22,6 +22,7 @@ #include <linux/slab.h> #include <linux/mutex.h> #include <linux/string.h> +#include <linux/delay.h> #ifdef CONFIG_RTC_DRV_M41T80_WDT #include <linux/fs.h> #include <linux/ioctl.h> @@ -204,7 +205,7 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return flags; if (flags & M41T80_FLAGS_OF) { - dev_err(&client->dev, "Oscillator failure, data is invalid.\n"); + dev_err(&client->dev, "Oscillator failure, time may not be accurate, write time to RTC to fix it.\n"); return -EINVAL; } @@ -227,21 +228,31 @@ static int m41t80_rtc_read_time(struct device *dev, struct rtc_time *tm) return 0; } -static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) +static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *in_tm) { struct i2c_client *client = to_i2c_client(dev); struct m41t80_data *clientdata = i2c_get_clientdata(client); + struct rtc_time tm = *in_tm; unsigned char buf[8]; int err, flags; + time64_t time = 0; + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) + return flags; + if (flags & M41T80_FLAGS_OF) { + /* add 4sec of oscillator stablize time otherwise we are behind 4sec */ + time = rtc_tm_to_time64(&tm); + rtc_time64_to_tm(time + 4, &tm); + } buf[M41T80_REG_SSEC] = 0; - buf[M41T80_REG_SEC] = bin2bcd(tm->tm_sec); - buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min); - buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour); - buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday); - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1); - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100); - buf[M41T80_REG_WDAY] = tm->tm_wday; + buf[M41T80_REG_SEC] = bin2bcd(tm.tm_sec); + buf[M41T80_REG_MIN] = bin2bcd(tm.tm_min); + buf[M41T80_REG_HOUR] = bin2bcd(tm.tm_hour); + buf[M41T80_REG_DAY] = bin2bcd(tm.tm_mday); + buf[M41T80_REG_MON] = bin2bcd(tm.tm_mon + 1); + buf[M41T80_REG_YEAR] = bin2bcd(tm.tm_year - 100); + buf[M41T80_REG_WDAY] = tm.tm_wday; /* If the square wave output is controlled in the weekday register */ if (clientdata->features & M41T80_FEATURE_SQ_ALT) { @@ -260,17 +271,34 @@ static int m41t80_rtc_set_time(struct device *dev, struct rtc_time *tm) dev_err(&client->dev, "Unable to write to date registers\n"); return err; } - - /* Clear the OF bit of Flags Register */ - flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); - if (flags < 0) - return flags; - - err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, - flags & ~M41T80_FLAGS_OF); - if (err < 0) { - dev_err(&client->dev, "Unable to write flags register\n"); - return err; + if (flags & M41T80_FLAGS_OF) { + /* OF cannot be immediately reset: oscillator has to be restarted. */ + dev_warn(&client->dev, "OF bit is still set, kickstarting clock.\n"); + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't set ST bit\n"); + return err; + } + err = i2c_smbus_write_byte_data(client, M41T80_REG_SEC, flags & ~M41T80_SEC_ST); + if (err < 0) { + dev_err(&client->dev, "Can't clear ST bit\n"); + return err; + } + /* oscillator must run for 4sec before we attempt to reset OF bit */ + msleep(4000); + /* Clear the OF bit of Flags Register */ + err = i2c_smbus_write_byte_data(client, M41T80_REG_FLAGS, flags & ~M41T80_FLAGS_OF); + if (err < 0) { + dev_err(&client->dev, "Unable to write flags register\n"); + return err; + } + flags = i2c_smbus_read_byte_data(client, M41T80_REG_FLAGS); + if (flags < 0) { + return flags; + } else if (flags & M41T80_FLAGS_OF) { + dev_err(&client->dev, "Can't clear the OF bit check battery\n"); + return err; + } } return err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] rtc-m41t62: kickstart ocillator upon failure 2025-04-02 12:05 ` [PATCH v2] " nmydeen @ 2025-05-25 22:24 ` Alexandre Belloni 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Belloni @ 2025-05-25 22:24 UTC (permalink / raw) To: nmydeen; +Cc: linux-rtc, linux-kernel, cminyard On Wed, 02 Apr 2025 17:35:46 +0530, nmydeen@mvista.com wrote: > The ocillator on the m41t62 (and other chips of this type) needs > a kickstart upon a failure; the RTC read routine will notice the > oscillator failure and fail reads. This is added in the RTC write > routine; this allows the system to know that the time in the RTC > is accurate. This is following the procedure described in section > 3.11 of "https://www.st.com/resource/en/datasheet/m41t62.pdf" > > [...] Applied, thanks! [1/1] rtc-m41t62: kickstart ocillator upon failure https://git.kernel.org/abelloni/c/1a7ed2fffbe3 Best regards, -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-05-25 22:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-16 6:26 [PATCH] rtc-m41t62: kickstart ocillator upon failure nmydeen 2025-02-04 11:54 ` Corey Minyard 2025-04-01 9:04 ` Alexandre Belloni 2025-04-01 19:40 ` Niyas Mydeen 2025-04-02 12:05 ` nmydeen 2025-04-02 12:05 ` [PATCH v2] " nmydeen 2025-05-25 22:24 ` Alexandre Belloni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox