diff mbox series

[V3,2/2] tty: serial: uartps: Add rs485 support to uartps driver

Message ID 20231024144847.2316941-3-manikanta.guntupalli@amd.com
State New
Headers show
Series Add rs485 support to uartps driver | expand

Commit Message

Manikanta Guntupalli Oct. 24, 2023, 2:48 p.m. UTC
Add rs485 support to uartps driver. Use either rts-gpios or RTS
to control RS485 phy as driver or a receiver.

Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
---
Changes for V2:
Modify optional gpio name to xlnx,phy-ctrl-gpios.
Update commit description.
Add support for RTS, delay_rts_before_send and delay_rts_after_send in RS485 mode.
Changes for V3:
Modify optional gpio name to rts-gpios.
Update commit description.
Move cdns_uart_tx_empty function to avoid prototype statement.
Remove assignment of struct serial_rs485 to port->rs485 as
serial core performs that.
Switch to native RTS in non GPIO case.
Handle rs485 during stop tx.
Remove explicit calls to configure gpio direction and value,
as devm_gpiod_get_optional performs that by using GPIOD_OUT_LOW argument.
Update implementation to support configuration of GPIO/RTS value
based on user configuration of SER_RS485_RTS_ON_SEND and
SER_RS485_RTS_AFTER_SEND. Move implementation to start_tx from handle_tx.
---
 drivers/tty/serial/xilinx_uartps.c | 180 ++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 15 deletions(-)

Comments

Maarten Brock Nov. 4, 2023, 3:46 p.m. UTC | #1
Manikanta Guntupalli wrote on 2023-10-24 16:48:
> @@ -193,6 +196,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
>   * @clk_rate_change_nb:	Notifier block for clock changes
>   * @quirks:		Flags for RXBS support.
>   * @cts_override:	Modem control state override
> + * @gpiod:		Pointer to the gpio descriptor

Change gpiod to gpiod_rts maybe?
Later someone might want to use a gpio for cts/dtr/dsr/dcd/ri as well.

>   */
>  struct cdns_uart {
>  	struct uart_port	*port;
> @@ -203,10 +207,19 @@ struct cdns_uart {
>  	struct notifier_block	clk_rate_change_nb;
>  	u32			quirks;
>  	bool cts_override;
> +	struct gpio_desc	*gpiod;
>  };
>  struct cdns_platform_data {
>  	u32 quirks;
>  };
> +
> +struct serial_rs485 cdns_rs485_supported = {
> +	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
> +		 SER_RS485_RTS_AFTER_SEND,
> +	.delay_rts_before_send = 1,
> +	.delay_rts_after_send = 1,
> +};
> +
>  #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
>  		clk_rate_change_nb)
> 
> @@ -305,6 +318,79 @@ static void cdns_uart_handle_rx(void *dev_id,
> unsigned int isrstatus)
>  	tty_flip_buffer_push(&port->state->port);
>  }
> 
> +/**
> + * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_config_gpio_rts_high(struct cdns_uart 
> *cdns_uart)
> +{
> +	u32 val;
> +
> +	if (cdns_uart->gpiod) {
> +		gpiod_set_value(cdns_uart->gpiod, 1);
> +	} else {
> +		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +		val &= ~CDNS_UART_MODEMCR_RTS;
> +		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +	}
> +}
> +
> +/**
> + * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_config_gpio_rts_low(struct cdns_uart 
> *cdns_uart)
> +{
> +	u32 val;
> +
> +	if (cdns_uart->gpiod) {
> +		gpiod_set_value(cdns_uart->gpiod, 0);
> +	} else {
> +		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +		val |= CDNS_UART_MODEMCR_RTS;
> +		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +	}
> +}
> +
> +/**
> + * cdns_rs485_tx_setup - Tx setup specific to rs485
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_tx_setup(struct cdns_uart *cdns_uart)
> +{
> +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND)
> +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> +	else
> +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> +}
> +
> +/**
> + * cdns_rs485_rx_setup - Rx setup specific to rs485
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_rx_setup(struct cdns_uart *cdns_uart)
> +{
> +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> +	else
> +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> +}

Why not simply create:
void cdns_rs485_driver_enable(struct cdns_uart *cdns_uart, bool enable)

And let it handle the rs485.flags itself?

> +
> +/**
> + * cdns_uart_tx_empty -  Check whether TX is empty
> + * @port: Handle to the uart port structure
> + *
> + * Return: TIOCSER_TEMT on success, 0 otherwise
> + */
> +static unsigned int cdns_uart_tx_empty(struct uart_port *port)
> +{
> +	unsigned int status;
> +
> +	status = readl(port->membase + CDNS_UART_SR) &
> +		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> +	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
> +}
> +
>  /**
>   * cdns_uart_handle_tx - Handle the bytes to be Txed.
>   * @dev_id: Id of the UART port
> @@ -571,6 +657,8 @@ static int cdns_uart_clk_notifier_cb(struct
> notifier_block *nb,
>  static void cdns_uart_start_tx(struct uart_port *port)
>  {
>  	unsigned int status;
> +	unsigned long time_out;
> +	struct cdns_uart *cdns_uart = port->private_data;
> 
>  	if (uart_tx_stopped(port))
>  		return;
> @@ -589,8 +677,31 @@ static void cdns_uart_start_tx(struct uart_port 
> *port)
> 
>  	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_ISR);
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		cdns_rs485_tx_setup(cdns_uart);
> +		if (cdns_uart->port->rs485.delay_rts_before_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_before_send);

Would it not be better to start a timer here with a callback that 
enables
the TXEMPTY interrupt? That would automatically call 
cdns_uart_handle_tx().

> +	}
> +
>  	cdns_uart_handle_tx(port);
> 
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT);
> +		/* Wait for tx completion */
> +		while ((cdns_uart_tx_empty(cdns_uart->port) != TIOCSER_TEMT) &&
> +		       time_before(jiffies, time_out))
> +			cpu_relax();
> +
> +		if (cdns_uart->port->rs485.delay_rts_after_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
> +
> +		/*
> +		 * Default Rx should be setup, because RX signaling path
> +		 * need to enable to receive data.
> +		 */
> +		cdns_rs485_rx_setup(cdns_uart);
> +	}

