Re: [PATCH] tty: serial: amba-pl011: added RS485 support - Greg Kroah-Hartman

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Ivan Sistik <sistik@3ksolutions.sk>
Cc: Russell King <linux@armlinux.org.uk>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	Eric Anholt <eric@anholt.net>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Jiri Slaby <jslaby@suse.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-rpi-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH] tty: serial: amba-pl011: added RS485 support
Date: Tue, 7 Jan 2020 08:27:48 +0100	[thread overview]
Message-ID: <20200107072748.GA1014453@kroah.com> (raw)
In-Reply-To: <20200106235203.27256-1-sistik@3ksolutions.sk>

On Tue, Jan 07, 2020 at 12:52:03AM +0100, Ivan Sistik wrote:
> 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 <sistik@3ksolutions.sk>
> ---
>  arch/arm/configs/bcm2835_defconfig |   1 +
>  drivers/tty/serial/Kconfig         |  12 +
>  drivers/tty/serial/amba-pl011.c    | 470 ++++++++++++++++++++++++++++-
>  3 files changed, 480 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
> index e9bc88937b1e..514888681913 100644
> --- a/arch/arm/configs/bcm2835_defconfig
> +++ b/arch/arm/configs/bcm2835_defconfig
> @@ -78,6 +78,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 df8bd0c7b97d..3cecd8a75691 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ 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

Why "=y" here?  Shouldn't this just read:
	depends on SERIAL_AMBA_PL011?

> +	select SERIAL_CORE

Is this needed?  SERIAL_AMBA_PL011 already selects this.

thanks,

greg k-h