[RFC PATCH v2 0/6] serial: uartps: Add run time support for more IPs than hardcoded 2

[RFC PATCH v2 0/6] 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.

The first 5 patches are preparation patches to have the last patch as
small as possible to focus on changes there.

Cases without DT aliases are not solved in this series but one function
was shared in RFC v1.
The purpose of this series to get feedback on solution where every
driver instance allocate at run time own uart_driver.

For example this is how it works.
uart0 on higher alias
serial0 = &uart1;
serial100 = &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/ttyPS100

Thanks,
Michal


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

Michal Simek (6):
  serial: uartps: Do not initialize field to zero again
  serial: uartps: Move register to probe based on run time detection
  serial: uartps: Do not use static struct uart_driver out of probe()
  serial: uartps: Move alias reading higher in probe()
  serial: uartps: Fill struct uart_driver in probe()
  serial: uartps: Remove CDNS_UART_NR_PORTS macro

 drivers/tty/serial/xilinx_uartps.c | 126 ++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 58 deletions(-)

-- 
1.9.1

[RFC PATCH v2 0/6] 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.

The first 5 patches are preparation patches to have the last patch as
small as possible to focus on changes there.

Cases without DT aliases are not solved in this series but one function
was shared in RFC v1.
The purpose of this series to get feedback on solution where every
driver instance allocate at run time own uart_driver.

For example this is how it works.
uart0 on higher alias
serial0 = &uart1;
serial100 = &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/ttyPS100

Thanks,
Michal


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

Michal Simek (6):
  serial: uartps: Do not initialize field to zero again
  serial: uartps: Move register to probe based on run time detection
  serial: uartps: Do not use static struct uart_driver out of probe()
  serial: uartps: Move alias reading higher in probe()
  serial: uartps: Fill struct uart_driver in probe()
  serial: uartps: Remove CDNS_UART_NR_PORTS macro

 drivers/tty/serial/xilinx_uartps.c | 126 ++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 58 deletions(-)

-- 
1.9.1

[RFC PATCH v2 1/6] 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 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 8a3e34234e98..5f116f3ecd4a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1510,15 +1510,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

[RFC PATCH v2 1/6] 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 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 8a3e34234e98..5f116f3ecd4a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1510,15 +1510,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

[RFC PATCH v2 2/6] serial: uartps: Move register to probe based on run time detection

[Michal Simek]

Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.

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

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 | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5f116f3ecd4a..e24382f58dea 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1438,6 +1438,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	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;
+		}
+	}
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1617,28 +1625,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

[RFC PATCH v2 2/6] serial: uartps: Move register to probe based on run time detection

[Michal Simek]

Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.

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

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 | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5f116f3ecd4a..e24382f58dea 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1438,6 +1438,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	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;
+		}
+	}
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1617,28 +1625,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

[RFC PATCH v2 3/6] 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 referece to private data structure
as preparation step for dynamic struct uart_driver allocation.

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

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 e24382f58dea..fe96fd950d3a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -179,6 +179,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.
@@ -187,6 +188,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;
@@ -1280,6 +1282,7 @@ static int __init 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;
 	struct tty_struct *tty;
 	struct device *tty_dev;
 	int may_wake = 0;
@@ -1296,7 +1299,7 @@ static int cdns_uart_suspend(struct device *device)
 	 * Call the API provided in serial_core.c file which handles
 	 * the suspend.
 	 */
-	uart_suspend_port(&cdns_uart_uart_driver, port);
+	uart_suspend_port(cdns_uart->cdns_uart_driver, port);
 	if (!(console_suspend_enabled && !may_wake)) {
 		unsigned long flags = 0;
 
@@ -1324,6 +1327,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;
 	struct tty_struct *tty;
@@ -1339,8 +1343,6 @@ static int cdns_uart_resume(struct device *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);
 
@@ -1374,7 +1376,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)
@@ -1446,6 +1448,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		}
 	}
 
+	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;
@@ -1603,7 +1607,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

[RFC PATCH v2 3/6] 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 referece to private data structure
as preparation step for dynamic struct uart_driver allocation.

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

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 e24382f58dea..fe96fd950d3a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -179,6 +179,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.
@@ -187,6 +188,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;
@@ -1280,6 +1282,7 @@ static int __init 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;
 	struct tty_struct *tty;
 	struct device *tty_dev;
 	int may_wake = 0;
@@ -1296,7 +1299,7 @@ static int cdns_uart_suspend(struct device *device)
 	 * Call the API provided in serial_core.c file which handles
 	 * the suspend.
 	 */
