2021-06-10 13:53:15

by Lino Sanfilippo

[permalink] [raw]
Subject: [PATCH] serial: amba-pl011: add RS485 support

Add basic support for RS485: Provide a callback to configure rs485
settings. Handle the RS485 specific part in the functions
pl011_rs485_tx_start() and pl011_rs485_tx_stop() which extend the generic
start/stop callbacks.
Beside via IOCTL from userspace RS485 can be enabled by means of the
device tree property "rs485-enabled-at-boot-time".

Signed-off-by: Lino Sanfilippo <[email protected]>
---
This patch has been tested with a Raspberry Pi CM3.

drivers/tty/serial/amba-pl011.c | 143 +++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 78682c12156a..36e8b938cdba 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -265,6 +265,8 @@ struct uart_amba_port {
unsigned int old_cr; /* state during shutdown */
unsigned int fixed_baud; /* vendor-set fixed baud rate */
char type[12];
+ bool rs485_tx_started;
+ unsigned int rs485_tx_drain_interval; /* usecs */
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
bool using_tx_dma;
@@ -275,6 +277,8 @@ struct uart_amba_port {
#endif
};

+static unsigned int pl011_tx_empty(struct uart_port *port);
+
static unsigned int pl011_reg_to_offset(const struct uart_amba_port *uap,
unsigned int reg)
{
@@ -1282,6 +1286,34 @@ static inline bool pl011_dma_rx_running(struct uart_amba_port *uap)
#define pl011_dma_flush_buffer NULL
#endif

+static int pl011_rs485_tx_stop(struct uart_amba_port *uap)
+{
+ struct uart_port *port = &uap->port;
+ u32 cr;
+
+ /* Wait until hardware tx queue is empty */
+ while (!pl011_tx_empty(port))
+ udelay(uap->rs485_tx_drain_interval);
+
+ if (port->rs485.delay_rts_after_send)
+ mdelay(port->rs485.delay_rts_after_send);
+
+ cr = pl011_read(uap, REG_CR);
+
+ if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+ cr &= ~UART011_CR_RTS;
+ else
+ cr |= UART011_CR_RTS;
+ /* Disable the transmitter and reenable the transceiver */
+ cr &= ~UART011_CR_TXE;
+ cr |= UART011_CR_RXE;
+ pl011_write(cr, uap, REG_CR);
+
+ uap->rs485_tx_started = false;
+
+ return 0;
+}
+
static void pl011_stop_tx(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1290,6 +1322,9 @@ static void pl011_stop_tx(struct uart_port *port)
uap->im &= ~UART011_TXIM;
pl011_write(uap->im, uap, REG_IMSC);
pl011_dma_tx_stop(uap);
+
+ if ((port->rs485.flags & SER_RS485_ENABLED) && uap->rs485_tx_started)
+ pl011_rs485_tx_stop(uap);
}

static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq);
@@ -1380,6 +1415,31 @@ static bool pl011_tx_char(struct uart_amba_port *uap, unsigned char c,
return true;
}

+static void pl011_rs485_tx_start(struct uart_amba_port *uap)
+{
+ struct uart_port *port = &uap->port;
+ u32 cr;
+
+ /* Enable transmitter */
+ cr = pl011_read(uap, REG_CR);
+ cr |= UART011_CR_TXE;
+ /* Disable receiver if half-duplex */
+ if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ cr &= ~UART011_CR_RXE;
+
+ if (port->rs485.flags & SER_RS485_RTS_ON_SEND)
+ cr &= ~UART011_CR_RTS;
+ else
+ cr |= UART011_CR_RTS;
+
+ pl011_write(cr, uap, REG_CR);
+
+ if (port->rs485.delay_rts_before_send)
+ mdelay(port->rs485.delay_rts_before_send);
+
+ uap->rs485_tx_started = true;
+}
+
/* Returns true if tx interrupts have to be (kept) enabled */
static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq)
{
@@ -1397,6 +1457,10 @@ static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq)
return false;
}

+ if ((uap->port.rs485.flags & SER_RS485_ENABLED) &&
+ !uap->rs485_tx_started)
+ pl011_rs485_tx_start(uap);
+
/* If we are using DMA mode, try to send some characters. */
if (pl011_dma_tx_irq(uap))
return true;
@@ -1542,6 +1606,9 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl)
container_of(port, struct uart_amba_port, port);
unsigned int cr;

