linux-kernel - Re: [PATCH RESENT] [RTC] Add Freescale MC13783 RTC driver

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: <20091122214907.GA22054@pengutronix.de>
Date:	Sun, 22 Nov 2009 22:49:07 +0100
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Alessandro Zummo <a.zummo@...ertech.it>
Cc:	linux-kernel@...r.kernel.org,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Paul Gortmaker <p_gortmaker@...oo.com>,
	rtc-linux@...glegroups.com
Subject: Re: [PATCH RESENT] [RTC] Add Freescale MC13783 RTC driver

Hi Alessandro,

On Tue, Nov 10, 2009 at 12:10:01PM +0100, Uwe Kleine-König wrote:
> On Tue, Nov 10, 2009 at 10:30:54AM +0100, Alessandro Zummo wrote:
> > On Tue, 10 Nov 2009 09:32:47 +0100
> > Uwe Kleine-König <u.kleine-koenig@...gutronix.de> wrote:
> > 
> > > +	ret = mc13783_irq_request_nounmask(priv->mc13783, MC13783_IRQ_1HZ,
> > > +			mc13783_rtc_update_handler, DRIVER_NAME, priv);
> > > +	if (ret)
> > > +		goto err_update_irq_request;
> > > +
> > > +	mc13783_unlock(priv->mc13783);
> > > +
> > > +	priv->rtc = rtc_device_register(pdev->name,
> > > +			&pdev->dev, &mc13783_rtc_ops, THIS_MODULE);
> > > +
> > 
> >  isn't better to enable irqs after registration.
> IMHO it doesn't make a difference for the 1HZ irq, still more as it
> remains masked.  The reset irq should be requested before registration.
I thought again about the 1HZ irq.  There are two different things to
consider:

 - After rtc_device_register the .update_irq_enable callback could be
   called at once.  At that time the update irq should already be
   registered.  So register irq before rtc_device.
 - The update irq handler calls rtc_update_irq(struct rtc_device *,
   ...) so the rtc_device should already be complete.  That is, register
   before the irq can trigger.

To satisfy both you need to make sure the irq is registered before
registration but masked (either at controller level as in my patch or at
device level (which should be the usual thing but there is no knob in
the mc13783's rtc for that)).

Will you take my patch?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
--
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/