[PATCH v3 00/10] serial: uartps: Add run time support for more IPs than hardcoded 2

[PATCH v3 00/10] serial: uartps: Add run time support for more IPs than hardcoded 2

[Michal Simek]

Hi,

this series is trying to address discussion I had with Alan in past
https://patchwork.kernel.org/patch/9738445/ and also with Rob in v1
https://lkml.org/lkml/2018/4/26/551.

These patches are doing preparation to enable dynamic ID port allocation
which is capable to create devices with higher IDs.

For example this is how it works.
uart0 on higher alias
serial0 = &uart1;
serial30 = &uart0;

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jun  6 12:19 /dev/ttyPS0
crw--w----    1 root     root      253, 100 Jan  1  1970 /dev/ttyPS30

Thanks,
Michal

Changes in v3:
- New patch found by testing
- New patch - can be sent separately but there is dependency that's why
  sent in this series
- Fix uart_unregister_driver() in error path or when driver is removed.
- Change commit message
- s/,/;/ in filling structure
- get cdns_uart_uart_driver out of PS_UART_CONSOLE ifdefs
- New patch in series
- Rebase on the top of previous broken patch
- Change patch subject ("was serial: uartps: Remove CDNS_UART_NR_PORTS
  macro")
- Keep CDNS_UART_NR_PORTS in this patch and remove it in next one and
  align commit message to reflect this
- Allocate struct console dynamically too to be unique for every
  instance
- Cleanup error path
- New patch in series

Changes in v2:
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- Remove nr field logic
- new patch - it can be sent separately too
- Register one uart_driver with unique minor at probe time

Michal Simek (9):
  serial: uartps: console_setup() can't be placed to init section
  serial: uartps: Do not initialize field to zero again
  serial: uartps: Do not use static struct uart_driver out of probe()
  serial: uartps: Move alias reading higher in probe()
  serial: uartps: Move register to probe based on run time detection
  serial: uartps: Fill struct uart_driver in probe()
  serial: uartps: Change logic how console_port is setup
  serial: uartps: Register own uart console and driver structures
  serial: uartps: Move Port ID to device data structure

Nava kishore Manne (1):
  serial: uartps: Fix suspend functionality

 drivers/tty/serial/xilinx_uartps.c | 203 +++++++++++++++++++------------------
 1 file changed, 107 insertions(+), 96 deletions(-)

-- 
1.9.1

[PATCH v3 01/10] serial: uartps: console_setup() can't be placed to init section

[Michal Simek]

When console device is rebinded, console_setup() is called again.
But marking it as __init means that function will be clear after boot is
complete. If console device is binded again console_setup() is not found
and error "Unable to handle kernel paging request at virtual address"
is reported.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- New patch found by testing

Changes in v2: None

 drivers/tty/serial/xilinx_uartps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e5d90611c03f..dd905c981f67 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1216,7 +1216,7 @@ static void cdns_uart_console_write(struct console *co, const char *s,
  *
  * Return: 0 on success, negative errno otherwise.
  */
-static int __init cdns_uart_console_setup(struct console *co, char *options)
+static int cdns_uart_console_setup(struct console *co, char *options)
 {
 	struct uart_port *port = console_port;
 
-- 
1.9.1

[PATCH v3 02/10] serial: uartps: Do not initialize field to zero again

[Michal Simek]

Writing zero and NULLs to already initialized fields is not needed.
Remove this additional writes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3: None
Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index dd905c981f67..9a08542cec47 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1502,15 +1502,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 	/* 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.
-- 
1.9.1

[PATCH v3 03/10] serial: uartps: Fix suspend functionality

[Michal Simek]

From: Nava kishore Manne <nava.manne@xilinx.com>

The driver's suspend/resume functions were buggy.
If UART node contains any child node in the DT and
the child is established a communication path with
the parent UART. The relevant /dev/ttyPS* node will
be not available for other operations.
If the driver is trying to do any operations like
suspend/resume without checking the tty->dev status
it leads to the kernel crash/hang.

This patch fix this issue by call the device_may_wake()
with the generic parameter of type struct device.
in the uart suspend and resume paths.

It also fixes a race condition in the uart suspend
path(i.e uart_suspend_port() should be called at the
end of cdns_uart_suspend API this path updates the same)

Signed-off-by: Nava kishore Manne <navam@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- New patch - can be sent separately but there is dependency that's why
  sent in this series

Changes in v2: None

 drivers/tty/serial/xilinx_uartps.c | 41 +++++++++++---------------------------
 1 file changed, 12 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 9a08542cec47..fc0ecaf3615d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1272,24 +1272,11 @@ static int cdns_uart_console_setup(struct console *co, char *options)
 static int cdns_uart_suspend(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
-	struct tty_struct *tty;
-	struct device *tty_dev;
-	int may_wake = 0;
-
-	/* Get the tty which could be NULL so don't assume it's valid */
-	tty = tty_port_tty_get(&port->state->port);
-	if (tty) {
-		tty_dev = tty->dev;
-		may_wake = device_may_wakeup(tty_dev);
-		tty_kref_put(tty);
-	}
+	int may_wake;
 
-	/*
-	 * Call the API provided in serial_core.c file which handles
-	 * the suspend.
-	 */
-	uart_suspend_port(&cdns_uart_uart_driver, port);
-	if (!(console_suspend_enabled && !may_wake)) {
+	may_wake = device_may_wakeup(device);
+
+	if (console_suspend_enabled && may_wake) {
 		unsigned long flags = 0;
 
 		spin_lock_irqsave(&port->lock, flags);
@@ -1304,7 +1291,11 @@ static int cdns_uart_suspend(struct device *device)
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
-	return 0;
+	/*
+	 * Call the API provided in serial_core.c file which handles
+	 * the suspend.
+	 */
+	return uart_suspend_port(&cdns_uart_uart_driver, port);
 }
 
 /**
@@ -1318,17 +1309,9 @@ static int cdns_uart_resume(struct device *device)
 	struct uart_port *port = dev_get_drvdata(device);
 	unsigned long flags = 0;
 	u32 ctrl_reg;
-	struct tty_struct *tty;
-	struct device *tty_dev;
-	int may_wake = 0;
-
-	/* Get the tty which could be NULL so don't assume it's valid */
-	tty = tty_port_tty_get(&port->state->port);
-	if (tty) {
-		tty_dev = tty->dev;
-		may_wake = device_may_wakeup(tty_dev);
-		tty_kref_put(tty);
-	}
+	int may_wake;
+
+	may_wake = device_may_wakeup(device);
 
 	if (console_suspend_enabled && !may_wake) {
 		struct cdns_uart *cdns_uart = port->private_data;
-- 
1.9.1

[PATCH v3 04/10] serial: uartps: Do not use static struct uart_driver out of probe()

[Michal Simek]

cdns_uart_suspend()/resume() and remove() are using static reference
to struct uart_driver. Assign this reference to private data structure
as preparation step for dynamic struct uart_driver allocation.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3: None
Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fc0ecaf3615d..5bd2b0607df6 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -180,6 +180,7 @@
  * @port:		Pointer to the UART port
  * @uartclk:		Reference clock
  * @pclk:		APB clock
+ * @cdns_uart_driver:	Pointer to UART driver
  * @baud:		Current baud rate
  * @clk_rate_change_nb:	Notifier block for clock changes
  * @quirks:		Flags for RXBS support.
@@ -188,6 +189,7 @@ struct cdns_uart {
 	struct uart_port	*port;
 	struct clk		*uartclk;
 	struct clk		*pclk;
+	struct uart_driver	*cdns_uart_driver;
 	unsigned int		baud;
 	struct notifier_block	clk_rate_change_nb;
 	u32			quirks;
@@ -1272,6 +1274,7 @@ static int cdns_uart_console_setup(struct console *co, char *options)
 static int cdns_uart_suspend(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	int may_wake;
 
 	may_wake = device_may_wakeup(device);
@@ -1295,7 +1298,7 @@ static int cdns_uart_suspend(struct device *device)
 	 * Call the API provided in serial_core.c file which handles
 	 * the suspend.
 	 */
-	return uart_suspend_port(&cdns_uart_uart_driver, port);
+	return uart_suspend_port(cdns_uart->cdns_uart_driver, port);
 }
 
 /**
@@ -1307,6 +1310,7 @@ static int cdns_uart_suspend(struct device *device)
 static int cdns_uart_resume(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	unsigned long flags = 0;
 	u32 ctrl_reg;
 	int may_wake;
@@ -1314,8 +1318,6 @@ static int cdns_uart_resume(struct device *device)
 	may_wake = device_may_wakeup(device);
 
 	if (console_suspend_enabled && !may_wake) {
-		struct cdns_uart *cdns_uart = port->private_data;
-
 		clk_enable(cdns_uart->pclk);
 		clk_enable(cdns_uart->uartclk);
 
@@ -1349,7 +1351,7 @@ static int cdns_uart_resume(struct device *device)
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
-	return uart_resume_port(&cdns_uart_uart_driver, port);
+	return uart_resume_port(cdns_uart->cdns_uart_driver, port);
 }
 #endif /* ! CONFIG_PM_SLEEP */
 static int __maybe_unused cdns_runtime_suspend(struct device *dev)
@@ -1413,6 +1415,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1570,7 +1574,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);
 #endif
-	rc = uart_remove_one_port(&cdns_uart_uart_driver, port);
+	rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
 	port->mapbase = 0;
 	clk_disable_unprepare(cdns_uart_data->uartclk);
 	clk_disable_unprepare(cdns_uart_data->pclk);
-- 
1.9.1

[PATCH v3 05/10] serial: uartps: Move alias reading higher in probe()

[Michal Simek]

This cosmetic change is done only for having next patch much easier to
read. Moving id setup higher in probe is not affecting any usage of this
driver and it also simplify error path.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3: None
Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5bd2b0607df6..a3e97ccb376d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1415,6 +1415,16 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	/* Look for a serialN alias */
+	id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (id < 0)
+		id = 0;
+
+	if (id >= CDNS_UART_NR_PORTS) {
+		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
+		return -ENODEV;
+	}
+
 	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
@@ -1476,16 +1486,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 				&cdns_uart_data->clk_rate_change_nb))
 		dev_warn(&pdev->dev, "Unable to register clock notifier.\n");
 #endif
-	/* Look for a serialN alias */
-	id = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (id < 0)
-		id = 0;
-
-	if (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);
@@ -1544,7 +1544,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
-err_out_notif_unreg:
 #ifdef CONFIG_COMMON_CLK
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);
-- 
1.9.1

[PATCH v3 06/10] serial: uartps: Move register to probe based on run time detection

[Michal Simek]

Register uart driver in probe to be able to register one device with
unique major/minor separately. Also calculate number of instances of
this driver to be able to call uart_unregister_driver() when there is no
instance.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- Fix uart_unregister_driver() in error path or when driver is removed.
- Change commit message

Changes in v2:
- Remove nr field logic

Discussed here: https://patchwork.kernel.org/patch/9738445/

The same solution is done in pl011 driver.

---
 drivers/tty/serial/xilinx_uartps.c | 42 +++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index a3e97ccb376d..54735a7f9960 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1393,6 +1393,9 @@ static int __maybe_unused cdns_runtime_resume(struct device *dev)
 };
 MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
 
+/* Temporary variable for storing number of instances */
+static int instances;
+
 /**
  * cdns_uart_probe - Platform driver probe
  * @pdev: Pointer to the platform device structure
@@ -1425,6 +1428,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (!cdns_uart_uart_driver.state) {
+		rc = uart_register_driver(&cdns_uart_uart_driver);
+		if (rc < 0) {
+			dev_err(&pdev->dev, "Failed to register driver\n");
+			return rc;
+		}
+	}
+
 	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
@@ -1442,7 +1453,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 	if (IS_ERR(cdns_uart_data->pclk)) {
 		dev_err(&pdev->dev, "pclk clock not found.\n");
-		return PTR_ERR(cdns_uart_data->pclk);
+		rc = PTR_ERR(cdns_uart_data->pclk);
+		goto err_out_unregister_driver;
 	}
 
 	cdns_uart_data->uartclk = devm_clk_get(&pdev->dev, "uart_clk");
@@ -1453,13 +1465,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 	if (IS_ERR(cdns_uart_data->uartclk)) {
 		dev_err(&pdev->dev, "uart_clk clock not found.\n");
-		return PTR_ERR(cdns_uart_data->uartclk);
+		rc = PTR_ERR(cdns_uart_data->uartclk);
+		goto err_out_unregister_driver;
 	}
 
 	rc = clk_prepare_enable(cdns_uart_data->pclk);
 	if (rc) {
 		dev_err(&pdev->dev, "Unable to enable pclk clock.\n");
-		return rc;
+		goto err_out_unregister_driver;
 	}
 	rc = clk_prepare_enable(cdns_uart_data->uartclk);
 	if (rc) {
@@ -1538,6 +1551,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		console_port = NULL;
 #endif
 
+	instances++;
 	return 0;
 
 err_out_pm_disable:
@@ -1552,7 +1566,9 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	clk_disable_unprepare(cdns_uart_data->uartclk);
 err_out_clk_dis_pclk:
 	clk_disable_unprepare(cdns_uart_data->pclk);
-
+err_out_unregister_driver:
+	if (!instances)
+		uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
@@ -1580,6 +1596,8 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	if (!--instances)
+		uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
@@ -1595,28 +1613,14 @@ static int cdns_uart_remove(struct platform_device *pdev)
 
 static int __init cdns_uart_init(void)
 {
-	int retval = 0;
-
-	/* Register the cdns_uart driver with the serial core */
-	retval = uart_register_driver(&cdns_uart_uart_driver);
-	if (retval)
-		return retval;
-
 	/* Register the platform driver */
-	retval = platform_driver_register(&cdns_uart_platform_driver);
-	if (retval)
-		uart_unregister_driver(&cdns_uart_uart_driver);
-
-	return retval;
+	return platform_driver_register(&cdns_uart_platform_driver);
 }
 
 static void __exit cdns_uart_exit(void)
 {
 	/* Unregister the platform driver */
 	platform_driver_unregister(&cdns_uart_platform_driver);
-
-	/* Unregister the cdns_uart driver */
-	uart_unregister_driver(&cdns_uart_uart_driver);
 }
 
 arch_initcall(cdns_uart_init);
