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/scdc-helper: Add scdc_status debugfs entry Date: Thu, 28 May 2026 12:16:28 +1000 Message-ID: In-Reply-To: <20260527-scdc-link-health-v4-2-622ea40a1f59@collabora.com> References: <20260527-scdc-link-health-v4-0-622ea40a1f59@collabora.com> <20260527-scdc-link-health-v4-2-622ea40a1f59@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Verdict: Minor issues.** This is the bulk of the series. The overall architecture is clean: `drm_scdc_read_state` as the core state reader, separate `drm_scdc_read_status0_flags` and `drm_scdc_read_error_counters` for individual fields, and `scdc_status_show` as the debugfs presentation layer. **1. Doc comment typo** in `drm_scdc_state`: ```c /** @scramling_enabled: true if TMDS scrambling is on */ ``` Should be `@scrambling_enabled`. **2. Exported functions with no external callers yet**: `drm_scdc_read_status0_flags`, `drm_scdc_read_error_counters`, and `drm_scdc_read_state` are all `EXPORT_SYMBOL` and declared in the public header, but currently only used internally by `scdc_status_show`. If no driver is expected to call these directly soon, they could be `static` to reduce API surface. However, if the intent is to provide a public API for drivers that want to query SCDC state programmatically, then exporting proactively is fine -- just worth stating the intent. **3. Status flag clear-before-read ordering**: `drm_scdc_read_status0_flags` writes `SCDC_STATUS_UPDATE` to clear the flag *before* reading the status register. The HDMI spec recommends reading the data first, then clearing the flag. For a debugfs polling interface this is fine in practice, but worth noting the deviation. **4. Connector get/put pattern is correct**: The success path drops the ref after `drm_scdc_read_state` returns (before printing cached state), and the error path uses `err_conn_put`. Clean. --- Generated by Claude Code Patch Reviewer