[PATCH 0/7] Add RS485 support to DW UART

[PATCH 0/7] Add RS485 support to DW UART

[Ilpo Järvinen]

This patchset adds RS-485 support to the DW UART driver. The patchset
has two main parts. The first part adds HW support for RS-485 itself
in various modes of operation and the second part focuses on enabling
9th bit addressing mode that can be used on a multipoint RS-485
communications line.

SW half-duplex patch (3/7) depends on UART_CAP_NOTEMT for which there
is existing work from others:
  https://marc.info/?l=linux-kernel&m=161245538311420&w=2
That patchset is not yet applied (and requires revision from its
author).

To configure multipoint addressing, ADDRB flag is added to termios
and two new IOCTLs are added into serial core. On the driver side,
I looked into using mux subsystem but its model didn't seem to match
well enough to how RS-485 multipoint can be operated.

I'm aware of the RS485 changes Lino Sanfilippo recently posted
which will make one assignment in the patchset redundant. I'll make
the adjustment if those get applied.

Ilpo Järvinen (7):
  serial: 8250_dwlib: RS485 HW half duplex support
  serial: 8250_dwlib: RS485 HW full duplex support
  serial: 8250_dwlib: Implement SW half duplex support
  dt_bindings: snps-dw-apb-uart: Add RS485
  serial: termbits: ADDRB to indicate 9th bit addressing mode
  serial: General support for multipoint addresses
  serial: 8250_dwlib: Support for 9th bit multipoint addressing

 .../bindings/serial/snps-dw-apb-uart.yaml     |  17 ++
 .../driver-api/serial/serial-rs485.rst        |  23 +-
 arch/alpha/include/uapi/asm/ioctls.h          |   3 +
 arch/alpha/include/uapi/asm/termbits.h        |   1 +
 arch/mips/include/uapi/asm/ioctls.h           |   3 +
 arch/mips/include/uapi/asm/termbits.h         |   1 +
 arch/parisc/include/uapi/asm/ioctls.h         |   3 +
 arch/parisc/include/uapi/asm/termbits.h       |   1 +
 arch/powerpc/include/uapi/asm/ioctls.h        |   3 +
 arch/powerpc/include/uapi/asm/termbits.h      |   1 +
 arch/sh/include/uapi/asm/ioctls.h             |   3 +
 arch/sparc/include/uapi/asm/ioctls.h          |   3 +
 arch/sparc/include/uapi/asm/termbits.h        |   1 +
 arch/xtensa/include/uapi/asm/ioctls.h         |   3 +
 drivers/tty/amiserial.c                       |   6 +-
 drivers/tty/moxa.c                            |   1 +
 drivers/tty/mxser.c                           |   1 +
 drivers/tty/serial/8250/8250_core.c           |   2 +
 drivers/tty/serial/8250/8250_dwlib.c          | 246 +++++++++++++++++-
 drivers/tty/serial/8250/8250_dwlib.h          |   6 +
 drivers/tty/serial/serial_core.c              |  62 +++++
 drivers/tty/tty_ioctl.c                       |   2 +
 drivers/usb/serial/usb-serial.c               |   5 +-
 include/linux/serial_core.h                   |   6 +
 include/uapi/asm-generic/ioctls.h             |   3 +
 include/uapi/asm-generic/termbits.h           |   1 +
 include/uapi/linux/serial.h                   |  10 +
 27 files changed, 410 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

The Synopsys DesignWare UART has a build-in support for the
RS485 protocol from IP version 4.0 onward. This commit
enables basic hardware-controlled half duplex mode support
for it.

HW will take care of managing DE and RE, the driver just gives
it permission to use either by setting both to 1.

Co-developed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Co-developed-by: Raymond Tan <raymond.tan@intel.com>
Signed-off-by: Raymond Tan <raymond.tan@intel.com>
Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 67 +++++++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_dwlib.h |  3 ++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 622d3b0d89e7..a4f09a95049b 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -2,19 +2,33 @@
 /* Synopsys DesignWare 8250 library. */
 
 #include <linux/bitops.h>
+#include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/property.h>
 #include <linux/serial_8250.h>
 #include <linux/serial_core.h>
 
 #include "8250_dwlib.h"
 
 /* Offsets for the DesignWare specific registers */
+#define DW_UART_TCR	0xac /* Transceiver Control Register (RS485) */
+#define DW_UART_DE_EN	0xb0 /* Driver Output Enable Register */
+#define DW_UART_RE_EN	0xb4 /* Receiver Output Enable Register */
 #define DW_UART_DLF	0xc0 /* Divisor Latch Fraction Register */
 #define DW_UART_CPR	0xf4 /* Component Parameter Register */
 #define DW_UART_UCV	0xf8 /* UART Component Version */
 
+/* Transceiver Control Register bits */
+#define DW_UART_TCR_RS485_EN		BIT(0)
+#define DW_UART_TCR_RE_POL		BIT(1)
+#define DW_UART_TCR_DE_POL		BIT(2)
+#define DW_UART_TCR_XFER_MODE		GENMASK(4, 3)
+#define DW_UART_TCR_XFER_MODE_DE_DURING_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 0)
+#define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
+#define DW_UART_TCR_XFER_MODE_DE_OR_RE		FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
+
 /* Component Parameter Register bits */
 #define DW_UART_CPR_ABP_DATA_WIDTH	(3 << 0)
 #define DW_UART_CPR_AFCE_MODE		(1 << 4)
@@ -87,11 +101,62 @@ void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct
 }
 EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
 
+static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
+{
+	u32 tcr;
+
+	tcr = dw8250_readl_ext(p, DW_UART_TCR);
+	tcr &= ~DW_UART_TCR_XFER_MODE;
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		/* Clearing unsupported flags. */
+		rs485->flags &= SER_RS485_ENABLED;
+
+		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
+		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
+		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
+	} else {
+		rs485->flags = 0;
+
+		tcr &= ~DW_UART_TCR_RS485_EN;
+		dw8250_writel_ext(p, DW_UART_DE_EN, 0);
+		dw8250_writel_ext(p, DW_UART_RE_EN, 0);
+	}
+
+	/* Resetting the default DE_POL & RE_POL */
+	tcr &= ~(DW_UART_TCR_DE_POL | DW_UART_TCR_RE_POL);
+
+	if (device_property_read_bool(p->dev, "snps,de-active-high"))
+		tcr |= DW_UART_TCR_DE_POL;
+	if (device_property_read_bool(p->dev, "snps,re-active-high"))
+		tcr |= DW_UART_TCR_RE_POL;
+
+	dw8250_writel_ext(p, DW_UART_TCR, tcr);
+
+	/*
+	 * XXX: Though we could interpret the "RTS" timings as Driver Enable
+	 * (DE) assertion/de-assertion timings, initially not supporting that.
+	 * Ideally we should have timing values for the Driver instead of the
+	 * RTS signal.
+	 */
+	rs485->delay_rts_before_send = 0;
+	rs485->delay_rts_after_send = 0;
+
+	p->rs485 = *rs485;
+
+	return 0;
+}
+
 void dw8250_setup_port(struct uart_port *p)
 {
+	struct dw8250_port_data *d = p->private_data;
 	struct uart_8250_port *up = up_to_u8250p(p);
 	u32 reg;
 
+	d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
+	if (d->hw_rs485_support)
+		p->rs485_config = dw8250_rs485_config;
+
 	/*
 	 * If the Component Version Register returns zero, we know that
 	 * ADDITIONAL_FEATURES are not enabled. No need to go any further.
@@ -108,8 +173,6 @@ void dw8250_setup_port(struct uart_port *p)
 	dw8250_writel_ext(p, DW_UART_DLF, 0);
 
 	if (reg) {
-		struct dw8250_port_data *d = p->private_data;
-
 		d->dlf_size = fls(reg);
 		p->get_divisor = dw8250_get_divisor;
 		p->set_divisor = dw8250_set_divisor;
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 83d528e5cc21..a8fa020ca544 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -14,6 +14,9 @@ struct dw8250_port_data {
 
 	/* Hardware configuration */
 	u8			dlf_size;
+
+	/* RS485 variables */
+	bool			hw_rs485_support;
 };
 
 void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old);