+ if (port->rs485.flags & SER_RS485_ENABLED)
+ mctrl &= ~TIOCM_RTS;
+
cr = pl011_read(uap, REG_CR);

#define TIOCMBIT(tiocmbit, uartbit) \
@@ -1763,7 +1830,17 @@ static int pl011_startup(struct uart_port *port)

/* restore RTS and DTR */
cr = uap->old_cr & (UART011_CR_RTS | UART011_CR_DTR);
- cr |= UART01x_CR_UARTEN | UART011_CR_RXE | UART011_CR_TXE;
+ cr |= UART01x_CR_UARTEN | UART011_CR_RXE;
+
+ if (port->rs485.flags & SER_RS485_ENABLED) {
+ if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+ cr &= ~UART011_CR_RTS;
+ else
+ cr |= UART011_CR_RTS;
+ } else {
+ cr |= UART011_CR_TXE;
+ }
+
pl011_write(cr, uap, REG_CR);

spin_unlock_irq(&uap->port.lock);
@@ -1864,6 +1941,9 @@ static void pl011_shutdown(struct uart_port *port)

pl011_dma_shutdown(uap);

+ if ((port->rs485.flags & SER_RS485_ENABLED) && uap->rs485_tx_started)
+ pl011_rs485_tx_stop(uap);
+
free_irq(uap->port.irq, uap);

pl011_disable_uart(uap);
@@ -1941,6 +2021,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned int lcr_h, old_cr;
unsigned long flags;
unsigned int baud, quot, clkdiv;
+ unsigned int bits;

if (uap->vendor->oversampling)
clkdiv = 8;
@@ -1968,25 +2049,32 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
switch (termios->c_cflag & CSIZE) {
case CS5:
lcr_h = UART01x_LCRH_WLEN_5;
+ bits = 7;
break;
case CS6:
lcr_h = UART01x_LCRH_WLEN_6;
+ bits = 8;
break;
case CS7:
lcr_h = UART01x_LCRH_WLEN_7;
+ bits = 9;
break;
default: // CS8
lcr_h = UART01x_LCRH_WLEN_8;
+ bits = 10;
break;
}
- if (termios->c_cflag & CSTOPB)
+ if (termios->c_cflag & CSTOPB) {
lcr_h |= UART01x_LCRH_STP2;
+ bits++;
+ }
if (termios->c_cflag & PARENB) {
lcr_h |= UART01x_LCRH_PEN;
if (!(termios->c_cflag & PARODD))
lcr_h |= UART01x_LCRH_EPS;
if (termios->c_cflag & CMSPAR)
lcr_h |= UART011_LCRH_SPS;
+ bits++;
}
if (uap->fifosize > 1)
lcr_h |= UART01x_LCRH_FEN;
@@ -1997,12 +2085,21 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
* Update the per-port timeout.
*/
uart_update_timeout(port, termios->c_cflag, baud);
+ /*
+ * Calculate the approximated time it takes to transmit one character
+ * with the given baud rate. We use this as the poll interval when we
+ * wait for the tx queue to empty.
+ */
+ uap->rs485_tx_drain_interval = (bits * 1000 * 1000) / baud;

pl011_setup_status_masks(port, termios);

if (UART_ENABLE_MS(port, termios->c_cflag))
pl011_enable_ms(port);

+ if (port->rs485.flags & SER_RS485_ENABLED)
+ termios->c_cflag &= ~CRTSCTS;
+
/* first, disable everything */
old_cr = pl011_read(uap, REG_CR);
pl011_write(0, uap, REG_CR);
@@ -2124,6 +2221,41 @@ static int pl011_verify_port(struct uart_port *port, struct serial_struct *ser)
return ret;
}

