* [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
2017-07-21 9:32 [RFC PATCH 0/4] serial: uartps: Dynamic allocation Michal Simek
@ 2017-07-21 9:32 ` Michal Simek
2017-07-21 15:47 ` Sören Brinkmann
2017-07-21 9:32 ` [RFC PATCH 2/4] serial: uartps: Use dynamic array for console port Michal Simek
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2017-07-21 9:32 UTC (permalink / raw)
To: linux-kernel, monstr, Alan Cox
Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
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.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
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(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fde55dcdea5a..4614349403c1 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1298,20 +1298,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
.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 = {
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
2017-07-21 9:32 ` [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver Michal Simek
@ 2017-07-21 15:47 ` Sören Brinkmann
2017-07-31 7:37 ` Michal Simek
0 siblings, 1 reply; 9+ messages in thread
From: Sören Brinkmann @ 2017-07-21 15:47 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, monstr, Alan Cox, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
On Fri, 2017-07-21 at 11:32:24 +0200, Michal Simek wrote:
> 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.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> 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.
Doesn't this break early console/printk? I would expect that the UART
initialization may happen later than console init.
Sören
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
2017-07-21 15:47 ` Sören Brinkmann
@ 2017-07-31 7:37 ` Michal Simek
0 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2017-07-31 7:37 UTC (permalink / raw)
To: Sören Brinkmann, Michal Simek
Cc: linux-kernel, monstr, Alan Cox, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
On 21.7.2017 17:47, Sören Brinkmann wrote:
> On Fri, 2017-07-21 at 11:32:24 +0200, Michal Simek wrote:
>> 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.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> 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.
>
> Doesn't this break early console/printk? I would expect that the UART
> initialization may happen later than console init.
as I said. I can't see any issue with it. Definitely please try.
Thanks,
Michal
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH 2/4] serial: uartps: Use dynamic array for console port
2017-07-21 9:32 [RFC PATCH 0/4] serial: uartps: Dynamic allocation Michal Simek
2017-07-21 9:32 ` [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver Michal Simek
@ 2017-07-21 9:32 ` Michal Simek
2017-07-21 9:32 ` [RFC PATCH 3/4] serial: uartps: Move cnds_uart_get_port to probe Michal Simek
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2017-07-21 9:32 UTC (permalink / raw)
To: linux-kernel, monstr, Alan Cox
Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
Driver 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.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/tty/serial/xilinx_uartps.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 4614349403c1..e6470a3111ce 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1211,6 +1211,10 @@ static int __init cdns_early_console_setup(struct earlycon_device *device,
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
@@ -1220,7 +1224,7 @@ static int __init cdns_early_console_setup(struct earlycon_device *device,
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;
@@ -1266,15 +1270,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);
@@ -1570,6 +1572,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,
@@ -1577,6 +1590,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:
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 3/4] serial: uartps: Move cnds_uart_get_port to probe
2017-07-21 9:32 [RFC PATCH 0/4] serial: uartps: Dynamic allocation Michal Simek
2017-07-21 9:32 ` [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver Michal Simek
2017-07-21 9:32 ` [RFC PATCH 2/4] serial: uartps: Use dynamic array for console port Michal Simek
@ 2017-07-21 9:32 ` Michal Simek
2017-07-21 9:32 ` [RFC PATCH 4/4] serial: uartps: Remove static port array Michal Simek
2017-07-28 18:39 ` [RFC PATCH 0/4] serial: uartps: Dynamic allocation Alan Cox
4 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2017-07-21 9:32 UTC (permalink / raw)
To: linux-kernel, monstr, Alan Cox
Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
c&p this function to probe as preparation for removing
cdns_uart_port[] static array.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/tty/serial/xilinx_uartps.c | 61 +++++++++++++-------------------------
1 file changed, 21 insertions(+), 40 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e6470a3111ce..4fb74baeae35 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1104,43 +1104,6 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
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 (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
@@ -1545,15 +1508,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 (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
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 4/4] serial: uartps: Remove static port array
2017-07-21 9:32 [RFC PATCH 0/4] serial: uartps: Dynamic allocation Michal Simek
` (2 preceding siblings ...)
2017-07-21 9:32 ` [RFC PATCH 3/4] serial: uartps: Move cnds_uart_get_port to probe Michal Simek
@ 2017-07-21 9:32 ` Michal Simek
2017-07-28 18:39 ` [RFC PATCH 0/4] serial: uartps: Dynamic allocation Alan Cox
4 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2017-07-21 9:32 UTC (permalink / raw)
To: linux-kernel, monstr, Alan Cox
Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
Allocate uart port structure dynamically.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/tty/serial/xilinx_uartps.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 4fb74baeae35..1c9ec8c4c2b6 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1102,8 +1102,6 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
#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
@@ -1443,6 +1441,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) {
@@ -1508,16 +1509,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 (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;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 0/4] serial: uartps: Dynamic allocation
2017-07-21 9:32 [RFC PATCH 0/4] serial: uartps: Dynamic allocation Michal Simek
` (3 preceding siblings ...)
2017-07-21 9:32 ` [RFC PATCH 4/4] serial: uartps: Remove static port array Michal Simek
@ 2017-07-28 18:39 ` Alan Cox
2017-07-31 7:42 ` Michal Simek
4 siblings, 1 reply; 9+ messages in thread
From: Alan Cox @ 2017-07-28 18:39 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, monstr, Sören Brinkmann, Jiri Slaby,
linux-serial, Greg Kroah-Hartman, linux-arm-kernel
On Fri, 21 Jul 2017 11:32:23 +0200
Michal Simek <michal.simek@xilinx.com> wrote:
> Hi Alan,
>
> 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.
>
> Thanks,
> Michal
Sorry for the delay been on jury service
Series
Reviewed-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 0/4] serial: uartps: Dynamic allocation
2017-07-28 18:39 ` [RFC PATCH 0/4] serial: uartps: Dynamic allocation Alan Cox
@ 2017-07-31 7:42 ` Michal Simek
0 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2017-07-31 7:42 UTC (permalink / raw)
To: Alan Cox, Michal Simek
Cc: linux-kernel, monstr, Sören Brinkmann, Jiri Slaby,
linux-serial, Greg Kroah-Hartman, linux-arm-kernel
On 28.7.2017 20:39, Alan Cox wrote:
> On Fri, 21 Jul 2017 11:32:23 +0200
> Michal Simek <michal.simek@xilinx.com> wrote:
>
>> Hi Alan,
>>
>> 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.
>>
>> Thanks,
>> Michal
>
> Sorry for the delay been on jury service
np at all.
Do you have any suggestion how to do the last step?
>
> Series
>
> Reviewed-by: Alan Cox <alan@linux.intel.com>
>
Thanks,
Michal
^ permalink raw reply [flat|nested] 9+ messages in thread