Re: [PATCH v11 06/21] drm/bridge: synopsys: dw-dp: Fix AUX transfer timeout race condition

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-0d508505f383@collabora.com?part=6