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

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 4/29/25 12:43 PM, 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>
> ---

[...]

> +static void qcom_geni_set_rts_pin(struct uart_port *uport, bool pin_state)
> +{
> +	u32 rfr = UART_MANUAL_RFR_EN;
> +
> +	/* Set the logical level of RTS GPIO pin based on the bool variable. */

This comment doesn't add much

> +	rfr |= pin_state ? UART_RFR_NOT_READY : UART_RFR_READY;
> +	writel(rfr, uport->membase + SE_UART_MANUAL_RFR);
> +}
> +
>  static int qcom_geni_serial_request_port(struct uart_port *uport)
>  {
>  	struct platform_device *pdev = to_platform_device(uport->dev);
> @@ -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 */

This comment is incorrectly indented (need 1 more tab)

[...]

>  	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);
> +				// Set or clear the RTS pin according to the determined state
> +				qcom_geni_set_rts_pin(uport, pin_state);

All of these comments repeat what the code says quite clearly

[...]

> +		}
> +	}
>  
>  		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.
> + */
> +
> +static int qcom_geni_rs485_config(struct uart_port *uport,
> +				  struct ktermios *termios, struct serial_rs485 *rs485)
> +{
> +	if (rs485->flags & SER_RS485_ENABLED)

Because SER_RS485_ENABLED is always checked before calling
qcom_geni_set_rts_pin(), it may make more sense to combine that
and this function, perhaps calling it from qcom_geni_serial_start_tx_dma()
too

Konrad

> +		qcom_geni_set_rts_pin(uport, true); // Set RTS pin to ACTIVE state
> +	else
> +		writel(0, uport->membase + SE_UART_MANUAL_RFR); // Revert to auto flow control
> +	return 0;
> +}