Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH 2/4] serial: 8250: Copy mctrl when register port.
From: Giulio Benetti @ 2018-06-06  9:49 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, Matthias Brugger, Kees Cook, Allen Pais,
	Sean Young, open list:SERIAL DRIVERS, open list
In-Reply-To: <CAJs94EZqdQAQG+inWvj89FCQtv06LxhY_J4YgNmK8mtvgDTchQ@mail.gmail.com>

RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is on
TIOCM_RTS is set, then need to keep it set when registering port.

Copy mctrl to new port too.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index c8c2b260c681..c8e62fbd6570 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->port.unthrottle	= up->port.unthrottle;
 		uart->port.rs485_config	= up->port.rs485_config;
 		uart->port.rs485	= up->port.rs485;
+		uart->port.mctrl	= up->port.mctrl;
 		uart->dma		= up->dma;
 		uart->em485		= up->em485;
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH 3/4] serial: 8250: Make em485_rts_after_send() set mctrl according to rts state.
From: Giulio Benetti @ 2018-06-06  9:49 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, Matthias Brugger, Kees Cook, Sean Young,
	Aaron Sierra, Tomas Melin, Sean Wang, Rafael Gago, Joel Stanley,
	open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606094942.71190-1-giulio.benetti@micronovasrl.com>

When rs485 enabled and RTS_AFTER_SEND set on startup, need to preserve
mctrl status, because later functions will call set_mctrl passing
port->mctrl=0 overriding rts status, resulting in rts pin in
transmission when idle.

Make mctrl reflect rts pin state.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_port.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 95833cbc4338..c8c10b5ec6d6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -562,10 +562,13 @@ static inline void serial8250_em485_rts_after_send(struct uart_8250_port *p)
 {
 	unsigned char mcr = serial8250_in_MCR(p);
 
-	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND)
+	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
 		mcr |= UART_MCR_RTS;
-	else
+		p->port.mctrl |= TIOCM_RTS;
+	} else {
 		mcr &= ~UART_MCR_RTS;
+		p->port.mctrl &= ~TIOCM_RTS;
+	}
 	serial8250_out_MCR(p, mcr);
 }
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH 4/4] serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and RTS_AFTER_SEND set.
From: Giulio Benetti @ 2018-06-06  9:49 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, Matthias Brugger, Kees Cook, Sean Young,
	open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606094942.71190-1-giulio.benetti@micronovasrl.com>

If rs485 is enabled and RTS_AFTER_SEND is set on startup need to keep
TIOCM_RTS asserted to keep rs485 transceiver in RX when idle.

