[PATCH] RTC: M41T80: Century Bit support

[PATCH] RTC: M41T80: Century Bit support

[Maciej W. Rozycki]

 Make use of the Century Bit to support years in the range from 1970 to 
2169.  Enable toggling of the bit at the end of a century on a clock 
update.
---
 The clock is used with the Broadcom SWARM and the D-Link DNS-323 
platform.

 I have verified correct operation with the SWARM -- the firmware assumes
19YY when CB is clear and 20YY otherwise.  Which means years 1900-1969
will be shown as 2100-2169 in Linux.  I think this is a feature rather
than a problem.  The firmware does not set the CEB bit itself and does not 
care of what its state is.

 I will be happy to hear from a DNS-323 user to confirm or deny whether
such an interpretation is compatible.

 Otherwise, please apply.

  Maciej

patch-2.6.26-rc1-20080505-m41t80-century-4
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/drivers/rtc/rtc-m41t80.c linux-2.6.26-rc1-20080505/drivers/rtc/rtc-m41t80.c
--- linux-2.6.26-rc1-20080505.macro/drivers/rtc/rtc-m41t80.c	2008-05-05 02:55:40.000000000 +0000
+++ linux-2.6.26-rc1-20080505/drivers/rtc/rtc-m41t80.c	2008-05-16 21:16:48.000000000 +0000
@@ -52,6 +52,8 @@
 	(M41T80_REG_ALARM_SEC + 1 - M41T80_REG_ALARM_MON)
 
 #define M41T80_SEC_ST		(1 << 7)	/* ST: Stop Bit */
+#define M41T80_HOUR_CEB		(1 << 7)	/* CEB: Century Enable Bit */
+#define M41T80_HOUR_CB		(1 << 6)	/* CB: Century Bit */
 #define M41T80_ALMON_AFE	(1 << 7)	/* AFE: AF Enable Bit */
 #define M41T80_ALMON_SQWE	(1 << 6)	/* SQWE: SQW Enable Bit */
 #define M41T80_ALHOUR_HT	(1 << 6)	/* HT: Halt Update Bit */
@@ -164,8 +166,12 @@ static int m41t80_get_datetime(struct i2
 	tm->tm_wday = buf[M41T80_REG_WDAY] & 0x07;
 	tm->tm_mon = BCD2BIN(buf[M41T80_REG_MON] & 0x1f) - 1;
 
-	/* assume 20YY not 19YY, and ignore the Century Bit */
-	tm->tm_year = BCD2BIN(buf[M41T80_REG_YEAR]) + 100;
+	/* Assume 1970-2169, taking the Century Bit into account. */
+	tm->tm_year = BCD2BIN(buf[M41T80_REG_YEAR]);
+	if ((buf[M41T80_REG_HOUR] & M41T80_HOUR_CB) != 0)
+		tm->tm_year += 100;
+	if (tm->tm_year < 70)
+		tm->tm_year += 200;
 	return 0;
 }
 
@@ -196,8 +202,14 @@ static int m41t80_set_datetime(struct i2
 		BIN2BCD(tm->tm_mday) | (buf[M41T80_REG_DAY] & ~0x3f);
 	buf[M41T80_REG_MON] =
 		BIN2BCD(tm->tm_mon + 1) | (buf[M41T80_REG_MON] & ~0x1f);
-	/* assume 20YY not 19YY */
+	/* Assume 1970-2169 and set the Century Bit for 19YY/21YY. */
 	buf[M41T80_REG_YEAR] = BIN2BCD(tm->tm_year % 100);
+	if ((tm->tm_year / 100) % 2 != 0)
+		buf[M41T80_REG_HOUR] |= M41T80_HOUR_CB;
+	else
+		buf[M41T80_REG_HOUR] &= ~M41T80_HOUR_CB;
+	/* Enable toggling of the Century Bit at the end of a century. ;-) */
+	buf[M41T80_REG_HOUR] |= M41T80_HOUR_CEB;
 
 	if (m41t80_write_block_data(client, M41T80_REG_SSEC,
 				    M41T80_DATETIME_REG_SIZE, buf) < 0) {

Re: [PATCH] RTC: M41T80: Century Bit support

[Atsushi Nemoto]

On Sat, 17 May 2008 01:25:18 +0100 (BST), "Maciej W. Rozycki" <macro@linux-mips.org> wrote:
>  Make use of the Century Bit to support years in the range from 1970 to 
> 2169.  Enable toggling of the bit at the end of a century on a clock 
> update.
> ---
>  The clock is used with the Broadcom SWARM and the D-Link DNS-323 
> platform.
> 
>  I have verified correct operation with the SWARM -- the firmware assumes
> 19YY when CB is clear and 20YY otherwise.  Which means years 1900-1969
> will be shown as 2100-2169 in Linux.  I think this is a feature rather
> than a problem.  The firmware does not set the CEB bit itself and does not 
> care of what its state is.
> 
>  I will be happy to hear from a DNS-323 user to confirm or deny whether
> such an interpretation is compatible.

This patch enforces that all (including future) users of this device
must use same interpretation of CB bit.  I think this is too
intrusive.

And I have one (out-of-tree, and only one in the world) board with
this device and its firmware uses different interpretation.
Fortunately I can change this firmware, so this is not a serious
problem for me. ;)

How about doing like this?

1. If CEB was 0, keep current behavior. (does not touch CB bit)

2. If CEB was 1, detect polarity of CB bit on get_datetime, and set or
   clear CB bit on set_datetime based on the polarity.

Please look at "c_polarity" variable in rtc-pcf8563.c driver.

---
Atsushi Nemoto

Re: [PATCH] RTC: M41T80: Century Bit support

[Maciej W. Rozycki]

On Sun, 18 May 2008, Atsushi Nemoto wrote:

> This patch enforces that all (including future) users of this device
> must use same interpretation of CB bit.  I think this is too
> intrusive.

 I feared so, but on the other hand I did not want to introduce yet 
another kernel/module command line parameter -- there are too many of them 
already. :-(

> And I have one (out-of-tree, and only one in the world) board with
> this device and its firmware uses different interpretation.
> Fortunately I can change this firmware, so this is not a serious
> problem for me. ;)

 Hmm, how about pushing your bits upstream? ;-)

> How about doing like this?
> 
> 1. If CEB was 0, keep current behavior. (does not touch CB bit)
> 
> 2. If CEB was 1, detect polarity of CB bit on get_datetime, and set or
>    clear CB bit on set_datetime based on the polarity.

 That's what I did initially as it is as obvious as you can get.  But as I
noted, CFE, the firmware used with the SWARM, does not set CEB even though
it takes CB into account.  The approach is not useful therefore at least
for one major user of the device.

 Of course CFE is BSD-licensed and can be changed too, but based on my
experience with how serious bugs are handled, I would not count on getting
such a minor change integrated into the official sources.

> Please look at "c_polarity" variable in rtc-pcf8563.c driver.

 Hmm, not terribly useful as few of us if anybody live back in the 20th
century. ;-)  I think let's scrap the patch in this case and let our
grandchildren solve the problem -- a proposal has been published and can
be reused as needed.

  Maciej

Re: [PATCH] RTC: M41T80: Century Bit support

[Atsushi Nemoto]

On Sat, 17 May 2008 20:16:20 +0100 (BST), "Maciej W. Rozycki" <macro@linux-mips.org> wrote:
> > How about doing like this?
> > 
> > 1. If CEB was 0, keep current behavior. (does not touch CB bit)
> > 
> > 2. If CEB was 1, detect polarity of CB bit on get_datetime, and set or
> >    clear CB bit on set_datetime based on the polarity.
> 
>  That's what I did initially as it is as obvious as you can get.  But as I
> noted, CFE, the firmware used with the SWARM, does not set CEB even though
> it takes CB into account.  The approach is not useful therefore at least
> for one major user of the device.

Oh I had missed that the firmware does not set CEB.  Hmm...

