Add optional gpio property to uartps node to support rs485.
Add rs485 support to uartps driver.
---
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.
Manikanta Guntupalli (2):
dt-bindings: Add optional gpio property to uartps node to support
rs485
tty: serial: uartps: Add rs485 support to uartps driver
.../devicetree/bindings/serial/cdns,uart.yaml | 6 +
drivers/tty/serial/xilinx_uartps.c | 180 ++++++++++++++++--
2 files changed, 171 insertions(+), 15 deletions(-)
--
2.25.1
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 <[email protected]>
---
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);
+ }
+
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);
--
2.25.1
Add optional gpio property to uartps node and reference to rs485.yaml
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 are shorted to each other,
and at a time, any node acts as either a driver or a receiver.
Here,
DE - Driver enable. If pin is floating, driver is disabled.
RE - Receiver enable. If pin is floating, receiver buffer is disabled.
For more deatils, please find below link which contains Transceiver
device(ISOW1432) datasheet
https://www.ti.com/lit/ds/symlink/isow1432.pdf?ts=1682607122706&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FISOW1432%252Fpart-details%252FISOW1432DFMR%253FkeyMatch%253DISOW1432DFMR%2526tisearch%253Dsearch-everything%2526usecase%253DOPN
rts-gpios is optional property, because it is not required
for uart console node.
Signed-off-by: Manikanta Guntupalli <[email protected]>
---
Changes for V2:
Modify optional gpio name to xlnx,phy-ctrl-gpios.
Update commit description.
Changes for V3:
Modify optional gpio name to rts-gpios.
Update commit description.
---
Documentation/devicetree/bindings/serial/cdns,uart.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
index e35ad1109efc..7ee305f9a45f 100644
--- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
+++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
@@ -46,6 +46,11 @@ properties:
power-domains:
maxItems: 1
+ rts-gpios:
+ description: Optional GPIO to control transmit/receive on RS485 phy
+ in halfduplex mode.
+ maxItems: 1
+
required:
- compatible
- reg
@@ -55,6 +60,7 @@ required:
allOf:
- $ref: serial.yaml#
+ - $ref: rs485.yaml#
- if:
properties:
compatible:
--
2.25.1
On Tue, 24 Oct 2023, 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 <[email protected]>
> ---
> 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);
Split to two lines since you need two lines anyway.
> + 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();
Use iopoll.h helper instead of handcrafted delay loop ?
> +
> + 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;
> -}
This is just a relocation of code? Move it in another patch in the
series, don't put it within the feature patch..
> -
> /**
> * 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) {
Combine the if conditions into a single if.
> + val = readl(cdns_uart_data->port->membase
> + + CDNS_UART_MODEMCR);
One line.
> + val |= CDNS_UART_MODEMCR_RTS;
> + writel(val, cdns_uart_data->port->membase
> + + CDNS_UART_MODEMCR);
Ditto.
> + }
> + }
> +
> 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);
>
--
i.
On Tue, Oct 24, 2023 at 08:18:46PM +0530, Manikanta Guntupalli wrote:
> Add optional gpio property to uartps node and reference to rs485.yaml
>
> 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 are shorted to each other,
> and at a time, any node acts as either a driver or a receiver.
>
> Here,
> DE - Driver enable. If pin is floating, driver is disabled.
> RE - Receiver enable. If pin is floating, receiver buffer is disabled.
What happens when pin is not floating? Is floating (i.e. open drain) for
RTS a requirement? And floating doesn't define high or low because it
could be pulled either way.
>
> For more deatils, please find below link which contains Transceiver
> device(ISOW1432) datasheet
> https://www.ti.com/lit/ds/symlink/isow1432.pdf?ts=1682607122706&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FISOW1432%252Fpart-details%252FISOW1432DFMR%253FkeyMatch%253DISOW1432DFMR%2526tisearch%253Dsearch-everything%2526usecase%253DOPN
>
> rts-gpios is optional property, because it is not required
> for uart console node.
>
> Signed-off-by: Manikanta Guntupalli <[email protected]>
> ---
> Changes for V2:
> Modify optional gpio name to xlnx,phy-ctrl-gpios.
> Update commit description.
> Changes for V3:
> Modify optional gpio name to rts-gpios.
> Update commit description.
> ---
> Documentation/devicetree/bindings/serial/cdns,uart.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> index e35ad1109efc..7ee305f9a45f 100644
> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> @@ -46,6 +46,11 @@ properties:
> power-domains:
> maxItems: 1
>
> + rts-gpios:
> + description: Optional GPIO to control transmit/receive on RS485 phy
> + in halfduplex mode.
You need to define what 'active' means here because -gpios all have an
active flag. Is it always active low (or high) or depends on the board?
> + maxItems: 1
> +
> required:
> - compatible
> - reg
> @@ -55,6 +60,7 @@ required:
>
> allOf:
> - $ref: serial.yaml#
> + - $ref: rs485.yaml#
> - if:
> properties:
> compatible:
> --
> 2.25.1
>
Manikanta Guntupalli schreef op 2023-10-24 16:48:
> Add optional gpio property to uartps node and reference to rs485.yaml
>
> 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 are shorted to each
> other,
> and at a time, any node acts as either a driver or a receiver.
>
> Here,
> DE - Driver enable. If pin is floating, driver is disabled.
> RE - Receiver enable. If pin is floating, receiver buffer is disabled.
>
> For more deatils, please find below link which contains Transceiver
> device(ISOW1432) datasheet
> https://www.ti.com/lit/ds/symlink/isow1432.pdf?ts=1682607122706&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FISOW1432%252Fpart-details%252FISOW1432DFMR%253FkeyMatch%253DISOW1432DFMR%2526tisearch%253Dsearch-everything%2526usecase%253DOPN
>
> rts-gpios is optional property, because it is not required
> for uart console node.
>
> Signed-off-by: Manikanta Guntupalli <[email protected]>
> ---
> Changes for V2:
> Modify optional gpio name to xlnx,phy-ctrl-gpios.
> Update commit description.
> Changes for V3:
> Modify optional gpio name to rts-gpios.
> Update commit description.
> ---
> Documentation/devicetree/bindings/serial/cdns,uart.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> index e35ad1109efc..7ee305f9a45f 100644
> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> @@ -46,6 +46,11 @@ properties:
> power-domains:
> maxItems: 1
>
> + rts-gpios:
> + description: Optional GPIO to control transmit/receive on RS485
> phy
> + in halfduplex mode.
> + maxItems: 1
> +
Why would this be related to RS485? A user could also have a need for a
gpio instead of the native pin to be used as normal rts.
All RS485 references can be removed.
> required:
> - compatible
> - reg
> @@ -55,6 +60,7 @@ required:
>
> allOf:
> - $ref: serial.yaml#
> + - $ref: rs485.yaml#
> - if:
> properties:
> compatible:
Maarten
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
Hi,
> -----Original Message-----
> From: Lino Sanfilippo <[email protected]>
> Sent: Monday, November 13, 2023 1:08 AM
> To: Guntupalli, Manikanta <[email protected]>; git (AMD-
> Xilinx) <[email protected]>; Simek, Michal <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]
> Cc: Pandey, Radhey Shyam <[email protected]>; Goud,
> Srinivas <[email protected]>; Datta, Shubhrajyoti
> <[email protected]>; [email protected]
> 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 <[email protected]>
> > ---
> > 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.
Hi,
On 15.11.23 07:50, Guntupalli, Manikanta wrote:
>> From: Lino Sanfilippo <[email protected]>
>>
>> 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