All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Malathi A <malathi.a2000@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2] serial: 8250_uniphier: Use devm_clk_get_enabled()
Date: Thu, 17 Sep 2026 09:42:21 +0300	[thread overview]
Message-ID: <aquLzbM25C8dDb0p@ashevche-desk.local> (raw)
In-Reply-To: <20260917040511.103232-1-malathi.a2000@gmail.com>

On Thu, Sep 17, 2026 at 04:05:09AM +0000, Malathi A wrote:
> uniphier_uart_probe() enables priv->clk by hand and is then responsible
> for disabling it again on every error path. It misses one: when
> uart_read_port_properties() fails the function returns directly, leaving
> the clock prepared and enabled. The neighbouring path, taken when
> serial8250_register_8250_port() fails, does call clk_disable_unprepare(),
> which shows the leak is an oversight rather than intent.
> 
> Rather than adding another manual unwind, switch to
> devm_clk_get_enabled() so the clock is released by devres. That removes
> the error path entirely, along with the explicit disables in the
> register failure path and in uniphier_uart_remove().
> 
> The clock is still disabled and re-enabled by hand across system sleep,
> which stays balanced: only SET_SYSTEM_SLEEP_PM_OPS is used, so the
> device cannot be unbound while suspended and devres always sees an
> enabled clock at detach.
> 
> Found by smatch:
> 
>   drivers/tty/serial/8250/8250_uniphier.c:232 uniphier_uart_probe() warn: 'priv->clk' from clk_prepare_enable() not released on lines: 205.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> -	priv->clk = devm_clk_get(dev, NULL);
> +	priv->clk = devm_clk_get_enabled(dev, NULL);
>  	if (IS_ERR(priv->clk)) {
> -		dev_err(dev, "failed to get clock\n");
> +		dev_err(dev, "failed to get and enable clock\n");
>  		return PTR_ERR(priv->clk);

Consider fixing another latent bug here, id est with intermittent probe defer
this will spam the kernel dmesg a lot, so converting to use dev_err_probe()
will fix the issue (but at the same time you may convert all of the error
messages in probe to use return dev_err_probe() pattern). This needs to be
a separate change.

>  	}

-- 
With Best Regards,
Andy Shevchenko



  parent reply	other threads:[~2026-09-17  6:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  4:05 [PATCH v2] serial: 8250_uniphier: Use devm_clk_get_enabled() Malathi A
2026-09-17  4:11 ` sashiko-bot
2026-09-17  6:42 ` Andy Shevchenko [this message]
2026-09-24  9:07 ` Kunihiko Hayashi
2026-09-24 14:09   ` Andy Shevchenko
2026-09-25 11:32     ` Kunihiko Hayashi
2026-09-25 13:37       ` Andy Shevchenko
2026-09-28  6:51         ` Kunihiko Hayashi
2026-09-28  9:05           ` Andy Shevchenko
2026-10-05  6:55             ` Kunihiko Hayashi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aquLzbM25C8dDb0p@ashevche-desk.local \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=malathi.a2000@gmail.com \
    --cc=mhiramat@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.