[PATCH 0/3] serial: atmel: cleanup code

[PATCH 0/3] serial: atmel: cleanup code

[Claudiu Beznea]

Hi,

The following patches does some cleanup for atmel_serial driver by
switching to dev_pm_ops and improving clock management code. Along with
it I took the chance and introduced a patch for st-asc which removes the
include of pm_runtime.h.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  tty: serial: atmel: stop using legacy pm ops
  tty: serial: atmel: improve clock management
  serial: st-asc: remove include of pm_runtime.h

 drivers/tty/serial/atmel_serial.c | 93 +++++++++----------------------
 drivers/tty/serial/st-asc.c       |  1 -
 2 files changed, 26 insertions(+), 68 deletions(-)

-- 
2.34.1

[PATCH 0/3] serial: atmel: cleanup code

[Claudiu Beznea]

Hi,

The following patches does some cleanup for atmel_serial driver by
switching to dev_pm_ops and improving clock management code. Along with
it I took the chance and introduced a patch for st-asc which removes the
include of pm_runtime.h.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  tty: serial: atmel: stop using legacy pm ops
  tty: serial: atmel: improve clock management
  serial: st-asc: remove include of pm_runtime.h

 drivers/tty/serial/atmel_serial.c | 93 +++++++++----------------------
 drivers/tty/serial/st-asc.c       |  1 -
 2 files changed, 26 insertions(+), 68 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] tty: serial: atmel: stop using legacy pm ops

[Claudiu Beznea]

Stop using legacy PM ops and switch using dev_pm_ops. Along with
it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used
instead. Coding style recommends (at chapter Conditional Compilation)
to avoid using preprocessor conditional and use __maybe_unused
instead.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index dd1c7e4bd1c9..5c793a23dc54 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -166,7 +166,6 @@ struct atmel_uart_port {
 	unsigned int		fidi_min;
 	unsigned int		fidi_max;
 
-#ifdef CONFIG_PM
 	struct {
 		u32		cr;
 		u32		mr;
@@ -177,7 +176,6 @@ struct atmel_uart_port {
 		u32		fmr;
 		u32		fimr;
 	} cache;
-#endif
 
 	int (*prepare_rx)(struct uart_port *port);
 	int (*prepare_tx)(struct uart_port *port);
@@ -2711,7 +2709,6 @@ static struct uart_driver atmel_uart = {
 	.cons		= ATMEL_CONSOLE_DEVICE,
 };
 
-#ifdef CONFIG_PM
 static bool atmel_serial_clk_will_stop(void)
 {
 #ifdef CONFIG_ARCH_AT91
@@ -2721,10 +2718,9 @@ static bool atmel_serial_clk_will_stop(void)
 #endif
 }
 
-static int atmel_serial_suspend(struct platform_device *pdev,
-				pm_message_t state)
+static int __maybe_unused atmel_serial_suspend(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	if (uart_console(port) && console_suspend_enabled) {
@@ -2749,14 +2745,14 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	}
 
 	/* we can not wake up if we're running on slow clock */
-	atmel_port->may_wakeup = device_may_wakeup(&pdev->dev);
+	atmel_port->may_wakeup = device_may_wakeup(dev);
 	if (atmel_serial_clk_will_stop()) {
 		unsigned long flags;
 
 		spin_lock_irqsave(&atmel_port->lock_suspended, flags);
 		atmel_port->suspended = true;
 		spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
-		device_set_wakeup_enable(&pdev->dev, 0);
+		device_set_wakeup_enable(dev, 0);
 	}
 
 	uart_suspend_port(&atmel_uart, port);
@@ -2764,9 +2760,9 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	return 0;
 }
 
-static int atmel_serial_resume(struct platform_device *pdev)
+static int __maybe_unused atmel_serial_resume(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 	unsigned long flags;
 
@@ -2801,14 +2797,10 @@ static int atmel_serial_resume(struct platform_device *pdev)
 	spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
 
 	uart_resume_port(&atmel_uart, port);
-	device_set_wakeup_enable(&pdev->dev, atmel_port->may_wakeup);
+	device_set_wakeup_enable(dev, atmel_port->may_wakeup);
 
 	return 0;
 }
-#else
-#define atmel_serial_suspend NULL
-#define atmel_serial_resume NULL
-#endif
 
 static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 				     struct platform_device *pdev)
@@ -3012,14 +3004,16 @@ static int atmel_serial_remove(struct platform_device *pdev)
 	return ret;
 }
 
+static SIMPLE_DEV_PM_OPS(atmel_serial_pm_ops, atmel_serial_suspend,
+			 atmel_serial_resume);
+
 static struct platform_driver atmel_serial_driver = {
 	.probe		= atmel_serial_probe,
 	.remove		= atmel_serial_remove,
-	.suspend	= atmel_serial_suspend,
-	.resume		= atmel_serial_resume,
 	.driver		= {
 		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
+		.pm			= pm_ptr(&atmel_serial_pm_ops),
 	},
 };
 
-- 
2.34.1

[PATCH 1/3] tty: serial: atmel: stop using legacy pm ops

[Claudiu Beznea]

Stop using legacy PM ops and switch using dev_pm_ops. Along with
it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used
instead. Coding style recommends (at chapter Conditional Compilation)
to avoid using preprocessor conditional and use __maybe_unused
instead.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index dd1c7e4bd1c9..5c793a23dc54 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -166,7 +166,6 @@ struct atmel_uart_port {
 	unsigned int		fidi_min;
 	unsigned int		fidi_max;
 
-#ifdef CONFIG_PM
 	struct {
 		u32		cr;
 		u32		mr;
@@ -177,7 +176,6 @@ struct atmel_uart_port {
 		u32		fmr;
 		u32		fimr;
 	} cache;
-#endif
 
 	int (*prepare_rx)(struct uart_port *port);
 	int (*prepare_tx)(struct uart_port *port);
@@ -2711,7 +2709,6 @@ static struct uart_driver atmel_uart = {
 	.cons		= ATMEL_CONSOLE_DEVICE,
 };
 
-#ifdef CONFIG_PM
 static bool atmel_serial_clk_will_stop(void)
 {
 #ifdef CONFIG_ARCH_AT91
@@ -2721,10 +2718,9 @@ static bool atmel_serial_clk_will_stop(void)
 #endif
 }
 
-static int atmel_serial_suspend(struct platform_device *pdev,
-				pm_message_t state)
+static int __maybe_unused atmel_serial_suspend(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	if (uart_console(port) && console_suspend_enabled) {
@@ -2749,14 +2745,14 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	}
 
 	/* we can not wake up if we're running on slow clock */
-	atmel_port->may_wakeup = device_may_wakeup(&pdev->dev);
+	atmel_port->may_wakeup = device_may_wakeup(dev);
 	if (atmel_serial_clk_will_stop()) {
 		unsigned long flags;
 
 		spin_lock_irqsave(&atmel_port->lock_suspended, flags);
 		atmel_port->suspended = true;
 		spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
-		device_set_wakeup_enable(&pdev->dev, 0);
+		device_set_wakeup_enable(dev, 0);
 	}
 
 	uart_suspend_port(&atmel_uart, port);
@@ -2764,9 +2760,9 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	return 0;
 }
 
-static int atmel_serial_resume(struct platform_device *pdev)
+static int __maybe_unused atmel_serial_resume(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 	unsigned long flags;
 
@@ -2801,14 +2797,10 @@ static int atmel_serial_resume(struct platform_device *pdev)
 	spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
 
 	uart_resume_port(&atmel_uart, port);
-	device_set_wakeup_enable(&pdev->dev, atmel_port->may_wakeup);
+	device_set_wakeup_enable(dev, atmel_port->may_wakeup);
 
 	return 0;
 }
-#else
-#define atmel_serial_suspend NULL
-#define atmel_serial_resume NULL
-#endif
 
 static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 				     struct platform_device *pdev)
@@ -3012,14 +3004,16 @@ static int atmel_serial_remove(struct platform_device *pdev)
 	return ret;
 }
 