I think this should be done from the TXEMPTY interrupt. And again 
schedule a
timer to drop the DE line. You really can do this without using 
mdelay().

> +
>  	/* Enable the TX Empty interrupt */
>  	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IER);
>  }
> @@ -602,6 +713,14 @@ static void cdns_uart_start_tx(struct uart_port 
> *port)
>  static void cdns_uart_stop_tx(struct uart_port *port)
>  {
>  	unsigned int regval;
> +	struct cdns_uart *cdns_uart = port->private_data;
> +
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		if (cdns_uart->port->rs485.delay_rts_after_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
> +
> +		cdns_rs485_rx_setup(cdns_uart);
> +	}

Again, start a timer and wait for completion?

>  	regval = readl(port->membase + CDNS_UART_CR);
>  	regval |= CDNS_UART_CR_TX_DIS;
> @@ -626,21 +745,6 @@ static void cdns_uart_stop_rx(struct uart_port 
> *port)
>  	writel(regval, port->membase + CDNS_UART_CR);
>  }
> 
> -/**
> - * cdns_uart_tx_empty -  Check whether TX is empty
> - * @port: Handle to the uart port structure
> - *
> - * Return: TIOCSER_TEMT on success, 0 otherwise
> - */
> -static unsigned int cdns_uart_tx_empty(struct uart_port *port)
> -{
> -	unsigned int status;
> -
> -	status = readl(port->membase + CDNS_UART_SR) &
> -		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> -	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
> -}
> -
>  /**
>   * cdns_uart_break_ctl - Based on the input ctl we have to start or 
> stop
>   *			transmitting char breaks
> @@ -829,6 +933,9 @@ static int cdns_uart_startup(struct uart_port 
> *port)
>  		(CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST))
>  		cpu_relax();
> 
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED)
> +		cdns_rs485_rx_setup(cdns_uart);
> +
>  	/*
>  	 * Clear the RX disable bit and then set the RX enable bit to enable
>  	 * the receiver.
> @@ -1455,6 +1562,23 @@ MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
>  /* Temporary variable for storing number of instances */
>  static int instances;
> 
> +/**
> + * cdns_rs485_config - Called when an application calls TIOCSRS485 
> ioctl.
> + * @port: Pointer to the uart_port structure
> + * @termios: Pointer to the ktermios structure
> + * @rs485: Pointer to the serial_rs485 structure
> + *
> + * Return: 0
> + */
> +static int cdns_rs485_config(struct uart_port *port, struct ktermios 
> *termios,
> +			     struct serial_rs485 *rs485)
> +{
> +	if (rs485->flags & SER_RS485_ENABLED)
> +		dev_dbg(port->dev, "Setting UART to RS485\n");
> +

Shouldn't you force automatic RTS control off here?
And also call cdns_rs485_rx_setup()

> +	return 0;
> +}
> +
>  /**
>   * cdns_uart_probe - Platform driver probe
>   * @pdev: Pointer to the platform device structure
> @@ -1463,6 +1587,7 @@ static int instances;
>   */
>  static int cdns_uart_probe(struct platform_device *pdev)
>  {
> +	u32 val;
>  	int rc, id, irq;
>  	struct uart_port *port;
>  	struct resource *res;
> @@ -1597,9 +1722,23 @@ static int cdns_uart_probe(struct 
> platform_device *pdev)
>  	port->private_data = cdns_uart_data;
>  	port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG 
> |
>  			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;
> +	port->rs485_config = cdns_rs485_config;
> +	port->rs485_supported = cdns_rs485_supported;
>  	cdns_uart_data->port = port;
>  	platform_set_drvdata(pdev, port);
> 
> +	rc = uart_get_rs485_mode(port);
> +	if (rc)
> +		goto err_out_clk_notifier;
> +
> +	cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev, "rts",
> +							GPIOD_OUT_LOW);
> +	if (IS_ERR(cdns_uart_data->gpiod)) {
> +		rc = PTR_ERR(cdns_uart_data->gpiod);
> +		dev_err(port->dev, "xuartps: devm_gpiod_get_optional failed\n");
> +		goto err_out_clk_notifier;
> +	}
> +
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 
> UART_AUTOSUSPEND_TIMEOUT);
>  	pm_runtime_set_active(&pdev->dev);
> @@ -1638,6 +1777,16 @@ static int cdns_uart_probe(struct 
> platform_device *pdev)
>  	cdns_uart_data->cts_override = 
> of_property_read_bool(pdev->dev.of_node,
>  							     "cts-override");
> 
> +	if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) {
> +		if (!cdns_uart_data->gpiod) {
> +			val = readl(cdns_uart_data->port->membase
> +				    + CDNS_UART_MODEMCR);
> +			val |= CDNS_UART_MODEMCR_RTS;
> +			writel(val, cdns_uart_data->port->membase
> +			       + CDNS_UART_MODEMCR);
> +		}
> +	}

Simply call cdns_rs485_rx_setup() ?

