Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Missing SRCU read lock release in error path of sn65dsi83_atomic_enable(). Pre-existing issues: - [High] Missing SRCU read lock release in error path of sn65dsi83_reset_work(). -- --- Patch [2]: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking --- commit 0c69248e520fff24e51d45bfab961991fbdb4267 Author: Esben Haabendal <[email protected]> drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking This patch moves the PLL locking sequence from atomic_pre_enable to atomic_enable. This ensures the DSI HS clock is running before attempting to lock the PLL. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > index 32e9eadd7452f..e87bed3a6640b 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c [ ... ] > @@ -712,11 +697,24 @@ static void sn65dsi83_atomic_enable(struct drm_bridge > *bridge, > { > struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); > unsigned int pval; > - int idx; > + int idx, ret; > > if (!drm_bridge_enter(bridge, &idx)) > return; > > + /* Enable PLL */ > + regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN); > + usleep_range(3000, 4000); > + ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval, > + pval & REG_RC_LVDS_PLL_PLL_EN_STAT, > + 1000, 100000); > + if (ret) { > + dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret); > + /* On failure, disable PLL again and exit. */ > + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00); > + return; [Severity: High] Does this error path leak the SRCU read lock acquired by drm_bridge_enter()? If the PLL lock fails and regmap_read_poll_timeout() returns an error, the function returns immediately without calling drm_bridge_exit(idx). This unreleased lock could cause subsequent calls to synchronize_srcu() to hang during module unload or bridge unplug. > + } > + > /* Clear all errors that got asserted during initialization. */ > regmap_read(ctx->regmap, REG_IRQ_STAT, &pval); [Severity: High] This is a pre-existing issue, but I noticed a similar pattern in another function in this file. Does sn65dsi83_reset_work() also leak the SRCU read lock on failure? drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() { if (!drm_bridge_enter(&ctx->bridge, &idx)) return; ... ret = sn65dsi83_reset_pipe(ctx); if (ret) { dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret)); return; } ... drm_bridge_exit(idx); } Like the atomic_enable path, if sn65dsi83_reset_pipe() fails, it appears to return early without calling drm_bridge_exit(idx). -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
