[PATCH 0/2] Add rs485 support to uartps driver

[PATCH 0/2] Add rs485 support to uartps driver

[Manikanta Guntupalli]

Add optional gpio property to uartps node to support rs485
Add rs485 support to uartps driver

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 |  5 +
 drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
 2 files changed, 100 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Manikanta Guntupalli]

Add optional gpio property to uartps node and add reference to rs485.yaml

Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
---
 Documentation/devicetree/bindings/serial/cdns,uart.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
index a8b323d7bf94..34ed0d80b923 100644
--- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
+++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
@@ -46,6 +46,10 @@ properties:
   power-domains:
     maxItems: 1
 
+  txrx-gpios:
+    description: Optional GPIO to switch transmit and receive modes via pmod.
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -55,6 +59,7 @@ required:
 
 allOf:
   - $ref: serial.yaml#
+  - $ref: rs485.yaml#
   - if:
       properties:
         compatible:
-- 
2.25.1

Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Rob Herring]

On Wed, Apr 26, 2023 at 05:59:46PM +0530, Manikanta Guntupalli wrote:
> Add optional gpio property to uartps node and add reference to rs485.yaml

The diff tells me that already. Why?

> 
> Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> ---
>  Documentation/devicetree/bindings/serial/cdns,uart.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> index a8b323d7bf94..34ed0d80b923 100644
> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> @@ -46,6 +46,10 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  txrx-gpios:
> +    description: Optional GPIO to switch transmit and receive modes via pmod.

What's pmod?

> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg
> @@ -55,6 +59,7 @@ required:
>  
>  allOf:
>    - $ref: serial.yaml#
> +  - $ref: rs485.yaml#
>    - if:
>        properties:
>          compatible:
> -- 
> 2.25.1
> 

RE: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Guntupalli, Manikanta]

Hi Rob Herring,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, April 27, 2023 10:14 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Cc: gregkh@linuxfoundation.org; krzysztof.kozlowski+dt@linaro.org;
> michal.simek@xilinx.com; linux-serial@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> jirislaby@kernel.org; linux-arm-kernel@lists.infradead.org; Simek, Michal
> <michal.simek@amd.com>; git (AMD-Xilinx) <git@amd.com>; Pandey,
> Radhey Shyam <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> Subject: Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps
> node to support rs485
> 
> On Wed, Apr 26, 2023 at 05:59:46PM +0530, Manikanta Guntupalli wrote:
> > Add optional gpio property to uartps node and add reference to rs485.yaml
> 
> The diff tells me that already. Why?
Please check Figure 11-3 (Page number 37) in the below mentioned data sheet, It shows PMOD (ISOW14x2) devices used in half duplex configuration. Driver outputs Y and Z are shorted to A and B respectively. This reduces overall cabling requirements. Also DE/RE are shorted to each other, and at a time, any node acts as either a driver or a receiver.
Using above optional GPIO controlling driver or a receiver. This GPIO is optional because it is not required for uart console node.

Here,
DE - Driver enable. If pin is floating, driver is disabled (internal pull-down resistor)
RE - Receiver enable. If pin is floating, receiver buffer is disabled (internal pull-up resistor)

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

> 
> >
> > Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > ---
> >  Documentation/devicetree/bindings/serial/cdns,uart.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> > index a8b323d7bf94..34ed0d80b923 100644
> > --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
> > @@ -46,6 +46,10 @@ properties:
> >    power-domains:
> >      maxItems: 1
> >
> > +  txrx-gpios:
> > +    description: Optional GPIO to switch transmit and receive modes via
> pmod.
> 
> What's pmod?
Pmod is galvanically-isolated RS-485/RS-422 transceivers with a built-in isolated
DC-DC converter, that eliminates the need for a separate isolated power supply in
space constrained isolated designs.

> 
> > +    maxItems: 1
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -55,6 +59,7 @@ required:
> >
> >  allOf:
> >    - $ref: serial.yaml#
> > +  - $ref: rs485.yaml#
> >    - if:
> >        properties:
> >          compatible:
> > --
> > 2.25.1
> >

Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Krzysztof Kozlowski]