-	uart_suspend_port(&cdns_uart_uart_driver, port);
+	uart_suspend_port(cdns_uart->cdns_uart_driver, port);
 	if (!(console_suspend_enabled && !may_wake)) {
 		unsigned long flags = 0;
 
@@ -1324,6 +1327,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;
 	struct tty_struct *tty;
@@ -1339,8 +1343,6 @@ static int cdns_uart_resume(struct device *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);
 
@@ -1374,7 +1376,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)
@@ -1446,6 +1448,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		}
 	}
 
+	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;
@@ -1603,7 +1607,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

[RFC PATCH v2 4/6] 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 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 fe96fd950d3a..b47d7ccbc38d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1440,6 +1440,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;
+	}
+
 	if (!cdns_uart_uart_driver.state) {
 		rc = uart_register_driver(&cdns_uart_uart_driver);
 		if (rc < 0) {
@@ -1509,16 +1519,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);
@@ -1577,7 +1577,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

[RFC PATCH v2 4/6] 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 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 fe96fd950d3a..b47d7ccbc38d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1440,6 +1440,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;
+	}
+
 	if (!cdns_uart_uart_driver.state) {
 		rc = uart_register_driver(&cdns_uart_uart_driver);
 		if (rc < 0) {
@@ -1509,16 +1519,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);
@@ -1577,7 +1577,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

[RFC PATCH v2 5/6] 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 v2:
- new patch - it can be sent separately too

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

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b47d7ccbc38d..d76efe8cb3df 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1260,18 +1260,6 @@ static int __init 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
@@ -1451,6 +1439,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

[RFC PATCH v2 5/6] 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 v2:
- new patch - it can be sent separately too

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

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b47d7ccbc38d..d76efe8cb3df 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1260,18 +1260,6 @@ static int __init 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
@@ -1451,6 +1439,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

[RFC PATCH v2 6/6] serial: uartps: Remove CDNS_UART_NR_PORTS macro

[Michal Simek]

This patch is removing CDNS_UART_NR_PORTS macro which limits number of
ports which can be used. Every instance is registering own 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.

~# find /proc/tty/ -name "*uartps*"
/proc/tty/driver/xuartps0
/proc/tty/driver/xuartps100

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

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

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

This patch not done because id is not unique. This needs to be solved
before this patch can be applied. In v1 I have created
of_alias_check_id() for that. https://lkml.org/lkml/2018/4/26/551

Also we need to run more testing on this to make sure that everything is
working properly.

---
 drivers/tty/serial/xilinx_uartps.c | 78 +++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index d76efe8cb3df..82cc17ec7b5d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -30,8 +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
 
@@ -1247,8 +1245,6 @@ static int __init 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,
@@ -1256,7 +1252,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 	.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 */
 
@@ -1419,6 +1414,8 @@ 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;
 
 	cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
 			GFP_KERNEL);
@@ -1431,32 +1428,49 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	/* Look for a serialN alias */
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0)
+		/*
+		 * FIXME this is not enough because if there the next instance
+		 * without alias it will get also id = 0 which is wrong
+		 */
 		id = 0;
 
-	if (id >= CDNS_UART_NR_PORTS) {
-		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
-		return -ENODEV;
-	}
+	cdns_uart_uart_driver = devm_kzalloc(&pdev->dev,
+					     sizeof(*cdns_uart_uart_driver),
+					     GFP_KERNEL);
+	if (!cdns_uart_uart_driver)
+		return -ENOMEM;
+
+	/* 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;
 
-	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,
+	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_uart_driver->cons = &cdns_uart_console;
+	cdns_uart_console.data = cdns_uart_uart_driver;
 #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;
+	/*
+	 * 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;
+
+	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) {
@@ -1473,7 +1487,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");
@@ -1484,13 +1499,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) {
@@ -1525,7 +1541,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.
@@ -1552,11 +1567,11 @@ 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 (cdns_uart_uart_driver->cons->index == -1)
 		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);
@@ -1565,7 +1580,7 @@ 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 (cdns_uart_uart_driver->cons->index == -1)
 		console_port = NULL;
 #endif
 
@@ -1583,6 +1598,8 @@ 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:
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 
 	return rc;
 }
@@ -1611,6 +1628,7 @@ 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);
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
-- 
1.9.1

[RFC PATCH v2 6/6] serial: uartps: Remove CDNS_UART_NR_PORTS macro

[Michal Simek]

This patch is removing CDNS_UART_NR_PORTS macro which limits number of
ports which can be used. Every instance is registering own 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.

~# find /proc/tty/ -name "*uartps*"
/proc/tty/driver/xuartps0
/proc/tty/driver/xuartps100

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

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

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

This patch not done because id is not unique. This needs to be solved
before this patch can be applied. In v1 I have created
of_alias_check_id() for that. https://lkml.org/lkml/2018/4/26/551

Also we need to run more testing on this to make sure that everything is
working properly.

---
 drivers/tty/serial/xilinx_uartps.c | 78 +++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index d76efe8cb3df..82cc17ec7b5d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -30,8 +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
 
@@ -1247,8 +1245,6 @@ static int __init 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,
@@ -1256,7 +1252,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 	.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 */
 
