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