> +
>  	instances++;
> 
>  	return 0;
> @@ -1646,6 +1795,7 @@ static int cdns_uart_probe(struct platform_device 
> *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_set_suspended(&pdev->dev);
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
> +err_out_clk_notifier:
>  #ifdef CONFIG_COMMON_CLK
>  	clk_notifier_unregister(cdns_uart_data->uartclk,
>  			&cdns_uart_data->clk_rate_change_nb);

Please also modify cdns_uart_[s|g]et_mctrl() so they support rts-gpios.

Maarten
Lino Sanfilippo Nov. 12, 2023, 7:37 p.m. UTC | #2
Hi,

On 24.10.23 16:48, Manikanta Guntupalli wrote:
> Add rs485 support to uartps driver. Use either rts-gpios or RTS
> to control RS485 phy as driver or a receiver.
>
> Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> ---
> Changes for V2:
> Modify optional gpio name to xlnx,phy-ctrl-gpios.
> Update commit description.
> Add support for RTS, delay_rts_before_send and delay_rts_after_send in RS485 mode.
> Changes for V3:
> Modify optional gpio name to rts-gpios.
> Update commit description.
> Move cdns_uart_tx_empty function to avoid prototype statement.
> Remove assignment of struct serial_rs485 to port->rs485 as
> serial core performs that.
> Switch to native RTS in non GPIO case.
> Handle rs485 during stop tx.
> Remove explicit calls to configure gpio direction and value,
> as devm_gpiod_get_optional performs that by using GPIOD_OUT_LOW argument.
> Update implementation to support configuration of GPIO/RTS value
> based on user configuration of SER_RS485_RTS_ON_SEND and
> SER_RS485_RTS_AFTER_SEND. Move implementation to start_tx from handle_tx.
> ---
>  drivers/tty/serial/xilinx_uartps.c | 180 ++++++++++++++++++++++++++---
>  1 file changed, 165 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 9c13dac1d4d1..32229cf5c508 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -23,6 +23,9 @@
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/iopoll.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/delay.h>
>
>  #define CDNS_UART_TTY_NAME	"ttyPS"
>  #define CDNS_UART_NAME		"xuartps"
> @@ -193,6 +196,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
>   * @clk_rate_change_nb:	Notifier block for clock changes
>   * @quirks:		Flags for RXBS support.
>   * @cts_override:	Modem control state override
> + * @gpiod:		Pointer to the gpio descriptor
>   */
>  struct cdns_uart {
>  	struct uart_port	*port;
> @@ -203,10 +207,19 @@ struct cdns_uart {
>  	struct notifier_block	clk_rate_change_nb;
>  	u32			quirks;
>  	bool cts_override;
> +	struct gpio_desc	*gpiod;
>  };
>  struct cdns_platform_data {
>  	u32 quirks;
>  };
> +
> +struct serial_rs485 cdns_rs485_supported = {
> +	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
> +		 SER_RS485_RTS_AFTER_SEND,
> +	.delay_rts_before_send = 1,
> +	.delay_rts_after_send = 1,
> +};
> +
>  #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
>  		clk_rate_change_nb)
>
> @@ -305,6 +318,79 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus)
>  	tty_flip_buffer_push(&port->state->port);
>  }
>
> +/**
> + * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_config_gpio_rts_high(struct cdns_uart *cdns_uart)
> +{
> +	u32 val;
> +
> +	if (cdns_uart->gpiod) {
> +		gpiod_set_value(cdns_uart->gpiod, 1);
> +	} else {
> +		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +		val &= ~CDNS_UART_MODEMCR_RTS;
> +		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +	}
> +}
> +
> +/**
> + * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_config_gpio_rts_low(struct cdns_uart *cdns_uart)
> +{
> +	u32 val;
> +
> +	if (cdns_uart->gpiod) {
> +		gpiod_set_value(cdns_uart->gpiod, 0);
> +	} else {
> +		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +		val |= CDNS_UART_MODEMCR_RTS;
> +		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
> +	}
> +}
> +
> +/**
> + * cdns_rs485_tx_setup - Tx setup specific to rs485
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_tx_setup(struct cdns_uart *cdns_uart)
> +{
> +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND)
> +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> +	else
> +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> +}
> +
> +/**
> + * cdns_rs485_rx_setup - Rx setup specific to rs485
> + * @cdns_uart: Handle to the cdns_uart
> + */
> +static void cdns_rs485_rx_setup(struct cdns_uart *cdns_uart)
> +{
> +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> +	else
> +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> +}
> +
> +/**
> + * cdns_uart_tx_empty -  Check whether TX is empty
> + * @port: Handle to the uart port structure
> + *
> + * Return: TIOCSER_TEMT on success, 0 otherwise
> + */
> +static unsigned int cdns_uart_tx_empty(struct uart_port *port)
> +{
> +	unsigned int status;
> +
> +	status = readl(port->membase + CDNS_UART_SR) &
> +		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> +	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
> +}
> +
>  /**
>   * cdns_uart_handle_tx - Handle the bytes to be Txed.
>   * @dev_id: Id of the UART port
> @@ -571,6 +657,8 @@ static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
>  static void cdns_uart_start_tx(struct uart_port *port)
>  {
>  	unsigned int status;
> +	unsigned long time_out;
> +	struct cdns_uart *cdns_uart = port->private_data;
>
>  	if (uart_tx_stopped(port))
>  		return;
> @@ -589,8 +677,31 @@ static void cdns_uart_start_tx(struct uart_port *port)
>
>  	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_ISR);
>
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		cdns_rs485_tx_setup(cdns_uart);
> +		if (cdns_uart->port->rs485.delay_rts_before_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_before_send);

So, this will be executed each time (including the rts_before_send delay) the core wants
to send data? This is not how it is supposed to work: The tx setup (and the
delay before send) has to be done once when transmission starts. Note that when sending
a bulk of data the core may call cdns_uart_start_tx() several times before
it eventually calls cdns_uart_stop_tx() to stop the transmission.


> +	}
> +
>  	cdns_uart_handle_tx(port);
>
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT);
> +		/* Wait for tx completion */
> +		while ((cdns_uart_tx_empty(cdns_uart->port) != TIOCSER_TEMT) &&
> +		       time_before(jiffies, time_out))
> +			cpu_relax();
> +
> +		if (cdns_uart->port->rs485.delay_rts_after_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
> +
> +		/*
> +		 * Default Rx should be setup, because RX signaling path
> +		 * need to enable to receive data.
> +		 */
> +		cdns_rs485_rx_setup(cdns_uart);
> +	}
> +
>  	/* Enable the TX Empty interrupt */
>  	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IER);
>  }
> @@ -602,6 +713,14 @@ static void cdns_uart_start_tx(struct uart_port *port)
>  static void cdns_uart_stop_tx(struct uart_port *port)
>  {
>  	unsigned int regval;
> +	struct cdns_uart *cdns_uart = port->private_data;
> +
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> +		if (cdns_uart->port->rs485.delay_rts_after_send)
> +			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
> +
> +		cdns_rs485_rx_setup(cdns_uart);
> +	}
>
>  	regval = readl(port->membase + CDNS_UART_CR);
>  	regval |= CDNS_UART_CR_TX_DIS;
> @@ -626,21 +745,6 @@ static void cdns_uart_stop_rx(struct uart_port *port)
>  	writel(regval, port->membase + CDNS_UART_CR);
>  }
>
> -/**
> - * cdns_uart_tx_empty -  Check whether TX is empty
> - * @port: Handle to the uart port structure
> - *
> - * Return: TIOCSER_TEMT on success, 0 otherwise
> - */
> -static unsigned int cdns_uart_tx_empty(struct uart_port *port)
> -{
> -	unsigned int status;
> -
> -	status = readl(port->membase + CDNS_UART_SR) &
> -		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> -	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
> -}
> -
>  /**
>   * cdns_uart_break_ctl - Based on the input ctl we have to start or stop
>   *			transmitting char breaks
> @@ -829,6 +933,9 @@ static int cdns_uart_startup(struct uart_port *port)
>  		(CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST))
>  		cpu_relax();
>
> +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED)
> +		cdns_rs485_rx_setup(cdns_uart);
> +
>  	/*
>  	 * Clear the RX disable bit and then set the RX enable bit to enable
>  	 * the receiver.
> @@ -1455,6 +1562,23 @@ MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
>  /* Temporary variable for storing number of instances */
>  static int instances;
>
> +/**
> + * cdns_rs485_config - Called when an application calls TIOCSRS485 ioctl.
> + * @port: Pointer to the uart_port structure
> + * @termios: Pointer to the ktermios structure
> + * @rs485: Pointer to the serial_rs485 structure
> + *
> + * Return: 0
> + */
> +static int cdns_rs485_config(struct uart_port *port, struct ktermios *termios,
> +			     struct serial_rs485 *rs485)
> +{
> +	if (rs485->flags & SER_RS485_ENABLED)
> +		dev_dbg(port->dev, "Setting UART to RS485\n");
> +
> +	return 0;
> +}

