* [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe()
@ 2026-09-16 7:06 Wentao Liang
2026-09-16 7:17 ` Alexey Charkov
0 siblings, 1 reply; 4+ messages in thread
From: Wentao Liang @ 2026-09-16 7:06 UTC (permalink / raw)
To: alchark
Cc: andi.shyti, krzk, linux-arm-kernel, linux-i2c, linux-kernel,
linux, wsa, Wentao Liang, stable
In wmt_i2c_probe(), i2c->clk is acquired using of_clk_get(). If any
subsequent operation fails or when the module is unloaded, the clock
reference count is never decremented via clk_put(), leading to a
resource leak.
Switch to devm_clk_get() so that the clock is automatically managed
and released.
Fixes: 560746eb79d3 ("i2c: vt8500: Add support for I2C bus on Wondermedia SoCs")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/i2c/busses/i2c-viai2c-wmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-viai2c-wmt.c b/drivers/i2c/busses/i2c-viai2c-wmt.c
index 2cf3cc0165fb..74ca93b44347 100644
--- a/drivers/i2c/busses/i2c-viai2c-wmt.c
+++ b/drivers/i2c/busses/i2c-viai2c-wmt.c
@@ -117,7 +117,7 @@ static int wmt_i2c_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, err,
"failed to request irq %i\n", i2c->irq);
- i2c->clk = of_clk_get(np, 0);
+ i2c->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(i2c->clk))
return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
"unable to request clock\n");
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe()
2026-09-16 7:06 [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe() Wentao Liang
@ 2026-09-16 7:17 ` Alexey Charkov
2026-10-10 14:34 ` Andi Shyti
2026-11-05 3:08 ` Wentao Liang
0 siblings, 2 replies; 4+ messages in thread
From: Alexey Charkov @ 2026-09-16 7:17 UTC (permalink / raw)
To: Wentao Liang
Cc: andi.shyti, krzk, linux-arm-kernel, linux-i2c, linux-kernel,
linux, wsa, stable
On Wed, Sep 16, 2026 at 11:07 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> In wmt_i2c_probe(), i2c->clk is acquired using of_clk_get(). If any
> subsequent operation fails or when the module is unloaded, the clock
> reference count is never decremented via clk_put(), leading to a
> resource leak.
>
> Switch to devm_clk_get() so that the clock is automatically managed
> and released.
>
> Fixes: 560746eb79d3 ("i2c: vt8500: Add support for I2C bus on Wondermedia SoCs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/i2c/busses/i2c-viai2c-wmt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-viai2c-wmt.c b/drivers/i2c/busses/i2c-viai2c-wmt.c
> index 2cf3cc0165fb..74ca93b44347 100644
> --- a/drivers/i2c/busses/i2c-viai2c-wmt.c
> +++ b/drivers/i2c/busses/i2c-viai2c-wmt.c
> @@ -117,7 +117,7 @@ static int wmt_i2c_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, err,
> "failed to request irq %i\n", i2c->irq);
>
> - i2c->clk = of_clk_get(np, 0);
> + i2c->clk = devm_clk_get(&pdev->dev, NULL);
Maybe we could go for devm_clk_get_enabled and drop some more
boilerplate which follows in wmt_i2c_reset_hardware, while at it?
Backporting this to stable seems like a stretch to me, though (it's
quite old code, so there would be quite a lot of backporting for a
rarely hit resource leak).
Best regards,
Alexey
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe()
2026-09-16 7:17 ` Alexey Charkov
@ 2026-10-10 14:34 ` Andi Shyti
2026-11-05 3:08 ` Wentao Liang
1 sibling, 0 replies; 4+ messages in thread
From: Andi Shyti @ 2026-10-10 14:34 UTC (permalink / raw)
To: Alexey Charkov
Cc: Wentao Liang, krzk, linux-arm-kernel, linux-i2c, linux-kernel,
linux, wsa, stable
Hi Wentao,
On Wed, Sep 16, 2026 at 11:17:49AM +0400, Alexey Charkov wrote:
> On Wed, Sep 16, 2026 at 11:07 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
> >
> > In wmt_i2c_probe(), i2c->clk is acquired using of_clk_get(). If any
> > subsequent operation fails or when the module is unloaded, the clock
> > reference count is never decremented via clk_put(), leading to a
> > resource leak.
> >
> > Switch to devm_clk_get() so that the clock is automatically managed
> > and released.
> >
> > Fixes: 560746eb79d3 ("i2c: vt8500: Add support for I2C bus on Wondermedia SoCs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
this is 4/20 but where are the rest of the patches?
> > ---
> > drivers/i2c/busses/i2c-viai2c-wmt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-viai2c-wmt.c b/drivers/i2c/busses/i2c-viai2c-wmt.c
> > index 2cf3cc0165fb..74ca93b44347 100644
> > --- a/drivers/i2c/busses/i2c-viai2c-wmt.c
> > +++ b/drivers/i2c/busses/i2c-viai2c-wmt.c
> > @@ -117,7 +117,7 @@ static int wmt_i2c_probe(struct platform_device *pdev)
> > return dev_err_probe(&pdev->dev, err,
> > "failed to request irq %i\n", i2c->irq);
> >
> > - i2c->clk = of_clk_get(np, 0);
> > + i2c->clk = devm_clk_get(&pdev->dev, NULL);
>
> Maybe we could go for devm_clk_get_enabled and drop some more
> boilerplate which follows in wmt_i2c_reset_hardware, while at it?
>
> Backporting this to stable seems like a stretch to me, though (it's
> quite old code, so there would be quite a lot of backporting for a
> rarely hit resource leak).
Alexey asked a question. Wouldn't it be good to follow up?
Unfortunately, this is becoming quite common with LLM generated
patches, where questions are often left unanswered, perhaps
because the patches are not fully understood by committers.
I think the patch is correct, though, and Alexey's suggestions
can be addressed after this is merged.
With some disappointment, I'm pushing this patch to
i2c/i2c-fixes-2.
Andi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe()
2026-09-16 7:17 ` Alexey Charkov
2026-10-10 14:34 ` Andi Shyti
@ 2026-11-05 3:08 ` Wentao Liang
1 sibling, 0 replies; 4+ messages in thread
From: Wentao Liang @ 2026-11-05 3:08 UTC (permalink / raw)
To: Alexey Charkov, Andi Shyti
Cc: krzk, linux-arm-kernel, linux-i2c, linux-kernel, linux, wsa,
wentao_liang_g
Hi Alexey, Andi,
Sorry for leaving Alexey's review unanswered, and thank you for reviewing
and merging the fix. I am following up from wentao_liang_g@163.com because
I can no longer send mail from my original address, vulab@iscas.ac.cn.
> Maybe we could go for devm_clk_get_enabled and drop some more
> boilerplate which follows in wmt_i2c_reset_hardware, while at it?
Yes, that would allow a broader cleanup. The current reset helper calls
clk_prepare_enable() and disables the clock if clk_set_rate() fails.
There are also explicit clk_disable_unprepare() calls on adapter
registration failure and in remove(). Converting to
devm_clk_get_enabled() would need to update all of these together to
avoid double-disabling the clock and account for the resulting cleanup
ordering. The merged devm_clk_get() change only manages the clock
reference; it preserves the existing explicit enable/disable handling.
I agree that the broader cleanup belongs in a separate follow-up rather
than a v2 of the fix that has already been merged.
> Backporting this to stable seems like a stretch to me, though
I understand your concern about the backporting effort for a rarely
triggered leak in old code. The original stable Cc was for the missing
clk_put() on probe failure and driver removal, but I should have
addressed your concern at the time. This reply is not a request for any
additional backporting or for the broader cleanup to go to stable.
Andi, the 04/20 numbering came from a batch of independent fixes across
multiple subsystems, sent to their respective maintainers and lists.
This fix has no dependency on the other nineteen patches. That numbering
was confusing here; future independent fixes should be sent separately.
I used OpenAI Codex to help inspect the existing clock cleanup paths and
draft this reply. No new patch or hardware test results are submitted
with this message.
Thank you, and sorry again for the missed follow-up.
Best regards,
Wentao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-05 3:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 7:06 [PATCH 04/20] i2c: viai2c-wmt: Fix clock leak in wmt_i2c_probe() Wentao Liang
2026-09-16 7:17 ` Alexey Charkov
2026-10-10 14:34 ` Andi Shyti
2026-11-05 3:08 ` Wentao Liang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®