+static SIMPLE_DEV_PM_OPS(atmel_serial_pm_ops, atmel_serial_suspend,
+			 atmel_serial_resume);
+
 static struct platform_driver atmel_serial_driver = {
 	.probe		= atmel_serial_probe,
 	.remove		= atmel_serial_remove,
-	.suspend	= atmel_serial_suspend,
-	.resume		= atmel_serial_resume,
 	.driver		= {
 		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
+		.pm			= pm_ptr(&atmel_serial_pm_ops),
 	},
 };
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] tty: serial: atmel: improve clock management

[Claudiu Beznea]

atmel_port->clk was requested in atmel_init_port() (which is called only
on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
NULL on probing path. Thus don't check this. Along with this the clock is
now requested with devm_clk_get() and the clock request has been moved in
atmel_serial_probe() function to avoid disabling/re-enabling it on probe
path for multiple times. All the checks of atmel_port->clk were removed
along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
clock is enabled at the beginning and disabled at the end of probe. As
atmel_console_setup() is called in the middle of probe and clock is
already enabled at that time the clk_prepare_enable() in
atmel_console_setup() has also been removed.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
 1 file changed, 15 insertions(+), 50 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 5c793a23dc54..2955b1012014 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	if (ret)
 		return ret;
 
-	/* for console, the clock could already be configured */
-	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&mpdev->dev, "usart");
-		if (IS_ERR(atmel_port->clk)) {
-			ret = PTR_ERR(atmel_port->clk);
-			atmel_port->clk = NULL;
-			return ret;
-		}
-		ret = clk_prepare_enable(atmel_port->clk);
-		if (ret) {
-			clk_put(atmel_port->clk);
-			atmel_port->clk = NULL;
-			return ret;
-		}
-		port->uartclk = clk_get_rate(atmel_port->clk);
-		clk_disable_unprepare(atmel_port->clk);
-		/* only enable clock when USART is in use */
-	}
+	port->uartclk = clk_get_rate(atmel_port->clk);
 
 	/*
 	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
@@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options)
 		return -ENODEV;
 	}
 
-	ret = clk_prepare_enable(atmel_ports[co->index].clk);
-	if (ret)
-		return ret;
-
 	atmel_uart_writel(port, ATMEL_US_IDR, -1);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
@@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	atomic_set(&atmel_port->tasklet_shutdown, 0);
 	spin_lock_init(&atmel_port->lock_suspended);
 
+	atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
+	if (IS_ERR(atmel_port->clk)) {
+		ret = PTR_ERR(atmel_port->clk);
+		goto err;
+	}
+	ret = clk_prepare_enable(atmel_port->clk);
+	if (ret)
+		goto err;
+
 	ret = atmel_init_port(atmel_port, pdev);
 	if (ret)
-		goto err_clear_bit;
+		goto err_clk_disable_unprepare;
 
 	atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
 	if (IS_ERR(atmel_port->gpios)) {
 		ret = PTR_ERR(atmel_port->gpios);
-		goto err_clear_bit;
+		goto err_clk_disable_unprepare;
 	}
 
 	if (!atmel_use_pdc_rx(&atmel_port->uart)) {
@@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
 				     sizeof(struct atmel_uart_char),
 				     GFP_KERNEL);
 		if (!data)
-			goto err_alloc_ring;
+			goto err_clk_disable_unprepare;
 		atmel_port->rx_ring.buf = data;
 	}
 
@@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_port;
 
-#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
-	if (uart_console(&atmel_port->uart)
-			&& ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
-		/*
-		 * The serial core enabled the clock for us, so undo
-		 * the clk_prepare_enable() in atmel_console_setup()
-		 */
-		clk_disable_unprepare(atmel_port->clk);
-	}
-#endif
-
 	device_init_wakeup(&pdev->dev, 1);
 	platform_set_drvdata(pdev, atmel_port);
 
-	/*
-	 * The peripheral clock has been disabled by atmel_init_port():
-	 * enable it before accessing I/O registers
-	 */
-	clk_prepare_enable(atmel_port->clk);
-
 	if (rs485_enabled) {
 		atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
 				  ATMEL_US_USMODE_NORMAL);
@@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
 err_add_port:
 	kfree(atmel_port->rx_ring.buf);
 	atmel_port->rx_ring.buf = NULL;
-err_alloc_ring:
-	if (!uart_console(&atmel_port->uart)) {
-		clk_put(atmel_port->clk);
-		atmel_port->clk = NULL;
-	}
-err_clear_bit:
+err_clk_disable_unprepare:
+	clk_disable_unprepare(atmel_port->clk);
 	clear_bit(atmel_port->uart.line, atmel_ports_in_use);
 err:
 	return ret;
@@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev)
 
 	clear_bit(port->line, atmel_ports_in_use);
 
-	clk_put(atmel_port->clk);
-	atmel_port->clk = NULL;
 	pdev->dev.of_node = NULL;
 
 	return ret;
-- 
2.34.1

[PATCH 2/3] tty: serial: atmel: improve clock management

[Claudiu Beznea]

