tty: serial: 8250_mtk: Add support for second clock

tty: serial: 8250_mtk: Add support for second clock

[Sascha Hauer]

The following changes since commit 39a8804455fb23f09157341d3ba7db6d7ae6ee76:

  Linux 4.0 (2015-04-12 15:12:50 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/sha/linux-2.6.git tags/v4.0-mtk-uart-clk

for you to fetch changes up to 91bb93e93bf0916f9ebf571e8ab7cb671322ebc8:

  tty: serial: 8250_mtk: Add support for bus clock (2015-04-23 10:36:04 +0200)

----------------------------------------------------------------
This series adds support for the second clock on the Mediatek UART.
This is necessary for SoCs needing a bus clock to be enabled before
the UART can be accessed.

----------------------------------------------------------------
Sascha Hauer (4):
      tty: serial: 8250_mtk: Use devm_clk_get
      tty: serial: 8250_mtk: remove unnecessary test
      tty: serial: 8250_mtk: use pm_runtime callbacks for enabling
      tty: serial: 8250_mtk: Add support for bus clock

 .../devicetree/bindings/serial/mtk-uart.txt        |  9 ++-
 drivers/tty/serial/8250/8250_mtk.c                 | 92 ++++++++++++----------
 2 files changed, 57 insertions(+), 44 deletions(-)

tty: serial: 8250_mtk: Add support for second clock

[Sascha Hauer]

The following changes since commit 39a8804455fb23f09157341d3ba7db6d7ae6ee76:

  Linux 4.0 (2015-04-12 15:12:50 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/sha/linux-2.6.git tags/v4.0-mtk-uart-clk

for you to fetch changes up to 91bb93e93bf0916f9ebf571e8ab7cb671322ebc8:

  tty: serial: 8250_mtk: Add support for bus clock (2015-04-23 10:36:04 +0200)

----------------------------------------------------------------
This series adds support for the second clock on the Mediatek UART.
This is necessary for SoCs needing a bus clock to be enabled before
the UART can be accessed.

----------------------------------------------------------------
Sascha Hauer (4):
      tty: serial: 8250_mtk: Use devm_clk_get
      tty: serial: 8250_mtk: remove unnecessary test
      tty: serial: 8250_mtk: use pm_runtime callbacks for enabling
      tty: serial: 8250_mtk: Add support for bus clock

 .../devicetree/bindings/serial/mtk-uart.txt        |  9 ++-
 drivers/tty/serial/8250/8250_mtk.c                 | 92 ++++++++++++----------
 2 files changed, 57 insertions(+), 44 deletions(-)

tty: serial: 8250_mtk: Add support for second clock

[Sascha Hauer]

The following changes since commit 39a8804455fb23f09157341d3ba7db6d7ae6ee76:

  Linux 4.0 (2015-04-12 15:12:50 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/sha/linux-2.6.git tags/v4.0-mtk-uart-clk

for you to fetch changes up to 91bb93e93bf0916f9ebf571e8ab7cb671322ebc8:

  tty: serial: 8250_mtk: Add support for bus clock (2015-04-23 10:36:04 +0200)

----------------------------------------------------------------
This series adds support for the second clock on the Mediatek UART.
This is necessary for SoCs needing a bus clock to be enabled before
the UART can be accessed.

----------------------------------------------------------------
Sascha Hauer (4):
      tty: serial: 8250_mtk: Use devm_clk_get
      tty: serial: 8250_mtk: remove unnecessary test
      tty: serial: 8250_mtk: use pm_runtime callbacks for enabling
      tty: serial: 8250_mtk: Add support for bus clock

 .../devicetree/bindings/serial/mtk-uart.txt        |  9 ++-
 drivers/tty/serial/8250/8250_mtk.c                 | 92 ++++++++++++----------
 2 files changed, 57 insertions(+), 44 deletions(-)

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
which is the correct function when a struct device * is available. Also
since it's a devm function we can drop the calls to clk_put(). While at
it also fix a wrong debug message.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 7a11fac..c6901dc 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
 	int err;
-	struct device_node *np = pdev->dev.of_node;
 
-	data->uart_clk = of_clk_get(np, 0);
+	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(data->uart_clk)) {
-		dev_warn(&pdev->dev, "Can't get timer clock\n");
+		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
 	}
 
 	err = clk_prepare_enable(data->uart_clk);
 	if (err) {
 		dev_warn(&pdev->dev, "Can't prepare clock\n");
-		clk_put(data->uart_clk);
 		return err;
 	}
 	p->uartclk = clk_get_rate(data->uart_clk);
@@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
 	serial8250_unregister_port(data->line);
 	if (!IS_ERR(data->uart_clk)) {
 		clk_disable_unprepare(data->uart_clk);
-		clk_put(data->uart_clk);
 	}
 
 	pm_runtime_disable(&pdev->dev);
-- 
2.1.4

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
which is the correct function when a struct device * is available. Also
since it's a devm function we can drop the calls to clk_put(). While at
it also fix a wrong debug message.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 7a11fac..c6901dc 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
 	int err;
-	struct device_node *np = pdev->dev.of_node;
 
-	data->uart_clk = of_clk_get(np, 0);
+	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(data->uart_clk)) {
-		dev_warn(&pdev->dev, "Can't get timer clock\n");
+		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
 	}
 
 	err = clk_prepare_enable(data->uart_clk);
 	if (err) {
 		dev_warn(&pdev->dev, "Can't prepare clock\n");
-		clk_put(data->uart_clk);
 		return err;
 	}
 	p->uartclk = clk_get_rate(data->uart_clk);
@@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
 	serial8250_unregister_port(data->line);
 	if (!IS_ERR(data->uart_clk)) {
 		clk_disable_unprepare(data->uart_clk);
-		clk_put(data->uart_clk);
 	}
 
 	pm_runtime_disable(&pdev->dev);
-- 
2.1.4

[PATCH 2/4] tty: serial: 8250_mtk: remove unnecessary test

[Sascha Hauer]

When the driver has probed successfully the clk pointer is always valid,
so no need to test for it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c6901dc..bcfaa8dc 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -212,9 +212,7 @@ static int mtk8250_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	serial8250_unregister_port(data->line);
-	if (!IS_ERR(data->uart_clk)) {
-		clk_disable_unprepare(data->uart_clk);
-	}
+	clk_disable_unprepare(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -246,8 +244,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->uart_clk);
 
 	return 0;
 }
@@ -256,8 +253,7 @@ static int mtk8250_runtime_resume(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_prepare_enable(data->uart_clk);
+	clk_prepare_enable(data->uart_clk);
 
 	return 0;
 }
-- 
2.1.4

[PATCH 2/4] tty: serial: 8250_mtk: remove unnecessary test

[Sascha Hauer]

When the driver has probed successfully the clk pointer is always valid,
so no need to test for it.

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/tty/serial/8250/8250_mtk.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c6901dc..bcfaa8dc 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -212,9 +212,7 @@ static int mtk8250_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	serial8250_unregister_port(data->line);
-	if (!IS_ERR(data->uart_clk)) {
-		clk_disable_unprepare(data->uart_clk);
-	}
+	clk_disable_unprepare(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -246,8 +244,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->uart_clk);
 
 	return 0;
 }
@@ -256,8 +253,7 @@ static int mtk8250_runtime_resume(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_prepare_enable(data->uart_clk);
+	clk_prepare_enable(data->uart_clk);
 
 	return 0;
 }
-- 
2.1.4

[PATCH 2/4] tty: serial: 8250_mtk: remove unnecessary test

[Sascha Hauer]