So what if userspace changes the RS485 configuration? When does it take effect?

> +
>  /**
>   * cdns_uart_probe - Platform driver probe
>   * @pdev: Pointer to the platform device structure
> @@ -1463,6 +1587,7 @@ static int instances;
>   */
>  static int cdns_uart_probe(struct platform_device *pdev)
>  {
> +	u32 val;
>  	int rc, id, irq;
>  	struct uart_port *port;
>  	struct resource *res;
> @@ -1597,9 +1722,23 @@ static int cdns_uart_probe(struct platform_device *pdev)
>  	port->private_data = cdns_uart_data;
>  	port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG |
>  			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;
> +	port->rs485_config = cdns_rs485_config;
> +	port->rs485_supported = cdns_rs485_supported;
>  	cdns_uart_data->port = port;
>  	platform_set_drvdata(pdev, port);
>
> +	rc = uart_get_rs485_mode(port);
> +	if (rc)
> +		goto err_out_clk_notifier;
> +
> +	cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev, "rts",
> +							GPIOD_OUT_LOW);
> +	if (IS_ERR(cdns_uart_data->gpiod)) {
> +		rc = PTR_ERR(cdns_uart_data->gpiod);
> +		dev_err(port->dev, "xuartps: devm_gpiod_get_optional failed\n");

Why bail out with an error if having cdns_uart_data->gpiod is only optional?

> +		goto err_out_clk_notifier;
> +	}
> +
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, UART_AUTOSUSPEND_TIMEOUT);
>  	pm_runtime_set_active(&pdev->dev);
> @@ -1638,6 +1777,16 @@ static int cdns_uart_probe(struct platform_device *pdev)
>  	cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,
>  							     "cts-override");
>
> +	if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) {
> +		if (!cdns_uart_data->gpiod) {
> +			val = readl(cdns_uart_data->port->membase
> +				    + CDNS_UART_MODEMCR);
> +			val |= CDNS_UART_MODEMCR_RTS;
> +			writel(val, cdns_uart_data->port->membase
> +			       + CDNS_UART_MODEMCR);
> +		}
> +	}

This covers the RTS_AFTER_SEND mode. What if SER_RS485_RTS_ON_SEND is configured instead
(as it is the default set by uart_get_rs485_mode())? What if cdns_uart_data->gpiod exists?
Why not simply call cdns_rs485_rx_setup() which covers all these cases?
Note that uart_add_one_port() will call into the serial core and eventually result in an
initial call to the ports rs485_config function (see uart_rs485_config()). So maybe put the
initial configuration into that function and remove the above code. However
in this case

cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,
							     "cts-override");
should be called before uart_add_one_port().


Regards,
Lino
Manikanta Guntupalli Nov. 15, 2023, 6:50 a.m. UTC | #3
Hi,