-- 
1.9.1

[PATCH v3 07/10] serial: uartps: Fill struct uart_driver in probe()

[Michal Simek]

This is preparation step for dynamic port allocation without
CDNS_UART_NR_PORTS macro. Fill the structure only once at probe.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- s/,/;/ in filling structure
- get cdns_uart_uart_driver out of PS_UART_CONSOLE ifdefs

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 54735a7f9960..61087233f411 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1099,6 +1099,8 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
 #endif
 };
 
+static struct uart_driver cdns_uart_uart_driver;
+
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 /**
  * cdns_uart_console_putchar - write the character to the FIFO buffer
@@ -1239,8 +1241,6 @@ static int cdns_uart_console_setup(struct console *co, char *options)
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
-static struct uart_driver cdns_uart_uart_driver;
-
 static struct console cdns_uart_console = {
 	.name	= CDNS_UART_TTY_NAME,
 	.write	= cdns_uart_console_write,
@@ -1252,18 +1252,6 @@ static int cdns_uart_console_setup(struct console *co, char *options)
 };
 #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
 
-static struct uart_driver cdns_uart_uart_driver = {
-	.owner		= THIS_MODULE,
-	.driver_name	= CDNS_UART_NAME,
-	.dev_name	= CDNS_UART_TTY_NAME,
-	.major		= CDNS_UART_MAJOR,
-	.minor		= CDNS_UART_MINOR,
-	.nr		= CDNS_UART_NR_PORTS,
-#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
-	.cons		= &cdns_uart_console,
-#endif
-};
-
 #ifdef CONFIG_PM_SLEEP
 /**
  * cdns_uart_suspend - suspend event
@@ -1429,6 +1417,16 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	}
 
 	if (!cdns_uart_uart_driver.state) {
+		cdns_uart_uart_driver.owner = THIS_MODULE;
+		cdns_uart_uart_driver.driver_name = CDNS_UART_NAME;
+		cdns_uart_uart_driver.dev_name = CDNS_UART_TTY_NAME;
+		cdns_uart_uart_driver.major = CDNS_UART_MAJOR;
+		cdns_uart_uart_driver.minor = CDNS_UART_MINOR;
+		cdns_uart_uart_driver.nr = CDNS_UART_NR_PORTS;
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+		cdns_uart_uart_driver.cons = &cdns_uart_console;
+#endif
+
 		rc = uart_register_driver(&cdns_uart_uart_driver);
 		if (rc < 0) {
 			dev_err(&pdev->dev, "Failed to register driver\n");
-- 
1.9.1

[PATCH v3 08/10] serial: uartps: Change logic how console_port is setup

[Michal Simek]

Change logic how console_port is setup by using CON_ENABLED flag
instead of index. There will be unique cdns_uart_console() structures
that's why code can't use id for console_port assignment.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- New patch in series

Changes in v2: None

 drivers/tty/serial/xilinx_uartps.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 61087233f411..0e578c0a8ce0 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1532,7 +1532,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 * If register_console() don't assign value, then console_port pointer
 	 * is cleanup.
 	 */
