From: Keerthy <a0393675@ti.com>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>,
Keerthy <j-keerthy@ti.com>
Cc: <a.zummo@towertech.it>, <nm@ti.com>, <rtc-linux@googlegroups.com>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] rtc: ds1307: Fix relying on reset value for weekday
Date: Tue, 7 Jun 2016 14:59:05 +0530 [thread overview]
Message-ID: <575693E1.2090302@ti.com> (raw)
In-Reply-To: <574ED6D1.70800@ti.com>
Hi Alexandre,
On Wednesday 01 June 2016 06:06 PM, Keerthy wrote:
>
>
> 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.
Let me know if you feel this is a valid fix.
Regards,
Keerthy
>>
>>> 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
>>
next prev parent reply other threads:[~2016-06-07 9:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-01 10:49 [PATCH] rtc: ds1307: Fix relying on reset value for weekday Keerthy
2016-06-01 12:18 ` Alexandre Belloni
2016-06-01 12:36 ` Keerthy
2016-06-07 9:29 ` Keerthy [this message]
2016-06-14 14:39 ` Alexandre Belloni
2016-06-15 4:14 ` Keerthy
2016-07-08 14:25 ` Alexandre Belloni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=575693E1.2090302@ti.com \
--to=a0393675@ti.com \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@free-electrons.com \
--cc=j-keerthy@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nm@ti.com \
--cc=rtc-linux@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).