When the driver has probed successfully the clk pointer is always valid,
so no need to test for it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c6901dc..bcfaa8dc 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -212,9 +212,7 @@ static int mtk8250_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	serial8250_unregister_port(data->line);
-	if (!IS_ERR(data->uart_clk)) {
-		clk_disable_unprepare(data->uart_clk);
-	}
+	clk_disable_unprepare(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -246,8 +244,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->uart_clk);
 
 	return 0;
 }
@@ -256,8 +253,7 @@ static int mtk8250_runtime_resume(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
-	if (!IS_ERR(data->uart_clk))
-		clk_prepare_enable(data->uart_clk);
+	clk_prepare_enable(data->uart_clk);
 
 	return 0;
 }
-- 
2.1.4

[PATCH 3/4] tty: serial: 8250_mtk: use pm_runtime callbacks for enabling

[Sascha Hauer]

The pm_runtime callbacks already enable and disable the device.
Use them in probe() and remove() instead of duplicating the
code. This allows us to concentrate more code for enabling/disabling
the UART in a single place.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 69 ++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index bcfaa8dc..2f28bd0 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -115,6 +115,29 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
 		tty_termios_encode_baud_rate(termios, baud, baud);
 }
 
+static int mtk8250_runtime_suspend(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(data->uart_clk);
+
+	return 0;
+}
+
+static int mtk8250_runtime_resume(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(data->uart_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable clock\n");
+		return err;
+	}
+
+	return 0;
+}
+
 static void
 mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 {
@@ -130,19 +153,12 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	int err;
-
 	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(data->uart_clk)) {
 		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
 	}
 
-	err = clk_prepare_enable(data->uart_clk);
-	if (err) {
-		dev_warn(&pdev->dev, "Can't prepare clock\n");
-		return err;
-	}
 	p->uartclk = clk_get_rate(data->uart_clk);
 
 	return 0;
@@ -193,14 +209,18 @@ static int mtk8250_probe(struct platform_device *pdev)
 	writel(0x0, uart.port.membase +
 			(MTK_UART_RATE_FIX << uart.port.regshift));
 
-	data->line = serial8250_register_8250_port(&uart);
-	if (data->line < 0)
-		return data->line;
-
 	platform_set_drvdata(pdev, data);
 
-	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		err = mtk8250_runtime_resume(&pdev->dev);
+		if (err)
+			return err;
+	}
+
+	data->line = serial8250_register_8250_port(&uart);
+	if (data->line < 0)
+		return data->line;
 
 	return 0;
 }
@@ -212,10 +232,13 @@ static int mtk8250_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	serial8250_unregister_port(data->line);
-	clk_disable_unprepare(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		mtk8250_runtime_suspend(&pdev->dev);
+
 	return 0;
 }
 
@@ -239,26 +262,6 @@ static int mtk8250_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-#ifdef CONFIG_PM
-static int mtk8250_runtime_suspend(struct device *dev)
-{
-	struct mtk8250_data *data = dev_get_drvdata(dev);
-
-	clk_disable_unprepare(data->uart_clk);
-
-	return 0;
-}
-
-static int mtk8250_runtime_resume(struct device *dev)
-{
-	struct mtk8250_data *data = dev_get_drvdata(dev);
-
-	clk_prepare_enable(data->uart_clk);
-
-	return 0;
-}
-#endif
-
 static const struct dev_pm_ops mtk8250_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mtk8250_suspend, mtk8250_resume)
 	SET_RUNTIME_PM_OPS(mtk8250_runtime_suspend, mtk8250_runtime_resume,
-- 
2.1.4

[PATCH 3/4] tty: serial: 8250_mtk: use pm_runtime callbacks for enabling

[Sascha Hauer]

The pm_runtime callbacks already enable and disable the device.
Use them in probe() and remove() instead of duplicating the
code. This allows us to concentrate more code for enabling/disabling
the UART in a single place.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/8250/8250_mtk.c | 69 ++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index bcfaa8dc..2f28bd0 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -115,6 +115,29 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
 		tty_termios_encode_baud_rate(termios, baud, baud);
 }
 
+static int mtk8250_runtime_suspend(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(data->uart_clk);
+
+	return 0;
+}
+
+static int mtk8250_runtime_resume(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(data->uart_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable clock\n");
+		return err;
+	}
+
+	return 0;
+}
+
 static void
 mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 {
@@ -130,19 +153,12 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	int err;
-
 	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(data->uart_clk)) {
 		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
 	}
 
-	err = clk_prepare_enable(data->uart_clk);
-	if (err) {
-		dev_warn(&pdev->dev, "Can't prepare clock\n");
-		return err;
-	}
 	p->uartclk = clk_get_rate(data->uart_clk);
 
 	return 0;
@@ -193,14 +209,18 @@ static int mtk8250_probe(struct platform_device *pdev)
 	writel(0x0, uart.port.membase +
 			(MTK_UART_RATE_FIX << uart.port.regshift));
 
-	data->line = serial8250_register_8250_port(&uart);
-	if (data->line < 0)
-		return data->line;
-
 	platform_set_drvdata(pdev, data);
 
-	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		err = mtk8250_runtime_resume(&pdev->dev);
+		if (err)
+			return err;
+	}
+
+	data->line = serial8250_register_8250_port(&uart);
+	if (data->line < 0)
+		return data->line;
 
 	return 0;
 }
@@ -212,10 +232,13 @@ static int mtk8250_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	serial8250_unregister_port(data->line);
-	clk_disable_unprepare(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		mtk8250_runtime_suspend(&pdev->dev);
+
 	return 0;
 }
 
@@ -239,26 +262,6 @@ static int mtk8250_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-#ifdef CONFIG_PM
-static int mtk8250_runtime_suspend(struct device *dev)
-{
-	struct mtk8250_data *data = dev_get_drvdata(dev);
-
-	clk_disable_unprepare(data->uart_clk);
-
-	return 0;
-}
-
-static int mtk8250_runtime_resume(struct device *dev)
-{
-	struct mtk8250_data *data = dev_get_drvdata(dev);
-
-	clk_prepare_enable(data->uart_clk);
-
-	return 0;
-}
-#endif
-
 static const struct dev_pm_ops mtk8250_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mtk8250_suspend, mtk8250_resume)
 	SET_RUNTIME_PM_OPS(mtk8250_runtime_suspend, mtk8250_runtime_resume,
-- 
2.1.4

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

The mtk 8250 needs two clocks, one for providing the baudrate and
one that needs to be enabled for register accesses. The latter has
not been supported, this patch adds support for it. It is optional
for now since not all SoCs provide a bus clock.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/devicetree/bindings/serial/mtk-uart.txt |  9 +++++++--
 drivers/tty/serial/8250/8250_mtk.c                    | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index 4415226..369b5c3 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -14,7 +14,11 @@ Required properties:
 
 - interrupts: A single interrupt specifier.
 
-- clocks: Clock driving the hardware.
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names:
+  - "baud": The clock the baudrate is derived from
+  - "bus": The bus clock for register accesses (optional)
 
 Example:
 
@@ -22,5 +26,6 @@ Example:
 		compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
 		reg = <0x11006000 0x400>;
 		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&uart_clk>;
+		clocks = <&uart_clk>, <&bus_clk>;
+		clock-names = "baud", "bus";
 	};
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 2f28bd0..20c27f0 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -34,6 +34,7 @@
 struct mtk8250_data {
 	int			line;
 	struct clk		*uart_clk;
+	struct clk		*bus_clk;
 };
 
 static void
@@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
 	clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->bus_clk);
 
 	return 0;
 }
@@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
 		return err;
 	}
 
+	err = clk_prepare_enable(data->bus_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable bus clock\n");
+		return err;
+	}
+
 	return 0;
 }
 
