Re: [PATCH v3 06/15] serial: 8250_mxpcie: add custom handle_irq callback

[Jiri Slaby <jirislaby@kernel.org>]

On 09. 07. 26, 7:33, Crescent Hsieh wrote:
...
> --- a/drivers/tty/serial/8250/8250_mxpcie.c
> +++ b/drivers/tty/serial/8250/8250_mxpcie.c
> @@ -266,6 +266,48 @@ static void mxpcie8250_unthrottle(struct uart_port *port)
>   	serial_out(up, UART_IER, up->ier);
>   }
>   
> +static u16 mxpcie8250_rx_chars(struct uart_8250_port *up, u16 lsr)
> +{
> +	struct uart_port *port = &up->port;
> +
> +	if (!(lsr & (UART_LSR_DR | UART_LSR_BI)))
> +		return lsr;
> +
> +	if (!(port->status & (UPSTAT_AUTOCTS | UPSTAT_AUTORTS)))
> +		goto do_rx;
> +	if (lsr & (UART_LSR_FIFOE | UART_LSR_BRK_ERROR_BITS))
> +		goto do_rx;
> +	if (port->read_status_mask & UART_LSR_DR)
> +		goto do_rx;

Perhaps extract the tests to a separate function (like bool 
mxpcie8250_should_rx()) to get rid of the goto+label?

mxpcie8250_should_rx() and serial8250_rx_chars() could be called 
directly in the ISR then...

> +	return lsr;
> +
> +do_rx:
> +	return serial8250_rx_chars(up, lsr);
> +}
> +
> +static int mxpcie8250_handle_irq(struct uart_port *port)
> +{
> +	struct uart_8250_port *up = up_to_u8250p(port);
> +	u16 lsr;
> +	u8 iir;
> +
> +	iir = serial_in(up, UART_IIR);
> +	if (iir & UART_IIR_NO_INT)
> +		return 0;
> +
> +	guard(uart_port_lock_check_sysrq_irqsave)(port);
> +
> +	lsr = mxpcie8250_rx_chars(up, serial_lsr_in(up));
> +
> +	serial8250_modem_status(up);
> +
> +	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI))
> +		serial8250_tx_chars(up);
> +
> +	return 1;
> +}


thanks,
-- 
js
suse labs