lkml.org 
[lkml]   [2015]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on
On 17/12/2015 at 11:37:06 +0000, Opensource [Steve Twiss] wrote :
> On 16 December 2015 23:47 Alexandre Belloni wrote:
> > Subject: Re: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on
> >
> > This seems mostly fine, however ...
>
> Hi Alexandre,
> Thanks for reviewing this.
>
> > On 08/12/2015 at 16:28:39 +0000, Steve Twiss wrote :
> > > irq_alarm = platform_get_irq_byname(pdev, "ALARM");
> > > ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> > > da9063_alarm_event,
> > > IRQF_TRIGGER_LOW |
> > IRQF_ONESHOT,
> > > "ALARM", rtc);
> > > - if (ret) {
> > > + if (ret)
> > > dev_err(&pdev->dev, "Failed to request ALARM IRQ %d:
> > %d\n",
> > > irq_alarm, ret);
> > > - return ret;
> > > - }
> > > -
> >
> > ... now that requesting the interrupt is optional, you probably want to
> > prevent userspace from thinking it will get an interrupt after setting
> > the alarm by returning -EINVAL in da9063_rtc_read_alarm() and
> > da9063_rtc_set_alarm() in that case.
> >
>
> .. I'm not quite certain I understand.
> Does the patch looks worse that it is?
> This part,
>
> + if (ret)
> dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
> irq_alarm, ret);
> - return ret;
>
> looks like it has erased the return ret,
>
> >
> > > - rtc->rtc_dev = devm_rtc_device_register(&pdev->dev,
> > DA9063_DRVNAME_RTC,
> > > - &da9063_rtc_ops, THIS_MODULE);
> > > - if (IS_ERR(rtc->rtc_dev))
> > > - return PTR_ERR(rtc->rtc_dev);
> > >
> > > - da9063_data_to_tm(data, &rtc->alarm_time, rtc);
> > > - rtc->rtc_sync = false;
> > > return ret;
>
> But it does exist at the end of the patch.
> So there will still be an error sent if the IRQ is not requested properly.
> Is this what you meant in your previous e-mail?
>

Indeed, you are right, I'll apply that patch.


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


\
 
 \ /
  Last update: 2015-12-17 13:01    [W:0.060 / U:2.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site