Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH 12/16] serial: mvebu-uart: support extended port registers layout
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Define the missing register offsets and bit fields for the extended
UART port. Add a second driver data structure filled with its port data,
selected with the right compatible (marvell,armada-3700-uart-ext).

Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/tty/serial/mvebu-uart.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 3e46affa09a8..38b067e0ef0c 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -39,10 +39,13 @@
 
 /* Register Map */
 #define UART_STD_RBR		0x00
+#define UART_EXT_RBR		0x18
 
 #define UART_STD_TSH		0x04
+#define UART_EXT_TSH		0x1C
 
 #define UART_STD_CTRL1		0x08
+#define UART_EXT_CTRL1		0x04
 #define  CTRL_SOFT_RST		BIT(31)
 #define  CTRL_TXFIFO_RST	BIT(15)
 #define  CTRL_RXFIFO_RST	BIT(14)
@@ -55,15 +58,20 @@
 				CTRL_PAR_ERR_INT | CTRL_OVR_ERR_INT)
 
 #define UART_STD_CTRL2		UART_STD_CTRL1
+#define UART_EXT_CTRL2		0x20
 #define  CTRL_STD_TX_RDY_INT	BIT(5)
+#define  CTRL_EXT_TX_RDY_INT	BIT(6)
 #define  CTRL_STD_RX_RDY_INT	BIT(4)
+#define  CTRL_EXT_RX_RDY_INT	BIT(5)
 
 #define UART_STAT		0x0C
 #define  STAT_TX_FIFO_EMP	BIT(13)
 #define  STAT_TX_FIFO_FUL	BIT(11)
 #define  STAT_TX_EMP		BIT(6)
 #define  STAT_STD_TX_RDY	BIT(5)
+#define  STAT_EXT_TX_RDY	BIT(15)
 #define  STAT_STD_RX_RDY	BIT(4)
+#define  STAT_EXT_RX_RDY	BIT(14)
 #define  STAT_BRK_DET		BIT(3)
 #define  STAT_FRM_ERR		BIT(2)
 #define  STAT_PAR_ERR		BIT(1)
@@ -858,12 +866,28 @@ static struct mvebu_uart_driver_data uart_std_driver_data = {
 	.flags.stat_rx_rdy = STAT_STD_RX_RDY,
 };
 
+static struct mvebu_uart_driver_data uart_ext_driver_data = {
+	.is_ext = true,
+	.regs.rbr = UART_EXT_RBR,
+	.regs.tsh = UART_EXT_TSH,
+	.regs.ctrl = UART_EXT_CTRL1,
+	.regs.intr = UART_EXT_CTRL2,
+	.flags.ctrl_tx_rdy_int = CTRL_EXT_TX_RDY_INT,
+	.flags.ctrl_rx_rdy_int = CTRL_EXT_RX_RDY_INT,
+	.flags.stat_tx_rdy = STAT_EXT_TX_RDY,
+	.flags.stat_rx_rdy = STAT_EXT_RX_RDY,
+};
+
 /* Match table for of_platform binding */
 static const struct of_device_id mvebu_uart_of_match[] = {
 	{
 		.compatible = "marvell,armada-3700-uart",
 		.data = (void *)&uart_std_driver_data,
 	},
+	{
+		.compatible = "marvell,armada-3700-uart-ext",
+		.data = (void *)&uart_ext_driver_data,
+	},
 	{}
 };
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 11/16] serial: mvebu-uart: augment the maximum number of ports
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

A3700 boards may have up to two UART ports. Set the new limit to two
maximum UART ports.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index b52cbe8c0558..3e46affa09a8 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -74,7 +74,7 @@
 #define UART_BRDV		0x10
 #define  BRDV_BAUD_MASK         0x3FF
 
-#define MVEBU_NR_UARTS		1
+#define MVEBU_NR_UARTS		2
 
 #define MVEBU_UART_TYPE		"mvebu-uart"
 #define DRIVER_NAME		"mvebu_serial"
-- 
2.11.0


^ permalink raw reply related

* [PATCH 10/16] serial: mvebu-uart: dissociate RX and TX interrupts
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

While the standard UART port can use a single IRQ that 'sums' both RX
and TX interrupts, the extended port cannot and has to use two different
ISR, one for each direction. The standard port also has the hability
to use two separate interrupts (one for each direction).

The logic is then: either there is only one unnamed interrupt on the
standard port and this interrupt must be used for both directions
(this is legacy bindings); or all the interrupts must be described and
named 'uart-sum' (if available), 'uart-rx', 'uart-tx' and two separate
handlers for each direction will be used.

Suggested-by: Allen Yan <yanwei@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 129 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 118 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 46d10209637a..b52cbe8c0558 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -79,7 +79,16 @@
 #define MVEBU_UART_TYPE		"mvebu-uart"
 #define DRIVER_NAME		"mvebu_serial"
 