> -----Original Message-----
> From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
> Sent: Monday, November 13, 2023 1:08 AM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; git (AMD-
> Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>;
> gregkh@linuxfoundation.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; jirislaby@kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud,
> Srinivas <srinivas.goud@amd.com>; Datta, Shubhrajyoti
> <shubhrajyoti.datta@amd.com>; manion05gk@gmail.com
> Subject: Re: [PATCH V3 2/2] tty: serial: uartps: Add rs485 support to uartps
> driver
> 
> Hi,
> 
> On 24.10.23 16:48, Manikanta Guntupalli wrote:
> > Add rs485 support to uartps driver. Use either rts-gpios or RTS to
> > control RS485 phy as driver or a receiver.
> >
> > Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > ---
> > Changes for V2:
> > Modify optional gpio name to xlnx,phy-ctrl-gpios.
> > Update commit description.
> > Add support for RTS, delay_rts_before_send and delay_rts_after_send in
> RS485 mode.
> > Changes for V3:
> > Modify optional gpio name to rts-gpios.
> > Update commit description.
> > Move cdns_uart_tx_empty function to avoid prototype statement.
> > Remove assignment of struct serial_rs485 to port->rs485 as serial core
> > performs that.
> > Switch to native RTS in non GPIO case.
> > Handle rs485 during stop tx.
> > Remove explicit calls to configure gpio direction and value, as
> > devm_gpiod_get_optional performs that by using GPIOD_OUT_LOW
> argument.
> > Update implementation to support configuration of GPIO/RTS value based
> > on user configuration of SER_RS485_RTS_ON_SEND and
> > SER_RS485_RTS_AFTER_SEND. Move implementation to start_tx from
> handle_tx.
> > ---
> >  drivers/tty/serial/xilinx_uartps.c | 180
> > ++++++++++++++++++++++++++---
> >  1 file changed, 165 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/tty/serial/xilinx_uartps.c
> > b/drivers/tty/serial/xilinx_uartps.c
> > index 9c13dac1d4d1..32229cf5c508 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -23,6 +23,9 @@
> >  #include <linux/module.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/iopoll.h>
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/delay.h>
> >
> >  #define CDNS_UART_TTY_NAME	"ttyPS"
> >  #define CDNS_UART_NAME		"xuartps"
> > @@ -193,6 +196,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-
> 255");
> >   * @clk_rate_change_nb:	Notifier block for clock changes
> >   * @quirks:		Flags for RXBS support.
> >   * @cts_override:	Modem control state override
> > + * @gpiod:		Pointer to the gpio descriptor
> >   */
> >  struct cdns_uart {
> >  	struct uart_port	*port;
> > @@ -203,10 +207,19 @@ struct cdns_uart {
> >  	struct notifier_block	clk_rate_change_nb;
> >  	u32			quirks;
> >  	bool cts_override;
> > +	struct gpio_desc	*gpiod;
> >  };
> >  struct cdns_platform_data {
> >  	u32 quirks;
> >  };
> > +
> > +struct serial_rs485 cdns_rs485_supported = {
> > +	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
> > +		 SER_RS485_RTS_AFTER_SEND,
> > +	.delay_rts_before_send = 1,
> > +	.delay_rts_after_send = 1,
> > +};
> > +
> >  #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
> >  		clk_rate_change_nb)
> >
> > @@ -305,6 +318,79 @@ static void cdns_uart_handle_rx(void *dev_id,
> unsigned int isrstatus)
> >  	tty_flip_buffer_push(&port->state->port);
> >  }
> >
> > +/**
> > + * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high
> > + * @cdns_uart: Handle to the cdns_uart  */ static void
> > +cdns_rs485_config_gpio_rts_high(struct cdns_uart *cdns_uart) {
> > +	u32 val;
> > +
> > +	if (cdns_uart->gpiod) {
> > +		gpiod_set_value(cdns_uart->gpiod, 1);
> > +	} else {
> > +		val = readl(cdns_uart->port->membase +
> CDNS_UART_MODEMCR);
> > +		val &= ~CDNS_UART_MODEMCR_RTS;
> > +		writel(val, cdns_uart->port->membase +
> CDNS_UART_MODEMCR);
> > +	}
> > +}
> > +
> > +/**
> > + * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low
> > + * @cdns_uart: Handle to the cdns_uart  */ static void
> > +cdns_rs485_config_gpio_rts_low(struct cdns_uart *cdns_uart) {
> > +	u32 val;
> > +
> > +	if (cdns_uart->gpiod) {
> > +		gpiod_set_value(cdns_uart->gpiod, 0);
> > +	} else {
> > +		val = readl(cdns_uart->port->membase +
> CDNS_UART_MODEMCR);
> > +		val |= CDNS_UART_MODEMCR_RTS;
> > +		writel(val, cdns_uart->port->membase +
> CDNS_UART_MODEMCR);
> > +	}
> > +}
> > +
> > +/**
> > + * cdns_rs485_tx_setup - Tx setup specific to rs485
> > + * @cdns_uart: Handle to the cdns_uart  */ static void
> > +cdns_rs485_tx_setup(struct cdns_uart *cdns_uart) {
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND)
> > +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> > +	else
> > +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> > +}
> > +
> > +/**
> > + * cdns_rs485_rx_setup - Rx setup specific to rs485
> > + * @cdns_uart: Handle to the cdns_uart  */ static void
> > +cdns_rs485_rx_setup(struct cdns_uart *cdns_uart) {
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> > +		cdns_rs485_config_gpio_rts_high(cdns_uart);
> > +	else
> > +		cdns_rs485_config_gpio_rts_low(cdns_uart);
> > +}
> > +
> > +/**
> > + * cdns_uart_tx_empty -  Check whether TX is empty
> > + * @port: Handle to the uart port structure
> > + *
> > + * Return: TIOCSER_TEMT on success, 0 otherwise  */ static unsigned
> > +int cdns_uart_tx_empty(struct uart_port *port) {
> > +	unsigned int status;
> > +
> > +	status = readl(port->membase + CDNS_UART_SR) &
> > +		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> > +	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0; }
> > +
> >  /**
> >   * cdns_uart_handle_tx - Handle the bytes to be Txed.
> >   * @dev_id: Id of the UART port
> > @@ -571,6 +657,8 @@ static int cdns_uart_clk_notifier_cb(struct
> > notifier_block *nb,  static void cdns_uart_start_tx(struct uart_port
> > *port)  {
> >  	unsigned int status;
> > +	unsigned long time_out;
> > +	struct cdns_uart *cdns_uart = port->private_data;
> >
> >  	if (uart_tx_stopped(port))
> >  		return;
> > @@ -589,8 +677,31 @@ static void cdns_uart_start_tx(struct uart_port
> > *port)
> >
> >  	writel(CDNS_UART_IXR_TXEMPTY, port->membase +
> CDNS_UART_ISR);
> >
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> > +		cdns_rs485_tx_setup(cdns_uart);
> > +		if (cdns_uart->port->rs485.delay_rts_before_send)
> > +			mdelay(cdns_uart->port-
> >rs485.delay_rts_before_send);
> 
> So, this will be executed each time (including the rts_before_send delay) the
> core wants to send data? This is not how it is supposed to work: The tx setup
> (and the delay before send) has to be done once when transmission starts.
> Note that when sending a bulk of data the core may call cdns_uart_start_tx()
> several times before it eventually calls cdns_uart_stop_tx() to stop the
> transmission.
We have tested bulk transfer (5000 bytes) and observed that cdns_uart_start_tx() is getting called only once. Can you please suggest how to reproduce the case where the core may call cdns_uart_start_tx() several times before it eventually calls cdns_uart_stop_tx().
> 
> 
> > +	}
> > +
> >  	cdns_uart_handle_tx(port);
> >
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> > +		time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT);
> > +		/* Wait for tx completion */
> > +		while ((cdns_uart_tx_empty(cdns_uart->port) !=
> TIOCSER_TEMT) &&
> > +		       time_before(jiffies, time_out))
> > +			cpu_relax();
> > +
> > +		if (cdns_uart->port->rs485.delay_rts_after_send)
> > +			mdelay(cdns_uart->port-
> >rs485.delay_rts_after_send);
> > +
> > +		/*
> > +		 * Default Rx should be setup, because RX signaling path
> > +		 * need to enable to receive data.
> > +		 */
> > +		cdns_rs485_rx_setup(cdns_uart);
> > +	}
> > +
> >  	/* Enable the TX Empty interrupt */
> >  	writel(CDNS_UART_IXR_TXEMPTY, port->membase +
> CDNS_UART_IER);  } @@
> > -602,6 +713,14 @@ static void cdns_uart_start_tx(struct uart_port
> > *port)  static void cdns_uart_stop_tx(struct uart_port *port)  {
> >  	unsigned int regval;
> > +	struct cdns_uart *cdns_uart = port->private_data;
> > +
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
> > +		if (cdns_uart->port->rs485.delay_rts_after_send)
> > +			mdelay(cdns_uart->port-
> >rs485.delay_rts_after_send);
> > +
> > +		cdns_rs485_rx_setup(cdns_uart);
> > +	}
> >
> >  	regval = readl(port->membase + CDNS_UART_CR);
> >  	regval |= CDNS_UART_CR_TX_DIS;
> > @@ -626,21 +745,6 @@ static void cdns_uart_stop_rx(struct uart_port
> *port)
> >  	writel(regval, port->membase + CDNS_UART_CR);  }
> >
> > -/**
> > - * cdns_uart_tx_empty -  Check whether TX is empty
> > - * @port: Handle to the uart port structure
> > - *
> > - * Return: TIOCSER_TEMT on success, 0 otherwise
> > - */
> > -static unsigned int cdns_uart_tx_empty(struct uart_port *port) -{
> > -	unsigned int status;
> > -
> > -	status = readl(port->membase + CDNS_UART_SR) &
> > -		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
> > -	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
> > -}
> > -
> >  /**
> >   * cdns_uart_break_ctl - Based on the input ctl we have to start or stop
> >   *			transmitting char breaks
> > @@ -829,6 +933,9 @@ static int cdns_uart_startup(struct uart_port *port)
> >  		(CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST))
> >  		cpu_relax();
> >
> > +	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED)
> > +		cdns_rs485_rx_setup(cdns_uart);
> > +
> >  	/*
> >  	 * Clear the RX disable bit and then set the RX enable bit to enable
> >  	 * the receiver.
> > @@ -1455,6 +1562,23 @@ MODULE_DEVICE_TABLE(of,
> cdns_uart_of_match);
> >  /* Temporary variable for storing number of instances */  static int
> > instances;
> >
> > +/**
> > + * cdns_rs485_config - Called when an application calls TIOCSRS485 ioctl.
> > + * @port: Pointer to the uart_port structure
> > + * @termios: Pointer to the ktermios structure
> > + * @rs485: Pointer to the serial_rs485 structure
> > + *
> > + * Return: 0
> > + */
> > +static int cdns_rs485_config(struct uart_port *port, struct ktermios
> *termios,
> > +			     struct serial_rs485 *rs485)
> > +{
> > +	if (rs485->flags & SER_RS485_ENABLED)
> > +		dev_dbg(port->dev, "Setting UART to RS485\n");
> > +
> > +	return 0;
> > +}
> 
> So what if userspace changes the RS485 configuration? When does it take
> effect?
We will disable auto RTS control and call cdns_uart_stop_tx() which will perform both stop tx and cdns_rs485_rx_setup()
> 
> > +
> >  /**
> >   * cdns_uart_probe - Platform driver probe
> >   * @pdev: Pointer to the platform device structure @@ -1463,6 +1587,7
> > @@ static int instances;
> >   */
> >  static int cdns_uart_probe(struct platform_device *pdev)  {
> > +	u32 val;
> >  	int rc, id, irq;
> >  	struct uart_port *port;
> >  	struct resource *res;
> > @@ -1597,9 +1722,23 @@ static int cdns_uart_probe(struct
> platform_device *pdev)
> >  	port->private_data = cdns_uart_data;
> >  	port->read_status_mask = CDNS_UART_IXR_TXEMPTY |
> CDNS_UART_IXR_RXTRIG |
> >  			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;
> > +	port->rs485_config = cdns_rs485_config;
> > +	port->rs485_supported = cdns_rs485_supported;
> >  	cdns_uart_data->port = port;
> >  	platform_set_drvdata(pdev, port);
> >
> > +	rc = uart_get_rs485_mode(port);
> > +	if (rc)
> > +		goto err_out_clk_notifier;
> > +
> > +	cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev,
> "rts",
> > +							GPIOD_OUT_LOW);
> > +	if (IS_ERR(cdns_uart_data->gpiod)) {
> > +		rc = PTR_ERR(cdns_uart_data->gpiod);
> > +		dev_err(port->dev, "xuartps: devm_gpiod_get_optional
> failed\n");
> 
> Why bail out with an error if having cdns_uart_data->gpiod is only optional?
We are using devm_gpiod_get_optional(), it will return NULL when gpio not passed, so IS_ERR() check will be false and will not  bail out.
> 
> > +		goto err_out_clk_notifier;
> > +	}
> > +
> >  	pm_runtime_use_autosuspend(&pdev->dev);
> >  	pm_runtime_set_autosuspend_delay(&pdev->dev,
> UART_AUTOSUSPEND_TIMEOUT);
> >  	pm_runtime_set_active(&pdev->dev);
> > @@ -1638,6 +1777,16 @@ static int cdns_uart_probe(struct
> platform_device *pdev)
> >  	cdns_uart_data->cts_override = of_property_read_bool(pdev-
> >dev.of_node,
> >  							     "cts-override");
> >
> > +	if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) {
> > +		if (!cdns_uart_data->gpiod) {
> > +			val = readl(cdns_uart_data->port->membase
> > +				    + CDNS_UART_MODEMCR);
> > +			val |= CDNS_UART_MODEMCR_RTS;
> > +			writel(val, cdns_uart_data->port->membase
> > +			       + CDNS_UART_MODEMCR);
> > +		}
> > +	}
> 
> This covers the RTS_AFTER_SEND mode. What if SER_RS485_RTS_ON_SEND is
> configured instead (as it is the default set by uart_get_rs485_mode())? What if
> cdns_uart_data->gpiod exists?
> Why not simply call cdns_rs485_rx_setup() which covers all these cases?
> Note that uart_add_one_port() will call into the serial core and eventually
> result in an initial call to the ports rs485_config function (see
> uart_rs485_config()). So maybe put the initial configuration into that function
> and remove the above code. However in this case
> 
> cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,
> 							     "cts-override");
> should be called before uart_add_one_port().
We will fix.