@@ -153,7 +161,7 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+	data->uart_clk = devm_clk_get(&pdev->dev, "baud");
 	if (IS_ERR(data->uart_clk)) {
 		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
@@ -161,6 +169,10 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 
 	p->uartclk = clk_get_rate(data->uart_clk);
 
+	data->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(data->bus_clk))
+		data->bus_clk = NULL;
+
 	return 0;
 }
 
-- 
2.1.4

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

The mtk 8250 needs two clocks, one for providing the baudrate and
one that needs to be enabled for register accesses. The latter has
not been supported, this patch adds support for it. It is optional
for now since not all SoCs provide a bus clock.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/devicetree/bindings/serial/mtk-uart.txt |  9 +++++++--
 drivers/tty/serial/8250/8250_mtk.c                    | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index 4415226..369b5c3 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -14,7 +14,11 @@ Required properties:
 
 - interrupts: A single interrupt specifier.
 
-- clocks: Clock driving the hardware.
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names:
+  - "baud": The clock the baudrate is derived from
+  - "bus": The bus clock for register accesses (optional)
 
 Example:
 
@@ -22,5 +26,6 @@ Example:
 		compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
 		reg = <0x11006000 0x400>;
 		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&uart_clk>;
+		clocks = <&uart_clk>, <&bus_clk>;
+		clock-names = "baud", "bus";
 	};
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 2f28bd0..20c27f0 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -34,6 +34,7 @@
 struct mtk8250_data {
 	int			line;
 	struct clk		*uart_clk;
+	struct clk		*bus_clk;
 };
 
 static void
@@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
 	clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->bus_clk);
 
 	return 0;
 }
@@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
 		return err;
 	}
 
+	err = clk_prepare_enable(data->bus_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable bus clock\n");
+		return err;
+	}
+
 	return 0;
 }
 
@@ -153,7 +161,7 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+	data->uart_clk = devm_clk_get(&pdev->dev, "baud");
 	if (IS_ERR(data->uart_clk)) {
 		dev_warn(&pdev->dev, "Can't get uart clock\n");
 		return PTR_ERR(data->uart_clk);
@@ -161,6 +169,10 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 
 	p->uartclk = clk_get_rate(data->uart_clk);
 
+	data->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(data->bus_clk))
+		data->bus_clk = NULL;
+
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> which is the correct function when a struct device * is available. Also
> since it's a devm function we can drop the calls to clk_put(). While at
> it also fix a wrong debug message.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 7a11fac..c6901dc 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
>         int err;
> -       struct device_node *np = pdev->dev.of_node;
>
> -       data->uart_clk = of_clk_get(np, 0);
> +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(data->uart_clk)) {
> -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
>         }
>
>         err = clk_prepare_enable(data->uart_clk);
>         if (err) {
>                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> -               clk_put(data->uart_clk);
>                 return err;
>         }
>         p->uartclk = clk_get_rate(data->uart_clk);
> @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>         serial8250_unregister_port(data->line);
>         if (!IS_ERR(data->uart_clk)) {
>                 clk_disable_unprepare(data->uart_clk);
> -               clk_put(data->uart_clk);
>         }

Please delete the braces.


-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> which is the correct function when a struct device * is available. Also
> since it's a devm function we can drop the calls to clk_put(). While at
> it also fix a wrong debug message.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 7a11fac..c6901dc 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
>         int err;
> -       struct device_node *np = pdev->dev.of_node;
>
> -       data->uart_clk = of_clk_get(np, 0);
> +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(data->uart_clk)) {
> -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
>         }
>
>         err = clk_prepare_enable(data->uart_clk);
>         if (err) {
>                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> -               clk_put(data->uart_clk);
>                 return err;
>         }
>         p->uartclk = clk_get_rate(data->uart_clk);
> @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>         serial8250_unregister_port(data->line);
>         if (!IS_ERR(data->uart_clk)) {
>                 clk_disable_unprepare(data->uart_clk);
> -               clk_put(data->uart_clk);
>         }

Please delete the braces.


-- 
motzblog.wordpress.com

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> which is the correct function when a struct device * is available. Also
> since it's a devm function we can drop the calls to clk_put(). While at
> it also fix a wrong debug message.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 7a11fac..c6901dc 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
>         int err;
> -       struct device_node *np = pdev->dev.of_node;
>
> -       data->uart_clk = of_clk_get(np, 0);
> +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(data->uart_clk)) {
> -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
>         }
>
>         err = clk_prepare_enable(data->uart_clk);
>         if (err) {
>                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> -               clk_put(data->uart_clk);
>                 return err;
>         }
>         p->uartclk = clk_get_rate(data->uart_clk);
> @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>         serial8250_unregister_port(data->line);
>         if (!IS_ERR(data->uart_clk)) {
>                 clk_disable_unprepare(data->uart_clk);
> -               clk_put(data->uart_clk);
>         }

Please delete the braces.


-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> > which is the correct function when a struct device * is available. Also
> > since it's a devm function we can drop the calls to clk_put(). While at
> > it also fix a wrong debug message.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 7a11fac..c6901dc 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >                            struct mtk8250_data *data)
> >  {
> >         int err;
> > -       struct device_node *np = pdev->dev.of_node;
> >
> > -       data->uart_clk = of_clk_get(np, 0);
> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >         if (IS_ERR(data->uart_clk)) {
> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >                 return PTR_ERR(data->uart_clk);
> >         }
> >
> >         err = clk_prepare_enable(data->uart_clk);
> >         if (err) {
> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> > -               clk_put(data->uart_clk);
> >                 return err;
> >         }
> >         p->uartclk = clk_get_rate(data->uart_clk);
> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >         serial8250_unregister_port(data->line);
> >         if (!IS_ERR(data->uart_clk)) {
> >                 clk_disable_unprepare(data->uart_clk);
> > -               clk_put(data->uart_clk);
> >         }
> 
> Please delete the braces.

The if() is removed completly in the next patch anyway.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> > which is the correct function when a struct device * is available. Also
> > since it's a devm function we can drop the calls to clk_put(). While at
> > it also fix a wrong debug message.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 7a11fac..c6901dc 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >                            struct mtk8250_data *data)
> >  {
> >         int err;
> > -       struct device_node *np = pdev->dev.of_node;
> >
> > -       data->uart_clk = of_clk_get(np, 0);
> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >         if (IS_ERR(data->uart_clk)) {
> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >                 return PTR_ERR(data->uart_clk);
> >         }
> >
> >         err = clk_prepare_enable(data->uart_clk);
> >         if (err) {
> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> > -               clk_put(data->uart_clk);
> >                 return err;
> >         }
> >         p->uartclk = clk_get_rate(data->uart_clk);
> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >         serial8250_unregister_port(data->line);
> >         if (!IS_ERR(data->uart_clk)) {
> >                 clk_disable_unprepare(data->uart_clk);
> > -               clk_put(data->uart_clk);
> >         }
> 
> Please delete the braces.

The if() is removed completly in the next patch anyway.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> > which is the correct function when a struct device * is available. Also
> > since it's a devm function we can drop the calls to clk_put(). While at
> > it also fix a wrong debug message.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 7a11fac..c6901dc 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >                            struct mtk8250_data *data)
> >  {
> >         int err;
> > -       struct device_node *np = pdev->dev.of_node;
> >
> > -       data->uart_clk = of_clk_get(np, 0);
> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >         if (IS_ERR(data->uart_clk)) {
> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >                 return PTR_ERR(data->uart_clk);
> >         }
> >
> >         err = clk_prepare_enable(data->uart_clk);
> >         if (err) {
> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> > -               clk_put(data->uart_clk);
> >                 return err;
> >         }
> >         p->uartclk = clk_get_rate(data->uart_clk);
> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >         serial8250_unregister_port(data->line);
> >         if (!IS_ERR(data->uart_clk)) {
> >                 clk_disable_unprepare(data->uart_clk);
> > -               clk_put(data->uart_clk);
> >         }
> 
> Please delete the braces.

The if() is removed completly in the next patch anyway.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>> > which is the correct function when a struct device * is available. Also
>> > since it's a devm function we can drop the calls to clk_put(). While at
>> > it also fix a wrong debug message.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 7a11fac..c6901dc 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>> >                            struct mtk8250_data *data)
>> >  {
>> >         int err;
>> > -       struct device_node *np = pdev->dev.of_node;
>> >
>> > -       data->uart_clk = of_clk_get(np, 0);
>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>> >         if (IS_ERR(data->uart_clk)) {
>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>> >                 return PTR_ERR(data->uart_clk);
>> >         }
>> >
>> >         err = clk_prepare_enable(data->uart_clk);
>> >         if (err) {
>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>> > -               clk_put(data->uart_clk);
>> >                 return err;
>> >         }
>> >         p->uartclk = clk_get_rate(data->uart_clk);
>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>> >         serial8250_unregister_port(data->line);
>> >         if (!IS_ERR(data->uart_clk)) {
>> >                 clk_disable_unprepare(data->uart_clk);
>> > -               clk_put(data->uart_clk);
>> >         }
>>
>> Please delete the braces.
>
> The if() is removed completly in the next patch anyway.

You are right. I propose to merge patch 2 and 3, so you get a clean code.
On the way, can you have a look on the commit message? For me it is
not clear what you want to say.

Cheers,
Matthias

-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>> > which is the correct function when a struct device * is available. Also
>> > since it's a devm function we can drop the calls to clk_put(). While at
>> > it also fix a wrong debug message.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 7a11fac..c6901dc 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>> >                            struct mtk8250_data *data)
>> >  {
>> >         int err;
>> > -       struct device_node *np = pdev->dev.of_node;
>> >
>> > -       data->uart_clk = of_clk_get(np, 0);
>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>> >         if (IS_ERR(data->uart_clk)) {
>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>> >                 return PTR_ERR(data->uart_clk);
>> >         }
>> >
>> >         err = clk_prepare_enable(data->uart_clk);
>> >         if (err) {
>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>> > -               clk_put(data->uart_clk);
>> >                 return err;
>> >         }
>> >         p->uartclk = clk_get_rate(data->uart_clk);
>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>> >         serial8250_unregister_port(data->line);
>> >         if (!IS_ERR(data->uart_clk)) {
>> >                 clk_disable_unprepare(data->uart_clk);
>> > -               clk_put(data->uart_clk);
>> >         }
>>
>> Please delete the braces.
>
> The if() is removed completly in the next patch anyway.

You are right. I propose to merge patch 2 and 3, so you get a clean code.
On the way, can you have a look on the commit message? For me it is
not clear what you want to say.

Cheers,
Matthias

-- 
motzblog.wordpress.com

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>> > which is the correct function when a struct device * is available. Also
>> > since it's a devm function we can drop the calls to clk_put(). While at
>> > it also fix a wrong debug message.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 7a11fac..c6901dc 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>> >                            struct mtk8250_data *data)
>> >  {
>> >         int err;
>> > -       struct device_node *np = pdev->dev.of_node;
>> >
>> > -       data->uart_clk = of_clk_get(np, 0);
>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>> >         if (IS_ERR(data->uart_clk)) {
>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>> >                 return PTR_ERR(data->uart_clk);
>> >         }
>> >
>> >         err = clk_prepare_enable(data->uart_clk);
>> >         if (err) {
>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>> > -               clk_put(data->uart_clk);
>> >                 return err;
>> >         }
>> >         p->uartclk = clk_get_rate(data->uart_clk);
>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>> >         serial8250_unregister_port(data->line);
>> >         if (!IS_ERR(data->uart_clk)) {
>> >                 clk_disable_unprepare(data->uart_clk);
>> > -               clk_put(data->uart_clk);
>> >         }
>>
>> Please delete the braces.
>
> The if() is removed completly in the next patch anyway.

You are right. I propose to merge patch 2 and 3, so you get a clean code.
On the way, can you have a look on the commit message? For me it is
not clear what you want to say.

Cheers,
Matthias

-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:53 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>>> > which is the correct function when a struct device * is available. Also
>>> > since it's a devm function we can drop the calls to clk_put(). While at
>>> > it also fix a wrong debug message.
>>> >
>>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> > ---
>>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>>> >
>>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>>> > index 7a11fac..c6901dc 100644
>>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>>> >                            struct mtk8250_data *data)
>>> >  {
>>> >         int err;
>>> > -       struct device_node *np = pdev->dev.of_node;
>>> >
>>> > -       data->uart_clk = of_clk_get(np, 0);
>>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>>> >         if (IS_ERR(data->uart_clk)) {
>>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>>> >                 return PTR_ERR(data->uart_clk);
>>> >         }
>>> >
>>> >         err = clk_prepare_enable(data->uart_clk);
>>> >         if (err) {
>>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>>> > -               clk_put(data->uart_clk);
>>> >                 return err;
>>> >         }
>>> >         p->uartclk = clk_get_rate(data->uart_clk);
>>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>>> >         serial8250_unregister_port(data->line);
>>> >         if (!IS_ERR(data->uart_clk)) {
>>> >                 clk_disable_unprepare(data->uart_clk);
>>> > -               clk_put(data->uart_clk);
>>> >         }
>>>
>>> Please delete the braces.
>>
>> The if() is removed completly in the next patch anyway.
>
> You are right. I propose to merge patch 2 and 3, so you get a clean code.
> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Actually I meant patch 1 and 2 of the series. Sorry for the confusion.


-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:53 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>>> > which is the correct function when a struct device * is available. Also
>>> > since it's a devm function we can drop the calls to clk_put(). While at
>>> > it also fix a wrong debug message.
>>> >
>>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> > ---
>>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>>> >
>>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>>> > index 7a11fac..c6901dc 100644
>>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>>> >                            struct mtk8250_data *data)
>>> >  {
>>> >         int err;
>>> > -       struct device_node *np = pdev->dev.of_node;
>>> >
>>> > -       data->uart_clk = of_clk_get(np, 0);
>>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>>> >         if (IS_ERR(data->uart_clk)) {
>>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>>> >                 return PTR_ERR(data->uart_clk);
>>> >         }
>>> >
>>> >         err = clk_prepare_enable(data->uart_clk);
>>> >         if (err) {
>>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>>> > -               clk_put(data->uart_clk);
>>> >                 return err;
>>> >         }
>>> >         p->uartclk = clk_get_rate(data->uart_clk);
>>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>>> >         serial8250_unregister_port(data->line);
>>> >         if (!IS_ERR(data->uart_clk)) {
>>> >                 clk_disable_unprepare(data->uart_clk);
>>> > -               clk_put(data->uart_clk);
>>> >         }
>>>
>>> Please delete the braces.
>>
>> The if() is removed completly in the next patch anyway.
>
> You are right. I propose to merge patch 2 and 3, so you get a clean code.
> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Actually I meant patch 1 and 2 of the series. Sorry for the confusion.


-- 
motzblog.wordpress.com

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Matthias Brugger]