-/* Register offsets, different depending on the UART */
+enum {
+	/* Either there is only one summed IRQ... */
+	UART_IRQ_SUM = 0,
+	/* ...or there are two separate IRQ for RX and TX */
+	UART_RX_IRQ = 0,
+	UART_TX_IRQ,
+	UART_IRQ_COUNT
+};
+
+/* Diverging register offsets */
 struct uart_regs_layout {
 	unsigned int rbr;
 	unsigned int tsh;
@@ -106,6 +115,8 @@ struct mvebu_uart_driver_data {
 struct mvebu_uart {
 	struct uart_port *port;
 	struct clk *clk;
+	int irq[UART_IRQ_COUNT];
+	unsigned char __iomem *nb;
 	struct mvebu_uart_driver_data *data;
 };
 
@@ -313,9 +324,32 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
 	unsigned int st = readl(port->membase + UART_STAT);
 
 	if (st & (STAT_RX_RDY(port) | STAT_OVR_ERR | STAT_FRM_ERR |
+		  STAT_BRK_DET))
+		mvebu_uart_rx_chars(port, st);
+
+	if (st & STAT_TX_RDY(port))
+		mvebu_uart_tx_chars(port, st);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t mvebu_uart_rx_isr(int irq, void *dev_id)
+{
+	struct uart_port *port = (struct uart_port *)dev_id;
+	unsigned int st = readl(port->membase + UART_STAT);
+
+	if (st & (STAT_RX_RDY(port) | STAT_OVR_ERR | STAT_FRM_ERR |
 			STAT_BRK_DET))
 		mvebu_uart_rx_chars(port, st);
 
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t mvebu_uart_tx_isr(int irq, void *dev_id)
+{
+	struct uart_port *port = (struct uart_port *)dev_id;
+	unsigned int st = readl(port->membase + UART_STAT);
+
 	if (st & STAT_TX_RDY(port))
 		mvebu_uart_tx_chars(port, st);
 
@@ -324,6 +358,7 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
 
 static int mvebu_uart_startup(struct uart_port *port)
 {
+	struct mvebu_uart *mvuart = to_mvuart(port);
 	unsigned int ctl;
 	int ret;
 
@@ -342,11 +377,37 @@ static int mvebu_uart_startup(struct uart_port *port)
 	ctl |= CTRL_RX_RDY_INT(port);
 	writel(ctl, port->membase + UART_INTR(port));
 
-	ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags,
-			  DRIVER_NAME, port);
-	if (ret) {
-		dev_err(port->dev, "failed to request irq\n");
-		return ret;
+	if (!mvuart->irq[UART_TX_IRQ]) {
+		/* Old bindings with just one interrupt (UART0 only) */
+		ret = devm_request_irq(port->dev, mvuart->irq[UART_IRQ_SUM],
+				       mvebu_uart_isr, port->irqflags,
+				       dev_name(port->dev), port);
+		if (ret) {
+			dev_err(port->dev, "unable to request IRQ %d\n",
+				mvuart->irq[UART_IRQ_SUM]);
+			return ret;
+		}
+	} else {
+		/* New bindings with an IRQ for RX and TX (both UART) */
+		ret = devm_request_irq(port->dev, mvuart->irq[UART_RX_IRQ],
+				       mvebu_uart_rx_isr, port->irqflags,
+				       dev_name(port->dev), port);
+		if (ret) {
+			dev_err(port->dev, "unable to request IRQ %d\n",
+				mvuart->irq[UART_RX_IRQ]);
+			return ret;
+		}
+
+		ret = devm_request_irq(port->dev, mvuart->irq[UART_TX_IRQ],
+				       mvebu_uart_tx_isr, port->irqflags,
+				       dev_name(port->dev),
+				       port);
+		if (ret) {
+			dev_err(port->dev, "unable to request IRQ %d\n",
+				mvuart->irq[UART_TX_IRQ]);
+			free_irq(mvuart->irq[UART_RX_IRQ], port);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -354,9 +415,16 @@ static int mvebu_uart_startup(struct uart_port *port)
 
 static void mvebu_uart_shutdown(struct uart_port *port)
 {
+	struct mvebu_uart *mvuart = to_mvuart(port);
+
 	writel(0, port->membase + UART_INTR(port));
 
-	free_irq(port->irq, port);
+	if (!mvuart->irq[UART_TX_IRQ]) {
+		free_irq(mvuart->irq[UART_IRQ_SUM], port);
+	} else {
+		free_irq(mvuart->irq[UART_RX_IRQ], port);
+		free_irq(mvuart->irq[UART_TX_IRQ], port);
+	}
 }
 
 static int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud)
@@ -658,15 +726,15 @@ static const struct of_device_id mvebu_uart_of_match[];
 static int mvebu_uart_probe(struct platform_device *pdev)
 {
 	struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	const struct of_device_id *match = of_match_device(mvebu_uart_of_match,
 							   &pdev->dev);
 	struct uart_port *port;
 	struct mvebu_uart *mvuart;
+	int irq;
 	int ret;
 
-	if (!reg || !irq) {
-		dev_err(&pdev->dev, "no registers/irq defined\n");
+	if (!reg) {
+		dev_err(&pdev->dev, "no registers defined\n");
 		return -EINVAL;
 	}
 
@@ -693,7 +761,12 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	port->flags      = UPF_FIXED_PORT;
 	port->line       = pdev->id;
 
-	port->irq        = irq->start;
+	/*
+	 * IRQ number is not stored in this structure because we may have two of
+	 * them per port (RX and TX). Instead, use the driver UART structure
+	 * array so called ->irq[].
+	 */
+	port->irq        = 0;
 	port->irqflags   = 0;
 	port->mapbase    = reg->start;
 
@@ -728,6 +801,40 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 			port->uartclk = clk_get_rate(mvuart->clk);
 	}
 
+	/* Manage interrupts */
+	memset(mvuart->irq, 0, UART_IRQ_COUNT);
+	if (platform_irq_count(pdev) == 1) {
+		/* Old bindings: no name on the single unamed UART0 IRQ */
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "unable to get UART IRQ\n");
+			return irq;
+		}
+
+		mvuart->irq[UART_IRQ_SUM] = irq;
+	} else {
+		/*
+		 * New bindings: named interrupts (RX, TX) for both UARTS,
+		 * only make use of uart-rx and uart-tx interrupts, do not use
+		 * uart-sum of UART0 port.
+		 */
+		irq = platform_get_irq_byname(pdev, "uart-rx");
+		if (irq < 0) {
+			dev_err(&pdev->dev, "unable to get 'uart-rx' IRQ\n");
+			return irq;
+		}
+
+		mvuart->irq[UART_RX_IRQ] = irq;
+
+		irq = platform_get_irq_byname(pdev, "uart-tx");
+		if (irq < 0) {
+			dev_err(&pdev->dev, "unable to get 'uart-tx' IRQ\n");
+			return irq;
+		}
+
+		mvuart->irq[UART_TX_IRQ] = irq;
+	}
+
 	/* UART Soft Reset*/
 	writel(CTRL_SOFT_RST, port->membase + UART_CTRL(port));
 	udelay(1);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 09/16] serial: mvebu-uart: add TX interrupt trigger for pulse interrupts
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Allen Yan, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

From: Allen Yan <yanwei@marvell.com>

Pulse interrupts (extended UART only) needs a change of state to trigger
the TX interrupt. In addition to enabling the TX_READY_INT_EN flag,
produce a FIFO state change from 'empty' to 'not full'. For this, write
only one data byte in TX start, making the TX FIFO not empty, and wait
for the TX interrupt to continue the transfer.

Signed-off-by: Allen Yan <yanwei@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 67f302748b78..46d10209637a 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -165,8 +165,16 @@ static void mvebu_uart_stop_tx(struct uart_port *port)
 
 static void mvebu_uart_start_tx(struct uart_port *port)
 {
-	unsigned int ctl = readl(port->membase + UART_INTR(port));
+	unsigned int ctl;
+	struct circ_buf *xmit = &port->state->xmit;
 
+	if (IS_EXTENDED(port) && !uart_circ_empty(xmit)) {
+		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		port->icount.tx++;
+	}
+
+	ctl = readl(port->membase + UART_INTR(port));
 	ctl |= CTRL_TX_RDY_INT(port);
 	writel(ctl, port->membase + UART_INTR(port));
 }
-- 
2.11.0


^ permalink raw reply related

* [PATCH 08/16] serial: mvebu-uart: clear state register before IRQ request
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Allen Yan,
	Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

From: Allen Yan <yanwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

When receiving data on RX pin before ->uart_startup() is called, some
error bits in the state register could be set up (like BRK_DET).

This is harmless when using only the standard UART (error bits are
read-only), but may procude an endless loop once in the extended UART
RX interrupt handler (error bits must be cleared).

Clear the status register in ->uart_startup() to avoid this situation.

Signed-off-by: Allen Yan <yanwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/tty/serial/mvebu-uart.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 81a3d2714fd3..67f302748b78 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -322,6 +322,12 @@ static int mvebu_uart_startup(struct uart_port *port)
 	writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
 	       port->membase + UART_CTRL(port));
 	udelay(1);
+
+	/* Clear the error bits of state register before IRQ request */
+	ret = readl(port->membase + UART_STAT);
+	ret |= STAT_BRK_ERR;
+	writel(ret, port->membase + UART_STAT);
+
 	writel(CTRL_BRK_INT, port->membase + UART_CTRL(port));
 
 	ctl = readl(port->membase + UART_INTR(port));
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 07/16] serial: mvebu-uart: add function to change baudrate
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Allen Yan, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

From: Allen Yan <yanwei@marvell.com>

Until now, the first UART port baudrate was set by the bootloader.

Add a function allowing to change the baudrate. Changes may be done
from userspace but also at probe time by the kernel. Use the simplest
method: baudrate divisor.

Works for all UART ports until 230400 baud. To achieve higher baudrates,
software should implement the fractional divisor feature that allows
more accuracy for higher rates.

Signed-off-by: Allen Yan <yanwei@marvell.com>
[<miquel.raynal@free-electrons.com>: changed termios handling]
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 69 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index da756cfec0bb..81a3d2714fd3 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -72,6 +72,7 @@
 				 | STAT_PAR_ERR | STAT_OVR_ERR)
 
 #define UART_BRDV		0x10
+#define  BRDV_BAUD_MASK         0x3FF
 
 #define MVEBU_NR_UARTS		1
 
@@ -344,6 +345,31 @@ static void mvebu_uart_shutdown(struct uart_port *port)
 	free_irq(port->irq, port);
 }
 
+static int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud)
+{
+	struct mvebu_uart *mvuart = to_mvuart(port);
+	unsigned int baud_rate_div;
+	u32 brdv;
+
+	if (IS_ERR(mvuart->clk))
+		return -PTR_ERR(mvuart->clk);
+
+	/*
+	 * The UART clock is divided by the value of the divisor to generate
+	 * UCLK_OUT clock, which is 16 times faster than the baudrate.
+	 * This prescaler can achieve all standard baudrates until 230400.
+	 * Higher baudrates could be achieved for the extended UART by using the
+	 * programmable oversampling stack (also called fractional divisor).
+	 */
+	baud_rate_div = DIV_ROUND_UP(port->uartclk, baud * 16);
+	brdv = readl(port->membase + UART_BRDV);
+	brdv &= ~BRDV_BAUD_MASK;
+	brdv |= baud_rate_div;
+	writel(brdv, port->membase + UART_BRDV);
+
+	return 0;
+}
+
 static void mvebu_uart_set_termios(struct uart_port *port,
 				   struct ktermios *termios,
 				   struct ktermios *old)
@@ -367,11 +393,30 @@ static void mvebu_uart_set_termios(struct uart_port *port,
 	if ((termios->c_cflag & CREAD) == 0)
 		port->ignore_status_mask |= STAT_RX_RDY(port) | STAT_BRK_ERR;
 
-	if (old)
-		tty_termios_copy_hw(termios, old);
+	/*
+	 * Maximum achievable frequency with simple baudrate divisor is 230400.
+	 * Since the error per bit frame would be of more than 15%, achieving
+	 * higher frequencies would require to implement the fractional divisor
+	 * feature.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 0, 230400);
+	if (mvebu_uart_baud_rate_set(port, baud)) {
+		/* No clock available, baudrate cannot be changed */
+		if (old)
+			baud = uart_get_baud_rate(port, old, NULL, 0, 230400);
+	} else {
+		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_update_timeout(port, termios->c_cflag, baud);
+	}
 
-	baud = uart_get_baud_rate(port, termios, old, 0, 460800);
-	uart_update_timeout(port, termios->c_cflag, baud);
+	/* Only the following flag changes are supported */
+	if (old) {
+		termios->c_iflag &= INPCK | IGNPAR;
+		termios->c_iflag |= old->c_iflag & ~(INPCK | IGNPAR);
+		termios->c_cflag &= CREAD | CBAUD;
+		termios->c_cflag |= old->c_cflag & ~(CREAD | CBAUD);
+		termios->c_lflag = old->c_lflag;
+	}
 
 	spin_unlock_irqrestore(&port->lock, flags);
 }
@@ -647,12 +692,28 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	if (!mvuart)
 		return -ENOMEM;
 
+	/* Get controller data depending on the compatible string */
 	mvuart->data = (struct mvebu_uart_driver_data *)match->data;
 	mvuart->port = port;
 
 	port->private_data = mvuart;
 	platform_set_drvdata(pdev, mvuart);
 
+	/* Get fixed clock frequency */
+	mvuart->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(mvuart->clk)) {
+		if (PTR_ERR(mvuart->clk) == -EPROBE_DEFER)
+			return PTR_ERR(mvuart->clk);
+
+		if (IS_EXTENDED(port)) {
+			dev_err(&pdev->dev, "unable to get UART clock\n");
+			return PTR_ERR(mvuart->clk);
+		}
+	} else {
+		if (!clk_prepare_enable(mvuart->clk))
+			port->uartclk = clk_get_rate(mvuart->clk);
+	}
+
 	/* UART Soft Reset*/
 	writel(CTRL_SOFT_RST, port->membase + UART_CTRL(port));
 	udelay(1);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 06/16] serial: mvebu-uart: add soft reset at probe
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Allen Yan, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

