Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/10313 )
Change subject: add synchronous UART transmission and use it in exceptions ...................................................................... Patch Set 1: Code-Review+1 (1 comment) I think in general I would have preferred to simply use a separate Uart transmit function from the hardirq handlers. Either by explicitly changing those calls away from printf, or by catching/redirecting printf using a #define that's #included only in the exceptions.c file in order to bypass normal printf(). Now we have one conditional in every printf (not really relevant) and some global state that we'd have to switch back should we ever want to have "non-final" exception handling. Another idea that just comes to my mind: Couldn't we simply check some CPU status flags to know that we're in an exception or in an interrupt? This way the decision to bypass the ring buffer could be entirely automatic without any explcit calls to disable the buffer, or any of the hacks I described above? I would suggest to look into this. If it turns out it's not that simple, please report back and we can merge your current patch as-is. https://gerrit.osmocom.org/#/c/10313/1/firmware/libboard/common/source/uart_console.c File firmware/libboard/common/source/uart_console.c: https://gerrit.osmocom.org/#/c/10313/1/firmware/libboard/common/source/uart_console.c@169 PS1, Line 169: hift register */ : while (!(pUart->UART_SR & UART_SR_TXEMPTY)); /* Wait for transfer shift register to be empty (i.e. transfer is complete) */ why do we wait for this? IMHO this effectively disables the "1 byte fifo" of the transfer register? Shouldn't it be safe to transmit the next byte as soon as the hold register has been transferred to the shift register? -- To view, visit https://gerrit.osmocom.org/10313 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b4ace5185cf2dc32684934ed12bf6a8682e9bad Gerrit-Change-Number: 10313 Gerrit-PatchSet: 1 Gerrit-Owner: Kévin Redon <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 02 Aug 2018 21:13:24 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
