Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The timeout recovery path fails to explicitly clear the latched `AUX_REPLY_EVENT` interrupt status, leaving a race window that corrupts the next AUX transfer. -- --- Patch [6]: [PATCH v11 06/21] drm/bridge: synopsys: dw-dp: Fix AUX transfer timeout race condition --- commit f3d806ec9da65b4fec77558418a758675e7524f1 Author: Sebastian Reichel <[email protected]> drm/bridge: synopsys: dw-dp: Fix AUX transfer timeout race condition This commit attempts to fix a race condition during DP AUX transfer timeouts by re-initializing the completion before sending the command, resetting the AUX controller, and synchronizing pending interrupts. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c > b/drivers/gpu/drm/bridge/synopsys/dw-dp.c > index b1856418f4419..2573ea2bb5744 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c [ ... ] > @@ -1492,6 +1494,12 @@ static ssize_t dw_dp_aux_transfer(struct drm_dp_aux > *aux, > status = wait_for_completion_timeout(&dp->complete, timeout); > if (!status) { > dev_err(dp->dev, "timeout waiting for AUX reply\n"); > + regmap_update_bits(dp->regmap, DW_DP_SOFT_RESET_CTRL, > + AUX_RESET, FIELD_PREP(AUX_RESET, 1)); > + usleep_range(10, 20); > + regmap_update_bits(dp->regmap, DW_DP_SOFT_RESET_CTRL, > + AUX_RESET, FIELD_PREP(AUX_RESET, 0)); > + synchronize_irq(dp->irq); [Severity: High] This is a pre-existing issue, but does this timeout recovery path fail to explicitly clear the latched AUX_REPLY_EVENT in the DW_DP_GENERAL_INTERRUPT register? Even with the newly added AUX_RESET and synchronize_irq(), if the hardware finishes concurrently with the timeout, it could latch the AUX_REPLY_EVENT in the top-level W1C register. Since the reset only targets the AUX sub-module, the latched interrupt status might remain uncleared. If a delayed spurious interrupt is latched but the threaded handler hasn't started (or the irqchip doesn't support pending state queries), could synchronize_irq() return without clearing it? If so, the caller could retry the transfer and call reinit_completion(), after which the delayed interrupt finally runs, sees the uncleared AUX_REPLY_EVENT, and calls complete() prematurely for the next transfer. Should the top-level interrupt status be explicitly cleared here before returning to fully close this race window? > return -ETIMEDOUT; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v11-0-0d508505f...@collabora.com?part=6
