* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
This is the fourth revision of the SBSA UART support series, now
based on the current tty-next branch.
Compared to v3 I addressed some comments Jakub had on v3 (thanks for
the review!). Also since some basic ACPI support for ARM64 is now
upstream, I include the necessary ACPI binding as a new patch 11
on top.
----
The ARM Server Base System Architecture[1] document describes a
generic UART which is a subset of the ARM PL011 UART.
It lacks DMA support, baud rate control and modem status line
control, among other things.
The idea is to move the UART initialization and setup into the
firmware (which does this job today already) and let the kernel just
use the UART for sending and receiving characters.
This patchset integrates support for this UART subset into the
existing PL011 driver - basically by refactoring some
functions and providing a new uart_ops structure for it. It also has
a separate probe function to be not dependent on AMBA/PrimeCell.
It provides a device tree and an ACPI binding.
Beside the obvious effect of code sharing reusing most of the PL011
code has the advantage of not introducing another serial device
prefix, so it can go with ttyAMA, which seems to be pretty common.
As changing the baudrate and other communication parameters is not
specified for the SBSA UART, any userland attempt will be denied by
the kernel. The device-tree provided fixed baud rate will be
reported, so stty for instance prints the right value.
This series is based on Greg's tty-next branch, a git repo can
be found at [2] (branch sbsa-uart/v4).
Patch 1/11 contains a bug fix which applies to the PL011 part also,
it should be considered regardless of the rest of the series.
According to Russell this is still racy, but still better than the
current solution.
Patch 2-7 refactor some PL011 functions by splitting them up into
smaller pieces, so that most of the code can be reused later by the
SBSA part.
Patch 8 and 9 introduce two new properties for the vendor structure,
this is for SBSA functionality which cannot be controlled by
separate uart_ops members only.
Patch 10 then finally drops in the SBSA specific code, by providing
a new uart_ops, vendor struct and probe function for it. Also the new
device tree binding is documented.
Patch 11 adds the necessary ACPI boilerplate.
For testing you should be able to take any hardware which has a PL011
and change the DT to use a "arm,sbsa-uart" compatible string and the
baud rate with the "current-speed" property.
Of course testing with a real SBSA Generic UART is welcomed - as well
as regression testing with any PL011 implementation.
I tested this on a FastModel, a Juno and a Midway machine, both in
PL011 and in (DT-emulated) SBSA mode.
Changelog:
v3 .. v4:
- fixed rebase artifact
- moved memory allocation out of pl011_allocate_port() function
(and consequently renamed that to pl011_find_free_port)
- added ACPI driver binding
v2 .. v3:
- rebased on top of tty-next and Dave's latest PL011 rework
- fixed module build
- removed redundant =NULL members in sbsa_uart_ops
v1 .. v2:
- rebased on top of 4.0-rc1 and Dave's newest PL011 fix [3]
- added mandatory current-speed property and report that to userland
Cheers,
Andre
[1] ARM-DEN-0029 Server Base System Architecture, available (click-
thru...) from http://infocenter.arm.com
[2] http://www.linux-arm.org/git?p=linux-ap.git
git://linux-arm.org/linux-ap.git
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327631.html
Andre Przywara (10):
drivers: PL011: avoid potential unregister_driver call
drivers: PL011: refactor pl011_startup()
drivers: PL011: refactor pl011_shutdown()
drivers: PL011: refactor pl011_set_termios()
drivers: PL011: refactor pl011_probe()
drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
drivers: PL011: move cts_event workaround into separate function
drivers: PL011: allow avoiding UART enabling/disabling
drivers: PL011: allow to supply fixed option string
drivers: PL011: add support for the ARM SBSA generic UART
Graeme Gregory (1):
drivers: PL011: add ACPI probing for SBSA UART
.../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 +
drivers/tty/serial/amba-pl011.c | 547 +++++++++++++++------
2 files changed, 409 insertions(+), 148 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
--
2.3.5
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
This is the fourth revision of the SBSA UART support series, now
based on the current tty-next branch.
Compared to v3 I addressed some comments Jakub had on v3 (thanks for
the review!). Also since some basic ACPI support for ARM64 is now
upstream, I include the necessary ACPI binding as a new patch 11
on top.
----
The ARM Server Base System Architecture[1] document describes a
generic UART which is a subset of the ARM PL011 UART.
It lacks DMA support, baud rate control and modem status line
control, among other things.
The idea is to move the UART initialization and setup into the
firmware (which does this job today already) and let the kernel just
use the UART for sending and receiving characters.
This patchset integrates support for this UART subset into the
existing PL011 driver - basically by refactoring some
functions and providing a new uart_ops structure for it. It also has
a separate probe function to be not dependent on AMBA/PrimeCell.
It provides a device tree and an ACPI binding.
Beside the obvious effect of code sharing reusing most of the PL011
code has the advantage of not introducing another serial device
prefix, so it can go with ttyAMA, which seems to be pretty common.
As changing the baudrate and other communication parameters is not
specified for the SBSA UART, any userland attempt will be denied by
the kernel. The device-tree provided fixed baud rate will be
reported, so stty for instance prints the right value.
This series is based on Greg's tty-next branch, a git repo can
be found at [2] (branch sbsa-uart/v4).
Patch 1/11 contains a bug fix which applies to the PL011 part also,
it should be considered regardless of the rest of the series.
According to Russell this is still racy, but still better than the
current solution.
Patch 2-7 refactor some PL011 functions by splitting them up into
smaller pieces, so that most of the code can be reused later by the
SBSA part.
Patch 8 and 9 introduce two new properties for the vendor structure,
this is for SBSA functionality which cannot be controlled by
separate uart_ops members only.
Patch 10 then finally drops in the SBSA specific code, by providing
a new uart_ops, vendor struct and probe function for it. Also the new
device tree binding is documented.
Patch 11 adds the necessary ACPI boilerplate.
For testing you should be able to take any hardware which has a PL011
and change the DT to use a "arm,sbsa-uart" compatible string and the
baud rate with the "current-speed" property.
Of course testing with a real SBSA Generic UART is welcomed - as well
as regression testing with any PL011 implementation.
I tested this on a FastModel, a Juno and a Midway machine, both in
PL011 and in (DT-emulated) SBSA mode.
Changelog:
v3 .. v4:
- fixed rebase artifact
- moved memory allocation out of pl011_allocate_port() function
(and consequently renamed that to pl011_find_free_port)
- added ACPI driver binding
v2 .. v3:
- rebased on top of tty-next and Dave's latest PL011 rework
- fixed module build
- removed redundant =NULL members in sbsa_uart_ops
v1 .. v2:
- rebased on top of 4.0-rc1 and Dave's newest PL011 fix [3]
- added mandatory current-speed property and report that to userland
Cheers,
Andre
[1] ARM-DEN-0029 Server Base System Architecture, available (click-
thru...) from http://infocenter.arm.com
[2] http://www.linux-arm.org/git?p=linux-ap.git
git://linux-arm.org/linux-ap.git
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327631.html
Andre Przywara (10):
drivers: PL011: avoid potential unregister_driver call
drivers: PL011: refactor pl011_startup()
drivers: PL011: refactor pl011_shutdown()
drivers: PL011: refactor pl011_set_termios()
drivers: PL011: refactor pl011_probe()
drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
drivers: PL011: move cts_event workaround into separate function
drivers: PL011: allow avoiding UART enabling/disabling
drivers: PL011: allow to supply fixed option string
drivers: PL011: add support for the ARM SBSA generic UART
Graeme Gregory (1):
drivers: PL011: add ACPI probing for SBSA UART
.../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 +
drivers/tty/serial/amba-pl011.c | 547 +++++++++++++++------
2 files changed, 409 insertions(+), 148 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
--
2.3.5
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 01/11] drivers: PL011: avoid potential unregister_driver call
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
Although we care about not unregistering the driver if there are
still ports connected during the .remove callback, we do miss this
check in the pl011_probe function. So if the current port allocation
fails, but there are other ports already registered, we will kill
those.
So factor out the port removal into a separate function and use that
in the probe function, too.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index f5bd842..d08951f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2135,6 +2135,24 @@ static int pl011_probe_dt_alias(int index, struct device *dev)
return ret;
}
+/* unregisters the driver also if no more ports are left */
+static void pl011_unregister_port(struct uart_amba_port *uap)
+{
+ int i;
+ bool busy = false;
+
+ for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
+ if (amba_ports[i] == uap)
+ amba_ports[i] = NULL;
+ else if (amba_ports[i])
+ busy = true;
+ }
+ pl011_dma_remove(uap);
+ if (!busy)
+ uart_unregister_driver(&amba_reg);
+}
+
+
static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
{
struct uart_amba_port *uap;
@@ -2200,10 +2218,8 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
}
ret = uart_add_one_port(&amba_reg, &uap->port);
- if (ret) {
- amba_ports[i] = NULL;
- uart_unregister_driver(&amba_reg);
- }
+ if (ret)
+ pl011_unregister_port(uap);
return ret;
}
@@ -2211,20 +2227,9 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
static int pl011_remove(struct amba_device *dev)
{
struct uart_amba_port *uap = amba_get_drvdata(dev);
- bool busy = false;
- int i;
uart_remove_one_port(&amba_reg, &uap->port);
-
- for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
- if (amba_ports[i] == uap)
- amba_ports[i] = NULL;
- else if (amba_ports[i])
- busy = true;
-
- pl011_dma_remove(uap);
- if (!busy)
- uart_unregister_driver(&amba_reg);
+ pl011_unregister_port(uap);
return 0;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 01/11] drivers: PL011: avoid potential unregister_driver call
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Although we care about not unregistering the driver if there are
still ports connected during the .remove callback, we do miss this
check in the pl011_probe function. So if the current port allocation
fails, but there are other ports already registered, we will kill
those.
So factor out the port removal into a separate function and use that
in the probe function, too.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index f5bd842..d08951f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2135,6 +2135,24 @@ static int pl011_probe_dt_alias(int index, struct device *dev)
return ret;
}
+/* unregisters the driver also if no more ports are left */
+static void pl011_unregister_port(struct uart_amba_port *uap)
+{
+ int i;
+ bool busy = false;
+
+ for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
+ if (amba_ports[i] == uap)
+ amba_ports[i] = NULL;
+ else if (amba_ports[i])
+ busy = true;
+ }
+ pl011_dma_remove(uap);
+ if (!busy)
+ uart_unregister_driver(&amba_reg);
+}
+
+
static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
{
struct uart_amba_port *uap;
@@ -2200,10 +2218,8 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
}
ret = uart_add_one_port(&amba_reg, &uap->port);
- if (ret) {
- amba_ports[i] = NULL;
- uart_unregister_driver(&amba_reg);
- }
+ if (ret)
+ pl011_unregister_port(uap);
return ret;
}
@@ -2211,20 +2227,9 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
static int pl011_remove(struct amba_device *dev)
{
struct uart_amba_port *uap = amba_get_drvdata(dev);
- bool busy = false;
- int i;
uart_remove_one_port(&amba_reg, &uap->port);
-
- for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
- if (amba_ports[i] == uap)
- amba_ports[i] = NULL;
- else if (amba_ports[i])
- busy = true;
-
- pl011_dma_remove(uap);
- if (!busy)
- uart_unregister_driver(&amba_reg);
+ pl011_unregister_port(uap);
return 0;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 02/11] drivers: PL011: refactor pl011_startup()
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
Split the pl011_startup() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 48 ++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d08951f..d6a081e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1556,6 +1556,32 @@ static void pl011_write_lcr_h(struct uart_amba_port *uap, unsigned int lcr_h)
}
}
+static int pl011_allocate_irq(struct uart_amba_port *uap)
+{
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+
+ return request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
+}
+
+/*
+ * Enable interrupts, only timeouts when using DMA
+ * if initial RX DMA job failed, start in interrupt mode
+ * as well.
+ */
+static void pl011_enable_interrupts(struct uart_amba_port *uap)
+{
+ spin_lock_irq(&uap->port.lock);
+
+ /* Clear out any spuriously appearing RX interrupts */
+ writew(UART011_RTIS | UART011_RXIS,
+ uap->port.membase + UART011_ICR);
+ uap->im = UART011_RTIM;
+ if (!pl011_dma_rx_running(uap))
+ uap->im |= UART011_RXIM;
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+ spin_unlock_irq(&uap->port.lock);
+}
+
static int pl011_startup(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1567,12 +1593,7 @@ static int pl011_startup(struct uart_port *port)
if (retval)
goto clk_dis;
- writew(uap->im, uap->port.membase + UART011_IMSC);
-
- /*
- * Allocate the IRQ
- */
- retval = request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
+ retval = pl011_allocate_irq(uap);
if (retval)
goto clk_dis;
@@ -1595,20 +1616,7 @@ static int pl011_startup(struct uart_port *port)
/* Startup DMA */
pl011_dma_startup(uap);
- /*
- * Finally, enable interrupts, only timeouts when using DMA
- * if initial RX DMA job failed, start in interrupt mode
- * as well.
- */
- spin_lock_irq(&uap->port.lock);
- /* Clear out any spuriously appearing RX interrupts */
- writew(UART011_RTIS | UART011_RXIS,
- uap->port.membase + UART011_ICR);
- uap->im = UART011_RTIM;
- if (!pl011_dma_rx_running(uap))
- uap->im |= UART011_RXIM;
- writew(uap->im, uap->port.membase + UART011_IMSC);
- spin_unlock_irq(&uap->port.lock);
+ pl011_enable_interrupts(uap);
return 0;
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 02/11] drivers: PL011: refactor pl011_startup()
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Split the pl011_startup() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 48 ++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d08951f..d6a081e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1556,6 +1556,32 @@ static void pl011_write_lcr_h(struct uart_amba_port *uap, unsigned int lcr_h)
}
}
+static int pl011_allocate_irq(struct uart_amba_port *uap)
+{
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+
+ return request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
+}
+
+/*
+ * Enable interrupts, only timeouts when using DMA
+ * if initial RX DMA job failed, start in interrupt mode
+ * as well.
+ */
+static void pl011_enable_interrupts(struct uart_amba_port *uap)
+{
+ spin_lock_irq(&uap->port.lock);
+
+ /* Clear out any spuriously appearing RX interrupts */
+ writew(UART011_RTIS | UART011_RXIS,
+ uap->port.membase + UART011_ICR);
+ uap->im = UART011_RTIM;
+ if (!pl011_dma_rx_running(uap))
+ uap->im |= UART011_RXIM;
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+ spin_unlock_irq(&uap->port.lock);
+}
+
static int pl011_startup(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1567,12 +1593,7 @@ static int pl011_startup(struct uart_port *port)
if (retval)
goto clk_dis;
- writew(uap->im, uap->port.membase + UART011_IMSC);
-
- /*
- * Allocate the IRQ
- */
- retval = request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
+ retval = pl011_allocate_irq(uap);
if (retval)
goto clk_dis;
@@ -1595,20 +1616,7 @@ static int pl011_startup(struct uart_port *port)
/* Startup DMA */
pl011_dma_startup(uap);
- /*
- * Finally, enable interrupts, only timeouts when using DMA
- * if initial RX DMA job failed, start in interrupt mode
- * as well.
- */
- spin_lock_irq(&uap->port.lock);
- /* Clear out any spuriously appearing RX interrupts */
- writew(UART011_RTIS | UART011_RXIS,
- uap->port.membase + UART011_ICR);
- uap->im = UART011_RTIM;
- if (!pl011_dma_rx_running(uap))
- uap->im |= UART011_RXIM;
- writew(uap->im, uap->port.membase + UART011_IMSC);
- spin_unlock_irq(&uap->port.lock);
+ pl011_enable_interrupts(uap);
return 0;
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 03/11] drivers: PL011: refactor pl011_shutdown()
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
Split the pl011_shutdown() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 57 +++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d6a081e..722a7ce 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1635,34 +1635,15 @@ static void pl011_shutdown_channel(struct uart_amba_port *uap,
writew(val, uap->port.membase + lcrh);
}
-static void pl011_shutdown(struct uart_port *port)
+/*
+ * disable the port. It should not disable RTS and DTR.
+ * Also RTS and DTR state should be preserved to restore
+ * it during startup().
+ */
+static void pl011_disable_uart(struct uart_amba_port *uap)
{
- struct uart_amba_port *uap =
- container_of(port, struct uart_amba_port, port);
unsigned int cr;
- /*
- * disable all interrupts
- */
- spin_lock_irq(&uap->port.lock);
- uap->im = 0;
- writew(uap->im, uap->port.membase + UART011_IMSC);
- writew(0xffff, uap->port.membase + UART011_ICR);
- spin_unlock_irq(&uap->port.lock);
-
- pl011_dma_shutdown(uap);
-
- /*
- * Free the interrupt
- */
- free_irq(uap->port.irq, uap);
-
- /*
- * disable the port
- * disable the port. It should not disable RTS and DTR.
- * Also RTS and DTR state should be preserved to restore
- * it during startup().
- */
uap->autorts = false;
spin_lock_irq(&uap->port.lock);
cr = readw(uap->port.membase + UART011_CR);
@@ -1678,6 +1659,32 @@ static void pl011_shutdown(struct uart_port *port)
pl011_shutdown_channel(uap, uap->lcrh_rx);
if (uap->lcrh_rx != uap->lcrh_tx)
pl011_shutdown_channel(uap, uap->lcrh_tx);
+}
+
+static void pl011_disable_interrupts(struct uart_amba_port *uap)
+{
+ spin_lock_irq(&uap->port.lock);
+
+ /* mask all interrupts and clear all pending ones */
+ uap->im = 0;
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
+
+ spin_unlock_irq(&uap->port.lock);
+}
+
+static void pl011_shutdown(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ pl011_disable_interrupts(uap);
+
+ pl011_dma_shutdown(uap);
+
+ free_irq(uap->port.irq, uap);
+
+ pl011_disable_uart(uap);
/*
* Shut down the clock producer
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 03/11] drivers: PL011: refactor pl011_shutdown()
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Split the pl011_shutdown() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 57 +++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d6a081e..722a7ce 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1635,34 +1635,15 @@ static void pl011_shutdown_channel(struct uart_amba_port *uap,
writew(val, uap->port.membase + lcrh);
}
-static void pl011_shutdown(struct uart_port *port)
+/*
+ * disable the port. It should not disable RTS and DTR.
+ * Also RTS and DTR state should be preserved to restore
+ * it during startup().
+ */
+static void pl011_disable_uart(struct uart_amba_port *uap)
{
- struct uart_amba_port *uap =
- container_of(port, struct uart_amba_port, port);
unsigned int cr;
- /*
- * disable all interrupts
- */
- spin_lock_irq(&uap->port.lock);
- uap->im = 0;
- writew(uap->im, uap->port.membase + UART011_IMSC);
- writew(0xffff, uap->port.membase + UART011_ICR);
- spin_unlock_irq(&uap->port.lock);
-
- pl011_dma_shutdown(uap);
-
- /*
- * Free the interrupt
- */
- free_irq(uap->port.irq, uap);
-
- /*
- * disable the port
- * disable the port. It should not disable RTS and DTR.
- * Also RTS and DTR state should be preserved to restore
- * it during startup().
- */
uap->autorts = false;
spin_lock_irq(&uap->port.lock);
cr = readw(uap->port.membase + UART011_CR);
@@ -1678,6 +1659,32 @@ static void pl011_shutdown(struct uart_port *port)
pl011_shutdown_channel(uap, uap->lcrh_rx);
if (uap->lcrh_rx != uap->lcrh_tx)
pl011_shutdown_channel(uap, uap->lcrh_tx);
+}
+
+static void pl011_disable_interrupts(struct uart_amba_port *uap)
+{
+ spin_lock_irq(&uap->port.lock);
+
+ /* mask all interrupts and clear all pending ones */
+ uap->im = 0;
+ writew(uap->im, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
+
+ spin_unlock_irq(&uap->port.lock);
+}
+
+static void pl011_shutdown(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ pl011_disable_interrupts(uap);
+
+ pl011_dma_shutdown(uap);
+
+ free_irq(uap->port.irq, uap);
+
+ pl011_disable_uart(uap);
/*
* Shut down the clock producer
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 04/11] drivers: PL011: refactor pl011_set_termios()
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
Split the pl011_set_termios() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 60 ++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 722a7ce..f465c1e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1706,6 +1706,38 @@ static void pl011_shutdown(struct uart_port *port)
}
static void
+pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
+{
+ port->read_status_mask = UART011_DR_OE | 255;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= UART011_DR_FE | UART011_DR_PE;
+ if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
+ port->read_status_mask |= UART011_DR_BE;
+
+ /*
+ * Characters to ignore
+ */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= UART011_DR_FE | UART011_DR_PE;
+ if (termios->c_iflag & IGNBRK) {
+ port->ignore_status_mask |= UART011_DR_BE;
+ /*
+ * If we're ignoring parity and break indicators,
+ * ignore overruns too (for real raw support).
+ */
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= UART011_DR_OE;
+ }
+
+ /*
+ * Ignore all characters if CREAD is not set.
+ */
+ if ((termios->c_cflag & CREAD) == 0)
+ port->ignore_status_mask |= UART_DUMMY_DR_RX;
+}
+
+static void
pl011_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
@@ -1769,33 +1801,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
*/
uart_update_timeout(port, termios->c_cflag, baud);
- port->read_status_mask = UART011_DR_OE | 255;
- if (termios->c_iflag & INPCK)
- port->read_status_mask |= UART011_DR_FE | UART011_DR_PE;
- if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
- port->read_status_mask |= UART011_DR_BE;
-
- /*
- * Characters to ignore
- */
- port->ignore_status_mask = 0;
- if (termios->c_iflag & IGNPAR)
- port->ignore_status_mask |= UART011_DR_FE | UART011_DR_PE;
- if (termios->c_iflag & IGNBRK) {
- port->ignore_status_mask |= UART011_DR_BE;
- /*
- * If we're ignoring parity and break indicators,
- * ignore overruns too (for real raw support).
- */
- if (termios->c_iflag & IGNPAR)
- port->ignore_status_mask |= UART011_DR_OE;
- }
-
- /*
- * Ignore all characters if CREAD is not set.
- */
- if ((termios->c_cflag & CREAD) == 0)
- port->ignore_status_mask |= UART_DUMMY_DR_RX;
+ pl011_setup_status_masks(port, termios);
if (UART_ENABLE_MS(port, termios->c_cflag))
pl011_enable_ms(port);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 04/11] drivers: PL011: refactor pl011_set_termios()
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Split the pl011_set_termios() function into smaller chunks to allow
easier reuse later when adding SBSA support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 60 ++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 722a7ce..f465c1e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1706,6 +1706,38 @@ static void pl011_shutdown(struct uart_port *port)
}
static void
+pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
+{
+ port->read_status_mask = UART011_DR_OE | 255;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= UART011_DR_FE | UART011_DR_PE;
+ if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
+ port->read_status_mask |= UART011_DR_BE;
+
+ /*
+ * Characters to ignore
+ */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= UART011_DR_FE | UART011_DR_PE;
+ if (termios->c_iflag & IGNBRK) {
+ port->ignore_status_mask |= UART011_DR_BE;
+ /*
+ * If we're ignoring parity and break indicators,
+ * ignore overruns too (for real raw support).
+ */
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= UART011_DR_OE;
+ }
+
+ /*
+ * Ignore all characters if CREAD is not set.
+ */
+ if ((termios->c_cflag & CREAD) == 0)
+ port->ignore_status_mask |= UART_DUMMY_DR_RX;
+}
+
+static void
pl011_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
@@ -1769,33 +1801,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
*/
uart_update_timeout(port, termios->c_cflag, baud);
- port->read_status_mask = UART011_DR_OE | 255;
- if (termios->c_iflag & INPCK)
- port->read_status_mask |= UART011_DR_FE | UART011_DR_PE;
- if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
- port->read_status_mask |= UART011_DR_BE;
-
- /*
- * Characters to ignore
- */
- port->ignore_status_mask = 0;
- if (termios->c_iflag & IGNPAR)
- port->ignore_status_mask |= UART011_DR_FE | UART011_DR_PE;
- if (termios->c_iflag & IGNBRK) {
- port->ignore_status_mask |= UART011_DR_BE;
- /*
- * If we're ignoring parity and break indicators,
- * ignore overruns too (for real raw support).
- */
- if (termios->c_iflag & IGNPAR)
- port->ignore_status_mask |= UART011_DR_OE;
- }
-
- /*
- * Ignore all characters if CREAD is not set.
- */
- if ((termios->c_cflag & CREAD) == 0)
- port->ignore_status_mask |= UART_DUMMY_DR_RX;
+ pl011_setup_status_masks(port, termios);
if (UART_ENABLE_MS(port, termios->c_cflag))
pl011_enable_ms(port);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 05/11] drivers: PL011: refactor pl011_probe()
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
Currently the pl011_probe() function is relying on some AMBA IDs
and a device tree node to initialize the driver and a port.
Both features are not necessarily required for the driver:
- we lack AMBA IDs in the ARM SBSA generic UART and
- we lack a DT node in ACPI systems.
So lets refactor the function to ease later reuse.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 97 ++++++++++++++++++++++++++---------------
1 file changed, 61 insertions(+), 36 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index f465c1e..6709c6c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2173,66 +2173,54 @@ static void pl011_unregister_port(struct uart_amba_port *uap)
uart_unregister_driver(&amba_reg);
}
-
-static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
+static int pl011_find_free_port(void)
{
- struct uart_amba_port *uap;
- struct vendor_data *vendor = id->data;
- void __iomem *base;
- int i, ret;
+ int i;
for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
if (amba_ports[i] == NULL)
- break;
-
- if (i == ARRAY_SIZE(amba_ports))
- return -EBUSY;
+ return i;
- uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
- GFP_KERNEL);
- if (uap == NULL)
- return -ENOMEM;
+ return -EBUSY;
+}
- i = pl011_probe_dt_alias(i, &dev->dev);
+static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
+ struct resource *mmiobase, int index)
+{
+ void __iomem *base;
- base = devm_ioremap(&dev->dev, dev->res.start,
- resource_size(&dev->res));
+ base = devm_ioremap_resource(dev, mmiobase);
if (!base)
return -ENOMEM;
- uap->clk = devm_clk_get(&dev->dev, NULL);
- if (IS_ERR(uap->clk))
- return PTR_ERR(uap->clk);
+ index = pl011_probe_dt_alias(index, dev);
- uap->vendor = vendor;
- uap->lcrh_rx = vendor->lcrh_rx;
- uap->lcrh_tx = vendor->lcrh_tx;
uap->old_cr = 0;
- uap->fifosize = vendor->get_fifosize(dev);
- uap->port.dev = &dev->dev;
- uap->port.mapbase = dev->res.start;
+ uap->port.dev = dev;
+ uap->port.mapbase = mmiobase->start;
uap->port.membase = base;
uap->port.iotype = UPIO_MEM;
- uap->port.irq = dev->irq[0];
uap->port.fifosize = uap->fifosize;
- uap->port.ops = &amba_pl011_pops;
uap->port.flags = UPF_BOOT_AUTOCONF;
- uap->port.line = i;
+ uap->port.line = index;
- /* Ensure interrupts from this UART are masked and cleared */
- writew(0, uap->port.membase + UART011_IMSC);
- writew(0xffff, uap->port.membase + UART011_ICR);
+ amba_ports[index] = uap;
- snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
+ return 0;
+}
- amba_ports[i] = uap;
+static int pl011_register_port(struct uart_amba_port *uap)
+{
+ int ret;
- amba_set_drvdata(dev, uap);
+ /* Ensure interrupts from this UART are masked and cleared */
+ writew(0, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
if (!amba_reg.state) {
ret = uart_register_driver(&amba_reg);
if (ret < 0) {
- dev_err(&dev->dev,
+ dev_err(uap->port.dev,
"Failed to register AMBA-PL011 driver\n");
return ret;
}
@@ -2245,6 +2233,43 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
return ret;
}
+static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
+{
+ struct uart_amba_port *uap;
+ struct vendor_data *vendor = id->data;
+ int portnr, ret;
+
+ portnr = pl011_find_free_port();
+ if (portnr < 0)
+ return portnr;
+
+ uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
+ GFP_KERNEL);
+ if (uap == NULL)
+ return -ENOMEM;
+
+ uap->clk = devm_clk_get(&dev->dev, NULL);
+ if (IS_ERR(uap->clk))
+ return PTR_ERR(uap->clk);
+
+ uap->vendor = vendor;
+ uap->lcrh_rx = vendor->lcrh_rx;
+ uap->lcrh_tx = vendor->lcrh_tx;
+ uap->fifosize = vendor->get_fifosize(dev);
+ uap->port.irq = dev->irq[0];
+ uap->port.ops = &amba_pl011_pops;
+
+ snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
+
+ ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr);
+ if (ret)
+ return ret;
+
+ amba_set_drvdata(dev, uap);
+
+ return pl011_register_port(uap);
+}
+
static int pl011_remove(struct amba_device *dev)
{
struct uart_amba_port *uap = amba_get_drvdata(dev);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 05/11] drivers: PL011: refactor pl011_probe()
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
Currently the pl011_probe() function is relying on some AMBA IDs
and a device tree node to initialize the driver and a port.
Both features are not necessarily required for the driver:
- we lack AMBA IDs in the ARM SBSA generic UART and
- we lack a DT node in ACPI systems.
So lets refactor the function to ease later reuse.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 97 ++++++++++++++++++++++++++---------------
1 file changed, 61 insertions(+), 36 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index f465c1e..6709c6c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2173,66 +2173,54 @@ static void pl011_unregister_port(struct uart_amba_port *uap)
uart_unregister_driver(&amba_reg);
}
-
-static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
+static int pl011_find_free_port(void)
{
- struct uart_amba_port *uap;
- struct vendor_data *vendor = id->data;
- void __iomem *base;
- int i, ret;
+ int i;
for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
if (amba_ports[i] == NULL)
- break;
-
- if (i == ARRAY_SIZE(amba_ports))
- return -EBUSY;
+ return i;
- uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
- GFP_KERNEL);
- if (uap == NULL)
- return -ENOMEM;
+ return -EBUSY;
+}
- i = pl011_probe_dt_alias(i, &dev->dev);
+static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
+ struct resource *mmiobase, int index)
+{
+ void __iomem *base;
- base = devm_ioremap(&dev->dev, dev->res.start,
- resource_size(&dev->res));
+ base = devm_ioremap_resource(dev, mmiobase);
if (!base)
return -ENOMEM;
- uap->clk = devm_clk_get(&dev->dev, NULL);
- if (IS_ERR(uap->clk))
- return PTR_ERR(uap->clk);
+ index = pl011_probe_dt_alias(index, dev);
- uap->vendor = vendor;
- uap->lcrh_rx = vendor->lcrh_rx;
- uap->lcrh_tx = vendor->lcrh_tx;
uap->old_cr = 0;
- uap->fifosize = vendor->get_fifosize(dev);
- uap->port.dev = &dev->dev;
- uap->port.mapbase = dev->res.start;
+ uap->port.dev = dev;
+ uap->port.mapbase = mmiobase->start;
uap->port.membase = base;
uap->port.iotype = UPIO_MEM;
- uap->port.irq = dev->irq[0];
uap->port.fifosize = uap->fifosize;
- uap->port.ops = &amba_pl011_pops;
uap->port.flags = UPF_BOOT_AUTOCONF;
- uap->port.line = i;
+ uap->port.line = index;
- /* Ensure interrupts from this UART are masked and cleared */
- writew(0, uap->port.membase + UART011_IMSC);
- writew(0xffff, uap->port.membase + UART011_ICR);
+ amba_ports[index] = uap;
- snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
+ return 0;
+}
- amba_ports[i] = uap;
+static int pl011_register_port(struct uart_amba_port *uap)
+{
+ int ret;
- amba_set_drvdata(dev, uap);
+ /* Ensure interrupts from this UART are masked and cleared */
+ writew(0, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
if (!amba_reg.state) {
ret = uart_register_driver(&amba_reg);
if (ret < 0) {
- dev_err(&dev->dev,
+ dev_err(uap->port.dev,
"Failed to register AMBA-PL011 driver\n");
return ret;
}
@@ -2245,6 +2233,43 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
return ret;
}
+static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
+{
+ struct uart_amba_port *uap;
+ struct vendor_data *vendor = id->data;
+ int portnr, ret;
+
+ portnr = pl011_find_free_port();
+ if (portnr < 0)
+ return portnr;
+
+ uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
+ GFP_KERNEL);
+ if (uap == NULL)
+ return -ENOMEM;
+
+ uap->clk = devm_clk_get(&dev->dev, NULL);
+ if (IS_ERR(uap->clk))
+ return PTR_ERR(uap->clk);
+
+ uap->vendor = vendor;
+ uap->lcrh_rx = vendor->lcrh_rx;
+ uap->lcrh_tx = vendor->lcrh_tx;
+ uap->fifosize = vendor->get_fifosize(dev);
+ uap->port.irq = dev->irq[0];
+ uap->port.ops = &amba_pl011_pops;
+
+ snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
+
+ ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr);
+ if (ret)
+ return ret;
+
+ amba_set_drvdata(dev, uap);
+
+ return pl011_register_port(uap);
+}
+
static int pl011_remove(struct amba_device *dev)
{
struct uart_amba_port *uap = amba_get_drvdata(dev);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 06/11] drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
The PL011 register UART_MIS is actually a bitwise AND of the
UART_RIS and the UART_MISC register.
Since the SBSA UART does not include the _MIS register, use the
two separate registers to get the same behaviour. Since we are
inside the spinlock and we read the _IMSC register only once, there
should be no race issue.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6709c6c..52cc2f5 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1322,11 +1322,13 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
struct uart_amba_port *uap = dev_id;
unsigned long flags;
unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
+ u16 imsc;
int handled = 0;
unsigned int dummy_read;
spin_lock_irqsave(&uap->port.lock, flags);
- status = readw(uap->port.membase + UART011_MIS);
+ imsc = readw(uap->port.membase + UART011_IMSC);
+ status = readw(uap->port.membase + UART011_RIS) & imsc;
if (status) {
do {
if (uap->vendor->cts_event_workaround) {
@@ -1361,7 +1363,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
if (pass_counter-- == 0)
break;
- status = readw(uap->port.membase + UART011_MIS);
+ status = readw(uap->port.membase + UART011_RIS) & imsc;
} while (status != 0);
handled = 1;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 06/11] drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
The PL011 register UART_MIS is actually a bitwise AND of the
UART_RIS and the UART_MISC register.
Since the SBSA UART does not include the _MIS register, use the
two separate registers to get the same behaviour. Since we are
inside the spinlock and we read the _IMSC register only once, there
should be no race issue.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6709c6c..52cc2f5 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1322,11 +1322,13 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
struct uart_amba_port *uap = dev_id;
unsigned long flags;
unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
+ u16 imsc;
int handled = 0;
unsigned int dummy_read;
spin_lock_irqsave(&uap->port.lock, flags);
- status = readw(uap->port.membase + UART011_MIS);
+ imsc = readw(uap->port.membase + UART011_IMSC);
+ status = readw(uap->port.membase + UART011_RIS) & imsc;
if (status) {
do {
if (uap->vendor->cts_event_workaround) {
@@ -1361,7 +1363,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
if (pass_counter-- == 0)
break;
- status = readw(uap->port.membase + UART011_MIS);
+ status = readw(uap->port.membase + UART011_RIS) & imsc;
} while (status != 0);
handled = 1;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 07/11] drivers: PL011: move cts_event workaround into separate function
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
To avoid lines with more than 80 characters and to make the
pl011_int() function more readable, move the workaround out into a
separate function.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 52cc2f5..d764533 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1317,6 +1317,25 @@ static void pl011_modem_status(struct uart_amba_port *uap)
wake_up_interruptible(&uap->port.state->port.delta_msr_wait);
}
+static void check_apply_cts_event_workaround(struct uart_amba_port *uap)
+{
+ unsigned int dummy_read;
+
+ if (!uap->vendor->cts_event_workaround)
+ return;
+
+ /* workaround to make sure that all bits are unlocked.. */
+ writew(0x00, uap->port.membase + UART011_ICR);
+
+ /*
+ * WA: introduce 26ns(1 uart clk) delay before W1C;
+ * single apb access will incur 2 pclk(133.12Mhz) delay,
+ * so add 2 dummy reads
+ */
+ dummy_read = readw(uap->port.membase + UART011_ICR);
+ dummy_read = readw(uap->port.membase + UART011_ICR);
+}
+
static irqreturn_t pl011_int(int irq, void *dev_id)
{
struct uart_amba_port *uap = dev_id;
@@ -1324,25 +1343,13 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
u16 imsc;
int handled = 0;
- unsigned int dummy_read;
spin_lock_irqsave(&uap->port.lock, flags);
imsc = readw(uap->port.membase + UART011_IMSC);
status = readw(uap->port.membase + UART011_RIS) & imsc;
if (status) {
do {
- if (uap->vendor->cts_event_workaround) {
- /* workaround to make sure that all bits are unlocked.. */
- writew(0x00, uap->port.membase + UART011_ICR);
-
- /*
- * WA: introduce 26ns(1 uart clk) delay before W1C;
- * single apb access will incur 2 pclk(133.12Mhz) delay,
- * so add 2 dummy reads
- */
- dummy_read = readw(uap->port.membase + UART011_ICR);
- dummy_read = readw(uap->port.membase + UART011_ICR);
- }
+ check_apply_cts_event_workaround(uap);
writew(status & ~(UART011_TXIS|UART011_RTIS|
UART011_RXIS),
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 07/11] drivers: PL011: move cts_event workaround into separate function
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
To avoid lines with more than 80 characters and to make the
pl011_int() function more readable, move the workaround out into a
separate function.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 52cc2f5..d764533 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1317,6 +1317,25 @@ static void pl011_modem_status(struct uart_amba_port *uap)
wake_up_interruptible(&uap->port.state->port.delta_msr_wait);
}
+static void check_apply_cts_event_workaround(struct uart_amba_port *uap)
+{
+ unsigned int dummy_read;
+
+ if (!uap->vendor->cts_event_workaround)
+ return;
+
+ /* workaround to make sure that all bits are unlocked.. */
+ writew(0x00, uap->port.membase + UART011_ICR);
+
+ /*
+ * WA: introduce 26ns(1 uart clk) delay before W1C;
+ * single apb access will incur 2 pclk(133.12Mhz) delay,
+ * so add 2 dummy reads
+ */
+ dummy_read = readw(uap->port.membase + UART011_ICR);
+ dummy_read = readw(uap->port.membase + UART011_ICR);
+}
+
static irqreturn_t pl011_int(int irq, void *dev_id)
{
struct uart_amba_port *uap = dev_id;
@@ -1324,25 +1343,13 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
u16 imsc;
int handled = 0;
- unsigned int dummy_read;
spin_lock_irqsave(&uap->port.lock, flags);
imsc = readw(uap->port.membase + UART011_IMSC);
status = readw(uap->port.membase + UART011_RIS) & imsc;
if (status) {
do {
- if (uap->vendor->cts_event_workaround) {
- /* workaround to make sure that all bits are unlocked.. */
- writew(0x00, uap->port.membase + UART011_ICR);
-
- /*
- * WA: introduce 26ns(1 uart clk) delay before W1C;
- * single apb access will incur 2 pclk(133.12Mhz) delay,
- * so add 2 dummy reads
- */
- dummy_read = readw(uap->port.membase + UART011_ICR);
- dummy_read = readw(uap->port.membase + UART011_ICR);
- }
+ check_apply_cts_event_workaround(uap);
writew(status & ~(UART011_TXIS|UART011_RTIS|
UART011_RXIS),
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 08/11] drivers: PL011: allow avoiding UART enabling/disabling
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
The SBSA UART should not be enabled or disabled (it is always on),
and consequently the spec lacks the UART_CR register.
Add a vendor specific property to skip disabling or enabling of the
UART. This will be used later by the SBSA UART support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d764533..fe92289 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -78,6 +78,7 @@ struct vendor_data {
bool oversampling;
bool dma_threshold;
bool cts_event_workaround;
+ bool always_enabled;
unsigned int (*get_fifosize)(struct amba_device *dev);
};
@@ -94,6 +95,7 @@ static struct vendor_data vendor_arm = {
.oversampling = false,
.dma_threshold = false,
.cts_event_workaround = false,
+ .always_enabled = false,
.get_fifosize = get_fifosize_arm,
};
@@ -109,6 +111,7 @@ static struct vendor_data vendor_st = {
.oversampling = true,
.dma_threshold = true,
.cts_event_workaround = true,
+ .always_enabled = false,
.get_fifosize = get_fifosize_st,
};
@@ -1958,7 +1961,7 @@ static void
pl011_console_write(struct console *co, const char *s, unsigned int count)
{
struct uart_amba_port *uap = amba_ports[co->index];
- unsigned int status, old_cr, new_cr;
+ unsigned int status, old_cr = 0, new_cr;
unsigned long flags;
int locked = 1;
@@ -1975,10 +1978,12 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
/*
* First save the CR then disable the interrupts
*/
- old_cr = readw(uap->port.membase + UART011_CR);
- new_cr = old_cr & ~UART011_CR_CTSEN;
- new_cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
- writew(new_cr, uap->port.membase + UART011_CR);
+ if (!uap->vendor->always_enabled) {
+ old_cr = readw(uap->port.membase + UART011_CR);
+ new_cr = old_cr & ~UART011_CR_CTSEN;
+ new_cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
+ writew(new_cr, uap->port.membase + UART011_CR);
+ }
uart_console_write(&uap->port, s, count, pl011_console_putchar);
@@ -1989,7 +1994,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
do {
status = readw(uap->port.membase + UART01x_FR);
} while (status & UART01x_FR_BUSY);
- writew(old_cr, uap->port.membase + UART011_CR);
+ if (!uap->vendor->always_enabled)
+ writew(old_cr, uap->port.membase + UART011_CR);
if (locked)
spin_unlock(&uap->port.lock);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 08/11] drivers: PL011: allow avoiding UART enabling/disabling
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
The SBSA UART should not be enabled or disabled (it is always on),
and consequently the spec lacks the UART_CR register.
Add a vendor specific property to skip disabling or enabling of the
UART. This will be used later by the SBSA UART support.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d764533..fe92289 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -78,6 +78,7 @@ struct vendor_data {
bool oversampling;
bool dma_threshold;
bool cts_event_workaround;
+ bool always_enabled;
unsigned int (*get_fifosize)(struct amba_device *dev);
};
@@ -94,6 +95,7 @@ static struct vendor_data vendor_arm = {
.oversampling = false,
.dma_threshold = false,
.cts_event_workaround = false,
+ .always_enabled = false,
.get_fifosize = get_fifosize_arm,
};
@@ -109,6 +111,7 @@ static struct vendor_data vendor_st = {
.oversampling = true,
.dma_threshold = true,
.cts_event_workaround = true,
+ .always_enabled = false,
.get_fifosize = get_fifosize_st,
};
@@ -1958,7 +1961,7 @@ static void
pl011_console_write(struct console *co, const char *s, unsigned int count)
{
struct uart_amba_port *uap = amba_ports[co->index];
- unsigned int status, old_cr, new_cr;
+ unsigned int status, old_cr = 0, new_cr;
unsigned long flags;
int locked = 1;
@@ -1975,10 +1978,12 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
/*
* First save the CR then disable the interrupts
*/
- old_cr = readw(uap->port.membase + UART011_CR);
- new_cr = old_cr & ~UART011_CR_CTSEN;
- new_cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
- writew(new_cr, uap->port.membase + UART011_CR);
+ if (!uap->vendor->always_enabled) {
+ old_cr = readw(uap->port.membase + UART011_CR);
+ new_cr = old_cr & ~UART011_CR_CTSEN;
+ new_cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
+ writew(new_cr, uap->port.membase + UART011_CR);
+ }
uart_console_write(&uap->port, s, count, pl011_console_putchar);
@@ -1989,7 +1994,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
do {
status = readw(uap->port.membase + UART01x_FR);
} while (status & UART01x_FR_BUSY);
- writew(old_cr, uap->port.membase + UART011_CR);
+ if (!uap->vendor->always_enabled)
+ writew(old_cr, uap->port.membase + UART011_CR);
if (locked)
spin_unlock(&uap->port.lock);
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 09/11] drivers: PL011: allow to supply fixed option string
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
The SBSA UART has a fixed baud rate and flow control setting, which
cannot be changed or queried by software.
Add a vendor specific property to always return fixed values when
trying to read the console options.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index fe92289..70e2958 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -79,6 +79,7 @@ struct vendor_data {
bool dma_threshold;
bool cts_event_workaround;
bool always_enabled;
+ bool fixed_options;
unsigned int (*get_fifosize)(struct amba_device *dev);
};
@@ -96,6 +97,7 @@ static struct vendor_data vendor_arm = {
.dma_threshold = false,
.cts_event_workaround = false,
.always_enabled = false,
+ .fixed_options = false,
.get_fifosize = get_fifosize_arm,
};
@@ -112,6 +114,7 @@ static struct vendor_data vendor_st = {
.dma_threshold = true,
.cts_event_workaround = true,
.always_enabled = false,
+ .fixed_options = false,
.get_fifosize = get_fifosize_st,
};
@@ -160,6 +163,7 @@ struct uart_amba_port {
unsigned int lcrh_rx; /* vendor-specific */
unsigned int old_cr; /* state during shutdown */
bool autorts;
+ unsigned int fixed_baud; /* vendor-set fixed baud rate */
char type[12];
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
@@ -2076,10 +2080,15 @@ static int __init pl011_console_setup(struct console *co, char *options)
uap->port.uartclk = clk_get_rate(uap->clk);
- if (options)
- uart_parse_options(options, &baud, &parity, &bits, &flow);
- else
- pl011_console_get_options(uap, &baud, &parity, &bits);
+ if (uap->vendor->fixed_options) {
+ baud = uap->fixed_baud;
+ } else {
+ if (options)
+ uart_parse_options(options,
+ &baud, &parity, &bits, &flow);
+ else
+ pl011_console_get_options(uap, &baud, &parity, &bits);
+ }
return uart_set_options(&uap->port, co, baud, parity, bits, flow);
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 09/11] drivers: PL011: allow to supply fixed option string
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
The SBSA UART has a fixed baud rate and flow control setting, which
cannot be changed or queried by software.
Add a vendor specific property to always return fixed values when
trying to read the console options.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
drivers/tty/serial/amba-pl011.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index fe92289..70e2958 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -79,6 +79,7 @@ struct vendor_data {
bool dma_threshold;
bool cts_event_workaround;
bool always_enabled;
+ bool fixed_options;
unsigned int (*get_fifosize)(struct amba_device *dev);
};
@@ -96,6 +97,7 @@ static struct vendor_data vendor_arm = {
.dma_threshold = false,
.cts_event_workaround = false,
.always_enabled = false,
+ .fixed_options = false,
.get_fifosize = get_fifosize_arm,
};
@@ -112,6 +114,7 @@ static struct vendor_data vendor_st = {
.dma_threshold = true,
.cts_event_workaround = true,
.always_enabled = false,
+ .fixed_options = false,
.get_fifosize = get_fifosize_st,
};
@@ -160,6 +163,7 @@ struct uart_amba_port {
unsigned int lcrh_rx; /* vendor-specific */
unsigned int old_cr; /* state during shutdown */
bool autorts;
+ unsigned int fixed_baud; /* vendor-set fixed baud rate */
char type[12];
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
@@ -2076,10 +2080,15 @@ static int __init pl011_console_setup(struct console *co, char *options)
uap->port.uartclk = clk_get_rate(uap->clk);
- if (options)
- uart_parse_options(options, &baud, &parity, &bits, &flow);
- else
- pl011_console_get_options(uap, &baud, &parity, &bits);
+ if (uap->vendor->fixed_options) {
+ baud = uap->fixed_baud;
+ } else {
+ if (options)
+ uart_parse_options(options,
+ &baud, &parity, &bits, &flow);
+ else
+ pl011_console_get_options(uap, &baud, &parity, &bits);
+ }
return uart_set_options(&uap->port, co, baud, parity, bits, flow);
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
The ARM Server Base System Architecture[1] document describes a
generic UART which is a subset of the PL011 UART.
It lacks DMA support, baud rate control and modem status line
control, among other things.
The idea is to move the UART initialization and setup into the
firmware (which does this job today already) and let the kernel just
use the UART for sending and receiving characters.
We use the recent refactoring to build a new struct uart_ops
variable which points to some new functions avoiding access to the
missing registers. We reuse as much existing PL011 code as possible.
In contrast to the PL011 the SBSA UART does not define any AMBA or
PrimeCell relations, so we go with a pretty generic probe function
which only uses platform device functions.
A DT binding is provided with this patch, ACPI support is added in a
separate one.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
.../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
drivers/tty/serial/amba-pl011.c | 168 +++++++++++++++++++++
2 files changed, 178 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
diff --git a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
new file mode 100644
index 0000000..4163e7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
@@ -0,0 +1,10 @@
+* ARM SBSA defined generic UART
+This UART uses a subset of the PL011 registers and consequently lives
+in the PL011 driver. It's baudrate and other communication parameters
+cannot be adjusted at runtime, so it lacks a clock specifier here.
+
+Required properties:
+- compatible: must be "arm,sbsa-uart"
+- reg: exactly one register range
+- interrupts: exactly one interrupt specifier
+- current-speed: the (fixed) baud rate set by the firmware
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 70e2958..cca93d9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -101,6 +101,14 @@ static struct vendor_data vendor_arm = {
.get_fifosize = get_fifosize_arm,
};
+static struct vendor_data vendor_sbsa = {
+ .oversampling = false,
+ .dma_threshold = false,
+ .cts_event_workaround = false,
+ .always_enabled = true,
+ .fixed_options = true,
+};
+
static unsigned int get_fifosize_st(struct amba_device *dev)
{
return 64;
@@ -1641,6 +1649,28 @@ static int pl011_startup(struct uart_port *port)
return retval;
}
+static int sbsa_uart_startup(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+ int retval;
+
+ retval = pl011_hwinit(port);
+ if (retval)
+ return retval;
+
+ retval = pl011_allocate_irq(uap);
+ if (retval)
+ return retval;
+
+ /* The SBSA UART does not support any modem status lines. */
+ uap->old_status = 0;
+
+ pl011_enable_interrupts(uap);
+
+ return 0;
+}
+
static void pl011_shutdown_channel(struct uart_amba_port *uap,
unsigned int lcrh)
{
@@ -1721,6 +1751,19 @@ static void pl011_shutdown(struct uart_port *port)
uap->port.ops->flush_buffer(port);
}
+static void sbsa_uart_shutdown(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ pl011_disable_interrupts(uap);
+
+ free_irq(uap->port.irq, uap);
+
+ if (uap->port.ops->flush_buffer)
+ uap->port.ops->flush_buffer(port);
+}
+
static void
pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
{
@@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
spin_unlock_irqrestore(&port->lock, flags);
}
+static void
+sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
+ struct ktermios *old)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+ unsigned long flags;
+
+ if (old)
+ tty_termios_copy_hw(termios, old);
+ tty_termios_encode_baud_rate(termios, uap->fixed_baud, uap->fixed_baud);
+
+ spin_lock_irqsave(&port->lock, flags);
+ uart_update_timeout(port, CS8, uap->fixed_baud);
+ pl011_setup_status_masks(port, termios);
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
static const char *pl011_type(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1947,6 +2008,37 @@ static struct uart_ops amba_pl011_pops = {
#endif
};
+static void sbsa_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}
+
+static unsigned int sbsa_uart_get_mctrl(struct uart_port *port)
+{
+ return 0;
+}
+
+static const struct uart_ops sbsa_uart_pops = {
+ .tx_empty = pl011_tx_empty,
+ .set_mctrl = sbsa_uart_set_mctrl,
+ .get_mctrl = sbsa_uart_get_mctrl,
+ .stop_tx = pl011_stop_tx,
+ .start_tx = pl011_start_tx,
+ .stop_rx = pl011_stop_rx,
+ .startup = sbsa_uart_startup,
+ .shutdown = sbsa_uart_shutdown,
+ .set_termios = sbsa_uart_set_termios,
+ .type = pl011_type,
+ .release_port = pl011_release_port,
+ .request_port = pl011_request_port,
+ .config_port = pl011_config_port,
+ .verify_port = pl011_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_init = pl011_hwinit,
+ .poll_get_char = pl011_get_poll_char,
+ .poll_put_char = pl011_put_poll_char,
+#endif
+};
+
static struct uart_amba_port *amba_ports[UART_NR];
#ifdef CONFIG_SERIAL_AMBA_PL011_CONSOLE
@@ -2327,6 +2419,79 @@ static int pl011_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
+static int sbsa_uart_probe(struct platform_device *pdev)
+{
+ struct uart_amba_port *uap;
+ struct resource *r;
+ int portnr, ret;
+ int baudrate;
+
+ /*
+ * Check the mandatory baud rate parameter in the DT node early
+ * so that we can easily exit with the error.
+ */
+ if (pdev->dev.of_node) {
+ struct device_node *np = pdev->dev.of_node;
+
+ ret = of_property_read_u32(np, "current-speed", &baudrate);
+ if (ret)
+ return ret;
+ } else {
+ baudrate = 115200;
+ }
+
+ portnr = pl011_find_free_port();
+ if (portnr < 0)
+ return portnr;
+
+ uap = devm_kzalloc(&pdev->dev, sizeof(struct uart_amba_port),
+ GFP_KERNEL);
+ if (uap == NULL)
+ return -ENOMEM;
+
+ uap->vendor = &vendor_sbsa;
+ uap->fifosize = 32;
+ uap->port.irq = platform_get_irq(pdev, 0);
+ uap->port.ops = &sbsa_uart_pops;
+ uap->fixed_baud = baudrate;
+
+ snprintf(uap->type, sizeof(uap->type), "SBSA");
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ ret = pl011_setup_port(&pdev->dev, uap, r, portnr);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, uap);
+
+ return pl011_register_port(uap);
+}
+
+static int sbsa_uart_remove(struct platform_device *pdev)
+{
+ struct uart_amba_port *uap = platform_get_drvdata(pdev);
+
+ uart_remove_one_port(&amba_reg, &uap->port);
+ pl011_unregister_port(uap);
+ return 0;
+}
+
+static const struct of_device_id sbsa_uart_of_match[] = {
+ { .compatible = "arm,sbsa-uart", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
+
+static struct platform_driver arm_sbsa_uart_platform_driver = {
+ .probe = sbsa_uart_probe,
+ .remove = sbsa_uart_remove,
+ .driver = {
+ .name = "sbsa-uart",
+ .of_match_table = of_match_ptr(sbsa_uart_of_match),
+ },
+};
+
static struct amba_id pl011_ids[] = {
{
.id = 0x00041011,
@@ -2357,11 +2522,14 @@ static int __init pl011_init(void)
{
printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
+ if (platform_driver_register(&arm_sbsa_uart_platform_driver))
+ pr_warn("could not register SBSA UART platform driver\n");
return amba_driver_register(&pl011_driver);
}
static void __exit pl011_exit(void)
{
+ platform_driver_unregister(&arm_sbsa_uart_platform_driver);
amba_driver_unregister(&pl011_driver);
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
The ARM Server Base System Architecture[1] document describes a
generic UART which is a subset of the PL011 UART.
It lacks DMA support, baud rate control and modem status line
control, among other things.
The idea is to move the UART initialization and setup into the
firmware (which does this job today already) and let the kernel just
use the UART for sending and receiving characters.
We use the recent refactoring to build a new struct uart_ops
variable which points to some new functions avoiding access to the
missing registers. We reuse as much existing PL011 code as possible.
In contrast to the PL011 the SBSA UART does not define any AMBA or
PrimeCell relations, so we go with a pretty generic probe function
which only uses platform device functions.
A DT binding is provided with this patch, ACPI support is added in a
separate one.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
.../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
drivers/tty/serial/amba-pl011.c | 168 +++++++++++++++++++++
2 files changed, 178 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
diff --git a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
new file mode 100644
index 0000000..4163e7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
@@ -0,0 +1,10 @@
+* ARM SBSA defined generic UART
+This UART uses a subset of the PL011 registers and consequently lives
+in the PL011 driver. It's baudrate and other communication parameters
+cannot be adjusted at runtime, so it lacks a clock specifier here.
+
+Required properties:
+- compatible: must be "arm,sbsa-uart"
+- reg: exactly one register range
+- interrupts: exactly one interrupt specifier
+- current-speed: the (fixed) baud rate set by the firmware
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 70e2958..cca93d9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -101,6 +101,14 @@ static struct vendor_data vendor_arm = {
.get_fifosize = get_fifosize_arm,
};
+static struct vendor_data vendor_sbsa = {
+ .oversampling = false,
+ .dma_threshold = false,
+ .cts_event_workaround = false,
+ .always_enabled = true,
+ .fixed_options = true,
+};
+
static unsigned int get_fifosize_st(struct amba_device *dev)
{
return 64;
@@ -1641,6 +1649,28 @@ static int pl011_startup(struct uart_port *port)
return retval;
}
+static int sbsa_uart_startup(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+ int retval;
+
+ retval = pl011_hwinit(port);
+ if (retval)
+ return retval;
+
+ retval = pl011_allocate_irq(uap);
+ if (retval)
+ return retval;
+
+ /* The SBSA UART does not support any modem status lines. */
+ uap->old_status = 0;
+
+ pl011_enable_interrupts(uap);
+
+ return 0;
+}
+
static void pl011_shutdown_channel(struct uart_amba_port *uap,
unsigned int lcrh)
{
@@ -1721,6 +1751,19 @@ static void pl011_shutdown(struct uart_port *port)
uap->port.ops->flush_buffer(port);
}
+static void sbsa_uart_shutdown(struct uart_port *port)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+
+ pl011_disable_interrupts(uap);
+
+ free_irq(uap->port.irq, uap);
+
+ if (uap->port.ops->flush_buffer)
+ uap->port.ops->flush_buffer(port);
+}
+
static void
pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
{
@@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
spin_unlock_irqrestore(&port->lock, flags);
}
+static void
+sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
+ struct ktermios *old)
+{
+ struct uart_amba_port *uap =
+ container_of(port, struct uart_amba_port, port);
+ unsigned long flags;
+
+ if (old)
+ tty_termios_copy_hw(termios, old);
+ tty_termios_encode_baud_rate(termios, uap->fixed_baud, uap->fixed_baud);
+
+ spin_lock_irqsave(&port->lock, flags);
+ uart_update_timeout(port, CS8, uap->fixed_baud);
+ pl011_setup_status_masks(port, termios);
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
static const char *pl011_type(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1947,6 +2008,37 @@ static struct uart_ops amba_pl011_pops = {
#endif
};
+static void sbsa_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}
+
+static unsigned int sbsa_uart_get_mctrl(struct uart_port *port)
+{
+ return 0;
+}
+
+static const struct uart_ops sbsa_uart_pops = {
+ .tx_empty = pl011_tx_empty,
+ .set_mctrl = sbsa_uart_set_mctrl,
+ .get_mctrl = sbsa_uart_get_mctrl,
+ .stop_tx = pl011_stop_tx,
+ .start_tx = pl011_start_tx,
+ .stop_rx = pl011_stop_rx,
+ .startup = sbsa_uart_startup,
+ .shutdown = sbsa_uart_shutdown,
+ .set_termios = sbsa_uart_set_termios,
+ .type = pl011_type,
+ .release_port = pl011_release_port,
+ .request_port = pl011_request_port,
+ .config_port = pl011_config_port,
+ .verify_port = pl011_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_init = pl011_hwinit,
+ .poll_get_char = pl011_get_poll_char,
+ .poll_put_char = pl011_put_poll_char,
+#endif
+};
+
static struct uart_amba_port *amba_ports[UART_NR];
#ifdef CONFIG_SERIAL_AMBA_PL011_CONSOLE
@@ -2327,6 +2419,79 @@ static int pl011_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
+static int sbsa_uart_probe(struct platform_device *pdev)
+{
+ struct uart_amba_port *uap;
+ struct resource *r;
+ int portnr, ret;
+ int baudrate;
+
+ /*
+ * Check the mandatory baud rate parameter in the DT node early
+ * so that we can easily exit with the error.
+ */
+ if (pdev->dev.of_node) {
+ struct device_node *np = pdev->dev.of_node;
+
+ ret = of_property_read_u32(np, "current-speed", &baudrate);
+ if (ret)
+ return ret;
+ } else {
+ baudrate = 115200;
+ }
+
+ portnr = pl011_find_free_port();
+ if (portnr < 0)
+ return portnr;
+
+ uap = devm_kzalloc(&pdev->dev, sizeof(struct uart_amba_port),
+ GFP_KERNEL);
+ if (uap == NULL)
+ return -ENOMEM;
+
+ uap->vendor = &vendor_sbsa;
+ uap->fifosize = 32;
+ uap->port.irq = platform_get_irq(pdev, 0);
+ uap->port.ops = &sbsa_uart_pops;
+ uap->fixed_baud = baudrate;
+
+ snprintf(uap->type, sizeof(uap->type), "SBSA");
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ ret = pl011_setup_port(&pdev->dev, uap, r, portnr);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, uap);
+
+ return pl011_register_port(uap);
+}
+
+static int sbsa_uart_remove(struct platform_device *pdev)
+{
+ struct uart_amba_port *uap = platform_get_drvdata(pdev);
+
+ uart_remove_one_port(&amba_reg, &uap->port);
+ pl011_unregister_port(uap);
+ return 0;
+}
+
+static const struct of_device_id sbsa_uart_of_match[] = {
+ { .compatible = "arm,sbsa-uart", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
+
+static struct platform_driver arm_sbsa_uart_platform_driver = {
+ .probe = sbsa_uart_probe,
+ .remove = sbsa_uart_remove,
+ .driver = {
+ .name = "sbsa-uart",
+ .of_match_table = of_match_ptr(sbsa_uart_of_match),
+ },
+};
+
static struct amba_id pl011_ids[] = {
{
.id = 0x00041011,
@@ -2357,11 +2522,14 @@ static int __init pl011_init(void)
{
printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
+ if (platform_driver_register(&arm_sbsa_uart_platform_driver))
+ pr_warn("could not register SBSA UART platform driver\n");
return amba_driver_register(&pl011_driver);
}
static void __exit pl011_exit(void)
{
+ platform_driver_unregister(&arm_sbsa_uart_platform_driver);
amba_driver_unregister(&pl011_driver);
}
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 11/11] drivers: PL011: add ACPI probing for SBSA UART
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 14:14 ` Andre Przywara
-1 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
From: Graeme Gregory <graeme.gregory@linaro.org>
Add the necessary driver boilerplate to let the driver be used when
the respective ACPI table is discovered by the ACPI subsystem.
[Andre: change table name, add MODULE_DEVICE_TABLE entry and improve
commit message]
Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/tty/serial/amba-pl011.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cca93d9..79f716c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -58,6 +58,7 @@
#include <linux/pinctrl/consumer.h>
#include <linux/sizes.h>
#include <linux/io.h>
+#include <linux/acpi.h>
#define UART_NR 14
@@ -2483,12 +2484,19 @@ static const struct of_device_id sbsa_uart_of_match[] = {
};
MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
+static const struct acpi_device_id sbsa_uart_acpi_match[] = {
+ { "ARMH0011", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, sbsa_uart_acpi_match);
+
static struct platform_driver arm_sbsa_uart_platform_driver = {
.probe = sbsa_uart_probe,
.remove = sbsa_uart_remove,
.driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
+ .acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
},
};
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 11/11] drivers: PL011: add ACPI probing for SBSA UART
@ 2015-05-12 14:14 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-12 14:14 UTC (permalink / raw)
To: linux-arm-kernel
From: Graeme Gregory <graeme.gregory@linaro.org>
Add the necessary driver boilerplate to let the driver be used when
the respective ACPI table is discovered by the ACPI subsystem.
[Andre: change table name, add MODULE_DEVICE_TABLE entry and improve
commit message]
Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/tty/serial/amba-pl011.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cca93d9..79f716c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -58,6 +58,7 @@
#include <linux/pinctrl/consumer.h>
#include <linux/sizes.h>
#include <linux/io.h>
+#include <linux/acpi.h>
#define UART_NR 14
@@ -2483,12 +2484,19 @@ static const struct of_device_id sbsa_uart_of_match[] = {
};
MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
+static const struct acpi_device_id sbsa_uart_acpi_match[] = {
+ { "ARMH0011", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, sbsa_uart_acpi_match);
+
static struct platform_driver arm_sbsa_uart_platform_driver = {
.probe = sbsa_uart_probe,
.remove = sbsa_uart_remove,
.driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
+ .acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
},
};
--
2.3.5
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 15:18 ` Jakub Kiciński
-1 siblings, 0 replies; 42+ messages in thread
From: Jakub Kiciński @ 2015-05-12 15:18 UTC (permalink / raw)
To: Andre Przywara
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Greg KH, linux-serial,
rmk+kernel, Jiri Slaby, dave.martin, linux-arm-kernel
On Tue, 12 May 2015 15:14:44 +0100, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
Thanks, apart from minor checkpatch --strict warnings it looks good now.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-12 15:18 ` Jakub Kiciński
0 siblings, 0 replies; 42+ messages in thread
From: Jakub Kiciński @ 2015-05-12 15:18 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 12 May 2015 15:14:44 +0100, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
Thanks, apart from minor checkpatch --strict warnings it looks good now.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-12 16:42 ` Robert Richter
-1 siblings, 0 replies; 42+ messages in thread
From: Robert Richter @ 2015-05-12 16:42 UTC (permalink / raw)
To: Andre Przywara
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski, Greg KH,
linux-serial, rmk+kernel, Jiri Slaby, dave.martin,
linux-arm-kernel
On 12.05.15 15:14:44, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
> Andre Przywara (10):
> drivers: PL011: avoid potential unregister_driver call
> drivers: PL011: refactor pl011_startup()
> drivers: PL011: refactor pl011_shutdown()
> drivers: PL011: refactor pl011_set_termios()
> drivers: PL011: refactor pl011_probe()
> drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
> drivers: PL011: move cts_event workaround into separate function
> drivers: PL011: allow avoiding UART enabling/disabling
> drivers: PL011: allow to supply fixed option string
> drivers: PL011: add support for the ARM SBSA generic UART
>
> Graeme Gregory (1):
> drivers: PL011: add ACPI probing for SBSA UART
Tested-by: Robert Richter <rrichter@cavium.com>
... with devicetree and ACPI.
-Robert
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-12 16:42 ` Robert Richter
0 siblings, 0 replies; 42+ messages in thread
From: Robert Richter @ 2015-05-12 16:42 UTC (permalink / raw)
To: linux-arm-kernel
On 12.05.15 15:14:44, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
> Andre Przywara (10):
> drivers: PL011: avoid potential unregister_driver call
> drivers: PL011: refactor pl011_startup()
> drivers: PL011: refactor pl011_shutdown()
> drivers: PL011: refactor pl011_set_termios()
> drivers: PL011: refactor pl011_probe()
> drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
> drivers: PL011: move cts_event workaround into separate function
> drivers: PL011: allow avoiding UART enabling/disabling
> drivers: PL011: allow to supply fixed option string
> drivers: PL011: add support for the ARM SBSA generic UART
>
> Graeme Gregory (1):
> drivers: PL011: add ACPI probing for SBSA UART
Tested-by: Robert Richter <rrichter@cavium.com>
... with devicetree and ACPI.
-Robert
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 11/11] drivers: PL011: add ACPI probing for SBSA UART
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-13 1:09 ` Hanjun Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Hanjun Guo @ 2015-05-13 1:09 UTC (permalink / raw)
To: Andre Przywara, rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
On 2015/5/12 22:14, Andre Przywara wrote:
> From: Graeme Gregory <graeme.gregory@linaro.org>
>
> Add the necessary driver boilerplate to let the driver be used when
> the respective ACPI table is discovered by the ACPI subsystem.
>
> [Andre: change table name, add MODULE_DEVICE_TABLE entry and improve
> commit message]
>
> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/tty/serial/amba-pl011.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index cca93d9..79f716c 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -58,6 +58,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/sizes.h>
> #include <linux/io.h>
> +#include <linux/acpi.h>
>
> #define UART_NR 14
>
> @@ -2483,12 +2484,19 @@ static const struct of_device_id sbsa_uart_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
>
> +static const struct acpi_device_id sbsa_uart_acpi_match[] = {
> + { "ARMH0011", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, sbsa_uart_acpi_match);
> +
> static struct platform_driver arm_sbsa_uart_platform_driver = {
> .probe = sbsa_uart_probe,
> .remove = sbsa_uart_remove,
> .driver = {
> .name = "sbsa-uart",
> .of_match_table = of_match_ptr(sbsa_uart_of_match),
> + .acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
> },
> };
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 11/11] drivers: PL011: add ACPI probing for SBSA UART
@ 2015-05-13 1:09 ` Hanjun Guo
0 siblings, 0 replies; 42+ messages in thread
From: Hanjun Guo @ 2015-05-13 1:09 UTC (permalink / raw)
To: linux-arm-kernel
On 2015/5/12 22:14, Andre Przywara wrote:
> From: Graeme Gregory <graeme.gregory@linaro.org>
>
> Add the necessary driver boilerplate to let the driver be used when
> the respective ACPI table is discovered by the ACPI subsystem.
>
> [Andre: change table name, add MODULE_DEVICE_TABLE entry and improve
> commit message]
>
> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/tty/serial/amba-pl011.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index cca93d9..79f716c 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -58,6 +58,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/sizes.h>
> #include <linux/io.h>
> +#include <linux/acpi.h>
>
> #define UART_NR 14
>
> @@ -2483,12 +2484,19 @@ static const struct of_device_id sbsa_uart_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
>
> +static const struct acpi_device_id sbsa_uart_acpi_match[] = {
> + { "ARMH0011", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, sbsa_uart_acpi_match);
> +
> static struct platform_driver arm_sbsa_uart_platform_driver = {
> .probe = sbsa_uart_probe,
> .remove = sbsa_uart_remove,
> .driver = {
> .name = "sbsa-uart",
> .of_match_table = of_match_ptr(sbsa_uart_of_match),
> + .acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
> },
> };
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-13 9:17 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 42+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-13 9:17 UTC (permalink / raw)
To: Andre Przywara
Cc: arnd, graeme.gregory, Jakub Kicinski, Greg KH, linux-serial,
rmk+kernel, Jiri Slaby, Dave P Martin, linux-arm-kernel
On Tue, May 12, 2015 at 03:14:44PM +0100, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
>
> The ARM Server Base System Architecture[1] document describes a
> generic UART which is a subset of the ARM PL011 UART.
> It lacks DMA support, baud rate control and modem status line
> control, among other things.
> The idea is to move the UART initialization and setup into the
> firmware (which does this job today already) and let the kernel just
> use the UART for sending and receiving characters.
>
> This patchset integrates support for this UART subset into the
> existing PL011 driver - basically by refactoring some
> functions and providing a new uart_ops structure for it. It also has
> a separate probe function to be not dependent on AMBA/PrimeCell.
> It provides a device tree and an ACPI binding.
> Beside the obvious effect of code sharing reusing most of the PL011
> code has the advantage of not introducing another serial device
> prefix, so it can go with ttyAMA, which seems to be pretty common.
> As changing the baudrate and other communication parameters is not
> specified for the SBSA UART, any userland attempt will be denied by
> the kernel. The device-tree provided fixed baud rate will be
> reported, so stty for instance prints the right value.
>
> This series is based on Greg's tty-next branch, a git repo can
> be found at [2] (branch sbsa-uart/v4).
>
> Patch 1/11 contains a bug fix which applies to the PL011 part also,
> it should be considered regardless of the rest of the series.
> According to Russell this is still racy, but still better than the
> current solution.
> Patch 2-7 refactor some PL011 functions by splitting them up into
> smaller pieces, so that most of the code can be reused later by the
> SBSA part.
> Patch 8 and 9 introduce two new properties for the vendor structure,
> this is for SBSA functionality which cannot be controlled by
> separate uart_ops members only.
> Patch 10 then finally drops in the SBSA specific code, by providing
> a new uart_ops, vendor struct and probe function for it. Also the new
> device tree binding is documented.
> Patch 11 adds the necessary ACPI boilerplate.
>
> For testing you should be able to take any hardware which has a PL011
> and change the DT to use a "arm,sbsa-uart" compatible string and the
> baud rate with the "current-speed" property.
> Of course testing with a real SBSA Generic UART is welcomed - as well
> as regression testing with any PL011 implementation.
> I tested this on a FastModel, a Juno and a Midway machine, both in
> PL011 and in (DT-emulated) SBSA mode.
>
> Changelog:
> v3 .. v4:
> - fixed rebase artifact
> - moved memory allocation out of pl011_allocate_port() function
> (and consequently renamed that to pl011_find_free_port)
> - added ACPI driver binding
>
> v2 .. v3:
> - rebased on top of tty-next and Dave's latest PL011 rework
> - fixed module build
> - removed redundant =NULL members in sbsa_uart_ops
>
> v1 .. v2:
> - rebased on top of 4.0-rc1 and Dave's newest PL011 fix [3]
> - added mandatory current-speed property and report that to userland
>
> Cheers,
> Andre
>
> [1] ARM-DEN-0029 Server Base System Architecture, available (click-
> thru...) from http://infocenter.arm.com
> [2] http://www.linux-arm.org/git?p=linux-ap.git
> git://linux-arm.org/linux-ap.git
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327631.html
>
> Andre Przywara (10):
> drivers: PL011: avoid potential unregister_driver call
> drivers: PL011: refactor pl011_startup()
> drivers: PL011: refactor pl011_shutdown()
> drivers: PL011: refactor pl011_set_termios()
> drivers: PL011: refactor pl011_probe()
> drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
> drivers: PL011: move cts_event workaround into separate function
> drivers: PL011: allow avoiding UART enabling/disabling
> drivers: PL011: allow to supply fixed option string
> drivers: PL011: add support for the ARM SBSA generic UART
>
> Graeme Gregory (1):
> drivers: PL011: add ACPI probing for SBSA UART
>
> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 +
> drivers/tty/serial/amba-pl011.c | 547 +++++++++++++++------
> 2 files changed, 409 insertions(+), 148 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-13 9:17 ` Lorenzo Pieralisi
0 siblings, 0 replies; 42+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-13 9:17 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 12, 2015 at 03:14:44PM +0100, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
>
> The ARM Server Base System Architecture[1] document describes a
> generic UART which is a subset of the ARM PL011 UART.
> It lacks DMA support, baud rate control and modem status line
> control, among other things.
> The idea is to move the UART initialization and setup into the
> firmware (which does this job today already) and let the kernel just
> use the UART for sending and receiving characters.
>
> This patchset integrates support for this UART subset into the
> existing PL011 driver - basically by refactoring some
> functions and providing a new uart_ops structure for it. It also has
> a separate probe function to be not dependent on AMBA/PrimeCell.
> It provides a device tree and an ACPI binding.
> Beside the obvious effect of code sharing reusing most of the PL011
> code has the advantage of not introducing another serial device
> prefix, so it can go with ttyAMA, which seems to be pretty common.
> As changing the baudrate and other communication parameters is not
> specified for the SBSA UART, any userland attempt will be denied by
> the kernel. The device-tree provided fixed baud rate will be
> reported, so stty for instance prints the right value.
>
> This series is based on Greg's tty-next branch, a git repo can
> be found at [2] (branch sbsa-uart/v4).
>
> Patch 1/11 contains a bug fix which applies to the PL011 part also,
> it should be considered regardless of the rest of the series.
> According to Russell this is still racy, but still better than the
> current solution.
> Patch 2-7 refactor some PL011 functions by splitting them up into
> smaller pieces, so that most of the code can be reused later by the
> SBSA part.
> Patch 8 and 9 introduce two new properties for the vendor structure,
> this is for SBSA functionality which cannot be controlled by
> separate uart_ops members only.
> Patch 10 then finally drops in the SBSA specific code, by providing
> a new uart_ops, vendor struct and probe function for it. Also the new
> device tree binding is documented.
> Patch 11 adds the necessary ACPI boilerplate.
>
> For testing you should be able to take any hardware which has a PL011
> and change the DT to use a "arm,sbsa-uart" compatible string and the
> baud rate with the "current-speed" property.
> Of course testing with a real SBSA Generic UART is welcomed - as well
> as regression testing with any PL011 implementation.
> I tested this on a FastModel, a Juno and a Midway machine, both in
> PL011 and in (DT-emulated) SBSA mode.
>
> Changelog:
> v3 .. v4:
> - fixed rebase artifact
> - moved memory allocation out of pl011_allocate_port() function
> (and consequently renamed that to pl011_find_free_port)
> - added ACPI driver binding
>
> v2 .. v3:
> - rebased on top of tty-next and Dave's latest PL011 rework
> - fixed module build
> - removed redundant =NULL members in sbsa_uart_ops
>
> v1 .. v2:
> - rebased on top of 4.0-rc1 and Dave's newest PL011 fix [3]
> - added mandatory current-speed property and report that to userland
>
> Cheers,
> Andre
>
> [1] ARM-DEN-0029 Server Base System Architecture, available (click-
> thru...) from http://infocenter.arm.com
> [2] http://www.linux-arm.org/git?p=linux-ap.git
> git://linux-arm.org/linux-ap.git
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327631.html
>
> Andre Przywara (10):
> drivers: PL011: avoid potential unregister_driver call
> drivers: PL011: refactor pl011_startup()
> drivers: PL011: refactor pl011_shutdown()
> drivers: PL011: refactor pl011_set_termios()
> drivers: PL011: refactor pl011_probe()
> drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
> drivers: PL011: move cts_event workaround into separate function
> drivers: PL011: allow avoiding UART enabling/disabling
> drivers: PL011: allow to supply fixed option string
> drivers: PL011: add support for the ARM SBSA generic UART
>
> Graeme Gregory (1):
> drivers: PL011: add ACPI probing for SBSA UART
>
> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 +
> drivers/tty/serial/amba-pl011.c | 547 +++++++++++++++------
> 2 files changed, 409 insertions(+), 148 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-12 14:14 ` Andre Przywara
(?)
@ 2015-05-13 14:32 ` Mark Langsdorf
2015-05-13 15:03 ` Graeme Gregory
2015-05-21 11:13 ` Andre Przywara
-1 siblings, 2 replies; 42+ messages in thread
From: Mark Langsdorf @ 2015-05-13 14:32 UTC (permalink / raw)
To: linux-arm-kernel
On 05/12/2015 09:14 AM, Andre Przywara wrote:
> The ARM Server Base System Architecture[1] document describes a
> generic UART which is a subset of the PL011 UART.
> It lacks DMA support, baud rate control and modem status line
> control, among other things.
> The idea is to move the UART initialization and setup into the
> firmware (which does this job today already) and let the kernel just
> use the UART for sending and receiving characters.
>
> We use the recent refactoring to build a new struct uart_ops
> variable which points to some new functions avoiding access to the
> missing registers. We reuse as much existing PL011 code as possible.
>
> In contrast to the PL011 the SBSA UART does not define any AMBA or
> PrimeCell relations, so we go with a pretty generic probe function
> which only uses platform device functions.
> A DT binding is provided with this patch, ACPI support is added in a
> separate one.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
> drivers/tty/serial/amba-pl011.c | 168 +++++++++++++++++++++
> 2 files changed, 178 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> new file mode 100644
> index 0000000..4163e7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> @@ -0,0 +1,10 @@
> +* ARM SBSA defined generic UART
> +This UART uses a subset of the PL011 registers and consequently lives
> +in the PL011 driver. It's baudrate and other communication parameters
> +cannot be adjusted at runtime, so it lacks a clock specifier here.
> +
> +Required properties:
> +- compatible: must be "arm,sbsa-uart"
> +- reg: exactly one register range
> +- interrupts: exactly one interrupt specifier
> +- current-speed: the (fixed) baud rate set by the firmware
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 70e2958..cca93d9 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -101,6 +101,14 @@ static struct vendor_data vendor_arm = {
> .get_fifosize = get_fifosize_arm,
> };
>
> +static struct vendor_data vendor_sbsa = {
> + .oversampling = false,
> + .dma_threshold = false,
> + .cts_event_workaround = false,
> + .always_enabled = true,
> + .fixed_options = true,
> +};
> +
> static unsigned int get_fifosize_st(struct amba_device *dev)
> {
> return 64;
> @@ -1641,6 +1649,28 @@ static int pl011_startup(struct uart_port *port)
> return retval;
> }
>
> +static int sbsa_uart_startup(struct uart_port *port)
> +{
> + struct uart_amba_port *uap =
> + container_of(port, struct uart_amba_port, port);
> + int retval;
> +
> + retval = pl011_hwinit(port);
> + if (retval)
> + return retval;
> +
> + retval = pl011_allocate_irq(uap);
> + if (retval)
> + return retval;
> +
> + /* The SBSA UART does not support any modem status lines. */
> + uap->old_status = 0;
> +
> + pl011_enable_interrupts(uap);
> +
> + return 0;
> +}
> +
> static void pl011_shutdown_channel(struct uart_amba_port *uap,
> unsigned int lcrh)
> {
> @@ -1721,6 +1751,19 @@ static void pl011_shutdown(struct uart_port *port)
> uap->port.ops->flush_buffer(port);
> }
>
> +static void sbsa_uart_shutdown(struct uart_port *port)
> +{
> + struct uart_amba_port *uap =
> + container_of(port, struct uart_amba_port, port);
> +
> + pl011_disable_interrupts(uap);
> +
> + free_irq(uap->port.irq, uap);
> +
> + if (uap->port.ops->flush_buffer)
> + uap->port.ops->flush_buffer(port);
> +}
> +
> static void
> pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
> {
> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> spin_unlock_irqrestore(&port->lock, flags);
> }
>
> +static void
> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
> + struct ktermios *old)
> +{
> + struct uart_amba_port *uap =
> + container_of(port, struct uart_amba_port, port);
> + unsigned long flags;
> +
> + if (old)
> + tty_termios_copy_hw(termios, old);
This code prevented login via the serial console on our test hardware.
Mark Salter suggested the following patch:
- if (old)
+ /*
+ * The first call to set_termios() comes when the console is
+ * registered via uart_add_one_port(). The serial core will
+ * pass in a dummy old termios rather than NULL. Check to make
+ * sure the old termios has reasonable info before copying from
+ * it.
+ */
+ if (old && old->c_cflag)
tty_termios_copy_hw(termios, old);
which fixed the issue.
> + tty_termios_encode_baud_rate(termios, uap->fixed_baud, uap->fixed_baud);
> +
> + spin_lock_irqsave(&port->lock, flags);
> + uart_update_timeout(port, CS8, uap->fixed_baud);
> + pl011_setup_status_masks(port, termios);
> + spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> static const char *pl011_type(struct uart_port *port)
> {
> struct uart_amba_port *uap =
> @@ -1947,6 +2008,37 @@ static struct uart_ops amba_pl011_pops = {
> #endif
> };
>
> +static void sbsa_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +}
> +
> +static unsigned int sbsa_uart_get_mctrl(struct uart_port *port)
> +{
> + return 0;
> +}
> +
> +static const struct uart_ops sbsa_uart_pops = {
> + .tx_empty = pl011_tx_empty,
> + .set_mctrl = sbsa_uart_set_mctrl,
> + .get_mctrl = sbsa_uart_get_mctrl,
> + .stop_tx = pl011_stop_tx,
> + .start_tx = pl011_start_tx,
> + .stop_rx = pl011_stop_rx,
> + .startup = sbsa_uart_startup,
> + .shutdown = sbsa_uart_shutdown,
> + .set_termios = sbsa_uart_set_termios,
> + .type = pl011_type,
> + .release_port = pl011_release_port,
> + .request_port = pl011_request_port,
> + .config_port = pl011_config_port,
> + .verify_port = pl011_verify_port,
> +#ifdef CONFIG_CONSOLE_POLL
> + .poll_init = pl011_hwinit,
> + .poll_get_char = pl011_get_poll_char,
> + .poll_put_char = pl011_put_poll_char,
> +#endif
> +};
> +
> static struct uart_amba_port *amba_ports[UART_NR];
>
> #ifdef CONFIG_SERIAL_AMBA_PL011_CONSOLE
> @@ -2327,6 +2419,79 @@ static int pl011_resume(struct device *dev)
>
> static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
>
> +static int sbsa_uart_probe(struct platform_device *pdev)
> +{
> + struct uart_amba_port *uap;
> + struct resource *r;
> + int portnr, ret;
> + int baudrate;
> +
> + /*
> + * Check the mandatory baud rate parameter in the DT node early
> + * so that we can easily exit with the error.
> + */
> + if (pdev->dev.of_node) {
> + struct device_node *np = pdev->dev.of_node;
> +
> + ret = of_property_read_u32(np, "current-speed", &baudrate);
> + if (ret)
> + return ret;
> + } else {
> + baudrate = 115200;
> + }
> +
> + portnr = pl011_find_free_port();
> + if (portnr < 0)
> + return portnr;
> +
> + uap = devm_kzalloc(&pdev->dev, sizeof(struct uart_amba_port),
> + GFP_KERNEL);
> + if (uap == NULL)
> + return -ENOMEM;
> +
> + uap->vendor = &vendor_sbsa;
> + uap->fifosize = 32;
> + uap->port.irq = platform_get_irq(pdev, 0);
> + uap->port.ops = &sbsa_uart_pops;
> + uap->fixed_baud = baudrate;
> +
> + snprintf(uap->type, sizeof(uap->type), "SBSA");
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + ret = pl011_setup_port(&pdev->dev, uap, r, portnr);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, uap);
> +
> + return pl011_register_port(uap);
> +}
> +
> +static int sbsa_uart_remove(struct platform_device *pdev)
> +{
> + struct uart_amba_port *uap = platform_get_drvdata(pdev);
> +
> + uart_remove_one_port(&amba_reg, &uap->port);
> + pl011_unregister_port(uap);
> + return 0;
> +}
> +
> +static const struct of_device_id sbsa_uart_of_match[] = {
> + { .compatible = "arm,sbsa-uart", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
> +
> +static struct platform_driver arm_sbsa_uart_platform_driver = {
> + .probe = sbsa_uart_probe,
> + .remove = sbsa_uart_remove,
> + .driver = {
> + .name = "sbsa-uart",
> + .of_match_table = of_match_ptr(sbsa_uart_of_match),
> + },
> +};
> +
> static struct amba_id pl011_ids[] = {
> {
> .id = 0x00041011,
> @@ -2357,11 +2522,14 @@ static int __init pl011_init(void)
> {
> printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
>
> + if (platform_driver_register(&arm_sbsa_uart_platform_driver))
> + pr_warn("could not register SBSA UART platform driver\n");
> return amba_driver_register(&pl011_driver);
> }
>
> static void __exit pl011_exit(void)
> {
> + platform_driver_unregister(&arm_sbsa_uart_platform_driver);
> amba_driver_unregister(&pl011_driver);
> }
--Mark Langsdorf
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-13 14:32 ` Mark Langsdorf
@ 2015-05-13 15:03 ` Graeme Gregory
2015-05-21 11:13 ` Andre Przywara
1 sibling, 0 replies; 42+ messages in thread
From: Graeme Gregory @ 2015-05-13 15:03 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 13 May 2015, at 03:32 PM, Mark Langsdorf wrote:
> On 05/12/2015 09:14 AM, Andre Przywara wrote:
> > The ARM Server Base System Architecture[1] document describes a
> > generic UART which is a subset of the PL011 UART.
> > It lacks DMA support, baud rate control and modem status line
> > control, among other things.
> > The idea is to move the UART initialization and setup into the
> > firmware (which does this job today already) and let the kernel just
> > use the UART for sending and receiving characters.
> >
> > We use the recent refactoring to build a new struct uart_ops
> > variable which points to some new functions avoiding access to the
> > missing registers. We reuse as much existing PL011 code as possible.
> >
> > In contrast to the PL011 the SBSA UART does not define any AMBA or
> > PrimeCell relations, so we go with a pretty generic probe function
> > which only uses platform device functions.
> > A DT binding is provided with this patch, ACPI support is added in a
> > separate one.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> > .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
> > drivers/tty/serial/amba-pl011.c | 168 +++++++++++++++++++++
> > 2 files changed, 178 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> >
> > diff --git a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> > new file mode 100644
> > index 0000000..4163e7e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> > @@ -0,0 +1,10 @@
> > +* ARM SBSA defined generic UART
> > +This UART uses a subset of the PL011 registers and consequently lives
> > +in the PL011 driver. It's baudrate and other communication parameters
> > +cannot be adjusted at runtime, so it lacks a clock specifier here.
> > +
> > +Required properties:
> > +- compatible: must be "arm,sbsa-uart"
> > +- reg: exactly one register range
> > +- interrupts: exactly one interrupt specifier
> > +- current-speed: the (fixed) baud rate set by the firmware
> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> > index 70e2958..cca93d9 100644
> > --- a/drivers/tty/serial/amba-pl011.c
> > +++ b/drivers/tty/serial/amba-pl011.c
> > @@ -101,6 +101,14 @@ static struct vendor_data vendor_arm = {
> > .get_fifosize = get_fifosize_arm,
> > };
> >
> > +static struct vendor_data vendor_sbsa = {
> > + .oversampling = false,
> > + .dma_threshold = false,
> > + .cts_event_workaround = false,
> > + .always_enabled = true,
> > + .fixed_options = true,
> > +};
> > +
> > static unsigned int get_fifosize_st(struct amba_device *dev)
> > {
> > return 64;
> > @@ -1641,6 +1649,28 @@ static int pl011_startup(struct uart_port *port)
> > return retval;
> > }
> >
> > +static int sbsa_uart_startup(struct uart_port *port)
> > +{
> > + struct uart_amba_port *uap =
> > + container_of(port, struct uart_amba_port, port);
> > + int retval;
> > +
> > + retval = pl011_hwinit(port);
> > + if (retval)
> > + return retval;
> > +
> > + retval = pl011_allocate_irq(uap);
> > + if (retval)
> > + return retval;
> > +
> > + /* The SBSA UART does not support any modem status lines. */
> > + uap->old_status = 0;
> > +
> > + pl011_enable_interrupts(uap);
> > +
> > + return 0;
> > +}
> > +
> > static void pl011_shutdown_channel(struct uart_amba_port *uap,
> > unsigned int lcrh)
> > {
> > @@ -1721,6 +1751,19 @@ static void pl011_shutdown(struct uart_port *port)
> > uap->port.ops->flush_buffer(port);
> > }
> >
> > +static void sbsa_uart_shutdown(struct uart_port *port)
> > +{
> > + struct uart_amba_port *uap =
> > + container_of(port, struct uart_amba_port, port);
> > +
> > + pl011_disable_interrupts(uap);
> > +
> > + free_irq(uap->port.irq, uap);
> > +
> > + if (uap->port.ops->flush_buffer)
> > + uap->port.ops->flush_buffer(port);
> > +}
> > +
> > static void
> > pl011_setup_status_masks(struct uart_port *port, struct ktermios *termios)
> > {
> > @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
> > spin_unlock_irqrestore(&port->lock, flags);
> > }
> >
> > +static void
> > +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
> > + struct ktermios *old)
> > +{
> > + struct uart_amba_port *uap =
> > + container_of(port, struct uart_amba_port, port);
> > + unsigned long flags;
> > +
> > + if (old)
> > + tty_termios_copy_hw(termios, old);
>
> This code prevented login via the serial console on our test hardware.
>
> Mark Salter suggested the following patch:
>
> - if (old)
> + /*
> + * The first call to set_termios() comes when the console is
> + * registered via uart_add_one_port(). The serial core will
> + * pass in a dummy old termios rather than NULL. Check to make
> + * sure the old termios has reasonable info before copying from
> + * it.
> + */
> + if (old && old->c_cflag)
> tty_termios_copy_hw(termios, old);
>
> which fixed the issue.
>
This certainly fixes the problem I was seeing with Fedora on Seattle.
Graeme
> > + tty_termios_encode_baud_rate(termios, uap->fixed_baud, uap->fixed_baud);
> > +
> > + spin_lock_irqsave(&port->lock, flags);
> > + uart_update_timeout(port, CS8, uap->fixed_baud);
> > + pl011_setup_status_masks(port, termios);
> > + spin_unlock_irqrestore(&port->lock, flags);
> > +}
> > +
> > static const char *pl011_type(struct uart_port *port)
> > {
> > struct uart_amba_port *uap =
> > @@ -1947,6 +2008,37 @@ static struct uart_ops amba_pl011_pops = {
> > #endif
> > };
> >
> > +static void sbsa_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> > +{
> > +}
> > +
> > +static unsigned int sbsa_uart_get_mctrl(struct uart_port *port)
> > +{
> > + return 0;
> > +}
> > +
> > +static const struct uart_ops sbsa_uart_pops = {
> > + .tx_empty = pl011_tx_empty,
> > + .set_mctrl = sbsa_uart_set_mctrl,
> > + .get_mctrl = sbsa_uart_get_mctrl,
> > + .stop_tx = pl011_stop_tx,
> > + .start_tx = pl011_start_tx,
> > + .stop_rx = pl011_stop_rx,
> > + .startup = sbsa_uart_startup,
> > + .shutdown = sbsa_uart_shutdown,
> > + .set_termios = sbsa_uart_set_termios,
> > + .type = pl011_type,
> > + .release_port = pl011_release_port,
> > + .request_port = pl011_request_port,
> > + .config_port = pl011_config_port,
> > + .verify_port = pl011_verify_port,
> > +#ifdef CONFIG_CONSOLE_POLL
> > + .poll_init = pl011_hwinit,
> > + .poll_get_char = pl011_get_poll_char,
> > + .poll_put_char = pl011_put_poll_char,
> > +#endif
> > +};
> > +
> > static struct uart_amba_port *amba_ports[UART_NR];
> >
> > #ifdef CONFIG_SERIAL_AMBA_PL011_CONSOLE
> > @@ -2327,6 +2419,79 @@ static int pl011_resume(struct device *dev)
> >
> > static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
> >
> > +static int sbsa_uart_probe(struct platform_device *pdev)
> > +{
> > + struct uart_amba_port *uap;
> > + struct resource *r;
> > + int portnr, ret;
> > + int baudrate;
> > +
> > + /*
> > + * Check the mandatory baud rate parameter in the DT node early
> > + * so that we can easily exit with the error.
> > + */
> > + if (pdev->dev.of_node) {
> > + struct device_node *np = pdev->dev.of_node;
> > +
> > + ret = of_property_read_u32(np, "current-speed", &baudrate);
> > + if (ret)
> > + return ret;
> > + } else {
> > + baudrate = 115200;
> > + }
> > +
> > + portnr = pl011_find_free_port();
> > + if (portnr < 0)
> > + return portnr;
> > +
> > + uap = devm_kzalloc(&pdev->dev, sizeof(struct uart_amba_port),
> > + GFP_KERNEL);
> > + if (uap == NULL)
> > + return -ENOMEM;
> > +
> > + uap->vendor = &vendor_sbsa;
> > + uap->fifosize = 32;
> > + uap->port.irq = platform_get_irq(pdev, 0);
> > + uap->port.ops = &sbsa_uart_pops;
> > + uap->fixed_baud = baudrate;
> > +
> > + snprintf(uap->type, sizeof(uap->type), "SBSA");
> > +
> > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + ret = pl011_setup_port(&pdev->dev, uap, r, portnr);
> > + if (ret)
> > + return ret;
> > +
> > + platform_set_drvdata(pdev, uap);
> > +
> > + return pl011_register_port(uap);
> > +}
> > +
> > +static int sbsa_uart_remove(struct platform_device *pdev)
> > +{
> > + struct uart_amba_port *uap = platform_get_drvdata(pdev);
> > +
> > + uart_remove_one_port(&amba_reg, &uap->port);
> > + pl011_unregister_port(uap);
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id sbsa_uart_of_match[] = {
> > + { .compatible = "arm,sbsa-uart", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, sbsa_uart_of_match);
> > +
> > +static struct platform_driver arm_sbsa_uart_platform_driver = {
> > + .probe = sbsa_uart_probe,
> > + .remove = sbsa_uart_remove,
> > + .driver = {
> > + .name = "sbsa-uart",
> > + .of_match_table = of_match_ptr(sbsa_uart_of_match),
> > + },
> > +};
> > +
> > static struct amba_id pl011_ids[] = {
> > {
> > .id = 0x00041011,
> > @@ -2357,11 +2522,14 @@ static int __init pl011_init(void)
> > {
> > printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
> >
> > + if (platform_driver_register(&arm_sbsa_uart_platform_driver))
> > + pr_warn("could not register SBSA UART platform driver\n");
> > return amba_driver_register(&pl011_driver);
> > }
> >
> > static void __exit pl011_exit(void)
> > {
> > + platform_driver_unregister(&arm_sbsa_uart_platform_driver);
> > amba_driver_unregister(&pl011_driver);
> > }
>
> --Mark Langsdorf
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
2015-05-12 14:14 ` Andre Przywara
@ 2015-05-15 3:31 ` Hanjun Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Hanjun Guo @ 2015-05-15 3:31 UTC (permalink / raw)
To: Andre Przywara, rmk+kernel, Greg KH, Jiri Slaby
Cc: Lorenzo Pieralisi, Graeme Gregory, arnd, Jakub Kicinski,
linux-serial, dave.martin, linux-arm-kernel
On 2015年05月12日 22:14, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
>
> The ARM Server Base System Architecture[1] document describes a
> generic UART which is a subset of the ARM PL011 UART.
> It lacks DMA support, baud rate control and modem status line
> control, among other things.
> The idea is to move the UART initialization and setup into the
> firmware (which does this job today already) and let the kernel just
> use the UART for sending and receiving characters.
>
> This patchset integrates support for this UART subset into the
> existing PL011 driver - basically by refactoring some
> functions and providing a new uart_ops structure for it. It also has
> a separate probe function to be not dependent on AMBA/PrimeCell.
> It provides a device tree and an ACPI binding.
> Beside the obvious effect of code sharing reusing most of the PL011
> code has the advantage of not introducing another serial device
> prefix, so it can go with ttyAMA, which seems to be pretty common.
> As changing the baudrate and other communication parameters is not
> specified for the SBSA UART, any userland attempt will be denied by
> the kernel. The device-tree provided fixed baud rate will be
> reported, so stty for instance prints the right value.
>
> This series is based on Greg's tty-next branch, a git repo can
> be found at [2] (branch sbsa-uart/v4).
>
> Patch 1/11 contains a bug fix which applies to the PL011 part also,
> it should be considered regardless of the rest of the series.
> According to Russell this is still racy, but still better than the
> current solution.
> Patch 2-7 refactor some PL011 functions by splitting them up into
> smaller pieces, so that most of the code can be reused later by the
> SBSA part.
> Patch 8 and 9 introduce two new properties for the vendor structure,
> this is for SBSA functionality which cannot be controlled by
> separate uart_ops members only.
> Patch 10 then finally drops in the SBSA specific code, by providing
> a new uart_ops, vendor struct and probe function for it. Also the new
> device tree binding is documented.
> Patch 11 adds the necessary ACPI boilerplate.
>
> For testing you should be able to take any hardware which has a PL011
> and change the DT to use a "arm,sbsa-uart" compatible string and the
> baud rate with the "current-speed" property.
> Of course testing with a real SBSA Generic UART is welcomed - as well
> as regression testing with any PL011 implementation.
> I tested this on a FastModel, a Juno and a Midway machine, both in
> PL011 and in (DT-emulated) SBSA mode.
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
in ACPI case with device HID of ARMH0011 in DSDT.
Hanjun
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support
@ 2015-05-15 3:31 ` Hanjun Guo
0 siblings, 0 replies; 42+ messages in thread
From: Hanjun Guo @ 2015-05-15 3:31 UTC (permalink / raw)
To: linux-arm-kernel
On 2015?05?12? 22:14, Andre Przywara wrote:
> This is the fourth revision of the SBSA UART support series, now
> based on the current tty-next branch.
> Compared to v3 I addressed some comments Jakub had on v3 (thanks for
> the review!). Also since some basic ACPI support for ARM64 is now
> upstream, I include the necessary ACPI binding as a new patch 11
> on top.
> ----
>
> The ARM Server Base System Architecture[1] document describes a
> generic UART which is a subset of the ARM PL011 UART.
> It lacks DMA support, baud rate control and modem status line
> control, among other things.
> The idea is to move the UART initialization and setup into the
> firmware (which does this job today already) and let the kernel just
> use the UART for sending and receiving characters.
>
> This patchset integrates support for this UART subset into the
> existing PL011 driver - basically by refactoring some
> functions and providing a new uart_ops structure for it. It also has
> a separate probe function to be not dependent on AMBA/PrimeCell.
> It provides a device tree and an ACPI binding.
> Beside the obvious effect of code sharing reusing most of the PL011
> code has the advantage of not introducing another serial device
> prefix, so it can go with ttyAMA, which seems to be pretty common.
> As changing the baudrate and other communication parameters is not
> specified for the SBSA UART, any userland attempt will be denied by
> the kernel. The device-tree provided fixed baud rate will be
> reported, so stty for instance prints the right value.
>
> This series is based on Greg's tty-next branch, a git repo can
> be found at [2] (branch sbsa-uart/v4).
>
> Patch 1/11 contains a bug fix which applies to the PL011 part also,
> it should be considered regardless of the rest of the series.
> According to Russell this is still racy, but still better than the
> current solution.
> Patch 2-7 refactor some PL011 functions by splitting them up into
> smaller pieces, so that most of the code can be reused later by the
> SBSA part.
> Patch 8 and 9 introduce two new properties for the vendor structure,
> this is for SBSA functionality which cannot be controlled by
> separate uart_ops members only.
> Patch 10 then finally drops in the SBSA specific code, by providing
> a new uart_ops, vendor struct and probe function for it. Also the new
> device tree binding is documented.
> Patch 11 adds the necessary ACPI boilerplate.
>
> For testing you should be able to take any hardware which has a PL011
> and change the DT to use a "arm,sbsa-uart" compatible string and the
> baud rate with the "current-speed" property.
> Of course testing with a real SBSA Generic UART is welcomed - as well
> as regression testing with any PL011 implementation.
> I tested this on a FastModel, a Juno and a Midway machine, both in
> PL011 and in (DT-emulated) SBSA mode.
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
in ACPI case with device HID of ARMH0011 in DSDT.
Hanjun
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-13 14:32 ` Mark Langsdorf
@ 2015-05-21 11:13 ` Andre Przywara
2015-05-21 11:13 ` Andre Przywara
1 sibling, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-21 11:13 UTC (permalink / raw)
To: Mark Langsdorf
Cc: Russell King - ARM Linux, Arnd Bergmann, Graeme Gregory,
Jakub Kiciński, Greg Kroah-Hartman, linux-serial,
Jiri Slaby, Dave P Martin, linux-arm-kernel
Hi Mark,
(please keep the people on CC:, I almost missed that mail)
On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>> The ARM Server Base System Architecture[1] document describes a
>> generic UART which is a subset of the PL011 UART.
>> It lacks DMA support, baud rate control and modem status line
>> control, among other things.
>> The idea is to move the UART initialization and setup into the
>> firmware (which does this job today already) and let the kernel just
>> use the UART for sending and receiving characters.
>>
>> We use the recent refactoring to build a new struct uart_ops
>> variable which points to some new functions avoiding access to the
>> missing registers. We reuse as much existing PL011 code as possible.
>>
>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>> PrimeCell relations, so we go with a pretty generic probe function
>> which only uses platform device functions.
>> A DT binding is provided with this patch, ACPI support is added in a
>> separate one.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>> drivers/tty/serial/amba-pl011.c | 168
>> +++++++++++++++++++++
>> 2 files changed, 178 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> new file mode 100644
>> index 0000000..4163e7e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> @@ -0,0 +1,10 @@
>> +* ARM SBSA defined generic UART
>> +This UART uses a subset of the PL011 registers and consequently lives
>> +in the PL011 driver. It's baudrate and other communication parameters
>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>> +
>> +Required properties:
>> +- compatible: must be "arm,sbsa-uart"
>> +- reg: exactly one register range
>> +- interrupts: exactly one interrupt specifier
>> +- current-speed: the (fixed) baud rate set by the firmware
>> diff --git a/drivers/tty/serial/amba-pl011.c
>> b/drivers/tty/serial/amba-pl011.c
>> index 70e2958..cca93d9 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
....
>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>> struct ktermios *termios,
>> spin_unlock_irqrestore(&port->lock, flags);
>> }
>>
>> +static void
>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>> + struct ktermios *old)
>> +{
>> + struct uart_amba_port *uap =
>> + container_of(port, struct uart_amba_port, port);
>> + unsigned long flags;
>> +
>> + if (old)
>> + tty_termios_copy_hw(termios, old);
>
> This code prevented login via the serial console on our test hardware.
>
> Mark Salter suggested the following patch:
>
> - if (old)
> + /*
> + * The first call to set_termios() comes when the console is
> + * registered via uart_add_one_port(). The serial core will
> + * pass in a dummy old termios rather than NULL. Check to make
> + * sure the old termios has reasonable info before copying from
> + * it.
> + */
> + if (old && old->c_cflag)
> tty_termios_copy_hw(termios, old);
That seems like a kludge to me.
Regardless of the reason why uart_send_options() is passing a
dummy variable instead of a NULL pointer (which documentation explicitly
declares as legal) I saw that this unconditional copying of the old
value being common practise in various drivers, actually the default for
drivers not implementing set_termios at all.
Can you describe a setup where this issue shows up? I haven't observed
this issue in my testing.
But in fact I think we actually shouldn't care about the old value at
all. Instead we should filter the new termios settings to avoid userland
configuring for instance flow control or an differing byte format which
the SBSA UART does not support.
I will cook up a patch and send out a v5 ASAP.
Cheers,
Andre.
>
> which fixed the issue.
>
>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>> uap->fixed_baud);
>> +
>> + spin_lock_irqsave(&port->lock, flags);
>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>> + pl011_setup_status_masks(port, termios);
>> + spin_unlock_irqrestore(&port->lock, flags);
>> +}
>> +
>> static const char *pl011_type(struct uart_port *port)
>> {
>> struct uart_amba_port *uap =
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
@ 2015-05-21 11:13 ` Andre Przywara
0 siblings, 0 replies; 42+ messages in thread
From: Andre Przywara @ 2015-05-21 11:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
(please keep the people on CC:, I almost missed that mail)
On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>> The ARM Server Base System Architecture[1] document describes a
>> generic UART which is a subset of the PL011 UART.
>> It lacks DMA support, baud rate control and modem status line
>> control, among other things.
>> The idea is to move the UART initialization and setup into the
>> firmware (which does this job today already) and let the kernel just
>> use the UART for sending and receiving characters.
>>
>> We use the recent refactoring to build a new struct uart_ops
>> variable which points to some new functions avoiding access to the
>> missing registers. We reuse as much existing PL011 code as possible.
>>
>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>> PrimeCell relations, so we go with a pretty generic probe function
>> which only uses platform device functions.
>> A DT binding is provided with this patch, ACPI support is added in a
>> separate one.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>> drivers/tty/serial/amba-pl011.c | 168
>> +++++++++++++++++++++
>> 2 files changed, 178 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> new file mode 100644
>> index 0000000..4163e7e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>> @@ -0,0 +1,10 @@
>> +* ARM SBSA defined generic UART
>> +This UART uses a subset of the PL011 registers and consequently lives
>> +in the PL011 driver. It's baudrate and other communication parameters
>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>> +
>> +Required properties:
>> +- compatible: must be "arm,sbsa-uart"
>> +- reg: exactly one register range
>> +- interrupts: exactly one interrupt specifier
>> +- current-speed: the (fixed) baud rate set by the firmware
>> diff --git a/drivers/tty/serial/amba-pl011.c
>> b/drivers/tty/serial/amba-pl011.c
>> index 70e2958..cca93d9 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
....
>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>> struct ktermios *termios,
>> spin_unlock_irqrestore(&port->lock, flags);
>> }
>>
>> +static void
>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>> + struct ktermios *old)
>> +{
>> + struct uart_amba_port *uap =
>> + container_of(port, struct uart_amba_port, port);
>> + unsigned long flags;
>> +
>> + if (old)
>> + tty_termios_copy_hw(termios, old);
>
> This code prevented login via the serial console on our test hardware.
>
> Mark Salter suggested the following patch:
>
> - if (old)
> + /*
> + * The first call to set_termios() comes when the console is
> + * registered via uart_add_one_port(). The serial core will
> + * pass in a dummy old termios rather than NULL. Check to make
> + * sure the old termios has reasonable info before copying from
> + * it.
> + */
> + if (old && old->c_cflag)
> tty_termios_copy_hw(termios, old);
That seems like a kludge to me.
Regardless of the reason why uart_send_options() is passing a
dummy variable instead of a NULL pointer (which documentation explicitly
declares as legal) I saw that this unconditional copying of the old
value being common practise in various drivers, actually the default for
drivers not implementing set_termios at all.
Can you describe a setup where this issue shows up? I haven't observed
this issue in my testing.
But in fact I think we actually shouldn't care about the old value at
all. Instead we should filter the new termios settings to avoid userland
configuring for instance flow control or an differing byte format which
the SBSA UART does not support.
I will cook up a patch and send out a v5 ASAP.
Cheers,
Andre.
>
> which fixed the issue.
>
>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>> uap->fixed_baud);
>> +
>> + spin_lock_irqsave(&port->lock, flags);
>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>> + pl011_setup_status_masks(port, termios);
>> + spin_unlock_irqrestore(&port->lock, flags);
>> +}
>> +
>> static const char *pl011_type(struct uart_port *port)
>> {
>> struct uart_amba_port *uap =
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-21 11:13 ` Andre Przywara
@ 2015-05-21 12:49 ` Naresh Bhat
-1 siblings, 0 replies; 42+ messages in thread
From: Naresh Bhat @ 2015-05-21 12:49 UTC (permalink / raw)
To: Andre Przywara
Cc: Mark Langsdorf, Russell King - ARM Linux, Arnd Bergmann,
Graeme Gregory, Jakub Kiciński, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, Dave P Martin, linux-arm-kernel
On Thu, May 21, 2015 at 4:43 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Mark,
>
> (please keep the people on CC:, I almost missed that mail)
>
> On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
>> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>>> The ARM Server Base System Architecture[1] document describes a
>>> generic UART which is a subset of the PL011 UART.
>>> It lacks DMA support, baud rate control and modem status line
>>> control, among other things.
>>> The idea is to move the UART initialization and setup into the
>>> firmware (which does this job today already) and let the kernel just
>>> use the UART for sending and receiving characters.
>>>
>>> We use the recent refactoring to build a new struct uart_ops
>>> variable which points to some new functions avoiding access to the
>>> missing registers. We reuse as much existing PL011 code as possible.
>>>
>>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>>> PrimeCell relations, so we go with a pretty generic probe function
>>> which only uses platform device functions.
>>> A DT binding is provided with this patch, ACPI support is added in a
>>> separate one.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>>> drivers/tty/serial/amba-pl011.c | 168
>>> +++++++++++++++++++++
>>> 2 files changed, 178 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> new file mode 100644
>>> index 0000000..4163e7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> @@ -0,0 +1,10 @@
>>> +* ARM SBSA defined generic UART
>>> +This UART uses a subset of the PL011 registers and consequently lives
>>> +in the PL011 driver. It's baudrate and other communication parameters
>>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>>> +
>>> +Required properties:
>>> +- compatible: must be "arm,sbsa-uart"
>>> +- reg: exactly one register range
>>> +- interrupts: exactly one interrupt specifier
>>> +- current-speed: the (fixed) baud rate set by the firmware
>>> diff --git a/drivers/tty/serial/amba-pl011.c
>>> b/drivers/tty/serial/amba-pl011.c
>>> index 70e2958..cca93d9 100644
>>> --- a/drivers/tty/serial/amba-pl011.c
>>> +++ b/drivers/tty/serial/amba-pl011.c
>
> ....
>
>>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>>> struct ktermios *termios,
>>> spin_unlock_irqrestore(&port->lock, flags);
>>> }
>>>
>>> +static void
>>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>> + struct ktermios *old)
>>> +{
>>> + struct uart_amba_port *uap =
>>> + container_of(port, struct uart_amba_port, port);
>>> + unsigned long flags;
>>> +
>>> + if (old)
>>> + tty_termios_copy_hw(termios, old);
>>
>> This code prevented login via the serial console on our test hardware.
>>
>> Mark Salter suggested the following patch:
>>
>> - if (old)
>> + /*
>> + * The first call to set_termios() comes when the console is
>> + * registered via uart_add_one_port(). The serial core will
>> + * pass in a dummy old termios rather than NULL. Check to make
>> + * sure the old termios has reasonable info before copying from
>> + * it.
>> + */
>> + if (old && old->c_cflag)
>> tty_termios_copy_hw(termios, old);
Yes, this fixes the problem I was seeing with Fedora21 on ThunderX.
>
> That seems like a kludge to me.
> Regardless of the reason why uart_send_options() is passing a
> dummy variable instead of a NULL pointer (which documentation explicitly
> declares as legal) I saw that this unconditional copying of the old
> value being common practise in various drivers, actually the default for
> drivers not implementing set_termios at all.
>
> Can you describe a setup where this issue shows up? I haven't observed
> this issue in my testing.
>
> But in fact I think we actually shouldn't care about the old value at
> all. Instead we should filter the new termios settings to avoid userland
> configuring for instance flow control or an differing byte format which
> the SBSA UART does not support.
> I will cook up a patch and send out a v5 ASAP.
>
> Cheers,
> Andre.
>
>>
>> which fixed the issue.
>>
>>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>>> uap->fixed_baud);
>>> +
>>> + spin_lock_irqsave(&port->lock, flags);
>>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>>> + pl011_setup_status_masks(port, termios);
>>> + spin_unlock_irqrestore(&port->lock, flags);
>>> +}
>>> +
>>> static const char *pl011_type(struct uart_port *port)
>>> {
>>> struct uart_amba_port *uap =
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
"For things to change, we must change"
-Naresh Bhat
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
@ 2015-05-21 12:49 ` Naresh Bhat
0 siblings, 0 replies; 42+ messages in thread
From: Naresh Bhat @ 2015-05-21 12:49 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 21, 2015 at 4:43 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Mark,
>
> (please keep the people on CC:, I almost missed that mail)
>
> On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
>> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>>> The ARM Server Base System Architecture[1] document describes a
>>> generic UART which is a subset of the PL011 UART.
>>> It lacks DMA support, baud rate control and modem status line
>>> control, among other things.
>>> The idea is to move the UART initialization and setup into the
>>> firmware (which does this job today already) and let the kernel just
>>> use the UART for sending and receiving characters.
>>>
>>> We use the recent refactoring to build a new struct uart_ops
>>> variable which points to some new functions avoiding access to the
>>> missing registers. We reuse as much existing PL011 code as possible.
>>>
>>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>>> PrimeCell relations, so we go with a pretty generic probe function
>>> which only uses platform device functions.
>>> A DT binding is provided with this patch, ACPI support is added in a
>>> separate one.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>>> drivers/tty/serial/amba-pl011.c | 168
>>> +++++++++++++++++++++
>>> 2 files changed, 178 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> new file mode 100644
>>> index 0000000..4163e7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> @@ -0,0 +1,10 @@
>>> +* ARM SBSA defined generic UART
>>> +This UART uses a subset of the PL011 registers and consequently lives
>>> +in the PL011 driver. It's baudrate and other communication parameters
>>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>>> +
>>> +Required properties:
>>> +- compatible: must be "arm,sbsa-uart"
>>> +- reg: exactly one register range
>>> +- interrupts: exactly one interrupt specifier
>>> +- current-speed: the (fixed) baud rate set by the firmware
>>> diff --git a/drivers/tty/serial/amba-pl011.c
>>> b/drivers/tty/serial/amba-pl011.c
>>> index 70e2958..cca93d9 100644
>>> --- a/drivers/tty/serial/amba-pl011.c
>>> +++ b/drivers/tty/serial/amba-pl011.c
>
> ....
>
>>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>>> struct ktermios *termios,
>>> spin_unlock_irqrestore(&port->lock, flags);
>>> }
>>>
>>> +static void
>>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>> + struct ktermios *old)
>>> +{
>>> + struct uart_amba_port *uap =
>>> + container_of(port, struct uart_amba_port, port);
>>> + unsigned long flags;
>>> +
>>> + if (old)
>>> + tty_termios_copy_hw(termios, old);
>>
>> This code prevented login via the serial console on our test hardware.
>>
>> Mark Salter suggested the following patch:
>>
>> - if (old)
>> + /*
>> + * The first call to set_termios() comes when the console is
>> + * registered via uart_add_one_port(). The serial core will
>> + * pass in a dummy old termios rather than NULL. Check to make
>> + * sure the old termios has reasonable info before copying from
>> + * it.
>> + */
>> + if (old && old->c_cflag)
>> tty_termios_copy_hw(termios, old);
Yes, this fixes the problem I was seeing with Fedora21 on ThunderX.
>
> That seems like a kludge to me.
> Regardless of the reason why uart_send_options() is passing a
> dummy variable instead of a NULL pointer (which documentation explicitly
> declares as legal) I saw that this unconditional copying of the old
> value being common practise in various drivers, actually the default for
> drivers not implementing set_termios at all.
>
> Can you describe a setup where this issue shows up? I haven't observed
> this issue in my testing.
>
> But in fact I think we actually shouldn't care about the old value at
> all. Instead we should filter the new termios settings to avoid userland
> configuring for instance flow control or an differing byte format which
> the SBSA UART does not support.
> I will cook up a patch and send out a v5 ASAP.
>
> Cheers,
> Andre.
>
>>
>> which fixed the issue.
>>
>>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>>> uap->fixed_baud);
>>> +
>>> + spin_lock_irqsave(&port->lock, flags);
>>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>>> + pl011_setup_status_masks(port, termios);
>>> + spin_unlock_irqrestore(&port->lock, flags);
>>> +}
>>> +
>>> static const char *pl011_type(struct uart_port *port)
>>> {
>>> struct uart_amba_port *uap =
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
"For things to change, we must change"
-Naresh Bhat
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
2015-05-21 11:13 ` Andre Przywara
@ 2015-05-21 15:12 ` Mark Langsdorf
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Langsdorf @ 2015-05-21 15:12 UTC (permalink / raw)
To: Andre Przywara
Cc: Russell King - ARM Linux, Arnd Bergmann, Graeme Gregory,
Jakub Kiciński, Greg Kroah-Hartman, linux-serial,
Jiri Slaby, Dave P Martin, linux-arm-kernel
On 05/21/2015 06:13 AM, Andre Przywara wrote:
> Hi Mark,
>
> (please keep the people on CC:, I almost missed that mail)
>
> On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
>> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>>> The ARM Server Base System Architecture[1] document describes a
>>> generic UART which is a subset of the PL011 UART.
>>> It lacks DMA support, baud rate control and modem status line
>>> control, among other things.
>>> The idea is to move the UART initialization and setup into the
>>> firmware (which does this job today already) and let the kernel just
>>> use the UART for sending and receiving characters.
>>>
>>> We use the recent refactoring to build a new struct uart_ops
>>> variable which points to some new functions avoiding access to the
>>> missing registers. We reuse as much existing PL011 code as possible.
>>>
>>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>>> PrimeCell relations, so we go with a pretty generic probe function
>>> which only uses platform device functions.
>>> A DT binding is provided with this patch, ACPI support is added in a
>>> separate one.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>>> drivers/tty/serial/amba-pl011.c | 168
>>> +++++++++++++++++++++
>>> 2 files changed, 178 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> new file mode 100644
>>> index 0000000..4163e7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> @@ -0,0 +1,10 @@
>>> +* ARM SBSA defined generic UART
>>> +This UART uses a subset of the PL011 registers and consequently lives
>>> +in the PL011 driver. It's baudrate and other communication parameters
>>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>>> +
>>> +Required properties:
>>> +- compatible: must be "arm,sbsa-uart"
>>> +- reg: exactly one register range
>>> +- interrupts: exactly one interrupt specifier
>>> +- current-speed: the (fixed) baud rate set by the firmware
>>> diff --git a/drivers/tty/serial/amba-pl011.c
>>> b/drivers/tty/serial/amba-pl011.c
>>> index 70e2958..cca93d9 100644
>>> --- a/drivers/tty/serial/amba-pl011.c
>>> +++ b/drivers/tty/serial/amba-pl011.c
>
> ....
>
>>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>>> struct ktermios *termios,
>>> spin_unlock_irqrestore(&port->lock, flags);
>>> }
>>>
>>> +static void
>>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>> + struct ktermios *old)
>>> +{
>>> + struct uart_amba_port *uap =
>>> + container_of(port, struct uart_amba_port, port);
>>> + unsigned long flags;
>>> +
>>> + if (old)
>>> + tty_termios_copy_hw(termios, old);
>>
>> This code prevented login via the serial console on our test hardware.
>>
>> Mark Salter suggested the following patch:
>>
>> - if (old)
>> + /*
>> + * The first call to set_termios() comes when the console is
>> + * registered via uart_add_one_port(). The serial core will
>> + * pass in a dummy old termios rather than NULL. Check to make
>> + * sure the old termios has reasonable info before copying from
>> + * it.
>> + */
>> + if (old && old->c_cflag)
>> tty_termios_copy_hw(termios, old);
>
> That seems like a kludge to me.
> Regardless of the reason why uart_send_options() is passing a
> dummy variable instead of a NULL pointer (which documentation explicitly
> declares as legal) I saw that this unconditional copying of the old
> value being common practise in various drivers, actually the default for
> drivers not implementing set_termios at all.
>
> Can you describe a setup where this issue shows up? I haven't observed
> this issue in my testing.
Since other people have said it:
AMD Seattle, booting with ACPI, connected via a serial console
and running Fedora. Systems boot just fine, but when you attempt
to log in, the first time you hit return it doesn't register
so you can't ask for your password. If you do hit return twice,
apparently both instances of the return are registered at the
same time and the system rejects your invalid password (less
sure of the mechanics here).
> But in fact I think we actually shouldn't care about the old value at
> all. Instead we should filter the new termios settings to avoid userland
> configuring for instance flow control or an differing byte format which
> the SBSA UART does not support.
> I will cook up a patch and send out a v5 ASAP.
Okay.
--Mark Langsdorf
>> which fixed the issue.
>>
>>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>>> uap->fixed_baud);
>>> +
>>> + spin_lock_irqsave(&port->lock, flags);
>>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>>> + pl011_setup_status_masks(port, termios);
>>> + spin_unlock_irqrestore(&port->lock, flags);
>>> +}
>>> +
>>> static const char *pl011_type(struct uart_port *port)
>>> {
>>> struct uart_amba_port *uap =
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART
@ 2015-05-21 15:12 ` Mark Langsdorf
0 siblings, 0 replies; 42+ messages in thread
From: Mark Langsdorf @ 2015-05-21 15:12 UTC (permalink / raw)
To: linux-arm-kernel
On 05/21/2015 06:13 AM, Andre Przywara wrote:
> Hi Mark,
>
> (please keep the people on CC:, I almost missed that mail)
>
> On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
>> On 05/12/2015 09:14 AM, Andre Przywara wrote:
>>> The ARM Server Base System Architecture[1] document describes a
>>> generic UART which is a subset of the PL011 UART.
>>> It lacks DMA support, baud rate control and modem status line
>>> control, among other things.
>>> The idea is to move the UART initialization and setup into the
>>> firmware (which does this job today already) and let the kernel just
>>> use the UART for sending and receiving characters.
>>>
>>> We use the recent refactoring to build a new struct uart_ops
>>> variable which points to some new functions avoiding access to the
>>> missing registers. We reuse as much existing PL011 code as possible.
>>>
>>> In contrast to the PL011 the SBSA UART does not define any AMBA or
>>> PrimeCell relations, so we go with a pretty generic probe function
>>> which only uses platform device functions.
>>> A DT binding is provided with this patch, ACPI support is added in a
>>> separate one.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++
>>> drivers/tty/serial/amba-pl011.c | 168
>>> +++++++++++++++++++++
>>> 2 files changed, 178 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> new file mode 100644
>>> index 0000000..4163e7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
>>> @@ -0,0 +1,10 @@
>>> +* ARM SBSA defined generic UART
>>> +This UART uses a subset of the PL011 registers and consequently lives
>>> +in the PL011 driver. It's baudrate and other communication parameters
>>> +cannot be adjusted at runtime, so it lacks a clock specifier here.
>>> +
>>> +Required properties:
>>> +- compatible: must be "arm,sbsa-uart"
>>> +- reg: exactly one register range
>>> +- interrupts: exactly one interrupt specifier
>>> +- current-speed: the (fixed) baud rate set by the firmware
>>> diff --git a/drivers/tty/serial/amba-pl011.c
>>> b/drivers/tty/serial/amba-pl011.c
>>> index 70e2958..cca93d9 100644
>>> --- a/drivers/tty/serial/amba-pl011.c
>>> +++ b/drivers/tty/serial/amba-pl011.c
>
> ....
>
>>> @@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,
>>> struct ktermios *termios,
>>> spin_unlock_irqrestore(&port->lock, flags);
>>> }
>>>
>>> +static void
>>> +sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>> + struct ktermios *old)
>>> +{
>>> + struct uart_amba_port *uap =
>>> + container_of(port, struct uart_amba_port, port);
>>> + unsigned long flags;
>>> +
>>> + if (old)
>>> + tty_termios_copy_hw(termios, old);
>>
>> This code prevented login via the serial console on our test hardware.
>>
>> Mark Salter suggested the following patch:
>>
>> - if (old)
>> + /*
>> + * The first call to set_termios() comes when the console is
>> + * registered via uart_add_one_port(). The serial core will
>> + * pass in a dummy old termios rather than NULL. Check to make
>> + * sure the old termios has reasonable info before copying from
>> + * it.
>> + */
>> + if (old && old->c_cflag)
>> tty_termios_copy_hw(termios, old);
>
> That seems like a kludge to me.
> Regardless of the reason why uart_send_options() is passing a
> dummy variable instead of a NULL pointer (which documentation explicitly
> declares as legal) I saw that this unconditional copying of the old
> value being common practise in various drivers, actually the default for
> drivers not implementing set_termios at all.
>
> Can you describe a setup where this issue shows up? I haven't observed
> this issue in my testing.
Since other people have said it:
AMD Seattle, booting with ACPI, connected via a serial console
and running Fedora. Systems boot just fine, but when you attempt
to log in, the first time you hit return it doesn't register
so you can't ask for your password. If you do hit return twice,
apparently both instances of the return are registered at the
same time and the system rejects your invalid password (less
sure of the mechanics here).
> But in fact I think we actually shouldn't care about the old value at
> all. Instead we should filter the new termios settings to avoid userland
> configuring for instance flow control or an differing byte format which
> the SBSA UART does not support.
> I will cook up a patch and send out a v5 ASAP.
Okay.
--Mark Langsdorf
>> which fixed the issue.
>>
>>> + tty_termios_encode_baud_rate(termios, uap->fixed_baud,
>>> uap->fixed_baud);
>>> +
>>> + spin_lock_irqsave(&port->lock, flags);
>>> + uart_update_timeout(port, CS8, uap->fixed_baud);
>>> + pl011_setup_status_masks(port, termios);
>>> + spin_unlock_irqrestore(&port->lock, flags);
>>> +}
>>> +
>>> static const char *pl011_type(struct uart_port *port)
>>> {
>>> struct uart_amba_port *uap =
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2015-05-21 15:12 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 14:14 [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 01/11] drivers: PL011: avoid potential unregister_driver call Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 02/11] drivers: PL011: refactor pl011_startup() Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 03/11] drivers: PL011: refactor pl011_shutdown() Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 04/11] drivers: PL011: refactor pl011_set_termios() Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 05/11] drivers: PL011: refactor pl011_probe() Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 06/11] drivers: PL011: replace UART_MIS reading with _RIS & _IMSC Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 07/11] drivers: PL011: move cts_event workaround into separate function Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 08/11] drivers: PL011: allow avoiding UART enabling/disabling Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 09/11] drivers: PL011: allow to supply fixed option string Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-12 14:14 ` [PATCH v4 10/11] drivers: PL011: add support for the ARM SBSA generic UART Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-13 14:32 ` Mark Langsdorf
2015-05-13 15:03 ` Graeme Gregory
2015-05-21 11:13 ` Andre Przywara
2015-05-21 11:13 ` Andre Przywara
2015-05-21 12:49 ` Naresh Bhat
2015-05-21 12:49 ` Naresh Bhat
2015-05-21 15:12 ` Mark Langsdorf
2015-05-21 15:12 ` Mark Langsdorf
2015-05-12 14:14 ` [PATCH v4 11/11] drivers: PL011: add ACPI probing for SBSA UART Andre Przywara
2015-05-12 14:14 ` Andre Przywara
2015-05-13 1:09 ` Hanjun Guo
2015-05-13 1:09 ` Hanjun Guo
2015-05-12 15:18 ` [PATCH v4 00/11] drivers: PL011: add ARM SBSA Generic UART support Jakub Kiciński
2015-05-12 15:18 ` Jakub Kiciński
2015-05-12 16:42 ` Robert Richter
2015-05-12 16:42 ` Robert Richter
2015-05-13 9:17 ` Lorenzo Pieralisi
2015-05-13 9:17 ` Lorenzo Pieralisi
2015-05-15 3:31 ` Hanjun Guo
2015-05-15 3:31 ` Hanjun Guo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.