[{"id":1758800,"web_url":"http://patchwork.ozlabs.org/comment/1758800/","msgid":"<20170828191518.ofntjb2tknttjexq@piout.net>","list_archive_url":null,"date":"2017-08-28T19:15:18","subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","submitter":{"id":26276,"url":"http://patchwork.ozlabs.org/api/people/26276/","name":"Alexandre Belloni","email":"alexandre.belloni@free-electrons.com"},"content":"On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:\n> mcp794xx as one chip supported by this driver needs the weekday for\n> alarm matching. RTC core ignores the weekday so we can't rely on\n> the values we receive in member tm_wday of struct rtc_time.\n> Therefore calculate the weekday from date/time when setting the\n> time and setting the alarm time for mcp794xx.\n> \n> When having this in place we don't have to check the weekday\n> at each driver load.\n> After a chip reset date/time and weekday may be out of sync but\n> in this case date/time need to be set anyway.\n> \n\nNope, the core issue is that you can actually set an alarm in the future\nwithout setting the time beforehand so this as to be fixed in the probe\n(this was the issue as reported at the time of the fix).\n\n> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>\n> ---\n>  drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------\n>  1 file changed, 15 insertions(+), 24 deletions(-)\n> \n> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c\n> index 9d680d36..83b8c997 100644\n> --- a/drivers/rtc/rtc-ds1307.c\n> +++ b/drivers/rtc/rtc-ds1307.c\n> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)\n>  \treturn rtc_valid_tm(t);\n>  }\n>  \n> +/*\n> + * Certain chips need the weekday for alarm matching and tm->t_wday\n> + * may be not or not properly populated\n> + */\n> +static int ds1307_get_weekday(struct rtc_time *tm)\n> +{\n> +\ttime64_t secs64 = rtc_tm_to_time64(tm);\n> +\tint days = div_s64(secs64, 24 * 60 * 60);\n> +\n> +\treturn (days + 4) % 7 + 1;\n> +}\n> +\n>  static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>  {\n>  \tstruct ds1307\t*ds1307 = dev_get_drvdata(dev);\n> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>  \tregs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);\n>  \tregs[DS1307_REG_MIN] = bin2bcd(t->tm_min);\n>  \tregs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);\n> -\tregs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);\n> +\tregs[DS1307_REG_WDAY] = ds1307_get_weekday(t);\n>  \tregs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);\n>  \tregs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);\n>  \n> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)\n>  \tregs[3] = bin2bcd(t->time.tm_sec);\n>  \tregs[4] = bin2bcd(t->time.tm_min);\n>  \tregs[5] = bin2bcd(t->time.tm_hour);\n> -\tregs[6] = bin2bcd(t->time.tm_wday + 1);\n> +\tregs[6] = ds1307_get_weekday(&t->time);\n>  \tregs[7] = bin2bcd(t->time.tm_mday);\n>  \tregs[8] = bin2bcd(t->time.tm_mon + 1);\n>  \n> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,\n>  {\n>  \tstruct ds1307\t\t*ds1307;\n>  \tint\t\t\terr = -ENODEV;\n> -\tint\t\t\ttmp, wday;\n> +\tint\t\t\ttmp;\n>  \tconst struct chip_desc\t*chip;\n>  \tbool\t\t\twant_irq;\n>  \tbool\t\t\tds1307_can_wakeup_device = false;\n>  \tunsigned char\t\tregs[8];\n>  \tstruct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);\n> -\tstruct rtc_time\t\ttm;\n> -\tunsigned long\t\ttimestamp;\n>  \tu8\t\t\ttrickle_charger_setup = 0;\n>  \n>  \tds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);\n> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,\n>  \t\t\t     bin2bcd(tmp));\n>  \t}\n>  \n> -\t/*\n> -\t * Some IPs have weekday reset value = 0x1 which might not correct\n> -\t * hence compute the wday using the current date/month/year values\n> -\t */\n> -\tds1307_get_time(ds1307->dev, &tm);\n> -\twday = tm.tm_wday;\n> -\ttimestamp = rtc_tm_to_time64(&tm);\n> -\trtc_time64_to_tm(timestamp, &tm);\n> -\n> -\t/*\n> -\t * Check if reset wday is different from the computed wday\n> -\t * If different then set the wday which we computed using\n> -\t * timestamp\n> -\t */\n> -\tif (wday != tm.tm_wday)\n> -\t\tregmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,\n> -\t\t\t\t   MCP794XX_REG_WEEKDAY_WDAY_MASK,\n> -\t\t\t\t   tm.tm_wday + 1);\n> -\n>  \tif (want_irq || ds1307_can_wakeup_device) {\n>  \t\tdevice_set_wakeup_capable(ds1307->dev, true);\n>  \t\tset_bit(HAS_ALARM, &ds1307->flags);\n> -- \n> 2.14.1\n>","headers":{"Return-Path":"<linux-rtc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh1gZ1jNHz9sNc\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 05:15:21 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751213AbdH1TPU (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 15:15:20 -0400","from mail.free-electrons.com ([62.4.15.54]:34479 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751192AbdH1TPU (ORCPT\n\t<rfc822; linux-rtc@vger.kernel.org>); Mon, 28 Aug 2017 15:15:20 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid EED7621D8E; Mon, 28 Aug 2017 21:15:17 +0200 (CEST)","from localhost (unknown [88.191.26.124])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id BE0EB209B5;\n\tMon, 28 Aug 2017 21:15:17 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Mon, 28 Aug 2017 21:15:18 +0200","From":"Alexandre Belloni <alexandre.belloni@free-electrons.com>","To":"Heiner Kallweit <hkallweit1@gmail.com>","Cc":"linux-rtc@vger.kernel.org","Subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","Message-ID":"<20170828191518.ofntjb2tknttjexq@piout.net>","References":"<57598a86-21c7-b354-2840-309563440435@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<57598a86-21c7-b354-2840-309563440435@gmail.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-rtc.vger.kernel.org>","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1758816,"web_url":"http://patchwork.ozlabs.org/comment/1758816/","msgid":"<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","list_archive_url":null,"date":"2017-08-28T19:58:23","subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","submitter":{"id":65365,"url":"http://patchwork.ozlabs.org/api/people/65365/","name":"Heiner Kallweit","email":"hkallweit1@gmail.com"},"content":"Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:\n> On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:\n>> mcp794xx as one chip supported by this driver needs the weekday for\n>> alarm matching. RTC core ignores the weekday so we can't rely on\n>> the values we receive in member tm_wday of struct rtc_time.\n>> Therefore calculate the weekday from date/time when setting the\n>> time and setting the alarm time for mcp794xx.\n>>\n>> When having this in place we don't have to check the weekday\n>> at each driver load.\n>> After a chip reset date/time and weekday may be out of sync but\n>> in this case date/time need to be set anyway.\n>>\n> \n> Nope, the core issue is that you can actually set an alarm in the future\n> without setting the time beforehand so this as to be fixed in the probe\n> (this was the issue as reported at the time of the fix).\n> \nAh, found the original thread where this was discussed.\nStill I don't really understand the problem. mcp794xx matches based on\nsecs, min, hour, wday, mday, month. When I use \"rtcwake -s 5\" I expect\nthe alarm to trigger 5 secs from now. How can this ever work if the\nRTC has random date/time (wday being valid or not)?\n\nI'd tend to say that if we know the RTC time is incorrect we shouldn't\nallow setting an alarm.\nAt least I wouldn't dare to program a bomb explosion time if I stand in\nfront of it and know that the bomb's clock has a random value ;)\n\n>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>\n>> ---\n>>  drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------\n>>  1 file changed, 15 insertions(+), 24 deletions(-)\n>>\n>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c\n>> index 9d680d36..83b8c997 100644\n>> --- a/drivers/rtc/rtc-ds1307.c\n>> +++ b/drivers/rtc/rtc-ds1307.c\n>> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)\n>>  \treturn rtc_valid_tm(t);\n>>  }\n>>  \n>> +/*\n>> + * Certain chips need the weekday for alarm matching and tm->t_wday\n>> + * may be not or not properly populated\n>> + */\n>> +static int ds1307_get_weekday(struct rtc_time *tm)\n>> +{\n>> +\ttime64_t secs64 = rtc_tm_to_time64(tm);\n>> +\tint days = div_s64(secs64, 24 * 60 * 60);\n>> +\n>> +\treturn (days + 4) % 7 + 1;\n>> +}\n>> +\n>>  static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>>  {\n>>  \tstruct ds1307\t*ds1307 = dev_get_drvdata(dev);\n>> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>>  \tregs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);\n>>  \tregs[DS1307_REG_MIN] = bin2bcd(t->tm_min);\n>>  \tregs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);\n>> -\tregs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);\n>> +\tregs[DS1307_REG_WDAY] = ds1307_get_weekday(t);\n>>  \tregs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);\n>>  \tregs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);\n>>  \n>> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)\n>>  \tregs[3] = bin2bcd(t->time.tm_sec);\n>>  \tregs[4] = bin2bcd(t->time.tm_min);\n>>  \tregs[5] = bin2bcd(t->time.tm_hour);\n>> -\tregs[6] = bin2bcd(t->time.tm_wday + 1);\n>> +\tregs[6] = ds1307_get_weekday(&t->time);\n>>  \tregs[7] = bin2bcd(t->time.tm_mday);\n>>  \tregs[8] = bin2bcd(t->time.tm_mon + 1);\n>>  \n>> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,\n>>  {\n>>  \tstruct ds1307\t\t*ds1307;\n>>  \tint\t\t\terr = -ENODEV;\n>> -\tint\t\t\ttmp, wday;\n>> +\tint\t\t\ttmp;\n>>  \tconst struct chip_desc\t*chip;\n>>  \tbool\t\t\twant_irq;\n>>  \tbool\t\t\tds1307_can_wakeup_device = false;\n>>  \tunsigned char\t\tregs[8];\n>>  \tstruct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);\n>> -\tstruct rtc_time\t\ttm;\n>> -\tunsigned long\t\ttimestamp;\n>>  \tu8\t\t\ttrickle_charger_setup = 0;\n>>  \n>>  \tds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);\n>> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,\n>>  \t\t\t     bin2bcd(tmp));\n>>  \t}\n>>  \n>> -\t/*\n>> -\t * Some IPs have weekday reset value = 0x1 which might not correct\n>> -\t * hence compute the wday using the current date/month/year values\n>> -\t */\n>> -\tds1307_get_time(ds1307->dev, &tm);\n>> -\twday = tm.tm_wday;\n>> -\ttimestamp = rtc_tm_to_time64(&tm);\n>> -\trtc_time64_to_tm(timestamp, &tm);\n>> -\n>> -\t/*\n>> -\t * Check if reset wday is different from the computed wday\n>> -\t * If different then set the wday which we computed using\n>> -\t * timestamp\n>> -\t */\n>> -\tif (wday != tm.tm_wday)\n>> -\t\tregmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,\n>> -\t\t\t\t   MCP794XX_REG_WEEKDAY_WDAY_MASK,\n>> -\t\t\t\t   tm.tm_wday + 1);\n>> -\n>>  \tif (want_irq || ds1307_can_wakeup_device) {\n>>  \t\tdevice_set_wakeup_capable(ds1307->dev, true);\n>>  \t\tset_bit(HAS_ALARM, &ds1307->flags);\n>> -- \n>> 2.14.1\n>>\n>","headers":{"Return-Path":"<linux-rtc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"om+eBaS/\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh2dS2rtLz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 05:58:36 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751300AbdH1T6f (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 15:58:35 -0400","from mail-wr0-f195.google.com ([209.85.128.195]:38229 \"EHLO\n\tmail-wr0-f195.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751298AbdH1T6d (ORCPT\n\t<rfc822; linux-rtc@vger.kernel.org>); Mon, 28 Aug 2017 15:58:33 -0400","by mail-wr0-f195.google.com with SMTP id j3so980039wrb.5\n\tfor <linux-rtc@vger.kernel.org>; Mon, 28 Aug 2017 12:58:32 -0700 (PDT)","from ?IPv6:2003:ea:8bc7:5e00:d03d:3b20:7ef4:d434?\n\t(p200300EA8BC75E00D03D3B207EF4D434.dip0.t-ipconnect.de.\n\t[2003:ea:8bc7:5e00:d03d:3b20:7ef4:d434])\n\tby smtp.googlemail.com with ESMTPSA id\n\tf65sm1040836wmd.41.2017.08.28.12.58.30\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 28 Aug 2017 12:58:30 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-transfer-encoding;\n\tbh=V/JZ7VePv88xBJdQC17371/KLVUg//P7oiJoxup0Fw8=;\n\tb=om+eBaS/HeSV2ZnIMAd6jbQB9w6O/Exom6tG7X9/knqAvqxr2YwjKIWIy9TQvOrnWu\n\tgO0UXf4iEtCOhxoimT7BDvytOlpVXf2Bjg4pRj/GyZOMSJDtuBgLcuJvkLiOHYcGoYHY\n\taJH+VHYndmkogzBImFi+u7Wk18CJ99kX/1CnlMuT7++U97q14mfXk9YS8eDQhu6px/bc\n\tJHskj8YowXLK1Mw6f5vMBfsLOUp6ws4dV6/eYa3uRV07IctCZk/+eMEdVGQX1Kp7/0vI\n\t7tQwusK06KEaXCQdO9W1wLprJu/K1QbmTDmdNU7Qtm7Gjd9AKz7wZwuBD6QcILATnW4M\n\tXveQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-transfer-encoding;\n\tbh=V/JZ7VePv88xBJdQC17371/KLVUg//P7oiJoxup0Fw8=;\n\tb=hUBGZsZUe1NVTKVKyR5wS4Qs1/Y01+pZpx83cPeBkgVwtfCseQCrnXKWTgZwTXh4vA\n\ttawTMqXCdfI/dGLz0uNRpUDvut+xBJYeyQvJl1mK2Npy5tltn+uovhxxOS25yVl5iyKV\n\t5OQg7odNBiHcJDNY0nStUetnIMR0KKOGy5bKIDTt5SD+szABDlN2X6eeywe292XxUoVa\n\t7wHKspRqTCjb9nDEtNLP5KHiez3ZxRDPGbTKZdsQgUjol0qiP0/7L1aRoAsBPZobH2RQ\n\tYbMzEu/AjU1EpRkM9fT+difFL6/2wANDNoUjFnfZby4cHNcsm7SIFR6zVOBmSBL2MzFn\n\t//RA==","X-Gm-Message-State":"AHYfb5j8nnzSYY7CBPjb8FKxA0Tnl3Ww3eZIcbZLCNI5K73zly5hJVyp\n\t1UViz0iq8M9TCXna","X-Received":"by 10.223.129.227 with SMTP id 90mr1009809wra.33.1503950311660; \n\tMon, 28 Aug 2017 12:58:31 -0700 (PDT)","Subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","To":"Alexandre Belloni <alexandre.belloni@free-electrons.com>","Cc":"linux-rtc@vger.kernel.org","References":"<57598a86-21c7-b354-2840-309563440435@gmail.com>\n\t<20170828191518.ofntjb2tknttjexq@piout.net>","From":"Heiner Kallweit <hkallweit1@gmail.com>","Message-ID":"<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","Date":"Mon, 28 Aug 2017 21:58:23 +0200","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170828191518.ofntjb2tknttjexq@piout.net>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-rtc.vger.kernel.org>","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1758823,"web_url":"http://patchwork.ozlabs.org/comment/1758823/","msgid":"<c41e959f-dd6f-62f4-86e6-ca9f2b714565@gmail.com>","list_archive_url":null,"date":"2017-08-28T20:12:11","subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","submitter":{"id":65365,"url":"http://patchwork.ozlabs.org/api/people/65365/","name":"Heiner Kallweit","email":"hkallweit1@gmail.com"},"content":"Am 28.08.2017 um 21:58 schrieb Heiner Kallweit:\n> Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:\n>> On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:\n>>> mcp794xx as one chip supported by this driver needs the weekday for\n>>> alarm matching. RTC core ignores the weekday so we can't rely on\n>>> the values we receive in member tm_wday of struct rtc_time.\n>>> Therefore calculate the weekday from date/time when setting the\n>>> time and setting the alarm time for mcp794xx.\n>>>\n>>> When having this in place we don't have to check the weekday\n>>> at each driver load.\n>>> After a chip reset date/time and weekday may be out of sync but\n>>> in this case date/time need to be set anyway.\n>>>\n>>\n>> Nope, the core issue is that you can actually set an alarm in the future\n>> without setting the time beforehand so this as to be fixed in the probe\n>> (this was the issue as reported at the time of the fix).\n>>\n> Ah, found the original thread where this was discussed.\n> Still I don't really understand the problem. mcp794xx matches based on\n> secs, min, hour, wday, mday, month. When I use \"rtcwake -s 5\" I expect\n> the alarm to trigger 5 secs from now. How can this ever work if the\n> RTC has random date/time (wday being valid or not)?\n> \nOK, if \"rtcwake -s 5\" sets the alarm relative to the rtc time, not the\nsys time then it would work. But allowing to use the rtc when it has\na random date / time still seems to be a weird use case to me.\n\n> I'd tend to say that if we know the RTC time is incorrect we shouldn't\n> allow setting an alarm.\n> At least I wouldn't dare to program a bomb explosion time if I stand in\n> front of it and know that the bomb's clock has a random value ;)\n> \n>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>\n>>> ---\n>>>  drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------\n>>>  1 file changed, 15 insertions(+), 24 deletions(-)\n>>>\n>>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c\n>>> index 9d680d36..83b8c997 100644\n>>> --- a/drivers/rtc/rtc-ds1307.c\n>>> +++ b/drivers/rtc/rtc-ds1307.c\n>>> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)\n>>>  \treturn rtc_valid_tm(t);\n>>>  }\n>>>  \n>>> +/*\n>>> + * Certain chips need the weekday for alarm matching and tm->t_wday\n>>> + * may be not or not properly populated\n>>> + */\n>>> +static int ds1307_get_weekday(struct rtc_time *tm)\n>>> +{\n>>> +\ttime64_t secs64 = rtc_tm_to_time64(tm);\n>>> +\tint days = div_s64(secs64, 24 * 60 * 60);\n>>> +\n>>> +\treturn (days + 4) % 7 + 1;\n>>> +}\n>>> +\n>>>  static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>>>  {\n>>>  \tstruct ds1307\t*ds1307 = dev_get_drvdata(dev);\n>>> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)\n>>>  \tregs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);\n>>>  \tregs[DS1307_REG_MIN] = bin2bcd(t->tm_min);\n>>>  \tregs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);\n>>> -\tregs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);\n>>> +\tregs[DS1307_REG_WDAY] = ds1307_get_weekday(t);\n>>>  \tregs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);\n>>>  \tregs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);\n>>>  \n>>> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)\n>>>  \tregs[3] = bin2bcd(t->time.tm_sec);\n>>>  \tregs[4] = bin2bcd(t->time.tm_min);\n>>>  \tregs[5] = bin2bcd(t->time.tm_hour);\n>>> -\tregs[6] = bin2bcd(t->time.tm_wday + 1);\n>>> +\tregs[6] = ds1307_get_weekday(&t->time);\n>>>  \tregs[7] = bin2bcd(t->time.tm_mday);\n>>>  \tregs[8] = bin2bcd(t->time.tm_mon + 1);\n>>>  \n>>> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,\n>>>  {\n>>>  \tstruct ds1307\t\t*ds1307;\n>>>  \tint\t\t\terr = -ENODEV;\n>>> -\tint\t\t\ttmp, wday;\n>>> +\tint\t\t\ttmp;\n>>>  \tconst struct chip_desc\t*chip;\n>>>  \tbool\t\t\twant_irq;\n>>>  \tbool\t\t\tds1307_can_wakeup_device = false;\n>>>  \tunsigned char\t\tregs[8];\n>>>  \tstruct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);\n>>> -\tstruct rtc_time\t\ttm;\n>>> -\tunsigned long\t\ttimestamp;\n>>>  \tu8\t\t\ttrickle_charger_setup = 0;\n>>>  \n>>>  \tds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);\n>>> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,\n>>>  \t\t\t     bin2bcd(tmp));\n>>>  \t}\n>>>  \n>>> -\t/*\n>>> -\t * Some IPs have weekday reset value = 0x1 which might not correct\n>>> -\t * hence compute the wday using the current date/month/year values\n>>> -\t */\n>>> -\tds1307_get_time(ds1307->dev, &tm);\n>>> -\twday = tm.tm_wday;\n>>> -\ttimestamp = rtc_tm_to_time64(&tm);\n>>> -\trtc_time64_to_tm(timestamp, &tm);\n>>> -\n>>> -\t/*\n>>> -\t * Check if reset wday is different from the computed wday\n>>> -\t * If different then set the wday which we computed using\n>>> -\t * timestamp\n>>> -\t */\n>>> -\tif (wday != tm.tm_wday)\n>>> -\t\tregmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,\n>>> -\t\t\t\t   MCP794XX_REG_WEEKDAY_WDAY_MASK,\n>>> -\t\t\t\t   tm.tm_wday + 1);\n>>> -\n>>>  \tif (want_irq || ds1307_can_wakeup_device) {\n>>>  \t\tdevice_set_wakeup_capable(ds1307->dev, true);\n>>>  \t\tset_bit(HAS_ALARM, &ds1307->flags);\n>>> -- \n>>> 2.14.1\n>>>\n>>\n>","headers":{"Return-Path":"<linux-rtc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"HCMi05PH\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh2xK5hQSz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 06:12:21 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751207AbdH1UMU (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 16:12:20 -0400","from mail-wr0-f196.google.com ([209.85.128.196]:35094 \"EHLO\n\tmail-wr0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750841AbdH1UMT (ORCPT\n\t<rfc822; linux-rtc@vger.kernel.org>); Mon, 28 Aug 2017 16:12:19 -0400","by mail-wr0-f196.google.com with SMTP id a47so1011812wra.2\n\tfor <linux-rtc@vger.kernel.org>; Mon, 28 Aug 2017 13:12:19 -0700 (PDT)","from ?IPv6:2003:ea:8bc7:5e00:d03d:3b20:7ef4:d434?\n\t(p200300EA8BC75E00D03D3B207EF4D434.dip0.t-ipconnect.de.\n\t[2003:ea:8bc7:5e00:d03d:3b20:7ef4:d434])\n\tby smtp.googlemail.com with ESMTPSA id\n\tr18sm1686585wrc.44.2017.08.28.13.12.17\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 28 Aug 2017 13:12:17 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=subject:from:to:cc:references:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-transfer-encoding;\n\tbh=Ck+ZSyKtVebJ6rlA+TwTXOPNPpZzoWz+MvQr89SipOk=;\n\tb=HCMi05PHYDAsH8taB7SB/SCEYdSGhD7cJDQs6C4fb+fPAlBUhOqYfbvEy986j9IYws\n\t8SbiyrmW03McHStrmoJAFO34+x09cyx90IzKMOmXPMwQBbMyhbD2x8/4a6Q3NunJXsCK\n\tLdj0Oejp4RWPhGgn4ZFAAI0EMIzgASChDdNzSgZ5LyxW9U2z87L4FbXXmA1yj22eVsoe\n\t7pcaUyHk0fwSsdFUx9Co9shlsTcazqwxlcJHq83eZ6YdNFw3YeGnhj2T0N7lAA11IOcv\n\tEXmrXwASEh0OeczqgXVnz5zeePf+qGMTiez2Tc3CAdJd6pf9AgMmo+0RFPKKOgyaImoL\n\tMIKg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:from:to:cc:references:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-transfer-encoding;\n\tbh=Ck+ZSyKtVebJ6rlA+TwTXOPNPpZzoWz+MvQr89SipOk=;\n\tb=ciPQ+meqnshIB/Zt9rgy3fLBWS28NhowgW3MVnyL1N3a+hg/WH/oAJxascl+RsMqWN\n\tg5fZcUWr8O+JALRuFhVyP3usSSCOnWTZqJ8bUlqm4Xo46prRUS4FdJXqXRykWptwfaem\n\toCuYaeWr+fh2A0Ce+en9ZhHT+SC+A8A6DKEvzIe3seHOW5B7ZSUh9+fYQo3Mur71ckYH\n\t70lqnuRbV5aBXUlasuDhTb2bP67Xlg0oFO8T2Lu3c5vhlUG/RjZYKEs6+c3qedJ1Ed6I\n\tUM1WA6AHZFtiQqty+1CCN4km4b4fuF+El0fdoPlPcrPatI424VwwHM/8iGgjI6NEPlM5\n\t2RPg==","X-Gm-Message-State":"AHYfb5j/R2d3Tyzf6+PJGEDYCsT/a/4tD7nFlSa/PmtJxQHL6av5yOqd\n\t9RcWPnZ7Bak8JP7/","X-Received":"by 10.223.141.135 with SMTP id o7mr1146330wrb.42.1503951138328; \n\tMon, 28 Aug 2017 13:12:18 -0700 (PDT)","Subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","From":"Heiner Kallweit <hkallweit1@gmail.com>","To":"Alexandre Belloni <alexandre.belloni@free-electrons.com>","Cc":"linux-rtc@vger.kernel.org","References":"<57598a86-21c7-b354-2840-309563440435@gmail.com>\n\t<20170828191518.ofntjb2tknttjexq@piout.net>\n\t<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","Message-ID":"<c41e959f-dd6f-62f4-86e6-ca9f2b714565@gmail.com>","Date":"Mon, 28 Aug 2017 22:12:11 +0200","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-rtc.vger.kernel.org>","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1758829,"web_url":"http://patchwork.ozlabs.org/comment/1758829/","msgid":"<20170828202551.p2ngspladxlquboc@piout.net>","list_archive_url":null,"date":"2017-08-28T20:25:51","subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","submitter":{"id":26276,"url":"http://patchwork.ozlabs.org/api/people/26276/","name":"Alexandre Belloni","email":"alexandre.belloni@free-electrons.com"},"content":"On 28/08/2017 at 21:58:23 +0200, Heiner Kallweit wrote:\n> Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:\n> > On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:\n> >> mcp794xx as one chip supported by this driver needs the weekday for\n> >> alarm matching. RTC core ignores the weekday so we can't rely on\n> >> the values we receive in member tm_wday of struct rtc_time.\n> >> Therefore calculate the weekday from date/time when setting the\n> >> time and setting the alarm time for mcp794xx.\n> >>\n> >> When having this in place we don't have to check the weekday\n> >> at each driver load.\n> >> After a chip reset date/time and weekday may be out of sync but\n> >> in this case date/time need to be set anyway.\n> >>\n> > \n> > Nope, the core issue is that you can actually set an alarm in the future\n> > without setting the time beforehand so this as to be fixed in the probe\n> > (this was the issue as reported at the time of the fix).\n> > \n> Ah, found the original thread where this was discussed.\n> Still I don't really understand the problem. mcp794xx matches based on\n> secs, min, hour, wday, mday, month. When I use \"rtcwake -s 5\" I expect\n> the alarm to trigger 5 secs from now. How can this ever work if the\n> RTC has random date/time (wday being valid or not)?\n> \n\nwakealarm_store() reads the rtc time, adds the relative offset and sets\nthe alarm.\n\n> I'd tend to say that if we know the RTC time is incorrect we shouldn't\n> allow setting an alarm.\n\nUnfortunately, this would break some platforms. Note that this is\nalready handled that way by some RTC drivers because they return an\nerror when it is known that the time is invalid.\n\nI've started to work on that but it is far from finished.\n\n> At least I wouldn't dare to program a bomb explosion time if I stand in\n> front of it and know that the bomb's clock has a random value ;)\n> \n\nA timer will work as long as the clock is ticking, it doesn't matter\nwhen it is started ;)","headers":{"Return-Path":"<linux-rtc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xh3F81HcWz9s7p\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 06:26:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750841AbdH1U0D (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 28 Aug 2017 16:26:03 -0400","from mail.free-electrons.com ([62.4.15.54]:35671 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750735AbdH1U0D (ORCPT\n\t<rfc822; linux-rtc@vger.kernel.org>); Mon, 28 Aug 2017 16:26:03 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid D967921E0F; Mon, 28 Aug 2017 22:26:00 +0200 (CEST)","from localhost (unknown [88.191.26.124])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id AABA821DEB;\n\tMon, 28 Aug 2017 22:25:50 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Mon, 28 Aug 2017 22:25:51 +0200","From":"Alexandre Belloni <alexandre.belloni@free-electrons.com>","To":"Heiner Kallweit <hkallweit1@gmail.com>","Cc":"linux-rtc@vger.kernel.org","Subject":"Re: [PATCH] rtc: ds1307: improve weekday handling","Message-ID":"<20170828202551.p2ngspladxlquboc@piout.net>","References":"<57598a86-21c7-b354-2840-309563440435@gmail.com>\n\t<20170828191518.ofntjb2tknttjexq@piout.net>\n\t<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-rtc.vger.kernel.org>","X-Mailing-List":"linux-rtc@vger.kernel.org"}}]