Re: [PATCH 01/26] drm/bridge: add drm_of_find_bridge()

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

Hi Louis,

On Wed Nov 19, 2025 at 3:22 PM CET, Louis Chauvet wrote:
>
>
> On 11/19/25 13:05, Luca Ceresoli wrote:
>> of_drm_find_bridge() does not increment the refcount for the returned
>> bridge, but that is required now. However converting it and all its users
>> is not realistically doable at once given the large amount of (direct and
>> indirect) callers and the complexity of some. Also, "of_drm_find_bridge is
>> oddly named according to our convention and it would make more sense to be
>> called drm_of_find_bridge()" (quoted from Link: below).
>>
>> Solve both issues by creating a new drm_of_find_bridge() that is identical
>> to of_drm_find_bridge() except it takes a reference. Then
>> of_drm_find_bridge() will be deprecated to be eventually removed.
>>
>> Suggested-by: Maxime Ripard <mripard@kernel.org>
>> Link: https://lore.kernel.org/dri-devel/20250319-stylish-lime-mongoose-0a18ad@houat/
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

...

>> +struct drm_bridge *drm_of_find_bridge(struct device_node *np)
>> +{
>> +	struct drm_bridge *bridge;
>> +
>> +	mutex_lock(&bridge_lock);
>> +
>> +	list_for_each_entry(bridge, &bridge_list, list) {
>> +		if (bridge->of_node == np) {
>> +			mutex_unlock(&bridge_lock);
>
> It seems a bit strange to unlock the mutex just before the
> drm_bridge_get, is it expected?

Ouch. No, it's not expected, it is a very silly mistake. Thanks for
noticing.

> If no, I think you can use scoped_guard(mutex, &bridge_lock) to avoid
> messing with mutex_unlock, IIRC, scoped_guard will unlock the mutex just
> after the return, so in your case, just after the drm_bridge_get.
>
>> +			return drm_bridge_get(bridge);
>> +		}
>> +	}

My intent was to keep the function as similar as possible to the original
one, thus I just added a drm_bridge_get(), but that is of course wrong.

So these lines should instead have been:

	if (bridge->of_node == np) {
		drm_bridge_get(bridge);
		mutex_unlock(&bridge_lock);
		return bridge;
	}

But indeed scoped_guard() is much cleaner and less error-prone, so I'm
probably going to use it in v2.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com