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: arm64: dts: qcom: Add Glymur-based Lenovo Yoga Slim 7x Gen11
Date: Fri, 05 Jun 2026 06:39:42 +1000
Message-ID: <review-patch3-20260604-topic-yoga_submission-v1-3-57c70c23d0d6@oss.qualcomm.com>
In-Reply-To: <20260604-topic-yoga_submission-v1-3-57c70c23d0d6@oss.qualcomm.com>
References: <20260604-topic-yoga_submission-v1-0-57c70c23d0d6@oss.qualcomm.com> <20260604-topic-yoga_submission-v1-3-57c70c23d0d6@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is the main patch -- a 1237-line DTS for the laptop. Overall it is well-written and covers a lot of hardware. A few issues to flag:

**1. Orphaned `pcie3b_default` pinctrl state (dead code)**

The `&tlmm` section defines `pcie3b_default` (GPIOs 155-157) but no `&pcie3b` node exists in this DTS that references it. In the CRD, `&pcie3b` uses this for a second NVMe slot. If this laptop doesn't have a second NVMe, this should be removed:

```
+	pcie3b_default: pcie3b-default-state {
+		clkreq-n-pins {
+			pins = "gpio156";
+			function = "pcie3b_clk";
+			...
```

**2. Orphaned `nvme_sec_reg_en` pinctrl state (dead code)**

Similarly, `nvme_sec_reg_en` is defined in `&pmh0110_f_e1_gpios` but never referenced by any regulator or device node in this DTS:

```
+&pmh0110_f_e1_gpios {
+	nvme_sec_reg_en: nvme-reg-en-state {
+		pins = "gpio14";
+		function = "normal";
+		bias-disable;
+	};
+};
```

This pairs with `pcie3b_default` -- both are CRD second-NVMe leftovers.

**3. Orphaned `ec_int_n_default` pinctrl state (dead code)**

The `ec_int_n_default` pinctrl state is defined but not referenced by any node:

```
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
```

The comment `/* EC @ 0x70, irq = TLMM 66 */` in `&i2c9` explains why the pin is reserved, but the pinctrl state itself is unused. Either remove it or add a comment noting it's reserved for future EC driver support.

**4. `key_vol_up_default` pinctrl referenced but no volume-up key defined**

The gpio-keys node references `key_vol_up_default` in its pinctrl but only defines a lid switch -- no volume-up key:

```
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&key_vol_up_default>, <&hall_int_n_default>;
+		pinctrl-names = "default";
+
+		switch-lid {
```

Either add the volume-up key child node (the CRD has one on `&pmh0101_gpios 6`), or remove `<&key_vol_up_default>` from pinctrl-0 and drop the `key_vol_up_default` definition from the second `&pmh0101_gpios` block. This looks like a copy-paste oversight from the CRD.

**5. `&i2c5` enabled with no child devices**

```
+&i2c5 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+};
```

Enabling an I2C bus with no devices attached is unusual. The CRD has PTN3222 redrivers on i2c5. If those aren't present on this board, the node should probably not be enabled, to avoid needlessly probing an empty bus.

**6. `&mdss_dp3` uses `/delete-property/ #sound-dai-cells` -- why?**

```
+&mdss_dp3 {
+	/delete-property/ #sound-dai-cells;
```

This suggests the base dtsi defines `#sound-dai-cells` for DP3 (for DP audio), but the Yoga explicitly removes it. A comment explaining why would help -- presumably because the eDP panel doesn't carry audio?

**7. Minor: Duplicate `&pmh0101_gpios` blocks**

Two separate `&pmh0101_gpios` blocks exist. DT overlays merge them, so this is functionally correct. However, per the CRD which follows the same pattern, this is acceptable -- though combining them into one block would be cleaner.

**Overall for Patch 3: Good first submission. The hardware coverage is comprehensive. Clean up the dead pinctrl/regulator definitions and fix the volume-up key inconsistency for v2.**

---
Generated by Claude Code Patch Reviewer