[PATCH v2] tty: serial: 8250_mtk: Add support for second clock

[PATCH v2] tty: serial: 8250_mtk: Add support for second clock

[Sascha Hauer]

The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:

  Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)

are available in the git repository at:

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

for you to fetch changes up to 02e3f747bea47f6c02dc3f52e40869ea8ce5342a:

  tty: serial: 8250_mtk: Add support for bus clock (2015-04-27 08:39:06 +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.

Changes since v1:
- keep support old binding with unnamed clk
- hopefully clarify commit message
- do not leak in if() with unnecessary braces.

----------------------------------------------------------------
Sascha Hauer (4):
      tty: serial: 8250_mtk: remove unnecessary test
      tty: serial: 8250_mtk: Use devm_clk_get
      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        |  12 ++-
 drivers/tty/serial/8250/8250_mtk.c                 | 104 ++++++++++++---------
 2 files changed, 70 insertions(+), 46 deletions(-)

[PATCH 1/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 | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 7a11fac..1297827 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -214,10 +214,8 @@ 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_put(data->uart_clk);
-	}
+	clk_disable_unprepare(data->uart_clk);
+	clk_put(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -249,8 +247,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;
 }
@@ -259,8 +256,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 1/4] tty: serial: 8250_mtk: remove unnecessary test

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> 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 | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>

Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index 7a11fac..1297827 100644
> --- a/drivers/tty/serial/8250/8250_mtk.c
> +++ b/drivers/tty/serial/8250/8250_mtk.c
> @@ -214,10 +214,8 @@ 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_put(data->uart_clk);
> -       }
> +       clk_disable_unprepare(data->uart_clk);
> +       clk_put(data->uart_clk);
>
>         pm_runtime_disable(&pdev->dev);
>         pm_runtime_put_noidle(&pdev->dev);
> @@ -249,8 +247,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;
>  }
> @@ -259,8 +256,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
>



-- 
motzblog.wordpress.com

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

[Sascha Hauer]

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.

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 1297827..bcfaa8dc 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);
@@ -215,7 +213,6 @@ static int mtk8250_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 	clk_disable_unprepare(data->uart_clk);
-	clk_put(data->uart_clk);
 
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
-- 
2.1.4

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

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> 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.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/tty/serial/8250/8250_mtk.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

[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

[Matthias Brugger]

2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> 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;

Why do you delete pm_runtime_set_active here?

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

[Sascha Hauer]

On Mon, May 04, 2015 at 01:05:27PM +0200, Matthias Brugger wrote:
> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > 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;
> 
> Why do you delete pm_runtime_set_active here?

pm_runtime_set_active() communicates the initial state of the device to
the pm core. Previously the driver activated the device manually by
directly calling clk_prepare_enable(). Since this manual enabling of
the clock is removed in this patch, the device is no longer active,
hence pm_runtime_set_active() has to be removed.

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 3/4] tty: serial: 8250_mtk: use pm_runtime callbacks for enabling

[Matthias Brugger]

2015-05-04 14:06 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Mon, May 04, 2015 at 01:05:27PM +0200, Matthias Brugger wrote:
>> 2015-04-27 8:49 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > 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;
>>
>> Why do you delete pm_runtime_set_active here?
>
> pm_runtime_set_active() communicates the initial state of the device to
> the pm core. Previously the driver activated the device manually by
> directly calling clk_prepare_enable(). Since this manual enabling of
> the clock is removed in this patch, the device is no longer active,
> hence pm_runtime_set_active() has to be removed.

Sounds resonable.
Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

-- 
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

[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]

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 |

[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