All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wentao Liang <vulab@iscas.ac.cn>
To: Alexey Charkov <alchark@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux@prisktech.co.nz, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: [PATCH] serial: vt8500: Use devm_clk_get_enabled()
Date: Sun, 08 Nov 2026 12:15:30 +0000	[thread overview]
Message-ID: <1794138380.2883200.vulab@iscas.ac.cn> (raw)
In-Reply-To: <CABjd4Yy-9A0xWCDfZemHOTZDrDdtdO05nUjmN=w=sLwYjzZn8Q@mail.gmail.com>

Hi Alexey,

Sorry for the very late reply. Working through the review comments I got
in September has taken me far longer than it should have.

Your suggestion was right, but the leak fix went in before I could act on
it: it is commit 9d8955281d92 ("serial: vt8500: Fix clock reference leak
in vt8500_serial_probe()") in mainline, and it kept the clk_put() you
asked me to replace. It has been backported to the stable trees by now as
well. So the patch below is not a v2 of that one, which would only resend
something that is already applied, but a follow-up that does the
conversion on top of it.

devm_clk_get_enabled() gets the clock and enables it in one call and
registers the disable and put with devm, so both the manual error handling
and the clk_put() go away. Probe now returns the error code from the clock
lookup instead of a hard-coded -EINVAL, and the clock is released
automatically through devres. The success path is unchanged and nothing
else in the driver is touched.

For the record on where this came from: the candidate came from our static
analysis tooling, and DeepSeek and GPT-6.1 Sol were used to audit it and to
help write the change. I reviewed the result and I am responsible for it.
The patch is build-tested only: it compiles cleanly with W=1 for arm
(vt8500_v6_v7_defconfig, CONFIG_SERIAL_VT8500=y) and passes
checkpatch.pl --strict; I have no VT8500 hardware to test it on.

Thanks for the review,
Wentao

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Wentao Liang <vulab@iscas.ac.cn>
Date: Sun, 8 Nov 2026 11:41:30 +0000
Subject: [PATCH] serial: vt8500: Use devm_clk_get_enabled()

vt8500_serial_probe() gets the clock with of_clk_get() and enables it
with clk_prepare_enable(), so the reference has to be dropped by hand on
the error path, and the clock is never disabled or released afterwards,
even though the rest of the probe is devm-managed.

Use devm_clk_get_enabled() instead, which gets and enables the clock in
one call and registers the teardown with devm. The manual error handling
and the clk_put() can then go, and probe returns the actual error code
from the clock lookup rather than a hard-coded -EINVAL.

The clock still stays enabled as long as the port is in use, so the
success path is unchanged.
---
 drivers/tty/serial/vt8500_serial.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index fc6fdc751b16..73922523dff6 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -603,7 +603,6 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	struct resource *mmres;
 	struct device_node *np = pdev->dev.of_node;
 	const unsigned int *flags;
-	int ret;
 	int port;
 	int irq;
 
@@ -647,17 +646,10 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	if (IS_ERR(vt8500_port->uart.membase))
 		return PTR_ERR(vt8500_port->uart.membase);
 
-	vt8500_port->clk = of_clk_get(pdev->dev.of_node, 0);
+	vt8500_port->clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(vt8500_port->clk)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
-		return  -EINVAL;
-	}
-
-	ret = clk_prepare_enable(vt8500_port->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
-		clk_put(vt8500_port->clk);
-		return ret;
+		return PTR_ERR(vt8500_port->clk);
 	}
 
 	vt8500_port->vt8500_uart_flags = *flags;
-- 
2.34.1


  reply	other threads:[~2026-11-08 12:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 15:58 [PATCH] serial: vt8500: Fix clock reference leak in vt8500_serial_probe() Wentao Liang
2026-09-17 16:03 ` sashiko-bot
2026-09-18  7:51 ` Alexey Charkov
2026-11-08 12:15   ` Wentao Liang [this message]
2026-11-08 12:27     ` [PATCH] serial: vt8500: Use devm_clk_get_enabled() sashiko-bot
2026-11-11 16:16     ` Alexey Charkov

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=1794138380.2883200.vulab@iscas.ac.cn \
    --to=vulab@iscas.ac.cn \
    --cc=alchark@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@prisktech.co.nz \
    /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.