atmel_port->clk was requested in atmel_init_port() (which is called only
on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
NULL on probing path. Thus don't check this. Along with this the clock is
now requested with devm_clk_get() and the clock request has been moved in
atmel_serial_probe() function to avoid disabling/re-enabling it on probe
path for multiple times. All the checks of atmel_port->clk were removed
along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
clock is enabled at the beginning and disabled at the end of probe. As
atmel_console_setup() is called in the middle of probe and clock is
already enabled at that time the clk_prepare_enable() in
atmel_console_setup() has also been removed.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
 1 file changed, 15 insertions(+), 50 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 5c793a23dc54..2955b1012014 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	if (ret)
 		return ret;
 
-	/* for console, the clock could already be configured */
-	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&mpdev->dev, "usart");
-		if (IS_ERR(atmel_port->clk)) {
-			ret = PTR_ERR(atmel_port->clk);
-			atmel_port->clk = NULL;
-			return ret;
-		}
-		ret = clk_prepare_enable(atmel_port->clk);
-		if (ret) {
-			clk_put(atmel_port->clk);
-			atmel_port->clk = NULL;
-			return ret;
-		}
-		port->uartclk = clk_get_rate(atmel_port->clk);
-		clk_disable_unprepare(atmel_port->clk);
-		/* only enable clock when USART is in use */
-	}
+	port->uartclk = clk_get_rate(atmel_port->clk);
 
 	/*
 	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
@@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options)
 		return -ENODEV;
 	}
 
-	ret = clk_prepare_enable(atmel_ports[co->index].clk);
-	if (ret)
-		return ret;
-
 	atmel_uart_writel(port, ATMEL_US_IDR, -1);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
@@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	atomic_set(&atmel_port->tasklet_shutdown, 0);
 	spin_lock_init(&atmel_port->lock_suspended);
 
+	atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
+	if (IS_ERR(atmel_port->clk)) {
+		ret = PTR_ERR(atmel_port->clk);
+		goto err;
+	}
+	ret = clk_prepare_enable(atmel_port->clk);
+	if (ret)
+		goto err;
+
 	ret = atmel_init_port(atmel_port, pdev);
 	if (ret)
-		goto err_clear_bit;
+		goto err_clk_disable_unprepare;
 
 	atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
 	if (IS_ERR(atmel_port->gpios)) {
 		ret = PTR_ERR(atmel_port->gpios);
-		goto err_clear_bit;
+		goto err_clk_disable_unprepare;
 	}
 
 	if (!atmel_use_pdc_rx(&atmel_port->uart)) {
@@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
 				     sizeof(struct atmel_uart_char),
 				     GFP_KERNEL);
 		if (!data)
-			goto err_alloc_ring;
+			goto err_clk_disable_unprepare;
 		atmel_port->rx_ring.buf = data;
 	}
 
@@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_port;
 
-#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
-	if (uart_console(&atmel_port->uart)
-			&& ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
-		/*
-		 * The serial core enabled the clock for us, so undo
-		 * the clk_prepare_enable() in atmel_console_setup()
-		 */
-		clk_disable_unprepare(atmel_port->clk);
-	}
-#endif
-
 	device_init_wakeup(&pdev->dev, 1);
 	platform_set_drvdata(pdev, atmel_port);
 
-	/*
-	 * The peripheral clock has been disabled by atmel_init_port():
-	 * enable it before accessing I/O registers
-	 */
-	clk_prepare_enable(atmel_port->clk);
-
 	if (rs485_enabled) {
 		atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
 				  ATMEL_US_USMODE_NORMAL);
@@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
 err_add_port:
 	kfree(atmel_port->rx_ring.buf);
 	atmel_port->rx_ring.buf = NULL;
-err_alloc_ring:
-	if (!uart_console(&atmel_port->uart)) {
-		clk_put(atmel_port->clk);
-		atmel_port->clk = NULL;
-	}
-err_clear_bit:
+err_clk_disable_unprepare:
+	clk_disable_unprepare(atmel_port->clk);
 	clear_bit(atmel_port->uart.line, atmel_ports_in_use);
 err:
 	return ret;
@@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev)
 
 	clear_bit(port->line, atmel_ports_in_use);
 
-	clk_put(atmel_port->clk);
-	atmel_port->clk = NULL;
 	pdev->dev.of_node = NULL;
 
 	return ret;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] serial: st-asc: remove include of pm_runtime.h

[Claudiu Beznea]

st-asc driver doesn't use helpers from pm_runtime.h thus remove its
include.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/st-asc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index 1b0da603ab54..cce42f4c9bc2 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -17,7 +17,6 @@
 #include <linux/tty_flip.h>
 #include <linux/delay.h>
 #include <linux/spinlock.h>
-#include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/serial_core.h>
-- 
2.34.1

[PATCH 3/3] serial: st-asc: remove include of pm_runtime.h

[Claudiu Beznea]

st-asc driver doesn't use helpers from pm_runtime.h thus remove its
include.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/tty/serial/st-asc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index 1b0da603ab54..cce42f4c9bc2 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -17,7 +17,6 @@
 #include <linux/tty_flip.h>
 #include <linux/delay.h>
 #include <linux/spinlock.h>
-#include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/serial_core.h>
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h

[Patrice CHOTARD]

Hi Claudiu

On 5/25/22 15:37, Claudiu Beznea wrote:
> st-asc driver doesn't use helpers from pm_runtime.h thus remove its
> include.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/tty/serial/st-asc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> index 1b0da603ab54..cce42f4c9bc2 100644
> --- a/drivers/tty/serial/st-asc.c
> +++ b/drivers/tty/serial/st-asc.c
> @@ -17,7 +17,6 @@
>  #include <linux/tty_flip.h>
>  #include <linux/delay.h>
>  #include <linux/spinlock.h>
> -#include <linux/pm_runtime.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/serial_core.h>


Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

Re: [PATCH 3/3] serial: st-asc: remove include of pm_runtime.h

[Patrice CHOTARD]

Hi Claudiu

On 5/25/22 15:37, Claudiu Beznea wrote:
> st-asc driver doesn't use helpers from pm_runtime.h thus remove its
> include.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/tty/serial/st-asc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> index 1b0da603ab54..cce42f4c9bc2 100644
> --- a/drivers/tty/serial/st-asc.c
> +++ b/drivers/tty/serial/st-asc.c
> @@ -17,7 +17,6 @@
>  #include <linux/tty_flip.h>
>  #include <linux/delay.h>
>  #include <linux/spinlock.h>
> -#include <linux/pm_runtime.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/serial_core.h>


Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops

[Richard Genoud]

Hi,

Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
> Stop using legacy PM ops and switch using dev_pm_ops. Along with
> it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used
> instead. Coding style recommends (at chapter Conditional Compilation)
> to avoid using preprocessor conditional and use __maybe_unused
> instead.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Richard Genoud <richard.genoud@gmail.com>



Thanks !

Re: [PATCH 1/3] tty: serial: atmel: stop using legacy pm ops

[Richard Genoud]

Hi,

Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
> Stop using legacy PM ops and switch using dev_pm_ops. Along with
> it #ifdef CONFIG_PM are removed and __maybe_unused and pm_ptr() used
> instead. Coding style recommends (at chapter Conditional Compilation)
> to avoid using preprocessor conditional and use __maybe_unused
> instead.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Acked-by: Richard Genoud <richard.genoud@gmail.com>



Thanks !

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Richard Genoud]

