diff mbox series

rtc: ds1307: Handle oscillator-stop bit correctly

Message ID 20210921010809.10472-1-mark.tomlinson@alliedtelesis.co.nz
State Superseded
Delegated to: Tom Rini
Headers show
Series rtc: ds1307: Handle oscillator-stop bit correctly | expand

Commit Message

Mark Tomlinson Sept. 21, 2021, 1:08 a.m. UTC
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 and 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 also been updated. Note that this 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@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.

 drivers/rtc/ds1307.c | 106 ++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 77 deletions(-)

Comments

Simon Glass Sept. 22, 2021, 4:19 p.m. UTC | #1
Hi Mark,

On Mon, 20 Sept 2021 at 19:17, Mark Tomlinson
<mark.tomlinson@alliedtelesis.co.nz> wrote:
>
> 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 and oscillator-stop flag, but the driver tried

an

> (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 also been updated. Note that this 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.

Please don't update the old-style code. Delete it (in a separate
patch) if it won't cause build errors.

>
> Signed-off-by: Mark Tomlinson <mark.tomlinson@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.
>
>  drivers/rtc/ds1307.c | 106 ++++++++++++-------------------------------
>  1 file changed, 29 insertions(+), 77 deletions(-)
>

Regards,
Simon
diff mbox series

Patch

diff --git a/drivers/rtc/ds1307.c b/drivers/rtc/ds1307.c
index 3be97c9d93..7fe4f590c5 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 */
@@ -92,10 +98,10 @@  int rtc_get (struct rtc_time *tmp)
 {
 	int rel = 0;
 	uchar sec, min, hour, mday, wday, mon, year;
-
-#ifdef CONFIG_RTC_MCP79411
-read_rtc:
+#ifdef CONFIG_RTC_DS1340
+	uchar status;
 #endif
+
 	sec = rtc_read (RTC_SEC_REG_ADDR);
 	min = rtc_read (RTC_MIN_REG_ADDR);
 	hour = rtc_read (RTC_HR_REG_ADDR);
@@ -108,32 +114,16 @@  read_rtc:
 		"hr: %02x min: %02x sec: %02x\n",
 		year, mon, mday, wday, hour, min, sec);
 
-#ifdef CONFIG_RTC_DS1307
-	if (sec & RTC_SEC_BIT_CH) {
-		printf ("### Warning: RTC oscillator has stopped\n");
-		/* clear the CH flag */
-		rtc_write (RTC_SEC_REG_ADDR,
-			   rtc_read (RTC_SEC_REG_ADDR) & ~RTC_SEC_BIT_CH);
+#ifdef CONFIG_RTC_DS1340
+	status = rtc_read (DS1340_STAT_REG_ADDR);
+	if (status & RTC_STAT_BIT_OSF) {
+		printf("### Warning: RTC oscillator has stopped\n");
+		/* clear the OSF flag */
+		rtc_write (DS1340_STAT_REG_ADDR, status & ~RTC_STAT_BIT_OSF);
 		rel = -1;
 	}
 #endif
 
-#ifdef CONFIG_RTC_MCP79411
-	/* make sure that the backup battery is enabled */
-	if (!(wday & MCP7941X_BIT_VBATEN)) {
-		rtc_write(RTC_DAY_REG_ADDR,
-			  wday | MCP7941X_BIT_VBATEN);
-	}
-
-	/* clock halted?  turn it on, so clock can tick. */
-	if (!(sec & MCP7941X_BIT_ST)) {
-		rtc_write(RTC_SEC_REG_ADDR, MCP7941X_BIT_ST);
-		printf("Started RTC\n");
-		goto read_rtc;
-	}
-#endif
-
-
 	tmp->tm_sec  = bcd2bin (sec & 0x7F);
 	tmp->tm_min  = bcd2bin (min & 0x7F);
 	tmp->tm_hour = bcd2bin (hour & 0x3F);
@@ -248,6 +238,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 +252,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 +313,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. */