Re: [PATCH] serial: pxa: Disable TX interrupt if there is no more data to transmit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonas and Jiri,

On 5/20/2024 2:30 AM, Jonas Gorski wrote:
Hi

On Mon, 20 May 2024 at 08:41, Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:

This does not make sense. If the circ buf is empty,
uart_port_tx_limited() should stop the TX already. You simply revert to
the state before 7bfb915a597a, but on a per-driver basis.

IOW all drivers using the helper would have exactly this issue after
7bfb915a597a.

I tested more, and it appears you are correct, Jiri. I think
7bfb915a597a has broken other drivers too.

For some background on this, after upgrading to 6.9 I observed that
transmits would stop working on my PXA168-based device with the pxa
driver until I caused another UART IRQ to fire, for example by typing a
character in the console. I bisected the problem to 7bfb915a597a.

I don't have a bunch of hardware to test with, but I cobbled together a
way to emulate a BeagleBoard in an old Linaro QEMU branch, and the omap-
serial driver also appears to be affected. Shortly after boot, the UART
just completely hangs and all transmits stop working. I can't even type
a character to recover it like I can with the pxa driver. Reverting
7bfb915a597a fixes the issue.

I think the reason the pxa driver recovers when I type a character is
because it always checks the THRE bit of the LSR in the IRQ handler,
even if the IIR doesn't say there's a TX interrupt ready. (This appears
to be a workaround for an erratum in the PXA serial IP...)

Perhaps, it should be reverted and the affected driver should just pass
UART_TX_NOSTOP instead?

       uart_port_tx_limited(&up->port, ch, up->port.fifosize / 2,
               true,
               serial_out(up, UART_TX, ch),

I can't speak for whether that would fix the issue Jonas originally saw,
but it does look like it needs to be reverted because other drivers are
also broken due to this change.

1. The kernel doc for uart_ops::stop_tx() says "The driver should stop
transmitting characters as soon as possible."

Very interesting. I guess the correct behavior depends on the exact
interpretation of "stop transmitting characters as soon as possible".
:-) It looks like, at least for the existing 16550-esque drivers that
are using the helper, they are treating stop_tx() more as a signal to
disable the TX interrupt, but still allow any characters already in the
FIFO to go out.

All I know is, in the PXA UART, I was observing the TX IRQ firing when
we had no data to load in the FIFO, and we weren't disabling the
interrupt. This causes a 16550A-style UART to get hung up because the TX
interrupt never fires again unless you disable it and re-enable it.

Doug




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux