From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754867AbdKJTd2 (ORCPT ); Fri, 10 Nov 2017 14:33:28 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:41364 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753727AbdKJTd0 (ORCPT ); Fri, 10 Nov 2017 14:33:26 -0500 Date: Fri, 10 Nov 2017 22:32:59 +0300 From: Dan Carpenter To: Marcus Wolf Cc: Greg Kroah-Hartman , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: staging: pi433: Possible bug in rf69.c Message-ID: <20171110193258.qv6dbokqpjrrq3yb@mwanda> References: <709d3e3f-be3b-aa98-1cf6-6a048ef14d7b@smarthome-wolf.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <709d3e3f-be3b-aa98-1cf6-6a048ef14d7b@smarthome-wolf.de> User-Agent: NeoMutt/20170609 (1.8.3) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Nov 10, 2017 at 06:23:32PM +0100, Marcus Wolf wrote: > Hi everybody! > > Just comparing the master of Gregs statging of pi433 with my local SVN > to review all changes, that were done the last monthes. > > I am not sure, but maybe we imported a bug in rf69.c lines 378 and > following: > > Gregs repo: > case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_AUTO) ); > my repo: > case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) & ~MASK_LNA_GAIN) | LNA_GAIN_AUTO) ); I edited the lines for clarity. The difference is that your repo does a bitwise OR "| LNA_GAIN_AUTO" and the kernel.org code does a bitwise "& LNA_GAIN_AUTO". The kernel repo hasn't changed since you sent us the driver in commit 874bcba65f9a ('staging: pi433: New driver'). I agree that & doesn't seem to make sense and I'm disapointed that it doesn't cause a Smatch warning. But LNA_GAIN_AUTO is zero so maybe | BIT(LNA_GAIN_AUTO) was intended instead of | LNA_GAIN_AUTO. I don't know... No one on this list knows the answer probably. :/ regards, dan caprenter