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

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

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/20260717085716.1619275-1-johan@kernel.org/
>> >
>> >> 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: stable@vger.kernel.org
>> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> >
>> > Reviewed-by: Johan Hovold <johan@kernel.org>
>>
>> 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