Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/msm/dp: mixing goto and scoped_guard in msm_dp_display_atomic_enable() --
commit 5389d76b8ea91b24082be0a6254cd0c780518e0d Author: Xilin Wu <[email protected]> drm/msm/dp: Report stream enable failures through link status This commit updates the driver to mark the connector link status as bad from a work item after unwinding a failed enable. It sends a single hotplug notification per failure episode, allowing fbdev to retry the modeset synchronously while preventing unbounded retry loops. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index b076d44b5118c..816f2dc11aea1 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c [ ... ] > @@ -1583,6 +1633,11 @@ void msm_dp_display_atomic_enable(struct msm_dp > *msm_dp_display, > if (msm_dp_display->is_edp) > msm_dp_hpd_unplug_handle(dp); > msm_dp_display_unprepare(dp); > + scoped_guard(mutex, &dp->plugged_lock) { [Severity: Low] This isn't a functional bug, but does it mix goto-based error handling and scope-based cleanup in the same function? The function msm_dp_display_atomic_enable() uses goto-based error handling with the err and err_prepare labels to manage its cleanup sequence. The kernel guidelines in include/linux/cleanup.h advise against mixing goto-based cleanup and guard declarations within the same routine, as it can create confusing ownership semantics. Could this be refactored to use standard mutex_lock() and mutex_unlock() to maintain structural consistency? > + dp->link_failed = true; > + /* Keep reporting BAD, but notify only once until recovery. */ > + schedule_work(&dp->link_status_work); > + } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
