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 <[email protected]> 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(<9611c->work); > > return IRQ_HANDLED; > } [ ... ] > @@ -1193,17 +1193,17 @@ static int lt9611c_bridge_suspend(struct device *dev) > disable_irq(lt9611c->client->irq); > cancel_work_sync(<9611c->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/[email protected]?part=2