@@ -1419,6 +1414,8 @@ 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;
 
 	cdns_uart_data = devm_kzalloc(&pdev->dev, sizeof(*cdns_uart_data),
 			GFP_KERNEL);
@@ -1431,32 +1428,49 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	/* Look for a serialN alias */
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0)
+		/*
+		 * FIXME this is not enough because if there the next instance
+		 * without alias it will get also id = 0 which is wrong
+		 */
 		id = 0;
 
-	if (id >= CDNS_UART_NR_PORTS) {
-		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
-		return -ENODEV;
-	}
+	cdns_uart_uart_driver = devm_kzalloc(&pdev->dev,
+					     sizeof(*cdns_uart_uart_driver),
+					     GFP_KERNEL);
+	if (!cdns_uart_uart_driver)
+		return -ENOMEM;
+
+	/* 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;
 
-	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,
+	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_uart_driver->cons = &cdns_uart_console;
+	cdns_uart_console.data = cdns_uart_uart_driver;
 #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;
+	/*
+	 * 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;
+
+	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) {
@@ -1473,7 +1487,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");
@@ -1484,13 +1499,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) {
@@ -1525,7 +1541,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.
@@ -1552,11 +1567,11 @@ 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 (cdns_uart_uart_driver->cons->index == -1)
 		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);
@@ -1565,7 +1580,7 @@ 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 (cdns_uart_uart_driver->cons->index == -1)
 		console_port = NULL;
 #endif
 
@@ -1583,6 +1598,8 @@ 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:
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 
 	return rc;
 }
@@ -1611,6 +1628,7 @@ 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);
+	uart_unregister_driver(cdns_uart_data->cdns_uart_driver);
 	return rc;
 }
 
-- 
1.9.1

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 6.6.2018 14:41, Michal Simek wrote:
> 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 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 8a3e34234e98..5f116f3ecd4a 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1510,15 +1510,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.
> 

Alan, Rob, Greg: Any comment about this RFC?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs

[RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 6.6.2018 14:41, Michal Simek wrote:
> 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 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 8a3e34234e98..5f116f3ecd4a 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1510,15 +1510,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.
> 

Alan, Rob, Greg: Any comment about this RFC?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Greg Kroah-Hartman]

On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> On 6.6.2018 14:41, Michal Simek wrote:
> > 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 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 8a3e34234e98..5f116f3ecd4a 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -1510,15 +1510,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.
> > 
> 
> Alan, Rob, Greg: Any comment about this RFC?

I rarely review RFC patchesets as obviously you don't think it is good
enough to be submitted "for real" :)

If you think this is all good, great, please resend it without the RFC
and it will end up in my queue.

thanks,

greg k-h

[RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Greg Kroah-Hartman]

On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> On 6.6.2018 14:41, Michal Simek wrote:
> > 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 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 8a3e34234e98..5f116f3ecd4a 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -1510,15 +1510,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.
> > 
> 
> Alan, Rob, Greg: Any comment about this RFC?

I rarely review RFC patchesets as obviously you don't think it is good
enough to be submitted "for real" :)

If you think this is all good, great, please resend it without the RFC
and it will end up in my queue.

thanks,

greg k-h

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>> On 6.6.2018 14:41, Michal Simek wrote:
>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>> @@ -1510,15 +1510,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.
>>>
>>
>> Alan, Rob, Greg: Any comment about this RFC?
> 
> I rarely review RFC patchesets as obviously you don't think it is good
> enough to be submitted "for real" :)

There is one missing minor part but I want to review concept first
because I didn't find any driver which is using this style.

> If you think this is all good, great, please resend it without the RFC
> and it will end up in my queue.

I will definitely do it but please look at the concept itself because I
would like to use this with at least 3 other drivers.

Thanks,
Michal

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>> On 6.6.2018 14:41, Michal Simek wrote:
>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>> @@ -1510,15 +1510,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.
>>>
>>
>> Alan, Rob, Greg: Any comment about this RFC?
> 
> I rarely review RFC patchesets as obviously you don't think it is good
> enough to be submitted "for real" :)

There is one missing minor part but I want to review concept first
because I didn't find any driver which is using this style.

> If you think this is all good, great, please resend it without the RFC
> and it will end up in my queue.

I will definitely do it but please look at the concept itself because I
would like to use this with at least 3 other drivers.

Thanks,
Michal

[RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>> On 6.6.2018 14:41, Michal Simek wrote:
>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>> @@ -1510,15 +1510,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.
>>>
>>
>> Alan, Rob, Greg: Any comment about this RFC?
> 
> I rarely review RFC patchesets as obviously you don't think it is good
> enough to be submitted "for real" :)

There is one missing minor part but I want to review concept first
because I didn't find any driver which is using this style.

> If you think this is all good, great, please resend it without the RFC
> and it will end up in my queue.

I will definitely do it but please look at the concept itself because I
would like to use this with at least 3 other drivers.

Thanks,
Michal

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Greg Kroah-Hartman]

On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> > On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> >> On 6.6.2018 14:41, Michal Simek wrote:
> >>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
> >>> --- a/drivers/tty/serial/xilinx_uartps.c
> >>> +++ b/drivers/tty/serial/xilinx_uartps.c
> >>> @@ -1510,15 +1510,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.
> >>>
> >>
> >> Alan, Rob, Greg: Any comment about this RFC?
> > 
> > I rarely review RFC patchesets as obviously you don't think it is good
> > enough to be submitted "for real" :)
> 
> There is one missing minor part but I want to review concept first
> because I didn't find any driver which is using this style.
> 
> > If you think this is all good, great, please resend it without the RFC
> > and it will end up in my queue.
> 
> I will definitely do it but please look at the concept itself because I
> would like to use this with at least 3 other drivers.

I don't have the time right now to review "concepts", sorry.

greg k-h

[RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Greg Kroah-Hartman]

On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> > On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> >> On 6.6.2018 14:41, Michal Simek wrote:
> >>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
> >>> --- a/drivers/tty/serial/xilinx_uartps.c
> >>> +++ b/drivers/tty/serial/xilinx_uartps.c
> >>> @@ -1510,15 +1510,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.
> >>>
> >>
> >> Alan, Rob, Greg: Any comment about this RFC?
> > 
> > I rarely review RFC patchesets as obviously you don't think it is good
> > enough to be submitted "for real" :)
> 
> There is one missing minor part but I want to review concept first
> because I didn't find any driver which is using this style.
> 
> > If you think this is all good, great, please resend it without the RFC
> > and it will end up in my queue.
> 
> I will definitely do it but please look at the concept itself because I
> would like to use this with at least 3 other drivers.

I don't have the time right now to review "concepts", sorry.

greg k-h

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 28.6.2018 01:48, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
>> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
>>> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>>>> On 6.6.2018 14:41, Michal Simek wrote:
>>>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>>>> @@ -1510,15 +1510,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.
>>>>>
>>>>
>>>> Alan, Rob, Greg: Any comment about this RFC?
>>>
>>> I rarely review RFC patchesets as obviously you don't think it is good
>>> enough to be submitted "for real" :)
>>
>> There is one missing minor part but I want to review concept first
>> because I didn't find any driver which is using this style.
>>
>>> If you think this is all good, great, please resend it without the RFC
>>> and it will end up in my queue.
>>
>> I will definitely do it but please look at the concept itself because I
>> would like to use this with at least 3 other drivers.
> 
> I don't have the time right now to review "concepts", sorry.

Ok. I will revup that missing part to get it review to be able to use it.

Thanks,
Michal

Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 28.6.2018 01:48, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
>> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
>>> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>>>> On 6.6.2018 14:41, Michal Simek wrote:
>>>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>>>> @@ -1510,15 +1510,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.
>>>>>
>>>>
>>>> Alan, Rob, Greg: Any comment about this RFC?
>>>
>>> I rarely review RFC patchesets as obviously you don't think it is good
>>> enough to be submitted "for real" :)
>>
>> There is one missing minor part but I want to review concept first
>> because I didn't find any driver which is using this style.
>>
>>> If you think this is all good, great, please resend it without the RFC
>>> and it will end up in my queue.
>>
>> I will definitely do it but please look at the concept itself because I
>> would like to use this with at least 3 other drivers.
> 
> I don't have the time right now to review "concepts", sorry.

Ok. I will revup that missing part to get it review to be able to use it.

Thanks,
Michal

[RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again

[Michal Simek]

On 28.6.2018 01:48, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
>> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
>>> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>>>> On 6.6.2018 14:41, Michal Simek wrote:
>>>>> 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 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 8a3e34234e98..5f116f3ecd4a 100644
>>>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>>>> @@ -1510,15 +1510,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.
>>>>>
>>>>
>>>> Alan, Rob, Greg: Any comment about this RFC?
>>>
>>> I rarely review RFC patchesets as obviously you don't think it is good
>>> enough to be submitted "for real" :)
>>
>> There is one missing minor part but I want to review concept first
>> because I didn't find any driver which is using this style.
>>
>>> If you think this is all good, great, please resend it without the RFC
>>> and it will end up in my queue.
>>
>> I will definitely do it but please look at the concept itself because I
>> would like to use this with at least 3 other drivers.
> 
> I don't have the time right now to review "concepts", sorry.

Ok. I will revup that missing part to get it review to be able to use it.

Thanks,
Michal