From: Allen Yan <yanwei@marvell.com>

The existing UART driver relies on the bootloader to initialize the
port(s). However, the secondary uart port may not be initialized
properly in early boot stage. This patch adds the UART soft reset when
probing, for all ports.

Signed-off-by: Allen Yan <yanwei@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 82438884af1e..da756cfec0bb 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -653,6 +653,11 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	port->private_data = mvuart;
 	platform_set_drvdata(pdev, mvuart);
 
+	/* UART Soft Reset*/
+	writel(CTRL_SOFT_RST, port->membase + UART_CTRL(port));
+	udelay(1);
+	writel(0, port->membase + UART_CTRL(port));
+
 	ret = uart_add_one_port(&mvebu_uart_driver, port);
 	if (ret)
 		return ret;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 05/16] serial: mvebu-uart: use a generic way to access the registers
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

There are two UART ports on Armada3700. The second UART is based on the
first one, plus additional features, but it has a different register
layout (some bit fields are also moved inside the registers).

Clearly separate register offsets and bit fields that differ between the
standard and the extended IP. Access them in a generic way. Rename the
defines with the "STD" prefix for future distinction with "EXT" defines.
Point to these defines in the main driver data structure.

The early console only uses the standard port (not extended).

Suggested-by: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/tty/serial/mvebu-uart.c | 213 ++++++++++++++++++++++++++--------------
 1 file changed, 140 insertions(+), 73 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 25b11ede3a97..82438884af1e 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -38,46 +38,32 @@
 #include <linux/tty_flip.h>
 
 /* Register Map */
-#define UART_RBR		0x00
-#define  RBR_BRK_DET		BIT(15)
-#define  RBR_FRM_ERR_DET	BIT(14)
-#define  RBR_PAR_ERR_DET	BIT(13)
-#define  RBR_OVR_ERR_DET	BIT(12)
+#define UART_STD_RBR		0x00
 
-#define UART_TSH		0x04
+#define UART_STD_TSH		0x04
 
-#define UART_CTRL		0x08
+#define UART_STD_CTRL1		0x08
 #define  CTRL_SOFT_RST		BIT(31)
 #define  CTRL_TXFIFO_RST	BIT(15)
 #define  CTRL_RXFIFO_RST	BIT(14)
-#define  CTRL_ST_MIRR_EN	BIT(13)
-#define  CTRL_LPBK_EN		BIT(12)
 #define  CTRL_SND_BRK_SEQ	BIT(11)
-#define  CTRL_PAR_EN		BIT(10)
-#define  CTRL_TWO_STOP		BIT(9)
-#define  CTRL_TX_HFL_INT	BIT(8)
-#define  CTRL_RX_HFL_INT	BIT(7)
-#define  CTRL_TX_EMP_INT	BIT(6)
-#define  CTRL_TX_RDY_INT	BIT(5)
-#define  CTRL_RX_RDY_INT	BIT(4)
 #define  CTRL_BRK_DET_INT	BIT(3)
 #define  CTRL_FRM_ERR_INT	BIT(2)
 #define  CTRL_PAR_ERR_INT	BIT(1)
 #define  CTRL_OVR_ERR_INT	BIT(0)
-#define  CTRL_RX_INT			(CTRL_RX_RDY_INT | CTRL_BRK_DET_INT |\
-	CTRL_FRM_ERR_INT | CTRL_PAR_ERR_INT | CTRL_OVR_ERR_INT)
+#define  CTRL_BRK_INT		(CTRL_BRK_DET_INT | CTRL_FRM_ERR_INT | \
+				CTRL_PAR_ERR_INT | CTRL_OVR_ERR_INT)
 
-#define UART_STAT		0x0c
+#define UART_STD_CTRL2		UART_STD_CTRL1
+#define  CTRL_STD_TX_RDY_INT	BIT(5)
+#define  CTRL_STD_RX_RDY_INT	BIT(4)
+
+#define UART_STAT		0x0C
 #define  STAT_TX_FIFO_EMP	BIT(13)
-#define  STAT_RX_FIFO_EMP	BIT(12)
 #define  STAT_TX_FIFO_FUL	BIT(11)
-#define  STAT_TX_FIFO_HFL	BIT(10)
-#define  STAT_RX_TOGL		BIT(9)
-#define  STAT_RX_FIFO_FUL	BIT(8)
-#define  STAT_RX_FIFO_HFL	BIT(7)
 #define  STAT_TX_EMP		BIT(6)
-#define  STAT_TX_RDY		BIT(5)
-#define  STAT_RX_RDY		BIT(4)
+#define  STAT_STD_TX_RDY	BIT(5)
+#define  STAT_STD_RX_RDY	BIT(4)
 #define  STAT_BRK_DET		BIT(3)
 #define  STAT_FRM_ERR		BIT(2)
 #define  STAT_PAR_ERR		BIT(1)
@@ -92,13 +78,55 @@
 #define MVEBU_UART_TYPE		"mvebu-uart"
 #define DRIVER_NAME		"mvebu_serial"
 
-static struct uart_port mvebu_uart_ports[MVEBU_NR_UARTS];
+/* Register offsets, different depending on the UART */
+struct uart_regs_layout {
+	unsigned int rbr;
+	unsigned int tsh;
+	unsigned int ctrl;
+	unsigned int intr;
+};
+
+/* Diverging flags */
+struct uart_flags {
+	unsigned int ctrl_tx_rdy_int;
+	unsigned int ctrl_rx_rdy_int;
+	unsigned int stat_tx_rdy;
+	unsigned int stat_rx_rdy;
+};
+
+/* Driver data, a structure for each UART port */
+struct mvebu_uart_driver_data {
+	bool is_ext;
+	struct uart_regs_layout regs;
+	struct uart_flags flags;
+};
 
