[PATCH v2] rtc: ds1307: Handle oscillator-stop bit correctly

[Mark Tomlinson]

The DS1307 driver was originally based on the DS1337 driver. However,
the functionality of the clock set/get functions has diverged. In the
original DS1337 driver, the set/get functions did the following:
  1) Setting the clock ensured the oscillator was enabled.
  2) Getting the clock checked and reset the oscillator-stop flag.
The DS1307 does not have an oscillator-stop flag, but the driver tried
(incorrectly) to emulate this by ensuring the oscillator was running. It
really makes no sense to start a stopped clock without setting it.

This patch makes the DS1307 driver behave like the original DS1337
driver again. For the DS1307 itself, this is just a removal of code,
since there is no oscillator-fail bit to check or reset, and the clock
is started when it is set. Since the DS1307 driver can now also be used
for the DS1337 and DS1340 which do have this bit, add code to handle the
oscillator-stop bit in the same was the original DS1337 driver did --
i.e. report that the oscillator had stopped and clear the flag.

This means that setting the date using the date command (which does both
a get and a set) will now clear the oscillator-stop flag in addition to
setting and starting the clock.

The old-style (non-DM) code has not been updated and will be removed in
a future patch. Note that this older code does not support the DS1337,
as there is a separate driver for this. Also note that the original (DM)
code used the wrong control-register address for the DS1337.

Signed-off-by: Mark Tomlinson <mark.tomlinson at alliedtelesis.co.nz>
---
Note that this patch is based on 'next' rather than 'master' as it is
dependent on a patch which has not yet made it to master.

Changes for v2:
   - Fix spelling mistake in commit.
   - Remove changes to old non-DM code.

 drivers/rtc/ds1307.c | 72 ++++++++++++--------------------------------
 1 file changed, 20 insertions(+), 52 deletions(-)

diff --git a/drivers/rtc/ds1307.c b/drivers/rtc/ds1307.c
index 3be97c9d93..1963565c5e 100644
--- a/drivers/rtc/ds1307.c
+++ b/drivers/rtc/ds1307.c
@@ -41,6 +41,12 @@ enum ds_type {
 #define RTC_YR_REG_ADDR		0x06
 #define RTC_CTL_REG_ADDR	0x07
 
+#define DS1337_CTL_REG_ADDR	0x0e
+#define DS1337_STAT_REG_ADDR	0x0f
+#define DS1340_STAT_REG_ADDR	0x09
+
+#define RTC_STAT_BIT_OSF	0x80
+
 #define RTC_SEC_BIT_CH		0x80	/* Clock Halt (in Register 0)   */
 
 /* DS1307-specific bits */
@@ -248,6 +254,11 @@ static int ds1307_rtc_set(struct udevice *dev, const struct rtc_time *tm)
 	if (ret < 0)
 		return ret;
 
+	if (type == ds_1337) {
+		/* Ensure oscillator is enabled */
+		dm_i2c_reg_write(dev, DS1337_CTL_REG_ADDR, 0);
+	}
+
 	return 0;
 }
 
@@ -257,62 +268,19 @@ static int ds1307_rtc_get(struct udevice *dev, struct rtc_time *tm)
 	uchar buf[7];
 	enum ds_type type = dev_get_driver_data(dev);
 
-read_rtc:
 	ret = dm_i2c_read(dev, 0, buf, sizeof(buf));
 	if (ret < 0)
 		return ret;
 
-	if (type == ds_1307) {
-		if (buf[RTC_SEC_REG_ADDR] & RTC_SEC_BIT_CH) {
-			printf("### Warning: RTC oscillator has stopped\n");
-			/* clear the CH flag */
-			buf[RTC_SEC_REG_ADDR] &= ~RTC_SEC_BIT_CH;
-			dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR,
-					 buf[RTC_SEC_REG_ADDR]);
-			return -1;
-		}
-	} else if (type == ds_1337) {
-		if (buf[RTC_CTL_REG_ADDR] & DS1337_CTL_BIT_EOSC) {
-			printf("### Warning: RTC oscillator has stopped\n");
-			/* clear the not oscillator enable (~EOSC) flag */
-			buf[RTC_CTL_REG_ADDR] &= ~DS1337_CTL_BIT_EOSC;
-			dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR,
-					 buf[RTC_CTL_REG_ADDR]);
-			return -1;
-		}
-	} else if (type == ds_1340) {
-		if (buf[RTC_SEC_REG_ADDR] & DS1340_SEC_BIT_EOSC) {
-			printf("### Warning: RTC oscillator has stopped\n");
-			/* clear the not oscillator enable (~EOSC) flag */
-			buf[RTC_SEC_REG_ADDR] &= ~DS1340_SEC_BIT_EOSC;
-			dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR,
-					 buf[RTC_SEC_REG_ADDR]);
-			return -1;
-		}
-	} else if (type == m41t11) {
-		/* clock halted?  turn it on, so clock can tick. */
-		if (buf[RTC_SEC_REG_ADDR] & RTC_SEC_BIT_CH) {
-			buf[RTC_SEC_REG_ADDR] &= ~RTC_SEC_BIT_CH;
-			dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR,
-					 MCP7941X_BIT_ST);
-			dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR,
-					 buf[RTC_SEC_REG_ADDR]);
-			goto read_rtc;
-		}
-	} else if (type == mcp794xx) {
-		/* make sure that the backup battery is enabled */
-		if (!(buf[RTC_DAY_REG_ADDR] & MCP7941X_BIT_VBATEN)) {
-			dm_i2c_reg_write(dev, RTC_DAY_REG_ADDR,
-					 buf[RTC_DAY_REG_ADDR] |
-					 MCP7941X_BIT_VBATEN);
-		}
+	if (type == ds_1337 || type == ds_1340) {
+		uint reg = (type == ds_1337) ? DS1337_STAT_REG_ADDR :
+					       DS1340_STAT_REG_ADDR;
+		int status = dm_i2c_reg_read(dev, reg);
 
-		/* clock halted?  turn it on, so clock can tick. */
-		if (!(buf[RTC_SEC_REG_ADDR] & MCP7941X_BIT_ST)) {
-			dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR,
-					 MCP7941X_BIT_ST);
-			printf("Started RTC\n");
-			goto read_rtc;
+		if (status >= 0 && (status & RTC_STAT_BIT_OSF)) {
+			printf("### Warning: RTC oscillator has stopped\n");
+			/* clear the OSF flag */
+			dm_i2c_reg_write(dev, reg, status & ~RTC_STAT_BIT_OSF);
 		}
 	}
 
@@ -361,7 +329,7 @@ static int ds1307_rtc_reset(struct udevice *dev)
 		/* Write control register in order to enable oscillator output
 		 * (not EOSC) and set a default rate of 32.768kHz (RS2|RS1).
 		 */
-		ret = dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR,
+		ret = dm_i2c_reg_write(dev, DS1337_CTL_REG_ADDR,
 				       DS1337_CTL_BIT_RS2 | DS1337_CTL_BIT_RS1);
 	} else if (type == ds_1340 || type == mcp794xx || type == m41t11) {
 		/* Reset clock calibration, frequency test and output level. */
-- 
2.33.0