Linux Serial subsystem development
 help / color / mirror / Atom feed
* [RESEND PATCH 04/14] serial: lantiq: Change ltq_w32_mask to asc_update_bits
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

ltq prefix is platform specific function, asc prefix
is more generic.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 66c671677761..4c14608b8ef8 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -113,6 +113,13 @@ struct ltq_uart_port {
 	unsigned int		err_irq;
 };
 
+static inline void asc_update_bits(u32 clear, u32 set, void __iomem *reg)
+{
+	u32 tmp = readl(reg);
+
+	writel((tmp & ~clear) | set, reg);
+}
+
 static inline struct
 ltq_uart_port *to_ltq_uart_port(struct uart_port *port)
 {
@@ -163,16 +170,16 @@ lqasc_rx_chars(struct uart_port *port)
 		if (rsr & ASCSTATE_ANY) {
 			if (rsr & ASCSTATE_PE) {
 				port->icount.parity++;
-				ltq_w32_mask(0, ASCWHBSTATE_CLRPE,
+				asc_update_bits(0, ASCWHBSTATE_CLRPE,
 					port->membase + LTQ_ASC_WHBSTATE);
 			} else if (rsr & ASCSTATE_FE) {
 				port->icount.frame++;
-				ltq_w32_mask(0, ASCWHBSTATE_CLRFE,
+				asc_update_bits(0, ASCWHBSTATE_CLRFE,
 					port->membase + LTQ_ASC_WHBSTATE);
 			}
 			if (rsr & ASCSTATE_ROE) {
 				port->icount.overrun++;
-				ltq_w32_mask(0, ASCWHBSTATE_CLRROE,
+				asc_update_bits(0, ASCWHBSTATE_CLRROE,
 					port->membase + LTQ_ASC_WHBSTATE);
 			}
 
@@ -252,7 +259,7 @@ lqasc_err_int(int irq, void *_port)
 	struct uart_port *port = (struct uart_port *)_port;
 	spin_lock_irqsave(&ltq_asc_lock, flags);
 	/* clear any pending interrupts */
-	ltq_w32_mask(0, ASCWHBSTATE_CLRPE | ASCWHBSTATE_CLRFE |
+	asc_update_bits(0, ASCWHBSTATE_CLRPE | ASCWHBSTATE_CLRFE |
 		ASCWHBSTATE_CLRROE, port->membase + LTQ_ASC_WHBSTATE);
 	spin_unlock_irqrestore(&ltq_asc_lock, flags);
 	return IRQ_HANDLED;
@@ -304,7 +311,7 @@ lqasc_startup(struct uart_port *port)
 		clk_enable(ltq_port->clk);
 	port->uartclk = clk_get_rate(ltq_port->fpiclk);
 
-	ltq_w32_mask(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
+	asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
 		port->membase + LTQ_ASC_CLC);
 
 	ltq_w32(0, port->membase + LTQ_ASC_PISEL);
@@ -320,7 +327,7 @@ lqasc_startup(struct uart_port *port)
 	 * setting enable bits
 	 */
 	wmb();
-	ltq_w32_mask(0, ASCCON_M_8ASYNC | ASCCON_FEN | ASCCON_TOEN |
+	asc_update_bits(0, ASCCON_M_8ASYNC | ASCCON_FEN | ASCCON_TOEN |
 		ASCCON_ROEN, port->membase + LTQ_ASC_CON);
 
 	retval = request_irq(ltq_port->tx_irq, lqasc_tx_int,
@@ -364,9 +371,9 @@ lqasc_shutdown(struct uart_port *port)
 	free_irq(ltq_port->err_irq, port);
 
 	ltq_w32(0, port->membase + LTQ_ASC_CON);
-	ltq_w32_mask(ASCRXFCON_RXFEN, ASCRXFCON_RXFFLU,
+	asc_update_bits(ASCRXFCON_RXFEN, ASCRXFCON_RXFFLU,
 		port->membase + LTQ_ASC_RXFCON);
-	ltq_w32_mask(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
+	asc_update_bits(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
 		port->membase + LTQ_ASC_TXFCON);
 	if (!IS_ERR(ltq_port->clk))
 		clk_disable(ltq_port->clk);
@@ -438,7 +445,7 @@ lqasc_set_termios(struct uart_port *port,
 	spin_lock_irqsave(&ltq_asc_lock, flags);
 
 	/* set up CON */
-	ltq_w32_mask(0, con, port->membase + LTQ_ASC_CON);
+	asc_update_bits(0, con, port->membase + LTQ_ASC_CON);
 
 	/* Set baud rate - take a divider of 2 into account */
 	baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
@@ -446,19 +453,19 @@ lqasc_set_termios(struct uart_port *port,
 	divisor = divisor / 2 - 1;
 
 	/* disable the baudrate generator */
-	ltq_w32_mask(ASCCON_R, 0, port->membase + LTQ_ASC_CON);
+	asc_update_bits(ASCCON_R, 0, port->membase + LTQ_ASC_CON);
 
 	/* make sure the fractional divider is off */
-	ltq_w32_mask(ASCCON_FDE, 0, port->membase + LTQ_ASC_CON);
+	asc_update_bits(ASCCON_FDE, 0, port->membase + LTQ_ASC_CON);
 
 	/* set up to use divisor of 2 */
-	ltq_w32_mask(ASCCON_BRS, 0, port->membase + LTQ_ASC_CON);
+	asc_update_bits(ASCCON_BRS, 0, port->membase + LTQ_ASC_CON);
 
 	/* now we can write the new baudrate into the register */
 	ltq_w32(divisor, port->membase + LTQ_ASC_BG);
 
 	/* turn the baudrate generator back on */
-	ltq_w32_mask(0, ASCCON_R, port->membase + LTQ_ASC_CON);
+	asc_update_bits(0, ASCCON_R, port->membase + LTQ_ASC_CON);
 
 	/* enable rx */
 	ltq_w32(ASCWHBSTATE_SETREN, port->membase + LTQ_ASC_WHBSTATE);
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 06/14] serial: lantiq: Use readl/writel instead of ltq_r32/ltq_w32
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

Previous implementation uses platform-dependent functions
ltq_w32()/ltq_r32() to access registers. Those functions are not
available for other SoC which uses the same IP.
Change to OS provided readl()/writel() and readb()/writeb(), so
that different SoCs can use the same driver.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 4c14608b8ef8..e351f80996d3 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -145,7 +145,7 @@ lqasc_start_tx(struct uart_port *port)
 static void
 lqasc_stop_rx(struct uart_port *port)
 {
-	ltq_w32(ASCWHBSTATE_CLRREN, port->membase + LTQ_ASC_WHBSTATE);
+	writel(ASCWHBSTATE_CLRREN, port->membase + LTQ_ASC_WHBSTATE);
 }
 
 static int
@@ -154,11 +154,11 @@ lqasc_rx_chars(struct uart_port *port)
 	struct tty_port *tport = &port->state->port;
 	unsigned int ch = 0, rsr = 0, fifocnt;
 
-	fifocnt = ltq_r32(port->membase + LTQ_ASC_FSTAT) & ASCFSTAT_RXFFLMASK;
+	fifocnt = readl(port->membase + LTQ_ASC_FSTAT) & ASCFSTAT_RXFFLMASK;
 	while (fifocnt--) {
 		u8 flag = TTY_NORMAL;
-		ch = ltq_r8(port->membase + LTQ_ASC_RBUF);
-		rsr = (ltq_r32(port->membase + LTQ_ASC_STATE)
+		ch = readb(port->membase + LTQ_ASC_RBUF);
+		rsr = (readl(port->membase + LTQ_ASC_STATE)
 			& ASCSTATE_ANY) | UART_DUMMY_UER_RX;
 		tty_flip_buffer_push(tport);
 		port->icount.rx++;
@@ -218,10 +218,10 @@ lqasc_tx_chars(struct uart_port *port)
 		return;
 	}
 
-	while (((ltq_r32(port->membase + LTQ_ASC_FSTAT) &
+	while (((readl(port->membase + LTQ_ASC_FSTAT) &
 		ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF) != 0) {
 		if (port->x_char) {
-			ltq_w8(port->x_char, port->membase + LTQ_ASC_TBUF);
+			writeb(port->x_char, port->membase + LTQ_ASC_TBUF);
 			port->icount.tx++;
 			port->x_char = 0;
 			continue;
@@ -230,7 +230,7 @@ lqasc_tx_chars(struct uart_port *port)
 		if (uart_circ_empty(xmit))
 			break;
 
-		ltq_w8(port->state->xmit.buf[port->state->xmit.tail],
+		writeb(port->state->xmit.buf[port->state->xmit.tail],
 			port->membase + LTQ_ASC_TBUF);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
@@ -246,7 +246,7 @@ lqasc_tx_int(int irq, void *_port)
 	unsigned long flags;
 	struct uart_port *port = (struct uart_port *)_port;
 	spin_lock_irqsave(&ltq_asc_lock, flags);
-	ltq_w32(ASC_IRNCR_TIR, port->membase + LTQ_ASC_IRNCR);
+	writel(ASC_IRNCR_TIR, port->membase + LTQ_ASC_IRNCR);
 	spin_unlock_irqrestore(&ltq_asc_lock, flags);
 	lqasc_start_tx(port);
 	return IRQ_HANDLED;
@@ -271,7 +271,7 @@ lqasc_rx_int(int irq, void *_port)
 	unsigned long flags;
 	struct uart_port *port = (struct uart_port *)_port;
 	spin_lock_irqsave(&ltq_asc_lock, flags);
-	ltq_w32(ASC_IRNCR_RIR, port->membase + LTQ_ASC_IRNCR);
+	writel(ASC_IRNCR_RIR, port->membase + LTQ_ASC_IRNCR);
 	lqasc_rx_chars(port);
 	spin_unlock_irqrestore(&ltq_asc_lock, flags);
 	return IRQ_HANDLED;
@@ -281,7 +281,7 @@ static unsigned int
 lqasc_tx_empty(struct uart_port *port)
 {
 	int status;
-	status = ltq_r32(port->membase + LTQ_ASC_FSTAT) & ASCFSTAT_TXFFLMASK;
+	status = readl(port->membase + LTQ_ASC_FSTAT) & ASCFSTAT_TXFFLMASK;
 	return status ? 0 : TIOCSER_TEMT;
 }
 
@@ -314,12 +314,12 @@ lqasc_startup(struct uart_port *port)
 	asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
 		port->membase + LTQ_ASC_CLC);
 
-	ltq_w32(0, port->membase + LTQ_ASC_PISEL);
-	ltq_w32(
+	writel(0, port->membase + LTQ_ASC_PISEL);
+	writel(
 		((TXFIFO_FL << ASCTXFCON_TXFITLOFF) & ASCTXFCON_TXFITLMASK) |
 		ASCTXFCON_TXFEN | ASCTXFCON_TXFFLU,
 		port->membase + LTQ_ASC_TXFCON);
-	ltq_w32(
+	writel(
 		((RXFIFO_FL << ASCRXFCON_RXFITLOFF) & ASCRXFCON_RXFITLMASK)
 		| ASCRXFCON_RXFEN | ASCRXFCON_RXFFLU,
 		port->membase + LTQ_ASC_RXFCON);
@@ -351,7 +351,7 @@ lqasc_startup(struct uart_port *port)
 		goto err2;
 	}
 
-	ltq_w32(ASC_IRNREN_RX | ASC_IRNREN_ERR | ASC_IRNREN_TX,
+	writel(ASC_IRNREN_RX | ASC_IRNREN_ERR | ASC_IRNREN_TX,
 		port->membase + LTQ_ASC_IRNREN);
 	return 0;
 
@@ -370,7 +370,7 @@ lqasc_shutdown(struct uart_port *port)
 	free_irq(ltq_port->rx_irq, port);
 	free_irq(ltq_port->err_irq, port);
 
-	ltq_w32(0, port->membase + LTQ_ASC_CON);
+	writel(0, port->membase + LTQ_ASC_CON);
 	asc_update_bits(ASCRXFCON_RXFEN, ASCRXFCON_RXFFLU,
 		port->membase + LTQ_ASC_RXFCON);
 	asc_update_bits(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
@@ -462,13 +462,13 @@ lqasc_set_termios(struct uart_port *port,
 	asc_update_bits(ASCCON_BRS, 0, port->membase + LTQ_ASC_CON);
 
 	/* now we can write the new baudrate into the register */
-	ltq_w32(divisor, port->membase + LTQ_ASC_BG);
+	writel(divisor, port->membase + LTQ_ASC_BG);
 
 	/* turn the baudrate generator back on */
 	asc_update_bits(0, ASCCON_R, port->membase + LTQ_ASC_CON);
 
 	/* enable rx */
-	ltq_w32(ASCWHBSTATE_SETREN, port->membase + LTQ_ASC_WHBSTATE);
+	writel(ASCWHBSTATE_SETREN, port->membase + LTQ_ASC_WHBSTATE);
 
 	spin_unlock_irqrestore(&ltq_asc_lock, flags);
 
@@ -579,10 +579,10 @@ lqasc_console_putchar(struct uart_port *port, int ch)
 		return;
 
 	do {
-		fifofree = (ltq_r32(port->membase + LTQ_ASC_FSTAT)
+		fifofree = (readl(port->membase + LTQ_ASC_FSTAT)
 			& ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF;
 	} while (fifofree == 0);
-	ltq_w8(ch, port->membase + LTQ_ASC_TBUF);
+	writeb(ch, port->membase + LTQ_ASC_TBUF);
 }
 
 static void lqasc_serial_port_write(struct uart_port *port, const char *s,
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 07/14] serial: lantiq: Rename fpiclk to freqclk
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

fpiclk is platform specific, freqclk is more generic.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index e351f80996d3..4acdbdf8fe7a 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -105,7 +105,7 @@ static DEFINE_SPINLOCK(ltq_asc_lock);
 struct ltq_uart_port {
 	struct uart_port	port;
 	/* clock used to derive divider */
-	struct clk		*fpiclk;
+	struct clk		*freqclk;
 	/* clock gating of the ASC core */
 	struct clk		*clk;
 	unsigned int		tx_irq;
@@ -309,7 +309,7 @@ lqasc_startup(struct uart_port *port)
 
 	if (!IS_ERR(ltq_port->clk))
 		clk_enable(ltq_port->clk);
-	port->uartclk = clk_get_rate(ltq_port->fpiclk);
+	port->uartclk = clk_get_rate(ltq_port->freqclk);
 
 	asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
 		port->membase + LTQ_ASC_CLC);
@@ -632,7 +632,7 @@ lqasc_console_setup(struct console *co, char *options)
 	if (!IS_ERR(ltq_port->clk))
 		clk_enable(ltq_port->clk);
 
-	port->uartclk = clk_get_rate(ltq_port->fpiclk);
+	port->uartclk = clk_get_rate(ltq_port->freqclk);
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -744,8 +744,8 @@ lqasc_probe(struct platform_device *pdev)
 	port->irq	= irqres[0].start;
 	port->mapbase	= mmres->start;
 
-	ltq_port->fpiclk = clk_get_fpi();
-	if (IS_ERR(ltq_port->fpiclk)) {
+	ltq_port->freqclk = clk_get_fpi();
+	if (IS_ERR(ltq_port->freqclk)) {
 		pr_err("failed to get fpi clk\n");
 		return -ENOENT;
 	}
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 08/14] serial: lantiq: Replace clk_enable/clk_disable with clk generic API
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

The clk driver has introduced new clock APIs that replace
the existing clk_enable and clk_disable.
- clk_enable() APIs is replaced with clk_prepare_enable()
- clk_disable() API is replaced with clk_disable_unprepare()

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 4acdbdf8fe7a..34b1ef3c12ce 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -308,7 +308,7 @@ lqasc_startup(struct uart_port *port)
 	int retval;
 
 	if (!IS_ERR(ltq_port->clk))
-		clk_enable(ltq_port->clk);
+		clk_prepare_enable(ltq_port->clk);
 	port->uartclk = clk_get_rate(ltq_port->freqclk);
 
 	asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
@@ -376,7 +376,7 @@ lqasc_shutdown(struct uart_port *port)
 	asc_update_bits(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
 		port->membase + LTQ_ASC_TXFCON);
 	if (!IS_ERR(ltq_port->clk))
-		clk_disable(ltq_port->clk);
+		clk_disable_unprepare(ltq_port->clk);
 }
 
 static void
@@ -630,7 +630,7 @@ lqasc_console_setup(struct console *co, char *options)
 	port = &ltq_port->port;
 
 	if (!IS_ERR(ltq_port->clk))
-		clk_enable(ltq_port->clk);
+		clk_prepare_enable(ltq_port->clk);
 
 	port->uartclk = clk_get_rate(ltq_port->freqclk);
 
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 09/14] serial: lantiq: Add CCF support
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

Previous implementation uses platform-dependent API to get the clock.
Those functions are not available for other SoC which uses the same IP.
The CCF (Common Clock Framework) have an abstraction based APIs for
clock. In future, the platform specific code will be removed when the
legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs
can use the same driver.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

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

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 34b1ef3c12ce..88210de00f35 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -744,14 +744,22 @@ lqasc_probe(struct platform_device *pdev)
 	port->irq	= irqres[0].start;
 	port->mapbase	= mmres->start;
 
-	ltq_port->freqclk = clk_get_fpi();
+	if (IS_ENABLED(CONFIG_LANTIQ) && !IS_ENABLED(CONFIG_COMMON_CLK))
+		ltq_port->freqclk = clk_get_fpi();
+	else
+		ltq_port->freqclk = devm_clk_get(&pdev->dev, "freq");
+
+
 	if (IS_ERR(ltq_port->freqclk)) {
 		pr_err("failed to get fpi clk\n");
 		return -ENOENT;
 	}
 
 	/* not all asc ports have clock gates, lets ignore the return code */
-	ltq_port->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ENABLED(CONFIG_LANTIQ) && !IS_ENABLED(CONFIG_COMMON_CLK))
+		ltq_port->clk = clk_get(&pdev->dev, NULL);
+	else
+		ltq_port->clk = devm_clk_get(&pdev->dev, "asc");
 
 	ltq_port->tx_irq = irqres[0].start;
 	ltq_port->rx_irq = irqres[1].start;
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 10/14] serial: lantiq: Reorder the head files
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

Reorder the head files according to the coding style.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 88210de00f35..c983694ba24d 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -8,22 +8,22 @@
  * Copyright (C) 2010 Thomas Langer, <thomas.langer@lantiq.com>
  */
 
-#include <linux/slab.h>
-#include <linux/ioport.h>
-#include <linux/init.h>
+#include <linux/clk.h>
 #include <linux/console.h>
-#include <linux/sysrq.h>
 #include <linux/device.h>
-#include <linux/tty.h>
-#include <linux/tty_flip.h>
-#include <linux/serial_core.h>
-#include <linux/serial.h>
-#include <linux/of_platform.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-#include <linux/io.h>
-#include <linux/clk.h>
-#include <linux/gpio.h>
+#include <linux/of_platform.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/slab.h>
+#include <linux/sysrq.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
 
 #include <lantiq_soc.h>
 
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 12/14] serial: lantiq: Replace lantiq_soc.h with lantiq.h
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

In this existing lantiq serial driver,
lantiq_soc.h is defined in the arch directory,

./arch/mips/include/asm/mach-lantiq/falcon/lantiq_soc.h
./arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h

This driver need to be extended to support more platform,
lantiq.h is added in include/linux/ to make it
globally available and provide some wrapper code.
Use lantiq.h to make the driver can find the correct
header file.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index c983694ba24d..ba0c70b16bda 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
+#include <linux/lantiq.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
@@ -25,8 +26,6 @@
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 
-#include <lantiq_soc.h>
-
 #define PORT_LTQ_ASC		111
 #define MAXPORTS		2
 #define UART_DUMMY_UER_RX	1
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 13/14] serial: lantiq: Change init_lqasc to static declaration
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, linux-kernel,
	linux-serial
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

init_lqasc() is only used internally, change to static declaration.

Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 drivers/tty/serial/lantiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index ba0c70b16bda..e052b69ceb98 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -784,7 +784,7 @@ static struct platform_driver lqasc_driver = {
 	},
 };
 
-int __init
+static int __init
 init_lqasc(void)
 {
 	int ret;
-- 
2.11.0

^ permalink raw reply related

* [RESEND PATCH 14/14] dt-bindings: serial: lantiq: Add optional properties for CCF
From: Songjun Wu @ 2018-10-16  9:19 UTC (permalink / raw)
  To: yixin.zhu, chuanhua.lei, hauke.mehrtens
  Cc: gregkh, paul.burton, jslaby, Songjun Wu, devicetree, linux-kernel,
	Rob Herring, linux-serial, Mark Rutland
In-Reply-To: <20181016091915.19909-1-songjun.wu@linux.intel.com>

Clocks and clock-names are updated in device tree binding.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

 Documentation/devicetree/bindings/serial/lantiq_asc.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/lantiq_asc.txt b/Documentation/devicetree/bindings/serial/lantiq_asc.txt
index 3acbd309ab9d..40e81a5818f6 100644
--- a/Documentation/devicetree/bindings/serial/lantiq_asc.txt
+++ b/Documentation/devicetree/bindings/serial/lantiq_asc.txt
@@ -6,8 +6,23 @@ Required properties:
 - interrupts: the 3 (tx rx err) interrupt numbers. The interrupt specifier
   depends on the interrupt-parent interrupt controller.
 
+Optional properties:
+- clocks: Should contain frequency clock and gate clock
+- clock-names: Should be "freq" and "asc"
+
 Example:
 
+asc0: serial@16600000 {
+	compatible = "lantiq,asc";
+	reg = <0x16600000 0x100000>;
+	interrupt-parent = <&gic>;
+	interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>,
+		<GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>,
+		<GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>;
+	clock-names = "freq", "asc";
+};
+
 asc1: serial@e100c00 {
 	compatible = "lantiq,asc";
 	reg = <0xE100C00 0x400>;
-- 
2.11.0

^ permalink raw reply related

* [LINUX PATCHv3 0/9] serial-uartlite: Add support for dynamic allocation
From: shubhrajyoti.datta @ 2018-10-16 10:17 UTC (permalink / raw)
  To: linux-serial, linux-kernel; +Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Currently the number of uarts are configured through a Kconfig
option Make it dynamic.
While at it adapt to the runtime framework.

It is based a similar series on the uartps from Michal

Shubhrajyoti Datta (9):
  serial-uartlite: Move the uart register
  serial-uartlite: Add get serial id if not provided
  serial-uartlite: Do not use static struct uart_driver out of probe()
  serial-uartlite: Add runtime support
  serial-uartlite: Fix the unbind path
  serial-uartlite: Change logic how console_port is setup
  serial-uartlite: Use allocated structure instead of static ones
  serial-uartlite: Remove ULITE_NR_PORTS macro
  serial-uartlite: Remove SERIAL_UARTLITE_NR_UARTS

 drivers/tty/serial/Kconfig    |   9 --
 drivers/tty/serial/uartlite.c | 325 +++++++++++++++++++++++++++++++-----------
 2 files changed, 243 insertions(+), 91 deletions(-)

-- 
2.1.1

^ permalink raw reply

* [LINUX PATCHv3 1/9] serial-uartlite: Move the uart register
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel; +Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Move the uart register. This fixes the error path where the
clock disable is missed out.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index f0344ad..77bc9d0 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -763,6 +763,15 @@ static int ulite_probe(struct platform_device *pdev)
 	if (prop)
 		id = be32_to_cpup(prop);
 #endif
+	if (!ulite_uart_driver.state) {
+		dev_dbg(&pdev->dev, "uartlite: calling uart_register_driver()\n");
+		ret = uart_register_driver(&ulite_uart_driver);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Failed to register driver\n");
+			return ret;
+		}
+	}
+
 	pdata = devm_kzalloc(&pdev->dev, sizeof(struct uartlite_data),
 			     GFP_KERNEL);
 	if (!pdata)
@@ -794,15 +803,6 @@ static int ulite_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (!ulite_uart_driver.state) {
-		dev_dbg(&pdev->dev, "uartlite: calling uart_register_driver()\n");
-		ret = uart_register_driver(&ulite_uart_driver);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "Failed to register driver\n");
-			return ret;
-		}
-	}
-
 	ret = ulite_assign(&pdev->dev, id, res->start, irq, pdata);
 
 	clk_disable(pdata->clk);
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 2/9] serial-uartlite: Add get serial id if not provided
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Add get serial id if not provided

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 77bc9d0..441a216 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -763,6 +763,13 @@ static int ulite_probe(struct platform_device *pdev)
 	if (prop)
 		id = be32_to_cpup(prop);
 #endif
+	if (id < 0) {
+		/* Look for a serialN alias */
+		id = of_alias_get_id(pdev->dev.of_node, "serial");
+		if (id < 0)
+			id = 0;
+	}
+
 	if (!ulite_uart_driver.state) {
 		dev_dbg(&pdev->dev, "uartlite: calling uart_register_driver()\n");
 		ret = uart_register_driver(&ulite_uart_driver);
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 3/9] serial-uartlite: Do not use static struct uart_driver out of probe()
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

ulite_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: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 441a216..64c7248 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -63,6 +63,7 @@ static struct uart_port *console_port;
 struct uartlite_data {
 	const struct uartlite_reg_ops *reg_ops;
 	struct clk *clk;
+	struct uart_driver *ulite_uart_driver;
 };
 
 struct uartlite_reg_ops {
@@ -691,10 +692,11 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 static int ulite_release(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
+	struct uartlite_data *pdata = port->private_data;
 	int rc = 0;
 
 	if (port) {
-		rc = uart_remove_one_port(&ulite_uart_driver, port);
+		rc = uart_remove_one_port(pdata->ulite_uart_driver, port);
 		dev_set_drvdata(dev, NULL);
 		port->mapbase = 0;
 	}
@@ -711,9 +713,10 @@ static int ulite_release(struct device *dev)
 static int __maybe_unused ulite_suspend(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
+	struct uartlite_data *pdata = port->private_data;
 
 	if (port)
-		uart_suspend_port(&ulite_uart_driver, port);
+		uart_suspend_port(pdata->ulite_uart_driver, port);
 
 	return 0;
 }
@@ -727,9 +730,10 @@ static int __maybe_unused ulite_suspend(struct device *dev)
 static int __maybe_unused ulite_resume(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
+	struct uartlite_data *pdata = port->private_data;
 
 	if (port)
-		uart_resume_port(&ulite_uart_driver, port);
+		uart_resume_port(pdata->ulite_uart_driver, port);
 
 	return 0;
 }
@@ -804,6 +808,7 @@ static int ulite_probe(struct platform_device *pdev)
 		pdata->clk = NULL;
 	}
 
+	pdata->ulite_uart_driver = &ulite_uart_driver;
 	ret = clk_prepare_enable(pdata->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to prepare clock\n");
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 4/9] serial-uartlite: Add runtime support
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Add runtime support

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 52 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 64c7248..58296eb 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -22,6 +22,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/clk.h>
+#include <linux/pm_runtime.h>
 
 #define ULITE_NAME		"ttyUL"
 #define ULITE_MAJOR		204
@@ -54,6 +55,7 @@
 #define ULITE_CONTROL_RST_TX	0x01
 #define ULITE_CONTROL_RST_RX	0x02
 #define ULITE_CONTROL_IE	0x10
+#define UART_AUTOSUSPEND_TIMEOUT	3000
 
 /* Static pointer to console port */
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
@@ -391,12 +393,12 @@ static int ulite_verify_port(struct uart_port *port, struct serial_struct *ser)
 static void ulite_pm(struct uart_port *port, unsigned int state,
 		     unsigned int oldstate)
 {
-	struct uartlite_data *pdata = port->private_data;
-
-	if (!state)
-		clk_enable(pdata->clk);
-	else
-		clk_disable(pdata->clk);
+	if (!state) {
+		pm_runtime_get_sync(port->dev);
+	} else {
+		pm_runtime_mark_last_busy(port->dev);
+		pm_runtime_put_autosuspend(port->dev);
+	}
 }
 
 #ifdef CONFIG_CONSOLE_POLL
@@ -738,11 +740,32 @@ static int __maybe_unused ulite_resume(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused ulite_runtime_suspend(struct device *dev)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	struct uartlite_data *pdata = port->private_data;
+
+	clk_disable(pdata->clk);
+	return 0;
+};
+
+static int __maybe_unused ulite_runtime_resume(struct device *dev)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	struct uartlite_data *pdata = port->private_data;
+
+	clk_enable(pdata->clk);
+	return 0;
+}
 /* ---------------------------------------------------------------------
  * Platform bus binding
  */
 
-static SIMPLE_DEV_PM_OPS(ulite_pm_ops, ulite_suspend, ulite_resume);
+static const struct dev_pm_ops ulite_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ulite_suspend, ulite_resume)
+	SET_RUNTIME_PM_OPS(ulite_runtime_suspend,
+			   ulite_runtime_resume, NULL)
+};
 
 #if defined(CONFIG_OF)
 /* Match table for of_platform binding */
@@ -815,9 +838,15 @@ static int ulite_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, UART_AUTOSUSPEND_TIMEOUT);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	ret = ulite_assign(&pdev->dev, id, res->start, irq, pdata);
 
-	clk_disable(pdata->clk);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
 
 	return ret;
 }
@@ -826,9 +855,14 @@ static int ulite_remove(struct platform_device *pdev)
 {
 	struct uart_port *port = dev_get_drvdata(&pdev->dev);
 	struct uartlite_data *pdata = port->private_data;
+	int rc;
 
 	clk_disable_unprepare(pdata->clk);
-	return ulite_release(&pdev->dev);
+	rc = ulite_release(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	return rc;
 }
 
 /* work with hotplug and coldplug */
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 5/9] serial-uartlite: Fix the unbind path
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Currently the clocks are not enabled at probe but when the port is used.
Remove the unconditional disable at remove.

Fixes the below
[   77.660196] ------------[ cut here ]------------
[   77.664749] WARNING: CPU: 0 PID: 1992 at drivers/clk/clk.c:622
clk_core_disable+0x78/0x80
[   77.672892] Modules linked in:
[   77.675930] CPU: 0 PID: 1992 Comm: sh Not tainted 4.14.0 #23
[   77.681570] Hardware name: xlnx,zynqmp (DT)
[   77.685736] task: ffffffc879e2e580 task.stack: ffffff800be30000
[   77.691641] PC is at clk_core_disable+0x78/0x80

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 58296eb..4a7989d 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -857,7 +857,7 @@ static int ulite_remove(struct platform_device *pdev)
 	struct uartlite_data *pdata = port->private_data;
 	int rc;
 
-	clk_disable_unprepare(pdata->clk);
+	clk_unprepare(pdata->clk);
 	rc = ulite_release(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 6/9] serial-uartlite: Change logic how console_port is setup
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Change logic how console_port is setup by using CON_ENABLED flag
instead of index. There will be unique uart_console
structure that's why code can't use id for console_port
assignment.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 4a7989d..9c99a1d9 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -665,7 +665,7 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 	 * If register_console() don't assign value, then console_port pointer
 	 * is cleanup.
 	 */
-	if (ulite_uart_driver.cons->index == -1)
+	if (!console_port)
 		console_port = port;
 #endif
 
@@ -680,7 +680,8 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
 	/* This is not port which is used for console that's why clean it up */
-	if (ulite_uart_driver.cons->index == -1)
+	if (console_port == port &&
+	    !(ulite_uart_driver->cons->flags & CON_ENABLED))
 		console_port = NULL;
 #endif
 
@@ -859,6 +860,11 @@ static int ulite_remove(struct platform_device *pdev)
 
 	clk_unprepare(pdata->clk);
 	rc = ulite_release(&pdev->dev);
+#ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
+	if (console_port == port)
+		console_port = NULL;
+#endif
+
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 7/9] serial-uartlite: Use allocated structure instead of static ones
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Remove the use of the static uartlite structure.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 9c99a1d9..20fe11f 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -670,7 +670,7 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 #endif
 
 	/* Register the port */
-	rc = uart_add_one_port(&ulite_uart_driver, port);
+	rc = uart_add_one_port(pdata->ulite_uart_driver, port);
 	if (rc) {
 		dev_err(dev, "uart_add_one_port() failed; err=%i\n", rc);
 		port->mapbase = 0;
@@ -681,7 +681,7 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
 	/* This is not port which is used for console that's why clean it up */
 	if (console_port == port &&
-	    !(ulite_uart_driver->cons->flags & CON_ENABLED))
+	    !(pdata->ulite_uart_driver->cons->flags & CON_ENABLED))
 		console_port = NULL;
 #endif
 
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 8/9] serial-uartlite: Remove ULITE_NR_PORTS macro
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

This patch is removing ULITE_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 ttyULX 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.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2:
Remove the stray unrelated changes
v3:
Fix a compile error

 drivers/tty/serial/uartlite.c | 249 ++++++++++++++++++++++++++++++------------
 1 file changed, 179 insertions(+), 70 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 20fe11f..a118b8e 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -27,7 +27,6 @@
 #define ULITE_NAME		"ttyUL"
 #define ULITE_MAJOR		204
 #define ULITE_MINOR		187
-#define ULITE_NR_UARTS		CONFIG_SERIAL_UARTLITE_NR_UARTS
 
 /* ---------------------------------------------------------------------
  * Register definitions
@@ -65,6 +64,7 @@ static struct uart_port *console_port;
 struct uartlite_data {
 	const struct uartlite_reg_ops *reg_ops;
 	struct clk *clk;
+	int id;
 	struct uart_driver *ulite_uart_driver;
 };
 
@@ -117,7 +117,6 @@ static inline void uart_out32(u32 val, u32 offset, struct uart_port *port)
 	pdata->reg_ops->out(val, port->membase + offset);
 }
 
-static struct uart_port ulite_ports[ULITE_NR_UARTS];
 
 /* ---------------------------------------------------------------------
  * Core UART driver operations
@@ -535,18 +534,6 @@ static int ulite_console_setup(struct console *co, char *options)
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
-static struct uart_driver ulite_uart_driver;
-
-static struct console ulite_console = {
-	.name	= ULITE_NAME,
-	.write	= ulite_console_write,
-	.device	= uart_console_device,
-	.setup	= ulite_console_setup,
-	.flags	= CON_PRINTBUFFER,
-	.index	= -1, /* Specified on the cmdline (e.g. console=ttyUL0 ) */
-	.data	= &ulite_uart_driver,
-};
-
 static void early_uartlite_putc(struct uart_port *port, int c)
 {
 	/*
@@ -590,18 +577,6 @@ OF_EARLYCON_DECLARE(uartlite_a, "xlnx,xps-uartlite-1.00.a", early_uartlite_setup
 
 #endif /* CONFIG_SERIAL_UARTLITE_CONSOLE */
 
-static struct uart_driver ulite_uart_driver = {
-	.owner		= THIS_MODULE,
-	.driver_name	= "uartlite",
-	.dev_name	= ULITE_NAME,
-	.major		= ULITE_MAJOR,
-	.minor		= ULITE_MINOR,
-	.nr		= ULITE_NR_UARTS,
-#ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
-	.cons		= &ulite_console,
-#endif
-};
-
 /* ---------------------------------------------------------------------
  * Port assignment functions (mapping devices to uart_port structures)
  */
@@ -622,24 +597,9 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 	struct uart_port *port;
 	int rc;
 
-	/* if id = -1; then scan for a free id and use that */
-	if (id < 0) {
-		for (id = 0; id < ULITE_NR_UARTS; id++)
-			if (ulite_ports[id].mapbase == 0)
-				break;
-	}
-	if (id < 0 || id >= ULITE_NR_UARTS) {
-		dev_err(dev, "%s%i too large\n", ULITE_NAME, id);
-		return -EINVAL;
-	}
-
-	if ((ulite_ports[id].mapbase) && (ulite_ports[id].mapbase != base)) {
-		dev_err(dev, "cannot assign to %s%i; it is already in use\n",
-			ULITE_NAME, id);
-		return -EBUSY;
-	}
-
-	port = &ulite_ports[id];
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
 
 	spin_lock_init(&port->lock);
 	port->fifosize = 16;
@@ -653,7 +613,6 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
 	port->flags = UPF_BOOT_AUTOCONF;
 	port->dev = dev;
 	port->type = PORT_UNKNOWN;
-	port->line = id;
 	port->private_data = pdata;
 
 	dev_set_drvdata(dev, port);
@@ -778,11 +737,24 @@ static const struct of_device_id ulite_of_match[] = {
 MODULE_DEVICE_TABLE(of, ulite_of_match);
 #endif /* CONFIG_OF */
 
-static int ulite_probe(struct platform_device *pdev)
+/*
+ * Maximum number of instances without alias IDs but if there is alias
+ * which target "< MAX_UART_INSTANCES" range this ID can't be used.
+ */
+#define MAX_UART_INSTANCES	256
+
+/* Stores static aliases list */
+static DECLARE_BITMAP(alias_bitmap, MAX_UART_INSTANCES);
+static int alias_bitmap_initialized;
+
+/* Stores actual bitmap of allocated IDs with alias IDs together */
+static DECLARE_BITMAP(bitmap, MAX_UART_INSTANCES);
+/* Protect bitmap operations to have unique IDs */
+static DEFINE_MUTEX(bitmap_lock);
+
+static int ulite_get_id(struct platform_device *pdev)
 {
-	struct resource *res;
-	struct uartlite_data *pdata;
-	int irq, ret;
+	int ret;
 	int id = pdev->id;
 #ifdef CONFIG_OF
 	const __be32 *prop;
@@ -790,40 +762,158 @@ static int ulite_probe(struct platform_device *pdev)
 	prop = of_get_property(pdev->dev.of_node, "port-number", NULL);
 	if (prop)
 		id = be32_to_cpup(prop);
+
+	/* Alias list is stable that's why get alias bitmap only once */
+	if (!alias_bitmap_initialized) {
+		ret = of_alias_get_alias_list(ulite_of_match, "serial",
+					      alias_bitmap, MAX_UART_INSTANCES);
+		if (ret)
+			return ret;
+
+		alias_bitmap_initialized++;
+	}
+
 #endif
-	if (id < 0) {
-		/* Look for a serialN alias */
+
+	mutex_lock(&bitmap_lock);
+
+	/* Make sure that alias ID is not taken by instance without alias */
+	bitmap_or(bitmap, bitmap, alias_bitmap, MAX_UART_INSTANCES);
+
+	dev_dbg(&pdev->dev, "Alias bitmap: %*pb\n",
+		MAX_UART_INSTANCES, bitmap);
+
+	/* Look for a serialN alias */
+	if (id < 0)
 		id = of_alias_get_id(pdev->dev.of_node, "serial");
-		if (id < 0)
-			id = 0;
-	}
 
-	if (!ulite_uart_driver.state) {
-		dev_dbg(&pdev->dev, "uartlite: calling uart_register_driver()\n");
-		ret = uart_register_driver(&ulite_uart_driver);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "Failed to register driver\n");
-			return ret;
+	if (id < 0) {
+		dev_warn(&pdev->dev,
+			 "No serial alias passed. Using the first free id\n");
+
+		/*
+		 * Start with id 0 and check if there is no serial0 alias
+		 * which points to device which is compatible with this driver.
+		 * If alias exists then try next free position.
+		 */
+		id = 0;
+
+		for (;;) {
+			dev_info(&pdev->dev, "Checking id %d\n", id);
+			id = find_next_zero_bit(bitmap, MAX_UART_INSTANCES, id);
+
+			/* No free empty instance */
+			if (id == MAX_UART_INSTANCES) {
+				dev_err(&pdev->dev, "No free ID\n");
+				mutex_unlock(&bitmap_lock);
+				return -EINVAL;
+			}
+
+			dev_dbg(&pdev->dev, "The empty id is %d\n", id);
+			/* Check if ID is empty */
+			if (!test_and_set_bit(id, bitmap)) {
+				/* Break the loop if bit is taken */
+				dev_dbg(&pdev->dev,
+					"Selected ID %d allocation passed\n",
+					id);
+				break;
+			}
+			dev_dbg(&pdev->dev,
+				"Selected ID %d allocation failed\n", id);
+			/* if taking bit fails then try next one */
+			id++;
 		}
 	}
 
+	mutex_unlock(&bitmap_lock);
+
+	return id;
+}
+
+static int ulite_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct uartlite_data *pdata;
+	int irq, ret;
+	struct uart_driver *ulite_uart_driver;
+	char *driver_name;
+#ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
+	struct console *ulite_console;
+#endif
+
 	pdata = devm_kzalloc(&pdev->dev, sizeof(struct uartlite_data),
 			     GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
+	ulite_uart_driver = devm_kzalloc(&pdev->dev,
+					 sizeof(*ulite_uart_driver),
+					 GFP_KERNEL);
+	if (!ulite_uart_driver)
+		return -ENOMEM;
+
+	pdata->id = ulite_get_id(pdev);
+	if (pdata->id < 0)
+		return pdata->id;
+
+	/* There is a need to use unique driver name */
+	driver_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%d",
+				     ULITE_NAME, pdata->id);
+	if (!driver_name) {
+		ret = -ENOMEM;
+		goto err_out_id;
+	}
+
+	ulite_uart_driver->owner = THIS_MODULE;
+	ulite_uart_driver->driver_name = driver_name;
+	ulite_uart_driver->dev_name = ULITE_NAME;
+	ulite_uart_driver->major = ULITE_MAJOR;
+	ulite_uart_driver->minor = pdata->id;
+	ulite_uart_driver->nr = 1;
+#ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
+	ulite_console = devm_kzalloc(&pdev->dev, sizeof(*ulite_console),
+				     GFP_KERNEL);
+	if (!ulite_console) {
+		ret = -ENOMEM;
+		goto err_out_id;
+	}
+
+	strncpy(ulite_console->name, ULITE_NAME,
+		sizeof(ulite_console->name));
+	ulite_console->index = pdata->id;
+	ulite_console->write = ulite_console_write;
+	ulite_console->device = uart_console_device;
+	ulite_console->setup = ulite_console_setup;
+	ulite_console->flags = CON_PRINTBUFFER;
+	ulite_uart_driver->cons = ulite_console;
+	ulite_console->data = ulite_uart_driver;
+#endif
+
+	dev_dbg(&pdev->dev, "uartlite: calling uart_register_driver()\n");
+	ret = uart_register_driver(ulite_uart_driver);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register driver\n");
+		goto err_out_id;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
+	if (!res) {
+		ret = -ENODEV;
+		goto err_out_unregister_driver;
+	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return -ENXIO;
+	if (irq <= 0) {
+		ret = -ENXIO;
+		goto err_out_unregister_driver;
+	}
 
 	pdata->clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
 	if (IS_ERR(pdata->clk)) {
-		if (PTR_ERR(pdata->clk) != -ENOENT)
-			return PTR_ERR(pdata->clk);
+		if (PTR_ERR(pdata->clk) != -ENOENT) {
+			ret = PTR_ERR(pdata->clk);
+			goto err_out_unregister_driver;
+		}
 
 		/*
 		 * Clock framework support is optional, continue on
@@ -832,11 +922,10 @@ static int ulite_probe(struct platform_device *pdev)
 		pdata->clk = NULL;
 	}
 
-	pdata->ulite_uart_driver = &ulite_uart_driver;
 	ret = clk_prepare_enable(pdata->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to prepare clock\n");
-		return ret;
+		goto err_out_unregister_driver;
 	}
 
 	pm_runtime_use_autosuspend(&pdev->dev);
@@ -844,11 +933,27 @@ static int ulite_probe(struct platform_device *pdev)
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
-	ret = ulite_assign(&pdev->dev, id, res->start, irq, pdata);
+	ulite_uart_driver->tty_driver->name_base = pdata->id;
+	pdata->ulite_uart_driver = ulite_uart_driver;
+	ret = ulite_assign(&pdev->dev, pdata->id, res->start, irq, pdata);
+	if (ret < 0)
+		goto err_out_clk_disable;
 
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
+	return 0;
 
+err_out_clk_disable:
+	clk_disable_unprepare(pdata->clk);
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+err_out_unregister_driver:
+	uart_unregister_driver(pdata->ulite_uart_driver);
+err_out_id:
+	mutex_lock(&bitmap_lock);
+	clear_bit(pdata->id, bitmap);
+	mutex_unlock(&bitmap_lock);
 	return ret;
 }
 
@@ -860,11 +965,16 @@ static int ulite_remove(struct platform_device *pdev)
 
 	clk_unprepare(pdata->clk);
 	rc = ulite_release(&pdev->dev);
+	mutex_lock(&bitmap_lock);
+	clear_bit(pdata->id, bitmap);
+	mutex_unlock(&bitmap_lock);
+
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
 	if (console_port == port)
 		console_port = NULL;
 #endif
 
+	uart_unregister_driver(pdata->ulite_uart_driver);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
@@ -898,7 +1008,6 @@ static int __init ulite_init(void)
 static void __exit ulite_exit(void)
 {
 	platform_driver_unregister(&ulite_platform_driver);
-	uart_unregister_driver(&ulite_uart_driver);
 }
 
 module_init(ulite_init);
-- 
2.1.1

^ permalink raw reply related

* [LINUX PATCHv3 9/9] serial-uartlite: Remove SERIAL_UARTLITE_NR_UARTS
From: shubhrajyoti.datta @ 2018-10-16 10:18 UTC (permalink / raw)
  To: linux-serial, linux-kernel
  Cc: gregkh, jslaby, jacmet, Shubhrajyoti Datta, Michal Simek
In-Reply-To: <1539685088-13465-1-git-send-email-shubhrajyoti.datta@gmail.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Remove SERIAL_UARTLITE_NR_UARTS from Kconfig as it is unused.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/tty/serial/Kconfig | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 32886c3..dc34d43 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -543,15 +543,6 @@ config SERIAL_UARTLITE_CONSOLE
 	  console (the system console is the device which receives all kernel
 	  messages and warnings and which allows logins in single user mode).
 
-config SERIAL_UARTLITE_NR_UARTS
-	int "Maximum number of uartlite serial ports"
-	depends on SERIAL_UARTLITE
-	range 1 256
-	default 1
-	help
-	  Set this to the number of uartlites in your system, or the number
-	  you think you might implement.
-
 config SERIAL_SUNCORE
 	bool
 	depends on SPARC
-- 
2.1.1

^ permalink raw reply related

* Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-16 11:38 UTC (permalink / raw)
  To: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Ivan T. Ivanov, Steven Rostedt (VMware), Kees Cook,
	Joel Fernandes (Google), Geliang Tang, Greg Kroah-Hartman,
	Pramod Gurav, linux-arm-msm, linux-soc, linux-serial,
	linux-kernel, Rajendra Nayak, Vivek Gautam, Sibi Sankar

Hi,

On dragonboard 410c, with "ftrace=function" boot args, the console 
output slows down and board resets without any backtrace as below. This 
is tested on latest kernel and seems to exist even in older kernels as well.

[    2.949164] EINJ: ACPI disabled.
[    3.133001] Serial: 8250/16550 dri
Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic

But with pstore enabled, able to get the below backtrace:

<4>[    2.949164] EINJ: ACPI disabled.
<6>[    3.133001] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
<6>[    3.164097] SuperH (H)SCI(F) driver initialized
<0>[    3.164471] Internal error: synchronous external abort: 96000010 
[#1] PREEMPT SMP
<4>[    3.164479] Modules linked in:
<4>[    3.164495] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 
4.19.0-rc8-00008-ge033b9909fff-dirty #175
<4>[    3.164501] Hardware name: Qualcomm Technologies, Inc. APQ 8016 
SBC (DT)
<4>[    3.164508] pstate: 40000085 (nZcv daIf -PAN -UAO)
<4>[    3.164514] pc : msm_read.isra.2+0x20/0x50
<4>[    3.164520] lr : msm_read.isra.2+0x1c/0x50
<4>[    3.164526] sp : ffff000008033a50
<4>[    3.164531] x29: ffff000008033a50 x28: ffff000009486018
<4>[    3.164548] x27: 0000000000000001 x26: ffff7dfffe7ff070
<4>[    3.164565] x25: 0000000000000034 x24: ffff000009486000
<4>[    3.164582] x23: 0000000000000000 x22: ffff00000978e190
<4>[    3.164599] x21: ffff0000095e8228 x20: 0000000000000034
<4>[    3.164616] x19: ffff7dfffe7ff008 x18: ffffffffffffffff
<4>[    3.164632] x17: 0000000000000000 x16: 0000000000000000
<4>[    3.164649] x15: ffff0000094a96c8 x14: ffff00008978e6bf
<4>[    3.164666] x13: ffff00000978e6cd x12: 0000000000000038
<4>[    3.164683] x11: ffff0000094c6000 x10: 0000000000000c24
<4>[    3.164699] x9 : ffff80003c89b400 x8 : ffff000008033970
<4>[    3.164716] x7 : ffff80000eb04100 x6 : 00000000000af304
<4>[    3.164732] x5 : 0000000000000c40 x4 : ffff80003c06f000
<4>[    3.164750] x3 : ffff80003c89b498 x2 : 0000000000000000
<4>[    3.164766] x1 : ffff80003ca68000 x0 : 0000000000000800
<0>[    3.164785] Process swapper/0 (pid: 1, stack limit = 
0x(____ptrval____))
<4>[    3.164791] Call trace:
<4>[    3.164797]  msm_read.isra.2+0x20/0x50
<4>[    3.164804]  msm_reset_dm_count+0x44/0x80
<4>[    3.164810]  __msm_console_write+0x1c8/0x1d0
<4>[    3.164816]  msm_serial_early_write_dm+0x3c/0x50
<4>[    3.164823]  console_unlock.part.6+0x468/0x528
<4>[    3.164829]  vprintk_emit+0x210/0x218
<4>[    3.164835]  vprintk_default+0x48/0x58
<4>[    3.164841]  vprintk_func+0xf0/0x1c0
<4>[    3.164847]  printk+0x74/0x94
<4>[    3.164853]  sci_init+0x24/0x3c
<4>[    3.164859]  do_one_initcall+0x54/0x248
<4>[    3.164866]  kernel_init_freeable+0x210/0x378
<4>[    3.164872]  kernel_init+0x18/0x118
<4>[    3.164878]  ret_from_fork+0x10/0x1c
<0>[    3.164884] Code: aa1e03e0 8b214273 97e616f7 d503201f (b9400260)

Seems to be issue with the msm serial driver and not ftrace.
Could someone look into it.

One more thing is for pstore dmesg-ramoops, I had to change 
late_initcall to postcore_initcall which brings the question as to why 
we changed to late_initcall?
Simple git blame shows to support crypto compress api, but is it really 
helpful? A lot of boottime issues can be caught with pstore enabled at 
postcore_initcall rather than late_initcall, this backtrace
is just one example. Is there any way we could change this?

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Petr Mladek @ 2018-10-16 11:40 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sergey Senozhatsky, linux-kernel, Steven Rostedt, Daniel Wang,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20181016072719.GB4030@hirez.programming.kicks-ass.net>

On Tue 2018-10-16 09:27:19, Peter Zijlstra wrote:
> On Tue, Oct 16, 2018 at 02:04:26PM +0900, Sergey Senozhatsky wrote:
> > Make printk_safe_enter_irqsave()/etc macros available to the
> > rest of the kernel.
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  include/linux/printk.h      | 40 +++++++++++++++++++++++++++++++++++++
> >  kernel/printk/internal.h    | 37 ----------------------------------
> >  kernel/printk/printk_safe.c |  6 ++++--
> >  3 files changed, 44 insertions(+), 39 deletions(-)
> > 
> > diff --git a/include/linux/printk.h b/include/linux/printk.h
> > index cf3eccfe1543..75f99441fd54 100644
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -157,6 +157,46 @@ static inline void printk_nmi_direct_enter(void) { }
> >  static inline void printk_nmi_direct_exit(void) { }
> >  #endif /* PRINTK_NMI */
> >  
> > +#ifdef CONFIG_PRINTK
> > +extern void printk_safe_enter(void);
> > +extern void printk_safe_exit(void);
> > +
> > +#define printk_safe_enter_irqsave(flags)	\
> > +	do {					\
> > +		local_irq_save(flags);		\
> > +		printk_safe_enter();		\
> > +	} while (0)
> > +
> > +#define printk_safe_exit_irqrestore(flags)	\
> > +	do {					\
> > +		printk_safe_exit();		\
> > +		local_irq_restore(flags);	\
> > +	} while (0)
> > +
> > +#define printk_safe_enter_irq()		\
> > +	do {					\
> > +		local_irq_disable();		\
> > +		printk_safe_enter();		\
> > +	} while (0)
> > +
> > +#define printk_safe_exit_irq()			\
> > +	do {					\
> > +		printk_safe_exit();		\
> > +		local_irq_enable();		\
> > +	} while (0)
> 
> So I really _really_ hate all this. Utterly detest it.
> 
> That whole magic 'safe' printk buffer crap is just that crap.
> 
> Instead of this tinkering around the edges, why don't you make the main
> logbuf a lockless ringbuffer and then delegate the actual printing of
> that buffer to a kthread, except for earlycon, which can do synchronous
> output.

In fact, there is no problem with printk log buffer. This patchset
tries to avoid deadlock caused by console-specific locks
used by console->write() methods.

By other words, we neither need printk_safe or lockless log buffer
to fix this prolem. Instead, we need either printk_deferred context
or lockless consoles.

Best Regards,
Petr

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Greg Kroah-Hartman @ 2018-10-16 11:44 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Ivan T. Ivanov, Steven Rostedt (VMware), Kees Cook,
	Joel Fernandes (Google), Geliang Tang, Pramod Gurav,
	linux-arm-msm, linux-soc, linux-serial, linux-kernel,
	Rajendra Nayak, Vivek Gautam, Sibi Sankar
In-Reply-To: <1cae8f10-55f5-20ce-9105-30af6f88bd6e@codeaurora.org>

On Tue, Oct 16, 2018 at 05:08:25PM +0530, Sai Prakash Ranjan wrote:
> Hi,
> 
> On dragonboard 410c, with "ftrace=function" boot args, the console output
> slows down and board resets without any backtrace as below. This is tested
> on latest kernel and seems to exist even in older kernels as well.
> 
> [    2.949164] EINJ: ACPI disabled.
> [    3.133001] Serial: 8250/16550 dri
> Format: Log Type - Time(microsec) - Message - Optional Info
> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
> 
> But with pstore enabled, able to get the below backtrace:
> 
> <4>[    2.949164] EINJ: ACPI disabled.
> <6>[    3.133001] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> <6>[    3.164097] SuperH (H)SCI(F) driver initialized
> <0>[    3.164471] Internal error: synchronous external abort: 96000010 [#1]
> PREEMPT SMP
> <4>[    3.164479] Modules linked in:
> <4>[    3.164495] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
> 4.19.0-rc8-00008-ge033b9909fff-dirty #175
> <4>[    3.164501] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
> (DT)
> <4>[    3.164508] pstate: 40000085 (nZcv daIf -PAN -UAO)
> <4>[    3.164514] pc : msm_read.isra.2+0x20/0x50
> <4>[    3.164520] lr : msm_read.isra.2+0x1c/0x50
> <4>[    3.164526] sp : ffff000008033a50
> <4>[    3.164531] x29: ffff000008033a50 x28: ffff000009486018
> <4>[    3.164548] x27: 0000000000000001 x26: ffff7dfffe7ff070
> <4>[    3.164565] x25: 0000000000000034 x24: ffff000009486000
> <4>[    3.164582] x23: 0000000000000000 x22: ffff00000978e190
> <4>[    3.164599] x21: ffff0000095e8228 x20: 0000000000000034
> <4>[    3.164616] x19: ffff7dfffe7ff008 x18: ffffffffffffffff
> <4>[    3.164632] x17: 0000000000000000 x16: 0000000000000000
> <4>[    3.164649] x15: ffff0000094a96c8 x14: ffff00008978e6bf
> <4>[    3.164666] x13: ffff00000978e6cd x12: 0000000000000038
> <4>[    3.164683] x11: ffff0000094c6000 x10: 0000000000000c24
> <4>[    3.164699] x9 : ffff80003c89b400 x8 : ffff000008033970
> <4>[    3.164716] x7 : ffff80000eb04100 x6 : 00000000000af304
> <4>[    3.164732] x5 : 0000000000000c40 x4 : ffff80003c06f000
> <4>[    3.164750] x3 : ffff80003c89b498 x2 : 0000000000000000
> <4>[    3.164766] x1 : ffff80003ca68000 x0 : 0000000000000800
> <0>[    3.164785] Process swapper/0 (pid: 1, stack limit =
> 0x(____ptrval____))
> <4>[    3.164791] Call trace:
> <4>[    3.164797]  msm_read.isra.2+0x20/0x50
> <4>[    3.164804]  msm_reset_dm_count+0x44/0x80
> <4>[    3.164810]  __msm_console_write+0x1c8/0x1d0
> <4>[    3.164816]  msm_serial_early_write_dm+0x3c/0x50
> <4>[    3.164823]  console_unlock.part.6+0x468/0x528
> <4>[    3.164829]  vprintk_emit+0x210/0x218
> <4>[    3.164835]  vprintk_default+0x48/0x58
> <4>[    3.164841]  vprintk_func+0xf0/0x1c0
> <4>[    3.164847]  printk+0x74/0x94
> <4>[    3.164853]  sci_init+0x24/0x3c
> <4>[    3.164859]  do_one_initcall+0x54/0x248
> <4>[    3.164866]  kernel_init_freeable+0x210/0x378
> <4>[    3.164872]  kernel_init+0x18/0x118
> <4>[    3.164878]  ret_from_fork+0x10/0x1c
> <0>[    3.164884] Code: aa1e03e0 8b214273 97e616f7 d503201f (b9400260)
> 
> Seems to be issue with the msm serial driver and not ftrace.
> Could someone look into it.

As you have the hardware to reproduce this, and it has always been an
issue, you are the best placed to help resolve this.

Good luck!

greg k-h

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-16 11:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Ivan T. Ivanov, Steven Rostedt (VMware), Kees Cook,
	Joel Fernandes (Google), Geliang Tang, Pramod Gurav,
	linux-arm-msm, linux-soc, linux-serial, linux-kernel,
	Rajendra Nayak, Vivek Gautam, Sibi Sankar
In-Reply-To: <20181016114448.GB21790@kroah.com>

On 10/16/2018 5:14 PM, Greg Kroah-Hartman wrote:
> On Tue, Oct 16, 2018 at 05:08:25PM +0530, Sai Prakash Ranjan wrote:
>> Hi,
>>
>> On dragonboard 410c, with "ftrace=function" boot args, the console output
>> slows down and board resets without any backtrace as below. This is tested
>> on latest kernel and seems to exist even in older kernels as well.
>>
>> [    2.949164] EINJ: ACPI disabled.
>> [    3.133001] Serial: 8250/16550 dri
>> Format: Log Type - Time(microsec) - Message - Optional Info
>> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>
>> But with pstore enabled, able to get the below backtrace:
>>
>> <4>[    2.949164] EINJ: ACPI disabled.
>> <6>[    3.133001] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
>> <6>[    3.164097] SuperH (H)SCI(F) driver initialized
>> <0>[    3.164471] Internal error: synchronous external abort: 96000010 [#1]
>> PREEMPT SMP
>> <4>[    3.164479] Modules linked in:
>> <4>[    3.164495] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
>> 4.19.0-rc8-00008-ge033b9909fff-dirty #175
>> <4>[    3.164501] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
>> (DT)
>> <4>[    3.164508] pstate: 40000085 (nZcv daIf -PAN -UAO)
>> <4>[    3.164514] pc : msm_read.isra.2+0x20/0x50
>> <4>[    3.164520] lr : msm_read.isra.2+0x1c/0x50
>> <4>[    3.164526] sp : ffff000008033a50
>> <4>[    3.164531] x29: ffff000008033a50 x28: ffff000009486018
>> <4>[    3.164548] x27: 0000000000000001 x26: ffff7dfffe7ff070
>> <4>[    3.164565] x25: 0000000000000034 x24: ffff000009486000
>> <4>[    3.164582] x23: 0000000000000000 x22: ffff00000978e190
>> <4>[    3.164599] x21: ffff0000095e8228 x20: 0000000000000034
>> <4>[    3.164616] x19: ffff7dfffe7ff008 x18: ffffffffffffffff
>> <4>[    3.164632] x17: 0000000000000000 x16: 0000000000000000
>> <4>[    3.164649] x15: ffff0000094a96c8 x14: ffff00008978e6bf
>> <4>[    3.164666] x13: ffff00000978e6cd x12: 0000000000000038
>> <4>[    3.164683] x11: ffff0000094c6000 x10: 0000000000000c24
>> <4>[    3.164699] x9 : ffff80003c89b400 x8 : ffff000008033970
>> <4>[    3.164716] x7 : ffff80000eb04100 x6 : 00000000000af304
>> <4>[    3.164732] x5 : 0000000000000c40 x4 : ffff80003c06f000
>> <4>[    3.164750] x3 : ffff80003c89b498 x2 : 0000000000000000
>> <4>[    3.164766] x1 : ffff80003ca68000 x0 : 0000000000000800
>> <0>[    3.164785] Process swapper/0 (pid: 1, stack limit =
>> 0x(____ptrval____))
>> <4>[    3.164791] Call trace:
>> <4>[    3.164797]  msm_read.isra.2+0x20/0x50
>> <4>[    3.164804]  msm_reset_dm_count+0x44/0x80
>> <4>[    3.164810]  __msm_console_write+0x1c8/0x1d0
>> <4>[    3.164816]  msm_serial_early_write_dm+0x3c/0x50
>> <4>[    3.164823]  console_unlock.part.6+0x468/0x528
>> <4>[    3.164829]  vprintk_emit+0x210/0x218
>> <4>[    3.164835]  vprintk_default+0x48/0x58
>> <4>[    3.164841]  vprintk_func+0xf0/0x1c0
>> <4>[    3.164847]  printk+0x74/0x94
>> <4>[    3.164853]  sci_init+0x24/0x3c
>> <4>[    3.164859]  do_one_initcall+0x54/0x248
>> <4>[    3.164866]  kernel_init_freeable+0x210/0x378
>> <4>[    3.164872]  kernel_init+0x18/0x118
>> <4>[    3.164878]  ret_from_fork+0x10/0x1c
>> <0>[    3.164884] Code: aa1e03e0 8b214273 97e616f7 d503201f (b9400260)
>>
>> Seems to be issue with the msm serial driver and not ftrace.
>> Could someone look into it.
> 
> As you have the hardware to reproduce this, and it has always been an
> issue, you are the best placed to help resolve this.
> 

I would definitely test if serial guys could point somewhere, not that 
the backtrace is not pointing :) but I am not aware of this serial driver.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Peter Zijlstra @ 2018-10-16 12:17 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, linux-kernel, Steven Rostedt, Daniel Wang,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20181016114006.6q5atyaitapcwbud@pathway.suse.cz>

On Tue, Oct 16, 2018 at 01:40:06PM +0200, Petr Mladek wrote:
> On Tue 2018-10-16 09:27:19, Peter Zijlstra wrote:

> > Instead of this tinkering around the edges, why don't you make the main
> > logbuf a lockless ringbuffer and then delegate the actual printing of
> > that buffer to a kthread, except for earlycon, which can do synchronous
> > output.
> 
> In fact, there is no problem with printk log buffer. This patchset
> tries to avoid deadlock caused by console-specific locks
> used by console->write() methods.
> 
> By other words, we neither need printk_safe or lockless log buffer
> to fix this prolem. Instead, we need either printk_deferred context
> or lockless consoles.

If you have a lockless buffer and a trailing printk thread, that's
deferred.

And earlycon _should_ be lockless (yes, I know, some suck)

But if you do this deferred nonsense that's currently in printk, then
you risk never seeing the message -- the same problem I have with the
whole printk_safe thing too.

Printing _after_ the fact is horrible for robustness.

^ permalink raw reply

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Sergey Senozhatsky @ 2018-10-16 12:27 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sergey Senozhatsky, linux-kernel, Petr Mladek, Steven Rostedt,
	Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
	Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
	Sergey Senozhatsky
In-Reply-To: <20181016072719.GB4030@hirez.programming.kicks-ass.net>

On (10/16/18 09:27), Peter Zijlstra wrote:
> 
> So I really _really_ hate all this. Utterly detest it.

I learned a new word today - detest. I can haz re-entrant consoles now please?

> That whole magic 'safe' printk buffer crap is just that crap.

No, I don't see it this way; printk_safe is *semi-magic* at best! :)

> Instead of this tinkering around the edges, why don't you make the main
> logbuf a lockless ringbuffer and then delegate the actual printing of
> that buffer to a kthread, except for earlycon, which can do synchronous
> output.

Well, hmm. These are good questions. Let me think.

per-CPU printk_safe _semi-magic_ makes some things simple to handle.
We can't just remove per-CPU buffers and add a wake_up_process() at
the bottom of vprintk_emit(). Because this will deadlock:

  printk()
   wake_up_process()
    try_to_wake_up()
     raw_spin_lock_irqsave()
 <NMI>
      printk()
       wake_up_process()
        try_to_wake_up()
         raw_spin_lock_irqsave()

So we still need some amount of per-CPU printk() semi-magic anyway.

And printk-kthread offloding will not eliminate the need of
printk_deferred(). Which is very hard to use in the right places, like
always; and we don't have any ways to find out that a random innocently
looking printk() may actually be invoked from somewhere deep in the
call-chain with rq lock or pi_lock being locked some 5 frames ago, until
that printk() actually gets invoked and possibly deadlocks the system.

Having "unprotected" wake_up_process() in vprintk_emit(), in fact,
will make the need for printk_deferred() even bigger.

On the other hand, we have wake_up_klogd_work_func() in printk, which
uses irq work, so we, may be, can wakeup printk_kthread from there and,
thus, remove all the external locks from printk()... But I doubt it that
anyone will ever ACK such a patch.

Speaking of lockless logbuf,
logbuf buffer and logbuf_lock are the smallest of the problems printk
currently has. Mainly because of lockless semi-magical printk_safe
buffers. Replacing one lockless buffer with another lockless buffer will
not simplify things in this department. We probably additionally will
have some nasty screw ups, e.g. when NMI printk on CPUA interrupts normal
printk on the same CPUA and now we have mixed in characters; and so on.
per-CPU printk_safe at least keeps us on the safe side in these cases and
looks fairly simple. I also sort of like that logbuf_lock lets us to have
a single static textbuf buffer which we use to vscnprintf() printk messages,
and how printk_safe helps us to get recursive errors/warnings from
vscnprintf(). So printk_safe/printk_nmi things are not _entirely_ crappy,
I guess.

We do, however, have loads of problems with all those dependencies which
come from serial drivers and friends: timekeeping, scheduler (scheduler
is brilliant and cool, but we do have some deadlocks in printk because of
it ;), tty, net, MM, wq, etc. So I generally like the idea of "detached
serial consoles" (that's what I call it). IOW, elimination of the direct
printk -> serial console path. I don't hate the idea of a complete printk
re-work. But some people do, tho. And they have their points. Some people
like the synchronous printk nature and see scheduler dependency as a
"regression".

The current approach - use the existing printk_safe mechanism and
case-by-case, manually, break dependencies which can deadlock us is
a lazy approach, yes; and not very convenient. I'm aware of it.

So, unless I'm missing something, things are not entirely that simple:
- throw away printk_safe semi-magic
- add a lockless logbuf
- add wake_up_process() to vprintk_emit().

It does not completely remove dependency on "external" locks - scheduler,
etc. And as long as we have them - printk can deadlock.

Am I missing something?


Another idea, which I had like a year ago, was to treat printk logbuf
messages in serial consoles the same way they treat uart xmit buffer.
Each console has an IRQ handler, which reads pending messages from xmit
buffer and prints them to the console:

	int serial_foo_irq(...)
	{
		unsigned int max_count = TX_FIFO_DEPTH;

		while (max_count) {
			unsigned int c;

			if (uart_circ_empty(xmit))
				break;

			c = xmit->buf[xmit->tail];
			writel(port, UART_TX, c);
			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
			max_count--;
		}
	}

We can do the same for printk logbuf. Each console can have last_seen_idx.
And read logbuf messages (poll logbuf) starting from that per-console
last_seen_idx in IRQ handler; we don't have to call into scheduler, net,
etc. printk() will simply add messages to logbuf, consoles will poll pending
messages from IRQs handlers; and everyone is happy... And we will do direct
printk -> console_drivers only for early_con or when in panic().

	-ss

^ 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