-struct mvebu_uart_data {
+/* MVEBU UART driver structure */
+struct mvebu_uart {
 	struct uart_port *port;
-	struct clk       *clk;
+	struct clk *clk;
+	struct mvebu_uart_driver_data *data;
 };
 
+static struct mvebu_uart *to_mvuart(struct uart_port *port)
+{
+	return (struct mvebu_uart *)port->private_data;
+}
+
+#define IS_EXTENDED(port) (to_mvuart(port)->data->is_ext)
+
+#define UART_RBR(port) (to_mvuart(port)->data->regs.rbr)
+#define UART_TSH(port) (to_mvuart(port)->data->regs.tsh)
+#define UART_CTRL(port) (to_mvuart(port)->data->regs.ctrl)
+#define UART_INTR(port) (to_mvuart(port)->data->regs.intr)
+
+#define CTRL_TX_RDY_INT(port) (to_mvuart(port)->data->flags.ctrl_tx_rdy_int)
+#define CTRL_RX_RDY_INT(port) (to_mvuart(port)->data->flags.ctrl_rx_rdy_int)
+#define STAT_TX_RDY(port) (to_mvuart(port)->data->flags.stat_tx_rdy)
+#define STAT_RX_RDY(port) (to_mvuart(port)->data->flags.stat_rx_rdy)
+
+static struct uart_port mvebu_uart_ports[MVEBU_NR_UARTS];
+
 /* Core UART Driver Operations */
 static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
 {
@@ -128,26 +156,31 @@ static void mvebu_uart_set_mctrl(struct uart_port *port,
 
 static void mvebu_uart_stop_tx(struct uart_port *port)
 {
-	unsigned int ctl = readl(port->membase + UART_CTRL);
+	unsigned int ctl = readl(port->membase + UART_INTR(port));
 
-	ctl &= ~CTRL_TX_RDY_INT;
-	writel(ctl, port->membase + UART_CTRL);
+	ctl &= ~CTRL_TX_RDY_INT(port);
+	writel(ctl, port->membase + UART_INTR(port));
 }
 
 static void mvebu_uart_start_tx(struct uart_port *port)
 {
-	unsigned int ctl = readl(port->membase + UART_CTRL);
+	unsigned int ctl = readl(port->membase + UART_INTR(port));
 
-	ctl |= CTRL_TX_RDY_INT;
-	writel(ctl, port->membase + UART_CTRL);
+	ctl |= CTRL_TX_RDY_INT(port);
+	writel(ctl, port->membase + UART_INTR(port));
 }
 
 static void mvebu_uart_stop_rx(struct uart_port *port)
 {
-	unsigned int ctl = readl(port->membase + UART_CTRL);
+	unsigned int ctl;
 
-	ctl &= ~CTRL_RX_INT;
-	writel(ctl, port->membase + UART_CTRL);
+	ctl = readl(port->membase + UART_CTRL(port));
+	ctl &= ~CTRL_BRK_INT;
+	writel(ctl, port->membase + UART_CTRL(port));
+
+	ctl = readl(port->membase + UART_INTR(port));
+	ctl &= ~CTRL_RX_RDY_INT(port);
+	writel(ctl, port->membase + UART_INTR(port));
 }
 
 static void mvebu_uart_break_ctl(struct uart_port *port, int brk)
@@ -156,12 +189,12 @@ static void mvebu_uart_break_ctl(struct uart_port *port, int brk)
 	unsigned long flags;
 
 	spin_lock_irqsave(&port->lock, flags);
-	ctl = readl(port->membase + UART_CTRL);
+	ctl = readl(port->membase + UART_CTRL(port));
 	if (brk == -1)
 		ctl |= CTRL_SND_BRK_SEQ;
 	else
 		ctl &= ~CTRL_SND_BRK_SEQ;
-	writel(ctl, port->membase + UART_CTRL);
+	writel(ctl, port->membase + UART_CTRL(port));
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
@@ -172,8 +205,8 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 	char flag = 0;
 
 	do {
-		if (status & STAT_RX_RDY) {
-			ch = readl(port->membase + UART_RBR);
+		if (status & STAT_RX_RDY(port)) {
+			ch = readl(port->membase + UART_RBR(port));
 			ch &= 0xff;
 			flag = TTY_NORMAL;
 			port->icount.rx++;
@@ -199,7 +232,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 			goto ignore_char;
 
 		if (status & port->ignore_status_mask & STAT_PAR_ERR)
-			status &= ~STAT_RX_RDY;
+			status &= ~STAT_RX_RDY(port);
 
 		status &= port->read_status_mask;
 
@@ -208,7 +241,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 
 		status &= ~port->ignore_status_mask;
 
-		if (status & STAT_RX_RDY)
+		if (status & STAT_RX_RDY(port))
 			tty_insert_flip_char(tport, ch, flag);
 
 		if (status & STAT_BRK_DET)
@@ -222,7 +255,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 
 ignore_char:
 		status = readl(port->membase + UART_STAT);
-	} while (status & (STAT_RX_RDY | STAT_BRK_DET));
+	} while (status & (STAT_RX_RDY(port) | STAT_BRK_DET));
 
 	tty_flip_buffer_push(tport);
 }
@@ -234,7 +267,7 @@ static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
 	unsigned int st;
 
 	if (port->x_char) {
-		writel(port->x_char, port->membase + UART_TSH);
+		writel(port->x_char, port->membase + UART_TSH(port));
 		port->icount.tx++;
 		port->x_char = 0;
 		return;
@@ -246,7 +279,7 @@ static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
 	}
 
 	for (count = 0; count < port->fifosize; count++) {
-		writel(xmit->buf[xmit->tail], port->membase + UART_TSH);
+		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
 
@@ -270,10 +303,11 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
 	struct uart_port *port = (struct uart_port *)dev_id;
 	unsigned int st = readl(port->membase + UART_STAT);
 
-	if (st & (STAT_RX_RDY | STAT_OVR_ERR | STAT_FRM_ERR | STAT_BRK_DET))
+	if (st & (STAT_RX_RDY(port) | STAT_OVR_ERR | STAT_FRM_ERR |
+			STAT_BRK_DET))
 		mvebu_uart_rx_chars(port, st);
 
-	if (st & STAT_TX_RDY)
+	if (st & STAT_TX_RDY(port))
 		mvebu_uart_tx_chars(port, st);
 
 	return IRQ_HANDLED;
@@ -281,12 +315,17 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
 
 static int mvebu_uart_startup(struct uart_port *port)
 {
+	unsigned int ctl;
 	int ret;
 
 	writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
-	       port->membase + UART_CTRL);
+	       port->membase + UART_CTRL(port));
 	udelay(1);
-	writel(CTRL_RX_INT, port->membase + UART_CTRL);
+	writel(CTRL_BRK_INT, port->membase + UART_CTRL(port));
+
+	ctl = readl(port->membase + UART_INTR(port));
+	ctl |= CTRL_RX_RDY_INT(port);
+	writel(ctl, port->membase + UART_INTR(port));
 
 	ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags,
 			  DRIVER_NAME, port);
@@ -300,7 +339,7 @@ static int mvebu_uart_startup(struct uart_port *port)
 
 static void mvebu_uart_shutdown(struct uart_port *port)
 {
-	writel(0, port->membase + UART_CTRL);
+	writel(0, port->membase + UART_INTR(port));
 
 	free_irq(port->irq, port);
 }
@@ -314,8 +353,8 @@ static void mvebu_uart_set_termios(struct uart_port *port,
 
 	spin_lock_irqsave(&port->lock, flags);
 
-	port->read_status_mask = STAT_RX_RDY | STAT_OVR_ERR |
-		STAT_TX_RDY | STAT_TX_FIFO_FUL;
+	port->read_status_mask = STAT_RX_RDY(port) | STAT_OVR_ERR |
+		STAT_TX_RDY(port) | STAT_TX_FIFO_FUL;
 
 	if (termios->c_iflag & INPCK)
 		port->read_status_mask |= STAT_FRM_ERR | STAT_PAR_ERR;
@@ -326,7 +365,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
 			STAT_FRM_ERR | STAT_PAR_ERR | STAT_OVR_ERR;
 
 	if ((termios->c_cflag & CREAD) == 0)
-		port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;
+		port->ignore_status_mask |= STAT_RX_RDY(port) | STAT_BRK_ERR;
 
 	if (old)
 		tty_termios_copy_hw(termios, old);
@@ -357,10 +396,10 @@ static int mvebu_uart_get_poll_char(struct uart_port *port)
 {
 	unsigned int st = readl(port->membase + UART_STAT);
 
-	if (!(st & STAT_RX_RDY))
+	if (!(st & STAT_RX_RDY(port)))
 		return NO_POLL_CHAR;
 
-	return readl(port->membase + UART_RBR);
+	return readl(port->membase + UART_RBR(port));
 }
 
 static void mvebu_uart_put_poll_char(struct uart_port *port, unsigned char c)
@@ -376,7 +415,7 @@ static void mvebu_uart_put_poll_char(struct uart_port *port, unsigned char c)
 		udelay(1);
 	}
 
-	writel(c, port->membase + UART_TSH);
+	writel(c, port->membase + UART_TSH(port));
 }
 #endif
 
@@ -414,7 +453,8 @@ static void mvebu_uart_putc(struct uart_port *port, int c)
 			break;
 	}
 