On 02/05/2023 12:14, Guntupalli, Manikanta wrote:
> Hi Rob Herring,
> 
>> -----Original Message-----
>> From: Rob Herring <robh@kernel.org>
>> Sent: Thursday, April 27, 2023 10:14 PM
>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>> Cc: gregkh@linuxfoundation.org; krzysztof.kozlowski+dt@linaro.org;
>> michal.simek@xilinx.com; linux-serial@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>> jirislaby@kernel.org; linux-arm-kernel@lists.infradead.org; Simek, Michal
>> <michal.simek@amd.com>; git (AMD-Xilinx) <git@amd.com>; Pandey,
>> Radhey Shyam <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>> manion05gk@gmail.com
>> Subject: Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps
>> node to support rs485
>>
>> On Wed, Apr 26, 2023 at 05:59:46PM +0530, Manikanta Guntupalli wrote:
>>> Add optional gpio property to uartps node and add reference to rs485.yaml
>>
>> The diff tells me that already. Why?
> Please check Figure 11-3 (Page number 37) in the below mentioned data sheet, It shows PMOD (ISOW14x2) devices used in half duplex configuration. Driver outputs Y and Z are shorted to A and B respectively. This reduces overall cabling requirements. Also DE/RE are shorted to each other, and at a time, any node acts as either a driver or a receiver.
> Using above optional GPIO controlling driver or a receiver. This GPIO is optional because it is not required for uart console node.
> 
> Here,
> DE - Driver enable. If pin is floating, driver is disabled (internal pull-down resistor)
> RE - Receiver enable. If pin is floating, receiver buffer is disabled (internal pull-up resistor)
> 
> 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

Your request for us to get into some datasheet somewhere, to get the
answer to the basic question, is not reasonable. Commit must justify its
existence, not some external source.

> 
>>
>>>
>>> Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
>>> ---
>>>  Documentation/devicetree/bindings/serial/cdns,uart.yaml | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
>> b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
>>> index a8b323d7bf94..34ed0d80b923 100644
>>> --- a/Documentation/devicetree/bindings/serial/cdns,uart.yaml
>>> +++ b/Documentation/devicetree/bindings/serial/cdns,uart.yaml
>>> @@ -46,6 +46,10 @@ properties:
>>>    power-domains:
>>>      maxItems: 1
>>>
>>> +  txrx-gpios:
>>> +    description: Optional GPIO to switch transmit and receive modes via
>> pmod.
>>
>> What's pmod?
> Pmod is galvanically-isolated RS-485/RS-422 transceivers with a built-in isolated
> DC-DC converter, that eliminates the need for a separate isolated power supply in
> space constrained isolated designs.

It's not about explaining it here, but making your changes justified and
obvious once implemented.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Michal Simek]

On 5/2/23 12:19, Krzysztof Kozlowski wrote:
> On 02/05/2023 12:14, Guntupalli, Manikanta wrote:
>> Hi Rob Herring,
>>
>>> -----Original Message-----
>>> From: Rob Herring <robh@kernel.org>
>>> Sent: Thursday, April 27, 2023 10:14 PM
>>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>>> Cc: gregkh@linuxfoundation.org; krzysztof.kozlowski+dt@linaro.org;
>>> michal.simek@xilinx.com; linux-serial@vger.kernel.org;
>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> jirislaby@kernel.org; linux-arm-kernel@lists.infradead.org; Simek, Michal
>>> <michal.simek@amd.com>; git (AMD-Xilinx) <git@amd.com>; Pandey,
>>> Radhey Shyam <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>>> manion05gk@gmail.com
>>> Subject: Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps
>>> node to support rs485
>>>
>>> On Wed, Apr 26, 2023 at 05:59:46PM +0530, Manikanta Guntupalli wrote:
>>>> Add optional gpio property to uartps node and add reference to rs485.yaml
>>>
>>> The diff tells me that already. Why?
>> Please check Figure 11-3 (Page number 37) in the below mentioned data sheet, It shows PMOD (ISOW14x2) devices used in half duplex configuration. Driver outputs Y and Z are shorted to A and B respectively. This reduces overall cabling requirements. Also DE/RE are shorted to each other, and at a time, any node acts as either a driver or a receiver.
>> Using above optional GPIO controlling driver or a receiver. This GPIO is optional because it is not required for uart console node.
>>
>> Here,
>> DE - Driver enable. If pin is floating, driver is disabled (internal pull-down resistor)
>> RE - Receiver enable. If pin is floating, receiver buffer is disabled (internal pull-up resistor)
>>
>> 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
> 
> Your request for us to get into some datasheet somewhere, to get the
> answer to the basic question, is not reasonable. Commit must justify its
> existence, not some external source.

