Hello. On 03/20/2015 10:47 AM, Nicholas Mc Guire wrote:
Sorry for late reply, I'm pretty busy these days.
wait_for_completion_timeout return 0 (timeout) or >=1 (completion) so the check for > 0 in the else branch is always true and can be dropped. The comment seems misleading as it is always going to pass the result up.
The sync of the completion access with __i2400m_dev_reset_handle (which checks for if (i2400m->reset_ctx) could race if i2400m_reset() returns negative so the resetting of i2400m->reset_ctx == NULL is moved to the out: path.
As wait_for_completion_timeout returns unsigned long not int, an appropriately named variable of type unsigned long is added and assignments fixed up.
Don't try to do several things in one patch.
normaly yes - this was marked as RFC and if I had split it up into 3 patches it would be hard to see how it fits together without actually applying them.
You could summarize your intent in the cover letter (PATCH #0).
The intent was to get feedback notably on moving i2400m->reset_ctx == NULL and if dropping the (I think missleading) comment about negative return is ok
Should this be in seperate patches even as RFC ?
I think the RFC patches should still conform to all the usual patch rules. How would we understand whether you intent to split the patch up later, if you didn't even write about it anywhere?
[...] WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