-	writel(c, port->membase + UART_TSH);
+	/* At early stage, DT is not parsed yet, only use UART0 */
+	writel(c, port->membase + UART_STD_TSH);
 
 	for (;;) {
 		st = readl(port->membase + UART_STAT);
@@ -459,7 +499,7 @@ static void wait_for_xmitr(struct uart_port *port)
 static void mvebu_uart_console_putchar(struct uart_port *port, int ch)
 {
 	wait_for_xmitr(port);
-	writel(ch, port->membase + UART_TSH);
+	writel(ch, port->membase + UART_TSH(port));
 }
 
 static void mvebu_uart_console_write(struct console *co, const char *s,
@@ -467,7 +507,7 @@ static void mvebu_uart_console_write(struct console *co, const char *s,
 {
 	struct uart_port *port = &mvebu_uart_ports[co->index];
 	unsigned long flags;
-	unsigned int ier;
+	unsigned int ier, intr, ctl;
 	int locked = 1;
 
 	if (oops_in_progress)
@@ -475,16 +515,23 @@ static void mvebu_uart_console_write(struct console *co, const char *s,
 	else
 		spin_lock_irqsave(&port->lock, flags);
 
-	ier = readl(port->membase + UART_CTRL) &
-		(CTRL_RX_INT | CTRL_TX_RDY_INT);
-	writel(0, port->membase + UART_CTRL);
+	ier = readl(port->membase + UART_CTRL(port)) & CTRL_BRK_INT;
+	intr = readl(port->membase + UART_INTR(port)) &
+		(CTRL_RX_RDY_INT(port) | CTRL_TX_RDY_INT(port));
+	writel(0, port->membase + UART_CTRL(port));
+	writel(0, port->membase + UART_INTR(port));
 
 	uart_console_write(port, s, count, mvebu_uart_console_putchar);
 
 	wait_for_xmitr(port);
 
 	if (ier)
-		writel(ier, port->membase + UART_CTRL);
+		writel(ier, port->membase + UART_CTRL(port));
+
+	if (intr) {
+		ctl = intr | readl(port->membase + UART_INTR(port));
+		writel(ctl, port->membase + UART_INTR(port));
+	}
 
 	if (locked)
 		spin_unlock_irqrestore(&port->lock, flags);
@@ -547,12 +594,16 @@ static struct uart_driver mvebu_uart_driver = {
 #endif
 };
 
+static const struct of_device_id mvebu_uart_of_match[];
+
 static int mvebu_uart_probe(struct platform_device *pdev)
 {
 	struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	const struct of_device_id *match = of_match_device(mvebu_uart_of_match,
+							   &pdev->dev);
 	struct uart_port *port;
-	struct mvebu_uart_data *data;
+	struct mvebu_uart *mvuart;
 	int ret;
 
 	if (!reg || !irq) {
@@ -591,15 +642,16 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	if (IS_ERR(port->membase))
 		return -PTR_ERR(port->membase);
 
-	data = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_uart_data),
-			    GFP_KERNEL);
-	if (!data)
+	mvuart = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_uart),
+			      GFP_KERNEL);
+	if (!mvuart)
 		return -ENOMEM;
 
-	data->port = port;
+	mvuart->data = (struct mvebu_uart_driver_data *)match->data;
+	mvuart->port = port;
 
-	port->private_data = data;
-	platform_set_drvdata(pdev, data);
+	port->private_data = mvuart;
+	platform_set_drvdata(pdev, mvuart);
 
 	ret = uart_add_one_port(&mvebu_uart_driver, port);
 	if (ret)
@@ -607,9 +659,24 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static struct mvebu_uart_driver_data uart_std_driver_data = {
+	.is_ext = false,
+	.regs.rbr = UART_STD_RBR,
+	.regs.tsh = UART_STD_TSH,
+	.regs.ctrl = UART_STD_CTRL1,
+	.regs.intr = UART_STD_CTRL2,
+	.flags.ctrl_tx_rdy_int = CTRL_STD_TX_RDY_INT,
+	.flags.ctrl_rx_rdy_int = CTRL_STD_RX_RDY_INT,
+	.flags.stat_tx_rdy = STAT_STD_TX_RDY,
+	.flags.stat_rx_rdy = STAT_STD_RX_RDY,
+};
+
 /* Match table for of_platform binding */
 static const struct of_device_id mvebu_uart_of_match[] = {
-	{ .compatible = "marvell,armada-3700-uart", },
+	{
+		.compatible = "marvell,armada-3700-uart",
+		.data = (void *)&uart_std_driver_data,
+	},
 	{}
 };
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 04/16] serial: mvebu-uart: support probe of multiple ports
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial, devicetree, linux-arm-kernel, linux-gpio,
	Thomas Petazzoni, Antoine Tenart, Nadav Haklai, Wilson Ding,
	Allen Yan, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal@free-electrons.com>

From: Allen Yan <yanwei@marvell.com>

Until now, the mvebu-uart driver only supported probing a single UART
port. However, some platforms have multiple instances of this UART
controller, and therefore the driver should support multiple ports.

In order to achieve this, we make sure to assign port->line properly,
instead of hardcoding it to zero.

Signed-off-by: Allen Yan <yanwei@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/tty/serial/mvebu-uart.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 7e0a3e9fee15..25b11ede3a97 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -560,7 +560,16 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	port = &mvebu_uart_ports[0];
+	if (pdev->dev.of_node)
+		pdev->id = of_alias_get_id(pdev->dev.of_node, "serial");
+
+	if (pdev->id >= MVEBU_NR_UARTS) {
+		dev_err(&pdev->dev, "cannot have more than %d UART ports\n",
+			MVEBU_NR_UARTS);
+		return -EINVAL;
+	}
+
+	port = &mvebu_uart_ports[pdev->id];
 
 	spin_lock_init(&port->lock);
 
@@ -572,7 +581,7 @@ static int mvebu_uart_probe(struct platform_device *pdev)
 	port->fifosize   = 32;
 	port->iotype     = UPIO_MEM32;
 	port->flags      = UPF_FIXED_PORT;
-	port->line       = 0; /* single port: force line number to  0 */
+	port->line       = pdev->id;
 
 	port->irq        = irq->start;
 	port->irqflags   = 0;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 03/16] serial: mvebu-uart: use driver name when requesting an interrupt
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Yehuda Yitschak,
	Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

From: Yehuda Yitschak <yehuday-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

Use the driver name when requesting an interrupt for consistency.

Avoids possible confusion with DW8250 driver interrupt names in
/proc/interrupts.

Signed-off-by: Yehuda Yitschak <yehuday-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/tty/serial/mvebu-uart.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 45b57c294d13..7e0a3e9fee15 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -90,6 +90,7 @@
 #define MVEBU_NR_UARTS		1
 
 #define MVEBU_UART_TYPE		"mvebu-uart"
+#define DRIVER_NAME		"mvebu_serial"
 
 static struct uart_port mvebu_uart_ports[MVEBU_NR_UARTS];
 
@@ -287,8 +288,8 @@ static int mvebu_uart_startup(struct uart_port *port)
 	udelay(1);
 	writel(CTRL_RX_INT, port->membase + UART_CTRL);
 
-	ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, "serial",
-			  port);
+	ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags,
+			  DRIVER_NAME, port);
 	if (ret) {
 		dev_err(port->dev, "failed to request irq\n");
 		return ret;
@@ -538,7 +539,7 @@ console_initcall(mvebu_uart_console_init);
 
 static struct uart_driver mvebu_uart_driver = {
 	.owner			= THIS_MODULE,
-	.driver_name		= "mvebu_serial",
+	.driver_name		= DRIVER_NAME,
 	.dev_name		= "ttyMV",
 	.nr			= MVEBU_NR_UARTS,
 #ifdef CONFIG_SERIAL_MVEBU_CONSOLE
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 02/16] pinctrl: dt-bindings: Fix A37xx uart2 group name
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Fix a typo in A37xx pin controllers documentation about uart2 pin group.

Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt       | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
index f64060908d5a..c7c088d2dd50 100644
--- a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
@@ -97,8 +97,8 @@ group spi_quad
  - pins 15-16
  - functions spi, gpio
 
-group uart_2
- - pins 9-10
+group uart2
+ - pins 9-10 and 18-19
  - functions uart, gpio
 
 Available groups and functions for the South bridge:
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 01/16] dt-bindings: mvebu-uart: update documentation with extended UART
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Miquel Raynal
In-Reply-To: <20171006101344.15590-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Update the Device Tree binding documentation for the Marvell EBU UART,
in order to allow describing the extended UART IP block, in addition to
the already supported standard UART IP. This requires adding a new
compatible string, the introduction of a clocks property, and extensions
to the interrupts property.

Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../devicetree/bindings/serial/mvebu-uart.txt      | 49 +++++++++++++++++++---
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/mvebu-uart.txt b/Documentation/devicetree/bindings/serial/mvebu-uart.txt
index d37fabe17bd1..3df3a3fab4bb 100644
--- a/Documentation/devicetree/bindings/serial/mvebu-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mvebu-uart.txt
@@ -1,13 +1,52 @@
-* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., Armada-3700)
+* Marvell UART : Non standard UART used in some of Marvell EBU SoCs
+                 e.g., Armada-3700.
 
 Required properties:
-- compatible: "marvell,armada-3700-uart"
+- compatible:
+    - "marvell,armada-3700-uart" for the standard variant of the UART
+      (32 bytes FIFO, no DMA, level interrupts, 8-bit access to the
+      FIFO, baudrate limited to 230400).
+    - "marvell,armada-3700-uart-ext" for the extended variant of the
+      UART (128 bytes FIFO, DMA, front interrupts, 8-bit or 32-bit
+      accesses to the FIFO, baudrate unlimited by the dividers).
 - reg: offset and length of the register set for the device.
-- interrupts: device interrupt
+- clocks: UART reference clock used to derive the baudrate (only
+      mandatory with "marvell,armada-3700-uart-ext" compatible).
+- interrupts:
+    - Must contain three elements for the standard variant of the IP
+      (marvell,armada-3700-uart): "uart-sum", "uart-tx" and "uart-rx",
+      respectively the UART sum interrupt, the UART TX interrupt and
+      UART RX interrupt. A corresponding interrupt-names property must
+      be defined.
+    - Must contain two elements for the extended variant of the IP
+      (marvell,armada-3700-uart-ext): "uart-tx" and "uart-rx",
+      respectively the UART TX interrupt and the UART RX interrupt. A
+      corresponding interrupts-names property must be defined.
+    - For backward compatibility reasons, a single element interrupts
+      property is also supported for the standard variant of the IP,
+      containing only the UART sum interrupt. This form is deprecated
+      and should no longer be used.
 
 Example:
-	serial@12000 {
+	uart0: serial@12000 {
 		compatible = "marvell,armada-3700-uart";
 		reg = <0x12000 0x200>;
-		interrupts = <43>;
+		clocks = <&xtalclk>;
+		interrupts =
+		<GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+		<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+		<GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "uart-sum", "uart-tx", "uart-rx";
+		status = "disabled";
+	};
+
+	uart1: serial@12200 {
+		compatible = "marvell,armada-3700-uart-ext";
+		reg = <0x12200 0x30>;
+		clocks = <&xtalclk>;
+		interrupts =
+		<GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
+		<GIC_SPI 31 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "uart-tx", "uart-rx";
+		status = "disabled";
 	};
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 00/16] Support armada-37xx second UART port
From: Miquel Raynal @ 2017-10-06 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
	Gregory Clement, Sebastian Hesselbarth, Jiri Slaby,
	Catalin Marinas, Will Deacon
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Tenart, Nadav Haklai, Wilson Ding, Miquel Raynal

Hi,

This series adds support for the armada-37xx extended UART port. The new
capabilities of this IP compared to the standard port are the possibility
to use DMA, it has bigger FIFOs (128 bytes instead of 32/64), it is
possible to fill the FIFO byte-per-byte or 4 bytes at a time, it as RTC
and CTS control, it supports transfer count for RX/TX with transfer
finish status and finally it may achieve higher baudrates by the use of
special dividers.

Almost none of these extended features is implemented though. For now,
the extended UART is just supported with the same features as the
standard UART.

This IP is present on the Armada 3720 SoC and is now enabled by the
device tree related patches on the Armada-3720-DB. As adding the node to
enable the second port on Armada-3720-ESPRESSObin could break existing
users by changing the configuration of some pins on the main headers,
only a side note is added on how to do it easily.

- The driver patches should go through Greg KH.
- The DT patches should go through the mvebu maintainers.
- We would also like the DT binding documentation update to go through
  the mvebu maintainers, as it conflicts with another DT change.

Thank you,
Miquel

Allen Yan (5):
  serial: mvebu-uart: support probe of multiple ports
  serial: mvebu-uart: add soft reset at probe
  serial: mvebu-uart: add function to change baudrate
  serial: mvebu-uart: clear state register before IRQ request
  serial: mvebu-uart: add TX interrupt trigger for pulse interrupts

