From patchwork Tue Feb 1 16:30:20 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Roberto Jimenez X-Patchwork-Id: 81333 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail-yx0-f184.google.com (mail-yx0-f184.google.com [209.85.213.184]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id 05B3CB70F4 for ; Wed, 2 Feb 2011 03:30:47 +1100 (EST) Received: by yxk8 with SMTP id 8sf4266967yxk.11 for ; Tue, 01 Feb 2011 08:30:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=beta; h=domainkey-signature:mime-version:x-beenthere:received-spf:from:to :cc:subject:date:message-id:x-mailer:x-original-sender :x-original-authentication-results:reply-to:precedence:mailing-list :list-id:list-post:list-help:list-archive:sender:list-subscribe :list-unsubscribe:content-type; bh=CY8EkxlavpkeOrYwIIxP55uWQuecnk75ODoKTtreO/I=; b=x+DP5qRLS8ZXpl7a1npsGVVsnAKKQ8Fehk+400ivF4YzrMgZIt9eky1iwBGGlBkN4b NkW4NWLstjlqFHqmNNdKUzl/QGDGlSaf1FwmIA/iVGBArO8jbPrz+T/Fle/ISPnPHKZ0 q+M3u294RJfpvQ5XX4bjRDSJ/uBVAMDAjGP+E= DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlegroups.com; s=beta; h=mime-version:x-beenthere:received-spf:from:to:cc:subject:date :message-id:x-mailer:x-original-sender :x-original-authentication-results:reply-to:precedence:mailing-list :list-id:list-post:list-help:list-archive:sender:list-subscribe :list-unsubscribe:content-type; b=MYMv6UhVNZ8BzeWwlcNq1tqtOPfjFE2jF2C0XQ4TqZi/pMqdHSvXwQ3xzEi9nFRs5t 5+GXxKWPjP0/Ngzhv0l2XGWyrXz5eliXwbSTw+4rKjmrrPpd4RpGMjqBXgZugAD2uK9/ VzfPBl92bB6IzqgeEXWZJWMKqJtAt25+p27Cg= Received: by 10.236.111.35 with SMTP id v23mr593769yhg.21.1296577844030; Tue, 01 Feb 2011 08:30:44 -0800 (PST) MIME-Version: 1.0 X-BeenThere: rtc-linux@googlegroups.com Received: by 10.150.31.5 with SMTP id e5ls1282709ybe.2.p; Tue, 01 Feb 2011 08:30:43 -0800 (PST) Received: by 10.151.45.2 with SMTP id x2mr1552343ybj.21.1296577843405; Tue, 01 Feb 2011 08:30:43 -0800 (PST) Received: by 10.151.45.2 with SMTP id x2mr1552342ybj.21.1296577843367; Tue, 01 Feb 2011 08:30:43 -0800 (PST) Received: from mail-yi0-f43.google.com (mail-yi0-f43.google.com [209.85.218.43]) by gmr-mx.google.com with ESMTPS id q2si3375070ybe.0.2011.02.01.08.30.43 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 01 Feb 2011 08:30:43 -0800 (PST) Received-SPF: neutral (google.com: 209.85.218.43 is neither permitted nor denied by best guess record for domain of mroberto@cpti.cetuc.puc-rio.br) client-ip=209.85.218.43; Received: by yie16 with SMTP id 16so3075544yie.2 for ; Tue, 01 Feb 2011 08:30:43 -0800 (PST) Received: by 10.150.53.12 with SMTP id b12mr2283437yba.189.1296577842583; Tue, 01 Feb 2011 08:30:42 -0800 (PST) Received: from localhost.localdomain ([139.82.178.36]) by mx.google.com with ESMTPS id q4sm2769614yba.14.2011.02.01.08.30.39 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 01 Feb 2011 08:30:41 -0800 (PST) From: Marcelo Roberto Jimenez To: mroberto@cpti.cetuc.puc-rio.br, a.zummo@towertech.it, john.stultz@linaro.org Cc: rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org Subject: [rtc-linux] [PATCH] RTC: Fix for issues in the kernel RTC API. Date: Tue, 1 Feb 2011 14:30:20 -0200 Message-Id: <1296577820-27456-1-git-send-email-mroberto@cpti.cetuc.puc-rio.br> X-Mailer: git-send-email 1.7.3.4 X-Original-Sender: mroberto@cpti.cetuc.puc-rio.br X-Original-Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 209.85.218.43 is neither permitted nor denied by best guess record for domain of mroberto@cpti.cetuc.puc-rio.br) smtp.mail=mroberto@cpti.cetuc.puc-rio.br Reply-To: rtc-linux@googlegroups.com Precedence: list Mailing-list: list rtc-linux@googlegroups.com; contact rtc-linux+owners@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: Sender: rtc-linux@googlegroups.com List-Subscribe: , List-Unsubscribe: , This patch fixes the following issues with the RTC API: 1 - The alarm_irq_enable() and the update_irq_enable() callbacks were not beeing called anywhere in the code. They are necessary if you want to write a driver that does not use the kernel pre-existent timer implemented alarm and update interrupts. 2 - In the current configuration, an update interrupt, for example, would be a trigger for an alarm interrupt as well, even though the mode variable stated it was an update interrupt. To avoid that, mode is beeing checked inside rtc_handle_legacy_irq(). That used to be the previous user space behaviour. 3 - The irq_set_freq() and rtc_irq_set_state() functions are entry points to the RTC API, and were not calling the irq_set_state() and irq_set_freq() callbacks. These are also necessary overrides to provide a proper independent RTC implementation. 4 - The rtc-test kernel RTC driver has been updated to work properly with the new kernel RTC timer infrastructure. This driver has been used together with the rtctest.c source code present in the file Documentation/rtc.txt to test the user space behaviour of all the modifications in this patch. Signed-off-by: Marcelo Roberto Jimenez --- drivers/rtc/interface.c | 63 +++++++++++++++++++++++++++++--------- drivers/rtc/rtc-dev.c | 12 ++++---- drivers/rtc/rtc-test.c | 77 ++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 121 insertions(+), 31 deletions(-) diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 90384b9..6cb4b65 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -194,6 +194,11 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled) if (err) return err; + if (rtc->ops->alarm_irq_enable) { + err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled); + goto out; + } + if (rtc->aie_timer.enabled != enabled) { if (enabled) { rtc->aie_timer.enabled = 1; @@ -211,6 +216,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled) else err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled); +out: mutex_unlock(&rtc->ops_lock); return err; } @@ -222,6 +228,11 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled) if (err) return err; + if (rtc->ops->update_irq_enable) { + err = rtc->ops->update_irq_enable(rtc->dev.parent, enabled); + goto out; + } + /* make sure we're changing state */ if (rtc->uie_rtctimer.enabled == enabled) goto out; @@ -263,19 +274,23 @@ static void rtc_handle_legacy_irq(struct rtc_device *rtc, int num, int mode) { unsigned long flags; - /* mark one irq of the appropriate mode */ - spin_lock_irqsave(&rtc->irq_lock, flags); - rtc->irq_data = (rtc->irq_data + (num << 8)) | (RTC_IRQF|mode); - spin_unlock_irqrestore(&rtc->irq_lock, flags); - - /* call the task func */ - spin_lock_irqsave(&rtc->irq_task_lock, flags); - if (rtc->irq_task) - rtc->irq_task->func(rtc->irq_task->private_data); - spin_unlock_irqrestore(&rtc->irq_task_lock, flags); - - wake_up_interruptible(&rtc->irq_queue); - kill_fasync(&rtc->async_queue, SIGIO, POLL_IN); + if (((mode & RTC_UF) && rtc->uie_rtctimer.enabled) || + ((mode & RTC_AF) && rtc->aie_timer.enabled) || + ((mode & RTC_PF) && rtc->pie_enabled)) { + /* mark one irq of the appropriate mode */ + spin_lock_irqsave(&rtc->irq_lock, flags); + rtc->irq_data = (rtc->irq_data + (num << 8)) | (RTC_IRQF|mode); + spin_unlock_irqrestore(&rtc->irq_lock, flags); + + /* call the task func */ + spin_lock_irqsave(&rtc->irq_task_lock, flags); + if (rtc->irq_task) + rtc->irq_task->func(rtc->irq_task->private_data); + spin_unlock_irqrestore(&rtc->irq_task_lock, flags); + + wake_up_interruptible(&rtc->irq_queue); + kill_fasync(&rtc->async_queue, SIGIO, POLL_IN); + } } @@ -423,9 +438,18 @@ EXPORT_SYMBOL_GPL(rtc_irq_unregister); */ int rtc_irq_set_state(struct rtc_device *rtc, struct rtc_task *task, int enabled) { - int err = 0; unsigned long flags; + int err = mutex_lock_interruptible(&rtc->ops_lock); + if (err) + return err; + if (rtc->ops->irq_set_state) { + err = rtc->ops->irq_set_state(rtc->dev.parent, enabled); + mutex_unlock(&rtc->ops_lock); + return err; + } + mutex_unlock(&rtc->ops_lock); + spin_lock_irqsave(&rtc->irq_task_lock, flags); if (rtc->irq_task != NULL && task == NULL) err = -EBUSY; @@ -457,9 +481,18 @@ EXPORT_SYMBOL_GPL(rtc_irq_set_state); */ int rtc_irq_set_freq(struct rtc_device *rtc, struct rtc_task *task, int freq) { - int err = 0; unsigned long flags; + int err = mutex_lock_interruptible(&rtc->ops_lock); + if (err) + return err; + if (rtc->ops->irq_set_freq) { + err = rtc->ops->irq_set_freq(rtc->dev.parent, freq); + mutex_unlock(&rtc->ops_lock); + return err; + } + mutex_unlock(&rtc->ops_lock); + spin_lock_irqsave(&rtc->irq_task_lock, flags); if (rtc->irq_task != NULL && task == NULL) err = -EBUSY; diff --git a/drivers/rtc/rtc-dev.c b/drivers/rtc/rtc-dev.c index 212b16e..d901160 100644 --- a/drivers/rtc/rtc-dev.c +++ b/drivers/rtc/rtc-dev.c @@ -261,12 +261,12 @@ static long rtc_dev_ioctl(struct file *file, return rtc_set_time(rtc, &tm); case RTC_PIE_ON: - err = rtc_irq_set_state(rtc, NULL, 1); - break; + mutex_unlock(&rtc->ops_lock); + return rtc_irq_set_state(rtc, NULL, 1); case RTC_PIE_OFF: - err = rtc_irq_set_state(rtc, NULL, 0); - break; + mutex_unlock(&rtc->ops_lock); + return rtc_irq_set_state(rtc, NULL, 0); case RTC_AIE_ON: mutex_unlock(&rtc->ops_lock); @@ -285,8 +285,8 @@ static long rtc_dev_ioctl(struct file *file, return rtc_update_irq_enable(rtc, 0); case RTC_IRQP_SET: - err = rtc_irq_set_freq(rtc, NULL, arg); - break; + mutex_unlock(&rtc->ops_lock); + return rtc_irq_set_freq(rtc, NULL, arg); case RTC_IRQP_READ: err = put_user(rtc->irq_freq, (unsigned long __user *)uarg); diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c index 51725f7..7cbe3e3 100644 --- a/drivers/rtc/rtc-test.c +++ b/drivers/rtc/rtc-test.c @@ -13,6 +13,8 @@ #include #include +static const unsigned long RTC_DEFAULT_PERIODIC_FREQ = 1024; + static struct platform_device *test0 = NULL, *test1 = NULL; static int test_rtc_read_alarm(struct device *dev, @@ -31,12 +33,54 @@ static int test_rtc_read_time(struct device *dev, struct rtc_time *tm) { rtc_time_to_tm(get_seconds(), tm); + return 0; } static int test_rtc_set_mmss(struct device *dev, unsigned long secs) { dev_info(dev, "%s, secs = %lu\n", __func__, secs); + + return 0; +} + +static int test_rtc_irq_set_state(struct device *dev, int enabled) +{ + struct platform_device *plat_dev = to_platform_device(dev); + struct rtc_device *rtc = platform_get_drvdata(plat_dev); + + rtc->pie_enabled = enabled; + + return 0; +} + +static int test_rtc_irq_set_freq(struct device *dev, int freq) +{ + struct platform_device *plat_dev = to_platform_device(dev); + struct rtc_device *rtc = platform_get_drvdata(plat_dev); + + rtc->irq_freq = freq; + + return 0; +} + +static int test_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) +{ + struct platform_device *plat_dev = to_platform_device(dev); + struct rtc_device *rtc = platform_get_drvdata(plat_dev); + + rtc->aie_timer.enabled = enabled; + + return 0; +} + +static int test_rtc_update_irq_enable(struct device *dev, unsigned int enabled) +{ + struct platform_device *plat_dev = to_platform_device(dev); + struct rtc_device *rtc = platform_get_drvdata(plat_dev); + + rtc->uie_rtctimer.enabled = enabled; + return 0; } @@ -50,12 +94,11 @@ static int test_rtc_proc(struct device *dev, struct seq_file *seq) return 0; } +/* As this is a test device, I am leaving the ioctl() code here to make it + * simpler for those want to test. The code currently does nothing. */ static int test_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg) { - /* We do support interrupts, they're generated - * using the sysfs interface. - */ switch (cmd) { case RTC_PIE_ON: case RTC_PIE_OFF: @@ -63,7 +106,9 @@ static int test_rtc_ioctl(struct device *dev, unsigned int cmd, case RTC_UIE_OFF: case RTC_AIE_ON: case RTC_AIE_OFF: - return 0; + /* Return zero in case you want to process the ioctl() */ + /*return 0;*/ + return -ENOIOCTLCMD; default: return -ENOIOCTLCMD; @@ -77,16 +122,22 @@ static const struct rtc_class_ops test_rtc_ops = { .set_alarm = test_rtc_set_alarm, .set_mmss = test_rtc_set_mmss, .ioctl = test_rtc_ioctl, + .irq_set_state = test_rtc_irq_set_state, + .irq_set_freq = test_rtc_irq_set_freq, + .alarm_irq_enable = test_rtc_alarm_irq_enable, + .update_irq_enable = test_rtc_update_irq_enable, }; static ssize_t test_irq_show(struct device *dev, - struct device_attribute *attr, char *buf) + struct device_attribute *attr, char *buf) { return sprintf(buf, "%d\n", 42); } + +/* We support interrupts generated using the sysfs interface. */ static ssize_t test_irq_store(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t count) + struct device_attribute *attr, + const char *buf, size_t count) { int retval; struct platform_device *plat_dev = to_platform_device(dev); @@ -94,11 +145,11 @@ static ssize_t test_irq_store(struct device *dev, retval = count; if (strncmp(buf, "tick", 4) == 0) - rtc_update_irq(rtc, 1, RTC_PF | RTC_IRQF); + rtc_pie_update_irq(&rtc->pie_timer); else if (strncmp(buf, "alarm", 5) == 0) - rtc_update_irq(rtc, 1, RTC_AF | RTC_IRQF); + rtc_aie_update_irq(rtc); else if (strncmp(buf, "update", 6) == 0) - rtc_update_irq(rtc, 1, RTC_UF | RTC_IRQF); + rtc_uie_update_irq(rtc); else retval = -EINVAL; @@ -120,6 +171,12 @@ static int test_probe(struct platform_device *plat_dev) if (err) goto err; + /* If we don't set rtc->irq_freq here, a spurious periodic interrup + * can cause a division by zero in the kernel. And with this driver + * we certainly can generate spurious interrupts through the sys + * interface. */ + rtc->irq_freq = RTC_DEFAULT_PERIODIC_FREQ; + platform_set_drvdata(plat_dev, rtc); return 0;