2015-04-24 12:53 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
>>> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>>> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
>>> > which is the correct function when a struct device * is available. Also
>>> > since it's a devm function we can drop the calls to clk_put(). While at
>>> > it also fix a wrong debug message.
>>> >
>>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> > ---
>>> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>>> >
>>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>>> > index 7a11fac..c6901dc 100644
>>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>>> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>>> >                            struct mtk8250_data *data)
>>> >  {
>>> >         int err;
>>> > -       struct device_node *np = pdev->dev.of_node;
>>> >
>>> > -       data->uart_clk = of_clk_get(np, 0);
>>> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
>>> >         if (IS_ERR(data->uart_clk)) {
>>> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
>>> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
>>> >                 return PTR_ERR(data->uart_clk);
>>> >         }
>>> >
>>> >         err = clk_prepare_enable(data->uart_clk);
>>> >         if (err) {
>>> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
>>> > -               clk_put(data->uart_clk);
>>> >                 return err;
>>> >         }
>>> >         p->uartclk = clk_get_rate(data->uart_clk);
>>> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
>>> >         serial8250_unregister_port(data->line);
>>> >         if (!IS_ERR(data->uart_clk)) {
>>> >                 clk_disable_unprepare(data->uart_clk);
>>> > -               clk_put(data->uart_clk);
>>> >         }
>>>
>>> Please delete the braces.
>>
>> The if() is removed completly in the next patch anyway.
>
> You are right. I propose to merge patch 2 and 3, so you get a clean code.
> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Actually I meant patch 1 and 2 of the series. Sorry for the confusion.


-- 
motzblog.wordpress.com

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/serial/mtk-uart.txt |  9 +++++++--
>  drivers/tty/serial/8250/8250_mtk.c                    | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..369b5c3 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,11 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
>
>  Example:
>
> @@ -22,5 +26,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..20c27f0 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };
>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);
>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +
>         return 0;
>  }
>
> @@ -153,7 +161,7 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
>  static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
> -       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> +       data->uart_clk = devm_clk_get(&pdev->dev, "baud");
>         if (IS_ERR(data->uart_clk)) {
>                 dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
> @@ -161,6 +169,10 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>
>         p->uartclk = clk_get_rate(data->uart_clk);
>
> +       data->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +       if (IS_ERR(data->bus_clk))
> +               data->bus_clk = NULL;
> +

We have to take into account the old DTS definition. This means, that
if devm_clk_get(&pdev->dev, "baud") fails, we have to look for
devm_clk_get(&pdev->dev, NULL).
Only in the case I when clock "baud" does exist, do we check for clock
"bus". Does this make sense?

The backwards compatibility should be reflected in the bindings
documentation as well.

Cheers,
Matthias

-- 
motzblog.wordpress.com

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/serial/mtk-uart.txt |  9 +++++++--
>  drivers/tty/serial/8250/8250_mtk.c                    | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..369b5c3 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,11 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
>
>  Example:
>
> @@ -22,5 +26,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..20c27f0 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };
>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);
>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +
>         return 0;
>  }
>
> @@ -153,7 +161,7 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
>  static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
> -       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> +       data->uart_clk = devm_clk_get(&pdev->dev, "baud");
>         if (IS_ERR(data->uart_clk)) {
>                 dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
> @@ -161,6 +169,10 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>
>         p->uartclk = clk_get_rate(data->uart_clk);
>
> +       data->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +       if (IS_ERR(data->bus_clk))
> +               data->bus_clk = NULL;
> +

We have to take into account the old DTS definition. This means, that
if devm_clk_get(&pdev->dev, "baud") fails, we have to look for
devm_clk_get(&pdev->dev, NULL).
Only in the case I when clock "baud" does exist, do we check for clock
"bus". Does this make sense?

The backwards compatibility should be reflected in the bindings
documentation as well.

Cheers,
Matthias

-- 
motzblog.wordpress.com

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/serial/mtk-uart.txt |  9 +++++++--
>  drivers/tty/serial/8250/8250_mtk.c                    | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..369b5c3 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,11 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
>
>  Example:
>
> @@ -22,5 +26,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..20c27f0 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };
>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);
>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +
>         return 0;
>  }
>
> @@ -153,7 +161,7 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
>  static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>                            struct mtk8250_data *data)
>  {
> -       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> +       data->uart_clk = devm_clk_get(&pdev->dev, "baud");
>         if (IS_ERR(data->uart_clk)) {
>                 dev_warn(&pdev->dev, "Can't get uart clock\n");
>                 return PTR_ERR(data->uart_clk);
> @@ -161,6 +169,10 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
>
>         p->uartclk = clk_get_rate(data->uart_clk);
>
> +       data->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +       if (IS_ERR(data->bus_clk))
> +               data->bus_clk = NULL;
> +

We have to take into account the old DTS definition. This means, that
if devm_clk_get(&pdev->dev, "baud") fails, we have to look for
devm_clk_get(&pdev->dev, NULL).
Only in the case I when clock "baud" does exist, do we check for clock
"bus". Does this make sense?

The backwards compatibility should be reflected in the bindings
documentation as well.

Cheers,
Matthias

-- 
motzblog.wordpress.com

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:53:08PM +0200, Matthias Brugger wrote:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> >> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> >> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> >> > which is the correct function when a struct device * is available. Also
> >> > since it's a devm function we can drop the calls to clk_put(). While at
> >> > it also fix a wrong debug message.
> >> >
> >> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >> > ---
> >> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> >> > index 7a11fac..c6901dc 100644
> >> > --- a/drivers/tty/serial/8250/8250_mtk.c
> >> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> >> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >> >                            struct mtk8250_data *data)
> >> >  {
> >> >         int err;
> >> > -       struct device_node *np = pdev->dev.of_node;
> >> >
> >> > -       data->uart_clk = of_clk_get(np, 0);
> >> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >> >         if (IS_ERR(data->uart_clk)) {
> >> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> >> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >> >                 return PTR_ERR(data->uart_clk);
> >> >         }
> >> >
> >> >         err = clk_prepare_enable(data->uart_clk);
> >> >         if (err) {
> >> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> >> > -               clk_put(data->uart_clk);
> >> >                 return err;
> >> >         }
> >> >         p->uartclk = clk_get_rate(data->uart_clk);
> >> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >> >         serial8250_unregister_port(data->line);
> >> >         if (!IS_ERR(data->uart_clk)) {
> >> >                 clk_disable_unprepare(data->uart_clk);
> >> > -               clk_put(data->uart_clk);
> >> >         }
> >>
> >> Please delete the braces.
> >
> > The if() is removed completly in the next patch anyway.
> 
> You are right. I propose to merge patch 2 and 3, so you get a clean code.

Patches 1 and 2 are different topics, I see no reason merging them. I
changed the order of the two patches, maybe this makes it better
readable.

> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Changed to:

    When a struct device * is present clk_get should be used rather
    than of_clk_get. Use the devm variant of this function to be able to
    drop the clk_put in the error and remove pathes. While at it fix
    a wrong error message.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:53:08PM +0200, Matthias Brugger wrote:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> >> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> >> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> >> > which is the correct function when a struct device * is available. Also
> >> > since it's a devm function we can drop the calls to clk_put(). While at
> >> > it also fix a wrong debug message.
> >> >
> >> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >> > ---
> >> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> >> > index 7a11fac..c6901dc 100644
> >> > --- a/drivers/tty/serial/8250/8250_mtk.c
> >> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> >> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >> >                            struct mtk8250_data *data)
> >> >  {
> >> >         int err;
> >> > -       struct device_node *np = pdev->dev.of_node;
> >> >
> >> > -       data->uart_clk = of_clk_get(np, 0);
> >> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >> >         if (IS_ERR(data->uart_clk)) {
> >> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> >> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >> >                 return PTR_ERR(data->uart_clk);
> >> >         }
> >> >
> >> >         err = clk_prepare_enable(data->uart_clk);
> >> >         if (err) {
> >> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> >> > -               clk_put(data->uart_clk);
> >> >                 return err;
> >> >         }
> >> >         p->uartclk = clk_get_rate(data->uart_clk);
> >> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >> >         serial8250_unregister_port(data->line);
> >> >         if (!IS_ERR(data->uart_clk)) {
> >> >                 clk_disable_unprepare(data->uart_clk);
> >> > -               clk_put(data->uart_clk);
> >> >         }
> >>
> >> Please delete the braces.
> >
> > The if() is removed completly in the next patch anyway.
> 
> You are right. I propose to merge patch 2 and 3, so you get a clean code.

Patches 1 and 2 are different topics, I see no reason merging them. I
changed the order of the two patches, maybe this makes it better
readable.

> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Changed to:

    When a struct device * is present clk_get should be used rather
    than of_clk_get. Use the devm variant of this function to be able to
    drop the clk_put in the error and remove pathes. While at it fix
    a wrong error message.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/4] tty: serial: 8250_mtk: Use devm_clk_get

[Sascha Hauer]

On Fri, Apr 24, 2015 at 12:53:08PM +0200, Matthias Brugger wrote:
> 2015-04-24 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > On Fri, Apr 24, 2015 at 12:28:33PM +0200, Matthias Brugger wrote:
> >> 2015-04-23 10:51 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> >> > Just because of_clk_get() doesn't mean it should be used. Use devm_clk_get
> >> > which is the correct function when a struct device * is available. Also
> >> > since it's a devm function we can drop the calls to clk_put(). While at
> >> > it also fix a wrong debug message.
> >> >
> >> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >> > ---
> >> >  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
> >> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> >> > index 7a11fac..c6901dc 100644
> >> > --- a/drivers/tty/serial/8250/8250_mtk.c
> >> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> >> > @@ -131,18 +131,16 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
> >> >                            struct mtk8250_data *data)
> >> >  {
> >> >         int err;
> >> > -       struct device_node *np = pdev->dev.of_node;
> >> >
> >> > -       data->uart_clk = of_clk_get(np, 0);
> >> > +       data->uart_clk = devm_clk_get(&pdev->dev, NULL);
> >> >         if (IS_ERR(data->uart_clk)) {
> >> > -               dev_warn(&pdev->dev, "Can't get timer clock\n");
> >> > +               dev_warn(&pdev->dev, "Can't get uart clock\n");
> >> >                 return PTR_ERR(data->uart_clk);
> >> >         }
> >> >
> >> >         err = clk_prepare_enable(data->uart_clk);
> >> >         if (err) {
> >> >                 dev_warn(&pdev->dev, "Can't prepare clock\n");
> >> > -               clk_put(data->uart_clk);
> >> >                 return err;
> >> >         }
> >> >         p->uartclk = clk_get_rate(data->uart_clk);
> >> > @@ -216,7 +214,6 @@ static int mtk8250_remove(struct platform_device *pdev)
> >> >         serial8250_unregister_port(data->line);
> >> >         if (!IS_ERR(data->uart_clk)) {
> >> >                 clk_disable_unprepare(data->uart_clk);
> >> > -               clk_put(data->uart_clk);
> >> >         }
> >>
> >> Please delete the braces.
> >
> > The if() is removed completly in the next patch anyway.
> 
> You are right. I propose to merge patch 2 and 3, so you get a clean code.

Patches 1 and 2 are different topics, I see no reason merging them. I
changed the order of the two patches, maybe this makes it better
readable.

> On the way, can you have a look on the commit message? For me it is
> not clear what you want to say.

Changed to:

    When a struct device * is present clk_get should be used rather
    than of_clk_get. Use the devm variant of this function to be able to
    drop the clk_put in the error and remove pathes. While at it fix
    a wrong error message.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-05-05 17:50 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
>> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > The mtk 8250 needs two clocks, one for providing the baudrate and
>> > one that needs to be enabled for register accesses. The latter has
>> > not been supported, this patch adds support for it. It is optional
>> > for now since not all SoCs provide a bus clock.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>> >  2 files changed, 34 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > index 4415226..8d63f1d 100644
>> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > @@ -14,7 +14,14 @@ Required properties:
>> >
>> >  - interrupts: A single interrupt specifier.
>> >
>> > -- clocks: Clock driving the hardware.
>> > +- clocks : Must contain an entry for each entry in clock-names.
>> > +  See ../clocks/clock-bindings.txt for details.
>> > +- clock-names:
>> > +  - "baud": The clock the baudrate is derived from
>> > +  - "bus": The bus clock for register accesses (optional)
>> > +
>> > +For compatibility with older device trees an unnamed clock is used for the
>> > +baud clock if the baudclk does not exist. Do not use this for new designs.
>> >
>> >  Example:
>> >
>> > @@ -22,5 +29,6 @@ Example:
>> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>> >                 reg = <0x11006000 0x400>;
>> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
>> > -               clocks = <&uart_clk>;
>> > +               clocks = <&uart_clk>, <&bus_clk>;
>> > +               clock-names = "baud", "bus";
>> >         };
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 2f28bd0..8eb3876 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -34,6 +34,7 @@
>> >  struct mtk8250_data {
>> >         int                     line;
>> >         struct clk              *uart_clk;
>> > +       struct clk              *bus_clk;
>> >  };
>>
>> Not sure if it would be better to rename the uart_clk to baud_clk
>> which reflects the new naming scheme.
>
> Can do.