Not really. Manual itself just shows picture of that connection if you are 
interested.
On schematics it looks like that DE and /RE are just handle via one gpio. That's 
why you need 3 wires instead of 4. Definitely it should be described better.

My concern was more about choosing proper name for this functionality.

Thanks,
Michal

Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[Krzysztof Kozlowski]

On 02/05/2023 12:57, Michal Simek wrote:
> 
> 
> On 5/2/23 12:19, Krzysztof Kozlowski wrote:
>> On 02/05/2023 12:14, Guntupalli, Manikanta wrote:
>>> Hi Rob Herring,
>>>
>>>> -----Original Message-----
>>>> From: Rob Herring <robh@kernel.org>
>>>> Sent: Thursday, April 27, 2023 10:14 PM
>>>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>>>> Cc: gregkh@linuxfoundation.org; krzysztof.kozlowski+dt@linaro.org;
>>>> michal.simek@xilinx.com; linux-serial@vger.kernel.org;
>>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>> jirislaby@kernel.org; linux-arm-kernel@lists.infradead.org; Simek, Michal
>>>> <michal.simek@amd.com>; git (AMD-Xilinx) <git@amd.com>; Pandey,
>>>> Radhey Shyam <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>>>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>>>> manion05gk@gmail.com
>>>> Subject: Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps
>>>> node to support rs485
>>>>
>>>> On Wed, Apr 26, 2023 at 05:59:46PM +0530, Manikanta Guntupalli wrote:
>>>>> Add optional gpio property to uartps node and add reference to rs485.yaml
>>>>
>>>> The diff tells me that already. Why?
>>> Please check Figure 11-3 (Page number 37) in the below mentioned data sheet, It shows PMOD (ISOW14x2) devices used in half duplex configuration. Driver outputs Y and Z are shorted to A and B respectively. This reduces overall cabling requirements. Also DE/RE are shorted to each other, and at a time, any node acts as either a driver or a receiver.
>>> Using above optional GPIO controlling driver or a receiver. This GPIO is optional because it is not required for uart console node.
>>>
>>> Here,
>>> DE - Driver enable. If pin is floating, driver is disabled (internal pull-down resistor)
>>> RE - Receiver enable. If pin is floating, receiver buffer is disabled (internal pull-up resistor)
>>>
>>> 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
>>
>> Your request for us to get into some datasheet somewhere, to get the
>> answer to the basic question, is not reasonable. Commit must justify its
>> existence, not some external source.
> 
> Not really. Manual itself just shows picture of that connection if you are 
> interested.
> On schematics it looks like that DE and /RE are just handle via one gpio. That's 
> why you need 3 wires instead of 4. Definitely it should be described better.

By looking at commit reviewer did not figure out why do we need it.
Referencing datasheet in a reply in LKML thread does not solve it
because the next person reading the commit might have the same question.

The commit must answer to "why" if this is not obvious. I don't find
responses "Please check..." to this helpful.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: Add optional gpio property to uartps node to support rs485

[m.brock]

Guntupalli, Manikanta wrote 2023-05-02 12:14:
>> > +  txrx-gpios:
>> > +    description: Optional GPIO to switch transmit and receive modes via
>> pmod.
>> 
>> What's pmod?
> Pmod is galvanically-isolated RS-485/RS-422 transceivers with a
> built-in isolated DC-DC converter, that eliminates the need for a
> separate isolated power supply in space constrained isolated designs.

AFAIK Pmod is an open standard connector by Digilent and present on many 
Xilinx
demo boards. It does not involve any galvanic isolation.
https://digilent.com/reference/_media/reference/pmod/pmod-interface-specification-1_3_1.pdf

The ISOW14x2 on the particular board you're using does however give 
isolation.
But you cannot name that board Pmod.

Maarten

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

[Manikanta Guntupalli]

Add rs485 half duplex mode support to uartps driver by making GPIO
high before tx and low before rx.

Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
---
 drivers/tty/serial/xilinx_uartps.c | 96 +++++++++++++++++++++++++++++-
 1 file changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 8e521c69a959..339b962460ee 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -23,6 +23,8 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/iopoll.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 
 #define CDNS_UART_TTY_NAME	"ttyPS"
 #define CDNS_UART_NAME		"xuartps"
@@ -193,6 +195,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 +206,16 @@ 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,
+};
+
 #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
 		clk_rate_change_nb)
 
