On 7/31/19 21:05, Pavel Machek wrote:
Hi!
hi Pavel,
[ Upstream commit ba3684f99f1b25d2a30b6956d02d339d7acb9799 ]
The function msm_wait_for_xmitr can be taken with interrupts disabled. In order to avoid a potential system lockup - demonstrated under stress testing conditions on SoC QCS404/5 - make sure we wait for a bounded amount of time.
Tested on SoC QCS404.
How long did it take to timeout?
Because... this is supposed to loop for 0.5 second with interrupts disabled, but 500000*udelay(1) is probably going to wait for more than that.
Is 500msec reasonable with interrupts disabled?
considering the original unbounded definition, it is hard to determine what would be a good amount of time to wait (msm_serial can be used for BT comms and I am not sure how critical that link might be for different clients..and I didnt want to create a regression hence the half a second delay).
yeah, I don't think disabling interrupts for half a second is a good idea on most systems hence why I chose it that big.
Should it use something like 5000*udelay(100), instead, as that has chance to result in closer-to-500msec wait?
the half a second timeout didnt mean to be accurate but a worst case scenario...I am not sure accuracy matters.
+++ b/drivers/tty/serial/msm_serial.c @@ -383,10 +383,14 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base) static inline void msm_wait_for_xmitr(struct uart_port *port) {
- unsigned int timeout = 500000;
- while (!(msm_read(port, UART_SR) & UART_SR_TX_EMPTY)) { if (msm_read(port, UART_ISR) & UART_ISR_TX_READY) break; udelay(1);
if (!timeout--)break;}
Plus, should it do some kind of dev_err() to let users know that something went very wrong with their serial?
I did consider this but then I thought that 1/2 second without interrupts on the core should not go unnoticed. But I might be wrong.
Thanks, Pavel