From patchwork Tue May 12 20:29:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Herring X-Patchwork-Id: 6391321 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 55FABBEEE1 for ; Tue, 12 May 2015 20:32:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2907A20416 for ; Tue, 12 May 2015 20:32:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 219F220411 for ; Tue, 12 May 2015 20:32:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YsGo5-0006EI-BK; Tue, 12 May 2015 20:29:37 +0000 Received: from mail-ob0-x22f.google.com ([2607:f8b0:4003:c01::22f]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YsGnz-00061n-17 for linux-arm-kernel@lists.infradead.org; Tue, 12 May 2015 20:29:31 +0000 Received: by obcus9 with SMTP id us9so14709481obc.2 for ; Tue, 12 May 2015 13:29:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=eYZuD0dOaYn/fgiYqdRdLsfEpzqI+ZsXZg8sNePr8wA=; b=ahOzI9uBEOvgmsmrt+gQN7ZaGiUOUCsoS9XLo8gIEDhNLyMAZcPbGx4dcRA6V0XxrM sj31ooI5RF6wBXgCjsDD3k/fofhbEZRU63OP13UK/MOROMtIoe8lrFA2Uc+IrPEDKvkt qtgloQv0X2yeZYHAbo4MIfi9MmSta+TgUdI5Jpkb9KENs/My3FuVsgH6T6OHg5sTAfw1 h9p5VpNuTyo9e3rz5M5bWkouJ+XyAbrbBC2XNwTLd4/Gy2mXuAZJlmKSKLZ5JQF+CKdV 8/4L/D5X4bJhlXjAk3z2aRSc45nmA0xsKIDkOkewZcG/DTvohPk/p7Y+6xb34a2lxCn2 QGSA== X-Received: by 10.182.120.69 with SMTP id la5mr13358349obb.62.1431462549270; Tue, 12 May 2015 13:29:09 -0700 (PDT) Received: from [192.168.1.89] (72-48-98-129.dyn.grandenetworks.net. [72.48.98.129]) by mx.google.com with ESMTPSA id ke9sm11523920obb.9.2015.05.12.13.29.08 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 May 2015 13:29:08 -0700 (PDT) Message-ID: <55526294.6090809@gmail.com> Date: Tue, 12 May 2015 15:29:08 -0500 From: Rob Herring User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Robert Jarzmik , Russell King - ARM Linux Subject: Re: [PATCH v3 1/4] ARM: pxa: add memory resource to RTC device References: <1431384089-28367-1-git-send-email-robh@kernel.org> <1431384089-28367-2-git-send-email-robh@kernel.org> <20150511224934.GL2067@n2100.arm.linux.org.uk> <87lhgumi4b.fsf@belgarion.home> In-Reply-To: <87lhgumi4b.fsf@belgarion.home> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150512_132931_131480_86FD6903 X-CRM114-Status: GOOD ( 28.88 ) X-Spam-Score: -0.6 (/) Cc: Alessandro Zummo , Eric Miao , rtc-linux@googlegroups.com, Arnd Bergmann , Haojian Zhuang , Alexandre Belloni , Daniel Mack , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, May 12, 2015 at 1:20 AM, Robert Jarzmik wrote: > Rob Herring writes: > >>> This really isn't a good idea - what do you think happens when >>> the same struct resource gets passed into insert_resource() >>> twice? >> >> Bad things. If you recall, we discussed this on v1[1]. See the commit >> msg of patch 2 for the summary. There is not currently a problem >> because the rtc-pxa driver does not request the resource. > > I think you're talking about resource claiming, while Russell is talking about > resource declaration. > > The point Russell is raising is if you do a platform_add_device() twice with the > same resource (which is done in pxa, once for sa1100-rtc, once for pxa-rtc), you > call insert_resource() twice with the same resource structure. > > I had not thought of that before (that's in my reply noted "nasty consequences" > in [1]), and I'll do my homework this week, and try this only patch, but if I > follow Russell's thinking, then sa1100-rtc and pxa-rtc cannot be declared > together anymore. Oh yes, that is a problem. So looks like we have to make PXA a single driver. Please take a look at the patch below. It's on top of this series currently, so I've got to go back and re-order things. It is build tested only. Technically, the locking is all wrong currently, and the spinlock should be shared, too. However, since this driver is always on a UP system everything is fine. Rob 8<--------------------------------------------------------- Author: Rob Herring Date: Tue May 12 14:31:13 2015 -0500 rtc rework Signed-off-by: Rob Herring diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c index 4561f37..70412c9c 100644 --- a/drivers/rtc/rtc-pxa.c +++ b/drivers/rtc/rtc-pxa.c @@ -32,6 +32,8 @@ #include +#include "rtc-sa1100.h" + #define RTC_DEF_DIVIDER (32768 - 1) #define RTC_DEF_TRIM 0 #define MAXFREQ_PERIODIC 1000 @@ -86,6 +88,7 @@ __raw_writel((value), (pxa_rtc)->base + (reg)) struct pxa_rtc { + struct sa1100_rtc sa1100_rtc; struct resource *ress; void __iomem *base; int irq_1Hz; @@ -321,7 +324,6 @@ static int __init pxa_rtc_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct pxa_rtc *pxa_rtc; int ret; - u32 rttr; pxa_rtc = devm_kzalloc(dev, sizeof(*pxa_rtc), GFP_KERNEL); if (!pxa_rtc) @@ -354,15 +356,16 @@ static int __init pxa_rtc_probe(struct platform_device *pdev) return -ENOMEM; } - /* - * If the clock divider is uninitialized then reset it to the - * default value to get the 1Hz clock. - */ - if (rtc_readl(pxa_rtc, RTTR) == 0) { - rttr = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16); - rtc_writel(pxa_rtc, RTTR, rttr); - dev_warn(dev, "warning: initializing default clock" - " divider/trim value\n"); + pxa_rtc->sa1100_rtc.rcnr = pxa_rtc->base + 0x0; + pxa_rtc->sa1100_rtc.rtsr = pxa_rtc->base + 0x8; + pxa_rtc->sa1100_rtc.rtar = pxa_rtc->base + 0x4; + pxa_rtc->sa1100_rtc.rttr = pxa_rtc->base + 0xc; + pxa_rtc->sa1100_rtc.irq_1hz = pxa_rtc->irq_1Hz; + pxa_rtc->sa1100_rtc.irq_alarm = pxa_rtc->irq_Alrm; + ret = sa1100_rtc_init(pdev, &pxa_rtc->sa1100_rtc); + if (!ret) { + dev_err(dev, "Unable to init SA1100 RTC sub-device\n"); + return ret; } rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE); diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c index 52d2a8a..6e3f63b 100644 --- a/drivers/rtc/rtc-sa1100.c +++ b/drivers/rtc/rtc-sa1100.c @@ -35,6 +35,8 @@ #include #include +#include "rtc-sa1100.h" + #define RTSR_HZE BIT(3) /* HZ interrupt enable */ #define RTSR_ALE BIT(2) /* RTC alarm interrupt enable */ #define RTSR_HZ BIT(1) /* HZ rising-edge detected */ @@ -44,17 +46,6 @@ #define RTC_DEF_TRIM 0 #define RTC_FREQ 1024 -struct sa1100_rtc { - spinlock_t lock; - void __iomem *rcnr; - void __iomem *rtar; - void __iomem *rtsr; - void __iomem *rttr; - int irq_1hz; - int irq_alarm; - struct rtc_device *rtc; - struct clk *clk; -}; static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id) { @@ -235,50 +226,18 @@ static const struct rtc_class_ops sa1100_rtc_ops = { .alarm_irq_enable = sa1100_rtc_alarm_irq_enable, }; -static int sa1100_rtc_probe(struct platform_device *pdev) +int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info) { struct rtc_device *rtc; - struct sa1100_rtc *info; - struct resource *iores; - void __iomem *base; - int irq_1hz, irq_alarm, ret = 0; + int ret; - irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz"); - irq_alarm = platform_get_irq_byname(pdev, "rtc alarm"); - if (irq_1hz < 0 || irq_alarm < 0) - return -ENODEV; + spin_lock_init(&info->lock); - info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL); - if (!info) - return -ENOMEM; info->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(info->clk)) { dev_err(&pdev->dev, "failed to find rtc clock source\n"); return PTR_ERR(info->clk); } - info->irq_1hz = irq_1hz; - info->irq_alarm = irq_alarm; - - iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); - base = devm_ioremap_resource(&pdev->dev, iores); - if (IS_ERR(base)) - return PTR_ERR(base); - - if (IS_ENABLED(CONFIG_ARCH_SA1100) || - of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) { - info->rcnr = base + 0x04; - info->rtsr = base + 0x10; - info->rtar = base + 0x00; - info->rttr = base + 0x08; - } else { - info->rcnr = base + 0x0; - info->rtsr = base + 0x8; - info->rtar = base + 0x4; - info->rttr = base + 0xc; - } - - spin_lock_init(&info->lock); - platform_set_drvdata(pdev, info); ret = clk_prepare_enable(info->clk); if (ret) @@ -298,14 +257,11 @@ static int sa1100_rtc_probe(struct platform_device *pdev) writel_relaxed(0, info->rcnr); } - device_init_wakeup(&pdev->dev, 1); - rtc = devm_rtc_device_register(&pdev->dev, pdev->name, &sa1100_rtc_ops, THIS_MODULE); - if (IS_ERR(rtc)) { - ret = PTR_ERR(rtc); - goto err_dev; + clk_disable_unprepare(info->clk); + return PTR_ERR(rtc); } info->rtc = rtc; @@ -334,9 +290,49 @@ static int sa1100_rtc_probe(struct platform_device *pdev) writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr); return 0; -err_dev: - clk_disable_unprepare(info->clk); - return ret; +} +EXPORT_SYMBOL_GPL(sa1100_rtc_init); + +static int sa1100_rtc_probe(struct platform_device *pdev) +{ + struct sa1100_rtc *info; + struct resource *iores; + void __iomem *base; + int irq_1hz, irq_alarm; + + irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz"); + irq_alarm = platform_get_irq_byname(pdev, "rtc alarm"); + if (irq_1hz < 0 || irq_alarm < 0) + return -ENODEV; + + info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL); + if (!info) + return -ENOMEM; + info->irq_1hz = irq_1hz; + info->irq_alarm = irq_alarm; + + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(&pdev->dev, iores); + if (IS_ERR(base)) + return PTR_ERR(base); + + if (IS_ENABLED(CONFIG_ARCH_SA1100) || + of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) { + info->rcnr = base + 0x04; + info->rtsr = base + 0x10; + info->rtar = base + 0x00; + info->rttr = base + 0x08; + } else { + info->rcnr = base + 0x0; + info->rtsr = base + 0x8; + info->rtar = base + 0x4; + info->rttr = base + 0xc; + } + + platform_set_drvdata(pdev, info); + device_init_wakeup(&pdev->dev, 1); + + return sa1100_rtc_init(pdev, info); } static int sa1100_rtc_remove(struct platform_device *pdev) diff --git a/drivers/rtc/rtc-sa1100.h b/drivers/rtc/rtc-sa1100.h new file mode 100644 index 0000000..2c79c0c --- /dev/null +++ b/drivers/rtc/rtc-sa1100.h @@ -0,0 +1,23 @@ +#ifndef __RTC_SA1100_H__ +#define __RTC_SA1100_H__ + +#include + +struct clk; +struct platform_device; + +struct sa1100_rtc { + spinlock_t lock; + void __iomem *rcnr; + void __iomem *rtar; + void __iomem *rtsr; + void __iomem *rttr; + int irq_1hz; + int irq_alarm; + struct rtc_device *rtc; + struct clk *clk; +}; + +int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info); + +#endif