If you do so you can add:
Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

Thanks,
Matthias
-- 
motzblog.wordpress.com

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-05-05 17:50 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
>> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > The mtk 8250 needs two clocks, one for providing the baudrate and
>> > one that needs to be enabled for register accesses. The latter has
>> > not been supported, this patch adds support for it. It is optional
>> > for now since not all SoCs provide a bus clock.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>> >  2 files changed, 34 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > index 4415226..8d63f1d 100644
>> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > @@ -14,7 +14,14 @@ Required properties:
>> >
>> >  - interrupts: A single interrupt specifier.
>> >
>> > -- clocks: Clock driving the hardware.
>> > +- clocks : Must contain an entry for each entry in clock-names.
>> > +  See ../clocks/clock-bindings.txt for details.
>> > +- clock-names:
>> > +  - "baud": The clock the baudrate is derived from
>> > +  - "bus": The bus clock for register accesses (optional)
>> > +
>> > +For compatibility with older device trees an unnamed clock is used for the
>> > +baud clock if the baudclk does not exist. Do not use this for new designs.
>> >
>> >  Example:
>> >
>> > @@ -22,5 +29,6 @@ Example:
>> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>> >                 reg = <0x11006000 0x400>;
>> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
>> > -               clocks = <&uart_clk>;
>> > +               clocks = <&uart_clk>, <&bus_clk>;
>> > +               clock-names = "baud", "bus";
>> >         };
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 2f28bd0..8eb3876 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -34,6 +34,7 @@
>> >  struct mtk8250_data {
>> >         int                     line;
>> >         struct clk              *uart_clk;
>> > +       struct clk              *bus_clk;
>> >  };
>>
>> Not sure if it would be better to rename the uart_clk to baud_clk
>> which reflects the new naming scheme.
>
> Can do.

If you do so you can add:
Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