-	if (cdns_uart_uart_driver.cons->index == -1)
+	if (!console_port)
 		console_port = port;
 #endif
 
@@ -1545,7 +1545,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 #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)
+	if (console_port == port &&
+	    !(cdns_uart_uart_driver.cons->flags & CON_ENABLED))
 		console_port = NULL;
 #endif
 
@@ -1594,6 +1595,12 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
+
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+	if (console_port == port)
+		console_port = NULL;
+#endif
+
 	if (!--instances)
 		uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
-- 
1.9.1

[PATCH v3 09/10] serial: uartps: Register own uart console and driver structures

[Michal Simek]

Every instance is registering own struct console and struct uart_driver
with minor number which corresponds to alias ID (or 0 now) and with 1 uart
port. The same alias ID is saved to tty_driver->name_base which is key
field for creating ttyPSX name.

Because name_base and minor number are setup already there is no need to
setup any port->line number because 0 is the right value.

Unfortunately this driver is setting up major number to 0 for using
dynamic assignment and kernel is allocating different major numbers for
every instance instead of using the same major and different minor
number.

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jan  1 03:36 /dev/ttyPS0
crw--w----    1 root     root      253,   1 Jan  1 00:00 /dev/ttyPS1

When major number is not 0. For example 252 then major/minor
combinations are in expected form

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jan  1 04:04 /dev/ttyPS0
crw--w----    1 root     root      252,   1 Jan  1 00:00 /dev/ttyPS1

