From patchwork Tue Aug 29 19:52:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiner Kallweit X-Patchwork-Id: 807255 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="YoiKsDKr"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3xhfSg506yz9sQl for ; Wed, 30 Aug 2017 05:53:07 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751280AbdH2TxG (ORCPT ); Tue, 29 Aug 2017 15:53:06 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:38614 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750822AbdH2TxD (ORCPT ); Tue, 29 Aug 2017 15:53:03 -0400 Received: by mail-wm0-f67.google.com with SMTP id u26so4768589wma.5 for ; Tue, 29 Aug 2017 12:53:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:cc:from:subject:message-id:date:user-agent:mime-version :content-transfer-encoding; bh=YYMstljRma4wAFDhDf5GS2x+KHNmEFRY3eChWnl49tw=; b=YoiKsDKryHXCrKngPkpzpVwh26pYsC3G+ZRoBzPZ15XV7LrEgYLO+lKCGa3E+uVJMQ sTXXSFriz9HtnCT/8O5n1ekQWFDtDRrdOzYQYj7CoeSO3trrYLDf+j0SQIHQsZzXz/DA zjtj1Z6IBTQP923njc3nz+cT6FV5qOHSBJFy2/a6xhpK+LrOA6/JbdKK+uz2GsooKcGW atSrEeILsrjc/dPplGV5VaQF5kHTc7luaN1mtvoqkIBZPUQD5HOn6wNdEeaEB5gtYQX6 M6JvFBX6KLb4aRjPAdKKdkEBuKqJ7Vv8i2DYI0gYLdEiyNM55DwPti/GoJ37Bo4ZciyG 0sgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:from:subject:message-id:date:user-agent :mime-version:content-transfer-encoding; bh=YYMstljRma4wAFDhDf5GS2x+KHNmEFRY3eChWnl49tw=; b=rqfrm2fX2kGHaaNR/hmWEUzTitxoLsCvQ5Bl0VYrHjUXi6+7YhzXWZZkIjNol0YEkN w7KZhEyocLs8HBoRvGhQ7nNP/ZrOdpVPQiClWZizTU+veYxc2jMYa6Eea6y2tcJ8sMKh ykyhWj4paBuzeM0VrL0a3i4QXL5rKv44Da2vNWl6Wg3WtiFAwldM/Q9AZ27m7sbgXr0S 20DFljBFSuu1bDFKu/GXiKSWbfSKIyo+V92ieu0pYpwjyCQHkZimQnVXoaY0azPl7LJa gseANEwK4nCpryQT4JXUbRd1fuVMbWxqdwX4lnHXE4TqrligTg0whq/T3fNnt9bvvXYe CFKA== X-Gm-Message-State: AHYfb5j94UlnR9QW3LfH0PXFRWsHvXUoksvHAEe/JighywCyaCT0ejuQ imM9V7vrEZ2ga3XI X-Received: by 10.28.151.141 with SMTP id z135mr521972wmd.50.1504036381667; Tue, 29 Aug 2017 12:53:01 -0700 (PDT) Received: from ?IPv6:2003:ea:8bc7:5e00:71b3:e5f3:921d:ea13? (p200300EA8BC75E0071B3E5F3921DEA13.dip0.t-ipconnect.de. [2003:ea:8bc7:5e00:71b3:e5f3:921d:ea13]) by smtp.googlemail.com with ESMTPSA id t12sm2833678wra.20.2017.08.29.12.53.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 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 Thunderbird/52.3.0 MIME-Version: 1.0 Sender: linux-rtc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org The current code for checking and fixing the weekday in ds1307_probe faces some issues: - This check is applied to all chips even if its applicable (AFAIK) to mcp794xx only - The check uses MCP794XX constants for registers and bits even though it's executed also on other chips (ok, this could be fixed easily) - It relies on tm_wday being properly populated when core calls set_time and set_alarm. This is not guaranteed at all. First two issue we could solve by moving the check to the mcp794xx-specific initialization (where also VBATEN flag is set). The proposed alternative is in the set_alarm path for mcp794xx only and calculates the alarm weekday based on the current weekday in the RTC timekeeping regs and the difference between alarm date and current date. So we are fine with any weekday even if it doesn't match the date. Still there are cases where this could fail, e.g.: - rtc date/time + weekday have power-on-reset default values - alarm is set to actual date/time + x - set_time is called (may change diff between rtc weekday and actual weekday) But similar issues we have with the current code too: - rtc date/time + weekday have power-on-reset default values - alarm is set to rtc date/time + x - set_time is called before the alarm triggers Using random rtc date/time with relative alarms simply can interfere with set_time. I'm not totally convinced of either option yet. Signed-off-by: Heiner Kallweit --- drivers/rtc/rtc-ds1307.c | 52 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 9d680d36..4ba62549 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -787,8 +787,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, unsigned int enabled) * 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 @@ -877,15 +875,38 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t) return 0; } +/* + * We may have a random RTC weekday, therefore calculate alarm weekday based + * on current weekday we read from the RTC timekeeping regs + */ +static int mcp794xx_alm_weekday(struct device *dev, struct rtc_time *tm_alarm) +{ + struct rtc_time tm_now; + int days_now, days_alarm, ret; + + ret = ds1307_get_time(dev, &tm_now); + if (ret) + return ret; + + days_now = div_s64(rtc_tm_to_time64(&tm_now), 24 * 60 * 60); + days_alarm = div_s64(rtc_tm_to_time64(tm_alarm), 24 * 60 * 60); + + return (tm_now.tm_wday + days_alarm - days_now) % 7 + 1; +} + static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t) { struct ds1307 *ds1307 = dev_get_drvdata(dev); unsigned char regs[10]; - int ret; + int wday, ret; if (!test_bit(HAS_ALARM, &ds1307->flags)) return -EINVAL; + wday = mcp794xx_alm_weekday(dev, &t->time); + if (wday < 0) + return wday; + dev_dbg(dev, "%s, sec=%d min=%d hour=%d wday=%d mday=%d mon=%d " "enabled=%d pending=%d\n", __func__, t->time.tm_sec, t->time.tm_min, t->time.tm_hour, @@ -902,7 +923,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t) regs[3] = bin2bcd(t->time.tm_sec); regs[4] = bin2bcd(t->time.tm_min); regs[5] = bin2bcd(t->time.tm_hour); - regs[6] = bin2bcd(t->time.tm_wday + 1); + regs[6] = wday; regs[7] = bin2bcd(t->time.tm_mday); regs[8] = bin2bcd(t->time.tm_mon + 1); @@ -1355,14 +1376,12 @@ static int ds1307_probe(struct i2c_client *client, { struct ds1307 *ds1307; int err = -ENODEV; - int tmp, wday; + int tmp; const struct chip_desc *chip; bool want_irq; bool ds1307_can_wakeup_device = false; unsigned char regs[8]; struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev); - struct rtc_time tm; - unsigned long timestamp; u8 trickle_charger_setup = 0; ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL); @@ -1642,25 +1661,6 @@ static int ds1307_probe(struct i2c_client *client, 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(ds1307->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) - regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY, - MCP794XX_REG_WEEKDAY_WDAY_MASK, - tm.tm_wday + 1); - if (want_irq || ds1307_can_wakeup_device) { device_set_wakeup_capable(ds1307->dev, true); set_bit(HAS_ALARM, &ds1307->flags);