serial: uartps: Dynamic allocation
Subsystems (2)
cover serial: uartps: Dynamic allocation
Hi,
this is the initial version before next step which is move uart_register_driver to probe function.
I was able to get rid of static array with uart_port structures. It was wired with console which is also fixed.
And the next step is the most complicated one handle .nr in uart_driver structure in more generic way.
I am sending these patches as v1. RFC was sent in past
(https://www.spinics.net/lists/linux-serial/msg27106.html)
and I promised to wait till Geert's
"serial: xuartps: Fix out-of-bounds access through DT alias" (sha1: e7d75e18d0fc3f7193b65282b651f980c778d935) is applied.
Thanks,
Michal
Michal Simek (4):
serial: uartps: Remove console_initcall from the driver
serial: uartps: Use dynamic array for console port
serial: uartps: Move cnds_uart_get_port to probe
serial: uartps: Remove static port array
drivers/tty/serial/xilinx_uartps.c | 102 +++++++++++------------------ 1 file changed, 40 insertions(+), 62 deletions(-)
Signature · 1 line
2.17.0
PATCH 1/4 serial: uartps: Remove console_initcall from the driver
register_console() is called from
uart_add_one_port()->uart_configure_port()
that's why register_console() is called twice.
This patch remove console_initcall to call register_console() only from one location.
Also based on my tests cdns_uart_console_setup() is not called from the first register_console() call.
---
I am not quite sure about this because console_initcall is called early but I can see any difference in usage.
pl011 is not calling this but others are doing it.
--- drivers/tty/serial/xilinx_uartps.c | 14 -------------- 1 file changed, 14 deletions(-)
drivers/tty/serial/xilinx_uartps.c +0 -14
@@ -1293,20 +1293,6 @@ static struct console cdns_uart_console = { .index = -1, /* Specified on the cmdline (e.g. console=ttyPS ) */ .data = &cdns_uart_uart_driver, }; - -/** -* cdns_uart_console_init - Initialization call -* -* Return: 0 on success, negative errno otherwise -*/ -static int __init cdns_uart_console_init (void ) -{ -register_console (&cdns_uart_console); -return 0 ; -} - -console_initcall (cdns_uart_console_init); -#endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */ static struct uart_driver cdns_uart_uart_driver = {
Signature · 1 line
2.17.0
PATCH 2/4 serial: uartps: Use dynamic array for console port
In reply to Michal SimekDriver console functions are using pointer to static array with fixed size. There can be only one serial console at the time which is found by register_console(). register_console() is filling cons->index to port->line value.
---
drivers/tty/serial/xilinx_uartps.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-)
drivers/tty/serial/xilinx_uartps.c +24 -5
@@ -1206,6 +1206,10 @@ OF_EARLYCON_DECLARE(cdns, "cdns,uart-r1p8", cdns_early_console_setup);OF_EARLYCON_DECLARE (cdns,"cdns,uart-r1p12" , cdns_early_console_setup);OF_EARLYCON_DECLARE (cdns,"xlnx,zynqmp-uart" , cdns_early_console_setup); + +/* Static pointer to console port */ +static struct uart_port *console_port; +/** * cdns_uart_console_write - perform write operation * @co: Console handle
@@ -1215,7 +1219,7 @@ OF_EARLYCON_DECLARE(cdns, "xlnx,zynqmp-uart", cdns_early_console_setup);static void cdns_uart_console_write (struct console *co,const char *s,unsigned int count) { -struct uart_port *port = &cdns_uart_port[co->index]; +struct uart_port *port = console_port;unsigned long flags;unsigned int imr, ctrl;int locked =1 ;
@@ -1261,15 +1265,13 @@ static void cdns_uart_console_write(struct console *co, const char *s,*/ static int __init cdns_uart_console_setup (struct console *co,char *options) { -struct uart_port *port = &cdns_uart_port[co->index]; +struct uart_port *port = console_port; +int baud =9600 ;int bits =8 ;int parity ='n' ;int flow ='n' ; -if (co->index <0 || co->index >=CDNS_UART_NR_PORTS ) -return -EINVAL ; -if (!port->membase) {pr_debug ("console on " CDNS_UART_TTY_NAME "%i not present\n" , co->index);
@@ -1565,6 +1567,17 @@ static int cdns_uart_probe(struct platform_device *pdev)pm_runtime_set_active (&pdev->dev);pm_runtime_enable (&pdev->dev); +#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE +/* +* If console hasn't been found yet try to assign this port +* because it is required to be assigned for console setup function. +* If register_console() don't assign value, then console_port pointer +* is cleanup. +*/ +if (cdns_uart_uart_driver.cons->index ==-1 ) + console_port = port; +#endif + rc =uart_add_one_port (&cdns_uart_uart_driver, port);if (rc) { dev_err(&pdev->dev,
@@ -1572,6 +1585,12 @@ static int cdns_uart_probe(struct platform_device *pdev) goto err_out_pm_disable; } +#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE +/* This is not port which is used for console that's why clean it up */ +if (cdns_uart_uart_driver.cons->index ==-1 ) + console_port =NULL ; +#endif +return 0 ; err_out_pm_disable:
Signature · 1 line
2.17.0
PATCH 3/4 serial: uartps: Move cnds_uart_get_port to probe
In reply to Michal Simekc&p this function to probe as preparation for removing cdns_uart_port[] static array.
---
drivers/tty/serial/xilinx_uartps.c | 61 ++++++++++-------------------- 1 file changed, 21 insertions(+), 40 deletions(-)
drivers/tty/serial/xilinx_uartps.c +21 -40
@@ -1099,43 +1099,6 @@ static const struct uart_ops cdns_uart_ops = {static struct uart_port cdns_uart_port[CDNS_UART_NR_PORTS ]; -/** -* cdns_uart_get_port - Configure the port from platform device resource info -* @id: Port id -* -* Return: a pointer to a uart_port or NULL for failure -*/ -static struct uart_port *cdns_uart_get_port (int id) -{ -struct uart_port *port; - -/* Try the given port id if failed use default method */ -if (id <CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase !=0 ) { -/* Find the next unused port */ -for (id =0 ; id <CDNS_UART_NR_PORTS ; id++) -if (cdns_uart_port[id].mapbase ==0 ) -break ; - } - -if (id >=CDNS_UART_NR_PORTS ) -return NULL ; - - port = &cdns_uart_port[id]; - -/* At this point, we've got an empty uart_port struct, initialize it */ -spin_lock_init (&port->lock); - port->membase =NULL ; - port->irq =0 ; - port->type =PORT_UNKNOWN ; - port->iotype =UPIO_MEM32 ; - port->flags =UPF_BOOT_AUTOCONF ; - port->ops = &cdns_uart_ops; - port->fifosize =CDNS_UART_FIFO_SIZE ; - port->line = id; - port->dev =NULL ; -return port; -} -#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE /** * cdns_uart_console_wait_tx - Wait for the TX to be full
@@ -1540,15 +1503,33 @@ static int cdns_uart_probe(struct platform_device *pdev)if (id <0 ) id =0 ; -/* Initialize the port structure */ - port =cdns_uart_get_port (id); +/* Try the given port id if failed use default method */ +if (id <CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase !=0 ) { +/* Find the next unused port */ +for (id =0 ; id <CDNS_UART_NR_PORTS ; id++) +if (cdns_uart_port[id].mapbase ==0 ) +break ; + } -if (!port) { + port = &cdns_uart_port[id]; +if (!port || id >=CDNS_UART_NR_PORTS ) {dev_err (&pdev->dev,"Cannot get uart_port structure\n" ); rc = -ENODEV ; goto err_out_notif_unreg; } +/* At this point, we've got an empty uart_port struct, initialize it */ +spin_lock_init (&port->lock); + port->membase =NULL ; + port->irq =0 ; + port->type =PORT_UNKNOWN ; + port->iotype =UPIO_MEM32 ; + port->flags =UPF_BOOT_AUTOCONF ; + port->ops = &cdns_uart_ops; + port->fifosize =CDNS_UART_FIFO_SIZE ; + port->line = id; + port->dev =NULL ; +/* * Register the port. * This function also registers this device with the tty layer
Signature · 1 line
2.17.0
PATCH 4/4 serial: uartps: Remove static port array
In reply to Michal SimekAllocate uart port structure dynamically.
---
drivers/tty/serial/xilinx_uartps.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/tty/serial/xilinx_uartps.c +4 -12
@@ -1097,8 +1097,6 @@ static const struct uart_ops cdns_uart_ops = {#endif }; -static struct uart_port cdns_uart_port[CDNS_UART_NR_PORTS ]; -#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE /** * cdns_uart_console_wait_tx - Wait for the TX to be full
@@ -1438,6 +1436,9 @@ static int cdns_uart_probe(struct platform_device *pdev) GFP_KERNEL);if (!cdns_uart_data)return -ENOMEM ; + port =devm_kzalloc (&pdev->dev,sizeof (*port),GFP_KERNEL ); +if (!port) +return -ENOMEM ; match =of_match_node (cdns_uart_of_match, pdev->dev.of_node);if (match && match->data) {
@@ -1503,16 +1504,7 @@ static int cdns_uart_probe(struct platform_device *pdev)if (id <0 ) id =0 ; -/* Try the given port id if failed use default method */ -if (id <CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase !=0 ) { -/* Find the next unused port */ -for (id =0 ; id <CDNS_UART_NR_PORTS ; id++) -if (cdns_uart_port[id].mapbase ==0 ) -break ; - } - - port = &cdns_uart_port[id]; -if (!port || id >=CDNS_UART_NR_PORTS ) { +if (id >=CDNS_UART_NR_PORTS ) {dev_err (&pdev->dev,"Cannot get uart_port structure\n" ); rc = -ENODEV ; goto err_out_notif_unreg;
Signature · 1 line
2.17.0