Hi Tim,
On Tue, Sep 24, 2013 at 05:39:09PM -0700, Tim Kryger wrote:
> The Designware UART has a limitation where it ignores writes into the
> LCR if the UART is busy. The current workaround stashes a copy of the
> last written LCR and writes it back down to the hardware if it receives
> a special busy interrupt which is raised when a write was ignored.
>
> Unfortunately, interrupts are typically disabled prior to performing a
> sequence of register writes that include the LCR so the point at which
> the retry occurs is too late. An example is serial8250_do_set_termios()
> where an ignored LCR write results in the baud divisor not being set and
> instead a garbage character is sent out the transmitter.
>
> Furthermore, since serial_port_out() offers no way to indicate failure,
> a serious effort must be made to ensure that the LCR is actually updated
> before returning back to the caller. This is difficult, however, as a
> UART that was busy during the first attempt is likely to still be busy
> when a subsequent attempt is made unless some extra action is taken.
>
> This updated workaround takes the extreme action of clearing the TX/RX
> FIFOs and reading the receive buffer before writing down the LCR in the
> hope that doing so will force the UART into an idle state. While this
> may seem unnecessarily aggressive, writes to the LCR are used to change
> the baud rate, parity, stop bit, or data length so the data that may be
> lost is likely not important. Admittedly, this is far from ideal but it
> seems to be the best that can be done given the hardware limitations.
<snip>
> @@ -76,17 +75,35 @@ static inline int dw8250_modify_msr(struct uart_port *p,
> int offset, int value)
> return value;
> }
>
> +/* The UART will ignore writes to LCR when busy we take aggressive steps
> + * to ensure that it is idle before attempting to write to LCR */
> +static void dw8250_force_idle(struct uart_port *p)
> +{
> + serial8250_clear_and_reinit_fifos(container_of
> + (p, struct uart_8250_port, port));
> + (void)p->serial_in(p, UART_LSR);
> + (void)p->serial_in(p, UART_MSR);
> + (void)p->serial_in(p, UART_RX);
> +}
This looks pretty brutal. Is it really necessary?
> static void dw8250_serial_out(struct uart_port *p, int offset, int value)
> {
> struct dw8250_data *d = p->private_data;
>
> - if (offset == UART_LCR)
> - d->last_lcr = value;
> -
> - if (offset == UART_MCR)
> - d->last_mcr = value;
> -
> - writeb(value, p->membase + (offset << p->regshift));
> + if (offset == UART_LCR) {
> + int tries = 1000;
> + while (tries--) {
> + if (value == p->serial_in(p, UART_LCR))
> + return;
> + dw8250_force_idle(p);
> + writeb(value, p->membase + (UART_LCR << p->regshift));
> + }
> + dev_err(p->dev, "Couldn't set LCR to %d\n", value);
Is it not enough to simply poll USR[0] to see when the UART becomes
free?
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
