Re: [RFC 1/2] clockevents: return 'int' from clockevents_set_mode()

[Viresh Kumar]

On 12 May 2014 15:49, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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
--
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/