+static int pl011_rs485_config(struct uart_port *port,
+ struct serial_rs485 *rs485)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ /* pick sane settings if the user hasn't */
+ if (!!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
+ !!(rs485->flags & SER_RS485_RTS_AFTER_SEND)) {
+ rs485->flags |= SER_RS485_RTS_ON_SEND;
+ rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
+ }
+
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 1000U);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send, 1000U);
+ memset(rs485->padding, 0, sizeof(rs485->padding));
+
+ if (port->rs485.flags & SER_RS485_ENABLED)
+ pl011_rs485_tx_stop(uap);
+
+ /* Set new configuration */
+ port->rs485 = *rs485;
+ /* Make sure auto RTS is disabled */
+ if (port->rs485.flags & SER_RS485_ENABLED) {
+ u32 cr = pl011_read(uap, REG_CR);
+
+ cr &= ~UART011_CR_RTSEN;
+ pl011_write(cr, uap, REG_CR);
+ port->status &= ~UPSTAT_AUTORTS;
+ }
+
+
+ return 0;
+}
+
static const struct uart_ops amba_pl011_pops = {
.tx_empty = pl011_tx_empty,
.set_mctrl = pl011_set_mctrl,
@@ -2592,6 +2724,7 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
struct resource *mmiobase, int index)
{
void __iomem *base;
+ int ret;

base = devm_ioremap_resource(dev, mmiobase);
if (IS_ERR(base))
@@ -2608,6 +2741,10 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
uap->port.flags = UPF_BOOT_AUTOCONF;
uap->port.line = index;

+ ret = uart_get_rs485_mode(&uap->port);
+ if (ret)
+ return ret;
+
amba_ports[index] = uap;

return 0;
@@ -2665,7 +2802,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
uap->port.iotype = vendor->access_32b ? UPIO_MEM32 : UPIO_MEM;
uap->port.irq = dev->irq[0];
uap->port.ops = &amba_pl011_pops;
-
+ uap->port.rs485_config = pl011_rs485_config;
snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));

ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr);

base-commit: cd1245d75ce93b8fd206f4b34eb58bcfe156d5e9
--
2.31.1


2021-06-16 06:19:06

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] serial: amba-pl011: add RS485 support

On 10. 06. 21, 15:50, Lino Sanfilippo wrote:
> Add basic support for RS485: Provide a callback to configure rs485
> settings. Handle the RS485 specific part in the functions
> pl011_rs485_tx_start() and pl011_rs485_tx_stop() which extend the generic
> start/stop callbacks.
> Beside via IOCTL from userspace RS485 can be enabled by means of the
> device tree property "rs485-enabled-at-boot-time".
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> This patch has been tested with a Raspberry Pi CM3.
>
> drivers/tty/serial/amba-pl011.c | 143 +++++++++++++++++++++++++++++++-
> 1 file changed, 140 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 78682c12156a..36e8b938cdba 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
...
> @@ -1380,6 +1415,31 @@ static bool pl011_tx_char(struct uart_amba_port *uap, unsigned char c,
> return true;
> }
>
> +static void pl011_rs485_tx_start(struct uart_amba_port *uap)
> +{
> + struct uart_port *port = &uap->port;
> + u32 cr;
> +
> + /* Enable transmitter */
> + cr = pl011_read(uap, REG_CR);
> + cr |= UART011_CR_TXE;
> + /* Disable receiver if half-duplex */
> + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> + cr &= ~UART011_CR_RXE;
> +
> + if (port->rs485.flags & SER_RS485_RTS_ON_SEND)
> + cr &= ~UART011_CR_RTS;
> + else
> + cr |= UART011_CR_RTS;
> +
> + pl011_write(cr, uap, REG_CR);
> +
> + if (port->rs485.delay_rts_before_send)
> + mdelay(port->rs485.delay_rts_before_send);

This is up to 1 second delay with interrupts disabled. Definitely not
nice. 8250 clamps this to 100 ms at least, why did you choose 1000 ms?

> +
> + uap->rs485_tx_started = true;
> +}
> +
> /* Returns true if tx interrupts have to be (kept) enabled */
> static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq)
> {
...
> @@ -1941,6 +2021,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> unsigned int lcr_h, old_cr;
> unsigned long flags;
> unsigned int baud, quot, clkdiv;
> + unsigned int bits;
>
> if (uap->vendor->oversampling)
> clkdiv = 8;
> @@ -1968,25 +2049,32 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> switch (termios->c_cflag & CSIZE) {
> case CS5:
> lcr_h = UART01x_LCRH_WLEN_5;
> + bits = 7;
> break;
> case CS6:
> lcr_h = UART01x_LCRH_WLEN_6;
> + bits = 8;
> break;
> case CS7:
> lcr_h = UART01x_LCRH_WLEN_7;
> + bits = 9;
> break;
> default: // CS8
> lcr_h = UART01x_LCRH_WLEN_8;
> + bits = 10;
> break;
> }
> - if (termios->c_cflag & CSTOPB)
> + if (termios->c_cflag & CSTOPB) {
> lcr_h |= UART01x_LCRH_STP2;
> + bits++;
> + }
> if (termios->c_cflag & PARENB) {
> lcr_h |= UART01x_LCRH_PEN;
> if (!(termios->c_cflag & PARODD))
> lcr_h |= UART01x_LCRH_EPS;
> if (termios->c_cflag & CMSPAR)
> lcr_h |= UART011_LCRH_SPS;
> + bits++;
> }

You can do simply:
bits = tty_get_frame_size(termios->c_cflag);
now:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-testing&id=3ec2ff37230e1c961d4b0d0118dd23c46b5bcdbb


thanks,
--
js

2021-06-16 06:31:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] serial: amba-pl011: add RS485 support