Check if rs485 is on and RTS_AFTER_SEND is set and mask port->mctrl with
TIOCM_RTS too and not only TIOCM_DTR.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/serial_core.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0466f9f08a91..06d9441f6d20 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2288,6 +2288,16 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 
 	if (port->type != PORT_UNKNOWN) {
 		unsigned long flags;
+		int rs485_on = port->rs485_config &&
+			(port->rs485.flags & SER_RS485_ENABLED);
+		int RTS_after_send = !!(port->rs485.flags &
+				SER_RS485_RTS_AFTER_SEND);
+		int mctrl;
+
+		if (rs485_on && RTS_after_send)
+			mctrl = port->mctrl & (TIOCM_DTR | TIOCM_RTS);
+		else
+			mctrl = port->mctrl & TIOCM_DTR;
 
 		uart_report_port(drv, port);
 
@@ -2300,7 +2310,7 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 		 * We probably don't need a spinlock around this, but
 		 */
 		spin_lock_irqsave(&port->lock, flags);
-		port->ops->set_mctrl(port, port->mctrl & TIOCM_DTR);
+		port->ops->set_mctrl(port, mctrl);
 		spin_unlock_irqrestore(&port->lock, flags);
 
 		/*
-- 
2.17.1

^ permalink raw reply related

* [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-06  9:49 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, Matthias Brugger, Kees Cook, Allen Pais,
	Sean Young, open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606094942.71190-1-giulio.benetti@micronovasrl.com>

em485 gets lost during serial8250_register_8250_port().

Copy em485 to final uart port.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..c8c2b260c681 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -994,6 +994,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->port.rs485_config	= up->port.rs485_config;
 		uart->port.rs485	= up->port.rs485;
 		uart->dma		= up->dma;
+		uart->em485		= up->em485;
 
 		/* Take tx_loadsz from fifosize if it wasn't set separately */
 		if (uart->port.fifosize && !uart->tx_loadsz)
-- 
2.17.1

^ permalink raw reply related

* [PATCH 2/4] serial: 8250_dw: allow enable rs485 at boot time
From: Giulio Benetti @ 2018-06-06  9:51 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Andy Shevchenko,
	Greg Kroah-Hartman, Jiri Slaby, Joshua Scott, Stefan Potyra,
	Ed Blake, open list:SERIAL DRIVERS, open list
In-Reply-To: <CAJs94EZqdQAQG+inWvj89FCQtv06LxhY_J4YgNmK8mtvgDTchQ@mail.gmail.com>

If "linux,rs485-enabled-at-boot-time" is specified need to setup 485
in probe function.

Call uart_get_rs485_mode() to get rs485 configuration, then call
rs485_config() callback directly to setup port as rs485.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_dw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 45366e6e5411..0f8b4da03d4e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -582,8 +582,11 @@ static int dw8250_probe(struct platform_device *pdev)
 	if (data->uart_16550_compatible)
 		p->handle_irq = NULL;
 
-	if (!data->skip_autocfg)
+	if (!data->skip_autocfg) {
 		dw8250_setup_port(p);
+		uart_get_rs485_mode(dev, &p->rs485);
+		dw8250_rs485_config(p, &p->rs485);
+	}
 
 	/* If we have a valid fifosize, try hooking up DMA */
 	if (p->fifosize) {
-- 
2.17.1

^ permalink raw reply related

* [PATCH 3/4] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Giulio Benetti @ 2018-06-06  9:51 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, Stefan Potyra, Philipp Zabel, Ed Blake,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Phil Elwell, Rafael Gago, Joel Stanley, Sean Wang,
	open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606095156.72628-1-giulio.benetti@micronovasrl.com>

Some 8250 ports only have TEMT interrupt, so current implementation
can't work for ports without it. The only chance to make it work is to
loop-read on LSR register.

With NO TEMT interrupt check if both TEMT and THRE are set looping on
LSR register.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250.h      |  2 +-
 drivers/tty/serial/8250/8250_dw.c   |  2 +-
 drivers/tty/serial/8250/8250_omap.c |  2 +-
 drivers/tty/serial/8250/8250_port.c | 26 ++++++++++++++++++--------
 include/linux/serial_8250.h         |  1 +
 5 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd5bef5..5c6ae5f69432 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -136,7 +136,7 @@ void serial8250_rpm_put(struct uart_8250_port *p);
 void serial8250_rpm_get_tx(struct uart_8250_port *p);
 void serial8250_rpm_put_tx(struct uart_8250_port *p);
 
-int serial8250_em485_init(struct uart_8250_port *p);
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr);
 void serial8250_em485_destroy(struct uart_8250_port *p);
 
 static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 0f8b4da03d4e..888280ff5451 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -330,7 +330,7 @@ static int dw8250_rs485_config(struct uart_port *p,
 	 * are idempotent
 	 */
 	if (rs485->flags & SER_RS485_ENABLED) {
-		int ret = serial8250_em485_init(up);
+		int ret = serial8250_em485_init(up, false);
 
 		if (ret) {
 			rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 624b501fd253..ab483c8b30c8 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -725,7 +725,7 @@ static int omap_8250_rs485_config(struct uart_port *port,
 	 * are idempotent
 	 */
 	if (rs485->flags & SER_RS485_ENABLED) {
-		int ret = serial8250_em485_init(up);
+		int ret = serial8250_em485_init(up, true);
 
 		if (ret) {
 			rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index c8c10b5ec6d6..8432445c80e5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -602,15 +602,16 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
 /**
  *	serial8250_em485_init() - put uart_8250_port into rs485 emulating
  *	@p:	uart_8250_port port instance
+ *	@p:	bool specify if 8250 port has TEMT interrupt or not
  *
  *	The function is used to start rs485 software emulating on the
  *	&struct uart_8250_port* @p. Namely, RTS is switched before/after
  *	transmission. The function is idempotent, so it is safe to call it
  *	multiple times.
  *
- *	The caller MUST enable interrupt on empty shift register before
- *	calling serial8250_em485_init(). This interrupt is not a part of
- *	8250 standard, but implementation defined.
+ *	If has_temt_isr is passed as true, the caller MUST enable interrupt
+ *	on empty shift register before calling serial8250_em485_init().
+ *	This interrupt is not a part of	8250 standard, but implementation defined.
  *
  *	The function is supposed to be called from .rs485_config callback
  *	or from any other callback protected with p->port.lock spinlock.
@@ -619,7 +620,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
  *
  *	Return 0 - success, -errno - otherwise
  */
-int serial8250_em485_init(struct uart_8250_port *p)
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr)
 {
 	if (p->em485)
 		return 0;
@@ -636,6 +637,7 @@ int serial8250_em485_init(struct uart_8250_port *p)
 	p->em485->start_tx_timer.function = &serial8250_em485_handle_start_tx;
 	p->em485->port = p;
 	p->em485->active_timer = NULL;
+	p->em485->has_temt_isr = has_temt_isr;
 	serial8250_em485_rts_after_send(p);
 
 	return 0;
@@ -1520,11 +1522,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
 		/*
 		 * To provide required timeing and allow FIFO transfer,
 		 * __stop_tx_rs485() must be called only when both FIFO and
-		 * shift register are empty. It is for device driver to enable
-		 * interrupt on TEMT.
+		 * shift register are empty. If 8250 port supports it,
+		 * it is for device driver to enable interrupt on TEMT.
+		 * Otherwise must loop-read until TEMT and THRE flags are set.
 		 */
-		if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
-			return;
+		if (em485->has_temt_isr) {
+			if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
+				return;
+		} else {
+			while ((lsr & BOTH_EMPTY) != BOTH_EMPTY) {
+				lsr = serial_in(p, UART_LSR);
+				cpu_relax();
+			}
+		}
 
 		em485->active_timer = NULL;
 
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index a27ef5f56431..9b13cf59726b 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -83,6 +83,7 @@ struct uart_8250_em485 {
 	struct hrtimer		start_tx_timer; /* "rs485 start tx" timer */
 	struct hrtimer		stop_tx_timer;  /* "rs485 stop tx" timer */
 	struct hrtimer		*active_timer;  /* pointer to active timer */
+	bool			has_temt_isr;	/* say if 8250 has TEMT interrupt or no */
 	struct uart_8250_port	*port;          /* for hrtimer callbacks */
 };
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH 4/4] serial: 8250_dw: treat rpm suspend with -EBUSY if RS485 ON and RTS_AFTER_SEND
From: Giulio Benetti @ 2018-06-06  9:51 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Andy Shevchenko,
	Greg Kroah-Hartman, Jiri Slaby, Stefan Potyra, Joshua Scott,
	Ed Blake, open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606095156.72628-1-giulio.benetti@micronovasrl.com>

If rs485 is on and RTS_AFTER_SEND it means that in idle state rts pin
must be asserted, othwerwise rs485 transceiver will enter tx state.
dw8250 when clocks are stopped keeps rts line de-asserted(high),
resulting in keeping rs485 transceiver in tx state when port is idle.

Check if rs485 is on with RTS_AFTER_SEND set, if so return -EBUSY in
rpm_suspend,

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_dw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 888280ff5451..6b0ee6dc8ad0 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -668,6 +668,11 @@ static int dw8250_resume(struct device *dev)
 static int dw8250_runtime_suspend(struct device *dev)
 {
 	struct dw8250_data *data = dev_get_drvdata(dev);
+	struct uart_8250_port *up = serial8250_get_port(data->line);
+	struct uart_port *p = &up->port;
+
+	if (p->rs485.flags & (SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND))
+		return -EBUSY;
 
 	if (!IS_ERR(data->clk))
 		clk_disable_unprepare(data->clk);
-- 
2.17.1

^ permalink raw reply related

* [PATCH 1/4] serial: 8250_dw: add em485 support
From: Giulio Benetti @ 2018-06-06  9:51 UTC (permalink / raw)
  Cc: matwey.kornilov, Giulio Benetti, Andy Shevchenko,
	Greg Kroah-Hartman, Jiri Slaby, Ed Blake, Joshua Scott,
	open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606095156.72628-1-giulio.benetti@micronovasrl.com>

Need to use rs485 transceiver so let's use existing em485 485 emulation
layer on top of 8250.

Add rs485_config callback to port.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/tty/serial/8250/8250_dw.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 6fcdb90f616a..45366e6e5411 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -314,6 +314,36 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
 	serial8250_do_set_ldisc(p, termios);
 }
 
+static int dw8250_rs485_config(struct uart_port *p,
+			       struct serial_rs485 *rs485)
+{
+	struct uart_8250_port *up = up_to_u8250p(p);
+
+	/* Clamp the delays to [0, 100ms] */
+	rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
+	rs485->delay_rts_after_send  = min(rs485->delay_rts_after_send, 100U);
+
+	p->rs485 = *rs485;
+
+	/*
+	 * Both serial8250_em485_init and serial8250_em485_destroy
+	 * are idempotent
+	 */
+	if (rs485->flags & SER_RS485_ENABLED) {
+		int ret = serial8250_em485_init(up);
+
+		if (ret) {
+			rs485->flags &= ~SER_RS485_ENABLED;
+			p->rs485.flags &= ~SER_RS485_ENABLED;
+		}
+		return ret;
+	}
+
+	serial8250_em485_destroy(up);
+
+	return 0;
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -449,6 +479,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->serial_out	= dw8250_serial_out;
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
+	p->rs485_config = dw8250_rs485_config;
 
 	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
 	if (!p->membase)
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port
From: Andy Shevchenko @ 2018-06-06  9:55 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: LKML, open list:SERIAL DRIVERS, Rob Herring, Johan Hovold,
	Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <CAPybu_2dqG1HX39e+oG8rYvzMG_vGDBQjiLewNjwUvkh+AiF=w@mail.gmail.com>

On Wed, Jun 6, 2018 at 10:47 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hi Andy,
> On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:

> I have defaulted the module to n as you suggested. You can take a look
> to the patches that I plan to submit tomorrow at:
>
> https://github.com/ribalda/linux/tree/serdev2

n _is_ default. So, just remove a line.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port
From: Ricardo Ribalda Delgado @ 2018-06-06  9:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: LKML, open list:SERIAL DRIVERS, Rob Herring, Johan Hovold,
	Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <CAHp75VfZBKTu5XHt5_0eQp6m0Tdyi0H3vNz00c3ok+jgx2By7A@mail.gmail.com>

On Wed, Jun 6, 2018 at 11:55 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> >
> > https://github.com/ribalda/linux/tree/serdev2
>
> n _is_ default. So, just remove a line.
>

Done. Thanks!

> --
> With Best Regards,
> Andy Shevchenko



-- 
Ricardo Ribalda

^ permalink raw reply

* Re: [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
From: Richard Genoud @ 2018-06-06 11:02 UTC (permalink / raw)
  To: Radu Pirea, broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial
In-Reply-To: <20180604165943.31381-7-radu.pirea@microchip.com>

Typo in the subject:
changed->change


On 04/06/2018 18:59, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 41 ++++++++++++++++++-------------
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..5c74e03396ef 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
Sorry, I didn't catch that before, but we can drop "atmel,at91sam9260-usart-serial" don't we ?
Only "atmel,at91rm9200-usart-serial" is used in the MFD driver.

>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
>  static int atmel_prepare_tx_dma(struct uart_port *port)
>  {
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +	struct device *mfd_dev = port->dev->parent;
>  	dma_cap_mask_t		mask;
>  	struct dma_slave_config config;
>  	int ret, nent;
> @@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_SLAVE, mask);
>  
> -	atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
> +	atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
>  	if (atmel_port->chan_tx == NULL)
>  		goto chan_err;
>  	dev_info(port->dev, "using %s for tx DMA transfers\n",
> @@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  static int atmel_prepare_rx_dma(struct uart_port *port)
>  {
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +	struct device *mfd_dev = port->dev->parent;
>  	struct dma_async_tx_descriptor *desc;
>  	dma_cap_mask_t		mask;
>  	struct dma_slave_config config;
> @@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_CYCLIC, mask);
>  
> -	atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
> +	atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
>  	if (atmel_port->chan_rx == NULL)
>  		goto chan_err;
>  	dev_info(port->dev, "using %s for rx DMA transfers\n",
> @@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
I think this is not needed anymore (cf atmel_probe())

>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2222,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
>   */
>  static void atmel_release_port(struct uart_port *port)
>  {
> -	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	struct platform_device *mpdev = to_platform_device(port->dev->parent);
> +	int size = resource_size(mpdev->resource);
>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>   */
>  static int atmel_request_port(struct uart_port *port)
>  {
> -	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	struct platform_device *mpdev = to_platform_device(port->dev->parent);
> +	int size = resource_size(mpdev->resource);
>  
>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>  {
>  	int ret;
>  	struct uart_port *port = &atmel_port->uart;
> +	struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
>  
>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(&mpdev->dev, &port->rs485);
>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= mpdev->resource[0].start;
> +	port->irq		= mpdev->resource[1].start;
>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(&mpdev->dev, "usart");
>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2652,11 +2655,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
>  static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  				     struct platform_device *pdev)
>  {
> +	struct device *dev = pdev->dev.parent;
> +
Ditto

>  	atmel_port->fifo_size = 0;
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(dev->of_node,
Ditto
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,13 +2699,15 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
>  
>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
> +	pdev->dev.of_node = np;
> +
As atmel_serial device's of_node is now the parent node, the changes
in atmel_init_property() and atmel_serial_probe_fifos() are not needed anymore.
And maybe add a comment to explain that this is part of a MFD and all
the attributes are in the parent node, so we're using the MFD device node
instead of this device node.

>  	ret = of_alias_get_id(np, "serial");
>  	if (ret < 0)
>  		/* port id not found in platform data nor device-tree aliases:
> @@ -2845,7 +2852,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

Thanks !

Richard.

^ permalink raw reply

* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Andy Shevchenko @ 2018-06-06 11:56 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
	open list
In-Reply-To: <20180606094942.71190-4-giulio.benetti@micronovasrl.com>

On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> em485 gets lost during serial8250_register_8250_port().
> 
> Copy em485 to final uart port.
> 

Is it needed at all?

The individual driver decides either to use software emulation (and
calls explicitly serial8250_em485_init() for that) or do HW assisted
stuff.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/8250/8250_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c
> b/drivers/tty/serial/8250/8250_core.c
> index 9342fc2ee7df..c8c2b260c681 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -994,6 +994,7 @@ int serial8250_register_8250_port(struct
> uart_8250_port *up)
>  		uart->port.rs485_config	= up-
> >port.rs485_config;
>  		uart->port.rs485	= up->port.rs485;
>  		uart->dma		= up->dma;
> +		uart->em485		= up->em485;
>  
>  		/* Take tx_loadsz from fifosize if it wasn't set
> separately */
>  		if (uart->port.fifosize && !uart->tx_loadsz)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 2/4] serial: 8250: Copy mctrl when register port.
From: Andy Shevchenko @ 2018-06-06 12:01 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
	open list
In-Reply-To: <20180606094942.71190-1-giulio.benetti@micronovasrl.com>

On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is
> on
> TIOCM_RTS is set, then need to keep it set when registering port.
> 
> Copy mctrl to new port too.
> 

Not sure if it would be useful.
Seems the only em485 user, i.e. OMAP, does survive without that.

Moreover, often individual drivers override ->set_mctrl() callback.

So, real example is needed to explain why.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/8250/8250_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c
> b/drivers/tty/serial/8250/8250_core.c
> index c8c2b260c681..c8e62fbd6570 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct
> uart_8250_port *up)
>  		uart->port.unthrottle	= up->port.unthrottle;
>  		uart->port.rs485_config	= up-
> >port.rs485_config;
>  		uart->port.rs485	= up->port.rs485;
> +		uart->port.mctrl	= up->port.mctrl;
>  		uart->dma		= up->dma;
>  		uart->em485		= up->em485;
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 3/4] serial: 8250: Make em485_rts_after_send() set mctrl according to rts state.
From: Andy Shevchenko @ 2018-06-06 12:02 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Sean Young, Aaron Sierra, Tomas Melin, Sean Wang,
	Rafael Gago, Joel Stanley, open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606094942.71190-2-giulio.benetti@micronovasrl.com>

On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> When rs485 enabled and RTS_AFTER_SEND set on startup, need to preserve
> mctrl status, because later functions will call set_mctrl passing
> port->mctrl=0 overriding rts status, resulting in rts pin in
> transmission when idle.
> 
> Make mctrl reflect rts pin state.
> 

This might make sense, I leave it to Matwey to Ack / NAK / etc.
But it also feels that patch 2/4 should be part of this change.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/8250/8250_port.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 95833cbc4338..c8c10b5ec6d6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -562,10 +562,13 @@ static inline void
> serial8250_em485_rts_after_send(struct uart_8250_port *p)
>  {
>  	unsigned char mcr = serial8250_in_MCR(p);
>  
> -	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>  		mcr |= UART_MCR_RTS;
> -	else
> +		p->port.mctrl |= TIOCM_RTS;
> +	} else {
>  		mcr &= ~UART_MCR_RTS;
> +		p->port.mctrl &= ~TIOCM_RTS;
> +	}
>  	serial8250_out_MCR(p, mcr);
>  }
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 4/4] serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and RTS_AFTER_SEND set.
From: Andy Shevchenko @ 2018-06-06 12:03 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Sean Young, open list:SERIAL DRIVERS, open list
In-Reply-To: <20180606094942.71190-3-giulio.benetti@micronovasrl.com>

On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> If rs485 is enabled and RTS_AFTER_SEND is set on startup need to keep
> TIOCM_RTS asserted to keep rs485 transceiver in RX when idle.
> 
> Check if rs485 is on and RTS_AFTER_SEND is set and mask port->mctrl
> with
> TIOCM_RTS too and not only TIOCM_DTR.
> 

This one feels wrong to be in serial_core.c. Perhaps in 8250/8250*.c.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/serial_core.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c
> b/drivers/tty/serial/serial_core.c
> index 0466f9f08a91..06d9441f6d20 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2288,6 +2288,16 @@ uart_configure_port(struct uart_driver *drv,
> struct uart_state *state,
>  
>  	if (port->type != PORT_UNKNOWN) {
>  		unsigned long flags;
> +		int rs485_on = port->rs485_config &&
> +			(port->rs485.flags & SER_RS485_ENABLED);
> +		int RTS_after_send = !!(port->rs485.flags &
> +				SER_RS485_RTS_AFTER_SEND);
> +		int mctrl;
> +
> +		if (rs485_on && RTS_after_send)
> +			mctrl = port->mctrl & (TIOCM_DTR |
> TIOCM_RTS);
> +		else
> +			mctrl = port->mctrl & TIOCM_DTR;
>  
>  		uart_report_port(drv, port);
>  
> @@ -2300,7 +2310,7 @@ uart_configure_port(struct uart_driver *drv,
> struct uart_state *state,
>  		 * We probably don't need a spinlock around this, but
>  		 */
>  		spin_lock_irqsave(&port->lock, flags);
> -		port->ops->set_mctrl(port, port->mctrl & TIOCM_DTR);
> +		port->ops->set_mctrl(port, mctrl);
>  		spin_unlock_irqrestore(&port->lock, flags);
>  
>  		/*

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 4/4] serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and RTS_AFTER_SEND set.
From: Giulio Benetti @ 2018-06-06 12:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Sean Young, open list:SERIAL DRIVERS, open list
In-Reply-To: <a156ce90ab7e45ecf6936c29cc5d976c56f49a51.camel@linux.intel.com>

Il 06/06/2018 14:03, Andy Shevchenko ha scritto:
> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>> If rs485 is enabled and RTS_AFTER_SEND is set on startup need to keep
>> TIOCM_RTS asserted to keep rs485 transceiver in RX when idle.
>>
>> Check if rs485 is on and RTS_AFTER_SEND is set and mask port->mctrl
>> with
>> TIOCM_RTS too and not only TIOCM_DTR.
>>
> 
> This one feels wrong to be in serial_core.c. Perhaps in 8250/8250*.c.

I've tried to avoid modifying serial_core.c but if it masks mctrl only 
with TIOCM_DTR, it forces RTS unasserted.
Another way could be:
If rs485 ON and RTS_AFTER_SEND set, then ignore RTS driving in 
8250_set_mctrl, would it make sense?

Thanks

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-06 12:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
	open list
In-Reply-To: <069f5cd3309e83d13c74929f240720b232ea7251.camel@linux.intel.com>

Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>> em485 gets lost during serial8250_register_8250_port().
>>
>> Copy em485 to final uart port.
>>
> 
> Is it needed at all?
> 
> The individual driver decides either to use software emulation (and
> calls explicitly serial8250_em485_init() for that) or do HW assisted
> stuff.

In 8250_dw.c, during probe(), I need to call dw8250_rs485_config() 
against local struct uart_8250_port uart = {};
Inside serial8250_register_8250_port() not all uart fields are 
copied(em485 too).
So after probe, em485 is NULL.

Another way could be to call dw8250_rs485_config() against real uart 
port, after calling serial8250_register_8250_port(),
would it make sense?

> 
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/tty/serial/8250/8250_core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_core.c
>> b/drivers/tty/serial/8250/8250_core.c
>> index 9342fc2ee7df..c8c2b260c681 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -994,6 +994,7 @@ int serial8250_register_8250_port(struct
>> uart_8250_port *up)
>>   		uart->port.rs485_config	= up-
>>> port.rs485_config;
>>   		uart->port.rs485	= up->port.rs485;
>>   		uart->dma		= up->dma;
>> +		uart->em485		= up->em485;
>>   
>>   		/* Take tx_loadsz from fifosize if it wasn't set
>> separately */
>>   		if (uart->port.fifosize && !uart->tx_loadsz)
> 

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* [RFC PATCH v2 0/6] serial: uartps: Add run time support for more IPs than hardcoded 2
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel

Hi,

this series is trying to address discussion I had with Alan in past
https://patchwork.kernel.org/patch/9738445/ and also with Rob in v1
https://lkml.org/lkml/2018/4/26/551.

The first 5 patches are preparation patches to have the last patch as
small as possible to focus on changes there.

Cases without DT aliases are not solved in this series but one function
was shared in RFC v1.
The purpose of this series to get feedback on solution where every
driver instance allocate at run time own uart_driver.

For example this is how it works.
uart0 on higher alias
serial0 = &uart1;
serial100 = &uart0;

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jun  6 12:19 /dev/ttyPS0
crw--w----    1 root     root      253, 100 Jan  1  1970 /dev/ttyPS100

Thanks,
Michal


Changes in v2:
- new patch - it can be sent separately too
- Remove nr field logic
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- Register one uart_driver with unique minor at probe time

Michal Simek (6):
  serial: uartps: Do not initialize field to zero again
  serial: uartps: Move register to probe based on run time detection
  serial: uartps: Do not use static struct uart_driver out of probe()
  serial: uartps: Move alias reading higher in probe()
  serial: uartps: Fill struct uart_driver in probe()
  serial: uartps: Remove CDNS_UART_NR_PORTS macro

 drivers/tty/serial/xilinx_uartps.c | 126 ++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 58 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

Writing zero and NULLs to already initialized fields is not needed.
Remove this additional writes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 8a3e34234e98..5f116f3ecd4a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 	/* At this point, we've got an empty uart_port struct, initialize it */
 	spin_lock_init(&port->lock);
-	port->membase	= NULL;
-	port->irq	= 0;
 	port->type	= PORT_UNKNOWN;
 	port->iotype	= UPIO_MEM32;
 	port->flags	= UPF_BOOT_AUTOCONF;
 	port->ops	= &cdns_uart_ops;
 	port->fifosize	= CDNS_UART_FIFO_SIZE;
 	port->line	= id;
-	port->dev	= NULL;
 
 	/*
 	 * Register the port.
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 2/6] serial: uartps: Move register to probe based on run time detection
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Remove nr field logic

Discussed here: https://patchwork.kernel.org/patch/9738445/

The same solution is done in pl011 driver.

---
 drivers/tty/serial/xilinx_uartps.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5f116f3ecd4a..e24382f58dea 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1438,6 +1438,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	if (!cdns_uart_uart_driver.state) {
+		rc = uart_register_driver(&cdns_uart_uart_driver);
+		if (rc < 0) {
+			dev_err(&pdev->dev, "Failed to register driver\n");
+			return rc;
+		}
+	}
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1617,28 +1625,14 @@ static int cdns_uart_remove(struct platform_device *pdev)
 
 static int __init cdns_uart_init(void)
 {
-	int retval = 0;
-
-	/* Register the cdns_uart driver with the serial core */
-	retval = uart_register_driver(&cdns_uart_uart_driver);
-	if (retval)
-		return retval;
-
 	/* Register the platform driver */
-	retval = platform_driver_register(&cdns_uart_platform_driver);
-	if (retval)
-		uart_unregister_driver(&cdns_uart_uart_driver);
-
-	return retval;
+	return platform_driver_register(&cdns_uart_platform_driver);
 }
 
 static void __exit cdns_uart_exit(void)
 {
 	/* Unregister the platform driver */
 	platform_driver_unregister(&cdns_uart_platform_driver);
-
-	/* Unregister the cdns_uart driver */
-	uart_unregister_driver(&cdns_uart_uart_driver);
 }
 
 arch_initcall(cdns_uart_init);
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 3/6] serial: uartps: Do not use static struct uart_driver out of probe()
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

cdns_uart_suspend()/resume() and remove() are using static reference
to struct uart_driver. Assign this referece to private data structure
as preparation step for dynamic struct uart_driver allocation.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e24382f58dea..fe96fd950d3a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -179,6 +179,7 @@
  * @port:		Pointer to the UART port
  * @uartclk:		Reference clock
  * @pclk:		APB clock
+ * @cdns_uart_driver:	Pointer to UART driver
  * @baud:		Current baud rate
  * @clk_rate_change_nb:	Notifier block for clock changes
  * @quirks:		Flags for RXBS support.
@@ -187,6 +188,7 @@ struct cdns_uart {
 	struct uart_port	*port;
 	struct clk		*uartclk;
 	struct clk		*pclk;
+	struct uart_driver	*cdns_uart_driver;
 	unsigned int		baud;
 	struct notifier_block	clk_rate_change_nb;
 	u32			quirks;
@@ -1280,6 +1282,7 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 static int cdns_uart_suspend(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	struct tty_struct *tty;
 	struct device *tty_dev;
 	int may_wake = 0;
@@ -1296,7 +1299,7 @@ static int cdns_uart_suspend(struct device *device)
 	 * Call the API provided in serial_core.c file which handles
 	 * the suspend.
 	 */
-	uart_suspend_port(&cdns_uart_uart_driver, port);
+	uart_suspend_port(cdns_uart->cdns_uart_driver, port);
 	if (!(console_suspend_enabled && !may_wake)) {
 		unsigned long flags = 0;
 
@@ -1324,6 +1327,7 @@ static int cdns_uart_suspend(struct device *device)
 static int cdns_uart_resume(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	unsigned long flags = 0;
 	u32 ctrl_reg;
 	struct tty_struct *tty;
@@ -1339,8 +1343,6 @@ static int cdns_uart_resume(struct device *device)
 	}
 
 	if (console_suspend_enabled && !may_wake) {
-		struct cdns_uart *cdns_uart = port->private_data;
-
 		clk_enable(cdns_uart->pclk);
 		clk_enable(cdns_uart->uartclk);
 
@@ -1374,7 +1376,7 @@ static int cdns_uart_resume(struct device *device)
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
-	return uart_resume_port(&cdns_uart_uart_driver, port);
+	return uart_resume_port(cdns_uart->cdns_uart_driver, port);
 }
 #endif /* ! CONFIG_PM_SLEEP */
 static int __maybe_unused cdns_runtime_suspend(struct device *dev)
@@ -1446,6 +1448,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		}
 	}
 
+	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1603,7 +1607,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);
 #endif
-	rc = uart_remove_one_port(&cdns_uart_uart_driver, port);
+	rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
 	port->mapbase = 0;
 	clk_disable_unprepare(cdns_uart_data->uartclk);
 	clk_disable_unprepare(cdns_uart_data->pclk);
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 4/6] serial: uartps: Move alias reading higher in probe()
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

This cosmetic change is done only for having next patch much easier to
read. Moving id setup higher in probe is not affecting any usage of this
driver and it also simplify error path.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fe96fd950d3a..b47d7ccbc38d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1440,6 +1440,16 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	/* Look for a serialN alias */
+	id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (id < 0)
+		id = 0;
+
+	if (id >= CDNS_UART_NR_PORTS) {
+		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
+		return -ENODEV;
+	}
+
 	if (!cdns_uart_uart_driver.state) {
 		rc = uart_register_driver(&cdns_uart_uart_driver);
 		if (rc < 0) {
@@ -1509,16 +1519,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 				&cdns_uart_data->clk_rate_change_nb))
 		dev_warn(&pdev->dev, "Unable to register clock notifier.\n");
 #endif
-	/* Look for a serialN alias */
-	id = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (id < 0)
-		id = 0;
-
-	if (id >= CDNS_UART_NR_PORTS) {
-		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
-		rc = -ENODEV;
-		goto err_out_notif_unreg;
-	}
 
 	/* At this point, we've got an empty uart_port struct, initialize it */
 	spin_lock_init(&port->lock);
@@ -1577,7 +1577,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
-err_out_notif_unreg:
 #ifdef CONFIG_COMMON_CLK
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 5/6] serial: uartps: Fill struct uart_driver in probe()
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

This is preparation step for dynamic port allocation without
CDNS_UART_NR_PORTS macro. Fill the structure only once at probe.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b47d7ccbc38d..d76efe8cb3df 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1260,18 +1260,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 };
 #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
 
-static struct uart_driver cdns_uart_uart_driver = {
-	.owner		= THIS_MODULE,
-	.driver_name	= CDNS_UART_NAME,
-	.dev_name	= CDNS_UART_TTY_NAME,
-	.major		= CDNS_UART_MAJOR,
-	.minor		= CDNS_UART_MINOR,
-	.nr		= CDNS_UART_NR_PORTS,
-#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
-	.cons		= &cdns_uart_console,
-#endif
-};
-
 #ifdef CONFIG_PM_SLEEP
 /**
  * cdns_uart_suspend - suspend event
@@ -1451,6 +1439,16 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 
 	if (!cdns_uart_uart_driver.state) {
+		cdns_uart_uart_driver.owner = THIS_MODULE,
+		cdns_uart_uart_driver.driver_name = CDNS_UART_NAME,
+		cdns_uart_uart_driver.dev_name = CDNS_UART_TTY_NAME,
+		cdns_uart_uart_driver.major = CDNS_UART_MAJOR,
+		cdns_uart_uart_driver.minor = CDNS_UART_MINOR,
+		cdns_uart_uart_driver.nr = CDNS_UART_NR_PORTS,
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+		cdns_uart_uart_driver.cons = &cdns_uart_console,
+#endif
+
 		rc = uart_register_driver(&cdns_uart_uart_driver);
 		if (rc < 0) {
 			dev_err(&pdev->dev, "Failed to register driver\n");
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 6/6] serial: uartps: Remove CDNS_UART_NR_PORTS macro
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-kernel, monstr, gnomes, Alexander Graf, shubhraj, robh
  Cc: Jiri Slaby, linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

This patch is removing CDNS_UART_NR_PORTS macro which limits number of
ports which can be used. Every instance is registering own struct
uart_driver with minor number which corresponds to alias ID (or 0 now).
and with 1 uart port. The same alias ID is saved to
tty_driver->name_base which is key field for creating ttyPSX name.

Because name_base and minor number are setup already there is no need to
setup any port->line number because 0 is the right value.

~# find /proc/tty/ -name "*uartps*"
/proc/tty/driver/xuartps0
/proc/tty/driver/xuartps100

Unfortunately this driver is setting up major number to 0 for using
dynamic assignment and kernel is allocating different major numbers for
every instance instead of using the same major and different minor
number.

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jan  1 03:36 /dev/ttyPS0
crw--w----    1 root     root      253,   1 Jan  1 00:00 /dev/ttyPS1

When major number is not 0. For example 252 then major/minor
combinations are in expected form

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jan  1 04:04 /dev/ttyPS0
crw--w----    1 root     root      252,   1 Jan  1 00:00 /dev/ttyPS1

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Register one uart_driver with unique minor at probe time

This patch not done because id is not unique. This needs to be solved
before this patch can be applied. In v1 I have created
of_alias_check_id() for that. https://lkml.org/lkml/2018/4/26/551

Also we need to run more testing on this to make sure that everything is
working properly.

---
 drivers/tty/serial/xilinx_uartps.c | 78 +++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index d76efe8cb3df..82cc17ec7b5d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -30,8 +30,6 @@
 #define CDNS_UART_TTY_NAME	"ttyPS"
 #define CDNS_UART_NAME		"xuartps"
 #define CDNS_UART_MAJOR		0	/* use dynamic node allocation */
-#define CDNS_UART_MINOR		0	/* works best with devtmpfs */
-#define CDNS_UART_NR_PORTS	2
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
 #define CDNS_UART_REGISTER_SPACE	0x1000
 
@@ -1247,8 +1245,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
-static struct uart_driver cdns_uart_uart_driver;
-
 static struct console cdns_uart_console = {
 	.name	= CDNS_UART_TTY_NAME,
 	.write	= cdns_uart_console_write,
@@ -1256,7 +1252,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 	.setup	= cdns_uart_console_setup,
 	.flags	= CON_PRINTBUFFER,
 	.index	= -1, /* Specified on the cmdline (e.g. console=ttyPS ) */
-	.data	= &cdns_uart_uart_driver,
 };
 #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
 
@@ -1419,6 +1414,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct cdns_uart *cdns_uart_data;
 	const struct of_device_id *match;
+	struct uart_driver *cdns_uart_uart_driver;
+	char *driver_name;
 
 	cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
 			GFP_KERNEL);
@@ -1431,32 +1428,49 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	/* Look for a serialN alias */
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0)
+		/*
+		 * FIXME this is not enough because if there the next instance
+		 * without alias it will get also id = 0 which is wrong
+		 */
 		id = 0;
 
-	if (id >= CDNS_UART_NR_PORTS) {
-		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
-		return -ENODEV;
-	}
+	cdns_uart_uart_driver = devm_kzalloc(&pdev->dev,
+					     sizeof(*cdns_uart_uart_driver),
+					     GFP_KERNEL);
+	if (!cdns_uart_uart_driver)
+		return -ENOMEM;
+
+	/* There is a need to use unique driver name */
+	driver_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%d",
+				     CDNS_UART_NAME, id);
+	if (!driver_name)
+		return -ENOMEM;
 
-	if (!cdns_uart_uart_driver.state) {
-		cdns_uart_uart_driver.owner = THIS_MODULE,
-		cdns_uart_uart_driver.driver_name = CDNS_UART_NAME,
-		cdns_uart_uart_driver.dev_name = CDNS_UART_TTY_NAME,
-		cdns_uart_uart_driver.major = CDNS_UART_MAJOR,
-		cdns_uart_uart_driver.minor = CDNS_UART_MINOR,
-		cdns_uart_uart_driver.nr = CDNS_UART_NR_PORTS,
+	cdns_uart_uart_driver->owner = THIS_MODULE;
+	cdns_uart_uart_driver->driver_name = driver_name;
+	cdns_uart_uart_driver->dev_name	= CDNS_UART_TTY_NAME;
+	cdns_uart_uart_driver->major = CDNS_UART_MAJOR;
+	cdns_uart_uart_driver->minor = id;
+	cdns_uart_uart_driver->nr = 1;
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
-		cdns_uart_uart_driver.cons = &cdns_uart_console,
+	cdns_uart_uart_driver->cons = &cdns_uart_console;
+	cdns_uart_console.data = cdns_uart_uart_driver;
 #endif
 
-		rc = uart_register_driver(&cdns_uart_uart_driver);
-		if (rc < 0) {
-			dev_err(&pdev->dev, "Failed to register driver\n");
-			return rc;
-		}
+	rc = uart_register_driver(cdns_uart_uart_driver);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Failed to register driver\n");
+		return rc;
 	}
 
-	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
+	/*
+	 * Setting up proper name_base needs to be done after uart
+	 * registration because tty_driver structure is not filled.
+	 * name_base is 0 by default.
+	 */
+	cdns_uart_uart_driver->tty_driver->name_base = id;
+
+	cdns_uart_data->cdns_uart_driver = cdns_uart_uart_driver;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
@@ -1473,7 +1487,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 	if (IS_ERR(cdns_uart_data->pclk)) {
 		dev_err(&pdev->dev, "pclk clock not found.\n");
-		return PTR_ERR(cdns_uart_data->pclk);
+		rc = PTR_ERR(cdns_uart_data->pclk);
+		goto err_out_unregister_driver;
 	}
 
 	cdns_uart_data->uartclk = devm_clk_get(&pdev->dev, "uart_clk");
@@ -1484,13 +1499,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 	if (IS_ERR(cdns_uart_data->uartclk)) {
 		dev_err(&pdev->dev, "uart_clk clock not found.\n");
-		return PTR_ERR(cdns_uart_data->uartclk);
+		rc = PTR_ERR(cdns_uart_data->uartclk);
+		goto err_out_unregister_driver;
 	}
 
 	rc = clk_prepare_enable(cdns_uart_data->pclk);
 	if (rc) {
 		dev_err(&pdev->dev, "Unable to enable pclk clock.\n");
-		return rc;
+		goto err_out_unregister_driver;
 	}
 	rc = clk_prepare_enable(cdns_uart_data->uartclk);
 	if (rc) {
@@ -1525,7 +1541,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	port->flags	= UPF_BOOT_AUTOCONF;
 	port->ops	= &cdns_uart_ops;
 	port->fifosize	= CDNS_UART_FIFO_SIZE;
-	port->line	= id;
 
 	/*
 	 * Register the port.
@@ -1552,11 +1567,11 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 * If register_console() don't assign value, then console_port pointer
 	 * is cleanup.
 	 */
-	if (cdns_uart_uart_driver.cons->index == -1)
+	if (cdns_uart_uart_driver->cons->index == -1)
 		console_port = port;
 #endif
 
-	rc = uart_add_one_port(&cdns_uart_uart_driver, port);
+	rc = uart_add_one_port(cdns_uart_uart_driver, port);
 	if (rc) {
 		dev_err(&pdev->dev,
 			"uart_add_one_port() failed; err=%i\n", rc);
@@ -1565,7 +1580,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 	/* This is not port which is used for console that's why clean it up */
-	if (cdns_uart_uart_driver.cons->index == -1)
+	if (cdns_uart_uart_driver->cons->index == -1)
 		console_port = NULL;
 #endif
 
@@ -1583,6 +1598,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	clk_disable_unprepare(cdns_uart_data->uartclk);
 err_out_clk_dis_pclk:
 	clk_disable_unprepare(cdns_uart_data->pclk);
+err_out_unregister_driver:
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 
 	return rc;
 }
@@ -1611,6 +1628,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Andy Shevchenko @ 2018-06-06 13:11 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
	Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
	open list
In-Reply-To: <0bc400b1-6178-2021-c9a3-3190d1a1de32@micronovasrl.com>

On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
> > On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> > > em485 gets lost during
> > > 
> > > Copy em485 to final uart port.
> > > 
> > 
> > Is it needed at all?
> > 
> > The individual driver decides either to use software emulation (and
> > calls explicitly serial8250_em485_init() for that) or do HW assisted
> > stuff.
> 
> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config() 
> against local struct uart_8250_port uart = {};
> Inside serial8250_register_8250_port() not all uart fields are 
> copied(em485 too).
> So after probe, em485 is NULL.
> 
> Another way could be to call dw8250_rs485_config() against real uart 
> port, after calling serial8250_register_8250_port(),
> would it make sense?

Look at OMAP case closely. They have a callback to configure RS485 which
is called in uart_set_rs485_config()  which is called whenever user
space does TIOCGRS485 IOCTL.

So, it's completely driven by user space which makes sense by my
opinion.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox