On Tue, 2023-11-07 at 18:53 +0300, Dmitry Antipov wrote: > On 11/7/23 18:41, Johannes Berg wrote: > > > Well, the function is actually documenting both TUs and ms depending > > where you look ;-) > > I've relied on the comments around 'struct iwl_time_event_cmd'. Yes but the function docs also say: * @min_duration: will start a new session if the current session will end * in less than min_duration. * @max_delay: maximum delay before starting the time event (in TU) so it expects input in TU already. Then it goes on to say: * This function can be used to start a session protection which means that the * fw will stay on the channel for %duration_ms milliseconds. This function so it's not consistent, but I'm not surprised, my son's teachers always praise him for tracking units and I think it's obvious you have to ;-) Why should the code be different :P (Then again, "duration_ms" isn't even an argument) The value from mac80211 that's passed in comes from * struct ieee80211_prep_tx_info - prepare TX information * @duration: if non-zero, hint about the required duration, * only used with the mgd_prepare_tx() method. which doesn't even say the unit ... but in the one place setting it uses jiffies_to_msecs() to fill it, so it's also not necessarily very accurate (depending on CONFIG_HZ.) Maybe we should rename IWL_MVM_TE_SESSION_PROTECTION_MAX_TIME_MS and IWL_MVM_TE_SESSION_PROTECTION_MIN_TIME_MS to _TU to make it more aligned? Dunno. > > It's also only 2.4% off, so ... > > The corner case when MSEC_TO_TU(1) yields to 0 may be more interesting. The input should be in the order of (a) hundred(s) TU/ms, so that won't really ever happen. The reason why I don't really want to convert it is that the beacon intervals are typically 100 TU, and so if we use 400 ms which converts to 390 TU we _just_ don't cover 4 beacon intervals which is a bit stupid since the precise timing doesn't matter. Covering 4 gives us a better chance here, and anyway the firmware will also have some delays etc. johannes
