Re: staging: pi433: Possible bug in rf69.c

[Marcus Wolf <marcus.wolf@smarthome-wolf.de>]

Hi Dan,

I checked it on my local SVN. You are right. I submitted the code with '&'.
Accodring to a check-in message on my SVN, there was a bugreport end of
July and most probably a patch - either from me, you, Joseph Wright,
Colin King or Julia Lawall, changing '&' to '|'. I guess the patch for
some reason wasn't accepted, but fortunatley I introduced the change to
my SVN.

So from my point of view, we need a change from '&' to '|'.

I could prepare such a patch, but I am still unsure, which repo to use.

Shortly befor I fell ill, you proposed me to use Gregs staging for my
further development. But Colin yesterday was working on a repo, called
linux-next.

Can you (or anyone else) please tell me, when (or for which kind of
patches) to use the Gregs staging and wehen (or for which kind of
patches) to use the linux-next? Sorry for not being familiar with that
stuff!

Thanks a lot,

Marcus



Am 10.11.2017 um 20:32 schrieb Dan Carpenter:
> 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
>