Thanks,
Matthias
-- 
motzblog.wordpress.com

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-05-05 17:50 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
>> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > The mtk 8250 needs two clocks, one for providing the baudrate and
>> > one that needs to be enabled for register accesses. The latter has
>> > not been supported, this patch adds support for it. It is optional
>> > for now since not all SoCs provide a bus clock.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>> >  2 files changed, 34 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > index 4415226..8d63f1d 100644
>> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
>> > @@ -14,7 +14,14 @@ Required properties:
>> >
>> >  - interrupts: A single interrupt specifier.
>> >
>> > -- clocks: Clock driving the hardware.
>> > +- clocks : Must contain an entry for each entry in clock-names.
>> > +  See ../clocks/clock-bindings.txt for details.
>> > +- clock-names:
>> > +  - "baud": The clock the baudrate is derived from
>> > +  - "bus": The bus clock for register accesses (optional)
>> > +
>> > +For compatibility with older device trees an unnamed clock is used for the
>> > +baud clock if the baudclk does not exist. Do not use this for new designs.
>> >
>> >  Example:
>> >
>> > @@ -22,5 +29,6 @@ Example:
>> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>> >                 reg = <0x11006000 0x400>;
>> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
>> > -               clocks = <&uart_clk>;
>> > +               clocks = <&uart_clk>, <&bus_clk>;
>> > +               clock-names = "baud", "bus";
>> >         };
>> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> > index 2f28bd0..8eb3876 100644
>> > --- a/drivers/tty/serial/8250/8250_mtk.c
>> > +++ b/drivers/tty/serial/8250/8250_mtk.c
>> > @@ -34,6 +34,7 @@
>> >  struct mtk8250_data {
>> >         int                     line;
>> >         struct clk              *uart_clk;
>> > +       struct clk              *bus_clk;
>> >  };
>>
>> Not sure if it would be better to rename the uart_clk to baud_clk
>> which reflects the new naming scheme.
>
> Can do.

If you do so you can add:
Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

Thanks,
Matthias
-- 
motzblog.wordpress.com

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > The mtk 8250 needs two clocks, one for providing the baudrate and
> > one that needs to be enabled for register accesses. The latter has
> > not been supported, this patch adds support for it. It is optional
> > for now since not all SoCs provide a bus clock.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > index 4415226..8d63f1d 100644
> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > @@ -14,7 +14,14 @@ Required properties:
> >
> >  - interrupts: A single interrupt specifier.
> >
> > -- clocks: Clock driving the hardware.
> > +- clocks : Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names:
> > +  - "baud": The clock the baudrate is derived from
> > +  - "bus": The bus clock for register accesses (optional)
> > +
> > +For compatibility with older device trees an unnamed clock is used for the
> > +baud clock if the baudclk does not exist. Do not use this for new designs.
> >
> >  Example:
> >
> > @@ -22,5 +29,6 @@ Example:
> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
> >                 reg = <0x11006000 0x400>;
> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> > -               clocks = <&uart_clk>;
> > +               clocks = <&uart_clk>, <&bus_clk>;
> > +               clock-names = "baud", "bus";
> >         };
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 2f28bd0..8eb3876 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -34,6 +34,7 @@
> >  struct mtk8250_data {
> >         int                     line;
> >         struct clk              *uart_clk;
> > +       struct clk              *bus_clk;
> >  };
> 
> Not sure if it would be better to rename the uart_clk to baud_clk
> which reflects the new naming scheme.

Can do.

> 
> >
> >  static void
> > @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
> >         struct mtk8250_data *data = dev_get_drvdata(dev);
> >
> >         clk_disable_unprepare(data->uart_clk);
> > +       clk_disable_unprepare(data->bus_clk);
> 
> bus_clk can be NULL.

The clk core handles NULL clks just fine. They are considered dummy
clks. You can prepare/enable them, clk_get_rate of course returns
nothing useful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> > The mtk 8250 needs two clocks, one for providing the baudrate and
> > one that needs to be enabled for register accesses. The latter has
> > not been supported, this patch adds support for it. It is optional
> > for now since not all SoCs provide a bus clock.
> >
> > Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > index 4415226..8d63f1d 100644
> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > @@ -14,7 +14,14 @@ Required properties:
> >
> >  - interrupts: A single interrupt specifier.
> >
> > -- clocks: Clock driving the hardware.
> > +- clocks : Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names:
> > +  - "baud": The clock the baudrate is derived from
> > +  - "bus": The bus clock for register accesses (optional)
> > +
> > +For compatibility with older device trees an unnamed clock is used for the
> > +baud clock if the baudclk does not exist. Do not use this for new designs.
> >
> >  Example:
> >
> > @@ -22,5 +29,6 @@ Example:
> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
> >                 reg = <0x11006000 0x400>;
> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> > -               clocks = <&uart_clk>;
> > +               clocks = <&uart_clk>, <&bus_clk>;
> > +               clock-names = "baud", "bus";
> >         };
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 2f28bd0..8eb3876 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -34,6 +34,7 @@
> >  struct mtk8250_data {
> >         int                     line;
> >         struct clk              *uart_clk;
> > +       struct clk              *bus_clk;
> >  };
> 
> Not sure if it would be better to rename the uart_clk to baud_clk
> which reflects the new naming scheme.

Can do.

> 
> >
> >  static void
> > @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
> >         struct mtk8250_data *data = dev_get_drvdata(dev);
> >
> >         clk_disable_unprepare(data->uart_clk);
> > +       clk_disable_unprepare(data->bus_clk);
> 
> bus_clk can be NULL.

The clk core handles NULL clks just fine. They are considered dummy
clks. You can prepare/enable them, clk_get_rate of course returns
nothing useful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

On Tue, May 05, 2015 at 05:41:16PM +0200, Matthias Brugger wrote:
> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > The mtk 8250 needs two clocks, one for providing the baudrate and
> > one that needs to be enabled for register accesses. The latter has
> > not been supported, this patch adds support for it. It is optional
> > for now since not all SoCs provide a bus clock.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
> >  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > index 4415226..8d63f1d 100644
> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > @@ -14,7 +14,14 @@ Required properties:
> >
> >  - interrupts: A single interrupt specifier.
> >
> > -- clocks: Clock driving the hardware.
> > +- clocks : Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names:
> > +  - "baud": The clock the baudrate is derived from
> > +  - "bus": The bus clock for register accesses (optional)
> > +
> > +For compatibility with older device trees an unnamed clock is used for the
> > +baud clock if the baudclk does not exist. Do not use this for new designs.
> >
> >  Example:
> >
> > @@ -22,5 +29,6 @@ Example:
> >                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
> >                 reg = <0x11006000 0x400>;
> >                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> > -               clocks = <&uart_clk>;
> > +               clocks = <&uart_clk>, <&bus_clk>;
> > +               clock-names = "baud", "bus";
> >         };
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index 2f28bd0..8eb3876 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -34,6 +34,7 @@
> >  struct mtk8250_data {
> >         int                     line;
> >         struct clk              *uart_clk;
> > +       struct clk              *bus_clk;
> >  };
> 
> Not sure if it would be better to rename the uart_clk to baud_clk
> which reflects the new naming scheme.

Can do.

> 
> >
> >  static void
> > @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
> >         struct mtk8250_data *data = dev_get_drvdata(dev);
> >
> >         clk_disable_unprepare(data->uart_clk);
> > +       clk_disable_unprepare(data->bus_clk);
> 
> bus_clk can be NULL.

The clk core handles NULL clks just fine. They are considered dummy
clks. You can prepare/enable them, clk_get_rate of course returns
nothing useful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..8d63f1d 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,14 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
> +
> +For compatibility with older device trees an unnamed clock is used for the
> +baud clock if the baudclk does not exist. Do not use this for new designs.
>
>  Example:
>
> @@ -22,5 +29,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..8eb3876 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };

Not sure if it would be better to rename the uart_clk to baud_clk
which reflects the new naming scheme.

>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);

bus_clk can be NULL.

>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +

Same here.

-- 
motzblog.wordpress.com

Re: [PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..8d63f1d 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,14 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
> +
> +For compatibility with older device trees an unnamed clock is used for the
> +baud clock if the baudclk does not exist. Do not use this for new designs.
>
>  Example:
>
> @@ -22,5 +29,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..8eb3876 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };

Not sure if it would be better to rename the uart_clk to baud_clk
which reflects the new naming scheme.

>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);

bus_clk can be NULL.

>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +

Same here.

-- 
motzblog.wordpress.com

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> The mtk 8250 needs two clocks, one for providing the baudrate and
> one that needs to be enabled for register accesses. The latter has
> not been supported, this patch adds support for it. It is optional
> for now since not all SoCs provide a bus clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
>  drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> index 4415226..8d63f1d 100644
> --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> @@ -14,7 +14,14 @@ Required properties:
>
>  - interrupts: A single interrupt specifier.
>
> -- clocks: Clock driving the hardware.
> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names:
> +  - "baud": The clock the baudrate is derived from
> +  - "bus": The bus clock for register accesses (optional)
> +
> +For compatibility with older device trees an unnamed clock is used for the
> +baud clock if the baudclk does not exist. Do not use this for new designs.
>
>  Example:
>
> @@ -22,5 +29,6 @@ Example:
>                 compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
>                 reg = <0x11006000 0x400>;
>                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -               clocks = <&uart_clk>;
> +               clocks = <&uart_clk>, <&bus_clk>;
> +               clock-names = "baud", "bus";
>         };
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 2f28bd0..8eb3876 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -34,6 +34,7 @@
>  struct mtk8250_data {
>         int                     line;
>         struct clk              *uart_clk;
> +       struct clk              *bus_clk;
>  };

Not sure if it would be better to rename the uart_clk to baud_clk
which reflects the new naming scheme.

>
>  static void
> @@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
>         struct mtk8250_data *data = dev_get_drvdata(dev);
>
>         clk_disable_unprepare(data->uart_clk);
> +       clk_disable_unprepare(data->bus_clk);

bus_clk can be NULL.

>
>         return 0;
>  }
> @@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
>                 return err;
>         }
>
> +       err = clk_prepare_enable(data->bus_clk);
> +       if (err) {
> +               dev_warn(dev, "Can't enable bus clock\n");
> +               return err;
> +       }
> +

Same here.

-- 
motzblog.wordpress.com

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

The mtk 8250 needs two clocks, one for providing the baudrate and
one that needs to be enabled for register accesses. The latter has
not been supported, this patch adds support for it. It is optional
for now since not all SoCs provide a bus clock.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
 drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index 4415226..8d63f1d 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -14,7 +14,14 @@ Required properties:
 
 - interrupts: A single interrupt specifier.
 
-- clocks: Clock driving the hardware.
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names:
+  - "baud": The clock the baudrate is derived from
+  - "bus": The bus clock for register accesses (optional)
+
+For compatibility with older device trees an unnamed clock is used for the
+baud clock if the baudclk does not exist. Do not use this for new designs.
 
 Example:
 
@@ -22,5 +29,6 @@ Example:
 		compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
 		reg = <0x11006000 0x400>;
 		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&uart_clk>;
+		clocks = <&uart_clk>, <&bus_clk>;
+		clock-names = "baud", "bus";
 	};
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 2f28bd0..8eb3876 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -34,6 +34,7 @@
 struct mtk8250_data {
 	int			line;
 	struct clk		*uart_clk;
+	struct clk		*bus_clk;
 };
 
 static void
@@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
 	clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->bus_clk);
 
 	return 0;
 }
@@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
 		return err;
 	}
 
+	err = clk_prepare_enable(data->bus_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable bus clock\n");
+		return err;
+	}
+
 	return 0;
 }
 
@@ -153,13 +161,24 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+	data->uart_clk = devm_clk_get(&pdev->dev, "baud");
 	if (IS_ERR(data->uart_clk)) {
-		dev_warn(&pdev->dev, "Can't get uart clock\n");
-		return PTR_ERR(data->uart_clk);
+		/*
+		 * For compatibility with older device trees try unnamed
+		 * clk when no baud clk can be found.
+		 */
+		data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(data->uart_clk)) {
+			dev_warn(&pdev->dev, "Can't get uart clock\n");
+			return PTR_ERR(data->uart_clk);
+		}
+
+		return 0;
 	}
 
-	p->uartclk = clk_get_rate(data->uart_clk);
+	data->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(data->bus_clk))
+		return PTR_ERR(data->bus_clk);
 
 	return 0;
 }
@@ -204,6 +223,7 @@ static int mtk8250_probe(struct platform_device *pdev)
 	uart.port.regshift = 2;
 	uart.port.private_data = data;
 	uart.port.set_termios = mtk8250_set_termios;
+	uart.port.uartclk = clk_get_rate(data->uart_clk);
 
 	/* Disable Rate Fix function */
 	writel(0x0, uart.port.membase +
-- 
2.1.4

[PATCH 4/4] tty: serial: 8250_mtk: Add support for bus clock

[Sascha Hauer]

The mtk 8250 needs two clocks, one for providing the baudrate and
one that needs to be enabled for register accesses. The latter has
not been supported, this patch adds support for it. It is optional
for now since not all SoCs provide a bus clock.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/serial/mtk-uart.txt        | 12 ++++++++--
 drivers/tty/serial/8250/8250_mtk.c                 | 28 ++++++++++++++++++----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index 4415226..8d63f1d 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -14,7 +14,14 @@ Required properties:
 
 - interrupts: A single interrupt specifier.
 
-- clocks: Clock driving the hardware.
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names:
+  - "baud": The clock the baudrate is derived from
+  - "bus": The bus clock for register accesses (optional)
+
+For compatibility with older device trees an unnamed clock is used for the
+baud clock if the baudclk does not exist. Do not use this for new designs.
 
 Example:
 
@@ -22,5 +29,6 @@ Example:
 		compatible = "mediatek,mt6589-uart", "mediatek,mt6577-uart";
 		reg = <0x11006000 0x400>;
 		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&uart_clk>;
+		clocks = <&uart_clk>, <&bus_clk>;
+		clock-names = "baud", "bus";
 	};
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 2f28bd0..8eb3876 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -34,6 +34,7 @@
 struct mtk8250_data {
 	int			line;
 	struct clk		*uart_clk;
+	struct clk		*bus_clk;
 };
 
 static void
@@ -120,6 +121,7 @@ static int mtk8250_runtime_suspend(struct device *dev)
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 
 	clk_disable_unprepare(data->uart_clk);
+	clk_disable_unprepare(data->bus_clk);
 
 	return 0;
 }
@@ -135,6 +137,12 @@ static int mtk8250_runtime_resume(struct device *dev)
 		return err;
 	}
 
+	err = clk_prepare_enable(data->bus_clk);
+	if (err) {
+		dev_warn(dev, "Can't enable bus clock\n");
+		return err;
+	}
+
 	return 0;
 }
 
@@ -153,13 +161,24 @@ mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
 static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p,
 			   struct mtk8250_data *data)
 {
-	data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+	data->uart_clk = devm_clk_get(&pdev->dev, "baud");
 	if (IS_ERR(data->uart_clk)) {
-		dev_warn(&pdev->dev, "Can't get uart clock\n");
-		return PTR_ERR(data->uart_clk);
+		/*
+		 * For compatibility with older device trees try unnamed
+		 * clk when no baud clk can be found.
+		 */
+		data->uart_clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(data->uart_clk)) {
+			dev_warn(&pdev->dev, "Can't get uart clock\n");
+			return PTR_ERR(data->uart_clk);
+		}
+
+		return 0;
 	}
 
-	p->uartclk = clk_get_rate(data->uart_clk);
+	data->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(data->bus_clk))
+		return PTR_ERR(data->bus_clk);
 
 	return 0;
 }
@@ -204,6 +223,7 @@ static int mtk8250_probe(struct platform_device *pdev)
 	uart.port.regshift = 2;
 	uart.port.private_data = data;
 	uart.port.set_termios = mtk8250_set_termios;
+	uart.port.uartclk = clk_get_rate(data->uart_clk);
 
 	/* Disable Rate Fix function */
 	writel(0x0, uart.port.membase +
-- 
2.1.4