Re: [PATCH v2] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC

[Matthias Kaehlcke]

Hi Venu,

thanks for you reply

El Thu, Aug 02, 2012 at 10:28:47AM +0530 Venu Byravarasu ha dit:

> > -----Original Message-----
> > From: Matthias Kaehlcke [mailto:matthias.list@xxxxxxxxxxxx]
> > Sent: Thursday, August 02, 2012 1:48 AM
> > To: Venu Byravarasu
> > Cc: a.zummo@xxxxxxxxxxxx; sameo@xxxxxxxxxxxxxxx;
> > broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx; Laxman Dewangan;
> > kyle.manna@xxxxxxxxx; sboyd@xxxxxxxxxxxxxx; rtc-
> > linux@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2] rtc: tps65910: Add RTC driver for TPS65910 PMIC RTC
> > 
> > hi,
> > 
> > El Wed, Aug 01, 2012 at 11:52:37AM +0530 Venu Byravarasu ha dit:
> > 
> > > TPS65910 PMIC is a MFD with RTC as one of the device.
> > > Adding RTC driver for supporting RTC device present
> > > inside TPS65910 PMIC.
> > >
> > > Only support for RTC alarm is implemented as part of this patch.
> > >
> > > Signed-off-by: Venu Byravarasu <vbyravarasu@xxxxxxxxxx>
> > > ---
> > 
> > ...
> > 
> > > +static irqreturn_t tps65910_rtc_interrupt(int irq, void *rtc)
> > > +{
> > > +	struct device *dev = rtc;
> > > +	unsigned long events = 0;
> > > +	struct tps65910 *tps = dev_get_drvdata(dev->parent);
> > > +	struct tps65910_rtc *tps_rtc = dev_get_drvdata(dev);
> > >
> > > ...
> > >
> > > +static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
> > > +{
> > >
> > > ...
> > >
> > > +	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> > > +		tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
> > > +		"rtc-tps65910", &pdev->dev);
> > > +	if (ret < 0) {
> > > +		dev_err(&pdev->dev, "IRQ is not free.\n");
> > > +		return ret;
> > > +	}
> > > +	device_init_wakeup(&pdev->dev, 1);
> > > +
> > > +	tps_rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
> > > +		&tps65910_rtc_ops, THIS_MODULE);
> > > +	if (IS_ERR(tps_rtc->rtc)) {
> > > +		ret = PTR_ERR(tps_rtc->rtc);
> > > +		dev_err(&pdev->dev, "RTC device register: err %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	platform_set_drvdata(pdev, tps_rtc);
> > 
> > the interrupt should be requested after this call. otherwise
> > tps65910_rtc_interrupt() could be executed before the RTC device is
> > registered and the driver data set. normally this shouldn't happen as
> > the RTC interrupts are disabled after reset, but the interrupts could
> > have been enabled for example by the bootloader
> 
> Hi Matthias,
> 
> Thanks for your comments.
> All pending RTC interrupts are being cleared in probe, before enabling RTC itself.
> Hence chances of stray interrupts from boot loader cannot cause any trouble here.
> 
> The only possible way of getting interrupt by kernel driver is by enabling 
> tps65910_rtc_alarm_irq_enable() && setting alarm time using tps65910_rtc_set_alarm.
> However before probe gets completed as these two APIs will not get 
> called together by the client, I do not see any false interrupt getting generated.

i was thinking in the periodic interrupts, but now noticed that the
driver only registers to the alarm interrupts, so this is not a
problem

kind regards

-- 
Matthias Kaehlcke
Embedded Linux Developer
Amsterdam

    El optimista tiene siempre un proyecto; el pesimista, una excusa
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/