Driver is not freeing struct cdns_uart_console in case that instance is
not used as console. The reason is that console is incorrectly unregistred
and "console [0] disabled" message will be shown.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- Rebase on the top of previous broken patch
- Change patch subject ("was serial: uartps: Remove CDNS_UART_NR_PORTS
  macro")
- Keep CDNS_UART_NR_PORTS in this patch and remove it in next one and
  align commit message to reflect this
- Allocate struct console dynamically too to be unique for every
  instance
- Cleanup error path

Changes in v2:
- Register one uart_driver with unique minor at probe time

---
 drivers/tty/serial/xilinx_uartps.c | 93 ++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 38 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 0e578c0a8ce0..d838612eda6f 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -30,7 +30,6 @@
 #define CDNS_UART_TTY_NAME	"ttyPS"
 #define CDNS_UART_NAME		"xuartps"
 #define CDNS_UART_MAJOR		0	/* use dynamic node allocation */
-#define CDNS_UART_MINOR		0	/* works best with devtmpfs */
 #define CDNS_UART_NR_PORTS	2
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
 #define CDNS_UART_REGISTER_SPACE	0x1000
@@ -1099,8 +1098,6 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
 #endif
 };
 
-static struct uart_driver cdns_uart_uart_driver;
-
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 /**
  * cdns_uart_console_putchar - write the character to the FIFO buffer
@@ -1240,16 +1237,6 @@ static int cdns_uart_console_setup(struct console *co, char *options)
 
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
-
-static struct console cdns_uart_console = {
-	.name	= CDNS_UART_TTY_NAME,
-	.write	= cdns_uart_console_write,
-	.device	= uart_console_device,
-	.setup	= cdns_uart_console_setup,
-	.flags	= CON_PRINTBUFFER,
-	.index	= -1, /* Specified on the cmdline (e.g. console=ttyPS ) */
-	.data	= &cdns_uart_uart_driver,
-};
 #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
 
 #ifdef CONFIG_PM_SLEEP