-- 
2.30.2

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> +static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
> +{
> +	u32 tcr;
> +
> +	tcr = dw8250_readl_ext(p, DW_UART_TCR);
> +	tcr &= ~DW_UART_TCR_XFER_MODE;
> +
> +	if (rs485->flags & SER_RS485_ENABLED) {
> +		/* Clearing unsupported flags. */

Nit:  Usually we use imperative mood, i.e. "/* clear unsupported flags */".


> +		rs485->flags &= SER_RS485_ENABLED;
> +
> +		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
> +		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
> +		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
> +	} else {
> +		rs485->flags = 0;
> +
> +		tcr &= ~DW_UART_TCR_RS485_EN;
> +		dw8250_writel_ext(p, DW_UART_DE_EN, 0);
> +		dw8250_writel_ext(p, DW_UART_RE_EN, 0);

Do the DW_UART_DE_EN and DW_UART_RE_EN registers have any effect at all
if DW_UART_TCR_RS485_EN is disabled in the TCR register?

If they don't, there's no need to clear them here.  It would be sufficient
to set them once (e.g. on probe).


> +	/* Resetting the default DE_POL & RE_POL */
> +	tcr &= ~(DW_UART_TCR_DE_POL | DW_UART_TCR_RE_POL);

Nit:  Imperative mood, i.e. "/* reset to default polarity */"


> +	if (device_property_read_bool(p->dev, "snps,de-active-high"))
> +		tcr |= DW_UART_TCR_DE_POL;

That device property is a duplication of the existing "rs485-rts-active-low"
property.  Please use the existing one unless there are devices already
in the field which use the new property (in which case that should be
provided as justification in the commit message).

Does the DesignWare UART use dedicated DE and RE pins instead of
the RTS pin?  That would be quite unusual.


> +	if (device_property_read_bool(p->dev, "snps,re-active-high"))
> +		tcr |= DW_UART_TCR_RE_POL;

Heiko Stübner (+cc) posted patches in 2020 to support a separate RE pin
in addition to a DE pin (which is usually simply the RTS pin):

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

He called the devicetree property for the pin "rs485-rx-enable",
so perhaps "rs485-rx-active-low" would be a better name here.


> +	/*
> +	 * XXX: Though we could interpret the "RTS" timings as Driver Enable
> +	 * (DE) assertion/de-assertion timings, initially not supporting that.
> +	 * Ideally we should have timing values for the Driver instead of the
> +	 * RTS signal.
> +	 */
> +	rs485->delay_rts_before_send = 0;
> +	rs485->delay_rts_after_send = 0;

I don't quite understand this code comment.


>  void dw8250_setup_port(struct uart_port *p)
>  {
> +	struct dw8250_port_data *d = p->private_data;
>  	struct uart_8250_port *up = up_to_u8250p(p);
>  	u32 reg;
>  
> +	d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
> +	if (d->hw_rs485_support)
> +		p->rs485_config = dw8250_rs485_config;
> +

You wrote in the commit message that rs485 support is present from
version 4.0 onward.  Can't we just check the IP version and enable
rs485 support for >= 4.0?  That would seem more appropriate instead
of introducing yet another new property.

Note that dw8250_setup_port() already reads the version from the
DW_UART_UCV register.

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Andy Shevchenko]

On Mon, Mar 7, 2022 at 12:00 AM Lukas Wunner <lukas@wunner.de> wrote:
> On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:

...

> Does the DesignWare UART use dedicated DE and RE pins instead of
> the RTS pin?  That would be quite unusual.

They are muxed with other UART pins on SoC level, but I don't remember
by heart which ones. According to the Synopsys datasheet they are
separate signals. It might be that I'm missing something, since the
last time I looked was last year.

...

> > +     d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
> > +     if (d->hw_rs485_support)
> > +             p->rs485_config = dw8250_rs485_config;
> > +
>
> You wrote in the commit message that rs485 support is present from
> version 4.0 onward.  Can't we just check the IP version and enable
> rs485 support for >= 4.0?  That would seem more appropriate instead
> of introducing yet another new property.

AFAIU this is dependent on the IP syntheses. I.o.w. version 4.0+ is a
prerequisite, but doesn't automatically mean that there is a support.
Unfortunately there is no way to tell this clearly in the IP
configuration register.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

On Mon, 7 Mar 2022, Andy Shevchenko wrote:

> On Mon, Mar 7, 2022 at 12:00 AM Lukas Wunner <lukas@wunner.de> wrote:
> > On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> 
> ...
> 
> > Does the DesignWare UART use dedicated DE and RE pins instead of
> > the RTS pin?  That would be quite unusual.
> 
> They are muxed with other UART pins on SoC level, but I don't remember
> by heart which ones. According to the Synopsys datasheet they are
> separate signals. It might be that I'm missing something, since the
> last time I looked was last year.

Unusual or not, there is a pin for both DE and RE. DE is muxed with RTS.

> > > +     d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
> > > +     if (d->hw_rs485_support)
> > > +             p->rs485_config = dw8250_rs485_config;
> > > +
> >
> > You wrote in the commit message that rs485 support is present from
> > version 4.0 onward.  Can't we just check the IP version and enable
> > rs485 support for >= 4.0?  That would seem more appropriate instead
> > of introducing yet another new property.
> 
> AFAIU this is dependent on the IP syntheses. I.o.w. version 4.0+ is a
> prerequisite, but doesn't automatically mean that there is a support.
> Unfortunately there is no way to tell this clearly in the IP
> configuration register.

And the IP synthesis only part of the picture, in general case, it'd
also matter that there's something connected to that RE (i.e.,
an RS485 transceiver).

On the board I'm testing with, I can also turn RS485 on/off from BIOS
which makes the pins (mainly RE) behave differently.

I initially had additional version check here while developing this
patch series but it seemed to not provide any added value due those
other factors that need to be considered.


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Mon, Mar 07, 2022 at 11:19:59AM +0200, Ilpo Järvinen wrote:
> On Mon, 7 Mar 2022, Andy Shevchenko wrote:
> > On Mon, Mar 7, 2022 at 12:00 AM Lukas Wunner <lukas@wunner.de> wrote:
> > > On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> > > > +     d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
> > > > +     if (d->hw_rs485_support)
> > > > +             p->rs485_config = dw8250_rs485_config;
> > > > +
> > >
> > > You wrote in the commit message that rs485 support is present from
> > > version 4.0 onward.  Can't we just check the IP version and enable
> > > rs485 support for >= 4.0?  That would seem more appropriate instead
> > > of introducing yet another new property.
> > 
> > AFAIU this is dependent on the IP syntheses. I.o.w. version 4.0+ is a
> > prerequisite, but doesn't automatically mean that there is a support.
> > Unfortunately there is no way to tell this clearly in the IP
> > configuration register.
> 
> And the IP synthesis only part of the picture, in general case, it'd
> also matter that there's something connected to that RE (i.e.,
> an RS485 transceiver).

If an RS-485 transceiver is *soldered* to the UART, the devicetree
is supposed to contain the property "linux,rs485-enabled-at-boot-time"
under the UART's of_node.  In that case the UART driver can (and should)
enable rs485 mode already on ->probe.

Of course there's also the possibility to enable RS-485 after ->open
with the TIOCSRS485 ioctl.  That can be used if the transceiver is
attached at runtime (which is likely a rare use case) or as a legacy
enablement method if the driver lacks linux,rs485-enabled-at-boot-time
support.

That's for DT platforms, but I suppose you've got ACPI.  Not sure
how it's handled there, the ACPI 6.4 spec contains a "UART Serial Bus
Connection Resource Descriptor" but nothing on RS-485, so I guess
the only option is to use regular DT properties in a _DSD object?


> I initially had additional version check here while developing this
> patch series but it seemed to not provide any added value due those
> other factors that need to be considered.

Here's another idea:

Read TCR register on ->probe.  It's POR value is 0x6 if RS-485 is
supported by the chip, else 0x0.  (Page 220 of the 4.01a spec says
UCV register does not exist if additional features are not implemented
and reading from this register address returns 0, I suppose the same
applies to TCR if RS-485 is not implemented.)

Since the driver may change the polarity in the TCR register, be sure
to write 0x6 to it on ->remove so that you can still correctly detect
presence of the RS-485 feature after unbind/rebind of the driver.

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Andy Shevchenko]

On Mon, Mar 07, 2022 at 08:18:54PM +0100, Lukas Wunner wrote:
> On Mon, Mar 07, 2022 at 11:19:59AM +0200, Ilpo Järvinen wrote:
> > On Mon, 7 Mar 2022, Andy Shevchenko wrote:

...

> That's for DT platforms, but I suppose you've got ACPI.  Not sure
> how it's handled there, the ACPI 6.4 spec contains a "UART Serial Bus
> Connection Resource Descriptor" but nothing on RS-485, so I guess
> the only option is to use regular DT properties in a _DSD object?

Which make me think that this series needs an additional patch to
describe RS485 enumeration for ACPI case (somewhere in
Documentation/firmware-guide/acpi/enumeration.rst IIRC the filename).

...

> > I initially had additional version check here while developing this
> > patch series but it seemed to not provide any added value due those
> > other factors that need to be considered.
> 
> Here's another idea:
> 
> Read TCR register on ->probe.  It's POR value is 0x6 if RS-485 is
> supported by the chip, else 0x0.  (Page 220 of the 4.01a spec says
> UCV register does not exist if additional features are not implemented
> and reading from this register address returns 0, I suppose the same
> applies to TCR if RS-485 is not implemented.)
> 
> Since the driver may change the polarity in the TCR register, be sure
> to write 0x6 to it on ->remove so that you can still correctly detect
> presence of the RS-485 feature after unbind/rebind of the driver.

What to do in the case when DE pin is muxed to RTS and locked in pin control
IP by firmware (no possibility to change the muxing in the OS)?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

On Mon, 7 Mar 2022, Andy Shevchenko wrote:

> On Mon, Mar 07, 2022 at 08:18:54PM +0100, Lukas Wunner wrote:
> > On Mon, Mar 07, 2022 at 11:19:59AM +0200, Ilpo Järvinen wrote:
> > > On Mon, 7 Mar 2022, Andy Shevchenko wrote:
> 
> ...
> 
> > That's for DT platforms, but I suppose you've got ACPI.  Not sure
> > how it's handled there, the ACPI 6.4 spec contains a "UART Serial Bus
> > Connection Resource Descriptor" but nothing on RS-485, so I guess
> > the only option is to use regular DT properties in a _DSD object?
> 
> Which make me think that this series needs an additional patch to
> describe RS485 enumeration for ACPI case (somewhere in
> Documentation/firmware-guide/acpi/enumeration.rst IIRC the filename).
> 
> ...
> 
> > > I initially had additional version check here while developing this
> > > patch series but it seemed to not provide any added value due those
> > > other factors that need to be considered.
> > 
> > Here's another idea:
> > 
> > Read TCR register on ->probe.  It's POR value is 0x6 if RS-485 is
> > supported by the chip, else 0x0.  (Page 220 of the 4.01a spec says
> > UCV register does not exist if additional features are not implemented
> > and reading from this register address returns 0, I suppose the same
> > applies to TCR if RS-485 is not implemented.)
> > 
> > Since the driver may change the polarity in the TCR register, be sure
> > to write 0x6 to it on ->remove so that you can still correctly detect
> > presence of the RS-485 feature after unbind/rebind of the driver.
> 
> What to do in the case when DE pin is muxed to RTS and locked in pin control
> IP by firmware (no possibility to change the muxing in the OS)?

The SoC also has a pin to select between RS485 and RS232. With a combo 
transceiver, TCR-based heuristic just runs into the same problems as the 
version-based one did.


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Tue, Mar 08, 2022 at 02:16:56PM +0200, Ilpo Järvinen wrote:
> The SoC also has a pin to select between RS485 and RS232. With a combo 
> transceiver, TCR-based heuristic just runs into the same problems as the 
> version-based one did.

I thought this was about detecting whether hardware-assisted DE assertion
may be used (versus software-controlled), not about whether to enable
RS-485 mode.  Right?

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Tue, 8 Mar 2022, Lukas Wunner wrote:

