On 12 May 2014 15:49, Thomas Gleixner tglx@linutronix.de wrote:
Does that handle_return_value() mean that you sprinkle WARN_ONs all over the place?
Yeah, probably it should have been done this way:
- dev->set_mode(mode, dev); + if (dev->set_dev_mode) { + int ret = dev->set_dev_mode(mode, dev); + + /* Currently available modes are not expected to fail */ + if (WARN_ON(ret)) + return ret; + } else { + dev->set_mode(mode, dev); + } +
My fault and a big one :(
Does it mean, that you change the return value semantics of functions which happen to call clock_events_set_mode() just because it now has a return value?
I didn't do it, probably yet another bad/confusing log by me :(
All other are updated to return error codes.
When I read it now, I can see why it was confusing :(. What I meant was:
"routines which had capability to return errors would now return them on failure of clockevents_set_mode() as well.."
But even that isn't required and handling should be added only to callers of clockevents_set_mode() which are issuing ONESHOT_STOPPED ..
Find someone competent who reviews your patches, deals with you and when they make sense sends them to me.
I will make sure to get some Reviewed-by/Signed-off-by for my patches from someone with good understanding of these frameworks before sending them out again..
Sorry for the noise :(
-- viresh