linux-kernel - RE: [rtc-linux] [PATCH 1/2] rtc: rtc-lpc32xx: Introduce RTC driver for the LPC32XX SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <083DF309106F364B939360100EC290F80AC6E5F76C@eu1rdcrdc1wx030.exi.nxp.com>
Date:	Tue, 10 Aug 2010 21:08:10 +0200
From:	Kevin Wells <kevin.wells@....com>
To:	Wolfram Sang <w.sang@...gutronix.de>,
	"rtc-linux@...glegroups.com" <rtc-linux@...glegroups.com>
CC:	Durgesh Pattamatta <durgesh.pattamatta@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Kevin Wells <wellsk40@...il.com>
Subject: RE: [rtc-linux] [PATCH 1/2] rtc: rtc-lpc32xx: Introduce RTC driver
	for the LPC32XX SoC

Hi Wolfram,

Thanks for helping review this.
I'll get your suggestions installed and an update posted in a few days.

> > +
> > +	device_init_wakeup(&pdev->dev, 1);
> > +	rtc->rtc = rtc_device_register(RTC_NAME, &pdev->dev, &lpc32xx_rtc_ops,
> > +		THIS_MODULE);
> > +	if (IS_ERR(rtc->rtc)) {
> > +		dev_err(&pdev->dev, "Can't get RTC\n");
> > +		retval = PTR_ERR(rtc->rtc);
> > +		goto err_noreg;
> > +	}
> > +
> > +	retval = request_irq(rtc->irq, lpc32xx_rtc_alarm_interrupt,
> > +		IRQF_DISABLED, "rtcalarm", rtc);
> > +	if (retval < 0) {
> > +		dev_err(&pdev->dev, "Can't request interrupt\n");
> > +		goto err_free_irq;
> > +	}
> 
> I saw that a number of rtc-drivers register their irq after they register the
> device. I wonder if this is OK here? Couldn't it happen that after
> rtc_device_register() there is a preemption and another process could set the
> alarm? Then there is a race between interrupts already enabled and no handler
> available, no?
> 

The 32xx RTC registers can save states across power or reset
cycles and are only initialized if they have never been previously
initialized. When waking up from suspend, power on, or reset - the
alarm IRQ may have fired and may be enabled from a previous
initialization, so the RTC device needs to be registered first.

You bring up a good point though, I wonder if going through probe
requires that the alarm be disabled (ie, suspend won't go via probe,
but a hard system reset will). I'll review this a bit more.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/