> On Tue, Mar 08, 2022 at 02:16:56PM +0200, Ilpo Järvinen wrote:
> > The SoC also has a pin to select between RS485 and RS232. With a combo 
> > transceiver, TCR-based heuristic just runs into the same problems as the 
> > version-based one did.
> 
> I thought this was about detecting whether hardware-assisted DE assertion
> may be used (versus software-controlled), not about whether to enable
> RS-485 mode.  Right?

HW DE assertion only works when RS485 mode is enabled so I don't see how 
these questions could be easily decoupled like that. That's assuming with 
"software-controlled" you mean RTS(RS232)+em485?


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Tue, Mar 08, 2022 at 02:59:59PM +0200, Ilpo Järvinen wrote:
> On Tue, 8 Mar 2022, Lukas Wunner wrote:
> > On Tue, Mar 08, 2022 at 02:16:56PM +0200, Ilpo Järvinen wrote:
> > > The SoC also has a pin to select between RS485 and RS232. With a combo 
> > > transceiver, TCR-based heuristic just runs into the same problems as the 
> > > version-based one did.
> > 
> > I thought this was about detecting whether hardware-assisted DE assertion
> > may be used (versus software-controlled), not about whether to enable
> > RS-485 mode.  Right?
> 
> HW DE assertion only works when RS485 mode is enabled so I don't see how 
> these questions could be easily decoupled like that. That's assuming with 
> "software-controlled" you mean RTS(RS232)+em485?

Right, that's what I meant.

Enabling RS-485 mode is only supposed to happen upon a TIOCSRS485 ioctl
or if the "linux,rs485-enabled-at-boot-time" property is present.
We don't need to second-guess the user's decision to enable RS-485 mode.
If that's what they've asked for, then we can and should assume that an
RS-485 transceiver is attached.

Of course, if hardware-assisted DE assertion requires a particular pinmux
state, we could double-check whether that pinmux state is set.  If the
RTS/DE pin is not muxed as a DE pin but rather as an RTS pin, one option
would be to fall back to software-controlled RTS assertion.  A warning
message may be warranted in that case.

Whether hardware-assisted DE assertion is supported by the chip can not
only be detected by checking for the POR 0x6 value in the TCR register:
You can alternatively write a non-zero value to any of the RS-485 registers,
then check if reading the register back returns a non-zero value
(RE_EN is probably a good candidate).  That approach is more robust
than relying on the POR value 0x6 in TCR because you never know if
boot firmware fiddled with the registers before passing control to the
kernel.

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Andy Shevchenko]

On Tue, Mar 8, 2022 at 4:50 PM Lukas Wunner <lukas@wunner.de> wrote:
> On Tue, Mar 08, 2022 at 02:59:59PM +0200, Ilpo Järvinen wrote:

...

> Of course, if hardware-assisted DE assertion requires a particular pinmux
> state, we could double-check whether that pinmux state is set.

I'm wondering how to achieve this.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Tue, Mar 08, 2022 at 04:53:56PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 8, 2022 at 4:50 PM Lukas Wunner <lukas@wunner.de> wrote:
> > Of course, if hardware-assisted DE assertion requires a particular pinmux
> > state, we could double-check whether that pinmux state is set.
> 
> I'm wondering how to achieve this.

On DT platforms, the devicetree specifies the pin controller settings
which need to be configured for a device to be usable, e.g.:

  pinctrl-names = "default";
  pinctrl-0 = <...>;

Before a driver is bound to the device, really_probe() in drivers/base/dd.c
calls pinctrl_bind_pins() which configures the pin controller accordingly.
In other words, the OS is fully in charge of configuring the pinmux.

I'm not sure how this is done on ACPI platforms.  If the pinmux is
exclusively under the control of the platform firmware and the OS has
no way of getting or setting the pinmux configuration, then that would
be a competitive disadvantage vis-à-vis DT platforms which should really
be addressed.  However I notice there are various drivers for Intel
chipsets in drivers/pinctrl/intel/, so surely there's a way to let the
OS handle pinmux settings?

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Tue, 8 Mar 2022, Lukas Wunner wrote:

> On Tue, Mar 08, 2022 at 04:53:56PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 8, 2022 at 4:50 PM Lukas Wunner <lukas@wunner.de> wrote:
> > > Of course, if hardware-assisted DE assertion requires a particular pinmux
> > > state, we could double-check whether that pinmux state is set.
> > 
> > I'm wondering how to achieve this.
> 
> On DT platforms, the devicetree specifies the pin controller settings
> which need to be configured for a device to be usable, e.g.:
> 
>   pinctrl-names = "default";
>   pinctrl-0 = <...>;
> 
> Before a driver is bound to the device, really_probe() in drivers/base/dd.c
> calls pinctrl_bind_pins() which configures the pin controller accordingly.
> In other words, the OS is fully in charge of configuring the pinmux.
> 
> I'm not sure how this is done on ACPI platforms.  If the pinmux is
> exclusively under the control of the platform firmware and the OS has
> no way of getting or setting the pinmux configuration, then that would
> be a competitive disadvantage vis-à-vis DT platforms which should really
> be addressed.  However I notice there are various drivers for Intel
> chipsets in drivers/pinctrl/intel/, so surely there's a way to let the
> OS handle pinmux settings?

The problem here is that the driver ("we could double-check" in your 
initial suggestion above) doesn't know which pins it should check the 
states for. I don't think any general mapping exists between drivers and 
pins.

Based on what I read, the mapping is a feature not wanted into pinmuxing.
Assuming I understood them correctly, they don't want to do such thing
on kernel side (based on experience with earlier approaches with 
mapping. It probably got too messy/unmaintainable in the end :-)).
So at best, one can only read and control pin mux states but that's about 
as far as pinmuxing in kernel goes (and control could be locked by FW).

Anyway, I've implemented the detection now based on RE_EN non-zero write + 
read + check based on your suggestion (despite still thinking myself it 
has these obvious problems with pinmux & other hw config unrelated dw uart 
itself).


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3920 bytes --]

On Sun, 6 Mar 2022, Lukas Wunner wrote:

> On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
>
> > +		rs485->flags &= SER_RS485_ENABLED;
> > +
> > +		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
> > +		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
> > +		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
> > +	} else {
> > +		rs485->flags = 0;
> > +
> > +		tcr &= ~DW_UART_TCR_RS485_EN;
> > +		dw8250_writel_ext(p, DW_UART_DE_EN, 0);
> > +		dw8250_writel_ext(p, DW_UART_RE_EN, 0);
> 
> Do the DW_UART_DE_EN and DW_UART_RE_EN registers have any effect at all
> if DW_UART_TCR_RS485_EN is disabled in the TCR register?
> 
> If they don't, there's no need to clear them here.  It would be sufficient
> to set them once (e.g. on probe).

They have no impact when in non-RS485 mode. I just removed them.

> > +	if (device_property_read_bool(p->dev, "snps,de-active-high"))
> > +		tcr |= DW_UART_TCR_DE_POL;
> 
> That device property is a duplication of the existing "rs485-rts-active-low"
> property.  Please use the existing one unless there are devices already
> in the field which use the new property (in which case that should be
> provided as justification in the commit message).
>
> Does the DesignWare UART use dedicated DE and RE pins instead of
> the RTS pin?  That would be quite unusual.
>
> > +	if (device_property_read_bool(p->dev, "snps,re-active-high"))
> > +		tcr |= DW_UART_TCR_RE_POL;
> 
> Heiko Stübner (+cc) posted patches in 2020 to support a separate RE pin
> in addition to a DE pin (which is usually simply the RTS pin):
> 
> https://lore.kernel.org/linux-serial/20200517215610.2131618-4-heiko@sntech.de/
> 
> He called the devicetree property for the pin "rs485-rx-enable",
> so perhaps "rs485-rx-active-low" would be a better name here.

While I believe there exist devices on the field with 
snps,re-active-high set to true, if the default matches to that, the 
impact of the naming mismatch will be near zero (likely zero).

Based on the Rob's earlier comment on the dt patch itself. I had already 
plans on changing these. My thought was to make it like this:
- rs485-de-active-low
- rs485-re-active-high

I don't have strong opinion on the actual names myself (every RS-485 
transceivers I've come across name their pins to DE and RE).

Given that you seemed to consider DE "unusual" despite being reality
with this hw, I don't know whether you still think the meaning of 
rs485-rts-active-low should be overloaded to also mean rs485-de-active-low?
(I think such overloading would be harmless so I'm not exactly opposing
other than noting FW/HW folks might find it odd to misname it to rts.)

What I think is more important though, is that RE would be by default
active low which matches to about every RS485 transceiver expectations.
Given what device_property_read_bool does when the property is missing,
it would make sense to have the property named as -active-high but I
don't know if that breaks some dt naming rule to have them opposites
of each other like that?

> > +	/*
> > +	 * XXX: Though we could interpret the "RTS" timings as Driver Enable
> > +	 * (DE) assertion/de-assertion timings, initially not supporting that.
> > +	 * Ideally we should have timing values for the Driver instead of the
> > +	 * RTS signal.
> > +	 */
> > +	rs485->delay_rts_before_send = 0;
> > +	rs485->delay_rts_after_send = 0;
> 
> I don't quite understand this code comment.

It seemed to be missing one "Enable" word.

Here's my interpretation of it (this comment was written by somebody 
else, perhaps either Heikki or Andy):

Since this HW has dedicated DE/RE in contrast to RTS, it is not specified 
anywhere that delay_rts_* should apply to them as well and that the 
writer of that comment was hoping to have something dedicated to them
rather than repurposing RTS-related fields.

I could of course change this if everything called RTS should be applied 
to DE as well?


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Mon, Mar 07, 2022 at 12:54:19PM +0200, Ilpo Järvinen wrote:
> I don't have strong opinion on the actual names myself (every RS-485 
> transceivers I've come across name their pins to DE and RE).

It's true that transceiver datasheets usually call these pins DE and RE,
but on the UART side, by convention the RTS pin is used to drive DE.

And RTS is then either asserted/deasserted in software (by the kernel
driver), or by the UART hardware if it's capable of it.

Hence the properties in Documentation/devicetree/bindings/serial/rs485.yaml
and the members in struct serial_rs485 refer to "rts".  It's synonymous
to DE.  I suppose Synopsys wanted to afford the integrator of the IP core
as much freedom as possible and therefore offers separate RTS+DE pins
as well as an RE pin.  But that degree of freedom also leads to confusion,
particularly if firmware might mux the pins in an unexpected way
behind the OS's back.