On Wed, Jun 16, 2021 at 08:18:01AM +0200, Jiri Slaby wrote:
> On 10. 06. 21, 15:50, Lino Sanfilippo wrote:
> > Add basic support for RS485: Provide a callback to configure rs485
> > settings. Handle the RS485 specific part in the functions
> > pl011_rs485_tx_start() and pl011_rs485_tx_stop() which extend the generic
> > start/stop callbacks.
> > Beside via IOCTL from userspace RS485 can be enabled by means of the
> > device tree property "rs485-enabled-at-boot-time".
> >
> > Signed-off-by: Lino Sanfilippo <[email protected]>
> > ---
> > This patch has been tested with a Raspberry Pi CM3.
> >
> > drivers/tty/serial/amba-pl011.c | 143 +++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> > index 78682c12156a..36e8b938cdba 100644
> > --- a/drivers/tty/serial/amba-pl011.c
> > +++ b/drivers/tty/serial/amba-pl011.c
> ...
> > @@ -1380,6 +1415,31 @@ static bool pl011_tx_char(struct uart_amba_port *uap, unsigned char c,
> > return true;
> > }
> > +static void pl011_rs485_tx_start(struct uart_amba_port *uap)
> > +{
> > + struct uart_port *port = &uap->port;
> > + u32 cr;
> > +
> > + /* Enable transmitter */
> > + cr = pl011_read(uap, REG_CR);
> > + cr |= UART011_CR_TXE;
> > + /* Disable receiver if half-duplex */
> > + if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> > + cr &= ~UART011_CR_RXE;
> > +
> > + if (port->rs485.flags & SER_RS485_RTS_ON_SEND)
> > + cr &= ~UART011_CR_RTS;
> > + else
> > + cr |= UART011_CR_RTS;
> > +
> > + pl011_write(cr, uap, REG_CR);
> > +
> > + if (port->rs485.delay_rts_before_send)
> > + mdelay(port->rs485.delay_rts_before_send);
>
> This is up to 1 second delay with interrupts disabled. Definitely not nice.
> 8250 clamps this to 100 ms at least, why did you choose 1000 ms?
>
> > +
> > + uap->rs485_tx_started = true;
> > +}
> > +
> > /* Returns true if tx interrupts have to be (kept) enabled */
> > static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq)
> > {
> ...
> > @@ -1941,6 +2021,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> > unsigned int lcr_h, old_cr;
> > unsigned long flags;
> > unsigned int baud, quot, clkdiv;
> > + unsigned int bits;
> > if (uap->vendor->oversampling)
> > clkdiv = 8;
> > @@ -1968,25 +2049,32 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> > switch (termios->c_cflag & CSIZE) {
> > case CS5:
> > lcr_h = UART01x_LCRH_WLEN_5;
> > + bits = 7;
> > break;
> > case CS6:
> > lcr_h = UART01x_LCRH_WLEN_6;
> > + bits = 8;
> > break;
> > case CS7:
> > lcr_h = UART01x_LCRH_WLEN_7;
> > + bits = 9;
> > break;
> > default: // CS8
> > lcr_h = UART01x_LCRH_WLEN_8;
> > + bits = 10;
> > break;
> > }
> > - if (termios->c_cflag & CSTOPB)
> > + if (termios->c_cflag & CSTOPB) {
> > lcr_h |= UART01x_LCRH_STP2;
> > + bits++;
> > + }
> > if (termios->c_cflag & PARENB) {
> > lcr_h |= UART01x_LCRH_PEN;
> > if (!(termios->c_cflag & PARODD))
> > lcr_h |= UART01x_LCRH_EPS;
> > if (termios->c_cflag & CMSPAR)
> > lcr_h |= UART011_LCRH_SPS;
> > + bits++;
> > }
>
> You can do simply:
> bits = tty_get_frame_size(termios->c_cflag);
> now:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-testing&id=3ec2ff37230e1c961d4b0d0118dd23c46b5bcdbb

I was wondering about that.

I'll go drop this patch from my tty-next branch and wait for a new
version that fixes the above delay issue and it can use this new call as
well.

thanks,

greg k-h

2021-06-17 16:34:57

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH] serial: amba-pl011: add RS485 support