Hi,

Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
> atmel_port->clk was requested in atmel_init_port() (which is called only
> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
> NULL on probing path. Thus don't check this. Along with this the clock is
> now requested with devm_clk_get() and the clock request has been moved in
> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
> path for multiple times. All the checks of atmel_port->clk were removed
> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
> clock is enabled at the beginning and disabled at the end of probe. As
> atmel_console_setup() is called in the middle of probe and clock is
> already enabled at that time the clk_prepare_enable() in
> atmel_console_setup() has also been removed.
Could you split this patch into smaller steps ?
I think it will be easier to read and review.

> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>   drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>   1 file changed, 15 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 5c793a23dc54..2955b1012014 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>   	if (ret)
>   		return ret;
>   
> -	/* for console, the clock could already be configured */
> -	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&mpdev->dev, "usart");
> -		if (IS_ERR(atmel_port->clk)) {
> -			ret = PTR_ERR(atmel_port->clk);
> -			atmel_port->clk = NULL;
> -			return ret;
> -		}
> -		ret = clk_prepare_enable(atmel_port->clk);
> -		if (ret) {
> -			clk_put(atmel_port->clk);
> -			atmel_port->clk = NULL;
> -			return ret;
> -		}
> -		port->uartclk = clk_get_rate(atmel_port->clk);
> -		clk_disable_unprepare(atmel_port->clk);
> -		/* only enable clock when USART is in use */
> -	}
> +	port->uartclk = clk_get_rate(atmel_port->clk);
>   
>   	/*
>   	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options)
>   		return -ENODEV;
>   	}
>   
> -	ret = clk_prepare_enable(atmel_ports[co->index].clk);
> -	if (ret)
> -		return ret;
> -
Now, "int ret;" is unused, you can remove it.

>   	atmel_uart_writel(port, ATMEL_US_IDR, -1);
>   	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
>   	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	atomic_set(&atmel_port->tasklet_shutdown, 0);
>   	spin_lock_init(&atmel_port->lock_suspended);
>   
> +	atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
> +	if (IS_ERR(atmel_port->clk)) {
> +		ret = PTR_ERR(atmel_port->clk);
> +		goto err;
> +	}
> +	ret = clk_prepare_enable(atmel_port->clk);
> +	if (ret)
> +		goto err;
> +
>   	ret = atmel_init_port(atmel_port, pdev);
>   	if (ret)
> -		goto err_clear_bit;
> +		goto err_clk_disable_unprepare;
>   
>   	atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>   	if (IS_ERR(atmel_port->gpios)) {
>   		ret = PTR_ERR(atmel_port->gpios);
> -		goto err_clear_bit;
> +		goto err_clk_disable_unprepare;
>   	}
>   
>   	if (!atmel_use_pdc_rx(&atmel_port->uart)) {
> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   				     sizeof(struct atmel_uart_char),
>   				     GFP_KERNEL);
>   		if (!data)
> -			goto err_alloc_ring;
> +			goto err_clk_disable_unprepare;
>   		atmel_port->rx_ring.buf = data;
>   	}
>   
> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_add_port;
>   
> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
> -	if (uart_console(&atmel_port->uart)
> -			&& ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
> -		/*
> -		 * The serial core enabled the clock for us, so undo
> -		 * the clk_prepare_enable() in atmel_console_setup()
> -		 */
> -		clk_disable_unprepare(atmel_port->clk);
> -	}
> -#endif
> -
>   	device_init_wakeup(&pdev->dev, 1);
>   	platform_set_drvdata(pdev, atmel_port);
>   
> -	/*
> -	 * The peripheral clock has been disabled by atmel_init_port():
> -	 * enable it before accessing I/O registers
> -	 */
> -	clk_prepare_enable(atmel_port->clk);
> -
>   	if (rs485_enabled) {
>   		atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>   				  ATMEL_US_USMODE_NORMAL);
> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   err_add_port:
>   	kfree(atmel_port->rx_ring.buf);
>   	atmel_port->rx_ring.buf = NULL;
> -err_alloc_ring:
> -	if (!uart_console(&atmel_port->uart)) {
> -		clk_put(atmel_port->clk);
> -		atmel_port->clk = NULL;
> -	}
> -err_clear_bit:
> +err_clk_disable_unprepare:
> +	clk_disable_unprepare(atmel_port->clk);
>   	clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>   err:
>   	return ret;
> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev)
>   
>   	clear_bit(port->line, atmel_ports_in_use);
>   
> -	clk_put(atmel_port->clk);
> -	atmel_port->clk = NULL;
>   	pdev->dev.of_node = NULL;
>   
>   	return ret;
Thanks !

Regards,
Richard

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Richard Genoud]

Hi,

Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
> atmel_port->clk was requested in atmel_init_port() (which is called only
> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
> NULL on probing path. Thus don't check this. Along with this the clock is
> now requested with devm_clk_get() and the clock request has been moved in
> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
> path for multiple times. All the checks of atmel_port->clk were removed
> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
> clock is enabled at the beginning and disabled at the end of probe. As
> atmel_console_setup() is called in the middle of probe and clock is
> already enabled at that time the clk_prepare_enable() in
> atmel_console_setup() has also been removed.
Could you split this patch into smaller steps ?
I think it will be easier to read and review.

> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>   drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>   1 file changed, 15 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 5c793a23dc54..2955b1012014 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>   	if (ret)
>   		return ret;
>   
> -	/* for console, the clock could already be configured */
> -	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&mpdev->dev, "usart");
> -		if (IS_ERR(atmel_port->clk)) {
> -			ret = PTR_ERR(atmel_port->clk);
> -			atmel_port->clk = NULL;
> -			return ret;
> -		}
> -		ret = clk_prepare_enable(atmel_port->clk);
> -		if (ret) {
> -			clk_put(atmel_port->clk);
> -			atmel_port->clk = NULL;
> -			return ret;
> -		}
> -		port->uartclk = clk_get_rate(atmel_port->clk);
> -		clk_disable_unprepare(atmel_port->clk);
> -		/* only enable clock when USART is in use */
> -	}
> +	port->uartclk = clk_get_rate(atmel_port->clk);
>   
>   	/*
>   	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct console *co, char *options)
>   		return -ENODEV;
>   	}
>   
> -	ret = clk_prepare_enable(atmel_ports[co->index].clk);
> -	if (ret)
> -		return ret;
> -
Now, "int ret;" is unused, you can remove it.

>   	atmel_uart_writel(port, ATMEL_US_IDR, -1);
>   	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
>   	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	atomic_set(&atmel_port->tasklet_shutdown, 0);
>   	spin_lock_init(&atmel_port->lock_suspended);
>   
> +	atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
> +	if (IS_ERR(atmel_port->clk)) {
> +		ret = PTR_ERR(atmel_port->clk);
> +		goto err;
> +	}
> +	ret = clk_prepare_enable(atmel_port->clk);
> +	if (ret)
> +		goto err;
> +
>   	ret = atmel_init_port(atmel_port, pdev);
>   	if (ret)
> -		goto err_clear_bit;
> +		goto err_clk_disable_unprepare;
>   
>   	atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>   	if (IS_ERR(atmel_port->gpios)) {
>   		ret = PTR_ERR(atmel_port->gpios);
> -		goto err_clear_bit;
> +		goto err_clk_disable_unprepare;
>   	}
>   
>   	if (!atmel_use_pdc_rx(&atmel_port->uart)) {
> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   				     sizeof(struct atmel_uart_char),
>   				     GFP_KERNEL);
>   		if (!data)
> -			goto err_alloc_ring;
> +			goto err_clk_disable_unprepare;
>   		atmel_port->rx_ring.buf = data;
>   	}
>   
> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_add_port;
>   
> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
> -	if (uart_console(&atmel_port->uart)
> -			&& ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
> -		/*
> -		 * The serial core enabled the clock for us, so undo
> -		 * the clk_prepare_enable() in atmel_console_setup()
> -		 */
> -		clk_disable_unprepare(atmel_port->clk);
> -	}
> -#endif
> -
>   	device_init_wakeup(&pdev->dev, 1);
>   	platform_set_drvdata(pdev, atmel_port);
>   
> -	/*
> -	 * The peripheral clock has been disabled by atmel_init_port():
> -	 * enable it before accessing I/O registers
> -	 */
> -	clk_prepare_enable(atmel_port->clk);
> -
>   	if (rs485_enabled) {
>   		atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>   				  ATMEL_US_USMODE_NORMAL);
> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   err_add_port:
>   	kfree(atmel_port->rx_ring.buf);
>   	atmel_port->rx_ring.buf = NULL;
> -err_alloc_ring:
> -	if (!uart_console(&atmel_port->uart)) {
> -		clk_put(atmel_port->clk);
> -		atmel_port->clk = NULL;
> -	}
> -err_clear_bit:
> +err_clk_disable_unprepare:
> +	clk_disable_unprepare(atmel_port->clk);
>   	clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>   err:
>   	return ret;
> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct platform_device *pdev)
>   
>   	clear_bit(port->line, atmel_ports_in_use);
>   
> -	clk_put(atmel_port->clk);
> -	atmel_port->clk = NULL;
>   	pdev->dev.of_node = NULL;
>   
>   	return ret;
Thanks !

