From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752009AbcACMrk (ORCPT ); Sun, 3 Jan 2016 07:47:40 -0500 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:36904 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751817AbcACMrh (ORCPT ); Sun, 3 Jan 2016 07:47:37 -0500 X-IronPort-AV: E=Sophos;i="5.20,516,1444687200"; d="scan'208";a="195342207" Date: Sun, 3 Jan 2016 13:47:20 +0100 (CET) From: Julia Lawall X-X-Sender: jll@localhost6.localdomain6 To: SF Markus Elfring cc: rtc-linux@googlegroups.com, Alessandro Zummo , Alexandre Belloni , LKML , kernel-janitors@vger.kernel.org, Julia Lawall Subject: Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer() In-Reply-To: <5688E268.9090600@users.sourceforge.net> Message-ID: References: <566ABCD9.1060404@users.sourceforge.net> <5688DF2D.6090204@users.sourceforge.net> <5688E268.9090600@users.sourceforge.net> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 3 Jan 2016, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 3 Jan 2016 09:19:32 +0100 > > Pass a value directly in a call of the function "regmap_update_bits" > instead of an extra initialisation for one local variable at the beginning. > > Signed-off-by: Markus Elfring > --- > drivers/rtc/rtc-ab-b5ze-s3.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c > index 88f1d0b..fed52d0 100644 > --- a/drivers/rtc/rtc-ab-b5ze-s3.c > +++ b/drivers/rtc/rtc-ab-b5ze-s3.c > @@ -561,7 +561,6 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm, > { > struct abb5zes3_rtc_data *data = dev_get_drvdata(dev); > u8 regs[ABB5ZES3_TIMA_SEC_LEN]; > - u8 mask = ABB5ZES3_REG_TIM_CLK_TAC0 | ABB5ZES3_REG_TIM_CLK_TAC1; > int ret; > > /* Program given number of seconds to Timer A registers */ > @@ -575,7 +574,9 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm, > > /* Configure Timer A as a watchdog timer */ > ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK, > - mask, ABB5ZES3_REG_TIM_CLK_TAC1); > + ABB5ZES3_REG_TIM_CLK_TAC0 > + | ABB5ZES3_REG_TIM_CLK_TAC1, > + ABB5ZES3_REG_TIM_CLK_TAC1); This doesn't seem like an improvement. The concept (mask) has disappeared, the binary operation is strangely broken, and the function call has one more line of arguments, which all look sort of the same and thus are hard to understand. Don't underestimate the value of naming things. julia > if (ret) > dev_err(dev, "%s: failed to update timer\n", __func__); > > -- > 2.6.3 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >