lkml.org 
[lkml]   [2019]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v2 08/10] rtc: bd70528: Initial support for ROHM bd70528 RTC
Hello Alexandre,

Big thanks for the review!

On Mon, Jan 28, 2019 at 10:20:09PM +0100, Alexandre Belloni wrote:
> Hello,
>
> On 25/01/2019 13:05:36+0200, Matti Vaittinen wrote:
> > +static const struct rtc_class_ops bd70528_rtc_ops = {
> > + .read_time = bd70528_get_time,
> > + .set_time = bd70528_set_time,
> > + .read_alarm = bd70528_read_alarm,
> > + .set_alarm = bd70528_set_alarm,
>
> You actually need to provide a .alarm_irq_enable callback, else there is
> no way to disable a set alarm.

Glad that you pointed this out. I thought the RTC core would use
set_alarm with enabled in struct rtc_wkalrm being zero for disabling
the alarm. I'll add alarm_irq_enable for disabling alarm at v3.

> > + rtc = devm_rtc_device_register(&pdev->dev, "bd70528-rtc",
> > + &bd70528_rtc_ops, THIS_MODULE);
> > + if (IS_ERR(rtc)) {
> > + dev_err(&pdev->dev, "Registering RTC failed\n");
> > + return PTR_ERR(rtc);
> > + }
>
> Please use devm_rtc_allocate_device() and set rtc->range_min and
> rtc->range_max before registering with rtc_register_device() (which can
> be done after trying to request the irq).

Right. I should've noted that devm_rtc_device_register was marked
deprecated. I'll fix this too at v3.

Br,
Matti Vaittinen

\
 
 \ /
  Last update: 2019-01-29 13:01    [W:0.143 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site