Miquel Raynal (10):
  dt-bindings: mvebu-uart: update documentation with extended UART
  pinctrl: dt-bindings: Fix A37xx uart2 group name
  serial: mvebu-uart: use a generic way to access the registers
  serial: mvebu-uart: dissociate RX and TX interrupts
  serial: mvebu-uart: augment the maximum number of ports
  serial: mvebu-uart: support extended port registers layout
  arm64: dts: marvell: armada-37xx: add UART clock
  arm64: dts: marvell: armada-37xx: add second UART port
  arm64: dts: marvell: armada-3720-db: enable second UART port
  arm64: dts: marvell: armada-3720-espressobin: fill UART nodes

Yehuda Yitschak (1):
  serial: mvebu-uart: use driver name when requesting an interrupt

 .../pinctrl/marvell,armada-37xx-pinctrl.txt        |   4 +-
 .../devicetree/bindings/serial/mvebu-uart.txt      |  49 ++-
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |   9 +-
 .../boot/dts/marvell/armada-3720-espressobin.dts   |   8 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |  19 +-
 drivers/tty/serial/mvebu-uart.c                    | 474 +++++++++++++++++----
 6 files changed, 461 insertions(+), 102 deletions(-)

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/2] ACPI serdev support
From: Frédéric Danis @ 2017-10-06  8:16 UTC (permalink / raw)
  To: Ian W MORRISON, Marcel Holtmann
  Cc: Rob Herring, Sebastian Reichel, Loic Poulain, Johan Hovold,
	Lukas Wunner, Hans de Goede,
	bluez mailin list (linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <25008d7b-db06-49ad-033f-63c0b72d9c34-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Ian,

Le 06/10/2017 à 09:33, Ian W MORRISON a écrit :
> On 10/6/17 2:21 AM, Marcel Holtmann wrote:
>> Hi Fred,
>>
>>> Add ACPI support for serial attached devices.
>>>
>>> Currently, serial devices are not set as enumerated during ACPI scan for SPI
>>> or i2c buses (but not for UART). This should also be done for UART serial
>>> devices.
>>> I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.
>>>
>>> For hci_bcm, this needs Hans de Goede's "Bluetooth: hci_bcm: Add (runtime)pm
>>> support to the serdev drv" patches to work.
>>>
>>> Tested on T100TA with Broadcom BCM2E39.
>>>
>>> Since RFC:
>>>   - Add or reword commit messages
>>>   - Rename *serial_slave* to *serial_bus_slave*
>>>   - Add specific check for Apple in acpi_is_serial_bus_slave(), thanks to
>>>     Lukas Wunner
>>>   - Update comment in acpi_default_enumeration()
>>>   - Remove patch 3 "Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39"
>>>     in favor of patches from Hans de Goede
>> I already applied Hans’ patch series to bluetooth-next tree. And I have no objections or comments for these changes. They should be reviewed and acked by the appropriate maintainers as well, but from my side this looks good.
>>
>> Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
>>
>> What we should discuss on how we get them upstream since besides the Bluetooth subsystem, they cover ACPI and TTY subsystems.
>>
>> Regards
>>
>> Marcel
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> Hi Marcel,
>
> I've tested these two patches along with the nine patches (plus revert patch) from Hans's series but cannot get bluetooth to work on some of my devices using these two patches as is, specifically on MINIX Z83-4 based PCs.
>
> In order for bluetooth to function on these devices I submitted the patch 'Bluetooth: BCM: Add support for MINIX Z83-4 based devices' as they require the trigger type IRQF_TRIGGER_FALLING. With Hans's new patch series I've had to modify the patch which I've resubmitted as version 2. However when I apply the second patch from Fred's series 'ACPI / scan: Fix enumeration for special UART devices' I have found the bluetooth device (BCM2EA4) is no longer enumerated. Specifically I've found that the removal of the check 'if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART)' causes this failure and if I re-add the 'if' statement then the device is loaded. However a 'btattach' is still required for functioning bluetooth.

It seems normal to me that BCM2EA4 is no more enumerated at ACPI level 
as this is moved to serdev.
When removing 'if (ares->data.common_serial_bus.type != 
ACPI_RESOURCE_SERIAL_TYPE_UART)' you stop the serdev module finding the 
Serial UART information. In this case it will not register the device 
and it will fall back to previous behavior needing to use btattach to 
setup Bluetooth.

Can you share:
- btattach you are currently using,
- dmesg with with dynamic debug enabled for serdev and hci_uart modules 
during boot (with Hans's patches, your MINIX Z83-4 patches and mine 
patches).

Regards,

Fred

^ permalink raw reply

* Re: [PATCH 0/2] ACPI serdev support
From: Ian W MORRISON @ 2017-10-06  7:33 UTC (permalink / raw)
  To: Marcel Holtmann, Frédéric Danis
  Cc: Rob Herring, Sebastian Reichel, Loic Poulain, Johan Hovold,
	Lukas Wunner, Hans de Goede,
	bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
	linux-acpi, Greg Kroah-Hartman
In-Reply-To: <A58D9E0F-99FC-4C13-95E8-F60EC61C18B2@holtmann.org>

On 10/6/17 2:21 AM, Marcel Holtmann wrote:
> Hi Fred,
> 
>> Add ACPI support for serial attached devices.
>>
>> Currently, serial devices are not set as enumerated during ACPI scan for SPI
>> or i2c buses (but not for UART). This should also be done for UART serial
>> devices.
>> I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.
>>
>> For hci_bcm, this needs Hans de Goede's "Bluetooth: hci_bcm: Add (runtime)pm
>> support to the serdev drv" patches to work.
>>
>> Tested on T100TA with Broadcom BCM2E39.
>>
>> Since RFC:
>>  - Add or reword commit messages
>>  - Rename *serial_slave* to *serial_bus_slave*
>>  - Add specific check for Apple in acpi_is_serial_bus_slave(), thanks to
>>    Lukas Wunner
>>  - Update comment in acpi_default_enumeration()
>>  - Remove patch 3 "Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39"
>>    in favor of patches from Hans de Goede
> 
> I already applied Hans’ patch series to bluetooth-next tree. And I have no objections or comments for these changes. They should be reviewed and acked by the appropriate maintainers as well, but from my side this looks good.
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> What we should discuss on how we get them upstream since besides the Bluetooth subsystem, they cover ACPI and TTY subsystems.
> 
> Regards
> 
> Marcel
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Hi Marcel,

I've tested these two patches along with the nine patches (plus revert patch) from Hans's series but cannot get bluetooth to work on some of my devices using these two patches as is, specifically on MINIX Z83-4 based PCs. 

In order for bluetooth to function on these devices I submitted the patch 'Bluetooth: BCM: Add support for MINIX Z83-4 based devices' as they require the trigger type IRQF_TRIGGER_FALLING. With Hans's new patch series I've had to modify the patch which I've resubmitted as version 2. However when I apply the second patch from Fred's series 'ACPI / scan: Fix enumeration for special UART devices' I have found the bluetooth device (BCM2EA4) is no longer enumerated. Specifically I've found that the removal of the check 'if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART)' causes this failure and if I re-add the 'if' statement then the device is loaded. However a 'btattach' is still required for functioning bluetooth.

In terms on applying the patches I have some questions. 

Like Hans I too would like one of my patches to be applied to v4.14. A lot of people have been waiting to get bluetooth working on the MINIX Z83-4 family of devices so is it possible to get my original (resend) patch for bluetooth on MINIX Z83-4 based devices applied to v4.14? If so and assuming the nine patches from Hans go into v4.15 I can then send an additional patch to extend his second patch (Bluetooth: hci_bcm: Fix setting of irq trigger type) to delete the extra line '.driver_data = &acpi_active_low,'. 

If the original (resend) version of my patch for MINIX Z83-4 cannot be applied to v4.14 then I would like to request that my second version of my patch for MINIX Z83-4 based devices be applied to bluetooth-next tree so that they are included in v4.15.

Finally I would like to request some help in addressing the issue caused by Fred's 'Fix enumeration for special UART devices' patch so as to prevent a regression.

Regards,
Ian



^ permalink raw reply

* Re: [PATCH 1/3] Arm: dts: stm32: remove extra compatible string for uart
From: Rob Herring @ 2017-10-05 23:32 UTC (permalink / raw)
  To: Vikas Manocha
  Cc: linux-kernel, alexandre.torgue, patrice.chotard,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM,
	Greg Kroah-Hartman, moderated list:ARM/STM32 ARCHITECTURE,
	open list:SERIAL DRIVERS, Mark Rutland, Maxime Coquelin,
	Vinod Koul
In-Reply-To: <1506639114-16913-2-git-send-email-vikas.manocha@st.com>

On Thu, Sep 28, 2017 at 03:51:24PM -0700, Vikas Manocha wrote:
> This patch removes the extra compatibility string "st,stm32-usart" to
> avoid confusion, save some time & space.

I'm confused why you don't need it anymore. I thought the h/w blocks 
were configured differently.

> 
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  Documentation/devicetree/bindings/dma/stm32-dma.txt         |  2 +-
>  Documentation/devicetree/bindings/serial/st,stm32-usart.txt | 10 +++-------
>  2 files changed, 4 insertions(+), 8 deletions(-)

