linux-kernel - Re: [PATCH 3/3] RTC: Update seconds time programming logic

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160420075741.GK29844@piout.net>
Date:	Wed, 20 Apr 2016 09:57:41 +0200
From:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:	Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>
Cc:	Alessandro Zummo <a.zummo@...ertech.it>,
	Soren Brinkmann <sorenb@...inx.com>,
	Michal Simek <michals@...inx.com>,
	"rtc-linux@...glegroups.com" <rtc-linux@...glegroups.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Punnaiah Choudary Kalluri <punnaia@...inx.com>,
	Anirudha Sarangi <anirudh@...inx.com>,
	Srikanth Vemula <svemula@...inx.com>,
	Srinivas Goud <sgoud@...inx.com>
Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic

On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
> delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
> load time + 1sec into the write register. So when read we would be getting the correct time without
> any delay. If any request comes from user to read time before RTC updating the read register, we
> need to give the previous loaded time instead of giving the time from the read register. 
> For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
> Clear the RTC_INT_STS register while programming the time into the write register . If we get a
> request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
> is set in RTC_INT_STS, we need to give the previous loaded time.
> This should be done if time is requested from user space within  1 sec period after writing time, after
> the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
> the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
> depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
> occurance in the interrupt handler.
> Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
> the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.
> 

Yeas, I understood that. But my question was whether the interrupt
handling was necessary at all.
Instead of waiting for an interrupt to set time_updated, can't you
simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
xlnx_rtc_read_time() ?

Something like:

status = readl(xrtcdev->reg_base + RTC_INT_STS)
if (status & RTC_INT_SEC)
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
else
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1, tm);

It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
updated even when it is not enabled as an interrupt.

> Thanks,
> Anurag Kumar V
>   
> > > +	xrtcdev->time_updated = 0;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > > struct rtc_time *tm)  {
> > >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > >
> > > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > +	if (xrtcdev->time_updated == 0) {
> > > +		/*
> > > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > > +		 * the seconds read register, so read the time from the
> > > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > +		 */
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_SET_TM_RD), tm);
> > > +		tm->tm_sec -= 1;
> > > +	} else {
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_CUR_TM), tm);
> > > +	}
> > >
> > >  	return rtc_valid_tm(tm);
> > >  }
> > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > *xrtcdev)  {
> > >  	u32 rtc_ctrl;
> > >
> > > +	/* Enable RTC SEC interrupts */
> > > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > +
> > >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> > >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > >  	rtc_ctrl |= RTC_BATT_EN;
> > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> > *id)
> > >  	/* Clear interrupt */
> > >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > >
> > > -	if (status & RTC_INT_SEC)
> > > +	if (status & RTC_INT_SEC) {
> > > +		if (xrtcdev->time_updated == 0) {
> > > +			/* RTC updated the seconds read register */
> > > +			xrtcdev->time_updated = 1;
> > > +		}
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > +	}
> > >  	if (status & RTC_INT_ALRM)
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > >
> > > --
> > > 2.1.2
> > >
> > 
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux, Kernel and Android engineering
> > http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com