Regards,
Richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Claudiu.Beznea]

Hi,

On 02.06.2022 12:29, Richard Genoud wrote:
> 
> Hi,
> 
> Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
>> atmel_port->clk was requested in atmel_init_port() (which is called only
>> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
>> NULL on probing path. Thus don't check this. Along with this the clock is
>> now requested with devm_clk_get() and the clock request has been moved in
>> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
>> path for multiple times. All the checks of atmel_port->clk were removed
>> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
>> clock is enabled at the beginning and disabled at the end of probe. As
>> atmel_console_setup() is called in the middle of probe and clock is
>> already enabled at that time the clk_prepare_enable() in
>> atmel_console_setup() has also been removed.
> Could you split this patch into smaller steps ?
> I think it will be easier to read and review.

I kept it as a single patch as it is all related to clock management.

Having the clock enabled only at the beginning of probe and disabled at the
end of probe lead to removing the code in atmel_init_port(), also removing
the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of
the removed code.

With this, would you still want to split it in multiple patches?

Thank you,
Claudiu Beznea
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>   drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>>   1 file changed, 15 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index 5c793a23dc54..2955b1012014 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port
>> *atmel_port,
>>       if (ret)
>>               return ret;
>>
>> -     /* for console, the clock could already be configured */
>> -     if (!atmel_port->clk) {
>> -             atmel_port->clk = clk_get(&mpdev->dev, "usart");
>> -             if (IS_ERR(atmel_port->clk)) {
>> -                     ret = PTR_ERR(atmel_port->clk);
>> -                     atmel_port->clk = NULL;
>> -                     return ret;
>> -             }
>> -             ret = clk_prepare_enable(atmel_port->clk);
>> -             if (ret) {
>> -                     clk_put(atmel_port->clk);
>> -                     atmel_port->clk = NULL;
>> -                     return ret;
>> -             }
>> -             port->uartclk = clk_get_rate(atmel_port->clk);
>> -             clk_disable_unprepare(atmel_port->clk);
>> -             /* only enable clock when USART is in use */
>> -     }
>> +     port->uartclk = clk_get_rate(atmel_port->clk);
>>
>>       /*
>>        * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct
>> console *co, char *options)
>>               return -ENODEV;
>>       }
>>
>> -     ret = clk_prepare_enable(atmel_ports[co->index].clk);
>> -     if (ret)
>> -             return ret;
>> -
> Now, "int ret;" is unused, you can remove it.
> 
>>       atmel_uart_writel(port, ATMEL_US_IDR, -1);
>>       atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA |
>> ATMEL_US_RSTRX);
>>       atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
>> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>       atomic_set(&atmel_port->tasklet_shutdown, 0);
>>       spin_lock_init(&atmel_port->lock_suspended);
>>
>> +     atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
>> +     if (IS_ERR(atmel_port->clk)) {
>> +             ret = PTR_ERR(atmel_port->clk);
>> +             goto err;
>> +     }
>> +     ret = clk_prepare_enable(atmel_port->clk);
>> +     if (ret)
>> +             goto err;
>> +
>>       ret = atmel_init_port(atmel_port, pdev);
>>       if (ret)
>> -             goto err_clear_bit;
>> +             goto err_clk_disable_unprepare;
>>
>>       atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>>       if (IS_ERR(atmel_port->gpios)) {
>>               ret = PTR_ERR(atmel_port->gpios);
>> -             goto err_clear_bit;
>> +             goto err_clk_disable_unprepare;
>>       }
>>
>>       if (!atmel_use_pdc_rx(&atmel_port->uart)) {
>> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>                                    sizeof(struct atmel_uart_char),
>>                                    GFP_KERNEL);
>>               if (!data)
>> -                     goto err_alloc_ring;
>> +                     goto err_clk_disable_unprepare;
>>               atmel_port->rx_ring.buf = data;
>>       }
>>
>> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>       if (ret)
>>               goto err_add_port;
>>
>> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
>> -     if (uart_console(&atmel_port->uart)
>> -                     && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
>> -             /*
>> -              * The serial core enabled the clock for us, so undo
>> -              * the clk_prepare_enable() in atmel_console_setup()
>> -              */
>> -             clk_disable_unprepare(atmel_port->clk);
>> -     }
>> -#endif
>> -
>>       device_init_wakeup(&pdev->dev, 1);
>>       platform_set_drvdata(pdev, atmel_port);
>>
>> -     /*
>> -      * The peripheral clock has been disabled by atmel_init_port():
>> -      * enable it before accessing I/O registers
>> -      */
>> -     clk_prepare_enable(atmel_port->clk);
>> -
>>       if (rs485_enabled) {
>>               atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>>                                 ATMEL_US_USMODE_NORMAL);
>> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>   err_add_port:
>>       kfree(atmel_port->rx_ring.buf);
>>       atmel_port->rx_ring.buf = NULL;
>> -err_alloc_ring:
>> -     if (!uart_console(&atmel_port->uart)) {
>> -             clk_put(atmel_port->clk);
>> -             atmel_port->clk = NULL;
>> -     }
>> -err_clear_bit:
>> +err_clk_disable_unprepare:
>> +     clk_disable_unprepare(atmel_port->clk);
>>       clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>>   err:
>>       return ret;
>> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct
>> platform_device *pdev)
>>
>>       clear_bit(port->line, atmel_ports_in_use);
>>
>> -     clk_put(atmel_port->clk);
>> -     atmel_port->clk = NULL;
>>       pdev->dev.of_node = NULL;
>>
>>       return ret;
> Thanks !
> 
> Regards,
> Richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Claudiu.Beznea]

