Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751830Ab2BPORN (ORCPT ); Thu, 16 Feb 2012 09:17:13 -0500 Received: from skyrme.org ([193.175.80.135]:46555 "EHLO skyrme.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267Ab2BPORL (ORCPT ); Thu, 16 Feb 2012 09:17:11 -0500 Date: 16 Feb 2012 14:17:07 +0000 Message-ID: <20120216141707.GA8647@curry> From: "Oskar Schirmer" To: "Wolfram Sang" Cc: "Oskar Schirmer" , wim@iguana.be, s.hauer@pengutronix.de, linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] watchdog: make imx2_wdt report boot status correctly References: <1329394665-7238-1-git-send-email-oskar@scara.com> <20120216133510.GA2541@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120216133510.GA2541@pengutronix.de> User-Agent: Mutt/1.4.2.3i Organisation: www.embedded-group.de Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1384 Lines: 36 Hi Wolfram, On Thu, Feb 16, 2012 at 14:35:10 +0100, Wolfram Sang wrote: > besides this minor thing (which may be just personal taste)... > > > + case WDIOC_GETBOOTSTATUS: > > + val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR); > > + new_value = 0; > > + if (val & IMX2_WDT_WRSR_TOUT) > > + new_value = WDIOF_CARDRESET; > > I'd go for this to save some lines: > new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0; A good alternative for sure. Seen from the perspective of code compactness ("save some lines"), one could try to be consequent here, and save the "val" variable altogether, ending up with some "new_value = __raw_readw ... & ... ? ... : 0;" But eventually there might be code readability issues, so it's a good idea to keep the balance. An argument for the original, longer version might be it is easier to extend, when more flags need to be handled, like "else if (val & ...) new_value = ...", while nested conditional expressions would most likely become quite complex soon. Actually, my personal taste doesn't show a preference for one version or the other, so I'm ok with Your proposal, too. Oskar -- 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/