The RE pin of transceivers is usually either pulled-up (i.e. always
asserted, full-duplex) or connected to negated RTS (half-duplex).
A lot of transceivers have a !RE pin so RTS can be wired directly
to DE and !RE.

Full-duplex is primarily for RS-422.  If full-duplex is enabled with
RS-485, you'll receive your own echo, which is often (but not always)
undesirable (depends on the application).

For simplicity and consistency, it is best if the existing properties
defined in rs485.yaml are used, instead of defining new ones which
have the same meaning.  For this reason I'd recommend using
rs485-rts-active-low for the polarity of the DE pin.


> What I think is more important though, is that RE would be by default
> active low which matches to about every RS485 transceiver expectations.
> Given what device_property_read_bool does when the property is missing,
> it would make sense to have the property named as -active-high but I
> don't know if that breaks some dt naming rule to have them opposites
> of each other like that?

That's a good point, I agree.  I don't think that would violate any
DT naming rule.

Thanks,

Lukas

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]

On Mon, 7 Mar 2022, Ilpo Järvinen wrote:

> On Sun, 6 Mar 2022, Lukas Wunner wrote:
> 
> > On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> 
> > > +	/*
> > > +	 * XXX: Though we could interpret the "RTS" timings as Driver Enable
> > > +	 * (DE) assertion/de-assertion timings, initially not supporting that.
> > > +	 * Ideally we should have timing values for the Driver instead of the
> > > +	 * RTS signal.
> > > +	 */
> > > +	rs485->delay_rts_before_send = 0;
> > > +	rs485->delay_rts_after_send = 0;
> > 
> > I don't quite understand this code comment.
> 
> It seemed to be missing one "Enable" word.
> 
> Here's my interpretation of it (this comment was written by somebody 
> else, perhaps either Heikki or Andy):
> 
> Since this HW has dedicated DE/RE in contrast to RTS, it is not specified 
> anywhere that delay_rts_* should apply to them as well and that the 
> writer of that comment was hoping to have something dedicated to them
> rather than repurposing RTS-related fields.
> 
> I could of course change this if everything called RTS should be applied 
> to DE as well?

Now that it has been pretty much established that anything called "rts" 
should be applied to DE as well, I took another look on implementing these 
delays.

It turns out to be impractical to do/ineffective because "serial clock 
periods" are used as the unit by the HW ("serial clock periods" is not as 
clearly defined by the datasheet as I'd like but it is most likely based 
on the high-rated uartclk cycles). With the uartclk I've on test HW, the 
combined delay with max turnaround time and DE assert/de-assert timings 
cannot do even the smallest possible non-zero value (1 msec). That's 
because the TAT and DET registers allow only 16-bit and 8-bit values for 
delays.

I also attempted to test it by writing the maximum values into them and 
got no visible difference. There a note about +1 for delay in TAT so to 
play safe I put 0xfff0 instead 0xffff (if the HW couldn't handle that 
16-bit overflow properly).

Perhaps the SW half-duplex with DE/RE will have to be used to cover this 
case?


-- 
 i.

Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

[Lukas Wunner]

On Wed, Mar 09, 2022 at 02:19:39PM +0200, Ilpo Järvinen wrote:
> Now that it has been pretty much established that anything called "rts" 
> should be applied to DE as well, I took another look on implementing these 
> delays.
> 
> It turns out to be impractical to do/ineffective because "serial clock 
> periods" are used as the unit by the HW ("serial clock periods" is not as 
> clearly defined by the datasheet as I'd like but it is most likely based 
> on the high-rated uartclk cycles). With the uartclk I've on test HW, the 
> combined delay with max turnaround time and DE assert/de-assert timings 
> cannot do even the smallest possible non-zero value (1 msec). That's 
> because the TAT and DET registers allow only 16-bit and 8-bit values for 
> delays.

A mistake was made when RS-485 support was introduced in the kernel
more than 10 years ago with commits c26c56c0 and 93f3350c:

The delays were defined in msec, but if you look at datasheets of
RS-485 transceivers, they only need a delay in the nanosecond or
single-digit usec range.

Here's a collection of delays I compiled two years ago:

                                 DrvEnable-to-Output    DriverPropagation
          MAX13450E/MAX13451E    5200 ns                 800 ns
          MAXM22510              2540 ns                1040 ns
          MAXM22511                80 ns                  65 ns
          SN65HVD72              9000 ns                1000 ns
          SN65HVD75              7000 ns                  17 ns
          SN65HVD78              8000 ns                  15 ns
          SN65HVD485E            2600 ns                  30 ns
          ADM1486                  15 ns                  17 ns
          ADM3485/ADM3490/ADM3491 900 ns                  35 ns
          ADM3483/ADM3488        1300 ns                1500 ns
          XR33193                2000 ns                1300 ns

Because these delays are so short, it is usually sufficient to set
them to zero in struct serial_rs485.

I've begun a commit to change the delays to nsec, it's still a WIP:

https://github.com/l1k/linux/commit/2c08878b63d6

This is a little tricky because the delays are user-space ABI,
so great care is needed to avoid breakage.  Also, every rs485
driver with support for delays needs to be touched.  Some
UARTs have fixed delays which depend on different clocks,
other UARTs support configurable delays.  Another complication
is that delay calculations easily overflow with nsec because
the numbers become quite large.

A positive side effect of changing the delays to nsec is that
the horrible hrtimer kludge for rs485 software emulation in the
8250_port.c can be eliminated.  Also, all the illegal mdelay()s
in spinlocked context (e.g. serial console output) are replaced
by much more reasonable ndelay()s.

Eliminating the hrtimer kludge in 8250_port.c might also make these
runtime PM patches by Andy simpler:

https://lore.kernel.org/linux-serial/20211115084203.56478-8-tony@atomide.com/

My suggestion would be to set the delays to zero for now in 8250_dw.c
and implement proper delay handling after I've finished the conversion
to nsec.

Thanks,

Lukas

[PATCH 2/7] serial: 8250_dwlib: RS485 HW full duplex support

[Ilpo Järvinen]

The Synopsys DesignWare UART has a build-in support for the
RS485 protocol from IP version 4.0 onward. This patch enables
the support of HW Full Duplex mode support for it.

To ask for full duplex mode, userspace sets SER_RS485_RX_DURING_TX
flag and HW will take care of the rest.