Hi,

On 02.06.2022 12:29, Richard Genoud wrote:
> 
> Hi,
> 
> Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
>> atmel_port->clk was requested in atmel_init_port() (which is called only
>> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
>> NULL on probing path. Thus don't check this. Along with this the clock is
>> now requested with devm_clk_get() and the clock request has been moved in
>> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
>> path for multiple times. All the checks of atmel_port->clk were removed
>> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
>> clock is enabled at the beginning and disabled at the end of probe. As
>> atmel_console_setup() is called in the middle of probe and clock is
>> already enabled at that time the clk_prepare_enable() in
>> atmel_console_setup() has also been removed.
> Could you split this patch into smaller steps ?
> I think it will be easier to read and review.

I kept it as a single patch as it is all related to clock management.

Having the clock enabled only at the beginning of probe and disabled at the
end of probe lead to removing the code in atmel_init_port(), also removing
the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of
the removed code.

With this, would you still want to split it in multiple patches?

Thank you,
Claudiu Beznea
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>   drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>>   1 file changed, 15 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index 5c793a23dc54..2955b1012014 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port
>> *atmel_port,
>>       if (ret)
>>               return ret;
>>
>> -     /* for console, the clock could already be configured */
>> -     if (!atmel_port->clk) {
>> -             atmel_port->clk = clk_get(&mpdev->dev, "usart");
>> -             if (IS_ERR(atmel_port->clk)) {
>> -                     ret = PTR_ERR(atmel_port->clk);
>> -                     atmel_port->clk = NULL;
>> -                     return ret;
>> -             }
>> -             ret = clk_prepare_enable(atmel_port->clk);
>> -             if (ret) {
>> -                     clk_put(atmel_port->clk);
>> -                     atmel_port->clk = NULL;
>> -                     return ret;
>> -             }
>> -             port->uartclk = clk_get_rate(atmel_port->clk);
>> -             clk_disable_unprepare(atmel_port->clk);
>> -             /* only enable clock when USART is in use */
>> -     }
>> +     port->uartclk = clk_get_rate(atmel_port->clk);
>>
>>       /*
>>        * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct
>> console *co, char *options)
>>               return -ENODEV;
>>       }
>>
>> -     ret = clk_prepare_enable(atmel_ports[co->index].clk);
>> -     if (ret)
>> -             return ret;
>> -
> Now, "int ret;" is unused, you can remove it.
> 
>>       atmel_uart_writel(port, ATMEL_US_IDR, -1);
>>       atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA |
>> ATMEL_US_RSTRX);
>>       atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
>> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>       atomic_set(&atmel_port->tasklet_shutdown, 0);
>>       spin_lock_init(&atmel_port->lock_suspended);
>>
>> +     atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
>> +     if (IS_ERR(atmel_port->clk)) {
>> +             ret = PTR_ERR(atmel_port->clk);
>> +             goto err;
>> +     }
>> +     ret = clk_prepare_enable(atmel_port->clk);
>> +     if (ret)
>> +             goto err;
>> +
>>       ret = atmel_init_port(atmel_port, pdev);
>>       if (ret)
>> -             goto err_clear_bit;
>> +             goto err_clk_disable_unprepare;
>>
>>       atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>>       if (IS_ERR(atmel_port->gpios)) {
>>               ret = PTR_ERR(atmel_port->gpios);
>> -             goto err_clear_bit;
>> +             goto err_clk_disable_unprepare;
>>       }
>>
>>       if (!atmel_use_pdc_rx(&atmel_port->uart)) {
>> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>                                    sizeof(struct atmel_uart_char),
>>                                    GFP_KERNEL);
>>               if (!data)
>> -                     goto err_alloc_ring;
>> +                     goto err_clk_disable_unprepare;
>>               atmel_port->rx_ring.buf = data;
>>       }
>>
>> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>       if (ret)
>>               goto err_add_port;
>>
>> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
>> -     if (uart_console(&atmel_port->uart)
>> -                     && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
>> -             /*
>> -              * The serial core enabled the clock for us, so undo
>> -              * the clk_prepare_enable() in atmel_console_setup()
>> -              */
>> -             clk_disable_unprepare(atmel_port->clk);
>> -     }
>> -#endif
>> -
>>       device_init_wakeup(&pdev->dev, 1);
>>       platform_set_drvdata(pdev, atmel_port);
>>
>> -     /*
>> -      * The peripheral clock has been disabled by atmel_init_port():
>> -      * enable it before accessing I/O registers
>> -      */
>> -     clk_prepare_enable(atmel_port->clk);
>> -
>>       if (rs485_enabled) {
>>               atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>>                                 ATMEL_US_USMODE_NORMAL);
>> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct
>> platform_device *pdev)
>>   err_add_port:
>>       kfree(atmel_port->rx_ring.buf);
>>       atmel_port->rx_ring.buf = NULL;
>> -err_alloc_ring:
>> -     if (!uart_console(&atmel_port->uart)) {
>> -             clk_put(atmel_port->clk);
>> -             atmel_port->clk = NULL;
>> -     }
>> -err_clear_bit:
>> +err_clk_disable_unprepare:
>> +     clk_disable_unprepare(atmel_port->clk);
>>       clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>>   err:
>>       return ret;
>> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct
>> platform_device *pdev)
>>
>>       clear_bit(port->line, atmel_ports_in_use);
>>
>> -     clk_put(atmel_port->clk);
>> -     atmel_port->clk = NULL;
>>       pdev->dev.of_node = NULL;
>>
>>       return ret;
> Thanks !
> 
> Regards,
> Richard

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Richard Genoud]

