linux-kernel - Re: rf69_set_deviation in rf69.c (pi433 driver)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180622020345.GA1914@hle-laptop.local>
Date:   Thu, 21 Jun 2018 22:03:45 -0400
From:   Hugo Lefeuvre <hle@....eu.com>
To:     Marcus Wolf <marcus.wolf@...rthome-wolf.de>
Cc:     Valentin Vidic <Valentin.Vidic@...Net.hr>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: rf69_set_deviation in rf69.c (pi433 driver)

Hi Marcus,

> > According to the datasheet[0], the deviation should always be smaller
> > than 300kHz, and the following equation should be respected:
> > 
> >   (1) FDA + BRF/2 =< 500 kHz
> > 
> > Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like
> > a bug to me.
> 
> My focus was always on OOK and ASK. PSK was only used for a few
> measurements in the laboratory, I engaged to check CE compliance.
> Most probably 500kHz was a value, that's common for PSK and I didn't pay
> any attention to the datasheet. So I think, you are right: This is a bug
> and could be revised.
> Never the less: While using it in the lab, the transmission was fine and
> the signals over air have been clean and fitted to the recommondations
> of the CE.
> > 
> > Concerning the TODO, I can see two solutions currently:
> > 
> > 1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB
> >    and REG_BITRATE_LSB and returns reconstructed BRF.
> >    We could use this function in rf69_set_deviation to retrieve the BRF.
> > 
> > + clean
> > + intuitive
> > - heavy / slow
> 
> Why not: I like clean and intuitive implementations. Since it's used
> during configuration, we shouldn't be that squeezed in time, that we
> need to hurry.
> > 
> > 2. Store BRF somewhere in rf69.c, initialize it with the default value
> >    (4.8 kb/s) and update it when rf69_set_bit_rate is called.
> > 
> > + easy
> > - dirty, doesn't fit well with the design of rf69.c (do not store
> >   anything, simply expose API)
> 
> Up to my experience, storing reg values is always a bit problematic,
> since the value may be outdated. And if you update the value every time
> you want to use it to be sure, it's correct, there is no win in having
> the copy of the reg value.
> So this wouldn't be my favourite.
> > 
> > I'd really prefer going for the first one, but I wanted to have your opinion
> > on this.
> 
> Agree.

I'll prepare a patch addressing both issues. However I don't own test devices
so it would be really great if you could test it !

I'm currently thinking of adapting this driver for other HopeRf modules like
RFM69HCW or RFM12 so I will probably try to find some test equipement in the
future.

Thanks for your answer !

Regards,
 Hugo

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)