From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chuansheng Liu Subject: Re: [PATCH 1/7] I2c-nomadik: Fix the usage of wait_for_completion_timeout Date: Thu, 15 Nov 2012 18:52:36 +0800 Message-ID: References: <1352222317.15558.1584.camel@cliu38-desktop-build> <20121115084858.GA29668@bnru10> <20121115092953.GA418@pengutronix.de> <20121115095741.GA6280@bnru10> Mime-Version: 1.0 (iPad Mail 8J3) Content-Type: multipart/mixed; boundary="===============9221670036613574084==" Return-path: In-Reply-To: <20121115095741.GA6280@bnru10> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Srinidhi Kasagar Cc: "linus.walleij@linaro.org" , "linux-kernel@vger.kernel.org" , Wolfram Sang , "linux-i2c@vger.kernel.org" , "chuansheng.liu@intel.com" , "linux-arm-kernel@lists.infradead.org" List-Id: linux-i2c@vger.kernel.org --===============9221670036613574084== Content-Transfer-Encoding: 7bit Content-Type: multipart/alternative; boundary=Apple-Mail-4-1007408005 --Apple-Mail-4-1007408005 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 timeout is never < 0, so status =3D timeout is never getting run. This patch= delete pointless code. Could you show the regression? Thanks. =E5=9C=A8 2012-11-15=EF=BC=8C17:57=EF=BC=8CSrinidhi Kasagar =E5=86=99=E9=81=93=EF=BC=9A > On Thu, Nov 15, 2012 at 10:29:53 +0100, Wolfram Sang wrote: >>=20 >>>> - if (timeout < 0) { >>>> - dev_err(&dev->adev->dev, >>>> - "wait_for_completion_timeout " >>>> - "returned %d waiting for event\n", timeout); >>>> - status =3D timeout; >>>> - } >>>> - >>> No, it is wrong. You need to update the status variable in the case of t= imeout. >>=20 >> Looking at the patch context, such code comes later. > But it causes regressions; without looking at the "later" code, we can't a= fford merging > this code now. >=20 > regards/srinidhi > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in= > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ --Apple-Mail-4-1007408005 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
timeout is never < 0, so status =3D timeout is never getting run. This patch delete p= ointless
co= de. Could you show the regression? Thanks.

=E5= =9C=A8 2012-11-15=EF=BC=8C17:57=EF=BC=8CSrinidhi Kasagar <srinidhi.kasagar@stericsson.com> =E5= =86=99=E9=81=93=EF=BC=9A

<= div>On Thu, Nov 15, 2012 at 10:29:53 +0100, Wolfram Sang wrote:=

-  =      if (timeout < 0) {
<= /blockquote>
-        = ;       dev_err(&dev->adev->dev= ,
-   &nb= sp;            &= nbsp;      "wait_for_completion_timeout "
-    &nb= sp;            &= nbsp;     "returned %d waiting for event\n", timeou= t);
-   &= nbsp;           statu= s =3D timeout;
- &nb= sp;     }
-
No, it is wrong. You need t= o update the status variable in the case of timeout.
=

Looking at the patch context, such code comes later= .
But it causes regressions; without looking at= the "later" code, we can't afford merging
this code now.

regards/srinidhi
--To unsubscribe from this list: send the line "unsubscribe linux-kerne= l" in
the body of a message to majordomo@vger.= kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
= --Apple-Mail-4-1007408005-- --===============9221670036613574084== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============9221670036613574084==--