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: 710709 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail-lf0-f64.google.com (mail-lf0-f64.google.com [209.85.215.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ttSQp0907z9sf9 for ; Wed, 4 Jan 2017 08:54:41 +1100 (AEDT) Received: by mail-lf0-f64.google.com with SMTP id x140sf33324752lfa.0 for ; Tue, 03 Jan 2017 13:54:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent:sender :x-original-sender:x-original-authentication-results:reply-to :precedence:mailing-list:list-id:x-spam-checked-in-group:list-post :list-help:list-archive:list-subscribe:list-unsubscribe; bh=0QLPxea8bxxtD+5PwMAcZNslOgSrcBfnrImROZ5pPWs=; b=Ae/QhshP63+Ok5eMbk0+k9gnTsVyvFpgmxokFt2kHPShYMSWPxPYTU2dyCW+tHBdgS O90dAoA401sR2VWql0w5segU14WdyIjyHBTqBoOrjmFvy2jG0ExYub709iqcvcnN78n9 PDLhJtNRIvvyg0C5reFaKEQ2TUpIKTQLlpJVWAUwAQ0gbIz1nfzUMGNXjw6xUpsHHJVb t6lVCS8WXgpNcaSwRAmRUxeBZtGMngUEsSDPpAl3vfD6UsCkpAMNw3hS24k1BvPWg0sz d8bWChkDxcAfjtZ0/eoa/M2LD0Ave2wQeRTWTxep/zMPSzZ+nf0wfNcoN2RVdv3X/TQA BDMQ== X-Gm-Message-State: AIkVDXJ7Meo8uLHXTHw2iGvyh0d2EdsKOl/rjbnEH7L3jidRXsA9l9WEY2QjE+FrYdPB8g== X-Received: by 10.25.79.12 with SMTP id d12mr379674lfb.19.1483480478671; Tue, 03 Jan 2017 13:54:38 -0800 (PST) X-BeenThere: rtc-linux@googlegroups.com Received: by 10.25.215.164 with SMTP id q36ls1482003lfi.16.gmail; Tue, 03 Jan 2017 13:54:38 -0800 (PST) X-Received: by 10.46.21.6 with SMTP id s6mr5997240ljd.23.1483480478265; Tue, 03 Jan 2017 13:54:38 -0800 (PST) Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk. [2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by gmr-mx.google.com with ESMTPS id d199si4788555wmd.1.2017.01.03.13.54.37 for (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 03 Jan 2017 13:54:38 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux+rtc-linux=googlegroups.com@armlinux.org.uk designates 2001:4d48:ad52:3201:214:fdff:fe10:1be6 as permitted sender) client-ip=2001:4d48:ad52:3201:214:fdff:fe10:1be6; 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 Cc: andrew@lunn.ch, Jason Cooper , rtc-linux@googlegroups.com, a.zummo@towertech.it, LKML , Julia Lawall , alexandre.belloni@free-electrons.com, "linux-arm-kernel@lists.infradead.org" , gregory.clement@free-electrons.com, Bhumika Goyal , sebastian.hesselbarth@gmail.com Subject: [rtc-linux] 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) Sender: Russell King - ARM Linux X-Original-Sender: linux@armlinux.org.uk X-Original-Authentication-Results: gmr-mx.google.com; dkim=pass (test mode) header.i=@armlinux.org.uk; spf=pass (google.com: best guess record for domain of linux+rtc-linux=googlegroups.com@armlinux.org.uk designates 2001:4d48:ad52:3201:214:fdff:fe10:1be6 as permitted sender) smtp.mailfrom=linux+rtc-linux=googlegroups.com@armlinux.org.uk; dmarc=pass (p=NONE dis=NONE) header.from=armlinux.org.uk Reply-To: rtc-linux@googlegroups.com Precedence: list Mailing-list: list rtc-linux@googlegroups.com; contact rtc-linux+owners@googlegroups.com List-ID: X-Spam-Checked-In-Group: rtc-linux@googlegroups.com X-Google-Group-Id: 712029733259 List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , 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);