Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

"Luca Ceresoli" <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-devicetree,org.kernel.vger.stable
Message-ID <[email protected]>
Hi Laurent,

On Fri Jul 17, 2026 at 8:28 PM CEST, Laurent Pinchart wrote:
> On Fri, Jul 17, 2026 at 05:21:11PM +0200, Luca Ceresoli wrote:
>> On Fri Jul 17, 2026 at 5:03 PM CEST, Johan Hovold wrote:
>> > On Mon, Jul 06, 2026 at 12:35:40AM +0300, Laurent Pinchart wrote:
>> >> If the probe function returns an error after getting the I2C adapter for
>> >> DDC, the reference to the adapter is never released. Fix it by releasing
>> >> it in the bridge .destroy() handler.
>> >>
>> >> There is no need to test the ddc pointer with !IS_ERR(), as
>> >> of_get_i2c_adapter_by_node() returns NULL on error.
>> >
>> > I stumbled over this this morning as well and posted a fix here (which
>> > releases the adapter on driver unbind as is currently done):
>> >
>> > 	https://lore.kernel.org/lkml/[email protected]/
>> >
>> >> Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
>> >
>> > I believe this issue was first introduced by commit 2e2bf3a5584d
>> > ("drm/bridge: display-connector: add DP support") a few releases
>> > earlier.
>> >
>> >> Cc: [email protected]
>> >> Signed-off-by: Laurent Pinchart <[email protected]>
>> >
>> > Reviewed-by: Johan Hovold <[email protected]>
>>
>> While Johan's patch has the benefit of putting the i2c adapter at remove
>> time (as opposed to destroy time, which is potentially a long time later),
>> it is also more complex. So I'd say this patch is fine.
>
> In this case putting the I2C adapter early is probably fine, but in
> general I really prefer reference-counting and releasing references at
> destroy time. This ensures that the resources stay available if they
> need to be accessed between .remove() and destruction (for instance in a
> .release() handler following a close() or munmap() from userspace). I
> think late release should be the default, as it's safer, and early
> release should be carefully reviewed when it's required.

Makes sense indeed.

Thanks for the clarification.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.