RE: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on

[Opensource [Steve Twiss]]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/