Thanks,
Manikanta.
Lino Sanfilippo Nov. 18, 2023, 7:35 p.m. UTC | #4
Hi,

On 15.11.23 07:50, Guntupalli, Manikanta wrote:

>> From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
>>
>> So, this will be executed each time (including the rts_before_send delay) the
>> core wants to send data? This is not how it is supposed to work: The tx setup
>> (and the delay before send) has to be done once when transmission starts.
>> Note that when sending a bulk of data the core may call cdns_uart_start_tx()
>> several times before it eventually calls cdns_uart_stop_tx() to stop the
>> transmission.
> We have tested bulk transfer (5000 bytes) and observed that cdns_uart_start_tx() is getting called only once. Can you please suggest how to reproduce the case where the core may call cdns_uart_start_tx() several times before it eventually calls cdns_uart_stop_tx().


Thats strange. Normally the uart_ports start_tx() function is called whenever there is new
data in the tty circular buffer (see uart_write()) and the writing process is suspended after
that (see n_tty_write() in case of N_TTY line discipline). The writing process is woken up via
uart_write_wakeup() called from the uart driver and then it writes new data into the circular
buffer which results in another call to the uart_ports start_tx(). There is no stop_tx() until
all data from the passed userspace buffer is written. But there is a start_tx for every new bulk
of data that is available in the circular buffer.
This is at least what I can observe in my test setup (using a PL011 UART with the amba driver). If
I write a test buffer of 9212 bytes to the tty device using one single write() I can see 10
consecutive calls of tx_start() before tx_stop() eventually is called. What does your test setup look like?

Regards,
Lino
diff mbox series

Patch

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 9c13dac1d4d1..32229cf5c508 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -23,6 +23,9 @@ 
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/iopoll.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
 
 #define CDNS_UART_TTY_NAME	"ttyPS"
 #define CDNS_UART_NAME		"xuartps"
@@ -193,6 +196,7 @@  MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
  * @clk_rate_change_nb:	Notifier block for clock changes
  * @quirks:		Flags for RXBS support.
  * @cts_override:	Modem control state override
+ * @gpiod:		Pointer to the gpio descriptor
  */
 struct cdns_uart {
 	struct uart_port	*port;
@@ -203,10 +207,19 @@  struct cdns_uart {
 	struct notifier_block	clk_rate_change_nb;
 	u32			quirks;
 	bool cts_override;
+	struct gpio_desc	*gpiod;
 };
 struct cdns_platform_data {
 	u32 quirks;
 };
+
+struct serial_rs485 cdns_rs485_supported = {
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
+		 SER_RS485_RTS_AFTER_SEND,
+	.delay_rts_before_send = 1,
+	.delay_rts_after_send = 1,
+};
+
 #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
 		clk_rate_change_nb)
 
@@ -305,6 +318,79 @@  static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus)
 	tty_flip_buffer_push(&port->state->port);
 }
 
+/**
+ * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high
+ * @cdns_uart: Handle to the cdns_uart
+ */
+static void cdns_rs485_config_gpio_rts_high(struct cdns_uart *cdns_uart)
+{
+	u32 val;
+
+	if (cdns_uart->gpiod) {
+		gpiod_set_value(cdns_uart->gpiod, 1);
+	} else {
+		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
+		val &= ~CDNS_UART_MODEMCR_RTS;
+		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
+	}
+}
+
+/**
+ * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low
+ * @cdns_uart: Handle to the cdns_uart
+ */
+static void cdns_rs485_config_gpio_rts_low(struct cdns_uart *cdns_uart)
+{
+	u32 val;
+
+	if (cdns_uart->gpiod) {
+		gpiod_set_value(cdns_uart->gpiod, 0);
+	} else {
+		val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR);
+		val |= CDNS_UART_MODEMCR_RTS;
+		writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR);
+	}
+}
+
+/**
+ * cdns_rs485_tx_setup - Tx setup specific to rs485
+ * @cdns_uart: Handle to the cdns_uart
+ */
+static void cdns_rs485_tx_setup(struct cdns_uart *cdns_uart)
+{
+	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND)
+		cdns_rs485_config_gpio_rts_high(cdns_uart);
+	else
+		cdns_rs485_config_gpio_rts_low(cdns_uart);
+}
+
+/**
+ * cdns_rs485_rx_setup - Rx setup specific to rs485
+ * @cdns_uart: Handle to the cdns_uart
+ */
+static void cdns_rs485_rx_setup(struct cdns_uart *cdns_uart)
+{
+	if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		cdns_rs485_config_gpio_rts_high(cdns_uart);
+	else
+		cdns_rs485_config_gpio_rts_low(cdns_uart);
+}
+
+/**
+ * cdns_uart_tx_empty -  Check whether TX is empty
+ * @port: Handle to the uart port structure
+ *
+ * Return: TIOCSER_TEMT on success, 0 otherwise
+ */
+static unsigned int cdns_uart_tx_empty(struct uart_port *port)
+{
+	unsigned int status;
+
+	status = readl(port->membase + CDNS_UART_SR) &
+		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
+	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
+}
+
 /**
  * cdns_uart_handle_tx - Handle the bytes to be Txed.
  * @dev_id: Id of the UART port
@@ -571,6 +657,8 @@  static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
 static void cdns_uart_start_tx(struct uart_port *port)
 {
 	unsigned int status;
+	unsigned long time_out;
+	struct cdns_uart *cdns_uart = port->private_data;
 
 	if (uart_tx_stopped(port))
 		return;
@@ -589,8 +677,31 @@  static void cdns_uart_start_tx(struct uart_port *port)
 
 	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_ISR);
 
+	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
+		cdns_rs485_tx_setup(cdns_uart);
+		if (cdns_uart->port->rs485.delay_rts_before_send)
+			mdelay(cdns_uart->port->rs485.delay_rts_before_send);
+	}
+
 	cdns_uart_handle_tx(port);
 
+	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
+		time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT);
+		/* Wait for tx completion */
+		while ((cdns_uart_tx_empty(cdns_uart->port) != TIOCSER_TEMT) &&
+		       time_before(jiffies, time_out))
+			cpu_relax();
+
+		if (cdns_uart->port->rs485.delay_rts_after_send)
+			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
+
+		/*
+		 * Default Rx should be setup, because RX signaling path
+		 * need to enable to receive data.
+		 */
+		cdns_rs485_rx_setup(cdns_uart);
+	}
+
 	/* Enable the TX Empty interrupt */
 	writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IER);
 }
