Claude review: drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This patch extends for HDMI 2.1's FRL mode, adding lane 3 support, FRL rate, FFE levels, and status flags 1/2. Several bugs here:

**1. BUG: Lane 3 error counter missing `SCDC_CHANNEL_VALID` check**

```c
if (num_lanes == 4)
    counter[3] = buf[ERR_DET_OFF(SCDC_ERR_DET_3_L)] |
                (buf[ERR_DET_OFF(SCDC_ERR_DET_3_H)] & ~SCDC_CHANNEL_VALID) << 8;
else
    counter[3] = 0;
```

Channels 0–2 check `if (buf[i + 1] & SCDC_CHANNEL_VALID)` before using the counter value, setting it to 0 if invalid. Lane 3 unconditionally uses the value. This should mirror the existing pattern:

```c
if (num_lanes == 4) {
    if (buf[ERR_DET_OFF(SCDC_ERR_DET_3_H)] & SCDC_CHANNEL_VALID)
        counter[3] = buf[ERR_DET_OFF(SCDC_ERR_DET_3_L)] |
                    (buf[ERR_DET_OFF(SCDC_ERR_DET_3_H)] & ~SCDC_CHANNEL_VALID) << 8;
    else
        counter[3] = 0;
}
```

**2. BUG: Lane 3 error counters not cleared on write-back**

After reading, the loop zeroes `buf[0..5]` (channels 0–2) and `buf[6]` (checksum), but `buf[7]` and `buf[8]` (lane 3) are left with their original read values. The subsequent write-back:

```c
return drm_scdc_write(connector->ddc, SCDC_ERR_DET_0_L, buf, buf_sz);
```

writes those un-cleared values back, so lane 3 counters are never reset. Add:

```c
buf[ERR_DET_OFF(SCDC_ERR_DET_3_L)] = 0;
buf[ERR_DET_OFF(SCDC_ERR_DET_3_H)] = 0;
```

before the write-back (and after extracting the counter value).

**3. Checksum scope for 4-lane case — verify against HDMI 2.1 spec**

```c
for (i = 0; i < buf_sz; i++)
    sum = wrapping_add(u8, sum, buf[i]);
```

For `buf_sz = 9` (4 lanes), this sums registers 0x50–0x58 including the lane 3 bytes. The HDMI 2.0 checksum at register 0x56 was designed to make bytes 0x50–0x56 sum to zero. If the HDMI 2.1 spec extends the checksum to cover 0x50–0x58, this is correct. If it doesn't, this will produce false `EPROTO` failures whenever lane 3 has non-zero error counts. Please verify this matches the HDMI 2.1 spec — the code should have a comment citing the relevant spec section.

**4. `drm_scdc_frl_rate_str` returns NULL in default case**

```c
default:
    return NULL;
```

This value is passed to `seq_printf` via `scdc_print_str` with `%s`. While the default case should be unreachable (all 4-bit values are covered by the enum), returning NULL for `%s` is a latent kernel-oops-in-waiting. Return `"(Unknown)"` instead, or add a `WARN_ON` and return a string.

**5. `drm_scdc_read_status1_2_flags` — same clear-before-read pattern**

```c
ret = drm_scdc_writeb(connector->ddc, SCDC_UPDATE_0, SCDC_FLT_UPDATE);
```

Same concern as patch 1's `drm_scdc_read_status0_flags` — the FLT_UPDATE flag is cleared before reading STATUS_FLAGS_1/2. For debugfs, fine; as a public API, document the behavior.

**6. Minor: `__pure` on `static inline` function**

```c
static inline __pure int drm_scdc_num_frl_lanes(enum drm_scdc_frl_rate rate)
```

`__pure` on a `static inline` is redundant — the compiler already has full visibility of the function body. It's harmless but unnecessary.

---

**Summary**: The series needs fixes for the lane 3 error counter bugs (missing valid check, missing clear-on-writeback), removal of the inappropriate comment, and ideally a spec citation for the 4-lane checksum scope. The bitfield portability concern should at least be acknowledged. The overall feature is useful and the design is sound.

---
Generated by Claude Code Patch Reviewer