On Sat, Oct 24, 2026 at 02:52:27PM -0600, James Hilliard wrote:
> uart_write_wakeup() expects the UART port lock to be held. Several worker,
> DMA completion and interrupt paths call it without that lock, including
> indirect calls from uart_handle_cts_change() and the transmit helpers.
>
> Take the port lock around these notifications or their transmit helper.
> Keep sleeping SPI and regmap accesses outside the locked region, and keep
> the FSL LPUART completion lock held through its wakeup. Acquire the port
> lock after releasing private channel locks to preserve the ordering used
> by UART callbacks. In the MUX timer, publish available FIFO space under
> the port lock and keep the hardware FIFO-drain wait outside it.
>
> Use IRQ-safe locking in the Atmel RX and TX tasklets and SB1250 termios
> path, since their interrupt handlers now acquire the same port lock.
> Preserve the caller's interrupt state when releasing those locks.
>
> Serialize CLPS711x TX servicing and its interrupt-enable state with the
> port lock. Disable TX interrupts without waiting in stop_tx(), since a
> threaded handler may need that same lock. Have an already queued handler
> leave stopped output untouched, and drain it outside the lock during
> shutdown before the transmit FIFO can be released.
>
> SCCNXP handles interrupts under a controller lock which its UART callbacks
> acquire with the port lock already held. Collect the ports needing wakeups
> while servicing the controller, then release that lock before taking each
> port lock and rechecking the wakeup threshold. Removal drains the IRQ or
> timer before unregistering the ports.
>
> This serializes write notifications with port-locked serial-core state
> changes, allowing readiness and transmit-storage reservations to gate the
> notification without a check-versus-shutdown race.
...
> -static void sccnxp_handle_events(struct sccnxp_port *s)
> +static unsigned int sccnxp_handle_events(struct sccnxp_port *s)
> {
> + unsigned int wakeup = 0;
unsigned long
> int i;
> u8 isr;
> @@ -491,20 +491,43 @@ static void sccnxp_handle_events(struct sccnxp_port *s)
> for (i = 0; i < s->uart.nr; i++) {
> if (s->opened[i] && (isr & ISR_RXRDY(i)))
> sccnxp_handle_rx(&s->port[i]);
> - if (s->opened[i] && (isr & ISR_TXRDY(i)))
> - sccnxp_handle_tx(&s->port[i]);
> + if (s->opened[i] && (isr & ISR_TXRDY(i)) &&
> + sccnxp_handle_tx(&s->port[i]))
> + wakeup |= BIT(i);
> }
> } while (1);
> +
> + return wakeup;
> +}
> +
> +static void sccnxp_write_wakeup(struct sccnxp_port *s, unsigned int wakeup)
unsigned long
> +{
> + int i;
Why is 'i' signed?
> + /* UART callbacks take s->lock with the port lock already held. */
> + for (i = 0; i < s->uart.nr; i++) {
> + struct uart_port *port = &s->port[i];
> +
> + if (!(wakeup & BIT(i)))
> + continue;
NIH [1] for_each_set_bit()
> + scoped_guard(uart_port_lock_irqsave, port) {
> + if (kfifo_len(&port->state->port.xmit_fifo) <
> WAKEUP_CHARS)
> + uart_write_wakeup(port);
> + }
> + }
> }
NIH syndrome is one of the weakest points of AI, please, read and study
the code you are changing.
[1]: https://en.wikipedia.org/wiki/Not_invented_here
--
With Best Regards,
Andy Shevchenko
