Bluetooth: Controller: DTM: Fix timer rollover #42866
Conversation
piotrParf
requested review from
carlescufi,
cvinayak,
thoh-ot and
kruithofa
as code owners
|
Thanks @piotrParf, can you please format the commit as per https://docs.zephyrproject.org/latest/contribute/index.html#commit-guidelines? |
carlescufi
changed the title
cvinayak
changed the title
| /*Previous calculation in while loop can take too long time and can rollout the timer*/ | ||
| if (t < s){ | ||
| t+=((((s - t) / SCAN_INT_UNIT_US) + 1) * SCAN_INT_UNIT_US); |
There was a problem hiding this comment.
Please add roll over handling. Feel free to change the implementation for better readability.
| /*Previous calculation in while loop can take too long time and can rollout the timer*/ | |
| if (t < s){ | |
| t+=((((s - t) / SCAN_INT_UNIT_US) + 1) * SCAN_INT_UNIT_US); | |
| if (t < s) { | |
| t += ((((s - t) / SCAN_INT_UNIT_US) + 1) * SCAN_INT_UNIT_US); | |
| } else { | |
| /* Handle rollover case here, timer is configured to 24-bits */ | |
| t += ((((BIT(24) - t + s) / SCAN_INT_UNIT_US) + 1) * SCAN_INT_UNIT_US); |
There was a problem hiding this comment.
Please check my last commit @cvinayak. I made some refactoring of variables naming and also did another type of preventing the timer rollover.
…ite big it can hapen that while loop will take a long time. This leads to setting up start of a radio timer before current timer value. This then leads to invocation of a timer after rollout and this will lead to very large delay in sending packet. It fixes for me a issue zephyrproject-rtos#42534. Signed-off-by: Piotr Parfeniuk piotr.parfeniuk@magitech.pl
piotrParf
force-pushed
the
fix_issue_42534
branch
from
445ecf9
to
1718515
Compare
…ted other type of timer rollover prevention. Signed-off-by: Piotr Parfeniuk piotr.parfeniuk@magitech.pl
Signed-off-by: Piotr Parfeniuk piotr.parfeniuk@magitech.pl
zephyrbot
requested review from
alwa-nordic,
asbjornsabo,
jhedberg,
mtpr-ot,
ppryga-nordic,
Thalley,
Vudentz and
wopu-ot
|
@piotrParf ping, can you take a look at the review comments please? |
Signed-off-by: Piotr Parfeniuk piotr.parfeniuk@magitech.pl
piotrParf
force-pushed
the
fix_issue_42534
branch
from
6b70dbf
to
a7d580a
Compare
|
Why do i got .github/labeler.yml Error: HttpError ? |
You can ignore that |
|
@KAGA164 could you please comment on this proposed change? |
@carlescufi I will check timings with Anritsu |
|
@piotrParf Please rebase and align your PR due to changes made by #42300. I included CTE time in the transmit time calculation. |
There was a problem hiding this comment.
The DTM Tx interval is incorrect with this fix because it doesn't follow the calculation from Core v5.3 Vol 6 Part F 4.1.6 where interval should follow basically pattern n*625us. I did a test on 1Mb PHY with payload length 37 bytes and I measured the 908us interval when it shall be 625us.
As I do not have proper tools for this what is your suggestion ? |
|
@piotrParf I made PR with my proposal how to handle Tx timings in the DTM, Could you test it and give me a feedback ? #44735 |
For me it is many changes to 2.7.0 but it looks like it works when loaded with this controller to network core. |
superceded by #44735, closing based on authors reply. |
It changes the behaviour of while loop in lll_test.c in isr_tx() . When bug happens and t<s is quite big it can hapen that while loop will take a long time. This leads to setting up start of a radio timer before current timer value. This then leads to invocation of a timer after rollout and this will lead to very large delay in sending packet.
Fixes #42534.
Signed-off-by: Piotr Parfeniuk piotr.parfeniuk@magitech.pl