From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/ci: add sc7180-trogdor-wormdingler-rev1-boe
Date: Fri, 27 Feb 2026 15:32:25 +1000
Message-ID: <review-patch1-20260224081036.220820-1-vignesh.raman@collabora.com>
In-Reply-To: <20260224081036.220820-1-vignesh.raman@collabora.com>
References: <20260224081036.220820-1-vignesh.raman@collabora.com> <20260224081036.220820-1-vignesh.raman@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

**Bug: Suspend skip pattern is commented out**

In `msm-sc7180-trogdor-wormdingler-rev1-boe-skips.txt`:
```
# Suspend to RAM seems to be broken on this machine
#.*suspend.*
```

The `#` prefix on `.*suspend.*` makes it a comment, so suspend tests will **not** be skipped despite the preceding comment saying suspend is broken. Compare with the equivalent in `msm-sc7180-trogdor-kingoftown-skips.txt` where the pattern is active:
```
# Suspend to RAM seems to be broken on this machine
.*suspend.*
```

This should be `.*suspend.*` (without the leading `#`), otherwise broken suspend tests will run and likely cause CI failures or hangs.

**Questionable: `kms_invalid_mode@overflow-vrefresh` in fails vs skips**

This test is listed in the wormdingler **fails** file:
```
kms_invalid_mode@overflow-vrefresh,Fail
```

But in the kingoftown **skips** file, it is explicitly skipped with the explanation:
```
# It causes other tests to fail, so skip it.
kms_invalid_mode@overflow-vrefresh
```

Since both devices share the same sc7180 SoC, if this test causes other tests to fail on kingoftown, it likely does the same on wormdingler. Having it as an expected failure rather than a skip means it will still run and potentially cause cascading failures in other tests, reducing overall CI coverage. It should probably be moved to the skips file with the same rationale.

**Minor: Missing kingoftown skips that may also apply**

The kingoftown skips file includes two entries not present in wormdingler:
```
# Timeout occurs
kms_flip@2x-wf_vblank-ts-check

# Hangs the machine
kms_cursor_crc@cursor-random-max-size
```

Since these are sc7180-trogdor siblings sharing the same GPU, it's worth confirming these tests don't also timeout/hang on wormdingler. If the wormdingler test results show these pass cleanly, then their absence is correct; if not, they should be added.

**test.yml job definition looks correct**

The job follows the established pattern: extends `.msm-sc7180`, sets `DEVICE_TYPE`, `DTB`, `GPU_VERSION`, and `RUNNER_TAG`. The `parallel: 6` matches kingoftown. No issues here.

**arm64.config and build.sh additions look correct**

- `CONFIG_DRM_PANEL_BOE_TV101WUM_NL6=y` is appropriate for the BOE panel on this board variant.
- `CONFIG_GPIO_SHARED_PROXY=y` is a valid Kconfig symbol (exists in `drivers/gpio/Kconfig`).
- The DTB path in `build.sh` matches the existing DTS file in `arch/arm64/boot/dts/qcom/`.

---
Generated by Claude Code Patch Reviewer