On 26 May 2014 12:21, Preeti U Murthy preeti@linux.vnet.ibm.com wrote:
In this case, just have a WARN_ON() and let the code fall through for now. The reason I am pushing on this front is when we look at the code, it should make sense one way or the other.
Either return the error code signifying that the call sites have to take care of this or don't return at this point. The early 'return' there is not of any significance. You may say that you will eventually replace that 'return' with 'return error'. But the code is not conveying this.
Hmmm... Continuing here doesn't make any sense, when we know the mode hasn't changed at all.
Maybe we can leave it as is and see what tglx thinks ?
The same goes for clockevents_update_freq(). On warning, we need to return an error code. This will ensure that this patchset is complete in itself. As of now since none of the modes are expected to fail, the call sites needn't handle the return value of clockevents_set_mode() and clockevents_update_freq().
In update_freq() we must never fail as all we are doing is setting a valid mode. So, we don't need to return an error even (to make it complex for
Then why have a WARN_ON() here at all?
Actually can be removed but is there just for protection that we aren't failing. It is very much similar to the WARN_ON in above routine. If we buy your argument, then we don't even need a WARN() in clocevents_set_mode(), we can just check for failure in some special case and return error. Why a WARN ? And so I added it here as well..