In any case,

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Hans de Goede @ 2017-10-05 17:53 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
	Sebastian Reichel, Rob Herring,
	bluez mailin list (linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <17592E57-3A4B-4386-B809-9EA928D38FDD-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi,

On 05-10-17 17:15, Marcel Holtmann wrote:
> Hi Hans,
> 
>>>> Make the serdev driver use struct bcm_device as its driver data and share
>>>> all the pm / GPIO / IRQ related code paths with the platform driver.
>>>>
>>>> After this commit the 2 drivers are in essence the same and the serdev
>>>> driver interface can be used for all ACPI enumerated HCI UARTs.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
>>>> 1 file changed, 68 insertions(+), 50 deletions(-)
>>> all 9 patches have been applied to bluetooth-next tree.
>>
>> Excellent, thank you!
>>
>> So I guess this means we can also move forward with getting
>> the 2 patches from Frédéric Danis merged ? There is a bit
>> of a bisect-ability problem there, if the acpi pull-req
>> gets merged first then uart attached bcm bt will stop
>> working until the bluetooth subsys is also merged.
>>
>> But I don't think this will impact a lot of users
>> (also given the need for a manual btattach so far),
>> so I don't think this is a big problem... ?
> 
> I wonder if we should just do this as all-in-one change for 4.15 kernel. We surely can get the ACPI changes into 4.15 and the Bluetooth changes as well. Then it should just work.
> 
> So I am leaning to just adding the revert patch into the bluetooth-next tree and then just add the ACPI PnP ID for the GPD device in the same pull request. And if the ACPI changes make it then nobody will know the difference that we switched from USB to UART.

Yes that should work fine.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH 0/2] ACPI serdev support
From: Marcel Holtmann @ 2017-10-05 15:21 UTC (permalink / raw)
  To: Frédéric Danis
  Cc: Rob Herring, Sebastian Reichel, Loic Poulain, Johan Hovold,
	Lukas Wunner, Hans de Goede,
	bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
	linux-acpi, Greg Kroah-Hartman
In-Reply-To: <1507107090-15992-1-git-send-email-frederic.danis.oss@gmail.com>

Hi Fred,

> Add ACPI support for serial attached devices.
> 
> Currently, serial devices are not set as enumerated during ACPI scan for SPI
> or i2c buses (but not for UART). This should also be done for UART serial
> devices.
> I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.
> 
> For hci_bcm, this needs Hans de Goede's "Bluetooth: hci_bcm: Add (runtime)pm
> support to the serdev drv" patches to work.
> 
> Tested on T100TA with Broadcom BCM2E39.
> 
> Since RFC:
>  - Add or reword commit messages
>  - Rename *serial_slave* to *serial_bus_slave*
>  - Add specific check for Apple in acpi_is_serial_bus_slave(), thanks to
>    Lukas Wunner
>  - Update comment in acpi_default_enumeration()
>  - Remove patch 3 "Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39"
>    in favor of patches from Hans de Goede

I already applied Hans’ patch series to bluetooth-next tree. And I have no objections or comments for these changes. They should be reviewed and acked by the appropriate maintainers as well, but from my side this looks good.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

What we should discuss on how we get them upstream since besides the Bluetooth subsystem, they cover ACPI and TTY subsystems.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Marcel Holtmann @ 2017-10-05 15:15 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
	Sebastian Reichel, Rob Herring,
	bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
	linux-acpi
In-Reply-To: <1389d64c-17aa-c6a1-d56f-6c2fe877f247@redhat.com>

Hi Hans,

>>> Make the serdev driver use struct bcm_device as its driver data and share
>>> all the pm / GPIO / IRQ related code paths with the platform driver.
>>> 
>>> After this commit the 2 drivers are in essence the same and the serdev
>>> driver interface can be used for all ACPI enumerated HCI UARTs.
>>> 
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
>>> 1 file changed, 68 insertions(+), 50 deletions(-)
>> all 9 patches have been applied to bluetooth-next tree.
> 
> Excellent, thank you!
> 
> So I guess this means we can also move forward with getting
> the 2 patches from Frédéric Danis merged ? There is a bit
> of a bisect-ability problem there, if the acpi pull-req
> gets merged first then uart attached bcm bt will stop
> working until the bluetooth subsys is also merged.
> 
> But I don't think this will impact a lot of users
> (also given the need for a manual btattach so far),
> so I don't think this is a big problem... ?

I wonder if we should just do this as all-in-one change for 4.15 kernel. We surely can get the ACPI changes into 4.15 and the Bluetooth changes as well. Then it should just work.

So I am leaning to just adding the revert patch into the bluetooth-next tree and then just add the ACPI PnP ID for the GPD device in the same pull request. And if the ACPI changes make it then nobody will know the difference that we switched from USB to UART.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Hans de Goede @ 2017-10-05 11:18 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
	Sebastian Reichel, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <DFCF661F-6FE0-4449-A169-D71ED9282074-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi,

On 05-10-17 12:45, Marcel Holtmann wrote:
> Hi Hans,
> 
>> Make the serdev driver use struct bcm_device as its driver data and share
>> all the pm / GPIO / IRQ related code paths with the platform driver.
>>
>> After this commit the 2 drivers are in essence the same and the serdev
>> driver interface can be used for all ACPI enumerated HCI UARTs.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
>> 1 file changed, 68 insertions(+), 50 deletions(-)
> 
> all 9 patches have been applied to bluetooth-next tree.

Excellent, thank you!

So I guess this means we can also move forward with getting
the 2 patches from Frédéric Danis merged ? There is a bit
of a bisect-ability problem there, if the acpi pull-req
gets merged first then uart attached bcm bt will stop
working until the bluetooth subsys is also merged.

But I don't think this will impact a lot of users
(also given the need for a manual btattach so far),
so I don't think this is a big problem... ?

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Marcel Holtmann @ 2017-10-05 10:45 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
	Sebastian Reichel, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171004184343.7855-9-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Hi Hans,

> Make the serdev driver use struct bcm_device as its driver data and share
> all the pm / GPIO / IRQ related code paths with the platform driver.
> 
> After this commit the 2 drivers are in essence the same and the serdev
> driver interface can be used for all ACPI enumerated HCI UARTs.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
> 1 file changed, 68 insertions(+), 50 deletions(-)

all 9 patches have been applied to bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* Re: [PATCH] serial: sh-sci: Use of_device_get_match_data() helper
From: Simon Horman @ 2017-10-05  9:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-renesas-soc
In-Reply-To: <1507119716-12958-1-git-send-email-geert+renesas@glider.be>

On Wed, Oct 04, 2017 at 02:21:56PM +0200, Geert Uytterhoeven wrote:
> Use the of_device_get_match_data() helper instead of open coding.
> Note that when used with DT, there's always a valid match.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

^ permalink raw reply

* [PATCH] Revert "Bluetooth: btusb: Add workaround for Broadcom devices without product id"
From: Hans de Goede @ 2017-10-05  7:42 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi

Commit 9834e586fa ("Bluetooth: btusb: Add workaround for Broadcom devices
without product id") was added to deal with the BT part of the BCM4356A2
on GPD pocket laptops having an usb vid:pid of 0000:0000.

After another commit to add support for the BCM UART connected BT ACPI-id
BCM2E7E used on the GPD win, it turns out that the BT on the GPD pocket is
connected via both USB and UART. Adding support for the BCM2E7E ACPI-id
causes it to switch to UART mode.

The Windows shipped with the device is using it in UART mode and the
presence of the BCM2E7E ACPI-id combined with the 0 USB vid:pid also
indicates that the BT part was never meant to be used in USB mode.

But not only is there no need to support the USB mode, supporting it
actually is troublesome, because with the workaround to support the all 0
USB vid:pid, BT will just work for end users, until we add support for
the ACPI-id, after which users need to have userspace doing a btattach,
which may be considered a regression.

To avoid this regression when switching things over to UART mode as
intended by the ACPI tables all along, this commit reverts the commit
adding the workaround to support the all 0 USB vid:pid.

This reverts commit 9834e586fa ("Bluetooth: btusb: Add workaround for
Broadcom devices without product id").

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btusb.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7a5c06aaa181..c054d7bce490 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -66,7 +66,6 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BCM2045		0x40000
 #define BTUSB_IFNUM_2		0x80000
 #define BTUSB_CW6622		0x100000
-#define BTUSB_BCM_NO_PRODID	0x200000
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
@@ -171,10 +170,6 @@ static const struct usb_device_id btusb_table[] = {
 	{ USB_VENDOR_AND_INTERFACE_INFO(0x0930, 0xff, 0x01, 0x01),
 	  .driver_info = BTUSB_BCM_PATCHRAM },
 
-	/* Broadcom devices with missing product id */
-	{ USB_DEVICE_AND_INTERFACE_INFO(0x0000, 0x0000, 0xff, 0x01, 0x01),
-	  .driver_info = BTUSB_BCM_PATCHRAM | BTUSB_BCM_NO_PRODID },
-
 	/* Intel Bluetooth USB Bootloader (RAM module) */
 	{ USB_DEVICE(0x8087, 0x0a5a),
 	  .driver_info = BTUSB_INTEL_BOOT | BTUSB_BROKEN_ISOC },
@@ -2909,19 +2904,6 @@ static int btusb_probe(struct usb_interface *intf,
 	if (id->driver_info == BTUSB_IGNORE)
 		return -ENODEV;
 
-	if (id->driver_info & BTUSB_BCM_NO_PRODID) {
-		struct usb_device *udev = interface_to_usbdev(intf);
-
-		/* For the broken Broadcom devices that show 0000:0000
-		 * as USB vendor and product information, check that the
-		 * manufacturer string identifies them as Broadcom based
-		 * devices.
-		 */
-		if (!udev->manufacturer ||
-		    strcmp(udev->manufacturer, "Broadcom Corp"))
-			return -ENODEV;
-	}
-
 	if (id->driver_info & BTUSB_ATH3012) {
 		struct usb_device *udev = interface_to_usbdev(intf);
 
-- 
2.14.2


^ permalink raw reply related

* Re: [PATCH v4 4/8] mtd: nand: atmel: Avoid ECC errors when leaving backup mode
From: Boris Brezillon @ 2017-10-05  7:22 UTC (permalink / raw)
  To: Romain Izard
  Cc: Michael Turquette, Stephen Boyd, Lee Jones, Wenyou Yang, Josh Wu,
	Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
	Cyrille Pitchen, Thierry Reding, Richard Genoud,
	Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Ludovic Desroches,
	Nicolas Ferre, Alexandre Belloni
In-Reply-To: <20170928094627.31017-5-romain.izard.pro@gmail.com>

On Thu, 28 Sep 2017 11:46:23 +0200
Romain Izard <romain.izard.pro@gmail.com> wrote:

> During backup mode, the contents of all registers will be cleared as the
> SoC will be completely powered down. For a product that boots on NAND
> Flash memory, the bootloader will obviously use the related controller
> to read the Flash and correct any detected error in the memory, before
> handling back control to the kernel's resuming entry point.
> 
> But it does not clean the NAND controller registers after use and on its
> side the kernel driver expects the error locator to be powered down and
> in a clean state. Add a resume hook for the PMECC error locator, and
> reset its registers.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Applied.

Thanks,

Boris

> ---
> Changes in v3:
> * keep the PMECC disabled when not in use, and use atmel_pmecc_resume to
>   reset the controller after the bootloader has left it enabled.
> 
> Changes in v4:
> * export atmel_pmecc_reset instead of atmel_pmecc_resume
> * use the correct pointer in atmel_nand_controller_resume
> 
>  drivers/mtd/nand/atmel/nand-controller.c |  3 +++
>  drivers/mtd/nand/atmel/pmecc.c           | 17 +++++++++--------
>  drivers/mtd/nand/atmel/pmecc.h           |  1 +
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
> index f25eca79f4e5..8afcff9a66ea 100644
> --- a/drivers/mtd/nand/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/atmel/nand-controller.c
> @@ -2530,6 +2530,9 @@ static __maybe_unused int atmel_nand_controller_resume(struct device *dev)
>  	struct atmel_nand_controller *nc = dev_get_drvdata(dev);
>  	struct atmel_nand *nand;
>  
> +	if (nc->pmecc)
> +		atmel_pmecc_reset(nc->pmecc);
> +
>  	list_for_each_entry(nand, &nc->chips, node) {
>  		int i;
>  
> diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c
> index 146af8218314..0a3f12141c45 100644
> --- a/drivers/mtd/nand/atmel/pmecc.c
> +++ b/drivers/mtd/nand/atmel/pmecc.c
> @@ -765,6 +765,13 @@ void atmel_pmecc_get_generated_eccbytes(struct atmel_pmecc_user *user,
>  }
>  EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);
>  
> +void atmel_pmecc_reset(struct atmel_pmecc *pmecc)
> +{
> +	writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
> +	writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(atmel_pmecc_reset);
> +
>  int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op)
>  {
>  	struct atmel_pmecc *pmecc = user->pmecc;
> @@ -797,10 +804,7 @@ EXPORT_SYMBOL_GPL(atmel_pmecc_enable);
>  
>  void atmel_pmecc_disable(struct atmel_pmecc_user *user)
>  {
> -	struct atmel_pmecc *pmecc = user->pmecc;
> -
> -	writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
> -	writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
> +	atmel_pmecc_reset(user->pmecc);
>  	mutex_unlock(&user->pmecc->lock);
>  }
>  EXPORT_SYMBOL_GPL(atmel_pmecc_disable);
> @@ -855,10 +859,7 @@ static struct atmel_pmecc *atmel_pmecc_create(struct platform_device *pdev,
>  
>  	/* Disable all interrupts before registering the PMECC handler. */
>  	writel(0xffffffff, pmecc->regs.base + ATMEL_PMECC_IDR);
> -
> -	/* Reset the ECC engine */
> -	writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
> -	writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
> +	atmel_pmecc_reset(pmecc);
>  
>  	return pmecc;
>  }
> diff --git a/drivers/mtd/nand/atmel/pmecc.h b/drivers/mtd/nand/atmel/pmecc.h
> index a8ddbfca2ea5..817e0dd9fd15 100644
> --- a/drivers/mtd/nand/atmel/pmecc.h
> +++ b/drivers/mtd/nand/atmel/pmecc.h
> @@ -61,6 +61,7 @@ atmel_pmecc_create_user(struct atmel_pmecc *pmecc,
>  			struct atmel_pmecc_user_req *req);
>  void atmel_pmecc_destroy_user(struct atmel_pmecc_user *user);
>  
> +void atmel_pmecc_reset(struct atmel_pmecc *pmecc);
>  int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op);
>  void atmel_pmecc_disable(struct atmel_pmecc_user *user);
>  int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user);

^ permalink raw reply

* Re: [PATCH] serial: imx-serial - move DMA buffer configuration to DT
From: Uwe Kleine-König @ 2017-10-04 21:49 UTC (permalink / raw)
  To: Han, Nandor (GE Healthcare)
  Cc: Romain Perier, Greg Kroah-Hartman, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <AM3P101MB0180F7019908E059648488F0E67D0-Irc2Ng3OI610aGNDtyle9VAr667LJVwUiGd9ebBGJoev3QGu/rdwKA@public.gmane.org>

Hello,

On Mon, Oct 02, 2017 at 01:17:41PM +0000, Han, Nandor (GE Healthcare) wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König [mailto:u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> > Sent: 29 June 2017 21:26
> > To: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>; Jiri Slaby
> > <jslaby-IBi9RG/b67k@public.gmane.org>; linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Han, Nandor (GE
> > Healthcare) <nandor.han-JJi787mZWgc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: EXT: Re: [PATCH] serial: imx-serial - move DMA buffer configuration
> > to DT
> > 
> > Hello,
> > 
> > Cc: += devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > 
> > On Wed, Jun 28, 2017 at 12:15:14PM +0200, Romain Perier wrote:
> > > From: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>
> > >
> > > The size of the DMA buffer can affect the delta time between data
> > > being produced and data being consumed. Basically the DMA system will
> > > move data to tty buffer when a) DMA buffer is full b) serial line is idle.
> > > The situation is visible when producer generates data continuously and
> > > there is no possibility for idle line. At this point the DMA buffer is
> > > directly affecting the delta time.
> 
> Hi Uwe,
>    Maybe I can explain a bit better the situation. At least I've tried to explain well enough
> the problem and the fix. :)
> 
> > 
> > This doesn't look like a hw property but a configuration item. Also I don't
> > understand the problematic case. The i.MX is sending continuously and then
> > doesn't receive bytes until the DMA buffer is full?
> 
> Yes
> 
> > What is the DMA buffer?
> > You don't mean the FIFO here, do you?
> > 
> 
> DMA buffer is not HW FIFO. Is the buffer provided by serial driver to DMA to store data.
> 
> > That doesn't sound like a good fix but more like a work around. Which other
> > options did you test to fix your problem?
> > 
> 
> I haven't tried any other, because except using maybe, ioctl I haven't
> got anything better.

My question didn't target where to configure the buffer size instead of
dts. I wonder if it would help to change the fifo watermark limits for
example.

> Our problem is that in our system some serial ports needs to have
> really low data latency, where others trade more bytes over data
> latency. This situation results in a need of beeing able to have
> different DMA buffer size for different ports. 
> 
> How can DMA buffer size affect latency?
> DMA works like this: (To answer to your question DMA buffer is not FIFO)
>  1. Transfer the data from HW FIFO to DMA buffer based on some interrupts (character received, etc)
>  2. Transfer the DMA buffer back to serial port based on some events (buffer full, aging timer, etc)
>  3. Serial port forwards to tty buffer.

BTW In the past I saw the serial core introduce latency, too. Are you
sure that's not your bottle neck?

> Data availability to consumer depends on: DMA buffer size, baud rate
> and communication pattern. By communication patter I'm refering that
> we send data continuoselly (serial line is never idle) or packet by
> packet (serial line is idle in between)
> Example:
>       Baud: 19200 (1Byte = 0.52 ms)
>       DMA buffer size: 100 bytes
>       Communication pattern: continuously 
>       =>  DMA will return data to serial port only when DMA buffer is
>       full, since the communication is continuously. This result in a
>       data latency of 0.52 ms* 100bytes = 52ms. In case the buffer
>       will be 200bytes the letency will be double.
> 
> I agree with you, this is not directly a hw property but a DMA configuration item. 
> But I've found this to be the best way to configure this comparing with using ioctl.
> 
> Let me know if you need more clarification and I would really be open
> to other options that will solve our problem.
> 
> <snip>
> 
> > > +- fsl,dma-size : Indicate the size of the DMA buffer and its periods
> > 
> > This is a sparse description, just from reading that I don't understand what it
> > does.
> > 
> 
> Serial driver configures a circular ring of buffers for DMA. Here we
> can configure the size and the number of buffers.

The problem is: How should a person, who wants to make available a port
on a machine via dts, choose what value to use for fsl,dma-size?

What you want (for a low latency port) is that also small amounts of
data received are passed quickly to the upper layer. The knob you
identified to be available for that is the dma buffer size.

I'd prefer to talk about low latency instead of buffer sizes when
setting parameters for the port. That this influences the buffer size
(and maybe watermark settings) under the hood shouldn't matter for the
person configuring the low latency property.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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