On 25 May 2014 15:16, Preeti U Murthy preeti@linux.vnet.ibm.com wrote:
When dev->set_dev_mode fails in the above calls, we must add error handling for each of the modes. A mere 'WARN_ON()' will not do,
I understand that the reason you have not implemented error handling for each mode is because these calls will not fail today. But without error handling, the patchset appears incomplete to me. IMO, we need to give a thought to what should be done if any of the modes fail, else it will remain a half baked idea.
This is what tglx wrote about this:
On 12 May 2014 15:49, Thomas Gleixner tglx@linutronix.de wrote:
if (dev->set_dev_mode) { ret = dev->set_dev_mode(dev, mode); handle_return_value(ret);
Does that handle_return_value() mean that you sprinkle WARN_ONs all over the place? 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?
Hell no.
handle_return_value() means handle the return value right there. We know what we want to set and we know what may fail and what not, so we can explicitely WARN right there. And for those things which might fail, we return the failure and handle it at the call site.
I believe I have done exactly what he had asked for. And I dare not play more with him anymore, its like entering Lion's Den :)