On 30. 12. 20, 4:16, Ivan Sistik wrote:
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -75,6 +75,17 @@ config SERIAL_AMBA_PL011_CONSOLE
your boot loader (lilo or loadlin) about how to pass options to the
kernel at boot time.)
+config SERIAL_AMBA_PL011_SOFT_RS485
+ bool "RS485 software direction switching for ARM AMBA PL011 serial"
+ depends on SERIAL_AMBA_PL011=y
+ help
+ Enable RS485 software direction switching of driver enable (RTS pin)
+ for ARM AMBA PL011 serial. AMBA PL011 does not have HW support for
+ RS485. This driver use 2 hrtimers. One is used for rs485 delays.
"uses"
+ Second one is used for polling of TX FIFO. There is not TX FIFO
"The second one"
"There is no"
+ empty interrupt in PL011. Secondary timer is started by empty
"The secondary". But I am confused, 2 timers: one, second, and secondary
-- three timers?
+ transmit buffer.
+
config SERIAL_EARLYCON_ARM_SEMIHOST
bool "Early console using ARM semihosting"
depends on ARM64 || ARM
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 16720c97a..6a40e5bc5 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -46,6 +46,7 @@
#include <linux/sizes.h>
#include <linux/io.h>
#include <linux/acpi.h>
+#include <linux/math64.h>
#include "amba-pl011.h"
@@ -60,6 +61,18 @@
#define UART_DR_ERROR (UART011_DR_OE|UART011_DR_BE|UART011_DR_PE|UART011_DR_FE)
#define UART_DUMMY_DR_RX (1 << 16)
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+/*
+ * Enum with current status
+ */
+enum rs485_status {
+ rs485_receiving,
+ rs485_delay_before_send,
+ rs485_sending,
+ rs485_delay_after_send
These are too generic names.
+};
+#endif
You don't have to add ifdeffery to declarations.
+
static u16 pl011_std_offsets[REG_ARRAY_SIZE] = {
[REG_DR] = UART01x_DR,
[REG_FR] = UART01x_FR,
@@ -270,6 +283,16 @@ struct uart_amba_port {
unsigned int old_cr; /* state during shutdown */
unsigned int fixed_baud; /* vendor-set fixed baud rate */
char type[12];
+
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ enum rs485_status rs485_current_status; /* status used for RTS */
+ enum rs485_status rs485_next_status; /* this status after tick */
+ struct hrtimer rs485_delay_timer;
+ struct hrtimer rs485_tx_empty_poll_timer;
+ unsigned long send_char_time; /* send char (nanoseconds) */
+ bool rs485_last_char_sending;
+#endif
+
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
bool using_tx_dma;
@@ -306,6 +329,36 @@ static void pl011_write(unsigned int val, const struct uart_amba_port *uap,
writew_relaxed(val, addr);
}
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+
+static void pl011_rs485_start_rts_delay(struct uart_amba_port *uap);
+
+static void rs485_set_rts_signal(struct uart_amba_port *uap, bool value)
+{
+ unsigned int rts_temp_cr;
+
+ rts_temp_cr = pl011_read(uap, REG_CR);
+
+ if (!value)
+ rts_temp_cr |= UART011_CR_RTS;
+ else
+ rts_temp_cr &= ~UART011_CR_RTS;
+
+ pl011_write(rts_temp_cr, uap, REG_CR);
+}
+
+void rs485_cancel_timers(struct uart_amba_port *uap)
Why not static? And too generic name. The same above.
+{
+ hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
+ hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
No need for the parentheses. They occur on many places in the patch.
Have you run the patch through checkpatch?
+}
+
+bool rs485_tx_fifo_empty(struct uart_amba_port *uap)
+{
+ return (pl011_read(uap, REG_FR) & UART011_FR_TXFE);
detto.
+}
+#endif
+
/*
* Reads up to 256 characters from the FIFO or until it's empty and
* inserts them into the TTY layer. Returns the number of characters
@@ -1301,6 +1354,11 @@ 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);
+
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ if (uap->port.rs485.flags & SER_RS485_ENABLED)
+ pl011_rs485_start_rts_delay(uap);
Could you do the check inside pl011_rs485_start_rts_delay and remove the
ifdef completely?
+#endif
}
static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq);
@@ -1319,8 +1377,113 @@ static void pl011_start_tx(struct uart_port *port)
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
- if (!pl011_dma_tx_start(uap))
- pl011_start_tx_pio(uap);
+#define START_PL011_TX() \
+ do { \
+ if (!pl011_dma_tx_start(uap)) \
+ pl011_start_tx_pio(uap); \
+ } while (0)
Defining a macro inside a function? No. This should be a separate
function as well as the code below anyway.
+#ifndef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ START_PL011_TX();
+#else
+
+ if (uap->port.rs485.flags & SER_RS485_ENABLED) {
+ ktime_t ktime;
+
+ switch (uap->rs485_current_status) {
+ case rs485_delay_after_send:
+
+ rs485_cancel_timers(uap);
+
+ /* check if timer expired */
+ if (uap->rs485_current_status
+ != rs485_delay_after_send) {
+ /* Timer expired and RTS is in wrong state.*/
+ uap->rs485_current_status
+ = rs485_delay_before_send;
+ uap->rs485_next_status = rs485_sending;
+
+ rs485_set_rts_signal(uap,
+ uap->port.rs485.flags
+ & SER_RS485_RTS_ON_SEND);
+
+ ktime = ktime_set(0,
+ uap->port.rs485
+ .delay_rts_before_send
+ * 1000000L);
+
+ hrtimer_start(
+ &(uap->rs485_delay_timer),
+ ktime,
+ HRTIMER_MODE_REL);
SPaghetti code. It will be eliminated by moving to a separate function
suggested above.
+ return;
+ }
+
+ /* timer was stopped and driver can continue sending */
+ uap->rs485_current_status = rs485_sending;
+ uap->rs485_next_status = rs485_sending;
+
+ /* driver is already in sending state */
+ START_PL011_TX();
+ break;
+
+
+ case rs485_sending:
+ /* stop old timer. There can be running timer */
+ /* which is checking TX FIFO empty flag */
+ rs485_cancel_timers(uap);
+
+ /* driver is already in sending state */
+ START_PL011_TX();
+ break;
+
+ case rs485_receiving:
+ default:
+ /* stop old timer. There can be running timer */
+ /* which is checking TX FIFO empty flag */
+ rs485_cancel_timers(uap);
+
+ /* Set RTS */
+ rs485_set_rts_signal(uap,
+ uap->port.rs485.flags
+ & SER_RS485_RTS_ON_SEND);
+
+ if (uap->port.rs485.delay_rts_before_send == 0) {
+ /* Change state */
+ uap->rs485_current_status
+ = rs485_sending;
+ uap->rs485_next_status
+ = rs485_sending;
+
+ /* driver is in sending state */
+ START_PL011_TX();
+ break;
+ }
+
+ /* Change state */
+ uap->rs485_current_status
+ = rs485_delay_before_send;
+ uap->rs485_next_status = rs485_sending;
+
+ /* Start timer */
+ ktime = ktime_set(0,
+ uap->port.rs485.delay_rts_before_send
+ * 1000000L);
This is overcomplicated ms_to_ktime().
+ hrtimer_start(&(uap->rs485_delay_timer),
+ ktime,
+ HRTIMER_MODE_REL);
+ break;
+
+ case rs485_delay_before_send:
+ /* do nothing because delay timer should be running */
+ break;
+ }
+ } else {
+ START_PL011_TX();
+ }
+#endif
+
+#undef START_PL011_TX
}
static void pl011_stop_rx(struct uart_port *port)
@@ -1476,6 +1639,166 @@ static void check_apply_cts_event_workaround(struct uart_amba_port *uap)
dummy_read = pl011_read(uap, REG_ICR);
}
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+
+/*
+ * Change state according to pending delay
+ * Locking: port is locked in this function
+ */
+static enum hrtimer_restart
+pl011_rs485_tx_poll_timer(struct hrtimer *timer)
+{
+ unsigned long flags;
+ ktime_t ktime;
+
+ struct uart_amba_port *uap =
+ container_of(timer, struct uart_amba_port,
+ rs485_tx_empty_poll_timer);
+
+ spin_lock_irqsave(&uap->port.lock, flags);
+
+ if (!(uart_circ_empty(&uap->port.state->xmit))) {
+ spin_unlock_irqrestore(&uap->port.lock, flags);
+ return HRTIMER_NORESTART;
Create a label at the end and goto from here.
+ }
+
+ if (!rs485_tx_fifo_empty(uap) || !uap->rs485_last_char_sending) {
+ /*
+ * FIFO is empty but there is last char in transmit shift
+ * register so we need one more tick
+ */
+ uap->rs485_last_char_sending = rs485_tx_fifo_empty(uap);
+
+ hrtimer_forward_now(timer, ktime_set(0, uap->send_char_time));
ns_to_ktime()
+
+ spin_unlock_irqrestore(&uap->port.lock, flags);
+ return HRTIMER_RESTART;
+ }
+
+ /* Check if delay after send is set*/
+ if (uap->port.rs485.delay_rts_after_send == 0) {
+ /* Change state */
+ uap->rs485_current_status = rs485_receiving;
+ uap->rs485_next_status = rs485_receiving;
+
+ /* if there is no delay after send change RTS value*/
+ rs485_set_rts_signal(uap,
+ uap->port.rs485.flags
+ & SER_RS485_RTS_AFTER_SEND);
+
+ spin_unlock_irqrestore(&uap->port.lock, flags);
+ return HRTIMER_NORESTART;
+ }
+
+ /* Change state */
+ uap->rs485_current_status = rs485_delay_after_send;
+ uap->rs485_next_status = rs485_receiving;
+
+ /* RTS will be set in timer handler */
+
+ /* Start delay timer */
+ ktime = ktime_set(0, (uap->port.rs485.delay_rts_after_send
+ * 1000000L));
ms_to_ktime().
+ hrtimer_start(&(uap->rs485_delay_timer), ktime, HRTIMER_MODE_REL);
+
+ spin_unlock_irqrestore(&uap->port.lock, flags);
+ return HRTIMER_NORESTART;
+}
...
+static void pl011_rs485_start_rts_delay(struct uart_amba_port *uap)
+{
+ ktime_t ktime;
+
+ if (uap->rs485_current_status == rs485_receiving)
+ return;
+
+ /* if there is timeout in progress cancel it and start new */
+ hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
+ hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
+
+
+ if (!rs485_tx_fifo_empty(uap)
+ || uap->port.rs485.delay_rts_after_send == 0) {
+ /*
+ * Schedule validation timer if there is data in TX FIFO
+ * because there is not TX FIFO empty interrupt
+ */
+
+ uap->rs485_current_status = rs485_sending;
+ uap->rs485_next_status = rs485_sending;
+
+ uap->rs485_last_char_sending = false;
+
+ ktime = ktime_set(0, uap->send_char_time);
ns_to_ktime()
+ hrtimer_start(&(uap->rs485_tx_empty_poll_timer),
+ ktime,
+ HRTIMER_MODE_REL);
+ return;
+ }
+
+ /* Change state */
+ uap->rs485_current_status = rs485_delay_after_send;
+ uap->rs485_next_status = rs485_receiving;
+
+ /* RTS will be set in timer handler */
+
+ /* Start timer */
+ ktime = ktime_set(0, (uap->port.rs485.delay_rts_after_send
+ * 1000000L));
ms_to_ktime().
+
+ hrtimer_start(&(uap->rs485_delay_timer),
+ ktime,
+ HRTIMER_MODE_REL);
+}
+#endif
+
static irqreturn_t pl011_int(int irq, void *dev_id)
{
struct uart_amba_port *uap = dev_id;
@@ -1618,6 +1941,11 @@ static void pl011_quiesce_irqs(struct uart_port *port)
*/
pl011_write(pl011_read(uap, REG_IMSC) & ~UART011_TXIM, uap,
REG_IMSC);
+
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ if (uap->port.rs485.flags & SER_RS485_ENABLED)
+ pl011_rs485_start_rts_delay(uap);
+#endif
The same as above.
}
static int pl011_get_poll_char(struct uart_port *port)
@@ -1690,6 +2018,27 @@ static int pl011_hwinit(struct uart_port *port)
if (plat->init)
plat->init();
}
+
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
Do this in a separate function and eliminate such ifdefs in the code.
+ /*
+ * Initialize timers used for RS485
+ */
+ hrtimer_init(&(uap->rs485_delay_timer),
+ CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL);
+
+ uap->rs485_delay_timer.function = &pl011_rs485_timer;
+
+ hrtimer_init(&(uap->rs485_tx_empty_poll_timer),
+ CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL);
+
+ uap->rs485_tx_empty_poll_timer.function = &pl011_rs485_tx_poll_timer;
+
+ uap->rs485_current_status = rs485_receiving;
+ rs485_set_rts_signal(uap, false);
+#endif
+
return 0;
}
@@ -1873,6 +2222,16 @@ static void pl011_shutdown(struct uart_port *port)
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ if (uap->port.rs485.flags & SER_RS485_ENABLED) {
+ hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
+ hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
+
+ uap->rs485_current_status = rs485_receiving;
+ rs485_set_rts_signal(uap, true);
+ }
+#endif
Separate function + no ifdef.
+
pl011_disable_interrupts(uap);
pl011_dma_shutdown(uap);
@@ -1955,6 +2314,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned long flags;
unsigned int baud, quot, clkdiv;
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ unsigned int transfer_bit_count;
+ unsigned long char_transfer_time;
+
+ /*
+ * Calculate bit count which will be send
"sent"
+ * by UART. It is used for calculation of
+ * time required to start timer until TX FIFO (HW) is empty
Dot at the end.
+ * There is not interrupt for FIFO empty in PL011.
"There is not an"
or
"There is no"
+ * There is only FIFO empty flag in REG_FR.
+ */
+ transfer_bit_count = 0;
+
+#define ADD_DATA_BITS(bits) (transfer_bit_count += bits)
This is ugly.
1) it's a macro.
2) it hides uses of transfer_bit_count from the reader.
+#else
+#define ADD_DATA_BITS(bits)
+#endif
+
if (uap->vendor->oversampling)
clkdiv = 8;
else
@@ -1981,29 +2358,53 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
switch (termios->c_cflag & CSIZE) {
case CS5:
lcr_h = UART01x_LCRH_WLEN_5;
+ ADD_DATA_BITS(7);
break;
case CS6:
lcr_h = UART01x_LCRH_WLEN_6;
+ ADD_DATA_BITS(8);
break;
case CS7:
lcr_h = UART01x_LCRH_WLEN_7;
+ ADD_DATA_BITS(9);
break;
default: // CS8
lcr_h = UART01x_LCRH_WLEN_8;
+ ADD_DATA_BITS(10);
break;
}
- if (termios->c_cflag & CSTOPB)
+
+ if (termios->c_cflag & CSTOPB) {
lcr_h |= UART01x_LCRH_STP2;
+ ADD_DATA_BITS(1);
+ }
+
if (termios->c_cflag & PARENB) {
lcr_h |= UART01x_LCRH_PEN;
+ ADD_DATA_BITS(1);
+
if (!(termios->c_cflag & PARODD))
lcr_h |= UART01x_LCRH_EPS;
+
if (termios->c_cflag & CMSPAR)
lcr_h |= UART011_LCRH_SPS;
}
+
+#undef ADD_DATA_BITS
+
if (uap->fifosize > 1)
lcr_h |= UART01x_LCRH_FEN;
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ /* Calculate time required to send one char (nanoseconds) */
+ char_transfer_time =
+ (unsigned long) div_u64(
+ mul_u32_u32(
+ (u32)transfer_bit_count,
+ (u32)NSEC_PER_SEC),
+ (u32)baud);
Wny do you cast all that?
+#endif
+
spin_lock_irqsave(&port->lock, flags);
/*
@@ -2020,6 +2421,11 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
old_cr = pl011_read(uap, REG_CR);
pl011_write(0, uap, REG_CR);
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ /* Update send_char_time in locked context */
+ uap->send_char_time = char_transfer_time;
+#endif
+
if (termios->c_cflag & CRTSCTS) {
if (old_cr & UART011_CR_RTS)
old_cr |= UART011_CR_RTSEN;
@@ -2122,6 +2528,47 @@ static void pl011_config_port(struct uart_port *port, int flags)
}
}
+/*
+ * Configure RS485
+ * Locking: called with port lock held and IRQs disabled
+ */
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+static int pl011_config_rs485(struct uart_port *port,
+ struct serial_rs485 *rs485)
+{
+ bool was_disabled;
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ was_disabled = !(port->rs485.flags & SER_RS485_ENABLED);
Where is this used?
+ port->rs485.flags = rs485->flags;
+ port->rs485.delay_rts_after_send = rs485->delay_rts_after_send;
+ port->rs485.delay_rts_before_send = rs485->delay_rts_before_send;
+
+ if (port->rs485.flags & SER_RS485_ENABLED) {
+ unsigned int cr;
+
+ hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
+ hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
+
+ /* If RS485 is enabled, disable auto RTS */
+ cr = pl011_read(uap, REG_CR);
+ cr &= ~UART011_CR_RTSEN;
+ pl011_write(cr, uap, REG_CR);
+
+ uap->rs485_current_status = rs485_receiving;
+ rs485_set_rts_signal(uap,
+ port->rs485.flags
+ & SER_RS485_RTS_AFTER_SEND);
+ } else {
+ rs485_set_rts_signal(uap, true);
+ }
+
+ return 0;
+}
+#endif
+
/*
* verify the new serial_struct (for TIOCSSERIAL).
*/
@@ -2647,6 +3094,11 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
uap->port.irq = dev->irq[0];
uap->port.ops = &amba_pl011_pops;
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+ uap->port.rs485_config = &pl011_config_rs485;
+ uap->port.rs485.flags = 0; /* RS485 is not enabled by default */
+#endif
+
snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr);
@@ -2819,10 +3271,15 @@ static struct amba_driver pl011_driver = {
static int __init pl011_init(void)
{
+#if IS_ENABLED(CONFIG_SERIAL_AMBA_PL011_SOFT_RS485)
+ printk(KERN_INFO "Serial: AMBA PL011 UART driver with soft RS485 support\n");
+#else
printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
+#endif
Didn't Stefan already commented on this?
You can actually remove the print completely (in a separate patch).
if (platform_driver_register(&arm_sbsa_uart_platform_driver))
pr_warn("could not register SBSA UART platform driver\n");
+
And I saw a comment about added whitespace too.
return amba_driver_register(&pl011_driver);
}
thanks,
--
js
suse labs
