Re: [rtc-linux] Re: invalid default values in RTC chip

From: David Brownell
Date: Thu Oct 23 2008 - 05:05:01 EST


On Thursday 23 October 2008, Jüri Reitel wrote:
>
> I included patch that just removes the RTC time registers check, when i
> compiled the module and booted up with invalid values in RTC then system
> automatically set system time and RTC time to 1.1.2000 (this is nice
> because i dont have to use date and hwclock manually)

I wouldn't rely on the initial time setting being valid;
it's not guaranteed by specs for all the chips, as I recall.


> i checked the rtc_valid_tm function and saw that it also checks for the
> year value among others but in ds1307_read_alarm (now this function is
> renamed to ds1337_read_alarm) year is set to -1 with some other fields
> so rtc_valid_tm function can not be used in ds1337_read_alarm function.

Right. I forget what alarm issues I was thinking of, but
that wasn't it ... other code in the RTC framework handles
morphing that time into a valid date.


>
> This patch removes rtc date time values check in probe function

Thanks. This looks OK ... got a Signed-off-By for us?


> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -298,7 +298,7 @@ static int ds1307_set_time(struct device *dev,
> struct rtc_time *t)
>      return 0;
>  }
>  
> -static int ds1307_read_alarm(struct device *dev, struct rtc_wkalrm *t)
> +static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
>  {
>      struct i2c_client       *client = to_i2c_client(dev);
>      struct ds1307        *ds1307 = i2c_get_clientdata(client);
> @@ -353,7 +353,7 @@ static int ds1307_read_alarm(struct device *dev,
> struct rtc_wkalrm *t)
>      return 0;
>  }
>  
> -static int ds1307_set_alarm(struct device *dev, struct rtc_wkalrm *t)
> +static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
>  {
>      struct i2c_client       *client = to_i2c_client(dev);
>      struct ds1307        *ds1307 = i2c_get_clientdata(client);
> @@ -475,8 +475,8 @@ static int ds1307_ioctl(struct device *dev, unsigned
> int cmd, unsigned long arg)
>  static const struct rtc_class_ops ds13xx_rtc_ops = {
>      .read_time    = ds1307_get_time,
>      .set_time    = ds1307_set_time,
> -    .read_alarm    = ds1307_read_alarm,
> -    .set_alarm    = ds1307_set_alarm,
> +    .read_alarm    = ds1337_read_alarm,
> +    .set_alarm    = ds1337_set_alarm,
>      .ioctl        = ds1307_ioctl,
>  };
>  
> @@ -707,22 +707,6 @@ read_rtc:
>          break;
>      }
>  
> -    tmp = ds1307->regs[DS1307_REG_SECS];
> -    tmp = bcd2bin(tmp & 0x7f);
> -    if (tmp > 60)
> -        goto exit_bad;
> -    tmp = bcd2bin(ds1307->regs[DS1307_REG_MIN] & 0x7f);
> -    if (tmp > 60)
> -        goto exit_bad;
> -
> -    tmp = bcd2bin(ds1307->regs[DS1307_REG_MDAY] & 0x3f);
> -    if (tmp == 0 || tmp > 31)
> -        goto exit_bad;
> -
> -    tmp = bcd2bin(ds1307->regs[DS1307_REG_MONTH] & 0x1f);
> -    if (tmp == 0 || tmp > 12)
> -        goto exit_bad;
> -
>      tmp = ds1307->regs[DS1307_REG_HOUR];
>      switch (ds1307->type) {
>      case ds_1340:
> @@ -779,13 +763,6 @@ read_rtc:
>  
>      return 0;
>  
> -exit_bad:
> -    dev_dbg(&client->dev, "%s: %02x %02x %02x %02x %02x %02x %02x\n",
> -            "bogus register",
> -            ds1307->regs[0], ds1307->regs[1],
> -            ds1307->regs[2], ds1307->regs[3],
> -            ds1307->regs[4], ds1307->regs[5],
> -            ds1307->regs[6]);
>  exit_irq:
>      if (ds1307->rtc)
>          rtc_device_unregister(ds1307->rtc);
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/