All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: amba-pl011: add RS485 support
@ 2021-06-10 13:50 Lino Sanfilippo
  2021-06-16  6:18 ` Jiri Slaby
  0 siblings, 1 reply; 5+ messages in thread
From: Lino Sanfilippo @ 2021-06-10 13:50 UTC (permalink / raw)
  To: gregkh; +Cc: linux, jirislaby, linux-serial, linux-kernel, Lino Sanfilippo

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 <LinoSanfilippo@gmx.de>
---
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: amba-pl011: add RS485 support
  2021-06-10 13:50 [PATCH] serial: amba-pl011: add RS485 support Lino Sanfilippo
@ 2021-06-16  6:18 ` Jiri Slaby
  2021-06-16  6:29   ` Greg KH
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jiri Slaby @ 2021-06-16  6:18 UTC (permalink / raw)
  To: Lino Sanfilippo, gregkh; +Cc: linux, linux-serial, linux-kernel

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 <LinoSanfilippo@gmx.de>
> ---
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: amba-pl011: add RS485 support
  2021-06-16  6:18 ` Jiri Slaby
@ 2021-06-16  6:29   ` Greg KH
  2021-06-17 16:12   ` Lino Sanfilippo
  2021-06-17 17:44   ` Lino Sanfilippo
  2 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-06-16  6:29 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Lino Sanfilippo, linux, linux-serial, linux-kernel

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 <LinoSanfilippo@gmx.de>
> > ---
> > 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: amba-pl011: add RS485 support
  2021-06-16  6:18 ` Jiri Slaby
  2021-06-16  6:29   ` Greg KH
@ 2021-06-17 16:12   ` Lino Sanfilippo
  2021-06-17 17:44   ` Lino Sanfilippo
  2 siblings, 0 replies; 5+ messages in thread
From: Lino Sanfilippo @ 2021-06-17 16:12 UTC (permalink / raw)
  To: Jiri Slaby, gregkh; +Cc: linux, linux-serial, linux-kernel



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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: amba-pl011: add RS485 support
  2021-06-16  6:18 ` Jiri Slaby
  2021-06-16  6:29   ` Greg KH
  2021-06-17 16:12   ` Lino Sanfilippo
@ 2021-06-17 17:44   ` Lino Sanfilippo
  2 siblings, 0 replies; 5+ messages in thread
From: Lino Sanfilippo @ 2021-06-17 17:44 UTC (permalink / raw)
  To: Jiri Slaby, gregkh; +Cc: linux, linux-serial, linux-kernel


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-17 17:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 13:50 [PATCH] serial: amba-pl011: add RS485 support Lino Sanfilippo
2021-06-16  6:18 ` Jiri Slaby
2021-06-16  6:29   ` Greg KH
2021-06-17 16:12   ` Lino Sanfilippo
2021-06-17 17:44   ` Lino Sanfilippo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.