Re: [PATCH] serial: vt8500: Use devm_clk_get_enabled()

[Alexey Charkov]

Hi Wentao,

On Sun, Nov 8, 2026 at 4:15 PM Wentao Liang <vulab@xxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxx>
> 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.

Thanks for the patch, but you missed the sign-off line, without which
it cannot be taken any further. The concern raised by Sashiko in its
reply is also valid. The whole bitmap is quite an eye sore frankly, so
it should probably just go anyway.

Let me take a stab at these - I'll put you in Cc.

Best regards,
Alexey