Co-developed-by: Raymond Tan <raymond.tan@intel.com>
Signed-off-by: Raymond Tan <raymond.tan@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index a4f09a95049b..d26792999984 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -110,9 +110,14 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
 
 	if (rs485->flags & SER_RS485_ENABLED) {
 		/* Clearing unsupported flags. */
-		rs485->flags &= SER_RS485_ENABLED;
-
-		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
+		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
+		tcr |= DW_UART_TCR_RS485_EN;
+
+		if (rs485->flags & SER_RS485_RX_DURING_TX) {
+			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
+		} else {
+			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
+		}
 		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
 		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
 	} else {
-- 
2.30.2

Re: [PATCH 2/7] serial: 8250_dwlib: RS485 HW full duplex support

[Lukas Wunner]

On Wed, Mar 02, 2022 at 11:56:01AM +0200, Ilpo Järvinen wrote:
> @@ -110,9 +110,14 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
>  
>  	if (rs485->flags & SER_RS485_ENABLED) {
>  		/* Clearing unsupported flags. */
> -		rs485->flags &= SER_RS485_ENABLED;
> -
> -		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
> +		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
> +		tcr |= DW_UART_TCR_RS485_EN;
> +
> +		if (rs485->flags & SER_RS485_RX_DURING_TX) {
> +			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
> +		} else {
> +			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
> +		}

This patch deletes lines introduced by the preceding patch.
I'd just squash the two together, I don't see much value in
introducing full duplex support in a separate patch.

Thanks,

Lukas

Re: [PATCH 2/7] serial: 8250_dwlib: RS485 HW full duplex support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Sun, 6 Mar 2022, Lukas Wunner wrote:

> On Wed, Mar 02, 2022 at 11:56:01AM +0200, Ilpo Järvinen wrote:
> > @@ -110,9 +110,14 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
> >  
> >  	if (rs485->flags & SER_RS485_ENABLED) {
> >  		/* Clearing unsupported flags. */
> > -		rs485->flags &= SER_RS485_ENABLED;
> > -
> > -		tcr |= DW_UART_TCR_RS485_EN | DW_UART_TCR_XFER_MODE_DE_OR_RE;
> > +		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
> > +		tcr |= DW_UART_TCR_RS485_EN;
> > +
> > +		if (rs485->flags & SER_RS485_RX_DURING_TX) {
> > +			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
> > +		} else {
> > +			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
> > +		}
> 
> This patch deletes lines introduced by the preceding patch.
> I'd just squash the two together, I don't see much value in
> introducing full duplex support in a separate patch.

Sure, I can merge them.

-- 
 i.

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

[Ilpo Järvinen]

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/

Cc: Eric Tremblay <etremblay@distech-controls.com>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 60 +++++++++++++++++++++++++---
 include/uapi/linux/serial.h          |  2 +
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index d26792999984..51b0f55ee9d0 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -101,8 +101,26 @@ void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct
 }
 EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
 
+static void dw8250_rs485_start_tx(struct uart_8250_port *up)
+{
+	struct uart_port *p = &(up->port);
+
+	dw8250_writel_ext(p, DW_UART_RE_EN, 0);
+	dw8250_writel_ext(p, DW_UART_DE_EN, 1);
+}
+
+static void dw8250_rs485_stop_tx(struct uart_8250_port *up)
+{
+	struct uart_port *p = &(up->port);
+
+	dw8250_writel_ext(p, DW_UART_DE_EN, 0);
+	dw8250_writel_ext(p, DW_UART_RE_EN, 1);
+}
+
 static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
 {
+	struct uart_8250_port *up = up_to_u8250p(p);
+	u32 re_en, de_en;
 	u32 tcr;
 
 	tcr = dw8250_readl_ext(p, DW_UART_TCR);
@@ -110,16 +128,29 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
 
 	if (rs485->flags & SER_RS485_ENABLED) {
 		/* Clearing unsupported flags. */
-		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
+		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX | SER_RS485_SW_RX_OR_TX;
 		tcr |= DW_UART_TCR_RS485_EN;
 
 		if (rs485->flags & SER_RS485_RX_DURING_TX) {
 			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
+			re_en = 1;
+			de_en = 1;
+			rs485->flags &= ~SER_RS485_SW_RX_OR_TX;
+		} else if (rs485->flags & SER_RS485_SW_RX_OR_TX) {
+			tcr |= DW_UART_TCR_XFER_MODE_SW_DE_OR_RE;
+			re_en = 1;
+			de_en = 0;
+			if (up->em485 && !up->em485->tx_stopped) {
+				re_en = 0;
+				de_en = 1;
+			}
 		} else {
 			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
+			re_en = 1;
+			de_en = 1;
 		}
-		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
-		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
+		dw8250_writel_ext(p, DW_UART_DE_EN, de_en);
+		dw8250_writel_ext(p, DW_UART_RE_EN, re_en);
 	} else {
 		rs485->flags = 0;
 
@@ -147,7 +178,16 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
 	rs485->delay_rts_before_send = 0;
 	rs485->delay_rts_after_send = 0;
 
-	p->rs485 = *rs485;
+	if (rs485->flags & SER_RS485_SW_RX_OR_TX) {
+		int ret;
+
+		ret = serial8250_em485_config(p, rs485);
+		if (ret)
+			return ret;
+	} else {
+		serial8250_em485_destroy(up_to_u8250p(p));
+		p->rs485 = *rs485;
+	}
 
 	return 0;
 }
@@ -159,8 +199,16 @@ void dw8250_setup_port(struct uart_port *p)
 	u32 reg;
 
 	d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
-	if (d->hw_rs485_support)
+	if (d->hw_rs485_support) {
 		p->rs485_config = dw8250_rs485_config;
+		up->rs485_start_tx = dw8250_rs485_start_tx;
+		up->rs485_stop_tx = dw8250_rs485_stop_tx;
+	} else {
+		p->rs485_config = serial8250_em485_config;
+		up->rs485_start_tx = serial8250_em485_start_tx;
+		up->rs485_stop_tx = serial8250_em485_stop_tx;
+	}
+	up->capabilities |= UART_CAP_NOTEMT;
 
 	/*
 	 * If the Component Version Register returns zero, we know that
@@ -192,7 +240,7 @@ void dw8250_setup_port(struct uart_port *p)
 		p->type = PORT_16550A;
 		p->flags |= UPF_FIXED_TYPE;
 		p->fifosize = DW_UART_CPR_FIFO_SIZE(reg);
-		up->capabilities = UART_CAP_FIFO;
+		up->capabilities = UART_CAP_FIFO | UART_CAP_NOTEMT;
 	}
 
 	if (reg & DW_UART_CPR_AFCE_MODE)
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index fa6b16e5fdd8..f868685b35a0 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -126,6 +126,8 @@ struct serial_rs485 {
 #define SER_RS485_TERMINATE_BUS		(1 << 5)	/* Enable bus
 							   termination
 							   (if supported) */
+#define SER_RS485_SW_RX_OR_TX		(1 << 6)	/* Prefer SW half-duplex
+							   mode over HW one */
 	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
 	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
2.30.2

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

[Lukas Wunner]

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

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

[Andy Shevchenko]

On Mon, Mar 7, 2022 at 12:01 AM Lukas Wunner <lukas@wunner.de> wrote:
> 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).

No, this is not a good idea because presence of the HW support does
not guarantee we will have pins on PCB level and/or properly muxed by
firmware, or be able to be properly muxed by software (OS). Or am I
missing something here and you have an idea how to enable RS485 in
that case?

> 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



-- 
With Best Regards,
Andy Shevchenko

[PATCH 4/7] dt_bindings: snps-dw-apb-uart: Add RS485

[Ilpo Järvinen]

Add RS485 enable & line polarity properties.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../bindings/serial/snps-dw-apb-uart.yaml       | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 12137fe80acf..8d440afabb1f 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -103,6 +103,23 @@ properties:
       register. Define this if your serial port does not use this pin.
     type: boolean
 
+  snps,rs485-interface-en:
+    description: Use true RS-485 DE and RE signals (in contrast to RS-485
+      emulation using RTS). Requires IP version 4 or above.
+    type: boolean
+
+  snps,de-active-high:
+    description: Defines the polarity of driver enable (DE_EN) signal.
+      Meaningful only with snps,rs485-interface-en. True indicates active
+      high.
+    type: boolean
+
+  snps,re-active-high:
+    description: Defines the polarity of receiver enable (RE_EN) signal.
+      Meaningful only with snps,rs485-interface-en. True indicates active
+      high.
+    type: boolean
+
 required:
   - compatible
   - reg
-- 
2.30.2

Re: [PATCH 4/7] dt_bindings: snps-dw-apb-uart: Add RS485

[Rob Herring]

On Wed, Mar 02, 2022 at 11:56:03AM +0200, Ilpo Järvinen wrote:
> Add RS485 enable & line polarity properties.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  .../bindings/serial/snps-dw-apb-uart.yaml       | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> index 12137fe80acf..8d440afabb1f 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> @@ -103,6 +103,23 @@ properties:
>        register. Define this if your serial port does not use this pin.
>      type: boolean
>  
> +  snps,rs485-interface-en:
> +    description: Use true RS-485 DE and RE signals (in contrast to RS-485
> +      emulation using RTS). Requires IP version 4 or above.
> +    type: boolean
> +

> +  snps,de-active-high:
> +    description: Defines the polarity of driver enable (DE_EN) signal.
> +      Meaningful only with snps,rs485-interface-en. True indicates active
> +      high.
> +    type: boolean
> +
> +  snps,re-active-high:
> +    description: Defines the polarity of receiver enable (RE_EN) signal.
> +      Meaningful only with snps,rs485-interface-en. True indicates active
> +      high.
> +    type: boolean

Seems plausible that other rs485 implementations could have varying 
polarities too. IOW, should be common properties.

> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.30.2
> 

[RFC PATCH 5/7] serial: termbits: ADDRB to indicate 9th bit addressing mode

[Ilpo Järvinen]

Add ADDRB to termbits to indicate 9th bit addressing mode.
This change is necessary for supporting devices with RS485
multipoint addressing [*]. A later patch in the patch series
adds support for Synopsys Designware UART capable for 9th bit
addressing mode. In this mode, 9th bit is used to indicate an
address (byte) within the communication line. The 9th bit
addressing mode is selected using ADDRB introduced by an earlier
patch.

[*] Technically, RS485 is just an electronic spec and does not
itself specify the 9th bit addressing mode but 9th bit seems
at least "semi-standard" way to do addressing with RS485.

Cc: linux-api@vger.kernel.org
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/alpha/include/uapi/asm/termbits.h   | 1 +
 arch/mips/include/uapi/asm/termbits.h    | 1 +
 arch/parisc/include/uapi/asm/termbits.h  | 1 +
 arch/powerpc/include/uapi/asm/termbits.h | 1 +
 arch/sparc/include/uapi/asm/termbits.h   | 1 +
 drivers/tty/amiserial.c                  | 6 +++++-
 drivers/tty/moxa.c                       | 1 +
 drivers/tty/mxser.c                      | 1 +
 drivers/tty/serial/serial_core.c         | 2 ++
 drivers/tty/tty_ioctl.c                  | 2 ++
 drivers/usb/serial/usb-serial.c          | 5 +++--
 include/uapi/asm-generic/termbits.h      | 1 +
 12 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index 4575ba34a0ea..285169c794ec 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -180,6 +180,7 @@ struct ktermios {
 #define HUPCL	00040000
 
 #define CLOCAL	00100000
+#define ADDRB	010000000		/* address bit */
 #define CMSPAR	  010000000000		/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000		/* flow control */
 
diff --git a/arch/mips/include/uapi/asm/termbits.h b/arch/mips/include/uapi/asm/termbits.h
index dfeffba729b7..e7ea31cfec78 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -181,6 +181,7 @@ struct ktermios {
 #define	 B3000000 0010015
 #define	 B3500000 0010016
 #define	 B4000000 0010017
+#define ADDRB	  0020000	/* address bit */
 #define CIBAUD	  002003600000	/* input baud rate */
 #define CMSPAR	  010000000000	/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000	/* flow control */
diff --git a/arch/parisc/include/uapi/asm/termbits.h b/arch/parisc/include/uapi/asm/termbits.h
index 40e920f8d683..629be061f5d5 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -158,6 +158,7 @@ struct ktermios {
 #define  B3000000 0010015
 #define  B3500000 0010016
 #define  B4000000 0010017
+#define ADDRB	  0020000		/* address bit */
 #define CIBAUD    002003600000		/* input baud rate */
 #define CMSPAR    010000000000          /* mark or space (stick) parity */
 #define CRTSCTS   020000000000          /* flow control */
diff --git a/arch/powerpc/include/uapi/asm/termbits.h b/arch/powerpc/include/uapi/asm/termbits.h
index ed18bc61f63d..1b778ac562a4 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -171,6 +171,7 @@ struct ktermios {
 #define HUPCL	00040000
 
 #define CLOCAL	00100000
+#define ADDRB	00200000		/* address bit */
 #define CMSPAR	  010000000000		/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000		/* flow control */
 
diff --git a/arch/sparc/include/uapi/asm/termbits.h b/arch/sparc/include/uapi/asm/termbits.h
index ce5ad5d0f105..4ad60c4acf65 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -200,6 +200,7 @@ struct ktermios {
 #define B3000000  0x00001011
 #define B3500000  0x00001012
 #define B4000000  0x00001013  */
+#define ADDRB	  0x00002000  /* address bit */
 #define CIBAUD	  0x100f0000  /* input baud rate (not used) */
 #define CMSPAR	  0x40000000  /* mark or space (stick) parity */
 #define CRTSCTS	  0x80000000  /* flow control */
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 533d02b38e02..3ca97007bd6e 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -1175,7 +1175,11 @@ static void rs_set_termios(struct tty_struct *tty, struct ktermios *old_termios)
 {
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
-	unsigned int cflag = tty->termios.c_cflag;
+	unsigned int cflag;
+
+	tty->termios.c_cflag &= ~ADDRB;
+
+	cflag = tty->termios.c_cflag;
 
 	change_speed(tty, info, old_termios);
 
diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index f3c72ab1476c..07cd88152d58 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -2050,6 +2050,7 @@ static int MoxaPortSetTermio(struct moxa_port *port, struct ktermios *termio,
 
 	ofsAddr = port->tableAddr;
 
+	termio->c_cflag &= ~ADDRB;
 	mode = termio->c_cflag & CSIZE;
 	if (mode == CS5)
 		mode = MX_CS5;
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 836c9eca2946..220676363a07 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -577,6 +577,7 @@ static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_term
 	struct mxser_port *info = tty->driver_data;
 	unsigned cflag, cval;
 
+	tty->termios.c_cflag &= ~ADDRB;
 	cflag = tty->termios.c_cflag;
 
 	if (mxser_set_baud(tty, tty_get_baud_rate(tty))) {
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 846192a7b4bf..8ab88293c917 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1489,6 +1489,8 @@ static void uart_set_termios(struct tty_struct *tty,
 		goto out;
 	}
 
+	tty->termios.c_cflag &= ~ADDRB;
+
 	uart_change_speed(tty, state, old_termios);
 	/* reload cflag from termios; port driver may have overridden flags */
 	cflag = tty->termios.c_cflag;
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 63181925ec1a..934037d78868 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -319,6 +319,8 @@ unsigned char tty_get_frame_size(unsigned int cflag)
 		bits++;
 	if (cflag & PARENB)
 		bits++;
+	if (cflag & ADDRB)
+		bits++;
 
 	return bits;
 }
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 24101bd7fcad..44b73aea80bb 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -525,9 +525,10 @@ static void serial_set_termios(struct tty_struct *tty, struct ktermios *old)
 
 	dev_dbg(&port->dev, "%s\n", __func__);
 
-	if (port->serial->type->set_termios)
+	if (port->serial->type->set_termios) {
+		tty->termios.c_cflag &= ~ADDRB;
 		port->serial->type->set_termios(tty, port, old);
-	else
+	} else
 		tty_termios_copy_hw(&tty->termios, old);
 }
 
diff --git a/include/uapi/asm-generic/termbits.h b/include/uapi/asm-generic/termbits.h
index 2fbaf9ae89dd..5f5228329d45 100644
--- a/include/uapi/asm-generic/termbits.h
+++ b/include/uapi/asm-generic/termbits.h
@@ -157,6 +157,7 @@ struct ktermios {
 #define  B3000000 0010015
 #define  B3500000 0010016
 #define  B4000000 0010017
+#define ADDRB     0020000	/* address bit */
 #define CIBAUD	  002003600000	/* input baud rate */
 #define CMSPAR	  010000000000	/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000	/* flow control */
-- 
2.30.2

[RFC PATCH 6/7] serial: General support for multipoint addresses

[Ilpo Järvinen]

This patch adds generic support for serial multipoint
addressing. Two new ioctls are added. TIOCSADDR is used to
indicate the destination/receive address. TIOCGADDR returns
the current address in use. The driver should implement
set_addr and get_addr to support addressing mode.

Adjust ADDRB clearing to happen only if driver does not provide
set_addr (=the driver doesn't support address mode).

This change is necessary for supporting devices with RS485
multipoint addressing [*]. A following patch in the patch series
adds support for Synopsys Designware UART capable for 9th bit
addressing mode. In this mode, 9th bit is used to indicate an
address (byte) within the communication line. The 9th bit
addressing mode is selected using ADDRB introduced by the
previous patch.

Transmit addresses / receiver filter are specified by setting
the flags SER_ADDR_DEST and/or SER_ADDR_RECV. When the user
supplies the transmit address, in the 9bit addressing mode it is
sent out immediately with the 9th bit set to 1. After that, the
subsequent normal data bytes are sent with 9th bit as 0 and they
are intended to the device with the given address. It is up to
receiver to enforce the filter using SER_ADDR_RECV. When userspace
has supplied the receive address, the driver is expected to handle
the matching of the address and only data with that address is
forwarded to the user. Both SER_ADDR_DEST and SER_ADDR_RECV can
be given at the same time in a single call if the addresses are
the same.

The user can clear the receive filter with SER_ADDR_RECV_CLEAR.

[*] Technically, RS485 is just an electronic spec and does not
itself specify the 9th bit addressing mode but 9th bit seems
at least "semi-standard" way to do addressing with RS485.

Cc: linux-api@vger.kernel.org
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-xtensa@linux-xtensa.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../driver-api/serial/serial-rs485.rst        | 23 ++++++-
 arch/alpha/include/uapi/asm/ioctls.h          |  3 +
 arch/mips/include/uapi/asm/ioctls.h           |  3 +
 arch/parisc/include/uapi/asm/ioctls.h         |  3 +
 arch/powerpc/include/uapi/asm/ioctls.h        |  3 +
 arch/sh/include/uapi/asm/ioctls.h             |  3 +
 arch/sparc/include/uapi/asm/ioctls.h          |  3 +
 arch/xtensa/include/uapi/asm/ioctls.h         |  3 +
 drivers/tty/serial/8250/8250_core.c           |  2 +
 drivers/tty/serial/serial_core.c              | 62 ++++++++++++++++++-
 include/linux/serial_core.h                   |  6 ++
 include/uapi/asm-generic/ioctls.h             |  3 +
 include/uapi/linux/serial.h                   |  8 +++
 13 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/serial/serial-rs485.rst b/Documentation/driver-api/serial/serial-rs485.rst
index 6bc824f948f9..2f45f007fa5b 100644
--- a/Documentation/driver-api/serial/serial-rs485.rst
+++ b/Documentation/driver-api/serial/serial-rs485.rst
@@ -95,7 +95,28 @@ RS485 Serial Communications
 		/* Error handling. See errno. */
 	}
 
-5. References
+5. Multipoint Addressing
+========================
+
+   The Linux kernel provides serial_addr structure to handle addressing within
+   multipoint serial communications line such as RS485. 9th bit addressiong mode
+   is enabled by adding ADDRB flag in termios c_cflag.
+
+   Serial core calls device specific set/get_addr in response to TIOCSADDR and
+   TIOCGADDR ioctls with a pointer to serial_addr. Destination and receive
+   address can be specified using serial_addr flags field. Receive address may
+   also be cleared using flags. Once an address is set, the communication
+   can occur only with the particular device and other peers are filtered out.
+   It is left up to the receiver side to enforce the filtering.
+
+   Address flags:
+	- SER_ADDR_RECV: Receive (filter) address.
+	- SER_ADDR_RECV_CLEAR: Clear receive filter (only for TIOCSADDR).
+	- SER_ADDR_DEST: Destination address.
+
+   Note: not all devices supporting RS485 support multipoint addressing.
+
+6. References
 =============
 
  [1]	include/uapi/linux/serial.h
diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 971311605288..500cab3e1d6b 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -125,4 +125,7 @@
 #define TIOCMIWAIT	0x545C	/* wait for a change on serial input line(s) */
 #define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 #endif /* _ASM_ALPHA_IOCTLS_H */
diff --git a/arch/mips/include/uapi/asm/ioctls.h b/arch/mips/include/uapi/asm/ioctls.h
index 16aa8a766aec..3859dc46857e 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -96,6 +96,9 @@
 #define TIOCGISO7816	_IOR('T', 0x42, struct serial_iso7816)
 #define TIOCSISO7816	_IOWR('T', 0x43, struct serial_iso7816)
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 /* I hope the range from 0x5480 on is free ... */
 #define TIOCSCTTY	0x5480		/* become controlling tty */
 #define TIOCGSOFTCAR	0x5481
diff --git a/arch/parisc/include/uapi/asm/ioctls.h b/arch/parisc/include/uapi/asm/ioctls.h
index 82d1148c6379..62337743db64 100644
--- a/arch/parisc/include/uapi/asm/ioctls.h
+++ b/arch/parisc/include/uapi/asm/ioctls.h
@@ -86,6 +86,9 @@
 #define TIOCSTOP	0x5462
 #define TIOCSLTC	0x5462
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 /* Used for packet mode */
 #define TIOCPKT_DATA		 0
 #define TIOCPKT_FLUSHREAD	 1
diff --git a/arch/powerpc/include/uapi/asm/ioctls.h b/arch/powerpc/include/uapi/asm/ioctls.h
index 2c145da3b774..84fd69ac366a 100644
--- a/arch/powerpc/include/uapi/asm/ioctls.h
+++ b/arch/powerpc/include/uapi/asm/ioctls.h
@@ -120,4 +120,7 @@
 #define TIOCMIWAIT	0x545C	/* wait for a change on serial input line(s) */
 #define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 #endif	/* _ASM_POWERPC_IOCTLS_H */
diff --git a/arch/sh/include/uapi/asm/ioctls.h b/arch/sh/include/uapi/asm/ioctls.h
index 11866d4f60e1..f82966b7dba2 100644
--- a/arch/sh/include/uapi/asm/ioctls.h
+++ b/arch/sh/include/uapi/asm/ioctls.h
@@ -113,4 +113,7 @@
 #define TIOCMIWAIT	_IO('T', 92) /* 0x545C */	/* wait for a change on serial input line(s) */
 #define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 #endif /* __ASM_SH_IOCTLS_H */
diff --git a/arch/sparc/include/uapi/asm/ioctls.h b/arch/sparc/include/uapi/asm/ioctls.h
index 7fd2f5873c9e..e44624c67c79 100644
--- a/arch/sparc/include/uapi/asm/ioctls.h
+++ b/arch/sparc/include/uapi/asm/ioctls.h
@@ -125,6 +125,9 @@
 #define TIOCMIWAIT	0x545C /* Wait for change on serial input line(s) */
 #define TIOCGICOUNT	0x545D /* Read serial port inline interrupt counts */
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 /* Kernel definitions */
 
 /* Used for packet mode */
diff --git a/arch/xtensa/include/uapi/asm/ioctls.h b/arch/xtensa/include/uapi/asm/ioctls.h
index 6d4a87296c95..759ca9377f2a 100644
--- a/arch/xtensa/include/uapi/asm/ioctls.h
+++ b/arch/xtensa/include/uapi/asm/ioctls.h
@@ -127,4 +127,7 @@
 #define TIOCMIWAIT	_IO('T', 92) /* wait for a change on serial input line(s) */
 #define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 #endif /* _XTENSA_IOCTLS_H */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 01d30f6ed8fb..f67bc3b76f65 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1008,6 +1008,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 		uart->port.rs485	= up->port.rs485;
 		uart->rs485_start_tx	= up->rs485_start_tx;
 		uart->rs485_stop_tx	= up->rs485_stop_tx;
+		uart->port.set_addr	= up->port.set_addr;
+		uart->port.get_addr	= up->port.get_addr;
 		uart->dma		= up->dma;
 
 		/* Take tx_loadsz from fifosize if it wasn't set separately */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 8ab88293c917..8235f696d073 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1346,6 +1346,56 @@ static int uart_set_iso7816_config(struct uart_port *port,
 	return 0;
 }
 
+static int uart_set_addr(struct uart_port *port,
+			 struct serial_addr __user *serial_addr_user)
+{
+	struct serial_addr addr;
+	unsigned long flags;
+	int ret;
+
+	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+		return -EFAULT;
+
+	spin_lock_irqsave(&port->lock, flags);
+	if (port->set_addr)
+		ret = port->set_addr(port, &addr);
+	else
+		ret = -EINVAL;
+	spin_unlock_irqrestore(&port->lock, flags);
+	if (ret)
+		return ret;
+
+	if (copy_to_user(serial_addr_user, &addr, sizeof(addr)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int uart_get_addr(struct uart_port *port,
+			 struct serial_addr __user *serial_addr_user)
+{
+	struct serial_addr addr;
+	unsigned long flags;
+	int ret;
+
+	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+		return -EFAULT;
+
+	spin_lock_irqsave(&port->lock, flags);
+	if (port->get_addr)
+		ret = port->get_addr(port, &addr);
+	else
+		ret = -EINVAL;
+	spin_unlock_irqrestore(&port->lock, flags);
+	if (ret)
+		return ret;
+
+	if (copy_to_user(serial_addr_user, &addr, sizeof(addr)))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  * Called via sys_ioctl.  We can use spin_lock_irq() here.
  */
@@ -1423,6 +1473,15 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 	case TIOCGISO7816:
 		ret = uart_get_iso7816_config(state->uart_port, uarg);
 		break;
+
+	case TIOCSADDR:
+		ret = uart_set_addr(uport, uarg);
+		break;
+
+	case TIOCGADDR:
+		ret = uart_get_addr(uport, uarg);
+		break;
+
 	default:
 		if (uport->ops->ioctl)
 			ret = uport->ops->ioctl(uport, cmd, arg);
@@ -1489,7 +1548,8 @@ static void uart_set_termios(struct tty_struct *tty,
 		goto out;
 	}
 
-	tty->termios.c_cflag &= ~ADDRB;
+	if (!uport->set_addr)
+		tty->termios.c_cflag &= ~ADDRB;
 
 	uart_change_speed(tty, state, old_termios);
 	/* reload cflag from termios; port driver may have overridden flags */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 31f7fe527395..ebe1d0ec11d8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -135,6 +135,12 @@ struct uart_port {
 						struct serial_rs485 *rs485);
 	int			(*iso7816_config)(struct uart_port *,
 						  struct serial_iso7816 *iso7816);
+
+	int			(*set_addr)(struct uart_port *p,
+					    struct serial_addr *addr);
+	int			(*get_addr)(struct uart_port *p,
+					    struct serial_addr *addr);
+
 	unsigned int		irq;			/* irq number */
 	unsigned long		irqflags;		/* irq flags  */
 	unsigned int		uartclk;		/* base uart clock */
diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
index cdc9f4ca8c27..689743366091 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -106,6 +106,9 @@
 # define FIOQSIZE	0x5460
 #endif
 
+#define TIOCSADDR	_IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR	_IOWR('T', 0x64, struct serial_addr)
+
 /* Used for packet mode */
 #define TIOCPKT_DATA		 0
 #define TIOCPKT_FLUSHREAD	 1
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index f868685b35a0..7b198fd5d390 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -151,4 +151,12 @@ struct serial_iso7816 {
 	__u32	reserved[5];
 };
 
+struct serial_addr {
+	__u32	flags;
+#define SER_ADDR_RECV			(1 << 0)
+#define SER_ADDR_RECV_CLEAR		(1 << 1)
+#define SER_ADDR_DEST			(1 << 2)
+	__u32	addr;
+};
+
 #endif /* _UAPI_LINUX_SERIAL_H */
-- 
2.30.2

Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

[Lukas Wunner]

On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo Järvinen wrote:
> This patch adds generic support for serial multipoint
> addressing. Two new ioctls are added. TIOCSADDR is used to

Nit:  "This patch adds..." is superfluous.  Just write "Add ..."
in imperative mood.


> This change is necessary for supporting devices with RS485
> multipoint addressing [*].

If this is only used with RS485, why can't we just store the
addresses in struct serial_rs485 and use the existing TIOCSRS485
and TIOCGRS485 ioctls?  There's 20 bytes of padding left in
struct serial_rs485 which you could use.  No need to add more
user-space ABI.


> [*] Technically, RS485 is just an electronic spec and does not
> itself specify the 9th bit addressing mode but 9th bit seems
> at least "semi-standard" way to do addressing with RS485.

Is 9th bit addressing actually used by an Intel customer or was
it implemented just for feature completeness?  I think this mode
isn't used often (I've never seen a use case myself), primarily
because it requires disabling parity.

Thanks,

Lukas

Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On Sun, 6 Mar 2022, Lukas Wunner wrote:

> On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo Järvinen wrote:
> 
> > This change is necessary for supporting devices with RS485
> > multipoint addressing [*].
> 
> If this is only used with RS485, why can't we just store the
> addresses in struct serial_rs485 and use the existing TIOCSRS485
> and TIOCGRS485 ioctls?  There's 20 bytes of padding left in
> struct serial_rs485 which you could use.  No need to add more
> user-space ABI.

It could if it is agreed that serial multipoint addressing is just
a thing in RS-485 and nowhere else? In that case, there is no point
in adding more generic support for it.

> > [*] Technically, RS485 is just an electronic spec and does not
> > itself specify the 9th bit addressing mode but 9th bit seems
> > at least "semi-standard" way to do addressing with RS485.
> 
> Is 9th bit addressing actually used by an Intel customer or was
> it implemented just for feature completeness? I think this mode
> isn't used often (I've never seen a use case myself), primarily
> because it requires disabling parity.

On what basis? ...The datasheet I'm looking at has a timing diagram 
with both D8 (9th bit) and parity so I think your information must be
incorrect. I don't have direct contacts with customers but I'm told
it's important for other org's customers.


-- 
 i.

Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

[Lukas Wunner]

On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo Järvinen wrote:
> On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo Järvinen wrote:
> > > This change is necessary for supporting devices with RS485
> > > multipoint addressing [*].
> > 
> > If this is only used with RS485, why can't we just store the
> > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > and TIOCGRS485 ioctls?  There's 20 bytes of padding left in
> > struct serial_rs485 which you could use.  No need to add more
> > user-space ABI.
> 
> It could if it is agreed that serial multipoint addressing is just
> a thing in RS-485 and nowhere else? In that case, there is no point
> in adding more generic support for it.

It's just that the above-quoted sentence in the commit message
specifically mentions RS485.  If you intend to use it with RS232
as well, that should be made explicit, otherwise one wonders why
it wasn't integrated into struct serial_rs485.

I have no idea how common 9th bit addressing mode is with RS232.
Goggle turns up links saying it's mainly used with RS485, "but also
RS232".  Since RS232 isn't a bus but a point-to-point link,
9th bit addressing doesn't seem to make as much sense.


> > > [*] Technically, RS485 is just an electronic spec and does not
> > > itself specify the 9th bit addressing mode but 9th bit seems
> > > at least "semi-standard" way to do addressing with RS485.
> > 
> > Is 9th bit addressing actually used by an Intel customer or was
> > it implemented just for feature completeness? I think this mode
> > isn't used often (I've never seen a use case myself), primarily
> > because it requires disabling parity.
> 
> On what basis? ...The datasheet I'm looking at has a timing diagram 
> with both D8 (9th bit) and parity so I think your information must be
> incorrect.

E.g. the discussion here says that 9th bit addressing requires that
parity is disabled or the character size is reduced to 7-bit:

https://www.microchip.com/forums/m299904.aspx

I guess that applies only to some UARTs, the Synopsys databook doesn't
mention any such constraints.

Thanks,

Lukas

Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

On Wed, 9 Mar 2022, Lukas Wunner wrote:

> On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo Järvinen wrote:
> > On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo Järvinen wrote:
> > > > This change is necessary for supporting devices with RS485
> > > > multipoint addressing [*].
> > > 
> > > If this is only used with RS485, why can't we just store the
> > > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > > and TIOCGRS485 ioctls?  There's 20 bytes of padding left in
> > > struct serial_rs485 which you could use.  No need to add more
> > > user-space ABI.
> > 
> > It could if it is agreed that serial multipoint addressing is just
> > a thing in RS-485 and nowhere else? In that case, there is no point
> > in adding more generic support for it.
> 
> It's just that the above-quoted sentence in the commit message
> specifically mentions RS485.

That sentence is just to justify why addressing mode is needed,
not to take a stance on whether it is only used with RS485 or not.

> If you intend to use it with RS232
> as well, that should be made explicit, otherwise one wonders why
> it wasn't integrated into struct serial_rs485.
> 
> I have no idea how common 9th bit addressing mode is with RS232.
> Goggle turns up links saying it's mainly used with RS485, "but also
> RS232".  Since RS232 isn't a bus but a point-to-point link,
> 9th bit addressing doesn't seem to make as much sense.

While I don't know any better, I can image though that with an 
RS232-to-RS485 converter, it could make some sense.

If I put them back to serial_rs485 / rs485 config, it's basically just 
where I initially started from with this patchset (offlist).


-- 
 i.

Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

[Andy Shevchenko]

On Wed, Mar 09, 2022 at 08:05:21PM +0100, Lukas Wunner wrote:
> On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo Järvinen wrote:
> > On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo Järvinen wrote:
> > > > This change is necessary for supporting devices with RS485
> > > > multipoint addressing [*].
> > > 
> > > If this is only used with RS485, why can't we just store the
> > > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > > and TIOCGRS485 ioctls?  There's 20 bytes of padding left in
> > > struct serial_rs485 which you could use.  No need to add more
> > > user-space ABI.
> > 
> > It could if it is agreed that serial multipoint addressing is just
> > a thing in RS-485 and nowhere else? In that case, there is no point
> > in adding more generic support for it.
> 
> It's just that the above-quoted sentence in the commit message
> specifically mentions RS485.  If you intend to use it with RS232
> as well, that should be made explicit, otherwise one wonders why
> it wasn't integrated into struct serial_rs485.
> 
> I have no idea how common 9th bit addressing mode is with RS232.
> Goggle turns up links saying it's mainly used with RS485, "but also
> RS232".  Since RS232 isn't a bus but a point-to-point link,
> 9th bit addressing doesn't seem to make as much sense.

In my student years I have an exercise to use 9-bit addressing mode on RS232.
Obviously I forgot all of the details, but I remember that that has a practical
application.

-- 
With Best Regards,
Andy Shevchenko

[RFC PATCH 7/7] serial: 8250_dwlib: Support for 9th bit multipoint addressing

[Ilpo Järvinen]

This change adds 9th bit multipoint addressing mode for DW UART using
the new ioctls introduced in the previous change. 9th bit addressing
can be used only when HW RS485 is available.

Updating RAR (receive address register) is bit tricky because
busy indication is not be available when DW UART is strictly
16550 compatible, which is the case with the hardware I was
testing with. RAR should not be updated while receive is in
progress which is now achieved by deasserting RE and waiting
for one frame (in case rx would be in progress, the driver
seems to have no way of knowing it w/o busy indication).

Co-developed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Co-developed-by: Raymond Tan <raymond.tan@intel.com>
Signed-off-by: Raymond Tan <raymond.tan@intel.com>
Co-developed-by: Lakshmi Sowjanya <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 124 +++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_dwlib.h |   3 +
 2 files changed, 127 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 51b0f55ee9d0..57bf6ca0309b 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -3,9 +3,11 @@
 
 #include <linux/bitops.h>
 #include <linux/bitfield.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/math.h>
 #include <linux/property.h>
 #include <linux/serial_8250.h>
 #include <linux/serial_core.h>
@@ -17,6 +19,9 @@
 #define DW_UART_DE_EN	0xb0 /* Driver Output Enable Register */
 #define DW_UART_RE_EN	0xb4 /* Receiver Output Enable Register */
 #define DW_UART_DLF	0xc0 /* Divisor Latch Fraction Register */
+#define DW_UART_RAR	0xc4 /* Receive Address Register */
+#define DW_UART_TAR	0xc8 /* Transmit Address Register */
+#define DW_UART_LCR_EXT	0xcc /* Line Extended Control Register */
 #define DW_UART_CPR	0xf4 /* Component Parameter Register */
 #define DW_UART_UCV	0xf8 /* UART Component Version */
 
@@ -29,6 +34,12 @@
 #define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
 #define DW_UART_TCR_XFER_MODE_DE_OR_RE		FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
 
+/* Line Extended Control Register bits */
+#define DW_UART_LCR_EXT_DLS_E		BIT(0)
+#define DW_UART_LCR_EXT_ADDR_MATCH	BIT(1)
+#define DW_UART_LCR_EXT_SEND_ADDR	BIT(2)
+#define DW_UART_LCR_EXT_TRANSMIT_MODE	BIT(3)
+
 /* Component Parameter Register bits */
 #define DW_UART_CPR_ABP_DATA_WIDTH	(3 << 0)
 #define DW_UART_CPR_AFCE_MODE		(1 << 4)
@@ -91,16 +102,126 @@ static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
 	serial8250_do_set_divisor(p, baud, quot, quot_frac);
 }
 
+/* Wait until re is de-asserted for sure. Without BUSY indication available,
+ * only available course of action is to wait until current frame is received.
+ */
+static void dw8250_wait_re_deassert(struct uart_port *p)
+{
+	struct dw8250_port_data *d = p->private_data;
+
+	if (d->rar_timeout_us)
+		udelay(d->rar_timeout_us);
+}
+
+static void dw8250_update_rar(struct uart_port *p, u32 addr)
+{
+	u32 re_en = dw8250_readl_ext(p, DW_UART_RE_EN);
+
+	/* RAR shouldn't be changed while receiving. Thus, de-assert RE_EN
+	 * if asserted and wait.
+	 */
+	if (re_en)
+		dw8250_writel_ext(p, DW_UART_RE_EN, 0);
+	dw8250_wait_re_deassert(p);
+	dw8250_writel_ext(p, DW_UART_RAR, addr);
+	if (re_en)
+		dw8250_writel_ext(p, DW_UART_RE_EN, re_en);
+}
+
+static void dw8250_addrmode_setup(struct uart_port *p, bool enable_addrmode)
+{
+	struct dw8250_port_data *d = p->private_data;
+
+	if (enable_addrmode) {
+		/* Clear RAR & TAR of any previous values */
+		dw8250_writel_ext(p, DW_UART_RAR, 0);
+		dw8250_writel_ext(p, DW_UART_TAR, 0);
+		dw8250_writel_ext(p, DW_UART_LCR_EXT, DW_UART_LCR_EXT_DLS_E);
+	} else {
+		dw8250_writel_ext(p, DW_UART_LCR_EXT, 0);
+	}
+
+	d->addrmode = enable_addrmode;
+}
+
 void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old)
 {
+	struct dw8250_port_data *d = p->private_data;
+
 	p->status &= ~UPSTAT_AUTOCTS;
 	if (termios->c_cflag & CRTSCTS)
 		p->status |= UPSTAT_AUTOCTS;
 
+	if (!(p->rs485.flags & SER_RS485_ENABLED) || !d->hw_rs485_support)
+		termios->c_cflag &= ~ADDRB;
+
+	if (!old || (termios->c_cflag ^ old->c_cflag) & ADDRB)
+		dw8250_addrmode_setup(p, termios->c_cflag & ADDRB);
+
 	serial8250_do_set_termios(p, termios, old);
+
+	d->rar_timeout_us = termios->c_ispeed ?
+			    DIV_ROUND_UP(tty_get_frame_size(termios->c_cflag) * USEC_PER_SEC,
+					 termios->c_ispeed) : 0;
 }
 EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
 
+static int dw8250_rs485_set_addr(struct uart_port *p, struct serial_addr *addr)
+{
+	struct dw8250_port_data *d = p->private_data;
+	u32 lcr;
+
+	if (!(p->rs485.flags & SER_RS485_ENABLED) || !d->addrmode)
+		return -EINVAL;
+
+	addr->flags &= SER_ADDR_RECV | SER_ADDR_RECV_CLEAR | SER_ADDR_DEST;
+	if (!addr->flags)
+		return -EINVAL;
+
+	lcr = dw8250_readl_ext(p, DW_UART_LCR_EXT);
+	if (addr->flags & SER_ADDR_RECV) {
+		dw8250_update_rar(p, addr->addr & 0xff);
+		lcr |= DW_UART_LCR_EXT_ADDR_MATCH;
+		addr->flags &= ~SER_ADDR_RECV_CLEAR;
+	} else if (addr->flags & SER_ADDR_RECV_CLEAR) {
+		lcr &= DW_UART_LCR_EXT_ADDR_MATCH;
+	}
+	if (addr->flags & SER_ADDR_DEST) {
+		dw8250_writel_ext(p, DW_UART_TAR, addr->addr & 0xff);
+		lcr |= DW_UART_LCR_EXT_SEND_ADDR;
+	}
+	dw8250_writel_ext(p, DW_UART_LCR_EXT, lcr);
+
+	return 0;
+}
+
+static int dw8250_rs485_get_addr(struct uart_port *p, struct serial_addr *addr)
+{
+	struct dw8250_port_data *d = p->private_data;
+
+	if (!(p->rs485.flags & SER_RS485_ENABLED) || !d->addrmode)
+		return -EINVAL;
+
+	if (addr->flags == SER_ADDR_DEST) {
+		addr->addr = dw8250_readl_ext(p, DW_UART_TAR) & 0xff;
+		return 0;
+	}
+	if (addr->flags == SER_ADDR_RECV) {
+		u32 lcr = dw8250_readl_ext(p, DW_UART_LCR_EXT);
+
+		if (!(lcr & DW_UART_LCR_EXT_ADDR_MATCH)) {
+			addr->flags = SER_ADDR_RECV_CLEAR;
+			addr->addr = 0;
+		} else {
+			addr->addr = dw8250_readl_ext(p, DW_UART_RAR) & 0xff;
+		}
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static void dw8250_rs485_start_tx(struct uart_8250_port *up)
 {
 	struct uart_port *p = &(up->port);
@@ -157,6 +278,7 @@ static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
 		tcr &= ~DW_UART_TCR_RS485_EN;
 		dw8250_writel_ext(p, DW_UART_DE_EN, 0);
 		dw8250_writel_ext(p, DW_UART_RE_EN, 0);
+		dw8250_addrmode_setup(p, false);
 	}
 
 	/* Resetting the default DE_POL & RE_POL */
@@ -203,6 +325,8 @@ void dw8250_setup_port(struct uart_port *p)
 		p->rs485_config = dw8250_rs485_config;
 		up->rs485_start_tx = dw8250_rs485_start_tx;
 		up->rs485_stop_tx = dw8250_rs485_stop_tx;
+		p->set_addr = dw8250_rs485_set_addr;
+		p->get_addr = dw8250_rs485_get_addr;
 	} else {
 		p->rs485_config = serial8250_em485_config;
 		up->rs485_start_tx = serial8250_em485_start_tx;
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index a8fa020ca544..bfb1d8af7f0c 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -17,6 +17,9 @@ struct dw8250_port_data {
 
 	/* RS485 variables */
 	bool			hw_rs485_support;
+	/* 9-bit framing (9th bit is address indicator) */
+	bool			addrmode;
+	unsigned int		rar_timeout_us;
 };
 
 void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old);
-- 
2.30.2