From patchwork Fri Dec 10 20:01:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566633 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=gdDtgUGB; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdM3s1Kz9s3q for ; Sat, 11 Dec 2021 07:02:15 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343937AbhLJUFt (ORCPT ); Fri, 10 Dec 2021 15:05:49 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:46225 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343929AbhLJUFt (ORCPT ); Fri, 10 Dec 2021 15:05:49 -0500 Received: (wp-smtpd smtp.tlen.pl 24446 invoked from network); 10 Dec 2021 21:02:10 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166531; bh=EQeAIwfyIUsx74+P6GrdREQBXO4gNs48bm/LqveNVbY=; h=From:To:Cc:Subject; b=gdDtgUGBjU20fselx3iBdhzVlnyKxDeTQNCQ/ZV/iv4/qSS6rf2hK3B10VYSvNMSi DgqajP4D0ZPOyJrPjDzcjbZO3pGPt1FTro+IROVoYoSPASLqDpf4LWWPJS3JbtYI05 MYmuFkpt9L9x2KBn3OiV0nPFYS+Ai1Dp2eaiXVo4= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:10 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Nobuhiro Iwamatsu , Alessandro Zummo , Alexandre Belloni , stable@vger.kernel.org Subject: [PATCH v4 1/9] rtc-cmos: take rtc_lock while reading from CMOS Date: Fri, 10 Dec 2021 21:01:23 +0100 Message-Id: <20211210200131.153887-2-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 588d400c426e2151a42bdb5fd8eff5c6 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0100001 [QXIz] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Reading from the CMOS involves writing to the index register and then reading from the data register. Therefore access to the CMOS has to be serialized with rtc_lock. This invocation of CMOS_READ was not serialized, which could cause trouble when other code is accessing CMOS at the same time. Use spin_lock_irq() like the rest of the function. Nothing in kernel modifies the RTC_DM_BINARY bit, so there could be a separate pair of spin_lock_irq() / spin_unlock_irq() before doing the math. Signed-off-by: Mateusz Jończyk Reviewed-by: Nobuhiro Iwamatsu Cc: Alessandro Zummo Cc: Alexandre Belloni Cc: stable@vger.kernel.org --- drivers/rtc/rtc-cmos.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index 4eb53412b808..dc3f8b0dde98 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -457,7 +457,10 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) min = t->time.tm_min; sec = t->time.tm_sec; + spin_lock_irq(&rtc_lock); rtc_control = CMOS_READ(RTC_CONTROL); + spin_unlock_irq(&rtc_lock); + if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { /* Writing 0xff means "don't care" or "match all". */ mon = (mon <= 12) ? bin2bcd(mon) : 0xff; From patchwork Fri Dec 10 20:01:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566634 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=Ysaq2tX+; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdR03Dqz9s3q for ; Sat, 11 Dec 2021 07:02:18 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343944AbhLJUFx (ORCPT ); Fri, 10 Dec 2021 15:05:53 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:25628 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343938AbhLJUFx (ORCPT ); Fri, 10 Dec 2021 15:05:53 -0500 Received: (wp-smtpd smtp.tlen.pl 29603 invoked from network); 10 Dec 2021 21:02:13 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166533; bh=BfhIsyR4sRSn7gNQK3PqdNVAm26OvNKnvdZ2c96NVTY=; h=From:To:Cc:Subject; b=Ysaq2tX+4pi0OxCwakOVIyyUgAMWCKXxOig+u1PAjo4AfcNa+B6pYNUJsluAkRH1J FWcGxyE16ypPH5zegubtbHiOtFu0O5Ngjhribo8ltx8UGL2Y/p5tg6Jou4v24AKtMS +QlYUih9uIo5seSLc6BbD1/LJqillDbfOcSSjwmI= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:13 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 2/9] rtc: change return values of mc146818_get_time() Date: Fri, 10 Dec 2021 21:01:24 +0100 Message-Id: <20211210200131.153887-3-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: cc64d9f77310f525e76f7ab7fe795b4a X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [4VOk] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org No function is checking mc146818_get_time() return values yet, so correct them to make them more customary. Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- v4: Added this patch drivers/rtc/rtc-mc146818-lib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index dcfaf09946ee..c186c8c4982b 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -25,7 +25,7 @@ unsigned int mc146818_get_time(struct rtc_time *time) if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x40) != 0)) { spin_unlock_irqrestore(&rtc_lock, flags); memset(time, 0xff, sizeof(*time)); - return 0; + return -EIO; } /* @@ -116,7 +116,7 @@ unsigned int mc146818_get_time(struct rtc_time *time) time->tm_mon--; - return RTC_24H; + return 0; } EXPORT_SYMBOL_GPL(mc146818_get_time); From patchwork Fri Dec 10 20:01:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566635 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=pQXnTTcQ; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdZ2rXfz9s3q for ; Sat, 11 Dec 2021 07:02:26 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343942AbhLJUGA (ORCPT ); Fri, 10 Dec 2021 15:06:00 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:42431 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243153AbhLJUGA (ORCPT ); Fri, 10 Dec 2021 15:06:00 -0500 Received: (wp-smtpd smtp.tlen.pl 9881 invoked from network); 10 Dec 2021 21:02:19 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166540; bh=g6F0PuH16odzLZ9wgJYTYAj2t0sK8w2j7QPAd9umlsc=; h=From:To:Cc:Subject; b=pQXnTTcQAivhhfd4DJ1OwLwbQEhA9VBZtrG+8RVMNrH2CJOqG2wmJaib2u8G9h6SD qVyNp/EGigBLXcYO1qk0ivSgBiE9uoJ6zZ4D36Q9PqZ+27XSCHpSEA+G2PZL28/yEx TdBNmHerm8MlyimqKLKVZkcQ5eIZPheS1YhN5eBo= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:19 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Richard Henderson , Ivan Kokshaysky , Matt Turner , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , Alessandro Zummo , Alexandre Belloni , linux-alpha@vger.kernel.org, x86@kernel.org Subject: [PATCH v4 3/9] Check return value from mc146818_get_time() Date: Fri, 10 Dec 2021 21:01:25 +0100 Message-Id: <20211210200131.153887-4-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 746c4dba93d60ac62f20f4b28fbbafa8 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [ceMk] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org There are 4 users of mc146818_get_time() and none of them was checking the return value from this function. Change this. Print the appropriate warnings in callers of mc146818_get_time() instead of in the function mc146818_get_time() itself, in order not to add strings to rtc-mc146818-lib.c, which is kind of a library. The callers of alpha_rtc_read_time() and cmos_read_time() may use the contents of (struct rtc_time *) even when the functions return a failure code. Therefore, set the contents of (struct rtc_time *) to 0x00, which looks more sensible then 0xff and aligns with the (possibly stale?) comment in cmos_read_time: /* * If pm_trace abused the RTC for storage, set the timespec to 0, * which tells the caller that this RTC value is unusable. */ For consistency, do this in mc146818_get_time(). Note: hpet_rtc_interrupt() may call mc146818_get_time() many times a second. It is very unlikely, though, that the RTC suddenly stops working and mc146818_get_time() would consistently fail. Only compile-tested on alpha. Signed-off-by: Mateusz Jończyk Cc: Richard Henderson Cc: Ivan Kokshaysky Cc: Matt Turner Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: Alessandro Zummo Cc: Alexandre Belloni Cc: linux-alpha@vger.kernel.org Cc: x86@kernel.org --- TODO: would it be appropriate to add attribute __must_check to mc146818_get_time()? v4: Added this patch arch/alpha/kernel/rtc.c | 7 ++++++- arch/x86/kernel/hpet.c | 8 ++++++-- drivers/base/power/trace.c | 6 +++++- drivers/rtc/rtc-cmos.c | 9 ++++++++- drivers/rtc/rtc-mc146818-lib.c | 2 +- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/arch/alpha/kernel/rtc.c b/arch/alpha/kernel/rtc.c index ce3077946e1d..fb3025396ac9 100644 --- a/arch/alpha/kernel/rtc.c +++ b/arch/alpha/kernel/rtc.c @@ -80,7 +80,12 @@ init_rtc_epoch(void) static int alpha_rtc_read_time(struct device *dev, struct rtc_time *tm) { - mc146818_get_time(tm); + int ret = mc146818_get_time(tm); + + if (ret < 0) { + dev_err_ratelimited(dev, "unable to read current time\n"); + return ret; + } /* Adjust for non-default epochs. It's easier to depend on the generic __get_rtc_time and adjust the epoch here than create diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 882213df3713..71f336425e58 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -1435,8 +1435,12 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id) hpet_rtc_timer_reinit(); memset(&curr_time, 0, sizeof(struct rtc_time)); - if (hpet_rtc_flags & (RTC_UIE | RTC_AIE)) - mc146818_get_time(&curr_time); + if (hpet_rtc_flags & (RTC_UIE | RTC_AIE)) { + if (unlikely(mc146818_get_time(&curr_time) < 0)) { + pr_err_ratelimited("unable to read current time from RTC\n"); + return IRQ_HANDLED; + } + } if (hpet_rtc_flags & RTC_UIE && curr_time.tm_sec != hpet_prev_update_sec) { diff --git a/drivers/base/power/trace.c b/drivers/base/power/trace.c index 94665037f4a3..72b7a92337b1 100644 --- a/drivers/base/power/trace.c +++ b/drivers/base/power/trace.c @@ -120,7 +120,11 @@ static unsigned int read_magic_time(void) struct rtc_time time; unsigned int val; - mc146818_get_time(&time); + if (mc146818_get_time(&time) < 0) { + pr_err("Unable to read current time from RTC\n"); + return 0; + } + pr_info("RTC time: %ptRt, date: %ptRd\n", &time, &time); val = time.tm_year; /* 100 years */ if (val > 100) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index dc3f8b0dde98..d0f58cca5c20 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -222,6 +222,8 @@ static inline void cmos_write_bank2(unsigned char val, unsigned char addr) static int cmos_read_time(struct device *dev, struct rtc_time *t) { + int ret; + /* * If pm_trace abused the RTC for storage, set the timespec to 0, * which tells the caller that this RTC value is unusable. @@ -229,7 +231,12 @@ static int cmos_read_time(struct device *dev, struct rtc_time *t) if (!pm_trace_rtc_valid()) return -EIO; - mc146818_get_time(t); + ret = mc146818_get_time(t); + if (ret < 0) { + dev_err_ratelimited(dev, "unable to read current time\n"); + return ret; + } + return 0; } diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index c186c8c4982b..ccd974b8a75a 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -24,7 +24,7 @@ unsigned int mc146818_get_time(struct rtc_time *time) /* Ensure that the RTC is accessible. Bit 6 must be 0! */ if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x40) != 0)) { spin_unlock_irqrestore(&rtc_lock, flags); - memset(time, 0xff, sizeof(*time)); + memset(time, 0, sizeof(*time)); return -EIO; } From patchwork Fri Dec 10 20:01:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566636 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=RY/cPtys; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdj0DWHz9s3q for ; Sat, 11 Dec 2021 07:02:33 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343988AbhLJUGH (ORCPT ); Fri, 10 Dec 2021 15:06:07 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:18512 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343973AbhLJUGC (ORCPT ); Fri, 10 Dec 2021 15:06:02 -0500 Received: (wp-smtpd smtp.tlen.pl 20365 invoked from network); 10 Dec 2021 21:02:24 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166545; bh=yNNQsr8MltNT+tglLaJcjj8fAEirwJ5sSmzehRD8MeM=; h=From:To:Cc:Subject; b=RY/cPtys6cvCGhsYAhkM0ZtpdFwA5t9072Sz7DXg6HzUUfy4tQUpB+eFN/59GJUJq D4t0mwgfbBrTHmFBIM6iJYF4q9Q0MjP4Pz8P+I7A5I5z7lZS4oh40cA6EhleZ0rvDs /oSj3Oc8SdBzUhP0LYmIu0GJtVl4poDauBy+Xl3g= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:24 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Thomas Gleixner , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 4/9] rtc-mc146818-lib: fix RTC presence check Date: Fri, 10 Dec 2021 21:01:26 +0100 Message-Id: <20211210200131.153887-5-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 88da70d397d8cd30d1928b5334146424 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [EVNk] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org To prevent an infinite loop in mc146818_get_time(), commit 211e5db19d15 ("rtc: mc146818: Detect and handle broken RTCs") added a check for RTC availability. Together with a later fix, it checked if bit 6 in register 0x0d is cleared. This, however, caused a false negative on a motherboard with an AMD SB710 southbridge; according to the specification [1], bit 6 of register 0x0d of this chipset is a scratchbit. This caused a regression in Linux 5.11 - the RTC was determined broken by the kernel and not used by rtc-cmos.c [3]. This problem was also reported in Fedora [4]. As a better alternative, check whether the UIP ("Update-in-progress") bit is set for longer then 10ms. If that is the case, then apparently the RTC is either absent (and all register reads return 0xff) or broken. Also limit the number of loop iterations in mc146818_get_time() to 10 to prevent an infinite loop there. The functions mc146818_get_time() and mc146818_does_rtc_work() will be refactored later in this patch series, in order to fix a separate problem with reading / setting the RTC alarm time. This is done so to avoid a confusion about what is being fixed when. In a previous approach to this problem, I implemented a check whether the RTC_HOURS register contains a value <= 24. This, however, sometimes did not work correctly on my Intel Kaby Lake laptop. According to Intel's documentation [2], "the time and date RAM locations (0-9) are disconnected from the external bus" during the update cycle so reading this register without checking the UIP bit is incorrect. [1] AMD SB700/710/750 Register Reference Guide, page 308, https://developer.amd.com/wordpress/media/2012/10/43009_sb7xx_rrg_pub_1.00.pdf [2] 7th Generation Intel ® Processor Family I/O for U/Y Platforms [...] Datasheet Volume 1 of 2, page 209 Intel's Document Number: 334658-006, https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf [3] Functions in arch/x86/kernel/rtc.c apparently were using it. [4] https://bugzilla.redhat.com/show_bug.cgi?id=1936688 Fixes: 211e5db19d15 ("rtc: mc146818: Detect and handle broken RTCs") Fixes: ebb22a059436 ("rtc: mc146818: Dont test for bit 0-5 in Register D") Signed-off-by: Mateusz Jończyk Cc: Thomas Gleixner Cc: Alessandro Zummo Cc: Alexandre Belloni --- v2: - tweak commit description, - remove "Cc: stable" (I'll send it manually after some regression testing). v3: - add "EXPORT_SYMBOL_GPL(mc146818_does_rtc_work)", - change return type from mc146818_does_rtc_work to bool. v4: - drop pr_err_ratelimited() in mc146818_get_time(), callers of this function are now required to notify the user, - tweak commit description. drivers/rtc/rtc-cmos.c | 10 ++++------ drivers/rtc/rtc-mc146818-lib.c | 34 ++++++++++++++++++++++++++++++---- include/linux/mc146818rtc.h | 1 + 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index d0f58cca5c20..b90a603d6b12 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -800,16 +800,14 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) rename_region(ports, dev_name(&cmos_rtc.rtc->dev)); - spin_lock_irq(&rtc_lock); - - /* Ensure that the RTC is accessible. Bit 6 must be 0! */ - if ((CMOS_READ(RTC_VALID) & 0x40) != 0) { - spin_unlock_irq(&rtc_lock); - dev_warn(dev, "not accessible\n"); + if (!mc146818_does_rtc_work()) { + dev_warn(dev, "broken or not accessible\n"); retval = -ENXIO; goto cleanup1; } + spin_lock_irq(&rtc_lock); + if (!(flags & CMOS_RTC_FLAGS_NOFREQ)) { /* force periodic irq to CMOS reset default of 1024Hz; * diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index ccd974b8a75a..d8e67a01220e 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -8,10 +8,36 @@ #include #endif +/* + * If the UIP (Update-in-progress) bit of the RTC is set for more then + * 10ms, the RTC is apparently broken or not present. + */ +bool mc146818_does_rtc_work(void) +{ + int i; + unsigned char val; + unsigned long flags; + + for (i = 0; i < 10; i++) { + spin_lock_irqsave(&rtc_lock, flags); + val = CMOS_READ(RTC_FREQ_SELECT); + spin_unlock_irqrestore(&rtc_lock, flags); + + if ((val & RTC_UIP) == 0) + return true; + + mdelay(1); + } + + return false; +} +EXPORT_SYMBOL_GPL(mc146818_does_rtc_work); + unsigned int mc146818_get_time(struct rtc_time *time) { unsigned char ctrl; unsigned long flags; + unsigned int iter_count = 0; unsigned char century = 0; bool retry; @@ -20,13 +46,13 @@ unsigned int mc146818_get_time(struct rtc_time *time) #endif again: - spin_lock_irqsave(&rtc_lock, flags); - /* Ensure that the RTC is accessible. Bit 6 must be 0! */ - if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x40) != 0)) { - spin_unlock_irqrestore(&rtc_lock, flags); + if (iter_count > 10) { memset(time, 0, sizeof(*time)); return -EIO; } + iter_count++; + + spin_lock_irqsave(&rtc_lock, flags); /* * Check whether there is an update in progress during which the diff --git a/include/linux/mc146818rtc.h b/include/linux/mc146818rtc.h index 0661af17a758..69c80c4325bf 100644 --- a/include/linux/mc146818rtc.h +++ b/include/linux/mc146818rtc.h @@ -123,6 +123,7 @@ struct cmos_rtc_board_info { #define RTC_IO_EXTENT_USED RTC_IO_EXTENT #endif /* ARCH_RTC_LOCATION */ +bool mc146818_does_rtc_work(void); unsigned int mc146818_get_time(struct rtc_time *time); int mc146818_set_time(struct rtc_time *time); From patchwork Fri Dec 10 20:01:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566637 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=dOgzOExt; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdk3jKmz9s3q for ; Sat, 11 Dec 2021 07:02:34 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240266AbhLJUGI (ORCPT ); Fri, 10 Dec 2021 15:06:08 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:30649 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343984AbhLJUGH (ORCPT ); Fri, 10 Dec 2021 15:06:07 -0500 Received: (wp-smtpd smtp.tlen.pl 24384 invoked from network); 10 Dec 2021 21:02:27 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166547; bh=kYD+hTYFSieDk/57cL+/glgb+v5hpzwVdu1Cpp8gPJ4=; h=From:To:Cc:Subject; b=dOgzOExtkA1PUmuJQuv5Oo1BmfuEooqj53OQDj1xhpp7/5O2Z3ZYK6Q1xnLlz+f5M M73SAAu/OoK4GCFnqJbVYuHSovjWW2Ja3/wRy9D8dKyu0ngargzPwE3uuGykBgp5ei wiHYiU4GdUNsW/EDJqB0IBnunoVU337kr8HHET9M= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:27 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 5/9] rtc-mc146818-lib: extract mc146818_avoid_UIP Date: Fri, 10 Dec 2021 21:01:27 +0100 Message-Id: <20211210200131.153887-6-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: ccb8c5fbfb6543a3b79bab8cd21a28b1 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [QeOk] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Function mc146818_get_time() contains an elaborate mechanism of reading the RTC time while no RTC update is in progress. It turns out that reading the RTC alarm clock also requires avoiding the RTC update. Therefore, the mechanism in mc146818_get_time() should be reused - so extract it into a separate function. The logic in mc146818_avoid_UIP() is same as in mc146818_get_time() except that after every if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { there is now "mdelay(1)". To avoid producing a very unreadable patch, mc146818_get_time() will be refactored to use mc146818_avoid_UIP() in the next patch. Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- v4: rename mc146818_do_avoiding_UIP() to mc146818_avoid_UIP(), remove definition of mc146818_callback_t, drivers/rtc/rtc-mc146818-lib.c | 70 ++++++++++++++++++++++++++++++++++ include/linux/mc146818rtc.h | 3 ++ 2 files changed, 73 insertions(+) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index d8e67a01220e..b20f4ebb2f3a 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -8,6 +8,76 @@ #include #endif +/* + * Execute a function while the UIP (Update-in-progress) bit of the RTC is + * unset. + * + * Warning: callback may be executed more then once. + */ +bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param), + void *param) +{ + int i; + unsigned long flags; + unsigned char seconds; + + for (i = 0; i < 10; i++) { + spin_lock_irqsave(&rtc_lock, flags); + + /* + * Check whether there is an update in progress during which the + * readout is unspecified. The maximum update time is ~2ms. Poll + * every msec for completion. + * + * Store the second value before checking UIP so a long lasting + * NMI which happens to hit after the UIP check cannot make + * an update cycle invisible. + */ + seconds = CMOS_READ(RTC_SECONDS); + + if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { + spin_unlock_irqrestore(&rtc_lock, flags); + mdelay(1); + continue; + } + + /* Revalidate the above readout */ + if (seconds != CMOS_READ(RTC_SECONDS)) { + spin_unlock_irqrestore(&rtc_lock, flags); + continue; + } + + if (callback) + callback(seconds, param); + + /* + * Check for the UIP bit again. If it is set now then + * the above values may contain garbage. + */ + if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { + spin_unlock_irqrestore(&rtc_lock, flags); + mdelay(1); + continue; + } + + /* + * A NMI might have interrupted the above sequence so check + * whether the seconds value has changed which indicates that + * the NMI took longer than the UIP bit was set. Unlikely, but + * possible and there is also virt... + */ + if (seconds != CMOS_READ(RTC_SECONDS)) { + spin_unlock_irqrestore(&rtc_lock, flags); + continue; + } + spin_unlock_irqrestore(&rtc_lock, flags); + + return true; + } + return false; +} +EXPORT_SYMBOL_GPL(mc146818_avoid_UIP); + /* * If the UIP (Update-in-progress) bit of the RTC is set for more then * 10ms, the RTC is apparently broken or not present. diff --git a/include/linux/mc146818rtc.h b/include/linux/mc146818rtc.h index 69c80c4325bf..67fb0a12becc 100644 --- a/include/linux/mc146818rtc.h +++ b/include/linux/mc146818rtc.h @@ -127,4 +127,7 @@ bool mc146818_does_rtc_work(void); unsigned int mc146818_get_time(struct rtc_time *time); int mc146818_set_time(struct rtc_time *time); +bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param), + void *param); + #endif /* _MC146818RTC_H */ From patchwork Fri Dec 10 20:01:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566638 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=qxnUntqh; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hdp2Nzdz9s3q for ; Sat, 11 Dec 2021 07:02:38 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240626AbhLJUGM (ORCPT ); Fri, 10 Dec 2021 15:06:12 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:31313 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343976AbhLJUGJ (ORCPT ); Fri, 10 Dec 2021 15:06:09 -0500 Received: (wp-smtpd smtp.tlen.pl 599 invoked from network); 10 Dec 2021 21:02:32 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166552; bh=0HUvyNzB/Qh98ao9Qn1RSPIaWiEazGlrC7sQMYoP7Xw=; h=From:To:Cc:Subject; b=qxnUntqh30cDJuJmwautFkASwQspt6o2RU0MMQ+XE+/t20hAZpB832RKK4Rp8snyF Cpk3/JS99GJIQXCwLlP3FMhsKRXWfFpwqv5BWOC2ByfJ3F3HTs2Tr5YaPy8AX7MdpR t101ik8C/nX2JmgwRzZ/AxYm1O3uouG7stdz7gDo= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:32 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 6/9] rtc-mc146818-lib: refactor mc146818_get_time Date: Fri, 10 Dec 2021 21:01:28 +0100 Message-Id: <20211210200131.153887-7-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 8192778acea93e0ec07c262b1754b881 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [0WME] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Refactor mc146818_get_time() so that it uses mc146818_avoid_UIP(). Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- I'm sorry that the diff is so unreadable, but I was unable to fix this easily. drivers/rtc/rtc-mc146818-lib.c | 109 +++++++++++++-------------------- 1 file changed, 42 insertions(+), 67 deletions(-) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index b20f4ebb2f3a..15604b7f164d 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -103,49 +103,20 @@ bool mc146818_does_rtc_work(void) } EXPORT_SYMBOL_GPL(mc146818_does_rtc_work); -unsigned int mc146818_get_time(struct rtc_time *time) -{ +struct mc146818_get_time_callback_param { + struct rtc_time *time; unsigned char ctrl; - unsigned long flags; - unsigned int iter_count = 0; - unsigned char century = 0; - bool retry; - +#ifdef CONFIG_ACPI + unsigned char century; +#endif #ifdef CONFIG_MACH_DECSTATION unsigned int real_year; #endif +}; -again: - if (iter_count > 10) { - memset(time, 0, sizeof(*time)); - return -EIO; - } - iter_count++; - - spin_lock_irqsave(&rtc_lock, flags); - - /* - * Check whether there is an update in progress during which the - * readout is unspecified. The maximum update time is ~2ms. Poll - * every msec for completion. - * - * Store the second value before checking UIP so a long lasting NMI - * which happens to hit after the UIP check cannot make an update - * cycle invisible. - */ - time->tm_sec = CMOS_READ(RTC_SECONDS); - - if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { - spin_unlock_irqrestore(&rtc_lock, flags); - mdelay(1); - goto again; - } - - /* Revalidate the above readout */ - if (time->tm_sec != CMOS_READ(RTC_SECONDS)) { - spin_unlock_irqrestore(&rtc_lock, flags); - goto again; - } +static void mc146818_get_time_callback(unsigned char seconds, void *param_in) +{ + struct mc146818_get_time_callback_param *p = param_in; /* * Only the values that we read from the RTC are set. We leave @@ -153,39 +124,39 @@ unsigned int mc146818_get_time(struct rtc_time *time) * RTC has RTC_DAY_OF_WEEK, we ignore it, as it is only updated * by the RTC when initially set to a non-zero value. */ - time->tm_min = CMOS_READ(RTC_MINUTES); - time->tm_hour = CMOS_READ(RTC_HOURS); - time->tm_mday = CMOS_READ(RTC_DAY_OF_MONTH); - time->tm_mon = CMOS_READ(RTC_MONTH); - time->tm_year = CMOS_READ(RTC_YEAR); + p->time->tm_sec = seconds; + p->time->tm_min = CMOS_READ(RTC_MINUTES); + p->time->tm_hour = CMOS_READ(RTC_HOURS); + p->time->tm_mday = CMOS_READ(RTC_DAY_OF_MONTH); + p->time->tm_mon = CMOS_READ(RTC_MONTH); + p->time->tm_year = CMOS_READ(RTC_YEAR); #ifdef CONFIG_MACH_DECSTATION - real_year = CMOS_READ(RTC_DEC_YEAR); + p->real_year = CMOS_READ(RTC_DEC_YEAR); #endif #ifdef CONFIG_ACPI if (acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID && - acpi_gbl_FADT.century) - century = CMOS_READ(acpi_gbl_FADT.century); + acpi_gbl_FADT.century) { + p->century = CMOS_READ(acpi_gbl_FADT.century); + } else { + p->century = 0; + } #endif - ctrl = CMOS_READ(RTC_CONTROL); - /* - * Check for the UIP bit again. If it is set now then - * the above values may contain garbage. - */ - retry = CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP; - /* - * A NMI might have interrupted the above sequence so check whether - * the seconds value has changed which indicates that the NMI took - * longer than the UIP bit was set. Unlikely, but possible and - * there is also virt... - */ - retry |= time->tm_sec != CMOS_READ(RTC_SECONDS); - spin_unlock_irqrestore(&rtc_lock, flags); + p->ctrl = CMOS_READ(RTC_CONTROL); +} - if (retry) - goto again; +unsigned int mc146818_get_time(struct rtc_time *time) +{ + struct mc146818_get_time_callback_param p = { + .time = time + }; + + if (!mc146818_avoid_UIP(mc146818_get_time_callback, &p)) { + memset(time, 0, sizeof(*time)); + return -EIO; + } - if (!(ctrl & RTC_DM_BINARY) || RTC_ALWAYS_BCD) + if (!(p.ctrl & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { time->tm_sec = bcd2bin(time->tm_sec); time->tm_min = bcd2bin(time->tm_min); @@ -193,15 +164,19 @@ unsigned int mc146818_get_time(struct rtc_time *time) time->tm_mday = bcd2bin(time->tm_mday); time->tm_mon = bcd2bin(time->tm_mon); time->tm_year = bcd2bin(time->tm_year); - century = bcd2bin(century); +#ifdef CONFIG_ACPI + p.century = bcd2bin(p.century); +#endif } #ifdef CONFIG_MACH_DECSTATION - time->tm_year += real_year - 72; + time->tm_year += p.real_year - 72; #endif - if (century > 20) - time->tm_year += (century - 19) * 100; +#ifdef CONFIG_ACPI + if (p.century > 20) + time->tm_year += (p.century - 19) * 100; +#endif /* * Account for differences between how the RTC uses the values From patchwork Fri Dec 10 20:01:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566639 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=VPwNYdrI; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hds2J94z9s3q for ; Sat, 11 Dec 2021 07:02:41 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240125AbhLJUGO (ORCPT ); Fri, 10 Dec 2021 15:06:14 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:6316 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343999AbhLJUGM (ORCPT ); Fri, 10 Dec 2021 15:06:12 -0500 Received: (wp-smtpd smtp.tlen.pl 4196 invoked from network); 10 Dec 2021 21:02:34 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166554; bh=A0cIBwAehXk+I4pzgSyjkimrribxL4f6aZtLeOI92Ko=; h=From:To:Cc:Subject; b=VPwNYdrId6QZmIYaEM4vFnYjIvRHGpYaWnnpcGmkF9UwXVUHoTsqCO6N4wXE4FO4j 7aA+R13oDrBvZco8PhBOQwUcgCRyBSvPHXGP0dWgyk8+lYI+vyCzgHTfDQs1t855m7 Dy/5T6opQCvzzqCMyXSdrRG9+e0ghw3DtDZ9qSwQ= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:34 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 7/9] rtc-mc146818-lib: refactor mc146818_does_rtc_work Date: Fri, 10 Dec 2021 21:01:29 +0100 Message-Id: <20211210200131.153887-8-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 3dc81ba09cd138f8ab3b941676cdfd7b X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [gQOk] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Refactor mc146818_does_rtc_work() so that it uses mc146818_avoid_UIP(). It is enough to call mc146818_avoid_UIP() with no callback. Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- v4: fixed a mistake in the patch description. drivers/rtc/rtc-mc146818-lib.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index 15604b7f164d..f62e658cbe23 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -84,22 +84,7 @@ EXPORT_SYMBOL_GPL(mc146818_avoid_UIP); */ bool mc146818_does_rtc_work(void) { - int i; - unsigned char val; - unsigned long flags; - - for (i = 0; i < 10; i++) { - spin_lock_irqsave(&rtc_lock, flags); - val = CMOS_READ(RTC_FREQ_SELECT); - spin_unlock_irqrestore(&rtc_lock, flags); - - if ((val & RTC_UIP) == 0) - return true; - - mdelay(1); - } - - return false; + return mc146818_avoid_UIP(NULL, NULL); } EXPORT_SYMBOL_GPL(mc146818_does_rtc_work); From patchwork Fri Dec 10 20:01:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566640 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=Q9b9uJsu; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hf16BVJz9s3q for ; Sat, 11 Dec 2021 07:02:49 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343992AbhLJUGY (ORCPT ); Fri, 10 Dec 2021 15:06:24 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:50920 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344002AbhLJUGQ (ORCPT ); Fri, 10 Dec 2021 15:06:16 -0500 Received: (wp-smtpd smtp.tlen.pl 7788 invoked from network); 10 Dec 2021 21:02:36 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166556; bh=BJO8HI8omQ1xLnjrYmlcM8vojcpUCARscQ9xK3W0xO0=; h=From:To:Cc:Subject; b=Q9b9uJsuzz6GEZCCRw4nUXC9d7uLGaSDDjeCDSvhq58QY0cE60kt8EZj1gZ/lL03T nJ15twmMlyGtJ5wtT3nx0Yt68z4x3C/MBpAP6CvtvXji7tMmlaNkESCJj2/ZX3H8T6 aFK4VnQSXVFeqvZQNks+RnnnKgV2OzdLOR1buir4= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:36 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 8/9] rtc-cmos: avoid UIP when reading alarm time Date: Fri, 10 Dec 2021 21:01:30 +0100 Message-Id: <20211210200131.153887-9-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: e8eb3772b7f1a8641871764c12adf324 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [gdME] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Some Intel chipsets disconnect the time and date RTC registers when the clock update is in progress: during this time reads may return bogus values and writes fail silently. This includes the RTC alarm registers. [1] cmos_read_alarm() did not take account for that, which caused alarm time reads to sometimes return bogus values. This can be shown with a test patch that I am attaching to this patch series. Fix this, by using mc146818_avoid_UIP(). [1] 7th Generation Intel ® Processor Family I/O for U/Y Platforms [...] Datasheet, Volume 1 of 2 (Intel's Document Number: 334658-006) Page 208 https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf "If a RAM read from the ten time and date bytes is attempted during an update cycle, the value read do not necessarily represent the true contents of those locations. Any RAM writes under the same conditions are ignored." Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- v4: fix some checkpatch --strict warnings drivers/rtc/rtc-cmos.c | 72 ++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 23 deletions(-) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index b90a603d6b12..6f47d68d2c86 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -249,10 +249,46 @@ static int cmos_set_time(struct device *dev, struct rtc_time *t) return mc146818_set_time(t); } +struct cmos_read_alarm_callback_param { + struct cmos_rtc *cmos; + struct rtc_time *time; + unsigned char rtc_control; +}; + +static void cmos_read_alarm_callback(unsigned char __always_unused seconds, + void *param_in) +{ + struct cmos_read_alarm_callback_param *p = + (struct cmos_read_alarm_callback_param *)param_in; + struct rtc_time *time = p->time; + + time->tm_sec = CMOS_READ(RTC_SECONDS_ALARM); + time->tm_min = CMOS_READ(RTC_MINUTES_ALARM); + time->tm_hour = CMOS_READ(RTC_HOURS_ALARM); + + if (p->cmos->day_alrm) { + /* ignore upper bits on readback per ACPI spec */ + time->tm_mday = CMOS_READ(p->cmos->day_alrm) & 0x3f; + if (!time->tm_mday) + time->tm_mday = -1; + + if (p->cmos->mon_alrm) { + time->tm_mon = CMOS_READ(p->cmos->mon_alrm); + if (!time->tm_mon) + time->tm_mon = -1; + } + } + + p->rtc_control = CMOS_READ(RTC_CONTROL); +} + static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t) { struct cmos_rtc *cmos = dev_get_drvdata(dev); - unsigned char rtc_control; + struct cmos_read_alarm_callback_param p = { + .cmos = cmos, + .time = &t->time, + }; /* This not only a rtc_op, but also called directly */ if (!is_valid_irq(cmos->irq)) @@ -263,28 +299,18 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t) * the future. */ - spin_lock_irq(&rtc_lock); - t->time.tm_sec = CMOS_READ(RTC_SECONDS_ALARM); - t->time.tm_min = CMOS_READ(RTC_MINUTES_ALARM); - t->time.tm_hour = CMOS_READ(RTC_HOURS_ALARM); - - if (cmos->day_alrm) { - /* ignore upper bits on readback per ACPI spec */ - t->time.tm_mday = CMOS_READ(cmos->day_alrm) & 0x3f; - if (!t->time.tm_mday) - t->time.tm_mday = -1; - - if (cmos->mon_alrm) { - t->time.tm_mon = CMOS_READ(cmos->mon_alrm); - if (!t->time.tm_mon) - t->time.tm_mon = -1; - } - } - - rtc_control = CMOS_READ(RTC_CONTROL); - spin_unlock_irq(&rtc_lock); + /* Some Intel chipsets disconnect the alarm registers when the clock + * update is in progress - during this time reads return bogus values + * and writes may fail silently. See for example "7th Generation Intel® + * Processor Family I/O for U/Y Platforms [...] Datasheet", section + * 27.7.1 + * + * Use the mc146818_avoid_UIP() function to avoid this. + */ + if (!mc146818_avoid_UIP(cmos_read_alarm_callback, &p)) + return -EIO; - if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { + if (!(p.rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { if (((unsigned)t->time.tm_sec) < 0x60) t->time.tm_sec = bcd2bin(t->time.tm_sec); else @@ -313,7 +339,7 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t) } } - t->enabled = !!(rtc_control & RTC_AIE); + t->enabled = !!(p.rtc_control & RTC_AIE); t->pending = 0; return 0; From patchwork Fri Dec 10 20:01:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Mateusz_Jo=C5=84czyk?= X-Patchwork-Id: 1566641 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=o2.pl header.i=@o2.pl header.a=rsa-sha256 header.s=1024a header.b=rhxKd7P7; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-rtc-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4J9hf36DZ7z9s3q for ; Sat, 11 Dec 2021 07:02:51 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344024AbhLJUG0 (ORCPT ); Fri, 10 Dec 2021 15:06:26 -0500 Received: from mx-out.tlen.pl ([193.222.135.142]:23041 "EHLO mx-out.tlen.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344018AbhLJUGT (ORCPT ); Fri, 10 Dec 2021 15:06:19 -0500 Received: (wp-smtpd smtp.tlen.pl 15013 invoked from network); 10 Dec 2021 21:02:40 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=o2.pl; s=1024a; t=1639166560; bh=EVwu6okZda3ea33LLk/febbyBMhJfcnyYZkfcTAZYN8=; h=From:To:Cc:Subject; b=rhxKd7P7gY4uYTtDNIMrXAcGmaQ12zMx4g8k7ccWAuhvkyNba9x364ZzG0G96vY58 m1FWcVAncJJYE7L4H4su6APVpYw3IGkdZ2Lx0DUs9z2102lFP0NYkBA5sbdQ8cyiUl JyFZn9FXvfsC8B5r/cu6CxUCUKUDcuoc23Wdx2oI= Received: from aaff136.neoplus.adsl.tpnet.pl (HELO localhost.localdomain) (mat.jonczyk@o2.pl@[83.4.135.136]) (envelope-sender ) by smtp.tlen.pl (WP-SMTPD) with SMTP for ; 10 Dec 2021 21:02:40 +0100 From: =?utf-8?q?Mateusz_Jo=C5=84czyk?= To: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Mateusz_Jo=C5=84czyk?= , Alessandro Zummo , Alexandre Belloni Subject: [PATCH v4 9/9] rtc-cmos: avoid UIP when writing alarm time Date: Fri, 10 Dec 2021 21:01:31 +0100 Message-Id: <20211210200131.153887-10-mat.jonczyk@o2.pl> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211210200131.153887-1-mat.jonczyk@o2.pl> References: <20211210200131.153887-1-mat.jonczyk@o2.pl> MIME-Version: 1.0 X-WP-MailID: 364add3b291a411200336a107af6cf95 X-WP-AV: skaner antywirusowy Poczty o2 X-WP-SPAM: NO 0000000 [USO0] Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org Some Intel chipsets disconnect the time and date RTC registers when the clock update is in progress: during this time reads may return bogus values and writes fail silently. This includes the RTC alarm registers. [1] cmos_set_alarm() did not take account for that, fix it. [1] 7th Generation Intel ® Processor Family I/O for U/Y Platforms [...] Datasheet, Volume 1 of 2 (Intel's Document Number: 334658-006) Page 208 https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf "If a RAM read from the ten time and date bytes is attempted during an update cycle, the value read do not necessarily represent the true contents of those locations. Any RAM writes under the same conditions are ignored." Signed-off-by: Mateusz Jończyk Cc: Alessandro Zummo Cc: Alexandre Belloni --- v4: fix some checkpatch --strict warnings drivers/rtc/rtc-cmos.c | 107 +++++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 41 deletions(-) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index 6f47d68d2c86..7c006c2b125f 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -470,10 +470,57 @@ static int cmos_validate_alarm(struct device *dev, struct rtc_wkalrm *t) return 0; } +struct cmos_set_alarm_callback_param { + struct cmos_rtc *cmos; + unsigned char mon, mday, hrs, min, sec; + struct rtc_wkalrm *t; +}; + +/* Note: this function may be executed by mc146818_avoid_UIP() more then + * once + */ +static void cmos_set_alarm_callback(unsigned char __always_unused seconds, + void *param_in) +{ + struct cmos_set_alarm_callback_param *p = + (struct cmos_set_alarm_callback_param *)param_in; + + /* next rtc irq must not be from previous alarm setting */ + cmos_irq_disable(p->cmos, RTC_AIE); + + /* update alarm */ + CMOS_WRITE(p->hrs, RTC_HOURS_ALARM); + CMOS_WRITE(p->min, RTC_MINUTES_ALARM); + CMOS_WRITE(p->sec, RTC_SECONDS_ALARM); + + /* the system may support an "enhanced" alarm */ + if (p->cmos->day_alrm) { + CMOS_WRITE(p->mday, p->cmos->day_alrm); + if (p->cmos->mon_alrm) + CMOS_WRITE(p->mon, p->cmos->mon_alrm); + } + + if (use_hpet_alarm()) { + /* + * FIXME the HPET alarm glue currently ignores day_alrm + * and mon_alrm ... + */ + hpet_set_alarm_time(p->t->time.tm_hour, p->t->time.tm_min, + p->t->time.tm_sec); + } + + if (p->t->enabled) + cmos_irq_enable(p->cmos, RTC_AIE); +} + static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) { struct cmos_rtc *cmos = dev_get_drvdata(dev); - unsigned char mon, mday, hrs, min, sec, rtc_control; + struct cmos_set_alarm_callback_param p = { + .cmos = cmos, + .t = t + }; + unsigned char rtc_control; int ret; /* This not only a rtc_op, but also called directly */ @@ -484,11 +531,11 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) if (ret < 0) return ret; - mon = t->time.tm_mon + 1; - mday = t->time.tm_mday; - hrs = t->time.tm_hour; - min = t->time.tm_min; - sec = t->time.tm_sec; + p.mon = t->time.tm_mon + 1; + p.mday = t->time.tm_mday; + p.hrs = t->time.tm_hour; + p.min = t->time.tm_min; + p.sec = t->time.tm_sec; spin_lock_irq(&rtc_lock); rtc_control = CMOS_READ(RTC_CONTROL); @@ -496,43 +543,21 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { /* Writing 0xff means "don't care" or "match all". */ - mon = (mon <= 12) ? bin2bcd(mon) : 0xff; - mday = (mday >= 1 && mday <= 31) ? bin2bcd(mday) : 0xff; - hrs = (hrs < 24) ? bin2bcd(hrs) : 0xff; - min = (min < 60) ? bin2bcd(min) : 0xff; - sec = (sec < 60) ? bin2bcd(sec) : 0xff; - } - - spin_lock_irq(&rtc_lock); - - /* next rtc irq must not be from previous alarm setting */ - cmos_irq_disable(cmos, RTC_AIE); - - /* update alarm */ - CMOS_WRITE(hrs, RTC_HOURS_ALARM); - CMOS_WRITE(min, RTC_MINUTES_ALARM); - CMOS_WRITE(sec, RTC_SECONDS_ALARM); - - /* the system may support an "enhanced" alarm */ - if (cmos->day_alrm) { - CMOS_WRITE(mday, cmos->day_alrm); - if (cmos->mon_alrm) - CMOS_WRITE(mon, cmos->mon_alrm); - } - - if (use_hpet_alarm()) { - /* - * FIXME the HPET alarm glue currently ignores day_alrm - * and mon_alrm ... - */ - hpet_set_alarm_time(t->time.tm_hour, t->time.tm_min, - t->time.tm_sec); + p.mon = (p.mon <= 12) ? bin2bcd(p.mon) : 0xff; + p.mday = (p.mday >= 1 && p.mday <= 31) ? bin2bcd(p.mday) : 0xff; + p.hrs = (p.hrs < 24) ? bin2bcd(p.hrs) : 0xff; + p.min = (p.min < 60) ? bin2bcd(p.min) : 0xff; + p.sec = (p.sec < 60) ? bin2bcd(p.sec) : 0xff; } - if (t->enabled) - cmos_irq_enable(cmos, RTC_AIE); - - spin_unlock_irq(&rtc_lock); + /* + * Some Intel chipsets disconnect the alarm registers when the clock + * update is in progress - during this time writes fail silently. + * + * Use mc146818_avoid_UIP() to avoid this. + */ + if (!mc146818_avoid_UIP(cmos_set_alarm_callback, &p)) + return -EIO; cmos->alarm_expires = rtc_tm_to_time64(&t->time);