From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
To: 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: Wed, 1 Jun 2016 14:18:02 +0200 [thread overview]
Message-ID: <20160601121802.GF4249@piout.net> (raw)
In-Reply-To: <1464778147-22075-1-git-send-email-j-keerthy@ti.com>
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?
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
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
next prev parent reply other threads:[~2016-06-01 12:18 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 [this message]
2016-06-01 12:36 ` Keerthy
2016-06-07 9:29 ` Keerthy
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=20160601121802.GF4249@piout.net \
--to=alexandre.belloni@free-electrons.com \
--cc=a.zummo@towertech.it \
--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).