[PATCH v4] rtc: abx80x: Don't warn about oscillator failure after POR

[PATCH v4] rtc: abx80x: Don't warn about oscillator failure after POR

[Fabio Estevam]

From: Sean Anderson <sean.anderson@seco.com>

According to the datasheet, the "oscillator failure" bit is set

  ...on a power on reset, when both the system and battery voltages have
  dropped below acceptable levels. It is also set if an Oscillator Failure
  occurs....

From testing, this bit is also set if a software reset is initiated.

This bit has a confusing name; it really tells us whether the time data
is valid. We clear it when writing the time. If it is still set, that
means there is a persistent issue, such as an oscillator failure,
instead of a transient one, such as power loss.

Because there are several other reasons which might cause this bit to be
set, including booting for the first time or a battery failure, do not
warn about oscillator failures unconditionally. This may cause system
integrators to waste time investigating the wrong issue.

Continue printing a message about invalid time data before the time has
been set. Only report an oscillator failure after the driver has
successfully written the time and cleared the flag.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
Signed-off-by: Fabio Estevam <festevam@nabladev.com>
---
Changes since v3:
- Rebase onto current mainline.
- Fix commit message wording and typos.

 drivers/rtc/rtc-abx80x.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 5486d9d0b1e5..2525ecb7fb6b 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -127,6 +127,7 @@ struct abx80x_priv {
 	struct rtc_device *rtc;
 	struct i2c_client *client;
 	struct watchdog_device wdog;
+	bool wrote_time;
 };
 
 static int abx80x_write_config_key(struct i2c_client *client, u8 key)
@@ -179,6 +180,7 @@ static int abx80x_enable_trickle_charger(struct i2c_client *client,
 static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[8];
 	int err, flags, rc_mode = 0;
 
@@ -193,7 +195,18 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 			return flags;
 
 		if (flags & ABX8XX_OSS_OF) {
-			dev_err(dev, "Oscillator failure, data is invalid.\n");
+			/*
+			 * The OF bit can be set either because of a reset
+			 * (POR/Software reset) or because of an oscillator
+			 * failure. Effectively, it indicates that the stored
+			 * time is invalid. When we write the time, we clear
+			 * this bit. If it stays set, then this indicates an
+			 * oscillator failure.
+			 */
+			if (priv->wrote_time)
+				dev_err(dev, "Oscillator failure\n");
+			else
+				dev_info(dev, "Time data invalid\n");
 			return -EINVAL;
 		}
 	}
@@ -219,6 +232,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[8];
 	int err, flags;
 
@@ -252,6 +266,7 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		dev_err(&client->dev, "Unable to write oscillator status register\n");
 		return err;
 	}
+	priv->wrote_time = true;
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v4] rtc: abx80x: Don't warn about oscillator failure after POR

[sashiko-bot]

> From: Sean Anderson <sean.anderson@seco.com>
> 
> According to the datasheet, the "oscillator failure" bit is set
> 
>   ...on a power on reset, when both the system and battery voltages have
>   dropped below acceptable levels. It is also set if an Oscillator Failure
>   occurs....
> 
> From testing, this bit is also set if a software reset is initiated.
> 
> This bit has a confusing name; it really tells us whether the time data
> is valid. We clear it when writing the time. If it is still set, that
> means there is a persistent issue, such as an oscillator failure,
> instead of a transient one, such as power loss.
> 
> Because there are several other reasons which might cause this bit to be
> set, including booting for the first time or a battery failure, do not
> warn about oscillator failures unconditionally. This may cause system
> integrators to waste time investigating the wrong issue.
> 
> Continue printing a message about invalid time data before the time has
> been set. Only report an oscillator failure after the driver has
> successfully written the time and cleared the flag.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> Signed-off-by: Fabio Estevam <festevam@nabladev.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725002320.96660-1-festevam@gmail.com?part=1