Re: [RFC PATCH 3/7] serial: 8250_dwlib: Implement SW half duplex support

From: Lukas Wunner <lukas@wunner.de>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: linux-serial@vger.kernel.org, "Jiri Slaby" <jirislaby@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, "Johan Hovold" <johan@kernel.org>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Eric Tremblay" <etremblay@distech-controls.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	heiko@sntech.de, giulio.benetti@micronovasrl.com
Subject: Re: [RFC PATCH 3/7] serial: 8250_dwlib: Implement SW half duplex support
Date: Sun, 6 Mar 2022 20:21:35 +0100	[thread overview]
Message-ID: <20220306192135.GC19394@wunner.de> (raw)
In-Reply-To: <20220302095606.14818-4-ilpo.jarvinen@linux.intel.com>

On Wed, Mar 02, 2022 at 11:56:02AM +0200, Ilpo Järvinen wrote:
> This patch enables support for SW half-duplex mode. Synopsys
> DesignWare UART has a build-in support for the RS485 protocol
> from IP version 4.0 onward with dedicated RE/DE_EN registers.
> This patch enables RS485 either using dedicated registers or
> em485 as fallback.
> 
> In order to select preference for SW half-duplex mode (em485 +
> RE/DE_EN) over HW managed one, as both are supported under
> some configurations, SER_RS485_SW_RX_OR_TX flag is added to
> serial_rs485.
> 
> This patch depends on UART_CAP_NOTEMT which is not provided
> by this series but another one:
>   https://lore.kernel.org/all/20210204161158.643-1-etremblay@distech-controls.com/

I don't see any benefit in using software emulated RTS assertion
if hardware support is present.  It just consumes more CPU time
and is slower to deassert RTS, thereby increasing bus turn-around time.

So if hardware support is present, I think you always want to
use that and you need to fallback to software emulation only
if hardware support is missing (i.e. on IP versions < 4).

The registers you're using here, DW_UART_RE_EN and DW_UART_DE_EN
don't seem to be present on older IP versions.  I'm looking at
the databook for version 3.04a and those registers aren't mentioned:

https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf

So the software emulation you've implemented here won't help with
older IP and the newer IP doesn't need it because it supports
RTS assertion in hardware.  Is that correct?  If so, I'd suggest
not supporting DW_UART_TCR_XFER_MODE_SW_DE_OR_RE at all.

A number of people have attempted to add rs485 software emulation
to 8250_dw.c but noone ever pursued it into mainline.  The last
attempt was by Heiko Stübner, who used patches by Giulio Benetti (+cc):

https://lore.kernel.org/linux-serial/20200517215610.2131618-1-heiko@sntech.de/

FWIW, newer TI Sitara SoCs have also added hardware support for
rs485 and in this (not yet upstreamed) patch, I likewise chose
to use software emulation only if hardware support is not available:

https://github.com/l1k/linux/commit/82c989617a05

Thanks,

Lukas