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

[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

[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

[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

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

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

[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