@@ -1381,9 +1368,6 @@ static int __maybe_unused cdns_runtime_resume(struct device *dev)
 };
 MODULE_DEVICE_TABLE(of, cdns_uart_of_match);
 
-/* Temporary variable for storing number of instances */
-static int instances;
-
 /**
  * cdns_uart_probe - Platform driver probe
  * @pdev: Pointer to the platform device structure
@@ -1397,6 +1381,11 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct cdns_uart *cdns_uart_data;
 	const struct of_device_id *match;
+	struct uart_driver *cdns_uart_uart_driver;
+	char *driver_name;
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+	struct console *cdns_uart_console;
+#endif
 
 	cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
 			GFP_KERNEL);
@@ -1406,6 +1395,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	cdns_uart_uart_driver = devm_kzalloc(&pdev->dev,
+					     sizeof(*cdns_uart_uart_driver),
+					     GFP_KERNEL);
+	if (!cdns_uart_uart_driver)
+		return -ENOMEM;
+
 	/* Look for a serialN alias */
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0)
@@ -1416,25 +1411,50 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	if (!cdns_uart_uart_driver.state) {
-		cdns_uart_uart_driver.owner = THIS_MODULE;
-		cdns_uart_uart_driver.driver_name = CDNS_UART_NAME;
-		cdns_uart_uart_driver.dev_name = CDNS_UART_TTY_NAME;
-		cdns_uart_uart_driver.major = CDNS_UART_MAJOR;
-		cdns_uart_uart_driver.minor = CDNS_UART_MINOR;
-		cdns_uart_uart_driver.nr = CDNS_UART_NR_PORTS;
+	/* There is a need to use unique driver name */
+	driver_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%d",
+				     CDNS_UART_NAME, id);
+	if (!driver_name)
+		return -ENOMEM;
+
+	cdns_uart_uart_driver->owner = THIS_MODULE;
+	cdns_uart_uart_driver->driver_name = driver_name;
+	cdns_uart_uart_driver->dev_name	= CDNS_UART_TTY_NAME;
+	cdns_uart_uart_driver->major = CDNS_UART_MAJOR;
+	cdns_uart_uart_driver->minor = id;
+	cdns_uart_uart_driver->nr = 1;
+
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
-		cdns_uart_uart_driver.cons = &cdns_uart_console;
+	cdns_uart_console = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_console),
+					 GFP_KERNEL);
+	if (!cdns_uart_console)
+		return -ENOMEM;
+
+	strncpy(cdns_uart_console->name, CDNS_UART_TTY_NAME,
+		sizeof(cdns_uart_console->name));
+	cdns_uart_console->index = id;
+	cdns_uart_console->write = cdns_uart_console_write;
+	cdns_uart_console->device = uart_console_device;
+	cdns_uart_console->setup = cdns_uart_console_setup;
+	cdns_uart_console->flags = CON_PRINTBUFFER;
+	cdns_uart_console->data = cdns_uart_uart_driver;
+	cdns_uart_uart_driver->cons = cdns_uart_console;
 #endif
 
-		rc = uart_register_driver(&cdns_uart_uart_driver);
-		if (rc < 0) {
-			dev_err(&pdev->dev, "Failed to register driver\n");
-			return rc;
-		}
+	rc = uart_register_driver(cdns_uart_uart_driver);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Failed to register driver\n");
+		return rc;
 	}
 
-	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
+	cdns_uart_data->cdns_uart_driver = cdns_uart_uart_driver;
+
+	/*
+	 * Setting up proper name_base needs to be done after uart
+	 * registration because tty_driver structure is not filled.
+	 * name_base is 0 by default.
+	 */
+	cdns_uart_uart_driver->tty_driver->name_base = id;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
@@ -1505,7 +1525,6 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	port->flags	= UPF_BOOT_AUTOCONF;
 	port->ops	= &cdns_uart_ops;
 	port->fifosize	= CDNS_UART_FIFO_SIZE;
-	port->line	= id;
 
 	/*
 	 * Register the port.
@@ -1536,7 +1555,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		console_port = port;
 #endif
 
-	rc = uart_add_one_port(&cdns_uart_uart_driver, port);
+	rc = uart_add_one_port(cdns_uart_uart_driver, port);
 	if (rc) {
 		dev_err(&pdev->dev,
 			"uart_add_one_port() failed; err=%i\n", rc);
@@ -1546,11 +1565,10 @@ static int cdns_uart_probe(struct platform_device *pdev)
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 	/* This is not port which is used for console that's why clean it up */
 	if (console_port == port &&
-	    !(cdns_uart_uart_driver.cons->flags & CON_ENABLED))
+	    !(cdns_uart_uart_driver->cons->flags & CON_ENABLED))
 		console_port = NULL;
 #endif
 
-	instances++;
 	return 0;
 
 err_out_pm_disable:
@@ -1566,8 +1584,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 err_out_clk_dis_pclk:
 	clk_disable_unprepare(cdns_uart_data->pclk);
 err_out_unregister_driver:
-	if (!instances)
-		uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
+
 	return rc;
 }
 
@@ -1601,8 +1619,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 		console_port = NULL;
 #endif
 
-	if (!--instances)
-		uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
-- 
1.9.1

[PATCH v3 10/10] serial: uartps: Move Port ID to device data structure

[Michal Simek]

Record port ID in device data structure to be have it connected to
certain instance.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v3:
- New patch in series

Changes in v2: None

 drivers/tty/serial/xilinx_uartps.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index d838612eda6f..71c032744dae 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -181,6 +181,7 @@
  * @pclk:		APB clock
  * @cdns_uart_driver:	Pointer to UART driver
  * @baud:		Current baud rate
