Messages in this thread | | | Subject | Re: [PATCH] rtc: ds1307: Fix relying on reset value for weekday | From | Keerthy <> | Date | Wed, 1 Jun 2016 18:06:33 +0530 |
| |
On Wednesday 01 June 2016 05:48 PM, Alexandre Belloni wrote: > Hi, > > On 01/06/2016 at 16:19:07 +0530, Keerthy wrote : >> The reset value of weekday is 0x1. This is wrong since >> the reset values of the day/month/year make up to Jan 1 2001. >> When computed weekday comes out to be Monday. On a scale >> of 1-7(Sunday - Saturday) it should be 0x2. So we should not >> be relying on the reset value. >> > > Hum, what are the chances that the reset value is actually the correct > date/time? > Won't that be corrected after the first call to set_time? Until then, > the date is not correct so, do we care anyway?
Yes if an alarm is programmed without set_time.
ex: rtcwake -d /dev/rtc0 -s 5
Even the basic rtctest under tools/testing/selftests/timers/rtctest.c fails as the wday is wrong and no alarm fires.
Instead of relying on some one to call set_time before programming alarm its better to fix the wday to reflect the current day right?
> > Maybe I'm missing something here. > >> Hence compute the wday using the current date/month/year values. >> Check if reset wday is any different from the computed wday, >> If different then set the wday which we computed using >> date/month/year values. >> >> Document Referred: >> http://ww1.microchip.com/downloads/en/DeviceDoc/20002266F.pdf >> >> Fixes: 1d1945d261a2af "drivers/rtc/rtc-ds1307.c: add alarm support for mcp7941x chips" >> Signed-off-by: Keerthy <j-keerthy@ti.com> >> --- >> drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++++++++++++- >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c >> index 821d9c0..d9045cc 100644 >> --- a/drivers/rtc/rtc-ds1307.c >> +++ b/drivers/rtc/rtc-ds1307.c >> @@ -602,6 +602,8 @@ static const struct rtc_class_ops ds13xx_rtc_ops = { >> * Alarm support for mcp794xx devices. >> */ >> >> +#define MCP794XX_REG_WEEKDAY 0x3 >> +#define MCP794XX_REG_WEEKDAY_WDAY_MASK 0x7 >> #define MCP794XX_REG_CONTROL 0x07 >> # define MCP794XX_BIT_ALM0_EN 0x10 >> # define MCP794XX_BIT_ALM1_EN 0x20 >> @@ -1231,13 +1233,16 @@ static int ds1307_probe(struct i2c_client *client, >> { >> struct ds1307 *ds1307; >> int err = -ENODEV; >> - int tmp; >> + int tmp, wday; >> struct chip_desc *chip = &chips[id->driver_data]; >> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); >> bool want_irq = false; >> bool ds1307_can_wakeup_device = false; >> unsigned char *buf; >> struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev); >> + struct rtc_time tm; >> + unsigned long timestamp; >> + >> irq_handler_t irq_handler = ds1307_irq; >> >> static const int bbsqi_bitpos[] = { >> @@ -1526,6 +1531,27 @@ read_rtc: >> bin2bcd(tmp)); >> } >> >> + /* >> + * Some IPs have weekday reset value = 0x1 which might not correct >> + * hence compute the wday using the current date/month/year values >> + */ >> + ds1307_get_time(&client->dev, &tm); >> + wday = tm.tm_wday; >> + timestamp = rtc_tm_to_time64(&tm); >> + rtc_time64_to_tm(timestamp, &tm); >> + >> + /* >> + * Check if reset wday is different from the computed wday >> + * If different then set the wday which we computed using >> + * timestamp >> + */ >> + if (wday != tm.tm_wday) { >> + wday = i2c_smbus_read_byte_data(client, MCP794XX_REG_WEEKDAY); >> + wday = wday & ~MCP794XX_REG_WEEKDAY_WDAY_MASK; >> + wday = wday | (tm.tm_wday + 1); >> + i2c_smbus_write_byte_data(client, MCP794XX_REG_WEEKDAY, wday); >> + } >> + >> if (want_irq) { >> device_set_wakeup_capable(&client->dev, true); >> set_bit(HAS_ALARM, &ds1307->flags); >> -- >> 1.9.1 >
| |