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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux