Date: Fri, 28 Apr 2017 10:06:13 -0700
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Martin Kepplinger <martin.kepplinger@ginzinger.com>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] input: touchscreen: ar1021_i2c: replace magic number
 with definition
Message-ID: <20170428170613.GB4949@dtor-ws>
References: <1493295756-17812-1-git-send-email-martin.kepplinger@ginzinger.com>
 <1493295756-17812-2-git-send-email-martin.kepplinger@ginzinger.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1493295756-17812-2-git-send-email-martin.kepplinger@ginzinger.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 979
Lines: 28

On Thu, Apr 27, 2017 at 02:22:36PM +0200, Martin Kepplinger wrote:
> We now have a few of this device's definitions. Let's avoid magic numbers
> and use them.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
>  drivers/input/touchscreen/ar1021_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/ar1021_i2c.c b/drivers/input/touchscreen/ar1021_i2c.c
> index 2a76231..edd5268 100644
> --- a/drivers/input/touchscreen/ar1021_i2c.c
> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> @@ -44,7 +44,7 @@ static irqreturn_t ar1021_i2c_irq(int irq, void *dev_id)
>  		goto out;
>  
>  	/* sync bit set ? */
> -	if ((data[0] & 0x80) == 0)
> +	if ((data[0] & AR1021_TOUCH) == 0)

I'd rather have it as "(data & BIT(7))". This constant does not provide
any better meaning than number 0x80. At least the latter shows that we
test the MSB or the first byte, while the former obfuscates it.

Thanks.

-- 
Dmitry