From patchwork Tue Jan 3 21:54:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9495769 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4FFF960413 for ; Tue, 3 Jan 2017 21:57:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 422AC27031 for ; Tue, 3 Jan 2017 21:57:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 36A2C27CEA; Tue, 3 Jan 2017 21:57:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 06BD927031 for ; Tue, 3 Jan 2017 21:57:10 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1cOX3F-0001dP-0V; Tue, 03 Jan 2017 21:55:25 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1cOX31-0000Ph-Q7 for linux-arm-kernel@lists.infradead.org; Tue, 03 Jan 2017 21:55:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=71q5eQU/Vn1KwDWwWLhHFjpbLjR5FoCWvegOcUziycw=; b=JbaVvmcIWnGydlmtIC3B9ziDVjY5ekhOQeBzLZtMDL9LUmPDhfDpH6YgY/fO1bEoBy8aRqygqKADdIhtWNhtmi098H9C+E4OJgAT4OxcmIhLeGbK9xzBPkM+19BDRHgN6W6/P/DLcLooT/u2ScWRkdw3QXHCNa/mRgDECwo29Kc=; Received: from n2100.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:38225) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1cOX2L-0005gB-9D; Tue, 03 Jan 2017 21:54:29 +0000 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1cOX2F-0002vq-1Q; Tue, 03 Jan 2017 21:54:23 +0000 Date: Tue, 3 Jan 2017 21:54:21 +0000 From: Russell King - ARM Linux To: Kees Cook Subject: Re: [PATCH] rtc: armada38x: add __ro_after_init to armada38x_rtc_ops Message-ID: <20170103215421.GN14217@n2100.armlinux.org.uk> References: <1482751862-18699-1-git-send-email-bhumirks@gmail.com> <20170102140654.GF14217@n2100.armlinux.org.uk> <20170103213118.GM14217@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170103213118.GM14217@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170103_135512_215019_DD7F3027 X-CRM114-Status: GOOD ( 26.89 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: andrew@lunn.ch, Jason Cooper , rtc-linux@googlegroups.com, a.zummo@towertech.it, LKML , Julia Lawall , alexandre.belloni@free-electrons.com, Bhumika Goyal , gregory.clement@free-electrons.com, "linux-arm-kernel@lists.infradead.org" , sebastian.hesselbarth@gmail.com Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jan 03, 2017 at 09:31:18PM +0000, Russell King - ARM Linux wrote: > On Tue, Jan 03, 2017 at 01:18:29PM -0800, Kees Cook wrote: > > On Mon, Jan 2, 2017 at 6:06 AM, Russell King - ARM Linux > > wrote: > > > On Mon, Dec 26, 2016 at 05:01:02PM +0530, Bhumika Goyal wrote: > > >> The object armada38x_rtc_ops of type rtc_class_ops structure is not > > >> modified after getting initialized by armada38x_rtc_probe. Apart from > > >> getting referenced in init it is also passed as an argument to the function > > >> devm_rtc_device_register but this argument is of type const struct > > >> rtc_class_ops *. Therefore add __ro_after_init to its declaration. > > > > > > What I'd prefer here is for the structure to be duplicated, with one > > > copy having the alarm methods and one which does not. Both can then > > > be made "const" (so placed into the read-only section at link time) > > > and the probe function select between the two. > > > > > > I think that's a cleaner and better solution, even though it's > > > slightly larger. > > > > > > I'm not a fan of __ro_after_init being used where other solutions are > > > possible. > > > > Can the pointer that points to the struct rtc_class_ops be made ro_after_init? > > It's passed into the RTC core code, and probably stored in some dynamically > allocated object, so probably no. It's the same class of problem as every > file_operations pointer in the kernel, or the thousand other operations > structure pointers that a running kernel has. For the elimination of doubt, this is what I meant in my original email. As you can see, there's nothing to be marked as __ro_after_init anymore. drivers/rtc/rtc-armada38x.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c index 9a3f2a6f512e..a4166ccfce36 100644 --- a/drivers/rtc/rtc-armada38x.c +++ b/drivers/rtc/rtc-armada38x.c @@ -202,7 +202,7 @@ static irqreturn_t armada38x_rtc_alarm_irq(int irq, void *data) return IRQ_HANDLED; } -static struct rtc_class_ops armada38x_rtc_ops = { +static const struct rtc_class_ops armada38x_rtc_ops = { .read_time = armada38x_rtc_read_time, .set_time = armada38x_rtc_set_time, .read_alarm = armada38x_rtc_read_alarm, @@ -210,8 +210,15 @@ static struct rtc_class_ops armada38x_rtc_ops = { .alarm_irq_enable = armada38x_rtc_alarm_irq_enable, }; +static const struct rtc_class_ops armada38x_rtc_ops_noirq = { + .read_time = armada38x_rtc_read_time, + .set_time = armada38x_rtc_set_time, + .read_alarm = armada38x_rtc_read_alarm, +}; + static __init int armada38x_rtc_probe(struct platform_device *pdev) { + const struct rtc_class_ops *ops; struct resource *res; struct armada38x_rtc *rtc; int ret; @@ -242,19 +249,22 @@ static __init int armada38x_rtc_probe(struct platform_device *pdev) 0, pdev->name, rtc) < 0) { dev_warn(&pdev->dev, "Interrupt not available.\n"); rtc->irq = -1; + } + platform_set_drvdata(pdev, rtc); + + if (rtc->irq != -1) { + device_init_wakeup(&pdev->dev, 1); + ops = &armada38x_rtc_ops; + } else { /* * If there is no interrupt available then we can't * use the alarm */ - armada38x_rtc_ops.set_alarm = NULL; - armada38x_rtc_ops.alarm_irq_enable = NULL; + ops = &armada38x_rtc_ops_noirq; } - platform_set_drvdata(pdev, rtc); - if (rtc->irq != -1) - device_init_wakeup(&pdev->dev, 1); rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name, - &armada38x_rtc_ops, THIS_MODULE); + ops, THIS_MODULE); if (IS_ERR(rtc->rtc_dev)) { ret = PTR_ERR(rtc->rtc_dev); dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);