From patchwork Fri Dec 2 19:50:57 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Fox X-Patchwork-Id: 128948 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail-qy0-f184.google.com (mail-qy0-f184.google.com [209.85.216.184]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id B6565B6F18 for ; Sat, 3 Dec 2011 06:51:02 +1100 (EST) Received: by qcsr22 with SMTP id r22sf705176qcs.11 for ; Fri, 02 Dec 2011 11:50:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=beta; h=x-beenthere:received-spf:to:subject:from:mime-version:date :message-id:x-original-sender:x-original-authentication-results :reply-to:precedence:mailing-list:list-id:x-google-group-id :list-post:list-help:list-archive:sender:list-subscribe :list-unsubscribe:content-type:content-id; bh=FO4Y/I2zCFE2pwKvPQS0UQkul1RXCKnMfMzS52gUxvw=; b=NvDuj8058BPan+XoeN6u/YPY4h05kQ5/pPMrJGnFMvdu8qGQ0+gXQ2TBTqwvneljAb EL3Y/N9hYpMcqEpUzOb8/k0gvDhnelyDwM5D4NZxULrTIBPmh+LMLLQUEotjFIwO7Z5s 02ZpyS9VMm7n6iP0MueHbBZDoTwpkWsDy6cSM= Received: by 10.229.240.199 with SMTP id lb7mr123849qcb.9.1322855458221; Fri, 02 Dec 2011 11:50:58 -0800 (PST) X-BeenThere: rtc-linux@googlegroups.com Received: by 10.224.209.134 with SMTP id gg6ls2524504qab.2.gmail; Fri, 02 Dec 2011 11:50:57 -0800 (PST) Received: by 10.224.109.10 with SMTP id h10mr5095372qap.5.1322855457824; Fri, 02 Dec 2011 11:50:57 -0800 (PST) Received: by 10.224.109.10 with SMTP id h10mr5095370qap.5.1322855457812; Fri, 02 Dec 2011 11:50:57 -0800 (PST) Received: from colo.foxharp.net (colo.foxharp.net. [166.84.7.52]) by gmr-mx.google.com with ESMTP id g14si5484174qcy.2.2011.12.02.11.50.57; Fri, 02 Dec 2011 11:50:57 -0800 (PST) Received-SPF: neutral (google.com: 166.84.7.52 is neither permitted nor denied by best guess record for domain of pgf@laptop.org) client-ip=166.84.7.52; Received: from grass.foxharp.boston.ma.us (localhost [127.0.0.1]) by colo.foxharp.net (Postfix) with ESMTP id 806FE540B8 for ; Fri, 2 Dec 2011 14:50:54 -0500 (EST) Received: by grass.foxharp.boston.ma.us (Postfix, from userid 406) id 22A182E80AB; Fri, 2 Dec 2011 14:50:57 -0500 (EST) Received: from foxharp.boston.ma.us (localhost [127.0.0.1]) by grass.foxharp.boston.ma.us (Postfix) with ESMTP id 1D8612E80AA for ; Fri, 2 Dec 2011 14:50:57 -0500 (EST) To: rtc-linux@googlegroups.com Subject: [rtc-linux] [RFC] timely RTC wakeup events on the MMP2 RTC From: Paul Fox MIME-Version: 1.0 Date: Fri, 02 Dec 2011 14:50:57 -0500 Message-ID: <14101.1322855457@foxharp.boston.ma.us> X-Original-Sender: pgf@laptop.org X-Original-Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 166.84.7.52 is neither permitted nor denied by best guess record for domain of pgf@laptop.org) smtp.mail=pgf@laptop.org Reply-To: rtc-linux@googlegroups.com Precedence: list Mailing-list: list rtc-linux@googlegroups.com; contact rtc-linux+owners@googlegroups.com List-ID: X-Google-Group-Id: 712029733259 List-Post: , List-Help: , List-Archive: Sender: rtc-linux@googlegroups.com List-Subscribe: , List-Unsubscribe: , Content-ID: <14098.1322855457.1@foxharp.boston.ma.us> this message is an RFC, to see if i'm on the right track. the patch below will clearly be affected by the cleanup patch stream on this driver that's currently in progress, so consider it a work-in-progress. (also, i don't have a lot of experience with the RTC subsystem, so bear with me.) on the OLPC XO-1.75 laptop (MMP2-based), we're using the linux 3.0 /power/wakeup_count facilities to help identify the primary cause of a system wakeup, as well as the /sys/power/wakeup_count facility to prevent wakeup/suspend races. to that end, i've been adding calls to pm_wake_event to our drivers of interest. to be clear, for the RTC driver, this looks like: [if i've already gone wrong at this point, the rest of this message may be moot. i'm somewhat surprised that we're the first platform to need pm_wakeup_event() functionality in an RTC, but i haven't seen any other support for this in the rtc subsystem.] Author: Paul Fox Date: Thu Nov 10 17:27:10 2011 -0500 generate a pm_wakeup when we get an RTC alarm interrupt this will cause them to be counted, and the count can then be used to determine wakeup source. diff --git a/drivers/rtc/rtc-mmp.c b/drivers/rtc/rtc-mmp.c index 65e67cb..2841bf5 100755 --- a/drivers/rtc/rtc-mmp.c +++ b/drivers/rtc/rtc-mmp.c @@ -144,6 +144,8 @@ static irqreturn_t mmp_rtc_interrupt(int irq, void *dev_id) spin_unlock(&mmp_rtc_lock); + pm_wakeup_event(&pdev->dev, 1000); + return IRQ_HANDLED; } it turns out, though, that this doesn't always work. because the driver enables and disables interrupts in the open/close routines, no interrupt can ever occur unless someone has the device open. furthermore, interrupts that would have occurred (after device close) are delayed until someone next opens the device, usually long after the actual alarm time. wakeups, of course, do occur correctly, because those aren't affected by whether the OS is actually going to handle the interrupt. the specific symptom scenario we see is this: rtcwake is used to schedule a wakeup sometime in the future. before that wakeup has a chance to happen, something else (e.g., a keystroke) wakes the system. rtcwake exits, leaving the alarm scheduled. some time later that alarm fires, and the irq is disabled, so the interrupt isn't handled. time passes, and rtcwake is again invoked, to put the system to sleep. as soon as the device is opened the RTC's pending interrupt is finally handled. the handler runs, and calls pm_wakeup_event() -- far too late, of course. rtcwake continues, sets a new alarm, and attempts to put the system to sleep. but since the /sys/power/wakeup_count suspend protocol is in effect, the kernel aborts the suspend, since there's already a pending wakeup event. one solution would be to try to cancel the pending alarm when the system wakes up, but that seems racy, and not necessarily what the user wanted. it seems far simpler just to let the alarm interrupt happen. the following patch does this. the open and close routines go away entirely, because there's no longer anything for them to do. comments? paul (much of this patch -- all of the last two hunks -- is simply to let pending alarms be cancelled during device probe. that's really just a safety/paranoia thing, and unrelated to my questions above.) [PATCH] eliminate delayed RTC interrupts from the MMP2 RTC don't enable/disable interrupts on every device open/close -- just leave them enabled, and take interrupts as they occur. otherwise alarms which expire while the device is closed will cause the handler to run at the time of the next open. this in turn will cause a wakeup event, which may confuse PM wakeup event processing. the probe routine is rearranged so hardware RTC interrupt source(s) can be cleared before enabling interrupts. Signed-off-by: Paul Fox --- drivers/rtc/rtc-mmp.c | 66 +++++++++++++++++------------------------------- 1 files changed, 24 insertions(+), 42 deletions(-) diff --git a/drivers/rtc/rtc-mmp.c b/drivers/rtc/rtc-mmp.c index 2841bf5..8e20aad 100755 --- a/drivers/rtc/rtc-mmp.c +++ b/drivers/rtc/rtc-mmp.c @@ -1,5 +1,5 @@ /* - * Real Time Clock interface for StrongARM SA1x00 and XScale PXA2xx + * Real Time Clock interface for Marvell MMP2 * * Copyright (c) 2000 Nils Faerber * @@ -16,7 +16,8 @@ * by Richard Purdie * * Support RTC on Marvell MMP - * by Bin Yang + * Bin Yang + * Paul Fox * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -149,24 +150,6 @@ static irqreturn_t mmp_rtc_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } -static int mmp_rtc_open(struct device *dev) -{ - spin_lock_irq(&mmp_rtc_lock); - enable_irq(irq_1hz); - enable_irq(irq_alrm); - spin_unlock_irq(&mmp_rtc_lock); - return 0; -} - -static void mmp_rtc_release(struct device *dev) -{ - spin_lock_irq(&mmp_rtc_lock); - disable_irq(irq_1hz); - disable_irq(irq_alrm); - spin_unlock_irq(&mmp_rtc_lock); -} - - static int mmp_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg) { @@ -276,8 +259,6 @@ static int mmp_rtc_proc(struct device *dev, struct seq_file *seq) } static const struct rtc_class_ops mmp_rtc_ops = { - .open = mmp_rtc_open, - .release = mmp_rtc_release, .ioctl = mmp_rtc_ioctl, .read_time = mmp_rtc_read_time, .set_time = mmp_rtc_set_time, @@ -312,26 +293,6 @@ static int mmp_rtc_probe(struct platform_device *pdev) } clk_enable(clk); - ret = request_irq(irq_1hz, mmp_rtc_interrupt, IRQF_DISABLED, - "rtc 1Hz", dev); - if (ret) { - dev_err(dev, "IRQ %d already in use.\n", irq_1hz); - irq_1hz = irq_alrm = -1; - ret = -ENXIO; - goto err; - } - disable_irq(irq_1hz); - - ret = request_irq(irq_alrm, mmp_rtc_interrupt, IRQF_DISABLED, - "rtc Alrm", dev); - if (ret) { - dev_err(dev, "IRQ %d already in use.\n", irq_alrm); - irq_alrm = -1; - ret = -ENXIO; - goto err; - } - disable_irq(irq_alrm); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res == NULL) { dev_err(&pdev->dev, "failed to get I/O memory\n"); @@ -353,6 +314,27 @@ static int mmp_rtc_probe(struct platform_device *pdev) goto err; } + /* Clear any pending interrupts */ + RTSR = RTSR_AL | RTSR_HZ; + + ret = request_irq(irq_1hz, mmp_rtc_interrupt, IRQF_DISABLED, + "rtc 1Hz", dev); + if (ret) { + dev_err(dev, "IRQ %d already in use.\n", irq_1hz); + irq_1hz = irq_alrm = -1; + ret = -ENXIO; + goto err; + } + + ret = request_irq(irq_alrm, mmp_rtc_interrupt, IRQF_DISABLED, + "rtc Alrm", dev); + if (ret) { + dev_err(dev, "IRQ %d already in use.\n", irq_alrm); + irq_alrm = -1; + ret = -ENXIO; + goto err; + } + /* * According to the manual we should be able to let RTTR be zero * and then a default diviser for a 32.768KHz clock is used.