Hi,
Le 08/06/2022 à 10:18, Claudiu.Beznea@microchip.com a écrit :
> Hi,
> 
> On 02.06.2022 12:29, Richard Genoud wrote:
>>
>> Hi,
>>
>> Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
>>> atmel_port->clk was requested in atmel_init_port() (which is called only
>>> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
>>> NULL on probing path. Thus don't check this. Along with this the clock is
>>> now requested with devm_clk_get() and the clock request has been moved in
>>> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
>>> path for multiple times. All the checks of atmel_port->clk were removed
>>> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
>>> clock is enabled at the beginning and disabled at the end of probe. As
>>> atmel_console_setup() is called in the middle of probe and clock is
>>> already enabled at that time the clk_prepare_enable() in
>>> atmel_console_setup() has also been removed.
>> Could you split this patch into smaller steps ?
>> I think it will be easier to read and review.
> 
> I kept it as a single patch as it is all related to clock management.
> 
> Having the clock enabled only at the beginning of probe and disabled at the
> end of probe lead to removing the code in atmel_init_port(), also removing
> the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of
> the removed code.
> 
> With this, would you still want to split it in multiple patches?

I think that, at least, the switch from clk_get() to devm_clk_get() can 
be in a separate patch.
But in general, I prefer when patches are self-explaining rather than a 
bigger patch with a longer explanation of what it does in the commit 
message.

Regards,
Richard

> 
> Thank you,
> Claudiu Beznea
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>    drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>>>    1 file changed, 15 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>> b/drivers/tty/serial/atmel_serial.c
>>> index 5c793a23dc54..2955b1012014 100644
>>> --- a/drivers/tty/serial/atmel_serial.c
>>> +++ b/drivers/tty/serial/atmel_serial.c
>>> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port
>>> *atmel_port,
>>>        if (ret)
>>>                return ret;
>>>
>>> -     /* for console, the clock could already be configured */
>>> -     if (!atmel_port->clk) {
>>> -             atmel_port->clk = clk_get(&mpdev->dev, "usart");
>>> -             if (IS_ERR(atmel_port->clk)) {
>>> -                     ret = PTR_ERR(atmel_port->clk);
>>> -                     atmel_port->clk = NULL;
>>> -                     return ret;
>>> -             }
>>> -             ret = clk_prepare_enable(atmel_port->clk);
>>> -             if (ret) {
>>> -                     clk_put(atmel_port->clk);
>>> -                     atmel_port->clk = NULL;
>>> -                     return ret;
>>> -             }
>>> -             port->uartclk = clk_get_rate(atmel_port->clk);
>>> -             clk_disable_unprepare(atmel_port->clk);
>>> -             /* only enable clock when USART is in use */
>>> -     }
>>> +     port->uartclk = clk_get_rate(atmel_port->clk);
>>>
>>>        /*
>>>         * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>>> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct
>>> console *co, char *options)
>>>                return -ENODEV;
>>>        }
>>>
>>> -     ret = clk_prepare_enable(atmel_ports[co->index].clk);
>>> -     if (ret)
>>> -             return ret;
>>> -
>> Now, "int ret;" is unused, you can remove it.
>>
>>>        atmel_uart_writel(port, ATMEL_US_IDR, -1);
>>>        atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA |
>>> ATMEL_US_RSTRX);
>>>        atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
>>> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>        atomic_set(&atmel_port->tasklet_shutdown, 0);
>>>        spin_lock_init(&atmel_port->lock_suspended);
>>>
>>> +     atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
>>> +     if (IS_ERR(atmel_port->clk)) {
>>> +             ret = PTR_ERR(atmel_port->clk);
>>> +             goto err;
>>> +     }
>>> +     ret = clk_prepare_enable(atmel_port->clk);
>>> +     if (ret)
>>> +             goto err;
>>> +
>>>        ret = atmel_init_port(atmel_port, pdev);
>>>        if (ret)
>>> -             goto err_clear_bit;
>>> +             goto err_clk_disable_unprepare;
>>>
>>>        atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>>>        if (IS_ERR(atmel_port->gpios)) {
>>>                ret = PTR_ERR(atmel_port->gpios);
>>> -             goto err_clear_bit;
>>> +             goto err_clk_disable_unprepare;
>>>        }
>>>
>>>        if (!atmel_use_pdc_rx(&atmel_port->uart)) {
>>> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>                                     sizeof(struct atmel_uart_char),
>>>                                     GFP_KERNEL);
>>>                if (!data)
>>> -                     goto err_alloc_ring;
>>> +                     goto err_clk_disable_unprepare;
>>>                atmel_port->rx_ring.buf = data;
>>>        }
>>>
>>> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>        if (ret)
>>>                goto err_add_port;
>>>
>>> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
>>> -     if (uart_console(&atmel_port->uart)
>>> -                     && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
>>> -             /*
>>> -              * The serial core enabled the clock for us, so undo
>>> -              * the clk_prepare_enable() in atmel_console_setup()
>>> -              */
>>> -             clk_disable_unprepare(atmel_port->clk);
>>> -     }
>>> -#endif
>>> -
>>>        device_init_wakeup(&pdev->dev, 1);
>>>        platform_set_drvdata(pdev, atmel_port);
>>>
>>> -     /*
>>> -      * The peripheral clock has been disabled by atmel_init_port():
>>> -      * enable it before accessing I/O registers
>>> -      */
>>> -     clk_prepare_enable(atmel_port->clk);
>>> -
>>>        if (rs485_enabled) {
>>>                atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>>>                                  ATMEL_US_USMODE_NORMAL);
>>> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>    err_add_port:
>>>        kfree(atmel_port->rx_ring.buf);
>>>        atmel_port->rx_ring.buf = NULL;
>>> -err_alloc_ring:
>>> -     if (!uart_console(&atmel_port->uart)) {
>>> -             clk_put(atmel_port->clk);
>>> -             atmel_port->clk = NULL;
>>> -     }
>>> -err_clear_bit:
>>> +err_clk_disable_unprepare:
>>> +     clk_disable_unprepare(atmel_port->clk);
>>>        clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>>>    err:
>>>        return ret;
>>> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct
>>> platform_device *pdev)
>>>
>>>        clear_bit(port->line, atmel_ports_in_use);
>>>
>>> -     clk_put(atmel_port->clk);
>>> -     atmel_port->clk = NULL;
>>>        pdev->dev.of_node = NULL;
>>>
>>>        return ret;
>> Thanks !
>>
>> Regards,
>> Richard
> 

Re: [PATCH 2/3] tty: serial: atmel: improve clock management

[Richard Genoud]

Hi,
Le 08/06/2022 à 10:18, Claudiu.Beznea@microchip.com a écrit :
> Hi,
> 
> On 02.06.2022 12:29, Richard Genoud wrote:
>>
>> Hi,
>>
>> Le 25/05/2022 à 15:37, Claudiu Beznea a écrit :
>>> atmel_port->clk was requested in atmel_init_port() (which is called only
>>> on probe path) only if atmel_port->clk was NULL. But atmel_port->clk is
>>> NULL on probing path. Thus don't check this. Along with this the clock is
>>> now requested with devm_clk_get() and the clock request has been moved in
>>> atmel_serial_probe() function to avoid disabling/re-enabling it on probe
>>> path for multiple times. All the checks of atmel_port->clk were removed
>>> along with clk_put() and atmel_port->clk = NULL. Now, on probing time the
>>> clock is enabled at the beginning and disabled at the end of probe. As
>>> atmel_console_setup() is called in the middle of probe and clock is
>>> already enabled at that time the clk_prepare_enable() in
>>> atmel_console_setup() has also been removed.
>> Could you split this patch into smaller steps ?
>> I think it will be easier to read and review.
> 
> I kept it as a single patch as it is all related to clock management.
> 
> Having the clock enabled only at the beginning of probe and disabled at the
> end of probe lead to removing the code in atmel_init_port(), also removing
> the code under CONFIG_SERIAL_ATMEL_CONSOLE in probe. Same for the rest of
> the removed code.
> 
> With this, would you still want to split it in multiple patches?