>  Of course CFE is BSD-licensed and can be changed too, but based on my
> experience with how serious bugs are handled, I would not count on getting
> such a minor change integrated into the official sources.
> 
> > Please look at "c_polarity" variable in rtc-pcf8563.c driver.
> 
>  Hmm, not terribly useful as few of us if anybody live back in the 20th
> century. ;-)  I think let's scrap the patch in this case and let our
> grandchildren solve the problem -- a proposal has been published and can
> be reused as needed.

OK for me.  Lets hear how our grandchildren complain on this fault. :)

---
Atsushi Nemoto

Re: [PATCH] RTC: M41T80: Century Bit support

[Maciej W. Rozycki]

On Mon, 19 May 2008, Atsushi Nemoto wrote:

> >  That's what I did initially as it is as obvious as you can get.  But as I
> > noted, CFE, the firmware used with the SWARM, does not set CEB even though
> > it takes CB into account.  The approach is not useful therefore at least
> > for one major user of the device.
> 
> Oh I had missed that the firmware does not set CEB.  Hmm...

 As I said, this is one of the lesser problems with CFE, sigh...

> >  Hmm, not terribly useful as few of us if anybody live back in the 20th
> > century. ;-)  I think let's scrap the patch in this case and let our
> > grandchildren solve the problem -- a proposal has been published and can
> > be reused as needed.
> 
> OK for me.  Lets hear how our grandchildren complain on this fault. :)

 Well, Andrew has applied this patch to the -mm tree now and after a bit
of thinking I have made up my mind and decided we should keep the patch.  
This is an I2C device which means it will always only be explicitly
requested by platform code.  This is an opportunity for the platform to
express the will and the way to use the century bit.

 I'll cook-up a follow-on patch as soon as the SWARM bits propagate to
upstream, to add a set of flags that will let platforms specify the
desired way the century bit is meant to be handled and which this driver
will honour.  Please feel free to keep the interpretation within your pet
board intact -- the flags will be capable to express your needs.

 Andrew, could you please hold the patch in -mm till the flags have been 
added?

  Maciej

Re: [PATCH] RTC: M41T80: Century Bit support

[Andrew Morton]

On Tue, 20 May 2008 20:51:56 +0100 (BST)
"Maciej W. Rozycki" <macro@linux-mips.org> wrote:

>  Andrew, could you please hold the patch in -mm till the flags have been 
> added?

It's be saner for me to drop it and have you resend when appropriate. 
The chances of me being able to succesfully correlate this patch and
something called "the flags" at some time in the future are less than
100% ;)

Re: [PATCH] RTC: M41T80: Century Bit support

[Maciej W. Rozycki]

On Tue, 20 May 2008, Andrew Morton wrote:

> It's be saner for me to drop it and have you resend when appropriate.  

 OK, no problem with that.

> The chances of me being able to succesfully correlate this patch and
> something called "the flags" at some time in the future are less than
> 100% ;)

 You mean like 99.9% less? ;-)  Some people call it "epsilon".

  Maciej

Re: [PATCH] RTC: M41T80: Century Bit support

[Atsushi Nemoto]

On Tue, 20 May 2008 20:51:56 +0100 (BST), "Maciej W. Rozycki" <macro@linux-mips.org> wrote:
>  Well, Andrew has applied this patch to the -mm tree now and after a bit
> of thinking I have made up my mind and decided we should keep the patch.  
> This is an I2C device which means it will always only be explicitly
> requested by platform code.  This is an opportunity for the platform to
> express the will and the way to use the century bit.
> 
>  I'll cook-up a follow-on patch as soon as the SWARM bits propagate to
> upstream, to add a set of flags that will let platforms specify the
> desired way the century bit is meant to be handled and which this driver
> will honour.  Please feel free to keep the interpretation within your pet
> board intact -- the flags will be capable to express your needs.

OK, it would be the best way to handle the century bit.
I'll test your patch (if my board is still alive at the time ;-))

---
Atsushi Nemoto