On 21 May 2014 23:29, Kevin Hilman khilman@linaro.org wrote:
return 0;
return ret;}
First, this change isn't mentioned in the changelog. But more importantly (as Daniel has mentioned), returning an error here has consequences.
Specifically, why would the caller of this function care that the specific clkevt mode was not implmented? As Thomas mentioned in his reviews, the clockevent code should handle the failures itself, not just pass them up and expect other layers to handle them.
But what should we do when we have hit a WARN()? return zero or return the error code. I understand that the callers might not be going to handle the error code at all, unless its for ONESHOT_STOPPED, which isn't implemented yet.
One more thing apart from the comments you gave, should I drop this change for now then?
-void clockevents_set_mode(struct clock_event_device *dev, +int clockevents_set_mode(struct clock_event_device *dev,
As, we don't want to return the error code on failures currently. Probably might be required during the ONESHOT_STOPPED series though, but doesn't make much sense here.
Also, if PERIODIC mode can't fail today, why would we expect it to fail in the changed version?
We don't, that's why we have a WARN() there.. Are you asking me to get rid of the WARN as well ? and just don't check the returned value?