From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752145AbcACR33 (ORCPT <rfc822;w@1wt.eu>);
	Sun, 3 Jan 2016 12:29:29 -0500
Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:56705 "EHLO
	mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1751539AbcACR30 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Sun, 3 Jan 2016 12:29:26 -0500
X-IronPort-AV: E=Sophos;i="5.20,516,1444687200"; 
   d="scan'208";a="195358288"
Date: Sun, 3 Jan 2016 18:29:24 +0100 (CET)
From: Julia Lawall <julia.lawall@lip6.fr>
X-X-Sender: jll@localhost6.localdomain6
To: SF Markus Elfring <elfring@users.sourceforge.net>
cc: Julia Lawall <julia.lawall@lip6.fr>, rtc-linux@googlegroups.com,
        Alessandro Zummo <a.zummo@towertech.it>,
        Alexandre Belloni <alexandre.belloni@free-electrons.com>,
        LKML <linux-kernel@vger.kernel.org>, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in
 _abb5zes3_rtc_set_timer()
In-Reply-To: <56895976.5040009@users.sourceforge.net>
Message-ID: <alpine.DEB.2.02.1601031827340.2074@localhost6.localdomain6>
References: <566ABCD9.1060404@users.sourceforge.net> <5688DF2D.6090204@users.sourceforge.net> <5688E268.9090600@users.sourceforge.net> <alpine.DEB.2.02.1601031345240.2053@localhost6.localdomain6> <56895976.5040009@users.sourceforge.net>
User-Agent: Alpine 2.02 (DEB 1266 2009-07-14)
MIME-Version: 1.0
Content-Type: MULTIPART/MIXED; BOUNDARY="8323328-719743927-1451842165=:2074"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-719743927-1451842165=:2074
Content-Type: TEXT/PLAIN; charset=windows-1252
Content-Transfer-Encoding: 8BIT

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> >>  	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.
> 
> Interesting …
> 
> 
> > The concept (mask) has disappeared,
> 
> I suggest to drop another local variable.
> Can the operator "Bitwise OR" be sufficient to indicate the concept "mask"?
> 
> 
> > the binary operation is strangely broken,
> 
> Do you prefer an other source code formatting within the usual line length range?
> 
> 
> > and the function call has one more line of arguments,
> 
> How should several long preprocessor symbols be combined together with indentation
> so that they will fit into the limit of 80 characters?
> 
> 
> > which all look sort of the same and thus are hard to understand.
> 
> Is this an usual consequence from an ordinary name pattern?

The original code was better.  No 80 character problem, easy to 
distinguish one argument from another, moderately meaningful variable 
name, etc.

julia
--8323328-719743927-1451842165=:2074--