| Message ID | 20231011145602.3619616-3-manikanta.guntupalli@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add rs485 support to uartps driver | expand |
Manikanta Guntupalli wrote on 2023-10-11 16:56: > In RS485 half duplex configuration, DriverEnable and ReceiverEnable > shorted to each other, and at a time, any node acts as either a driver > or a receiver. Use either xlnx,phy-ctrl-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. > --- > drivers/tty/serial/xilinx_uartps.c | 116 ++++++++++++++++++++++++++++- > 1 file changed, 115 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/xilinx_uartps.c > b/drivers/tty/serial/xilinx_uartps.c > index 8e521c69a959..abddcf1a8bf4 100644 > --- a/drivers/tty/serial/xilinx_uartps.c > +++ b/drivers/tty/serial/xilinx_uartps.c > @@ -23,6 +23,7 @@ > #include <linux/module.h> > #include <linux/pm_runtime.h> > #include <linux/iopoll.h> > +#include <linux/gpio.h> > > #define CDNS_UART_TTY_NAME "ttyPS" > #define CDNS_UART_NAME "xuartps" > @@ -193,6 +194,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 +205,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, You promise here to support both RTS-on-send and RTS-after-send, but... > + .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 +316,42 @@ static void cdns_uart_handle_rx(void *dev_id, > unsigned int isrstatus) > tty_flip_buffer_push(&port->state->port); > } > > +/** > + * 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) > +{ > + 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); Here you don't care about RTS-on-send or RTS-after-send anymore. And neither do you btw. in the if clause. > + } > +} > + > +/** > + * 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) > +{ > + 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); > + } Same here. > +} > + > +static unsigned int cdns_uart_tx_empty(struct uart_port *port); > + I think it's better to move up the implementation than to use a forward declaration. > /** > * cdns_uart_handle_tx - Handle the bytes to be Txed. > * @dev_id: Id of the UART port > @@ -313,12 +360,20 @@ static void cdns_uart_handle_rx(void *dev_id, > unsigned int isrstatus) > static void cdns_uart_handle_tx(void *dev_id) > { > struct uart_port *port = (struct uart_port *)dev_id; > + struct cdns_uart *cdns_uart = port->private_data; > struct circ_buf *xmit = &port->state->xmit; > + unsigned long time_out; > unsigned int numbytes; > > + 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); mdelay? https://www.kernel.org/doc/html/latest/timers/timers-howto.html "In general, use of mdelay is discouraged and code should be refactored to allow for the use of msleep." Furthermore, you're delaying before every burst of bytes here! Every TXEMPTY interrupt! > + } > + > if (uart_circ_empty(xmit)) { > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR); > - return; > + goto rs485_rx_setup; And when there was nothing more to send you waited for nothing. > } > > numbytes = port->fifosize; > @@ -332,6 +387,23 @@ static void cdns_uart_handle_tx(void *dev_id) > > if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) > uart_write_wakeup(port); > + > +rs485_rx_setup: > + 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(); > + > + /* > + * Default Rx should be setup, because RX signaling path > + * need to enable to receive data. > + */ > + cdns_rs485_rx_setup(cdns_uart); > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); This is not delaying rts after send. You must keep RTS aka DE active for a little longer so even the last stop bit(s) are transmitted correctly. So this delay must happen before cdns_rs485_rx_setup(). > + } > } > > /** > @@ -829,6 +901,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 +1530,25 @@ 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) > +{ > + port->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 > @@ -1597,9 +1691,28 @@ 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, > "xlnx,phy-ctrl", > + 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; > + } > + > + if (cdns_uart_data->gpiod) { > + gpiod_direction_output(cdns_uart_data->gpiod, GPIOD_OUT_LOW); > + gpiod_set_value(cdns_uart_data->gpiod, 0); > + } > + > pm_runtime_use_autosuspend(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, > UART_AUTOSUSPEND_TIMEOUT); > pm_runtime_set_active(&pdev->dev); > @@ -1646,6 +1759,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); Maarten
Hi, > -----Original Message----- > From: m.brock@vanmierlo.com <m.brock@vanmierlo.com> > Sent: Friday, October 13, 2023 12:35 AM > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com> > Cc: 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; 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 V2 2/2] tty: serial: uartps: Add rs485 support to uartps > driver > > Manikanta Guntupalli wrote on 2023-10-11 16:56: > > In RS485 half duplex configuration, DriverEnable and ReceiverEnable > > shorted to each other, and at a time, any node acts as either a driver > > or a receiver. Use either xlnx,phy-ctrl-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. > > --- > > drivers/tty/serial/xilinx_uartps.c | 116 > > ++++++++++++++++++++++++++++- > > 1 file changed, 115 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/xilinx_uartps.c > > b/drivers/tty/serial/xilinx_uartps.c > > index 8e521c69a959..abddcf1a8bf4 100644 > > --- a/drivers/tty/serial/xilinx_uartps.c > > +++ b/drivers/tty/serial/xilinx_uartps.c > > @@ -23,6 +23,7 @@ > > #include <linux/module.h> > > #include <linux/pm_runtime.h> > > #include <linux/iopoll.h> > > +#include <linux/gpio.h> > > > > #define CDNS_UART_TTY_NAME "ttyPS" > > #define CDNS_UART_NAME "xuartps" > > @@ -193,6 +194,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 +205,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, > > You promise here to support both RTS-on-send and RTS-after-send, but... > > > + .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 +316,42 @@ static void cdns_uart_handle_rx(void *dev_id, > > unsigned int isrstatus) > > tty_flip_buffer_push(&port->state->port); > > } > > > > +/** > > + * 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) { > > + 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); > > Here you don't care about RTS-on-send or RTS-after-send anymore. > And neither do you btw. in the if clause. On Xilinx/AMD Kria SOM KD240 board rs485 connects via TI ISOW1432 Transceiver device, where one GPIO is used for driving DE/RE signals. With rs485 half duplex configuration, DE and RE shorted to each other, and at a time, any node acts as either a driver or a receiver. We recommend using either GPIO or RTS to control RS485 phy as driver or a receiver. So, we fall back to RTS, if rts-gpios not passed. > > > + } > > +} > > + > > +/** > > + * 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) { > > + 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); > > + } > > Same here. > > > +} > > + > > +static unsigned int cdns_uart_tx_empty(struct uart_port *port); > > + > > I think it's better to move up the implementation than to use a forward > declaration. We will fix. > > > /** > > * cdns_uart_handle_tx - Handle the bytes to be Txed. > > * @dev_id: Id of the UART port > > @@ -313,12 +360,20 @@ static void cdns_uart_handle_rx(void *dev_id, > > unsigned int isrstatus) static void cdns_uart_handle_tx(void *dev_id) > > { > > struct uart_port *port = (struct uart_port *)dev_id; > > + struct cdns_uart *cdns_uart = port->private_data; > > struct circ_buf *xmit = &port->state->xmit; > > + unsigned long time_out; > > unsigned int numbytes; > > > > + 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); > > mdelay? > https://www.kernel.org/doc/html/latest/timers/timers-howto.html > "In general, use of mdelay is discouraged and code should be refactored to > allow for the use of msleep." For refactoring the code will move this code snippet to start_tx, even there, interrupts are disabled, so can't use msleep. https://docs.kernel.org/driver-api/serial/driver.html Documentation recommends, start_tx void ()(struct uart_port *port) Start transmitting characters. Locking: port->lock taken. Interrupts: locally disabled. This call must not sleep. > > Furthermore, you're delaying before every burst of bytes here! > Every TXEMPTY interrupt! We will move code to start_tx. > > > + } > > + > > if (uart_circ_empty(xmit)) { > > writel(CDNS_UART_IXR_TXEMPTY, port->membase + > CDNS_UART_IDR); > > - return; > > + goto rs485_rx_setup; > > And when there was nothing more to send you waited for nothing. We will fix. > > > } > > > > numbytes = port->fifosize; > > @@ -332,6 +387,23 @@ static void cdns_uart_handle_tx(void *dev_id) > > > > if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) > > uart_write_wakeup(port); > > + > > +rs485_rx_setup: > > + 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(); > > + > > + /* > > + * Default Rx should be setup, because RX signaling path > > + * need to enable to receive data. > > + */ > > + cdns_rs485_rx_setup(cdns_uart); > > + if (cdns_uart->port->rs485.delay_rts_after_send) > > + mdelay(cdns_uart->port- > >rs485.delay_rts_after_send); > > This is not delaying rts after send. You must keep RTS aka DE active for a little > longer so even the last stop bit(s) are transmitted correctly. So this delay must > happen before cdns_rs485_rx_setup(). We will fix. > > > + } > > } > > > > /** > > @@ -829,6 +901,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 +1530,25 @@ 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) > > +{ > > + port->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 @@ -1597,9 > > +1691,28 @@ 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, > > "xlnx,phy-ctrl", > > + 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; > > + } > > + > > + if (cdns_uart_data->gpiod) { > > + gpiod_direction_output(cdns_uart_data->gpiod, > GPIOD_OUT_LOW); > > + gpiod_set_value(cdns_uart_data->gpiod, 0); > > + } > > + > > pm_runtime_use_autosuspend(&pdev->dev); > > pm_runtime_set_autosuspend_delay(&pdev->dev, > > UART_AUTOSUSPEND_TIMEOUT); > > pm_runtime_set_active(&pdev->dev); > > @@ -1646,6 +1759,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); > > Maarten Thanks, Manikanta.
On 11.10.23 at 16:56, Manikanta Guntupalli wrote: > In RS485 half duplex configuration, DriverEnable and ReceiverEnable > shorted to each other, and at a time, any node acts as either a driver > or a receiver. Use either xlnx,phy-ctrl-gpios or RTS to control > RS485 phy as driver or a receiver. This is a very specific hardware setup. RS485 is supposed to work with various hardware configurations though. So the above assumptions must not be hardcoded in the RS485 implementation (as it is in cdns_rs485_rx_setup() and cdns_rs485_tx_setup() IIUC) > > 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. > --- > drivers/tty/serial/xilinx_uartps.c | 116 ++++++++++++++++++++++++++++- > 1 file changed, 115 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c > index 8e521c69a959..abddcf1a8bf4 100644 > --- a/drivers/tty/serial/xilinx_uartps.c > +++ b/drivers/tty/serial/xilinx_uartps.c > @@ -23,6 +23,7 @@ > #include <linux/module.h> > #include <linux/pm_runtime.h> > #include <linux/iopoll.h> > +#include <linux/gpio.h> > > #define CDNS_UART_TTY_NAME "ttyPS" > #define CDNS_UART_NAME "xuartps" > @@ -193,6 +194,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 +205,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 +316,42 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) > tty_flip_buffer_push(&port->state->port); > } > > +/** > + * 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) > +{ > + u32 val; > + > + if (cdns_uart->gpiod) { If both RTS_ON_SEND and RTS_AFTER_SEND are supported than you have to set the GPIO value here depending on which one is currently configured. Note that userspace may change RTS mode via TIOCSRS485 ioctl. > + 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); Same for the native RTS case. > + } > +} > + > +/** > + * 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) > +{ > + 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); > + } See above. > +} > + > +static unsigned int cdns_uart_tx_empty(struct uart_port *port); > + > /** > * cdns_uart_handle_tx - Handle the bytes to be Txed. > * @dev_id: Id of the UART port > @@ -313,12 +360,20 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) > static void cdns_uart_handle_tx(void *dev_id) > { > struct uart_port *port = (struct uart_port *)dev_id; > + struct cdns_uart *cdns_uart = port->private_data; > struct circ_buf *xmit = &port->state->xmit; > + unsigned long time_out; > unsigned int numbytes; > > + 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); > + } > + > if (uart_circ_empty(xmit)) { > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR); > - return; > + goto rs485_rx_setup; > } > > numbytes = port->fifosize; > @@ -332,6 +387,23 @@ static void cdns_uart_handle_tx(void *dev_id) > > if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) > uart_write_wakeup(port); > + > +rs485_rx_setup: > + 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(); > + > + /* > + * Default Rx should be setup, because RX signaling path > + * need to enable to receive data. > + */ > + cdns_rs485_rx_setup(cdns_uart); > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); > + } > } > > /** > @@ -829,6 +901,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 +1530,25 @@ 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) > +{ > + port->rs485 = *rs485; > + This assignment is not needed, the serial core will do that. > + 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 > @@ -1597,9 +1691,28 @@ 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, "xlnx,phy-ctrl", > + 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; > + } > + If using the GPIO is optional, why not fall back to the use of the native RTS? > + if (cdns_uart_data->gpiod) { > + gpiod_direction_output(cdns_uart_data->gpiod, GPIOD_OUT_LOW); > + gpiod_set_value(cdns_uart_data->gpiod, 0); You already set both of these attributes when you requested the GPIO. > + } > + > pm_runtime_use_autosuspend(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, UART_AUTOSUSPEND_TIMEOUT); > pm_runtime_set_active(&pdev->dev); > @@ -1646,6 +1759,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); > Regards, Lino
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 8e521c69a959..abddcf1a8bf4 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -23,6 +23,7 @@ #include <linux/module.h> #include <linux/pm_runtime.h> #include <linux/iopoll.h> +#include <linux/gpio.h> #define CDNS_UART_TTY_NAME "ttyPS" #define CDNS_UART_NAME "xuartps" @@ -193,6 +194,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 +205,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 +316,42 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) tty_flip_buffer_push(&port->state->port); } +/** + * 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) +{ + 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_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) +{ + 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); + } +} + +static unsigned int cdns_uart_tx_empty(struct uart_port *port); + /** * cdns_uart_handle_tx - Handle the bytes to be Txed. * @dev_id: Id of the UART port @@ -313,12 +360,20 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) static void cdns_uart_handle_tx(void *dev_id) { struct uart_port *port = (struct uart_port *)dev_id; + struct cdns_uart *cdns_uart = port->private_data; struct circ_buf *xmit = &port->state->xmit; + unsigned long time_out; unsigned int numbytes; + 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); + } + if (uart_circ_empty(xmit)) { writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR); - return; + goto rs485_rx_setup; } numbytes = port->fifosize; @@ -332,6 +387,23 @@ static void cdns_uart_handle_tx(void *dev_id) if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) uart_write_wakeup(port); + +rs485_rx_setup: + 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(); + + /* + * Default Rx should be setup, because RX signaling path + * need to enable to receive data. + */ + cdns_rs485_rx_setup(cdns_uart); + if (cdns_uart->port->rs485.delay_rts_after_send) + mdelay(cdns_uart->port->rs485.delay_rts_after_send); + } } /** @@ -829,6 +901,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 +1530,25 @@ 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) +{ + port->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 @@ -1597,9 +1691,28 @@ 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, "xlnx,phy-ctrl", + 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; + } + + if (cdns_uart_data->gpiod) { + gpiod_direction_output(cdns_uart_data->gpiod, GPIOD_OUT_LOW); + gpiod_set_value(cdns_uart_data->gpiod, 0); + } + pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, UART_AUTOSUSPEND_TIMEOUT); pm_runtime_set_active(&pdev->dev); @@ -1646,6 +1759,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);
In RS485 half duplex configuration, DriverEnable and ReceiverEnable shorted to each other, and at a time, any node acts as either a driver or a receiver. Use either xlnx,phy-ctrl-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. --- drivers/tty/serial/xilinx_uartps.c | 116 ++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-)