Re: [PATCH 0/4] RTC regression fixups

[John Stultz]

On Thu, 2011-02-03 at 18:31 -0200, Marcelo Roberto Jimenez wrote:
> Hi John,
> 
> Currently, the RTC driver _must_ declare the read_alarm() callback,
> even if it does nothing. But the code in drivers/rtc/interface.c does
> 
> 	if (rtc->ops == NULL)
> 		err = -ENODEV;
> 	else if (!rtc->ops->read_alarm)
> 		err = -EINVAL;
> 	else {
> 		memset(alarm, 0, sizeof(struct rtc_wkalrm));
> 		alarm->enabled = rtc->aie_timer.enabled;
> 		alarm->time = rtc_ktime_to_tm(rtc->aie_timer.node.expires);
> 	}
> 
> The read_alarm() callback is not being performed.

So this was recently added by d5553a556165535337ece8592f066407c62eec2e
to handle the cases where the driver didn't support alarm irqs, and the
error wasn't being properly propagated upwards.

> Two questions:
> 
> 1 - Should the callback be removed or should it be kept and called in
> the else part?

So, we probably should change the check to set_alarm or some other flag
to check if the hardware supports irqs, then remove the driver
read_alarm() function.


> 2 - In case we are keeping it, should it be enforced like it is now,
> or should it be kept optional? I'd rather have it optional, that means
> less useless code in the drivers.

Yes. We want to minimize the unnecessary code. I'll take a shot at it
shortly here.

Also, to avoid duplicate work, please see my dev/rtc-cleanups branch
here:
http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=shortlog;h=refs/heads/dev/rtc-cleanups

I've got some patches that remove the dead irq_set_state, irq_set_freq
and update_irq_enable functions from the rtc drivers. It would be great
if you were able to give those patches a whirl to make sure I didn't
flub anything.

thanks
-john



--
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/