wifi: iwlwifi: add missing milliseconds to TUs conversion

Since 'max_delay' and 'duration' of 'struct iwl_time_event_cmd'
are in TUs rather than milliseconds, add missing 'MSEC_TO_TU()'
in 'iwl_mvm_protect_session()' and 'iwl_mvm_schedule_csa_period()'.
Compile tested only.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/intel/iwlwifi/mvm/time-event.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On Tue, 2023-11-07 at 18:26 +0300, Dmitry Antipov wrote:
> Since 'max_delay' and 'duration' of 'struct iwl_time_event_cmd'
> are in TUs rather than milliseconds, add missing 'MSEC_TO_TU()'
> in 'iwl_mvm_protect_session()' and 'iwl_mvm_schedule_csa_period()'.
> Compile tested only.

Well, the function is actually documenting both TUs and ms depending
where you look ;-)

Not sure I'd want to actually change the behaviour here, but maybe clean
up the docs a bit?

It's also only 2.4% off, so ...

johannes

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'.

> It's also only 2.4% off, so ...

The corner case when MSEC_TO_TU(1) yields to 0 may be more interesting.

Dmitry

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