Message ID | 20210610135004.7585-1-LinoSanfilippo@gmx.de |
---|---|
State | Superseded |
Headers | show |
Series | serial: amba-pl011: add RS485 support | expand |
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
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
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
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
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);
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(-) base-commit: cd1245d75ce93b8fd206f4b34eb58bcfe156d5e9