mbox series

[0/2] Add rs485 support to uartps driver

Message ID 1682512187-8828-1-git-send-email-manikanta.guntupalli@amd.com
Headers show
Series Add rs485 support to uartps driver | expand

Message

Manikanta Guntupalli April 26, 2023, 12:29 p.m. UTC
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(-)

Comments

Maarten Brock May 4, 2023, 12:22 p.m. UTC | #1
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
Manikanta Guntupalli May 10, 2023, 4:26 p.m. UTC | #2
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.
Michal Simek May 11, 2023, 7:26 a.m. UTC | #3
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
Maarten Brock May 14, 2023, 11:01 a.m. UTC | #4
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
Michal Simek May 15, 2023, 6:35 a.m. UTC | #5
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