From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost.localdomain (89-178-69-49.broadband.corbina.ru [89.178.69.49]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTP id 23DABDDFBE for ; Fri, 27 Apr 2007 08:10:06 +1000 (EST) Message-ID: <46311FC8.9020906@ru.mvista.com> Date: Fri, 27 Apr 2007 01:55:20 +0400 From: Andrei Konovalov MIME-Version: 1.0 To: Arnd Bergmann Subject: Re: [PATCH] Xilinx framebuffer device driver - 2nd version References: <4630F018.3010200@ru.mvista.com> <200704262235.22048.arnd@arndb.de> In-Reply-To: <200704262235.22048.arnd@arndb.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Cc: rick.moleres@xilinx.com, linuxppc-embedded@ozlabs.org List-Id: Linux on Embedded PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Arnd Bergmann wrote: >>@@ -118,6 +118,12 @@ struct xilinxfb_drvdata { >> #define to_xilinxfb_drvdata(_info) \ >> container_of(_info, struct xilinxfb_drvdata, info) >> >>+#define xilinx_fb_out_be32(driverdata, offset, val) \ >>+ if (driverdata->use_dcr) \ >>+ mtdcr(driverdata->regs_phys + offset, val); \ >>+ else \ >>+ out_be32(driverdata->regs + offset, val) >>+ >> static int >> xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, >> unsigned transp, struct fb_info *fbi) > > > This should probably be an inline function, or use do { ... } while (0) > instead, to make it less error-prone, see > http://kernelnewbies.org/FAQ/DoWhile0 for an explanation. > > Arnd <>< xilinx_fb_out_be32 macro doesn't declare local variables, and is a single "line of code" in terms of the web page you've referenced. IOW I can't get an example where this macro would do wrong off the top of my head. Anyway, the do { ... } while (0) is safe and common, so I'll regenerate the patch to use it. Thanks a lot for reviewing that stuff! Best regards, Andrei