@@ -602,6 +713,14 @@  static void cdns_uart_start_tx(struct uart_port *port)
 static void cdns_uart_stop_tx(struct uart_port *port)
 {
 	unsigned int regval;
+	struct cdns_uart *cdns_uart = port->private_data;
+
+	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) {
+		if (cdns_uart->port->rs485.delay_rts_after_send)
+			mdelay(cdns_uart->port->rs485.delay_rts_after_send);
+
+		cdns_rs485_rx_setup(cdns_uart);
+	}
 
 	regval = readl(port->membase + CDNS_UART_CR);
 	regval |= CDNS_UART_CR_TX_DIS;
@@ -626,21 +745,6 @@  static void cdns_uart_stop_rx(struct uart_port *port)
 	writel(regval, port->membase + CDNS_UART_CR);
 }
 
-/**
- * cdns_uart_tx_empty -  Check whether TX is empty
- * @port: Handle to the uart port structure
- *
- * Return: TIOCSER_TEMT on success, 0 otherwise
- */
-static unsigned int cdns_uart_tx_empty(struct uart_port *port)
-{
-	unsigned int status;
-
-	status = readl(port->membase + CDNS_UART_SR) &
-		       (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
-	return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;
-}
-
 /**
  * cdns_uart_break_ctl - Based on the input ctl we have to start or stop
  *			transmitting char breaks
@@ -829,6 +933,9 @@  static int cdns_uart_startup(struct uart_port *port)
 		(CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST))
 		cpu_relax();
 
+	if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED)
+		cdns_rs485_rx_setup(cdns_uart);
+
 	/*
 	 * Clear the RX disable bit and then set the RX enable bit to enable
 	 * the receiver.
@@ -1455,6 +1562,23 @@  MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
 /* Temporary variable for storing number of instances */
 static int instances;
 
+/**
+ * cdns_rs485_config - Called when an application calls TIOCSRS485 ioctl.
+ * @port: Pointer to the uart_port structure
+ * @termios: Pointer to the ktermios structure
+ * @rs485: Pointer to the serial_rs485 structure
+ *
+ * Return: 0
+ */
+static int cdns_rs485_config(struct uart_port *port, struct ktermios *termios,
+			     struct serial_rs485 *rs485)
+{
+	if (rs485->flags & SER_RS485_ENABLED)
+		dev_dbg(port->dev, "Setting UART to RS485\n");
+
+	return 0;
+}
+
 /**
  * cdns_uart_probe - Platform driver probe
  * @pdev: Pointer to the platform device structure
@@ -1463,6 +1587,7 @@  static int instances;
  */
 static int cdns_uart_probe(struct platform_device *pdev)
 {
+	u32 val;
 	int rc, id, irq;
 	struct uart_port *port;
 	struct resource *res;
@@ -1597,9 +1722,23 @@  static int cdns_uart_probe(struct platform_device *pdev)
 	port->private_data = cdns_uart_data;
 	port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG |
 			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;
+	port->rs485_config = cdns_rs485_config;
+	port->rs485_supported = cdns_rs485_supported;
 	cdns_uart_data->port = port;
 	platform_set_drvdata(pdev, port);
 
+	rc = uart_get_rs485_mode(port);
+	if (rc)
+		goto err_out_clk_notifier;
+
+	cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev, "rts",
+							GPIOD_OUT_LOW);
+	if (IS_ERR(cdns_uart_data->gpiod)) {
+		rc = PTR_ERR(cdns_uart_data->gpiod);
+		dev_err(port->dev, "xuartps: devm_gpiod_get_optional failed\n");
+		goto err_out_clk_notifier;
+	}
+
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, UART_AUTOSUSPEND_TIMEOUT);
 	pm_runtime_set_active(&pdev->dev);
@@ -1638,6 +1777,16 @@  static int cdns_uart_probe(struct platform_device *pdev)
 	cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,
 							     "cts-override");
 
+	if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) {
+		if (!cdns_uart_data->gpiod) {
+			val = readl(cdns_uart_data->port->membase
+				    + CDNS_UART_MODEMCR);
+			val |= CDNS_UART_MODEMCR_RTS;
+			writel(val, cdns_uart_data->port->membase
+			       + CDNS_UART_MODEMCR);
+		}
+	}
+
 	instances++;
 
 	return 0;
@@ -1646,6 +1795,7 @@  static int cdns_uart_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
+err_out_clk_notifier:
 #ifdef CONFIG_COMMON_CLK
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);