Re: [PATCH v15 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/bridge: lt9611c: incorrect interrupt clear logic toggles unhandled bits
- [Medium] drm/bridge: lt9611c: missing delay after reset deassertion on suspend error path
--

commit 1c8b096eca4758947d0ce62950c84436cc31f839
Author: Sunyun Yang <syyang@lontium.com>

drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

This commit introduces the driver for the Lontium LT9611C(EX/UXD) DSI to
HDMI bridge, supporting conversion of single or dual MIPI DSI to HDMI.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611c.c b/drivers/gpu/drm/bridge/lontium-lt9611c.c
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611c.c
[ ... ]
> @@ -758,26 +758,26 @@ static irqreturn_t lt9611c_irq_thread_handler(int irq, void *dev_id)
>  	if (!(irq_status & BIT(0)))
>  		return IRQ_NONE;
>  
>  	/* Clear interrupt: hardware requires two writes with delay */
>  	regmap_write(lt9611c->regmap, 0xe0df, irq_status & BIT(0));
>  	usleep_range(10000, 12000);
>  	regmap_write(lt9611c->regmap, 0xe0df, irq_status & (~BIT(0)));

[Severity: High]
Can this second write unintentionally clear or toggle other pending
interrupts?

When clearing BIT(0), writing irq_status & (~BIT(0)) to the register
asserts the mask for any other bits that were active in irq_status
during the read. This might leave them in a partially toggled state or
inadvertently clear them without handling their events, potentially
dropping important hardware interrupts like HPD or EDID status.

>  
>  	schedule_work(&lt9611c->work);
>  
>  	return IRQ_HANDLED;
>  }
[ ... ]
> @@ -1193,17 +1193,17 @@ static int lt9611c_bridge_suspend(struct device *dev)
>  	disable_irq(lt9611c->client->irq);
>  	cancel_work_sync(&lt9611c->work);
>  
>  	gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
>  
>  	ret = regulator_bulk_disable(ARRAY_SIZE(lt9611c->supplies), lt9611c->supplies);
>  	if (ret) {
>  		dev_err(lt9611c->dev, "regulator bulk disable failed.\n");
>  		gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
>  		enable_irq(lt9611c->client->irq);
>  		return ret;
>  	}

[Severity: Medium]
Is a delay required here before re-enabling the interrupt?

In lt9611c_reset(), the driver uses a 400ms msleep after deasserting the
reset GPIO. Re-enabling the IRQ immediately on this suspend error recovery
path might interact with an uninitialized MCU, which could trigger I2C
timeouts or IRQ storms while the system resumes.

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-lt9611c-v7-v15-0-88f9e9387eab@oss.qualcomm.com?part=2