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/xe: Send 'none' recovery method for XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET Date: Wed, 04 Mar 2026 07:31:28 +1000 Message-ID: In-Reply-To: <20260303110556.3577612-4-raag.jadav@intel.com> References: <20260303110556.3577612-1-raag.jadav@intel.com> <20260303110556.3577612-4-raag.jadav@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review ```c + if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET) + xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_NONE); /* If no wedge recovery method is set, use default */ - if (!xe->wedged.method) + else if (!xe->wedged.method) xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET); ``` **Issue 1: Unconditional override of pre-set method.** The new code unconditionally sets `DRM_WEDGE_RECOVERY_NONE` when the mode is `XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET`, regardless of whether a caller had already set `xe->wedged.method` via `xe_device_set_wedged_method()`. The old `if (!xe->wedged.method)` pattern respected pre-set methods. If `NONE` should always be used in this mode, this is fine but should be documented in the comment. If a caller could have legitimate reasons to set a different method even in NO_RESET mode, this would silently override it. **Issue 2: Semantic mismatch with `WEDGED=none`.** As discussed in the overall review, `WEDGED=none` was designed for "device recovered, no action needed." Using it for "device permanently wedged, don't bother recovering" is a different semantic. In this mode, the device *does* need recovery (rebind) to become functional again - it's just that the intent is to leave it broken for debugging. A userspace daemon listening for wedged events and seeing `WEDGED=none` would reasonably conclude the device is fine, when it's actually non-functional. **Suggestion:** Consider whether this debug mode should simply not send a uevent at all (since it's a debug-only configuration), or use a different signaling mechanism. Alternatively, if the uevent is needed for telemetry collection triggers, the documentation needs to be much clearer about the dual meaning. --- Generated by Claude Code Patch Reviewer