+ * @id:			Port ID
  * @clk_rate_change_nb:	Notifier block for clock changes
  * @quirks:		Flags for RXBS support.
  */
@@ -190,6 +191,7 @@ struct cdns_uart {
 	struct clk		*pclk;
 	struct uart_driver	*cdns_uart_driver;
 	unsigned int		baud;
+	int			id;
 	struct notifier_block	clk_rate_change_nb;
 	u32			quirks;
 };
@@ -1376,7 +1378,7 @@ static int __maybe_unused cdns_runtime_resume(struct device *dev)
  */
 static int cdns_uart_probe(struct platform_device *pdev)
 {
-	int rc, id, irq;
+	int rc, irq;
 	struct uart_port *port;
 	struct resource *res;
 	struct cdns_uart *cdns_uart_data;
@@ -1402,18 +1404,18 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/* Look for a serialN alias */
-	id = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (id < 0)
-		id = 0;
+	cdns_uart_data->id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (cdns_uart_data->id < 0)
+		cdns_uart_data->id = 0;
 
-	if (id >= CDNS_UART_NR_PORTS) {
+	if (cdns_uart_data->id >= CDNS_UART_NR_PORTS) {
 		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
 		return -ENODEV;
 	}
 
 	/* There is a need to use unique driver name */
 	driver_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%d",
-				     CDNS_UART_NAME, id);
+				     CDNS_UART_NAME, cdns_uart_data->id);
 	if (!driver_name)
 		return -ENOMEM;
 
@@ -1421,7 +1423,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	cdns_uart_uart_driver->driver_name = driver_name;
 	cdns_uart_uart_driver->dev_name	= CDNS_UART_TTY_NAME;
 	cdns_uart_uart_driver->major = CDNS_UART_MAJOR;
-	cdns_uart_uart_driver->minor = id;
+	cdns_uart_uart_driver->minor = cdns_uart_data->id;
 	cdns_uart_uart_driver->nr = 1;
 
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
@@ -1432,7 +1434,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 	strncpy(cdns_uart_console->name, CDNS_UART_TTY_NAME,
 		sizeof(cdns_uart_console->name));
-	cdns_uart_console->index = id;
+	cdns_uart_console->index = cdns_uart_data->id;
 	cdns_uart_console->write = cdns_uart_console_write;
 	cdns_uart_console->device = uart_console_device;
 	cdns_uart_console->setup = cdns_uart_console_setup;
@@ -1454,7 +1456,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 * registration because tty_driver structure is not filled.
 	 * name_base is 0 by default.
 	 */
-	cdns_uart_uart_driver->tty_driver->name_base = id;
+	cdns_uart_uart_driver->tty_driver->name_base = cdns_uart_data->id;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
-- 
1.9.1

Re: [PATCH v3 00/10] serial: uartps: Add run time support for more IPs than hardcoded 2

[Michal Simek]

Hi Greg,

