From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from az33egw02.freescale.net (az33egw02.freescale.net [192.88.158.103]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "az33egw02.freescale.net", Issuer "Thawte Premium Server CA" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id C9A28DDFEB for ; Wed, 27 May 2009 04:19:52 +1000 (EST) Message-ID: <4A1C3230.8030009@freescale.com> Date: Tue, 26 May 2009 13:17:20 -0500 From: Timur Tabi MIME-Version: 1.0 To: Geoff Thorpe Subject: Re: [PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout() References: <1242761199-17875-1-git-send-email-timur@freescale.com> <1242761199-17875-2-git-send-email-timur@freescale.com> <9e4733910905251046y5f7377f4y49ce72e775faef16@mail.gmail.com> <4A1C16DF.9090802@freescale.com> <4A1C20EA.5080603@freescale.com> <4A1C3059.40303@freescale.com> In-Reply-To: <4A1C3059.40303@freescale.com> Content-Type: text/plain; charset=ISO-8859-1 Cc: linuxppc-dev@ozlabs.org, smaclennan@pikatech.com List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Geoff Thorpe wrote: > rc = spin_event_timeout((ret = in_be32(x) & 0x14), ...); It's an interesting idea, but I have two problems with it: 1) This approach is that it depends on the internals of the macro. That is, you're sneaking in an assignment in the hopes that the code will behave properly. You see that the current code evaluates the condition only once, so it works. The code will look like this: 2) Unlike the wait_event_xxx macros, I believe that the actual evaluated expression is important to the caller. So 90% of the time, the caller is going to pass in "ret = (condition)", which means it makes more sense to have this as a built-in feature of the macro. So if you're okay with v9 of the patch, please ACK it. -- Timur Tabi Linux kernel developer at Freescale