From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Introduce error threshold to drm_ras Date: Fri, 05 Jun 2026 06:10:07 +1000 Message-ID: In-Reply-To: <20260604184849.1011985-1-raag.jadav@intel.com> References: <20260604184849.1011985-1-raag.jadav@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: Introduce error threshold to drm_ras Author: Raag Jadav Patches: 5 Reviewed: 2026-06-05T06:10:07.157843 --- This 4-patch series adds error threshold get/set support to the DRM RAS net= link infrastructure (patch 1), adds system controller mailbox-based error c= ounter support for the Xe driver (patch 2), adds threshold get/set via the = system controller mailbox (patch 3), and wires the threshold callbacks into= the Xe DRM RAS nodes (patch 4). The architecture is clean and follows the existing patterns well. The core = drm_ras threshold code mirrors the existing counter code closely. However, = there are several issues worth addressing: **Major concerns:** - Patch 4 only wires up threshold callbacks for correctable errors, not unc= orrectable =E2=80=94 this is asymmetric and unexplained. - The `get_counter` response includes a `threshold` field but patch 2 never= uses it, and patch 3 introduces a separate `GET_THRESHOLD` command =E2=80= =94 the relationship between these two is unclear. - The `get_counter` function in patch 2 doesn't check `response.status` unl= ike `clear_counter` and `set_threshold`, which is inconsistent. **Minor concerns:** - Mixed initialization styles (`=3D {0}` in patch 2 vs `=3D {}` in patch 3). - Patch 2 is marked "Do not review, CI only" but it's a dependency for the = rest of the series and contains reviewable code that introduces new UAPI. --- --- Generated by Claude Code Patch Reviewer