Re: [PATCH] serial: qcom-geni: Enable support for half-duplex mode

[Jiri Slaby <jirislaby@kernel.org>]

On 29. 04. 25, 12:43, Anup Kulkarni wrote:
> Currently the RTS pin is used as flow control in the UART controller by
> default.For half-duplex modes, use the RTS pin for direction control to
> prevent data collisions by allowing only one direction at a time. Utilize
> the rs485 structure and callbacks in the serial core framework to support
> half-duplex modes. Use the IOCTL value, 'TIOCSRS485', and the 'struct
> serial_rs485' supported by the serial framework to implement support for
> related callbacks. Enable RS485 mode with these callbacks.
> 
> Signed-off-by: Anup Kulkarni <quic_anupkulk@quicinc.com>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 61 ++++++++++++++++++++++++++-
>   1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index a80ce7aaf309..ad3c63cecda0 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
...
> @@ -637,6 +650,7 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>   	struct tty_port *tport = &uport->state->port;
>   	unsigned int xmit_size;
>   	u8 *tail;
> +	bool pin_state;
>   	int ret;
>   
>   	if (port->tx_dma_addr)
> @@ -648,6 +662,12 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
>   	xmit_size = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail,
>   			UART_XMIT_SIZE);
>   
> +	if (uport->rs485.flags & SER_RS485_ENABLED) {
> +	/* For RS485 mode, the RTS can be set/cleared before transmission */
> +		pin_state = !!(uport->rs485.flags & SER_RS485_RTS_ON_SEND);

That !! is unnecessary.

> +		qcom_geni_set_rts_pin(uport, pin_state);
> +	}
> +
>   	qcom_geni_serial_setup_tx(uport, xmit_size);
>   
>   	ret = geni_se_tx_dma_prep(&port->se, tail, xmit_size,
...
> @@ -1055,8 +1076,17 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>   	}
>   
>   	if (dma) {
> -		if (dma_tx_status & TX_DMA_DONE)
> +		if (dma_tx_status & TX_DMA_DONE) {
>   			qcom_geni_serial_handle_tx_dma(uport);
> +			// Check if RS485 mode is enabled
> +			if (uport->rs485.flags & SER_RS485_ENABLED) {
> +				// Determine the RTS pin state based on the
> +				// RS485 RTS_AFTER_SEND flag.
> +				pin_state = !!(uport->rs485.flags & SER_RS485_RTS_AFTER_SEND);

The same here.

> +				// Set or clear the RTS pin according to the determined state
> +				qcom_geni_set_rts_pin(uport, pin_state);
> +		}
> +	}
>   
>   		if (dma_rx_status) {
>   			if (dma_rx_status & RX_RESET_DONE)
> @@ -1594,6 +1624,29 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
>   	}
>   }
>   
> +/**
> + * qcom_geni_rs485_config - Configure RS485 settings for the UART port
> + * @uport: Pointer to the UART port structure
> + * @termios: Pointer to the termios structure
> + * @rs485: Pointer to the RS485 configuration structure
> + *
> + * This function configures the RTS (Request to Send) pin behavior for RS485 mode.
> + * When RS485 mode is enabled, the RTS pin is kept in the ACTIVE state.
> + * When RS485 mode is disabled, the RTS pin is controlled by the QUP hardware for auto flow control.
> + *
> + * Return: Always returns 0.
> + */
> +

This \n is superfluous. IIRC it is even problematic for kernel-doc.

> +static int qcom_geni_rs485_config(struct uart_port *uport,
> +				  struct ktermios *termios, struct serial_rs485 *rs485)
> +{
> +	if (rs485->flags & SER_RS485_ENABLED)
> +		qcom_geni_set_rts_pin(uport, true); // Set RTS pin to ACTIVE state

C style comments ^^ and vv.

> +	else
> +		writel(0, uport->membase + SE_UART_MANUAL_RFR); // Revert to auto flow control
> +	return 0;
> +}
> +
>   static const struct uart_ops qcom_geni_console_pops = {
>   	.tx_empty = qcom_geni_serial_tx_empty,
>   	.stop_tx = qcom_geni_serial_stop_tx_fifo,

thanks,
-- 
js
suse labs