You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/01/14 19:20:59 UTC
[GitHub] [mynewt-nimble] nkaje opened a new pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
nkaje opened a new pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905
XCVR_TX_SCHED_DELAY is time the device needs to wake up before a
scheduled event occurs. With higher mbox size (1024), CMAC can take
longer to process the scheduled events. Scheduled events are processed
in an interrupt context and this parameter gives enough time to service
the scheduled event.
Signed-off-by: Naveen Kaje <na...@juul.com>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] nkaje commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
nkaje commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760478860
> I am wondering if we should set this based on the mailbox size. I am not sure how many pther configurable factors influence this number as well. @andrzej-kaczmarek do you have any comments about this?
I updated the change to depend on mailbox size. Not sure if we have to account for C2S or S2C alone is sufficient.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] wes3 edited a comment on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
wes3 edited a comment on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760966216
@andrzej-kaczmarek The reason this change was made was something you had commented on in the past. There was a note note saying if the mailbox size was made 1024 the scheduling delay should be increased from 250 to 300. This was not done because there was an issue seen in the code. So if you do not think it necessary then we have no issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760987326
@wes3 so could be that I had a hunch that larger s2c mbox size may introduce some timing issues, but tbh I do not recall what was my reasoning in the past. Anyway after reconsidering it now "from scratch" I think instead of just increasing scheduling delay to another magic number would be better to handle this in a cleaner way - deferring mbox read to task context seems like reasonable solution. I applied both patches to my local tree and will test them for any regressions, will push proper PRs if it's all ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760414421
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760479374
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760881674
tbh I do not really see any relation between mbox size and timer interrupt latency.
Larger c2s mailbox means it's less likely that CMAC will spin waiting for free space in mailbox to write data and this would only matter if writes disable interrupts, but iirc they do not.
Larger s2c mailbox means there may be more data for CMAC to read in single interrupt, but it should be enough that LL_TIMER2LLC irq has higher priority than SYS2CMAC, iirc the former has prio 2 while the latter has default prio (lowest).
So do you know what exactly causes delays here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760894230
So the only problem I could think of is reading s2c mailbox in an interrupt context causes some extra delays, although as I wrote above I think it should not really matter since timer interrupt has still higher priority. Anyway, should be really no problem if we just defer reading from that queue to LL task, here are quick patches for core and nimble to try:
https://github.com/andrzej-kaczmarek/apache-mynewt-core/commit/96d0f6e05779d1af27918d68b23ca6b033397c95
https://github.com/andrzej-kaczmarek/apache-mynewt-nimble/commit/665f92130f560f73e6ea7a8f4d5694e358f1d1dc
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760969709
One more thing to check: set only c2s mbox size to 1024 and leave s2c on default value. Large c2s mbox size guarantees that data can be pushed from CMAC quickly so it will not waste time spinning and waiting for sys while smaller s2c mbox size limits amount of data coming into CMAC. That should workaround the problem since I am pretty sure writing from CMAC to large mbox does not have any negative impact on performance (should be the opposite), it's just reading that can potentially cause issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] wes3 commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
wes3 commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760966216
@andrzej-kaczmarek The reason this change was made was something you had commented on in the past. Ben pulled up a note saying if the mailbox size the scheduling delay should be increased from 250 to 300. This was not done because there was an issue seen in the code. So if you do not think it necessary then we have no issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #905: dialog_cmac: update XCVF_TX_SCHED_DELAY
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #905:
URL: https://github.com/apache/mynewt-nimble/pull/905#issuecomment-760414421
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org