From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753569AbbC0RTP (ORCPT ); Fri, 27 Mar 2015 13:19:15 -0400 Received: from mail-lb0-f170.google.com ([209.85.217.170]:34162 "EHLO mail-lb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752322AbbC0RTN (ORCPT ); Fri, 27 Mar 2015 13:19:13 -0400 Message-ID: <5515910C.8000107@cogentembedded.com> Date: Fri, 27 Mar 2015 20:19:08 +0300 From: Sergei Shtylyov Organization: Cogent Embedded User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Nicholas Mc Guire CC: Nicholas Mc Guire , Inaky Perez-Gonzalez , linux-wimax@intel.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC] wimax/i2400m: fixup completion handling for resetting a device References: <1426585774-24204-1-git-send-email-hofrat@osadl.org> <55096FA1.9030908@cogentembedded.com> <20150320074705.GB21852@opentech.at> In-Reply-To: <20150320074705.GB21852@opentech.at> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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