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

[Kevin Wells]

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.