On Thu, May 30, 2024 at 03:45:54PM -0700, Douglas Anderson wrote: > The qcom_geni_serial_poll_bit() is supposed to be able to be used to > poll a bit that's will become set when a TX transfer finishes. Because > of this it tries to set its timeout based on how long the UART will > take to shift out all of the queued bytes. There are two problems > here: > 1. There appears to be a hidden extra word on the firmware side which > is the word that the firmware has already taken out of the FIFO and > is currently shifting out. We need to account for this. > 2. The timeout calculation was assuming that it would only need 8 bits > on the wire to shift out 1 byte. This isn't true. Typically 10 bits > are used (8 data bits, 1 start and 1 stop bit), but as much as 13 > bits could be used (14 if we allowed 9 bits per byte, which we > don't). > > The too-short timeout was seen causing problems in a future patch > which more properly waited for bytes to transfer out of the UART > before cancelling. ... > + /* > + * Add 1 to tx_fifo_depth to account for the hidden register > + * on the firmware side that can hold a word. > + */ > + max_queued_bytes = > + DIV_ROUND_UP((port->tx_fifo_depth + 1) * port->tx_fifo_width, > + BITS_PER_BYTE); BITS_TO_BYTES() ... > - timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500; > + timeout_us = ((max_queued_bits * USEC_PER_SEC) / baud) + 500; Too many parentheses. (The outer ones can be dropped. -- With Best Regards, Andy Shevchenko