I think that, at least, the switch from clk_get() to devm_clk_get() can 
be in a separate patch.
But in general, I prefer when patches are self-explaining rather than a 
bigger patch with a longer explanation of what it does in the commit 
message.

Regards,
Richard

> 
> Thank you,
> Claudiu Beznea
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>    drivers/tty/serial/atmel_serial.c | 65 +++++++------------------------
>>>    1 file changed, 15 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>> b/drivers/tty/serial/atmel_serial.c
>>> index 5c793a23dc54..2955b1012014 100644
>>> --- a/drivers/tty/serial/atmel_serial.c
>>> +++ b/drivers/tty/serial/atmel_serial.c
>>> @@ -2501,24 +2501,7 @@ static int atmel_init_port(struct atmel_uart_port
>>> *atmel_port,
>>>        if (ret)
>>>                return ret;
>>>
>>> -     /* for console, the clock could already be configured */
>>> -     if (!atmel_port->clk) {
>>> -             atmel_port->clk = clk_get(&mpdev->dev, "usart");
>>> -             if (IS_ERR(atmel_port->clk)) {
>>> -                     ret = PTR_ERR(atmel_port->clk);
>>> -                     atmel_port->clk = NULL;
>>> -                     return ret;
>>> -             }
>>> -             ret = clk_prepare_enable(atmel_port->clk);
>>> -             if (ret) {
>>> -                     clk_put(atmel_port->clk);
>>> -                     atmel_port->clk = NULL;
>>> -                     return ret;
>>> -             }
>>> -             port->uartclk = clk_get_rate(atmel_port->clk);
>>> -             clk_disable_unprepare(atmel_port->clk);
>>> -             /* only enable clock when USART is in use */
>>> -     }
>>> +     port->uartclk = clk_get_rate(atmel_port->clk);
>>>
>>>        /*
>>>         * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>>> @@ -2640,10 +2623,6 @@ static int __init atmel_console_setup(struct
>>> console *co, char *options)
>>>                return -ENODEV;
>>>        }
>>>
>>> -     ret = clk_prepare_enable(atmel_ports[co->index].clk);
>>> -     if (ret)
>>> -             return ret;
>>> -
>> Now, "int ret;" is unused, you can remove it.
>>
>>>        atmel_uart_writel(port, ATMEL_US_IDR, -1);
>>>        atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA |
>>> ATMEL_US_RSTRX);
>>>        atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
>>> @@ -2889,14 +2868,23 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>        atomic_set(&atmel_port->tasklet_shutdown, 0);
>>>        spin_lock_init(&atmel_port->lock_suspended);
>>>
>>> +     atmel_port->clk = devm_clk_get(&pdev->dev, "usart");
>>> +     if (IS_ERR(atmel_port->clk)) {
>>> +             ret = PTR_ERR(atmel_port->clk);
>>> +             goto err;
>>> +     }
>>> +     ret = clk_prepare_enable(atmel_port->clk);
>>> +     if (ret)
>>> +             goto err;
>>> +
>>>        ret = atmel_init_port(atmel_port, pdev);
>>>        if (ret)
>>> -             goto err_clear_bit;
>>> +             goto err_clk_disable_unprepare;
>>>
>>>        atmel_port->gpios = mctrl_gpio_init(&atmel_port->uart, 0);
>>>        if (IS_ERR(atmel_port->gpios)) {
>>>                ret = PTR_ERR(atmel_port->gpios);
>>> -             goto err_clear_bit;
>>> +             goto err_clk_disable_unprepare;
>>>        }
>>>
>>>        if (!atmel_use_pdc_rx(&atmel_port->uart)) {
>>> @@ -2905,7 +2893,7 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>                                     sizeof(struct atmel_uart_char),
>>>                                     GFP_KERNEL);
>>>                if (!data)
>>> -                     goto err_alloc_ring;
>>> +                     goto err_clk_disable_unprepare;
>>>                atmel_port->rx_ring.buf = data;
>>>        }
>>>
>>> @@ -2915,26 +2903,9 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>        if (ret)
>>>                goto err_add_port;
>>>
>>> -#ifdef CONFIG_SERIAL_ATMEL_CONSOLE
>>> -     if (uart_console(&atmel_port->uart)
>>> -                     && ATMEL_CONSOLE_DEVICE->flags & CON_ENABLED) {
>>> -             /*
>>> -              * The serial core enabled the clock for us, so undo
>>> -              * the clk_prepare_enable() in atmel_console_setup()
>>> -              */
>>> -             clk_disable_unprepare(atmel_port->clk);
>>> -     }
>>> -#endif
>>> -
>>>        device_init_wakeup(&pdev->dev, 1);
>>>        platform_set_drvdata(pdev, atmel_port);
>>>
>>> -     /*
>>> -      * The peripheral clock has been disabled by atmel_init_port():
>>> -      * enable it before accessing I/O registers
>>> -      */
>>> -     clk_prepare_enable(atmel_port->clk);
>>> -
>>>        if (rs485_enabled) {
>>>                atmel_uart_writel(&atmel_port->uart, ATMEL_US_MR,
>>>                                  ATMEL_US_USMODE_NORMAL);
>>> @@ -2958,12 +2929,8 @@ static int atmel_serial_probe(struct
>>> platform_device *pdev)
>>>    err_add_port:
>>>        kfree(atmel_port->rx_ring.buf);
>>>        atmel_port->rx_ring.buf = NULL;
>>> -err_alloc_ring:
>>> -     if (!uart_console(&atmel_port->uart)) {
>>> -             clk_put(atmel_port->clk);
>>> -             atmel_port->clk = NULL;
>>> -     }
>>> -err_clear_bit:
>>> +err_clk_disable_unprepare:
>>> +     clk_disable_unprepare(atmel_port->clk);
>>>        clear_bit(atmel_port->uart.line, atmel_ports_in_use);
>>>    err:
>>>        return ret;
>>> @@ -2997,8 +2964,6 @@ static int atmel_serial_remove(struct
>>> platform_device *pdev)
>>>
>>>        clear_bit(port->line, atmel_ports_in_use);
>>>
>>> -     clk_put(atmel_port->clk);
>>> -     atmel_port->clk = NULL;
>>>        pdev->dev.of_node = NULL;
>>>
>>>        return ret;
>> Thanks !
>>
>> Regards,
>> Richard
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel