| Message ID | 200901162014.26421.mfuchs@ma-fu.de |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Fri, 16 Jan 2009 20:14:25 +0100 Matthias Fuchs <mfuchs@ma-fu.de> wrote: > > I think the handling of the century bit in m48t35 RTCs is incorrect. > At least I got problems with recents years. > > I must admit that I do not really understand magic lines like this: > > yrs -= 1970; > if (yrs > 255) /* They are unsigned */ > return -EINVAL; > > if (yrs > 169) > return -EINVAL; > > I updated the driver to handle the RTC's century bit correctly. > This was totally missing before. With my following patch the RTC > seems to be fine and interpretation of the registers meets the > chip's manual. Also the bootloader U-Boot handles the RTC in the > same way (which of course could also be wrong :-). Thomas, can you give it a look? > Signed-off-by: Matthias Fuchs <mfuchs@ma-fu.de> > --- > drivers/rtc/rtc-m48t35.c | 37 +++++++++++++------------------------ > 1 files changed, 13 insertions(+), 24 deletions(-) > > diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c > index 0b21975..05ca9f5 100644 > --- a/drivers/rtc/rtc-m48t35.c > +++ b/drivers/rtc/rtc-m48t35.c > @@ -20,7 +20,7 @@ > #include <linux/bcd.h> > #include <linux/io.h> > > -#define DRV_VERSION "1.0" > +#define DRV_VERSION "1.1" > > struct m48t35_rtc { > u8 pad[0x7ff8]; /* starts at 0x7ff8 */ > @@ -37,6 +37,9 @@ struct m48t35_rtc { > #define M48T35_RTC_SET 0x80 > #define M48T35_RTC_READ 0x40 > > +#define M48T35_RTC_CEB 0x20 > +#define M48T35_RTC_CB 0x10 > + > struct m48t35_priv { > struct rtc_device *rtc; > struct m48t35_rtc __iomem *reg; > @@ -48,7 +51,7 @@ struct m48t35_priv { > static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > { > struct m48t35_priv *priv = dev_get_drvdata(dev); > - u8 control; > + u8 control, day; > > /* > * Only the values that we read from the RTC are set. We leave > @@ -62,6 +65,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > tm->tm_sec = readb(&priv->reg->sec); > tm->tm_min = readb(&priv->reg->min); > tm->tm_hour = readb(&priv->reg->hour); > + day = readb(&priv->reg->day); > tm->tm_mday = readb(&priv->reg->date); > tm->tm_mon = readb(&priv->reg->month); > tm->tm_year = readb(&priv->reg->year); > @@ -75,13 +79,8 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > tm->tm_mon = bcd2bin(tm->tm_mon); > tm->tm_year = bcd2bin(tm->tm_year); > > - /* > - * Account for differences between how the RTC uses the values > - * and how they are defined in a struct rtc_time; > - */ > - tm->tm_year += 70; > - if (tm->tm_year <= 69) > - tm->tm_year += 100; > + tm->tm_year += (day & M48T35_RTC_CB) ? 100 : 0; > + tm->tm_wday = (day & 0x07) - 1; > > tm->tm_mon--; > return rtc_valid_tm(tm); > @@ -90,7 +89,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > { > struct m48t35_priv *priv = dev_get_drvdata(dev); > - unsigned char mon, day, hrs, min, sec; > + unsigned char mon, day, wday, hrs, min, sec; > unsigned int yrs; > u8 control; > > @@ -100,26 +99,15 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > hrs = tm->tm_hour; > min = tm->tm_min; > sec = tm->tm_sec; > - > - if (yrs < 1970) > - return -EINVAL; > - > - yrs -= 1970; > - if (yrs > 255) /* They are unsigned */ > - return -EINVAL; > - > - if (yrs > 169) > - return -EINVAL; > - > - if (yrs >= 100) > - yrs -= 100; > + wday = (tm->tm_wday + 1) | > + M48T35_RTC_CEB | ((yrs >= 2000) ? M48T35_RTC_CB : 0); > > sec = bin2bcd(sec); > min = bin2bcd(min); > hrs = bin2bcd(hrs); > day = bin2bcd(day); > mon = bin2bcd(mon); > - yrs = bin2bcd(yrs); > + yrs = bin2bcd(yrs % 100); > > spin_lock_irq(&priv->lock); > control = readb(&priv->reg->control); > @@ -127,6 +115,7 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > writeb(yrs, &priv->reg->year); > writeb(mon, &priv->reg->month); > writeb(day, &priv->reg->date); > + writeb(wday, &priv->reg->day); > writeb(hrs, &priv->reg->hour); > writeb(min, &priv->reg->min); > writeb(sec, &priv->reg->sec); > -- > 1.5.6.3 > > > >
On Fri, 16 Jan 2009 20:14:25 +0100 Matthias Fuchs <mfuchs@ma-fu.de> wrote: > > I think the handling of the century bit in m48t35 RTCs is incorrect. > At least I got problems with recents years. > > I must admit that I do not really understand magic lines like this: > > yrs -= 1970; > if (yrs > 255) /* They are unsigned */ > return -EINVAL; > > if (yrs > 169) > return -EINVAL; > > I updated the driver to handle the RTC's century bit correctly. > This was totally missing before. With my following patch the RTC > seems to be fine and interpretation of the registers meets the > chip's manual. Also the bootloader U-Boot handles the RTC in the > same way (which of course could also be wrong :-). Thomas, can you give it a look? > Signed-off-by: Matthias Fuchs <mfuchs@ma-fu.de> > --- > drivers/rtc/rtc-m48t35.c | 37 +++++++++++++------------------------ > 1 files changed, 13 insertions(+), 24 deletions(-) > > diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c > index 0b21975..05ca9f5 100644 > --- a/drivers/rtc/rtc-m48t35.c > +++ b/drivers/rtc/rtc-m48t35.c > @@ -20,7 +20,7 @@ > #include <linux/bcd.h> > #include <linux/io.h> > > -#define DRV_VERSION "1.0" > +#define DRV_VERSION "1.1" > > struct m48t35_rtc { > u8 pad[0x7ff8]; /* starts at 0x7ff8 */ > @@ -37,6 +37,9 @@ struct m48t35_rtc { > #define M48T35_RTC_SET 0x80 > #define M48T35_RTC_READ 0x40 > > +#define M48T35_RTC_CEB 0x20 > +#define M48T35_RTC_CB 0x10 > + > struct m48t35_priv { > struct rtc_device *rtc; > struct m48t35_rtc __iomem *reg; > @@ -48,7 +51,7 @@ struct m48t35_priv { > static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > { > struct m48t35_priv *priv = dev_get_drvdata(dev); > - u8 control; > + u8 control, day; > > /* > * Only the values that we read from the RTC are set. We leave > @@ -62,6 +65,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > tm->tm_sec = readb(&priv->reg->sec); > tm->tm_min = readb(&priv->reg->min); > tm->tm_hour = readb(&priv->reg->hour); > + day = readb(&priv->reg->day); > tm->tm_mday = readb(&priv->reg->date); > tm->tm_mon = readb(&priv->reg->month); > tm->tm_year = readb(&priv->reg->year); > @@ -75,13 +79,8 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > tm->tm_mon = bcd2bin(tm->tm_mon); > tm->tm_year = bcd2bin(tm->tm_year); > > - /* > - * Account for differences between how the RTC uses the values > - * and how they are defined in a struct rtc_time; > - */ > - tm->tm_year += 70; > - if (tm->tm_year <= 69) > - tm->tm_year += 100; > + tm->tm_year += (day & M48T35_RTC_CB) ? 100 : 0; > + tm->tm_wday = (day & 0x07) - 1; > > tm->tm_mon--; > return rtc_valid_tm(tm); > @@ -90,7 +89,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) > static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > { > struct m48t35_priv *priv = dev_get_drvdata(dev); > - unsigned char mon, day, hrs, min, sec; > + unsigned char mon, day, wday, hrs, min, sec; > unsigned int yrs; > u8 control; > > @@ -100,26 +99,15 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > hrs = tm->tm_hour; > min = tm->tm_min; > sec = tm->tm_sec; > - > - if (yrs < 1970) > - return -EINVAL; > - > - yrs -= 1970; > - if (yrs > 255) /* They are unsigned */ > - return -EINVAL; > - > - if (yrs > 169) > - return -EINVAL; > - > - if (yrs >= 100) > - yrs -= 100; > + wday = (tm->tm_wday + 1) | > + M48T35_RTC_CEB | ((yrs >= 2000) ? M48T35_RTC_CB : 0); > > sec = bin2bcd(sec); > min = bin2bcd(min); > hrs = bin2bcd(hrs); > day = bin2bcd(day); > mon = bin2bcd(mon); > - yrs = bin2bcd(yrs); > + yrs = bin2bcd(yrs % 100); > > spin_lock_irq(&priv->lock); > control = readb(&priv->reg->control); > @@ -127,6 +115,7 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) > writeb(yrs, &priv->reg->year); > writeb(mon, &priv->reg->month); > writeb(day, &priv->reg->date); > + writeb(wday, &priv->reg->day); > writeb(hrs, &priv->reg->hour); > writeb(min, &priv->reg->min); > writeb(sec, &priv->reg->sec); > -- > 1.5.6.3 > > > >
On Thu, Apr 02, 2009 at 03:15:35PM +0200, Alessandro Zummo wrote: > On Fri, 16 Jan 2009 20:14:25 +0100 > Matthias Fuchs <mfuchs@ma-fu.de> wrote: > > > > > I think the handling of the century bit in m48t35 RTCs is incorrect. > > At least I got problems with recents years. > > > > I must admit that I do not really understand magic lines like this: > > > > yrs -= 1970; > > if (yrs > 255) /* They are unsigned */ > > return -EINVAL; > > > > if (yrs > 169) > > return -EINVAL; > > > > I updated the driver to handle the RTC's century bit correctly. > > This was totally missing before. With my following patch the RTC > > seems to be fine and interpretation of the registers meets the > > chip's manual. Also the bootloader U-Boot handles the RTC in the > > same way (which of course could also be wrong :-). > > Thomas, can you give it a look? this breaks rtc handling on my Origin 200 completely. Looks like we need an abstraction for the special SGI year treatment and other users of the rtc. Which platform uses this rtc ? Thomas.
diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c index 0b21975..05ca9f5 100644 --- a/drivers/rtc/rtc-m48t35.c +++ b/drivers/rtc/rtc-m48t35.c @@ -20,7 +20,7 @@ #include <linux/bcd.h> #include <linux/io.h> -#define DRV_VERSION "1.0" +#define DRV_VERSION "1.1" struct m48t35_rtc { u8 pad[0x7ff8]; /* starts at 0x7ff8 */ @@ -37,6 +37,9 @@ struct m48t35_rtc { #define M48T35_RTC_SET 0x80 #define M48T35_RTC_READ 0x40 +#define M48T35_RTC_CEB 0x20 +#define M48T35_RTC_CB 0x10 + struct m48t35_priv { struct rtc_device *rtc; struct m48t35_rtc __iomem *reg; @@ -48,7 +51,7 @@ struct m48t35_priv { static int m48t35_read_time(struct device *dev, struct rtc_time *tm) { struct m48t35_priv *priv = dev_get_drvdata(dev); - u8 control; + u8 control, day; /* * Only the values that we read from the RTC are set. We leave @@ -62,6 +65,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) tm->tm_sec = readb(&priv->reg->sec); tm->tm_min = readb(&priv->reg->min); tm->tm_hour = readb(&priv->reg->hour); + day = readb(&priv->reg->day); tm->tm_mday = readb(&priv->reg->date); tm->tm_mon = readb(&priv->reg->month); tm->tm_year = readb(&priv->reg->year); @@ -75,13 +79,8 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) tm->tm_mon = bcd2bin(tm->tm_mon); tm->tm_year = bcd2bin(tm->tm_year); - /* - * Account for differences between how the RTC uses the values - * and how they are defined in a struct rtc_time; - */ - tm->tm_year += 70; - if (tm->tm_year <= 69) - tm->tm_year += 100; + tm->tm_year += (day & M48T35_RTC_CB) ? 100 : 0; + tm->tm_wday = (day & 0x07) - 1; tm->tm_mon--; return rtc_valid_tm(tm); @@ -90,7 +89,7 @@ static int m48t35_read_time(struct device *dev, struct rtc_time *tm) static int m48t35_set_time(struct device *dev, struct rtc_time *tm) { struct m48t35_priv *priv = dev_get_drvdata(dev); - unsigned char mon, day, hrs, min, sec; + unsigned char mon, day, wday, hrs, min, sec; unsigned int yrs; u8 control; @@ -100,26 +99,15 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) hrs = tm->tm_hour; min = tm->tm_min; sec = tm->tm_sec; - - if (yrs < 1970) - return -EINVAL; - - yrs -= 1970; - if (yrs > 255) /* They are unsigned */ - return -EINVAL; - - if (yrs > 169) - return -EINVAL; - - if (yrs >= 100) - yrs -= 100; + wday = (tm->tm_wday + 1) | + M48T35_RTC_CEB | ((yrs >= 2000) ? M48T35_RTC_CB : 0); sec = bin2bcd(sec); min = bin2bcd(min); hrs = bin2bcd(hrs); day = bin2bcd(day); mon = bin2bcd(mon); - yrs = bin2bcd(yrs); + yrs = bin2bcd(yrs % 100); spin_lock_irq(&priv->lock); control = readb(&priv->reg->control); @@ -127,6 +115,7 @@ static int m48t35_set_time(struct device *dev, struct rtc_time *tm) writeb(yrs, &priv->reg->year); writeb(mon, &priv->reg->month); writeb(day, &priv->reg->date); + writeb(wday, &priv->reg->day); writeb(hrs, &priv->reg->hour); writeb(min, &priv->reg->min); writeb(sec, &priv->reg->sec);
Hi, I think the handling of the century bit in m48t35 RTCs is incorrect. At least I got problems with recents years. I must admit that I do not really understand magic lines like this: yrs -= 1970; if (yrs > 255) /* They are unsigned */ return -EINVAL; if (yrs > 169) return -EINVAL; I updated the driver to handle the RTC's century bit correctly. This was totally missing before. With my following patch the RTC seems to be fine and interpretation of the registers meets the chip's manual. Also the bootloader U-Boot handles the RTC in the same way (which of course could also be wrong :-). Signed-off-by: Matthias Fuchs <mfuchs@ma-fu.de> --- drivers/rtc/rtc-m48t35.c | 37 +++++++++++++------------------------ 1 files changed, 13 insertions(+), 24 deletions(-)