On 3.9.2018 15:10, Michal Simek wrote:
> Hi,
> 
> this series is trying to address discussion I had with Alan in past
> https://patchwork.kernel.org/patch/9738445/ and also with Rob in v1
> https://lkml.org/lkml/2018/4/26/551.
> 
> These patches are doing preparation to enable dynamic ID port allocation
> which is capable to create devices with higher IDs.
> 
> For example this is how it works.
> uart0 on higher alias
> serial0 = &uart1;
> serial30 = &uart0;
> 
> ~# ls -la /dev/ttyPS*
> crw-------    1 root     root      252,   0 Jun  6 12:19 /dev/ttyPS0
> crw--w----    1 root     root      253, 100 Jan  1  1970 /dev/ttyPS30
> 
> Thanks,
> Michal
> 
> Changes in v3:
> - New patch found by testing
> - New patch - can be sent separately but there is dependency that's why
>   sent in this series
> - Fix uart_unregister_driver() in error path or when driver is removed.
> - Change commit message
> - s/,/;/ in filling structure
> - get cdns_uart_uart_driver out of PS_UART_CONSOLE ifdefs
> - New patch in series
> - Rebase on the top of previous broken patch
> - Change patch subject ("was serial: uartps: Remove CDNS_UART_NR_PORTS
>   macro")
> - Keep CDNS_UART_NR_PORTS in this patch and remove it in next one and
>   align commit message to reflect this
> - Allocate struct console dynamically too to be unique for every
>   instance
> - Cleanup error path
> - New patch in series
> 
> Changes in v2:
> - new patch - it can be sent separately too
> - new patch - it can be sent separately too
> - new patch - it can be sent separately too
> - Remove nr field logic
> - new patch - it can be sent separately too
> - Register one uart_driver with unique minor at probe time
> 
> Michal Simek (9):
>   serial: uartps: console_setup() can't be placed to init section
>   serial: uartps: Do not initialize field to zero again
>   serial: uartps: Do not use static struct uart_driver out of probe()
>   serial: uartps: Move alias reading higher in probe()
>   serial: uartps: Move register to probe based on run time detection
>   serial: uartps: Fill struct uart_driver in probe()
>   serial: uartps: Change logic how console_port is setup
>   serial: uartps: Register own uart console and driver structures
>   serial: uartps: Move Port ID to device data structure
> 
> Nava kishore Manne (1):
>   serial: uartps: Fix suspend functionality
> 
>  drivers/tty/serial/xilinx_uartps.c | 203 +++++++++++++++++++------------------
>  1 file changed, 107 insertions(+), 96 deletions(-)
> 

Can you please look at this series?
I have sent 2 more patches on the top of this series where OF patch was
reviewed by Rob already.

Thanks,
Michal

Re: [PATCH v3 00/10] serial: uartps: Add run time support for more IPs than hardcoded 2

[Greg Kroah-Hartman]

On Tue, Sep 18, 2018 at 01:54:31PM +0200, Michal Simek wrote:
> Hi Greg,
> 
> On 3.9.2018 15:10, Michal Simek wrote:
> > Hi,
> > 
> > this series is trying to address discussion I had with Alan in past
> > https://patchwork.kernel.org/patch/9738445/ and also with Rob in v1
> > https://lkml.org/lkml/2018/4/26/551.
> > 
> > These patches are doing preparation to enable dynamic ID port allocation
> > which is capable to create devices with higher IDs.
> > 
> > For example this is how it works.
> > uart0 on higher alias
> > serial0 = &uart1;
> > serial30 = &uart0;
> > 
> > ~# ls -la /dev/ttyPS*
> > crw-------    1 root     root      252,   0 Jun  6 12:19 /dev/ttyPS0
> > crw--w----    1 root     root      253, 100 Jan  1  1970 /dev/ttyPS30
> > 
> > Thanks,
> > Michal
> > 
> > Changes in v3:
> > - New patch found by testing
> > - New patch - can be sent separately but there is dependency that's why
> >   sent in this series
> > - Fix uart_unregister_driver() in error path or when driver is removed.
> > - Change commit message
> > - s/,/;/ in filling structure
> > - get cdns_uart_uart_driver out of PS_UART_CONSOLE ifdefs
> > - New patch in series
> > - Rebase on the top of previous broken patch
> > - Change patch subject ("was serial: uartps: Remove CDNS_UART_NR_PORTS
> >   macro")
> > - Keep CDNS_UART_NR_PORTS in this patch and remove it in next one and
> >   align commit message to reflect this
> > - Allocate struct console dynamically too to be unique for every
> >   instance
> > - Cleanup error path
> > - New patch in series
> > 
> > Changes in v2:
> > - new patch - it can be sent separately too
> > - new patch - it can be sent separately too
> > - new patch - it can be sent separately too
> > - Remove nr field logic
> > - new patch - it can be sent separately too
> > - Register one uart_driver with unique minor at probe time
> > 
> > Michal Simek (9):
> >   serial: uartps: console_setup() can't be placed to init section
> >   serial: uartps: Do not initialize field to zero again
> >   serial: uartps: Do not use static struct uart_driver out of probe()
> >   serial: uartps: Move alias reading higher in probe()
> >   serial: uartps: Move register to probe based on run time detection
> >   serial: uartps: Fill struct uart_driver in probe()
> >   serial: uartps: Change logic how console_port is setup
> >   serial: uartps: Register own uart console and driver structures
> >   serial: uartps: Move Port ID to device data structure
> > 
> > Nava kishore Manne (1):
> >   serial: uartps: Fix suspend functionality
> > 
> >  drivers/tty/serial/xilinx_uartps.c | 203 +++++++++++++++++++------------------
> >  1 file changed, 107 insertions(+), 96 deletions(-)
> > 
> 
> Can you please look at this series?
> I have sent 2 more patches on the top of this series where OF patch was
> reviewed by Rob already.

My tty queue is a bit lagging right now, sorry, will get to that soon...

greg k-h