From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964793AbbLQLhL (ORCPT ); Thu, 17 Dec 2015 06:37:11 -0500 Received: from mail1.bemta5.messagelabs.com ([195.245.231.151]:5437 "EHLO mail1.bemta5.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799AbbLQLhJ convert rfc822-to-8bit (ORCPT ); Thu, 17 Dec 2015 06:37:09 -0500 X-Env-Sender: stwiss.opensource@diasemi.com X-Msg-Ref: server-14.tower-179.messagelabs.com!1450352227!10538313!1 X-Originating-IP: [94.185.165.51] X-StarScan-Received: X-StarScan-Version: 7.35.1; banners=-,-,- X-VirusChecked: Checked From: "Opensource [Steve Twiss]" To: Alexandre Belloni CC: Alessandro Zummo , LINUXKERNEL , RTC-LINUX , "David Dajun Chen" , Support Opensource Subject: RE: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on Thread-Topic: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on Thread-Index: AQHRMdZCEMnYOeIGYEWn5Z/Fj2JxDp7OVHIAgAC8ogA= Date: Thu, 17 Dec 2015 11:37:06 +0000 Message-ID: <6ED8E3B22081A4459DAC7699F3695FB70173D28DCB@SW-EX-MBX02.diasemi.com> References: <20151208163406.B34683FB25@swsrvapps-01.diasemi.com> <20151216234702.GK8574@piout.net> In-Reply-To: <20151216234702.GK8574@piout.net> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.20.26.77] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? Regards, Stephen