On 3 June 2014 01:51, Kevin Hilman khilman@linaro.org wrote:
Might I suggest you wait a day before sending updated versions and to be sure you've understood and addressed all the comments? You're quickly on a path to burn out reviewers.
Sigh, I have been trying to wait for few days before sending new version since sometime now to avoid that and this time it looked like that was the only comment you had.. Probably got confused when you said, you are stopping review after seeing the default behavior changed :)
I was typing some of the following up in reply to V4 but then noticed you already had a V5, so I'll reply here.
So sorry :(, will re-post only tomorrow now :)
You need to describe why KTIME_MAX is special here. You say "should be stopped" but you don't say why. This needs to be summarized better here.
Hmm, yes you are right..
- Few drivers still have 'switch cases' for handling unsupported modes (like: drivers not supporting ONESHOT have a 'case: ONESHOT' with or without a WARN/BUG/pr_err/etc). As we now have a WARN() in core, we don't need these in individual drivers and they can be removed.
This is very unclear. What is the "issue" and would be the fix?
Will try something better..
- Few clockevent drivers have disabled clock events as soon as we enter their ->set_dev_mode() callbacks and so they stay disabled for 'default case' as well. We *may* need to fix these drivers in case we don't want them to disable events in 'default case'. After this series we will never hit 'default case' as we are handling all cases separately, but it might be required to fix them before ONESHOT_STOPPED series gets in.
In general, your cover letters and change logs still mix up what things are like currently and what things will be like (or should be like) after this series. Now your adding another layer of adding things that will likely need fix *in addition to* this series. IMO, all three categories are still mixed up in the descriptions.
I will try to write that again clearly..