On 03-07-15, 13:11, Stefan Agner wrote:
On 2015-07-03 10:57, Viresh Kumar wrote:
On 03-07-15, 10:10, Stefan Agner wrote:
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
- .set_mode = pit_set_mode,
- .set_state_shutdown = pit_shutdown,
- .set_state_periodic = pit_set_periodic,
I'm not really familiar with the interface, but given that we announce the feature CLOCK_EVT_FEAT_ONESHOT shouldn't we add a set_state_oneshot callback here?
We weren't doing anything in pit_set_mode(ONESHOT) and so that callback is not implemented. In case you need to do something in set_state_oneshot(), we can add it back.
True, weren't doing anything. I wonder if that is right. Afaik, we should set the same timer for oneshot too, hence call pit_set_next_event. With your change we can just reuse the same function (pit_set_periodic) for set_state_oneshot.
pit_set_next_event() will be called by clockevents core directly after tying to set the device in oneshot mode. And so no changes are required.
To maintain the atomicity of the changes, this would need to be fixed in a separate patch anyway. So this change looks good to me:
Acked-by: Stefan Agner stefan@agner.ch
Thanks.
I guess "clockevents: Allow set-state callbacks to be optional" makes it before this patch? Otherwise we would call a null pointer...
Yeah, I have mentioned this in the cover-letter that there are dependencies over clockevent core's next branch.