apache
dialog_cmac: update XCVR_TX_SCHED_DELAY
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 [email protected]
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.
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?
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
@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.
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.
@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.
I don't think we need this, there were significant optimizations done in CMAC code that seem to resolve pretty much all known issues, at least those I was aware of.