[OpenWrt-Devel] [PATCH v4 1/2] serial: ar933x_uart: add rs485 support

[Petr Štetiar]

Daniel Golle <daniel at makrotopia.org> [2020-02-11 20:33:57]:

Hi,

it really still feels somehow weird, that's mainly why I've suggested to
take this through upstream first in my previous email.

> +@@ -103,10 +106,42 @@ static inline void ar933x_uart_stop_tx_i
> + static inline void ar933x_uart_putc(struct ar933x_uart_port *up, int ch)
> + {
> + 	unsigned int rdata;
> ++	struct serial_rs485 rs485conf = up->port.rs485;
> + 
> + 	rdata = ch & AR933X_UART_DATA_TX_RX_MASK;
> + 	rdata |= AR933X_UART_DATA_TX_CSR;
> +-	ar933x_uart_write(up, AR933X_UART_DATA_REG, rdata);
> ++
> ++	if (rs485conf.flags & SER_RS485_ENABLED) {
> ++		unsigned int timeout = 60000;
> ++		unsigned long flags;
> ++		unsigned int status;
> ++
> ++		/* Disable RX interrupt */
> ++		spin_lock_irqsave(&up->port.lock, flags);

FYI this code path:

 ar933x_uart_interrupt
  ar933x_uart_tx_chars
   ar933x_uart_putc

has acquired spin_lock, disabled TX interrupt, and this codepath:

 ar933x_uart_console_write
  ar933x_uart_console_putchar
   ar933x_uart_putc

has acquired spin_lock and disabled all interrupts already.

> ++		up->ier &= ~AR933X_UART_INT_RX_VALID;
> ++		ar933x_uart_write(up, AR933X_UART_INT_EN_REG, up->ier);

that looks like ar933x_uart_stop_rx() copy&paste

> ++		/* wait for transmission to end */
> ++		do {
> ++			status = ar933x_uart_read(up, AR933X_UART_CS_REG);
> ++			if (--timeout == 0)
> ++				break;
> ++			udelay(1);
> ++		} while ((status & AR933X_UART_CS_TX_BUSY) != 0);

This above looks like ar933x_uart_wait_xmitr() copy&paste but just done
differently, and I miss the point why ar933x_uart_wait_xmitr() cant be reused
here as well.

> ++		ar933x_uart_write(up, AR933X_UART_INT_REG, AR933X_UART_INT_RX_VALID);
> ++		/* remove the character from the FIFO */
> ++		ar933x_uart_write(up, AR933X_UART_DATA_REG, AR933X_UART_DATA_RX_CSR);

I really dont get this part and BTW it possibly breaks `rs485-rx-during-tx`
DTS binding.

-- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel