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 support for pixel 3a xl with the
 tianma panel
Date: Wed, 11 Feb 2026 16:47:58 +1000
Message-ID: <review-patch6-20260210023300.15785-7-mailingradian@gmail.com>
In-Reply-To: <20260210023300.15785-7-mailingradian@gmail.com>
References: <20260210023300.15785-1-mailingradian@gmail.com> <20260210023300.15785-7-mailingradian@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-Mailer: Claude Code Patch Reviewer

Patch Review

**Copyright Year:**

```c
+ * Copyright (c) 2023-2024, Richard Acayan. All rights reserved.
```

Submitted in 2026, should be 2023-2026 or just 2026.

**Misleading Comment:**

```c
+ * Device tree for Google Pixel 3a XL with the panel connected to the Samsung
+ * Display Controller.
```

The panel is **Tianma/Novatek**, not Samsung. The DSI controller is Qualcomm (part of SDM670). Suggested fix:

```c
/*
 * Device tree for Google Pixel 3a XL with Tianma NT37700F panel.
 */
```

**Panel Compatible (from Patch 3 issue):**

```c
+&panel {
+	compatible = "novatek,nt37700f";
```

Should be `"tianma,nt37700f"` as discussed in Patch 3.

**Physical Dimensions:**

```c
+&rmi4_f12 {
+	touchscreen-x-mm = <69>;
+	touchscreen-y-mm = <137>;
+};
```

Bonito XL: 69×137mm vs Sargo: 63×127mm (after patch 5). Physically larger screen on XL - correct.

**Display Resolution:**

```c
+	width = <1080>;
+	height = <2160>;
```

Bonito: 1080×2160 vs Sargo: 1080×2220. The XL actually has fewer vertical pixels despite larger screen. Verified against real specs - this is correct (Pixel 3a XL is 6.0" 18:9, Pixel 3a is 5.6" 18.5:9).

**Konrad Dybcio's Question:**

> should we anticipate a non-Tianma-panel one too (i.e. are you sure those are out in the wild)?

The cover letter mentions "two variants with panels from Samsung or from Tianma/Novatek", so `-tianma` suffix is appropriate. However, commit message should clarify this.

**Rating:** NEEDS REVISION - Fix copyright year, fix misleading Samsung comment, fix panel compatible

---

## SUMMARY OF REQUIRED CHANGES

### Must Fix (Critical):
1. **Patch 1:** Correct commit message (bonito-sdc → bonito-tianma), document base bonito compatible
2. **Patch 2:** Create dedicated novatek,nt37700f.yaml binding, remove from panel-simple-dsi.yaml  
3. **Patch 3:** Remove TODO, change compatible to "tianma,nt37700f", fix module description
4. **Patch 5:** Justify GPL-2.0-only license change or revert, explain touchscreen dimension change

### Should Fix (Major):
1. **Patch 5:** Use C-style comments in DT files
2. **Patch 6:** Fix copyright year, fix misleading Samsung comment
3. **All:** Update copyright years to match submission date
4. **Series:** Document dependency on IMX355 patches in cover letter

### Nice to Fix (Minor):
1. Add comments explaining vendor-specific register sequences in panel driver
2. Add comment explaining display timing porch values source
3. Consider thread-safety documentation for backlight mode_flags manipulation

**OVERALL RECOMMENDATION:** The series demonstrates good technical understanding and clean code structure, but contains too many critical issues to merge as-is. Please address critical and major issues and submit v2. The core approach of refactoring into common .dtsi is sound.

---
Generated by Claude Code Patch Reviewer