Hi,

On 16.06.21 at 08:18, Jiri Slaby wrote:

>> +??? if (port->rs485.delay_rts_before_send)
>> +??????? mdelay(port->rs485.delay_rts_before_send);
>
> This is up to 1 second delay with interrupts disabled. Definitely not nice. 8250 clamps this to 100 ms at least, why did you choose 1000 ms?
>

I got this value from rs485.yaml where the max number for both rts delay before and after send
is specified as 1000ms. I thought to keep it up to the user to choose a sane setting. But yes,
its a very long time with interrupts disabled. I will send an updated version of this patch that
limits both values to 100ms.


os(struct uart_port *port, struct ktermios *termios,
>> ????? unsigned int lcr_h, old_cr;
>> ????? unsigned long flags;
>> ????? unsigned int baud, quot, clkdiv;
>> +??? unsigned int bits;
>> ? ????? if (uap->vendor->oversampling)
>> ????????? clkdiv = 8;
>> @@ -1968,25 +2049,32 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
>> ????? switch (termios->c_cflag & CSIZE) {
>> ????? case CS5:
>> ????????? lcr_h = UART01x_LCRH_WLEN_5;
>> +??????? bits = 7;
>> ????????? break;
>> ????? case CS6:
>> ????????? lcr_h = UART01x_LCRH_WLEN_6;
>> +??????? bits = 8;
>> ????????? break;
>> ????? case CS7:
>> ????????? lcr_h = UART01x_LCRH_WLEN_7;
>> +??????? bits = 9;
>> ????????? break;
>> ????? default: // CS8
>> ????????? lcr_h = UART01x_LCRH_WLEN_8;
>> +??????? bits = 10;
>> ????????? break;
>> ????? }
>> -??? if (termios->c_cflag & CSTOPB)
>> +??? if (termios->c_cflag & CSTOPB) {
>> ????????? lcr_h |= UART01x_LCRH_STP2;
>> +??????? bits++;
>> +??? }
>> ????? if (termios->c_cflag & PARENB) {
>> ????????? lcr_h |= UART01x_LCRH_PEN;
>> ????????? if (!(termios->c_cflag & PARODD))
>> ????????????? lcr_h |= UART01x_LCRH_EPS;
>> ????????? if (termios->c_cflag & CMSPAR)
>> ????????????? lcr_h |= UART011_LCRH_SPS;
>> +??????? bits++;
>> ????? }
>
> You can do simply:
> ? bits = tty_get_frame_size(termios->c_cflag);
> now:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-testing&id=3ec2ff37230e1c961d4b0d0118dd23c46b5bcdbb

Ah thats a nice function, thanks for pointing at it. I will adjust the code accordingly.


>
> thanks,

Thanks for the review!

Regards,
Lino


2021-06-17 22:21:45

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH] serial: amba-pl011: add RS485 support


Hi,

On 16.06.21 at 08:18, Jiri Slaby wrote:
??????? mdelay(port->rs485.delay_rts_before_send);
>
> This is up to 1 second delay with interrupts disabled. Definitely not nice. 8250 clamps this to 100 ms at least, why did you choose 1000 ms?
>

AFAICS the 8250 driver does not clamp values read from the device tree property "rs485-rts-delay"
(set by uart_get_rs485_mode()). Is this on purpose?

Regards,
Lino