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 <[email protected]>
>> Link:
>> https://lore.kernel.org/dri-devel/20250319-stylish-lime-mongoose-0a18ad@houat/
>> Signed-off-by: Luca Ceresoli <[email protected]>
...
>> +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
