Re: [PATCH v3 3/3] drm/bridge: megachips: remove bridge when irq request fails

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

On Thu Apr 23, 2026 at 10:06 PM CEST, Osama Abdelkader wrote:
> If devm_request_threaded_irq() fails after drm_bridge_add(), remove the
> bridge before returning.
>
> Keep drm_bridge_add() rather than devm_drm_bridge_add(): registration is
> tied to the STDP4028 device while ge_b850v3_register() may complete from
> either I2C probe; devm would not unwind the bridge if the other client's
> probe fails.
>
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> Fixes: a68ee76f4a28 ("drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Fix bridge initialization")

That commit only moved the bug to a slightly different location. The bug
was present even before, since commit fcfa0ddc18ed ("drm/bridge: Drivers
for megachips-stdpxxxx-ge-b850v3-fw (LVDS-DP++)"), so you should update
your Fixes line to point to it.


> Cc: stable@vger.kernel.org
> ---
> v3: add Fixes and Cc tags
> v2: IRQ failure path only (explicit drm_bridge_remove)
> ---
>  .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> index c9e6505cbd88..2d02cc69f237 100644
> --- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> +++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> @@ -251,7 +251,6 @@ static void ge_b850v3_lvds_remove(void)
>  		goto out;
>
>  	drm_bridge_remove(&ge_b850v3_lvds_ptr->bridge);
> -
>  	ge_b850v3_lvds_ptr = NULL;
>  out:
>  	mutex_unlock(&ge_b850v3_lvds_dev_mutex);
> @@ -261,6 +260,7 @@ static int ge_b850v3_register(void)
>  {
>  	struct i2c_client *stdp4028_i2c = ge_b850v3_lvds_ptr->stdp4028_i2c;
>  	struct device *dev = &stdp4028_i2c->dev;
> +	int ret;
>
>  	/* drm bridge initialization */
>  	ge_b850v3_lvds_ptr->bridge.ops = DRM_BRIDGE_OP_DETECT |
> @@ -277,11 +277,15 @@ static int ge_b850v3_register(void)
>  	if (!stdp4028_i2c->irq)
>  		return 0;
>
> -	return devm_request_threaded_irq(&stdp4028_i2c->dev,
> -			stdp4028_i2c->irq, NULL,
> -			ge_b850v3_lvds_irq_handler,
> -			IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> -			"ge-b850v3-lvds-dp", ge_b850v3_lvds_ptr);
> +	ret = devm_request_threaded_irq(&stdp4028_i2c->dev,
> +					stdp4028_i2c->irq, NULL,
> +					ge_b850v3_lvds_irq_handler,
> +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> +					"ge-b850v3-lvds-dp", ge_b850v3_lvds_ptr);
> +	if (ret)
> +		drm_bridge_remove(&ge_b850v3_lvds_ptr->bridge);

Why not just using devm_drm_bridge_add() and keep everything else clean, as
you did in other patches in the series?

Luca

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