{"id":807255,"url":"http://patchwork.ozlabs.org/api/1.2/patches/807255/?format=json","web_url":"http://patchwork.ozlabs.org/project/rtc-linux/patch/abf5f5ff-b406-eff0-a1ec-9486efbe8506@gmail.com/","project":{"id":9,"url":"http://patchwork.ozlabs.org/api/1.2/projects/9/?format=json","name":"Linux RTC development","link_name":"rtc-linux","list_id":"linux-rtc.vger.kernel.org","list_email":"linux-rtc@vger.kernel.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"","list_archive_url":null,"date":"2017-08-29T19:52:56","name":"[RfC] rtc: ds1307: improve weekday handling","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"8cdf652d9a204538575e4e1bd21cd614168d539d","submitter":{"id":65365,"url":"http://patchwork.ozlabs.org/api/1.2/people/65365/?format=json","name":"Heiner Kallweit","email":"hkallweit1@gmail.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/rtc-linux/patch/abf5f5ff-b406-eff0-a1ec-9486efbe8506@gmail.com/mbox/","series":[{"id":456,"url":"http://patchwork.ozlabs.org/api/1.2/series/456/?format=json","web_url":"http://patchwork.ozlabs.org/project/rtc-linux/list/?series=456","date":"2017-08-29T19:52:56","name":"[RfC] rtc: ds1307: improve weekday handling","version":1,"mbox":"http://patchwork.ozlabs.org/series/456/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/807255/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/807255/checks/","tags":{},"related":[],"headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"YoiKsDKr\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhfSg506yz9sQl\n\tfor ;\n\tWed, 30 Aug 2017 05:53:07 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751280AbdH2TxG (ORCPT );\n\tTue, 29 Aug 2017 15:53:06 -0400","from mail-wm0-f67.google.com ([74.125.82.67]:38614 \"EHLO\n\tmail-wm0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750822AbdH2TxD (ORCPT\n\t); Tue, 29 Aug 2017 15:53:03 -0400","by mail-wm0-f67.google.com with SMTP id u26so4768589wma.5\n\tfor ; Tue, 29 Aug 2017 12:53:02 -0700 (PDT)","from ?IPv6:2003:ea:8bc7:5e00:71b3:e5f3:921d:ea13?\n\t(p200300EA8BC75E0071B3E5F3921DEA13.dip0.t-ipconnect.de.\n\t[2003:ea:8bc7:5e00:71b3:e5f3:921d:ea13])\n\tby smtp.googlemail.com with ESMTPSA id\n\tt12sm2833678wra.20.2017.08.29.12.53.00\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tTue, 29 Aug 2017 12:53:01 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=to:cc:from:subject:message-id:date:user-agent:mime-version\n\t:content-transfer-encoding;\n\tbh=YYMstljRma4wAFDhDf5GS2x+KHNmEFRY3eChWnl49tw=;\n\tb=YoiKsDKryHXCrKngPkpzpVwh26pYsC3G+ZRoBzPZ15XV7LrEgYLO+lKCGa3E+uVJMQ\n\tsTXXSFriz9HtnCT/8O5n1ekQWFDtDRrdOzYQYj7CoeSO3trrYLDf+j0SQIHQsZzXz/DA\n\tzjtj1Z6IBTQP923njc3nz+cT6FV5qOHSBJFy2/a6xhpK+LrOA6/JbdKK+uz2GsooKcGW\n\tatSrEeILsrjc/dPplGV5VaQF5kHTc7luaN1mtvoqkIBZPUQD5HOn6wNdEeaEB5gtYQX6\n\tM6JvFBX6KLb4aRjPAdKKdkEBuKqJ7Vv8i2DYI0gYLdEiyNM55DwPti/GoJ37Bo4ZciyG\n\t0sgQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:to:cc:from:subject:message-id:date:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=YYMstljRma4wAFDhDf5GS2x+KHNmEFRY3eChWnl49tw=;\n\tb=rqfrm2fX2kGHaaNR/hmWEUzTitxoLsCvQ5Bl0VYrHjUXi6+7YhzXWZZkIjNol0YEkN\n\tw7KZhEyocLs8HBoRvGhQ7nNP/ZrOdpVPQiClWZizTU+veYxc2jMYa6Eea6y2tcJ8sMKh\n\tykyhWj4paBuzeM0VrL0a3i4QXL5rKv44Da2vNWl6Wg3WtiFAwldM/Q9AZ27m7sbgXr0S\n\t20DFljBFSuu1bDFKu/GXiKSWbfSKIyo+V92ieu0pYpwjyCQHkZimQnVXoaY0azPl7LJa\n\tgseANEwK4nCpryQT4JXUbRd1fuVMbWxqdwX4lnHXE4TqrligTg0whq/T3fNnt9bvvXYe\n\tCFKA==","X-Gm-Message-State":"AHYfb5j94UlnR9QW3LfH0PXFRWsHvXUoksvHAEe/JighywCyaCT0ejuQ\n\timM9V7vrEZ2ga3XI","X-Received":"by 10.28.151.141 with SMTP id z135mr521972wmd.50.1504036381667; \n\tTue, 29 Aug 2017 12:53:01 -0700 (PDT)","To":"Alexandre Belloni ","Cc":"linux-rtc@vger.kernel.org","From":"Heiner Kallweit ","Subject":"[PATCH RfC] rtc: ds1307: improve weekday handling","Message-ID":"","Date":"Tue, 29 Aug 2017 21:52:56 +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","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"},"content":"The current code for checking and fixing the weekday in ds1307_probe\nfaces some issues:\n- This check is applied to all chips even if its applicable (AFAIK)\n to mcp794xx only\n- The check uses MCP794XX constants for registers and bits even though\n it's executed also on other chips (ok, this could be fixed easily)\n- It relies on tm_wday being properly populated when core calls set_time\n and set_alarm. This is not guaranteed at all.\n\nFirst two issue we could solve by moving the check to the\nmcp794xx-specific initialization (where also VBATEN flag is set).\n\nThe proposed alternative is in the set_alarm path for mcp794xx only and\ncalculates the alarm weekday based on the current weekday in the RTC\ntimekeeping regs and the difference between alarm date and current date.\nSo we are fine with any weekday even if it doesn't match the date.\n\nStill there are cases where this could fail, e.g.:\n- rtc date/time + weekday have power-on-reset default values\n- alarm is set to actual date/time + x\n- set_time is called (may change diff between rtc weekday and actual\n weekday)\n\nBut similar issues we have with the current code too:\n- rtc date/time + weekday have power-on-reset default values\n- alarm is set to rtc date/time + x\n- set_time is called before the alarm triggers\n\nUsing random rtc date/time with relative alarms simply can interfere\nwith set_time. I'm not totally convinced of either option yet.\n\nSigned-off-by: Heiner Kallweit \n---\n drivers/rtc/rtc-ds1307.c | 52 ++++++++++++++++++++++++------------------------\n 1 file changed, 26 insertions(+), 26 deletions(-)","diff":"diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c\nindex 9d680d36..4ba62549 100644\n--- a/drivers/rtc/rtc-ds1307.c\n+++ b/drivers/rtc/rtc-ds1307.c\n@@ -787,8 +787,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, unsigned int enabled)\n * Alarm support for mcp794xx devices.\n */\n \n-#define MCP794XX_REG_WEEKDAY\t\t0x3\n-#define MCP794XX_REG_WEEKDAY_WDAY_MASK\t0x7\n #define MCP794XX_REG_CONTROL\t\t0x07\n #\tdefine MCP794XX_BIT_ALM0_EN\t0x10\n #\tdefine MCP794XX_BIT_ALM1_EN\t0x20\n@@ -877,15 +875,38 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)\n \treturn 0;\n }\n \n+/*\n+ * We may have a random RTC weekday, therefore calculate alarm weekday based\n+ * on current weekday we read from the RTC timekeeping regs\n+ */\n+static int mcp794xx_alm_weekday(struct device *dev, struct rtc_time *tm_alarm)\n+{\n+\tstruct rtc_time tm_now;\n+\tint days_now, days_alarm, ret;\n+\n+\tret = ds1307_get_time(dev, &tm_now);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tdays_now = div_s64(rtc_tm_to_time64(&tm_now), 24 * 60 * 60);\n+\tdays_alarm = div_s64(rtc_tm_to_time64(tm_alarm), 24 * 60 * 60);\n+\n+\treturn (tm_now.tm_wday + days_alarm - days_now) % 7 + 1;\n+}\n+\n static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)\n {\n \tstruct ds1307 *ds1307 = dev_get_drvdata(dev);\n \tunsigned char regs[10];\n-\tint ret;\n+\tint wday, ret;\n \n \tif (!test_bit(HAS_ALARM, &ds1307->flags))\n \t\treturn -EINVAL;\n \n+\twday = mcp794xx_alm_weekday(dev, &t->time);\n+\tif (wday < 0)\n+\t\treturn wday;\n+\n \tdev_dbg(dev, \"%s, sec=%d min=%d hour=%d wday=%d mday=%d mon=%d \"\n \t\t\"enabled=%d pending=%d\\n\", __func__,\n \t\tt->time.tm_sec, t->time.tm_min, t->time.tm_hour,\n@@ -902,7 +923,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] = wday;\n \tregs[7] = bin2bcd(t->time.tm_mday);\n \tregs[8] = bin2bcd(t->time.tm_mon + 1);\n \n@@ -1355,14 +1376,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 +1661,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","prefixes":["RfC"]}