All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: sifive: enable clocks for UART when probed
@ 2022-09-19 22:39 ` Olof Johansson
  0 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2022-09-19 22:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-riscv, linux-kernel,
	Olof Johansson, Uwe Kleine-König, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley

When the PWM driver was changed to disable clocks if no PWMs are enabled,
it ended up also disabling the shared parent with the UART, since the
UART doesn't do any clock enablement on its own.

To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
calls.

Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Olof Johansson <olof@lixom.net>
---
 drivers/tty/serial/sifive.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..751f98068806 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "unable to find controller clock\n");
 		return PTR_ERR(clk);
 	}
+	clk_prepare_enable(clk);
 
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0) {
 		dev_err(&pdev->dev, "missing aliases entry\n");
-		return id;
+		r = id;
+		goto probe_out1;
 	}
 
 #ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
 	if (id > SIFIVE_SERIAL_MAX_PORTS) {
 		dev_err(&pdev->dev, "too many UARTs (%d)\n", id);
-		return -EINVAL;
+		r = -EINVAL;
+		goto probe_out1;
 	}
 #endif
 
 	ssp = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
-	if (!ssp)
-		return -ENOMEM;
+	if (!ssp) {
+		r = -ENOMEM;
+		goto probe_out1;
+	}
 
 	ssp->port.dev = &pdev->dev;
 	ssp->port.type = PORT_SIFIVE_V0;
@@ -1028,6 +1033,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
 probe_out2:
 	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
 probe_out1:
+	clk_disable_unprepare(clk);
 	return r;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] serial: sifive: enable clocks for UART when probed
@ 2022-09-19 22:39 ` Olof Johansson
  0 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2022-09-19 22:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-riscv, linux-kernel,
	Olof Johansson, Uwe Kleine-König, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley

When the PWM driver was changed to disable clocks if no PWMs are enabled,
it ended up also disabling the shared parent with the UART, since the
UART doesn't do any clock enablement on its own.

To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
calls.

Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Olof Johansson <olof@lixom.net>
---
 drivers/tty/serial/sifive.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..751f98068806 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "unable to find controller clock\n");
 		return PTR_ERR(clk);
 	}
+	clk_prepare_enable(clk);
 
 	id = of_alias_get_id(pdev->dev.of_node, "serial");
 	if (id < 0) {
 		dev_err(&pdev->dev, "missing aliases entry\n");
-		return id;
+		r = id;
+		goto probe_out1;
 	}
 
 #ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
 	if (id > SIFIVE_SERIAL_MAX_PORTS) {
 		dev_err(&pdev->dev, "too many UARTs (%d)\n", id);
-		return -EINVAL;
+		r = -EINVAL;
+		goto probe_out1;
 	}
 #endif
 
 	ssp = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
-	if (!ssp)
-		return -ENOMEM;
+	if (!ssp) {
+		r = -ENOMEM;
+		goto probe_out1;
+	}
 
 	ssp->port.dev = &pdev->dev;
 	ssp->port.type = PORT_SIFIVE_V0;
@@ -1028,6 +1033,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
 probe_out2:
 	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
 probe_out1:
+	clk_disable_unprepare(clk);
 	return r;
 }
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sifive: enable clocks for UART when probed
  2022-09-19 22:39 ` Olof Johansson
@ 2022-09-20  6:14   ` Uwe Kleine-König
  -1 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2022-09-20  6:14 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-riscv,
	linux-kernel, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley

[-- Attachment #1: Type: text/plain, Size: 2150 bytes --]

Hello,

On Mon, Sep 19, 2022 at 03:39:15PM -0700, Olof Johansson wrote:
> When the PWM driver was changed to disable clocks if no PWMs are enabled,
> it ended up also disabling the shared parent with the UART, since the
> UART doesn't do any clock enablement on its own.
> 
> To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
> calls.
> 
> Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  drivers/tty/serial/sifive.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> index 5c3a07546a58..751f98068806 100644
> --- a/drivers/tty/serial/sifive.c
> +++ b/drivers/tty/serial/sifive.c
> @@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "unable to find controller clock\n");
>  		return PTR_ERR(clk);
>  	}
> +	clk_prepare_enable(clk);

The return code of clk_prepare_enable() must be checked. Also there is a
simpler patch to fix that problem. I didn't test but I expect just doing 

diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..4b1d4fe8458e 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -945,7 +945,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 	}
 
-	clk = devm_clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "unable to find controller clock\n");
 		return PTR_ERR(clk);

