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
| ||
|
Date: Tue, 05 May 2015 13:44:21 -0700 From: Joe Perches <joe@...ches.com> To: Alexandre Belloni <alexandre.belloni@...e-electrons.com> Cc: Eddie Huang <eddie.huang@...iatek.com>, Lee Jones <lee.jones@...aro.org>, Alessandro Zummo <a.zummo@...ertech.it>, Matthias Brugger <matthias.bgg@...il.com>, Andrew Morton <akpm@...ux-foundation.org>, Tomasz Figa <tfiga@...gle.com>, Uwe Kleine-König <kernel@...gutronix.de>, srv_heupstream@...iatek.com, Samuel Ortiz <sameo@...ux.intel.com>, Greg KH <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org, rtc-linux@...glegroups.com, linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, Tianping Fang <tianping.fang@...iatek.com> Subject: Re: [rtc-linux] [PATCH v3 2/3] rtc: mediatek: Add MT6397 RTC driver On Tue, 2015-05-05 at 22:00 +0200, Alexandre Belloni wrote: > Hi, > > This looks mostly good. Could you align the wrapped function parameters > to the open parenthesis (use checkpatch --strict)? > > On 28/04/2015 at 15:35:55 +0800, Eddie Huang wrote : > > +static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc) > > +{ > > + unsigned long timeout = jiffies + HZ; > > + int ret; > > + u32 data; > > + > > + ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1); > > + if (ret < 0) > > + return ret; > > + > > + do { > > + cpu_relax(); > > + ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU, > > + &data); > > + if (ret < 0) > > + goto exit; > > + } while ((data & RTC_BBPU_CBUSY) && time_after(timeout, jiffies)); > > + > > Shouldn't you return -ETIMEDOUT if the loop breaks because of time_after? Probably yes. I believe as written the time_after test is too much for my little brain. I would have used time_before and reversed the args. I suggest moving the time_after() test into the loop, use break; and remove the exit label too. Maybe something like: while (1) { ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU, &data); if (ret < 0) break; if (!(data & RTC_BBPU_CBUSY)) break; if (time_after(jiffies, timeout)) { ret = -ETIMEDOUT; break; } cpu_relax(); } return ret; } -- 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/
Powered by blists - more mailing lists