@@ -305,6 +314,27 @@ 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)
+{
+	gpiod_set_value(cdns_uart->gpiod, 1);
+}
+
+/**
+ * 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)
+{
+	gpiod_set_value(cdns_uart->gpiod, 0);
+}
+
+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 +343,17 @@ 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 (uart_circ_empty(xmit)) {
 		writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR);
-		return;
+		goto rs485_rx_setup;
 	}
 
 	numbytes = port->fifosize;
@@ -332,6 +367,21 @@ 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);
+	}
 }
 
 /**
@@ -829,6 +879,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 +1508,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 +1669,30 @@ 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, "txrx",
+							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 +1739,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

Re: [PATCH 0/2] Add rs485 support to uartps driver

[m.brock]

Manikanta Guntupalli wrote 2023-04-26 14:29:
> Add optional gpio property to uartps node to support rs485
> Add rs485 support to uartps driver
> 
> 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 |  5 +
>  drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
>  2 files changed, 100 insertions(+), 1 deletion(-)

Why would you want to use a GPIO and not RTS for choosing the direction
as is more common in this case?
And have you thought about configuring the polarity?
How long will the signal be active before the real transmission begins
so the driver can settle?

Maarten

RE: [PATCH 0/2] Add rs485 support to uartps driver

[Guntupalli, Manikanta]

Hi Maarten,

> -----Original Message-----
> From: m.brock@vanmierlo.com <m.brock@vanmierlo.com>
> Sent: Thursday, May 4, 2023 5:52 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; michal.simek@xilinx.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; jirislaby@kernel.org; linux-arm-
> kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; git
> (AMD-Xilinx) <git@amd.com>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> Subject: Re: [PATCH 0/2] Add rs485 support to uartps driver
> 
> Manikanta Guntupalli wrote 2023-04-26 14:29:
> > Add optional gpio property to uartps node to support rs485 Add rs485
> > support to uartps driver
> >
> > 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 |  5 +
> >  drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
> >  2 files changed, 100 insertions(+), 1 deletion(-)
> 
> Why would you want to use a GPIO and not RTS for choosing the direction as
> is more common in this case?
In ZynqMp platform Cadence UART Controller RTS signal routed to external through the PL(Programmable Logic) design not through Multiplexed IO.

> And have you thought about configuring the polarity?
GPIO polarity configured through device tree property.

&uart0 {
        ...
        txrx-gpios = <&gpio 72 GPIO_ACTIVE_LOW>;
        linux,rs485-enabled-at-boot-time;
};

> How long will the signal be active before the real transmission begins so the
> driver can settle?
Default is RE(GPIO LOW) and while sending we drive the pin to HIGH. We wait for transmission completion, for that we check Transmitter state machine active status to ZERO and TX FIFO EMPTY. 

Thanks,
Manikanta.

Re: [PATCH 0/2] Add rs485 support to uartps driver

[Michal Simek]

On 5/10/23 18:26, Guntupalli, Manikanta wrote:
> Hi Maarten,
> 
>> -----Original Message-----
>> From: m.brock@vanmierlo.com <m.brock@vanmierlo.com>
>> Sent: Thursday, May 4, 2023 5:52 PM
>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; michal.simek@xilinx.com; linux-
>> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; jirislaby@kernel.org; linux-arm-
>> kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; git
>> (AMD-Xilinx) <git@amd.com>; Pandey, Radhey Shyam
>> <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>> manion05gk@gmail.com
>> Subject: Re: [PATCH 0/2] Add rs485 support to uartps driver
>>
>> Manikanta Guntupalli wrote 2023-04-26 14:29:
>>> Add optional gpio property to uartps node to support rs485 Add rs485
>>> support to uartps driver
>>>
>>> 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 |  5 +
>>>   drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
>>>   2 files changed, 100 insertions(+), 1 deletion(-)
>>
>> Why would you want to use a GPIO and not RTS for choosing the direction as
>> is more common in this case?
> In ZynqMp platform Cadence UART Controller RTS signal routed to external through the PL(Programmable Logic) design not through Multiplexed IO.

Just to extend this a little bit. Cadence IP has modem signals but they are not 
available over MIOs only over EMIO via PL logic. But board which this feature 
targets can't have connection to PL pins to be able to use any modem signal for 
this logic.

Thanks,
Michal

Re: [PATCH 0/2] Add rs485 support to uartps driver

[m.brock]

Guntupalli, Manikanta schreef op 2023-05-10 18:26:
> Hi Maarten,
> 
>> -----Original Message-----
>> From: m.brock@vanmierlo.com <m.brock@vanmierlo.com>
>> Sent: Thursday, May 4, 2023 5:52 PM
>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; michal.simek@xilinx.com; linux-
>> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; jirislaby@kernel.org; linux-arm-
>> kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; git
>> (AMD-Xilinx) <git@amd.com>; Pandey, Radhey Shyam
>> <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>> manion05gk@gmail.com
>> Subject: Re: [PATCH 0/2] Add rs485 support to uartps driver
>> 
>> Manikanta Guntupalli wrote 2023-04-26 14:29:
>> > Add optional gpio property to uartps node to support rs485 Add rs485
>> > support to uartps driver
>> >
>> > 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 |  5 +
>> >  drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
>> >  2 files changed, 100 insertions(+), 1 deletion(-)
>> 
>> Why would you want to use a GPIO and not RTS for choosing the 
>> direction as
>> is more common in this case?
> In ZynqMp platform Cadence UART Controller RTS signal routed to
> external through the PL(Programmable Logic) design not through
> Multiplexed IO.

Then why not route RXD & TXD to the PL as well and connect the module to 
a
PMOD connector connected to the PL? But I admit that a GPIO always works 
as
well.

>> And have you thought about configuring the polarity?
> GPIO polarity configured through device tree property.
> 
> &uart0 {
>         ...
>         txrx-gpios = <&gpio 72 GPIO_ACTIVE_LOW>;
>         linux,rs485-enabled-at-boot-time;
> };

Useable, but not honoring 
SER_RS485_RTS_ON_SEND/SER_RS485_RTS_AFTER_SEND.

>> How long will the signal be active before the real transmission begins 
>> so the
>> driver can settle?
> Default is RE(GPIO LOW) and while sending we drive the pin to HIGH. We
> wait for transmission completion, for that we check Transmitter state
> machine active status to ZERO and TX FIFO EMPTY.

How does that take delay_rts_before_send/delay_rts_after_send into 
account?
Not every driver switches direction as fast as you would like.

> Thanks,
> Manikanta.

Greetings,
Maarten

Re: [PATCH 0/2] Add rs485 support to uartps driver

[Michal Simek]

On 5/14/23 13:01, m.brock@vanmierlo.com wrote:
> Guntupalli, Manikanta schreef op 2023-05-10 18:26:
>> Hi Maarten,
>>
>>> -----Original Message-----
>>> From: m.brock@vanmierlo.com <m.brock@vanmierlo.com>
>>> Sent: Thursday, May 4, 2023 5:52 PM
>>> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
>>> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
>>> krzysztof.kozlowski+dt@linaro.org; michal.simek@xilinx.com; linux-
>>> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; jirislaby@kernel.org; linux-arm-
>>> kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; git
>>> (AMD-Xilinx) <git@amd.com>; Pandey, Radhey Shyam
>>> <radhey.shyam.pandey@amd.com>; Datta, Shubhrajyoti
>>> <shubhrajyoti.datta@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
>>> manion05gk@gmail.com
>>> Subject: Re: [PATCH 0/2] Add rs485 support to uartps driver
>>>
>>> Manikanta Guntupalli wrote 2023-04-26 14:29:
>>> > Add optional gpio property to uartps node to support rs485 Add rs485
>>> > support to uartps driver
>>> >
>>> > 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 |  5 +
>>> >  drivers/tty/serial/xilinx_uartps.c            | 96 ++++++++++++++++++-
>>> >  2 files changed, 100 insertions(+), 1 deletion(-)
>>>
>>> Why would you want to use a GPIO and not RTS for choosing the direction as
>>> is more common in this case?
>> In ZynqMp platform Cadence UART Controller RTS signal routed to
>> external through the PL(Programmable Logic) design not through
>> Multiplexed IO.
> 
> Then why not route RXD & TXD to the PL as well and connect the module to a
> PMOD connector connected to the PL? But I admit that a GPIO always works as
> well.

I will let Mani to comment other parts. Simply that's how PCB is wired now.
I remember some discussions to enhance silicon with being able to route MIO pins 
to PL but that capability has never been added.
And the second part of it is on PL pin constrained system there doesn't need to 
be free PL pin for this functionality.
And third thing is that routing via PL means that PL has to be loaded to get 
this functionality. Which also means much higher power consumption even if there 
is single wire between EMIO and PL pin.
It means GPIO routed via MIO through free existing pin is PCB design choice in 
the context of project they are focusing on.
And good that you see also GPIO as viable option for it.

Thanks,
Michal