would be enough and also cares for disabling the clock on remove.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sifive: enable clocks for UART when probed
@ 2022-09-20  6:14   ` Uwe Kleine-König
  0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2022-09-20  6:14 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-riscv,
	linux-kernel, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley


[-- Attachment #1.1: Type: text/plain, Size: 2150 bytes --]

Hello,

On Mon, Sep 19, 2022 at 03:39:15PM -0700, Olof Johansson wrote:
> When the PWM driver was changed to disable clocks if no PWMs are enabled,
> it ended up also disabling the shared parent with the UART, since the
> UART doesn't do any clock enablement on its own.
> 
> To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
> calls.
> 
> Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  drivers/tty/serial/sifive.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> index 5c3a07546a58..751f98068806 100644
> --- a/drivers/tty/serial/sifive.c
> +++ b/drivers/tty/serial/sifive.c
> @@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "unable to find controller clock\n");
>  		return PTR_ERR(clk);
>  	}
> +	clk_prepare_enable(clk);

The return code of clk_prepare_enable() must be checked. Also there is a
simpler patch to fix that problem. I didn't test but I expect just doing 

diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..4b1d4fe8458e 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -945,7 +945,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 	}
 
-	clk = devm_clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "unable to find controller clock\n");
 		return PTR_ERR(clk);

would be enough and also cares for disabling the clock on remove.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sifive: enable clocks for UART when probed
  2022-09-20  6:14   ` Uwe Kleine-König
@ 2022-09-20 16:01     ` Olof Johansson
  -1 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2022-09-20 16:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-riscv,
	linux-kernel, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley

On Mon, Sep 19, 2022 at 11:14 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> On Mon, Sep 19, 2022 at 03:39:15PM -0700, Olof Johansson wrote:
> > When the PWM driver was changed to disable clocks if no PWMs are enabled,
> > it ended up also disabling the shared parent with the UART, since the
> > UART doesn't do any clock enablement on its own.
> >
> > To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
> > calls.
> >
> > Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
> > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> > ---
> >  drivers/tty/serial/sifive.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> > index 5c3a07546a58..751f98068806 100644
> > --- a/drivers/tty/serial/sifive.c
> > +++ b/drivers/tty/serial/sifive.c
> > @@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
> >               dev_err(&pdev->dev, "unable to find controller clock\n");
> >               return PTR_ERR(clk);
> >       }
> > +     clk_prepare_enable(clk);
>
> The return code of clk_prepare_enable() must be checked. Also there is a
> simpler patch to fix that problem. I didn't test but I expect just doing
>
> diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> index 5c3a07546a58..4b1d4fe8458e 100644
> --- a/drivers/tty/serial/sifive.c
> +++ b/drivers/tty/serial/sifive.c
> @@ -945,7 +945,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
>                 return PTR_ERR(base);
>         }
>
> -       clk = devm_clk_get(&pdev->dev, NULL);
> +       clk = devm_clk_get_enabled(&pdev->dev, NULL);
>         if (IS_ERR(clk)) {
>                 dev_err(&pdev->dev, "unable to find controller clock\n");
>                 return PTR_ERR(clk);
>
> would be enough and also cares for disabling the clock on remove.

Yep, thanks for pointing that out. v2 sent out (after test).


-Olof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sifive: enable clocks for UART when probed
@ 2022-09-20 16:01     ` Olof Johansson
  0 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2022-09-20 16:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-riscv,
	linux-kernel, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley

On Mon, Sep 19, 2022 at 11:14 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> On Mon, Sep 19, 2022 at 03:39:15PM -0700, Olof Johansson wrote:
> > When the PWM driver was changed to disable clocks if no PWMs are enabled,
> > it ended up also disabling the shared parent with the UART, since the
> > UART doesn't do any clock enablement on its own.
> >
> > To avoid these surprises, add clk_prepare_enable/clk_disable_unprepare
> > calls.
> >
> > Fixes: ace41d7564e655 ("pwm: sifive: Ensure the clk is enabled exactly once per running PWM")
> > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> > ---
> >  drivers/tty/serial/sifive.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> > index 5c3a07546a58..751f98068806 100644
> > --- a/drivers/tty/serial/sifive.c
> > +++ b/drivers/tty/serial/sifive.c
> > @@ -950,23 +950,28 @@ static int sifive_serial_probe(struct platform_device *pdev)
> >               dev_err(&pdev->dev, "unable to find controller clock\n");
> >               return PTR_ERR(clk);
> >       }
> > +     clk_prepare_enable(clk);
>
> The return code of clk_prepare_enable() must be checked. Also there is a
> simpler patch to fix that problem. I didn't test but I expect just doing
>
> diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> index 5c3a07546a58..4b1d4fe8458e 100644
> --- a/drivers/tty/serial/sifive.c
> +++ b/drivers/tty/serial/sifive.c
> @@ -945,7 +945,7 @@ static int sifive_serial_probe(struct platform_device *pdev)
>                 return PTR_ERR(base);
>         }
>
> -       clk = devm_clk_get(&pdev->dev, NULL);
> +       clk = devm_clk_get_enabled(&pdev->dev, NULL);
>         if (IS_ERR(clk)) {
>                 dev_err(&pdev->dev, "unable to find controller clock\n");
>                 return PTR_ERR(clk);
>
> would be enough and also cares for disabling the clock on remove.

Yep, thanks for pointing that out. v2 sent out (after test).


-Olof

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-20 16:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 22:39 [PATCH] serial: sifive: enable clocks for UART when probed Olof Johansson
2022-09-19 22:39 ` Olof Johansson
2022-09-20  6:14 ` Uwe Kleine-König
2022-09-20  6:14   ` Uwe Kleine-König
2022-09-20 16:01   ` Olof Johansson
2022-09-20 16:01     ` Olof Johansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.