drivers: PL011: add ARM SBSA Generic UART support
Subsystems (3)
v3 cover drivers: PL011: add ARM SBSA Generic UART support
This is the third revision of the SBSA UART support series. It is based on tty-next plus Dave's latest PL011 work[2], which fixes all problems seen on the fast models and on some hardware. Compared to v2 building as a module has been fixed.
----
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.
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 binding, but can easily be adapted to other device configuration systems.
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 relies on Dave's recent PL011 fix[2], which gets rid of the loopback trick to get the UART going. There is a repo at [3] (branch sbsa-uart/v3), which has all patches already integrated.
Patch 1/10 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.
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.
Changelog:
v2 .. v3:
- rebased on top of tty-next and Dave's latest PL011 rework[2]
- 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 [4]
- 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://lists.infradead.org/pipermail/linux-arm-kernel/2015-April/334975.html
[3] http://www.linux-arm.org/git?p=linux-ap.git
git://linux-arm.org/linux-ap.git
[4] 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
.../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 + drivers/tty/serial/amba-pl011.c | 529 ++++++++++++++------ 2 files changed, 398 insertions(+), 141 deletions(-) create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
Signature · 1 line
1.7.9.5
PATCH v3 1/10 drivers: PL011: avoid potential unregister_driver call
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.
--- drivers/tty/serial/amba-pl011.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)
drivers/tty/serial/amba-pl011.c +21 -16
@@ -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 ; }
Signature · 1 line
1.7.9.5
PATCH v3 1/10 drivers: PL011: avoid potential unregister_driver call
Quoted 29 lines · Andre Przywara
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_probefunction. 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 <[email protected]>--- drivers/tty/serial/amba-pl011.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)drivers/tty/serial/amba-pl011.c +8
-0@@ -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)
drivers/tty/serial/amba-pl011.c static int pl011_probe_dt_alias(int index, struct device *dev) · line 2155
Quoted 10 lines
drivers/tty/serial/amba-pl011.c +10
-0+ amba_ports[i] =NULL ; + else if (amba_ports[i]) + busy = true; + } +pl011_dma_remove (uap); +if (!busy) +uart_unregister_driver (&amba_reg); +} + +
Double new line.
PATCH v3 2/10 drivers: PL011: refactor pl011_startup()
In reply to Andre PrzywaraSplit the pl011_startup() function into smaller chunks to allow easier reuse later when adding SBSA support.
--- drivers/tty/serial/amba-pl011.c | 48 +++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 20 deletions(-)
drivers/tty/serial/amba-pl011.c +28 -20
@@ -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 ;
Signature · 1 line
1.7.9.5
PATCH v3 3/10 drivers: PL011: refactor pl011_shutdown()
In reply to Andre PrzywaraSplit the pl011_shutdown() function into smaller chunks to allow easier reuse later when adding SBSA support.
--- drivers/tty/serial/amba-pl011.c | 58 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 25 deletions(-)
drivers/tty/serial/amba-pl011.c +33 -25
@@ -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,33 @@ 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 & ~UART011_TXIS , 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
Signature · 1 line
1.7.9.5
PATCH v3 3/10 drivers: PL011: refactor pl011_shutdown()
Quoted 55 lines · Andre Przywara
Split the
pl011_shutdown()function into smaller chunks to allow easier reuse later when adding SBSA support.Signed-off-by Andre Przywara <[email protected]>--- drivers/tty/serial/amba-pl011.c | 58 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 25 deletions(-)drivers/tty/serial/amba-pl011.c +6
-25@@ -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,33 @@ static void pl011_shutdown(struct uart_port *port)pl011_shutdown_channel (uap, uap->lcrh_rx);
drivers/tty/serial/amba-pl011.c static void pl011_shutdown(struct uart_port *port) · line 1669
Quoted 10 lines
drivers/tty/serial/amba-pl011.c +8
-0if (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 +*/
No need for a multi-line comment here.
drivers/tty/serial/amba-pl011.c static void pl011_shutdown(struct uart_port *port) · line 1672
Quoted 3 lines
drivers/tty/serial/amba-pl011.c +3
-0+ uap->im =0 ; +writew (uap->im, uap->port.membase +UART011_IMSC ); +writew (0xffff & ~UART011_TXIS , uap->port.membase +UART011_ICR );
The line you removed was:
writew(0xffff, uap->port.membase + UART011_ICR);
Please rebase more carefully.
PATCH v3 4/10 drivers: PL011: refactor pl011_set_termios()
In reply to Andre PrzywaraSplit the pl011_set_termios() function into smaller chunks to allow easier reuse later when adding SBSA support.
--- drivers/tty/serial/amba-pl011.c | 60 +++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 27 deletions(-)
drivers/tty/serial/amba-pl011.c +33 -27
@@ -1707,6 +1707,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) {
@@ -1770,33 +1802,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);
Signature · 1 line
1.7.9.5
PATCH v3 5/10 drivers: PL011: refactor pl011_probe()
In reply to Andre PrzywaraCurrently 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.
--- drivers/tty/serial/amba-pl011.c | 89 ++++++++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 29 deletions(-)
drivers/tty/serial/amba-pl011.c +60 -29
@@ -2174,13 +2174,10 @@ 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_allocate_port (struct device *dev,struct uart_amba_port **_uap) {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 )
@@ -2189,51 +2186,53 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)if (i ==ARRAY_SIZE (amba_ports))return -EBUSY ; - uap =devm_kzalloc (&dev->dev,sizeof (struct uart_amba_port ), -GFP_KERNEL ); + uap =devm_kzalloc (dev,sizeof (struct uart_amba_port ),GFP_KERNEL );if (uap ==NULL )return -ENOMEM ; - i =pl011_probe_dt_alias (i, &dev->dev); +if (_uap) + *_uap = uap; + +return i; +} - base =devm_ioremap (&dev->dev, dev->res.start, -resource_size (&dev->res)); +static int pl011_setup_port (struct device *dev,struct uart_amba_port *uap, +struct resource *mmiobase,int index) +{ +void __iomem *base; + + 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; }
@@ -2246,6 +2245,38 @@ 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_allocate_port (&dev->dev, &uap); +if (portnr <0 ) +return portnr; + + 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);
Signature · 1 line
1.7.9.5
PATCH v3 5/10 drivers: PL011: refactor pl011_probe()
Quoted 38 lines · Andre Przywara
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 <[email protected]>--- drivers/tty/serial/amba-pl011.c | 89 ++++++++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 29 deletions(-)drivers/tty/serial/amba-pl011.c +2
-7@@ -2174,13 +2174,10 @@ 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_allocate_port (struct device *dev,struct uart_amba_port **_uap) {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 )@@ -2189,51 +2186,53 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)if (i ==ARRAY_SIZE (amba_ports))return -EBUSY ; - uap =devm_kzalloc (&dev->dev,sizeof (struct uart_amba_port ), -GFP_KERNEL );
drivers/tty/serial/amba-pl011.c static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 2197
Quoted 10 lines
drivers/tty/serial/amba-pl011.c +6
-1+ uap =devm_kzalloc (dev,sizeof (struct uart_amba_port ),GFP_KERNEL );if (uap ==NULL )return -ENOMEM ; - i =pl011_probe_dt_alias (i, &dev->dev); +if (_uap) + *_uap = uap; + +return i; +}
Could we please move the allocation out of this function and spare ourselves the "out argument". It's just a matter of personal preference so the final decision is up to you.
Also you never pass _uap as NULL so the check is unnecessary.
PATCH v3 6/10 drivers: PL011: replace UART_MIS reading with _RIS & _IMSC
In reply to Andre PrzywaraThe 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.
--- drivers/tty/serial/amba-pl011.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/tty/serial/amba-pl011.c +4 -2
@@ -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 ; }
Signature · 1 line
1.7.9.5
PATCH v3 7/10 drivers: PL011: move cts_event workaround into separate function
In reply to Andre PrzywaraTo avoid lines with more than 80 characters and to make the pl011_int() function more readable, move the workaround out into a separate function.
--- drivers/tty/serial/amba-pl011.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
drivers/tty/serial/amba-pl011.c +20 -13
@@ -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),
Signature · 1 line
1.7.9.5
PATCH v3 8/10 drivers: PL011: allow avoiding UART enabling/disabling
In reply to Andre PrzywaraThe 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.
--- drivers/tty/serial/amba-pl011.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/tty/serial/amba-pl011.c +12 -6
@@ -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, };
@@ -1959,7 +1962,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 ;
@@ -1976,10 +1979,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);
@@ -1990,7 +1995,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);
Signature · 1 line
1.7.9.5
PATCH v3 9/10 drivers: PL011: allow to supply fixed option string
In reply to Andre PrzywaraThe 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.
--- drivers/tty/serial/amba-pl011.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
drivers/tty/serial/amba-pl011.c +13 -4
@@ -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 */
@@ -2077,10 +2081,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); }
Signature · 1 line
1.7.9.5
PATCH v3 10/10 drivers: PL011: add support for the ARM SBSA generic UART
In reply to Andre PrzywaraThe 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, but other systems can easily attach to it, too (hint, hint!).
--- .../devicetree/bindings/serial/arm_sbsa_uart.txt | 10 ++ drivers/tty/serial/amba-pl011.c | 165 ++++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt new file +10 -0
@@ -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
drivers/tty/serial/amba-pl011.c +165 -0
212 lines not on this page · raw message
Signature · 1 line
1.7.9.5