Re: [PATCH] tty: serial: amba-pl011: added RS485 support v2
Hi Ivan,
please place the patch version between [ ... ] in the subject.
Am 28.12.20 um 17:41 schrieb Ivan Sistik:
> AMBA PL011 do not have hardware support for RS485. This implementation is
> for drive enable signal (DE), which switch direction of RS485 driver chip.
> This signal si drived by RTS pin. Correct multiplexor settings have to be
> provided to Device Tree. Usually it is 'ctsrts', which is used for enabling
> of HW flow control, too.
>
> DE signal is switched by starting transmition from serial core and data
> transfer is initiated by first hrtimer if there is delay before send
> enabled.
>
> There is missing FIFO empty interrupt in PL011. It is replaced by second
> hrtimer which is started if there are no more data in port transmit buffer.
> Notice that port transmit buffer is not the same as HW TX FIFO. Time of
> this timmer is set to char send time and it is running until fifo is empty.
> This kind of implementation cause that there can be unwanted delay of one
> timer tick before DE signal is switched. This is used to prevent data loss
> during transmit. Second timer can start first if there is delay after send
> enabled.
>
> Signed-off-by: Ivan Sistik
> ---
>
> Notes:
> This patch is ported and corrected version of my previous patch which can
> be reviewed here:
> https://lore.kernel.org/lkml/20200106235203.27256-1-sis...@3ksolutions.sk/
>
> I have been waiting for some time to see if Lukas Wunner
> will create patch with his own solution.
>
> Now I am successfully running my imeplementation for almost one year in
> production environment. There are no problems with it.
Can you please tell which exact platform / config template has been tested?
> I have made
> corrections to patch according to notes from Greg Kroah-Hartman
> .
>
> arch/arm/configs/bcm2835_defconfig | 1 +
> drivers/tty/serial/Kconfig | 11 +
> drivers/tty/serial/amba-pl011.c| 474 -
> 3 files changed, 483 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/configs/bcm2835_defconfig
> b/arch/arm/configs/bcm2835_defconfig
> index 519ff58e6..c2f630937 100644
> --- a/arch/arm/configs/bcm2835_defconfig
> +++ b/arch/arm/configs/bcm2835_defconfig
> @@ -86,6 +86,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
> CONFIG_SERIAL_8250_BCM2835AUX=y
> CONFIG_SERIAL_AMBA_PL011=y
> CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
> +CONFIG_SERIAL_AMBA_PL011_SOFT_RS485=y
> CONFIG_SERIAL_DEV_BUS=y
> CONFIG_TTY_PRINTK=y
> CONFIG_I2C_CHARDEV=y
This change is fine, but must be a separate patch. In this case, you can
move all your notes and patch changelog in the cover letter.
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index a9751a83d..c33461511 100644
> --- 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.
> + Secon one is used for polling of TX FIFO. There is not TX FIFO
Secon +d?
> + empty interrupt in PL011. Secondary timer is started by empty
> + 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..f45b9042b 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
>
> #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
Please try to reduce the amout of these ifdefs. It makes the code harder
to maintain.
> +/*
> + * Enum with current status
> + */
> +enum rs485_status {
> + rs485_receiving,
> + rs485_delay_before_send,
> + rs485_sending,
> + rs485_delay_after_send
> +};
> +#endif
> +
> 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 intold_cr; /* state during shutdown */
> unsigned intfixed_baud; /* vendor-set fixed baud rate */
> chartype[12];
> +
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> + enum rs485_status rs485_current_status; /* status used for
[PATCH] tty: serial: amba-pl011: added RS485 support v2
AMBA PL011 do not have hardware support for RS485. This implementation is
for drive enable signal (DE), which switch direction of RS485 driver chip.
This signal si drived by RTS pin. Correct multiplexor settings have to be
provided to Device Tree. Usually it is 'ctsrts', which is used for enabling
of HW flow control, too.
DE signal is switched by starting transmition from serial core and data
transfer is initiated by first hrtimer if there is delay before send
enabled.
There is missing FIFO empty interrupt in PL011. It is replaced by second
hrtimer which is started if there are no more data in port transmit buffer.
Notice that port transmit buffer is not the same as HW TX FIFO. Time of
this timmer is set to char send time and it is running until fifo is empty.
This kind of implementation cause that there can be unwanted delay of one
timer tick before DE signal is switched. This is used to prevent data loss
during transmit. Second timer can start first if there is delay after send
enabled.
Signed-off-by: Ivan Sistik
---
Notes:
This patch is ported and corrected version of my previous patch which can
be reviewed here:
https://lore.kernel.org/lkml/20200106235203.27256-1-sis...@3ksolutions.sk/
I have been waiting for some time to see if Lukas Wunner
will create patch with his own solution.
Now I am successfully running my imeplementation for almost one year in
production environment. There are no problems with it. I have made
corrections to patch according to notes from Greg Kroah-Hartman
.
arch/arm/configs/bcm2835_defconfig | 1 +
drivers/tty/serial/Kconfig | 11 +
drivers/tty/serial/amba-pl011.c| 474 -
3 files changed, 483 insertions(+), 3 deletions(-)
diff --git a/arch/arm/configs/bcm2835_defconfig
b/arch/arm/configs/bcm2835_defconfig
index 519ff58e6..c2f630937 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -86,6 +86,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
CONFIG_SERIAL_8250_BCM2835AUX=y
CONFIG_SERIAL_AMBA_PL011=y
CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
+CONFIG_SERIAL_AMBA_PL011_SOFT_RS485=y
CONFIG_SERIAL_DEV_BUS=y
CONFIG_TTY_PRINTK=y
CONFIG_I2C_CHARDEV=y
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index a9751a83d..c33461511 100644
--- 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.
+ Secon one is used for polling of TX FIFO. There is not TX FIFO
+ empty interrupt in PL011. Secondary timer is started by empty
+ 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..f45b9042b 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
#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
+};
+#endif
+
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 intold_cr; /* state during shutdown */
unsigned intfixed_baud; /* vendor-set fixed baud rate */
chartype[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) */
+ boolrs485_last_char_sending;
+#endif
+
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
boolusing_tx_dma;
@@ -280,6 +303,25 @@ struct uart_amba_port {
#endif
};
+#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
+
+static void pl011_rs485_start_rts_delay(struct uart_amba_port
