Skip to content

Conversation

@Keylost
Copy link

@Keylost Keylost commented Nov 20, 2025

if conf->dtim_period is 0 subtracting 1 leads to incorrect behavior and delays in broadcast traffic.
fix issue #406

@Keylost Keylost changed the title [*] Fix setting an incorrect DTIM value. issue #406 Fix setting an incorrect DTIM value Nov 20, 2025
@dubhater
Copy link
Collaborator

You found the problem, that's great!

@dubhater
Copy link
Collaborator

I would suggest max(0, dtim_period - 1), it's more compact. Also if the type of the function parameter is not causing problems, don't change it.

It would be worth emailing [email protected] about this problem because maybe dtim_period is not supposed to be 0 in ad-hoc mode. Maybe there is a bug in mac80211 (that's where the value comes from).

@Keylost
Copy link
Author

Keylost commented Nov 20, 2025

It would be worth emailing [email protected] about this problem because maybe dtim_period is not supposed to be 0 in ad-hoc mode. Maybe there is a bug in mac80211 (that's where the value comes from).

As I understand it, a dtim_period of 0 is normal in ad-hoc mode, since there is no central access point to manage this value as happens in AP mode. That’s why I added 0 as a separate case here.

Also if the type of the function parameter is not causing problems, don't change it.

dtim_period always has type u8, and we write it into an 8-bit register. Although passing dtim_period as an int argument doesn’t cause issues in this case, it’s not entirely correct because it relies on implicit type conversions.

I would suggest max(0, dtim_period - 1), it's more compact.

How about this version?

void rtw_set_dtim_period(struct rtw_dev *rtwdev, u8 dtim_period)
{
    rtw_write32_set(rtwdev, REG_TCR, BIT_TCR_UPDATE_TIMIE);
    rtw_write8(rtwdev, REG_DTIM_COUNTER_ROOT, dtim_period ? dtim_period - 1 : 0);
}

@dubhater
Copy link
Collaborator

I don't like that either 😀 but it's not me you need to convince. Please send your patches to the email address I mentioned earlier.

@Keylost
Copy link
Author

Keylost commented Nov 21, 2025

I don't like that either 😀 but it's not me you need to convince. Please send your patches to the email address I mentioned earlier.

Thank you for your suggestions. Later I’ll try to prepare and send a patch to [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants