From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() on init failure Date: Thu, 04 Jun 2026 11:52:28 +1000 Message-ID: In-Reply-To: <20260603-fix_i915-v1-3-7479ff64e705@bootlin.com> References: <20260603-fix_i915-v1-0-7479ff64e705@bootlin.com> <20260603-fix_i915-v1-3-7479ff64e705@bootlin.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Verdict: Correct.** The analysis is clear: 1. When `intel_edp_init_connector()` fails, the code calls `intel_dp_aux_fi= ni(intel_dp)` then jumps to `fail:`. 2. The `fail:` label at line 7467 calls `drm_connector_cleanup()`. The conn= ector was initialized with `intel_dp_connector_funcs` which has `.destroy` = =E2=80=94 but `drm_connector_cleanup()` does NOT call `.destroy`, it just r= emoves from the list and cleans up state. 3. However, the *encoder* will still have its `.destroy` callback called du= ring device teardown =E2=80=94 either `intel_dp_encoder_destroy` or `intel_= ddi_encoder_destroy`, both of which call `intel_dp_encoder_flush_work()`, w= hich calls `intel_dp_aux_fini()` at line 6811. 4. So `intel_dp_aux_fini()` would be called twice: once explicitly in the e= rror path, and once via the encoder destroy callback. The fix correctly removes the explicit call, keeping only the one in the en= coder destroy path. The `intel_dp_aux_fini()` function unregisters the aux = I2C adapter, and calling it twice would indeed be problematic (double unreg= ister/double kfree of adapter name). One minor note: the patch also changes the code style from a `{ }` block to= a bare `if` statement with the `goto`, which is fine since the single stat= ement no longer needs braces. **Clean and correct fix.** --- Generated by Claude Code Patch Reviewer