Re: [PATCH] drm/bridge: samsung-dsim: fix TE GPIO ownership

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 05.06.2026 09:51, Alexander Stein wrote:
> Am Donnerstag, 4. Juni 2026, 13:46:04 CEST schrieb Guangshuo Li:
>> samsung_dsim_register_te_irq() acquires the TE GPIO with
>> devm_gpiod_get_optional(), but the descriptor is released manually in the
>> request_threaded_irq() error path and in samsung_dsim_unregister_te_irq().
>> The devres entry remains registered and will release the same descriptor
>> again when the device is detached.
>>
>> Use the non-managed gpiod_get_optional() helper instead, so the GPIO
>> descriptor lifetime matches the existing manual cleanup paths. Also clear
>> dsi->te_gpio after each gpiod_put() to avoid leaving a stale descriptor
>> pointer around if the host is attached again.
> Shouldn't you use devm_gpiod_put instead of gpiod_put?

Not really. The dev pointer here is not the device that the $subject driver binds to, so using devm_* interface is a bit fragile. Similar issue was fixed long time ago by commit fedc89821990 ("drm/exynos: Search for TE-gpio in DSI panel's node"). I would prefer explicitly use non-devm api for it.


For the initial patch:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

>> Fixes: e7447128ca4a ("drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge")
>> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
>> ---
>>  drivers/gpu/drm/bridge/samsung-dsim.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index c3eb437ef1b0..2299b7dc0c04 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1862,7 +1862,7 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
>>  	int te_gpio_irq;
>>  	int ret;
>>  
>> -	dsi->te_gpio = devm_gpiod_get_optional(dev, "te", GPIOD_IN);
>> +	dsi->te_gpio = gpiod_get_optional(dev, "te", GPIOD_IN);
>>  	if (!dsi->te_gpio)
>>  		return 0;
>>  	else if (IS_ERR(dsi->te_gpio))
>> @@ -1875,6 +1875,7 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
>>  	if (ret) {
>>  		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
>>  		gpiod_put(dsi->te_gpio);
>> +		dsi->te_gpio = NULL;
>>  		return ret;
>>  	}
>>  
>> @@ -1886,6 +1887,7 @@ static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi)
>>  	if (dsi->te_gpio) {
>>  		free_irq(gpiod_to_irq(dsi->te_gpio), dsi);
>>  		gpiod_put(dsi->te_gpio);
>> +		dsi->te_gpio = NULL;
>>  	}
>>  }
>>  
>>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland