Re: [PATCH 2/4] convert internal frequencies to KHz

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 17, 2020 at 11:15:32PM -0700, Thomas Pedersen wrote:
> In preparation for supporting (S1G) channels centered on
> non-integer MHz, such as 902.5Mhz, convert nearly all
> internal frequency representations into units of KHz.

This is a huge patch and very likely to cause conflicts with any other
pending work.. That's one of the reasons why I did not really want to
even consider this while there was quite a few pending patches in the
queue. That situation is now better, so I can at least consider. Still,
it would be nice if this patch could be split into smaller pieces that
would go through interfaces in steps rather than everything at once.

> A conversion must be done when:
> 
> - parsing existing API, ie. control interface, dbus, etc.
> - parsing config files
> - sending driver commands
> - receiving driver events

That sounds reasonable.

> When the user input is a string, we parse the input as a
> MHz float, then convert to KHz. This will existing
> commands to work on S1G channels unchanged.

But I don't really want to see floating point types for frequencies.
Integers in kHz with helper functions for parsing might be OK, but
ideally, I would not see even a single float or atof() in this patch..

> When generating events and log messages, we exploit the
> "%g" format specifier which will drop the "." and
> trailing zeroes if the input does not have a fractional
> component. Similar to parsing user input this lets us
> expose fractional MHz frequencies without disturbing
> existing users.

Needing floating point numbers for printing out integer values looks
undesired.. I guess "freq=902.5" might be more convenient to see in many
places instead of something like freq_khz=902500, though.. Anyway,
generating that with "%u.%u" or "%u.%03u" for 902.500 might be simpler.

> While dbus, driver_bsd, driver_atheros, driver_hostap,
> driver_privsep, driver_wext, etc. have been converted
> based on 'git grep freq ...', they have received no
> testing :(. Suggestions, extra review, or help testing
> these are welcome.

This is one of the major issues here. Alternative approach would be to
introduce new frequency-in-kHz variables for the places that really need
this while leaving the existing -in-MHz variants available to avoid
regressions.

And on the editorial front, I'd prefer correct spelling of the SI
unit/prefix (kHz) instead of that Kelvin Herz thingie used here in most
places.. ;-)

> This commit should introduce no functional change.

Maybe so, but this is going to hugely inconvenient to review and show to
be correct.

-- 
Jouni Malinen                                            PGP id EFC895FA

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux