public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-02-10  2:33 ` [PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-02-11  6:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  6:47 UTC (permalink / raw)
  To: dri-devel-reviews

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-02-17  0:08 ` [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-02-17  5:51   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-02-17  5:51 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

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

This comment says "Samsung Display Controller" but the file is `sdm670-google-bonito-tianma.dts` and uses the `novatek,nt37700f` compatible. The cover letter mentions this was addressed in v2 ("mention the sdc panel"), but the comment still describes the wrong panel variant. This should say something like "with the Tianma panel" rather than "Samsung Display Controller."

> +	compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670";

Matches the binding added in patch 1. Correct.

> +&battery {
> +	charge-full-design-microamp-hours = <3700000>;
> +};

3700 mAh for the XL variant vs 3000 mAh for the standard 3a. Reasonable.

> +&panel {
> +	compatible = "novatek,nt37700f";
> +	status = "okay";
> +};

Uses the panel driver from patch 3. The `status = "okay"` overrides the `"disabled"` from the common include. Correct.

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

Touchscreen dimensions match the panel physical size (69mm x 137mm) specified in the panel driver's `drm_display_mode`. Consistent.

The Makefile addition places the new dtb entry after `sdm670-google-sargo.dtb` but alphabetically `bonito` comes before `sargo`. Not a functional issue, but the Makefile entry could be placed in alphabetical order for consistency.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-03-10  0:26 ` [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-03-10  1:42   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-03-10  1:42 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Looks good.**

Clean device tree for the Pixel 3a XL Tianma variant. It includes the common dtsi and sets:
- `compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670"` — matches the binding from patch 1
- `charge-full-design-microamp-hours = <3700000>` — different battery from sargo (3000 vs 3700 mAh)
- `height = <2160>` — different panel resolution from sargo (2220 vs 2160)
- `compatible = "novatek,nt37700f"` — the Tianma panel
- Touchscreen dimensions matching the larger display

Already has Konrad's Reviewed-by. No issues.

**Summary of actionable items:**
1. **(Patch 2)** Clarify whether `panel-simple-dsi.yaml` is the correct place for the binding, given the dedicated driver in patch 3.
2. **(Patch 3)** Fix the error path in `nt37700f_tianma_prepare()` to disable the regulator on init failure.
3. **(Patch 3, minor)** Consider removing `MIPI_DSI_MODE_VIDEO_BURST` flag if this is a command-mode panel.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 0/2] arm64: dts: qcom: pixel 3a xl support with tianma panel
@ 2026-05-13 17:25 Richard Acayan
  2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Richard Acayan @ 2026-05-13 17:25 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree
  Cc: Neil Armstrong, Jessica Zhang, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Thierry Reding, Sam Ravnborg, dri-devel, Yifei Zhan,
	Richard Acayan

This expands Pixel 3a support to the Pixel 3a XL, starting with one of
the panel variants. The Pixel 3a XL has two variants with panels from
Samsung or from Tianma/Novatek.

This series depends on the IMX355 front camera devicetree patches
(https://lore.kernel.org/r/20260217002738.133534-1-mailingradian@gmail.com).

The most recent test on a Pixel 3a XL is in v3. v4 is still
compile-tested (with relevant schema) and tested on a Pixel 3a.

Changes since v3 (https://lore.kernel.org/r/20260310002606.16413-1-mailingradian@gmail.com):
- disclose AI involvement in review (Konrad's review tags retained) (1-2/2)

	Some changes appeared in v3 (under "Changes since v2") which
	were suggested by the "Claude Code Review Bot", an experimental
	AI reviewer deployed by a DRM maintainer:

	- preserve same touchscreen physical width in sargo (1/2)
	- move entry in Makefile to correct place (2/2)
	- correct top-of-file comment about which panel is supported (2/2)
	- explain license identifier change from GPL-2.0 to GPL-2.0-only (1/2)

	The AI involvement already occurred between v2 and v3. The full
	AI-generated output is accessible at:

	https://lore.gitlab.freedesktop.org/drm-ai-reviews/20260217000854.131242-1-mailingradian@gmail.com/T/#u

- rebase onto new front camera patches
- drop applied patches (previously 1-3/5)
- add review tags (1/2)

Changes since v2 (https://lore.kernel.org/r/20260217000854.131242-1-mailingradian@gmail.com):
- add support for regulator (Dmitry's review retained) (3/5)
- use common set page function (Dmitry's review retained) (3/5)
- squash new labels into common dtsi patch (review tags dropped) (4/5)
- specify framebuffer in terms of memory region (4/5)
- only specify height in XL/non-XL specific devicetrees (4/5, 5/5)
- remove status in display devices (4/5, 5/5)
- preserve same touchscreen physical width in sargo (4/5)
- move entry in Makefile to correct place (5/5)
- omit copyright year in new file (4/5)
- correct top-of-file comment about which panel is supported (5/5)
- explain license identifier change from GPL-2.0 to GPL-2.0-only (4/5)
- add review tags (1/5, 3/5, 5/5)

Changes since v1 (https://lore.kernel.org/r/20260210023300.15785-1-mailingradian@gmail.com):
- use multi functions in disable callback (3/6)
- add and reformat comments about XL differences (5/6)
- mention the sdc panel (6/6)
- explain google,bonito compatible (1/6)
- remove TODO item in code (3/6)
- add review and testing tags (2/6, 3/6, 5/6)

Richard Acayan (2):
  arm64: dts: qcom: sdm670-google: add common device tree include
  arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../dts/qcom/sdm670-google-bonito-tianma.dts  |  32 +
 ...le-sargo.dts => sdm670-google-common.dtsi} |  22 +-
 .../boot/dts/qcom/sdm670-google-sargo.dts     | 714 +-----------------
 4 files changed, 53 insertions(+), 716 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
 copy arch/arm64/boot/dts/qcom/{sdm670-google-sargo.dts => sdm670-google-common.dtsi} (97%)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include
  2026-05-13 17:25 [PATCH v4 0/2] arm64: dts: qcom: pixel 3a xl support with tianma panel Richard Acayan
@ 2026-05-13 17:25 ` Richard Acayan
  2026-05-13 17:57   ` Dmitry Baryshkov
  2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
  2026-05-13 17:25 ` [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
  2026-05-16  1:37 ` Claude review: arm64: dts: qcom: pixel 3a xl support with " Claude Code Review Bot
  2 siblings, 2 replies; 11+ messages in thread
From: Richard Acayan @ 2026-05-13 17:25 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree
  Cc: Neil Armstrong, Jessica Zhang, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Thierry Reding, Sam Ravnborg, dri-devel, Yifei Zhan,
	Richard Acayan

The Pixel 3a XL can be supported by reusing most of the device tree from
the Pixel 3a. Move the common elements to a common device tree include
like with other devices.

Since the original devicetree should only specify non-XL properties, it
needs to be completely rewritten. Also change the SPDX license
identifier from GPL-2.0 (which was deprecated as it can be
misinterpreted as GPLv2 or later) into GPL-2.0-only.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Assisted-by: "Claude Code Review Bot":claude-opus-4-6
---
 ...le-sargo.dts => sdm670-google-common.dtsi} |  22 +-
 .../boot/dts/qcom/sdm670-google-sargo.dts     | 714 +-----------------
 2 files changed, 20 insertions(+), 716 deletions(-)
 copy arch/arm64/boot/dts/qcom/{sdm670-google-sargo.dts => sdm670-google-common.dtsi} (97%)

diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
similarity index 97%
copy from arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
copy to arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
index eba2f3792dd0..0f57b915186b 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
@@ -6,8 +6,6 @@
  * Copyright (c) 2022, Richard Acayan. All rights reserved.
  */
 
-/dts-v1/;
-
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/leds/common.h>
@@ -28,9 +26,6 @@
 /delete-node/ &gpu_mem;
 
 / {
-	model = "Google Pixel 3a";
-	compatible = "google,sargo", "qcom,sdm670";
-
 	aliases { };
 
 	battery: battery {
@@ -38,7 +33,6 @@ battery: battery {
 
 		voltage-min-design-microvolt = <3312000>;
 		voltage-max-design-microvolt = <4400000>;
-		charge-full-design-microamp-hours = <3000000>;
 	};
 
 	chosen {
@@ -48,11 +42,11 @@ chosen {
 		#size-cells = <2>;
 		ranges;
 
-		framebuffer@9c000000 {
+		framebuffer: framebuffer@9c000000 {
 			compatible = "simple-framebuffer";
-			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+			memory-region = <&fb_mem>;
 			width = <1080>;
-			height = <2220>;
+			/* height is in specific device trees */
 			stride = <(1080 * 4)>;
 			format = "a8r8g8b8";
 		};
@@ -121,7 +115,7 @@ gpu_mem: gpu@97a15000 {
 			no-map;
 		};
 
-		framebuffer-region@9c000000 {
+		fb_mem: framebuffer-region@9c000000 {
 			reg = <0 0x9c000000 0 0x2400000>;
 			no-map;
 		};
@@ -521,10 +515,8 @@ rmi4-f01@1 {
 			syna,nosleep-mode = <1>;
 		};
 
-		rmi4-f12@12 {
+		rmi4_f12: rmi4-f12@12 {
 			reg = <0x12>;
-			touchscreen-x-mm = <62>;
-			touchscreen-y-mm = <127>;
 			syna,sensor-type = <1>;
 		};
 	};
@@ -538,8 +530,8 @@ &mdss_dsi0 {
 	vdda-supply = <&vreg_l1a_1p225>;
 	status = "okay";
 
-	panel@0 {
-		compatible = "samsung,s6e3fa7-ams559nk06";
+	panel: panel@0 {
+		/* compatible is provided in specific device trees */
 		reg = <0>;
 
 		reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index eba2f3792dd0..c67a0396436b 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -1,720 +1,32 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0-only
 /*
- * Device tree for Google Pixel 3a, adapted from google-blueline device tree,
- * xiaomi-lavender device tree, and oneplus-common device tree.
+ * Device tree for Google Pixel 3a (non-XL).
  *
- * Copyright (c) 2022, Richard Acayan. All rights reserved.
+ * Copyright (c) Richard Acayan. All rights reserved.
  */
 
 /dts-v1/;
 
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/input/input.h>
-#include <dt-bindings/leds/common.h>
-#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
-#include <dt-bindings/power/qcom-rpmpd.h>
-#include "sdm670.dtsi"
-#include "pm660.dtsi"
-#include "pm660l.dtsi"
-
-/delete-node/ &mpss_region;
-/delete-node/ &venus_mem;
-/delete-node/ &wlan_msa_mem;
-/delete-node/ &cdsp_mem;
-/delete-node/ &mba_region;
-/delete-node/ &adsp_mem;
-/delete-node/ &ipa_fw_mem;
-/delete-node/ &ipa_gsi_mem;
-/delete-node/ &gpu_mem;
+#include "sdm670-google-common.dtsi"
 
 / {
 	model = "Google Pixel 3a";
 	compatible = "google,sargo", "qcom,sdm670";
-
-	aliases { };
-
-	battery: battery {
-		compatible = "simple-battery";
-
-		voltage-min-design-microvolt = <3312000>;
-		voltage-max-design-microvolt = <4400000>;
-		charge-full-design-microamp-hours = <3000000>;
-	};
-
-	chosen {
-		stdout-path = "serial0:115200n8";
-
-		#address-cells = <2>;
-		#size-cells = <2>;
-		ranges;
-
-		framebuffer@9c000000 {
-			compatible = "simple-framebuffer";
-			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
-			width = <1080>;
-			height = <2220>;
-			stride = <(1080 * 4)>;
-			format = "a8r8g8b8";
-		};
-	};
-
-	gpio-keys {
-		compatible = "gpio-keys";
-		autorepeat;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&vol_up_pin>;
-
-		key-vol-up {
-			label = "Volume Up";
-			linux,code = <KEY_VOLUMEUP>;
-			gpios = <&pm660l_gpios 7 GPIO_ACTIVE_LOW>;
-		};
-	};
-
-	reserved-memory {
-		#address-cells = <2>;
-		#size-cells = <2>;
-
-		mpss_region: mpss@8b000000 {
-			reg = <0 0x8b000000 0 0x9800000>;
-			no-map;
-		};
-
-		venus_mem: venus@94800000 {
-			reg = <0 0x94800000 0 0x500000>;
-			no-map;
-		};
-
-		wlan_msa_mem: wlan-msa@94d00000 {
-			reg = <0 0x94d00000 0 0x100000>;
-			no-map;
-		};
-
-		cdsp_mem: cdsp@94e00000 {
-			reg = <0 0x94e00000 0 0x800000>;
-			no-map;
-		};
-
-		mba_region: mba@95600000 {
-			reg = <0 0x95600000 0 0x200000>;
-			no-map;
-		};
-
-		adsp_mem: adsp@95800000 {
-			reg = <0 0x95800000 0 0x2200000>;
-			no-map;
-		};
-
-		ipa_fw_mem: ipa-fw@97a00000 {
-			reg = <0 0x97a00000 0 0x10000>;
-			no-map;
-		};
-
-		ipa_gsi_mem: ipa-gsi@97a10000 {
-			reg = <0 0x97a10000 0 0x5000>;
-			no-map;
-		};
-
-		gpu_mem: gpu@97a15000 {
-			reg = <0 0x97a15000 0 0x2000>;
-			no-map;
-		};
-
-		framebuffer-region@9c000000 {
-			reg = <0 0x9c000000 0 0x2400000>;
-			no-map;
-		};
-
-		/* Also includes ramoops regions */
-		debug_info_mem: debug-info@a1800000 {
-			reg = <0 0xa1800000 0 0x411000>;
-			no-map;
-		};
-	};
-
-	/*
-	 * The touchscreen regulator seems to be controlled somehow by a gpio.
-	 * Model it as a fixed regulator and keep it on. Without schematics we
-	 * don't know how this is actually wired up...
-	 */
-	ts_1p8_supply: ts-1p8-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "ts_1p8_supply";
-
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-
-		gpio = <&pm660_gpios 12 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
-	vph_pwr: vph-pwr-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "vph_pwr";
-		regulator-min-microvolt = <3312000>;
-		regulator-max-microvolt = <3312000>;
-
-		regulator-always-on;
-		regulator-boot-on;
-	};
-
-	/*
-	 * Supply map from xiaomi-lavender specifies this as the supply for
-	 * ldob1, ldob9, ldob10, ldoa2, and ldoa3, while downstream specifies
-	 * this as a power domain. Set this as a fixed regulator with the same
-	 * voltage as lavender until display is needed to avoid unneccessarily
-	 * using a deprecated binding (regulator-fixed-domain).
-	 */
-	vreg_s2b_1p05: vreg-s2b-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "vreg_s2b";
-		regulator-min-microvolt = <1050000>;
-		regulator-max-microvolt = <1050000>;
-	};
-
-	cam_front_ldo: cam-front-ldo-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "cam_front_ldo";
-		regulator-min-microvolt = <1352000>;
-		regulator-max-microvolt = <1352000>;
-		regulator-enable-ramp-delay = <135>;
-
-		gpios = <&pm660l_gpios 4 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-
-		pinctrl-0 = <&cam_front_ldo_pin>;
-		pinctrl-names = "default";
-	};
-
-	cam_vio_ldo: cam-vio-ldo-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "cam_vio_ldo";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-enable-ramp-delay = <233>;
-
-		gpios = <&pm660_gpios 13 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-
-		pinctrl-0 = <&cam_vio_pin>;
-		pinctrl-names = "default";
-	};
-};
-
-&apps_rsc {
-	regulators-0 {
-		compatible = "qcom,pm660-rpmh-regulators";
-		qcom,pmic-id = "a";
-
-		vdd-s1-supply = <&vph_pwr>;
-		vdd-s2-supply = <&vph_pwr>;
-		vdd-s3-supply = <&vph_pwr>;
-		vdd-s4-supply = <&vph_pwr>;
-		vdd-s5-supply = <&vph_pwr>;
-		vdd-s6-supply = <&vph_pwr>;
-
-		vdd-l1-l6-l7-supply = <&vreg_s6a_0p87>;
-		vdd-l2-l3-supply = <&vreg_s2b_1p05>;
-		vdd-l5-supply = <&vreg_s2b_1p05>;
-		vdd-l8-l9-l10-l11-l12-l13-l14-supply = <&vreg_s4a_2p04>;
-		vdd-l15-l16-l17-l18-l19-supply = <&vreg_bob>;
-
-		/*
-		 * S1A (FTAPC0), S2A (FTAPC1), S3A (HFAPC1) are managed
-		 * by the Core Power Reduction hardened (CPRh) and the
-		 * Operating State Manager (OSM) HW automatically.
-		 */
-
-		vreg_s4a_2p04: smps4 {
-			regulator-min-microvolt = <1808000>;
-			regulator-max-microvolt = <2040000>;
-			regulator-enable-ramp-delay = <200>;
-		};
-
-		vreg_s6a_0p87: smps6 {
-			regulator-min-microvolt = <1224000>;
-			regulator-max-microvolt = <1352000>;
-			regulator-enable-ramp-delay = <150>;
-		};
-
-		/* LDOs */
-		vreg_l1a_1p225: ldo1 {
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1250000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l2a_1p0: ldo2 {
-			regulator-min-microvolt = <1000000>;
-			regulator-max-microvolt = <1000000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l3a_1p0: ldo3 {
-			regulator-min-microvolt = <1000000>;
-			regulator-max-microvolt = <1000000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l5a_0p848: ldo5 {
-			regulator-min-microvolt = <800000>;
-			regulator-max-microvolt = <800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l6a_1p3: ldo6 {
-			regulator-min-microvolt = <1248000>;
-			regulator-max-microvolt = <1304000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l7a_1p2: ldo7 {
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l8a_1p8: ldo8 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-			regulator-always-on;
-		};
-
-		vreg_l9a_1p8: ldo9 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l10a_1p8: ldo10 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l11a_1p8: ldo11 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l12a_1p8: ldo12 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l13a_1p8: ldo13 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l14a_1p8: ldo14 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l15a_1p8: ldo15 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2950000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l16a_2p7: ldo16 {
-			regulator-min-microvolt = <2696000>;
-			regulator-max-microvolt = <2696000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l17a_1p8: ldo17 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2950000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l19a_3p3: ldo19 {
-			regulator-min-microvolt = <3000000>;
-			regulator-max-microvolt = <3312000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-	};
-
-	regulators-1 {
-		compatible = "qcom,pm660l-rpmh-regulators";
-		qcom,pmic-id = "b";
-
-		vdd-s1-supply = <&vph_pwr>;
-		vdd-s2-supply = <&vph_pwr>;
-		vdd-s3-s4-supply = <&vph_pwr>;
-		vdd-s5-supply = <&vph_pwr>;
-
-		vdd-l1-l9-l10-supply = <&vreg_s2b_1p05>;
-		vdd-l2-supply = <&vreg_bob>;
-		vdd-l3-l5-l7-l8-supply = <&vreg_bob>;
-		vdd-l4-l6-supply = <&vreg_bob>;
-		vdd-bob-supply = <&vph_pwr>;
-
-		/* LDOs */
-		vreg_l1b_0p925: ldo1 {
-			regulator-min-microvolt = <880000>;
-			regulator-max-microvolt = <900000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l2b_2p95: ldo2 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2960000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l3b_3p0: ldo3 {
-			regulator-min-microvolt = <2850000>;
-			regulator-max-microvolt = <3008000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l4b_2p95: ldo4 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l5b_2p95: ldo5 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l6b_3p3: ldo6 {
-			regulator-min-microvolt = <3008000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l7b_3p125: ldo7 {
-			regulator-min-microvolt = <3088000>;
-			regulator-max-microvolt = <3100000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		vreg_l8b_3p3: ldo8 {
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3312000>;
-			regulator-enable-ramp-delay = <250>;
-		};
-
-		/*
-		 * Downstream specifies a fixed voltage of 3.312 V, but the
-		 * PMIC4 BOB ranges don't support that. Widen the range a
-		 * little to avoid adding a new BOB regulator type.
-		 */
-		vreg_bob: bob {
-			regulator-min-microvolt = <3296000>;
-			regulator-max-microvolt = <3328000>;
-			regulator-enable-ramp-delay = <500>;
-		};
-	};
-};
-
-&camss {
-	vdda-phy-supply = <&vreg_l1a_1p225>;
-	vdda-pll-supply = <&vreg_s6a_0p87>;
-
-	status = "okay";
-};
-
-&camss_port1 {
-	camss_endpoint1: endpoint {
-		data-lanes = <0 1 2 3>;
-		remote-endpoint = <&cam_front_endpoint>;
-	};
 };
 
-&cci {
-	pinctrl-0 = <&cci1_default>;
-	pinctrl-1 = <&cci1_sleep>;
-	pinctrl-names = "default", "sleep";
-
-	status = "okay";
-};
-
-&cci_i2c1 {
-	camera@1a {
-		compatible = "sony,imx355";
-		reg = <0x1a>;
-
-		clocks = <&camcc CAM_CC_MCLK2_CLK>;
-
-		/*
-		 * The sensor can accept a 24 MHz clock, but 19.2 MHz has
-		 * better driver compatibility.
-		 */
-		assigned-clocks = <&camcc CAM_CC_MCLK2_CLK>;
-		assigned-clock-rates = <19200000>;
-
-		reset-gpios = <&tlmm 9 GPIO_ACTIVE_LOW>;
-
-		avdd-supply = <&cam_front_ldo>;
-		dvdd-supply = <&cam_front_ldo>;
-		dovdd-supply = <&cam_vio_ldo>;
-
-		pinctrl-0 = <&cam_mclk2_default>;
-		pinctrl-names = "default";
-
-		rotation = <270>;
-		orientation = <0>;
-
-		port {
-			cam_front_endpoint: endpoint {
-				data-lanes = <1 2 3 4>;
-				link-frequencies = /bits/ 64 <360000000>;
-				remote-endpoint = <&camss_endpoint1>;
-			};
-		};
-	};
-};
-
-&gcc {
-	protected-clocks = <GCC_QSPI_CORE_CLK>,
-			   <GCC_QSPI_CORE_CLK_SRC>,
-			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
-};
-
-&gpi_dma1 {
-	status = "okay";
-};
-
-&gpu {
-	status = "okay";
-};
-
-&gpu_zap_shader {
-	firmware-name = "qcom/sdm670/sargo/a615_zap.mbn";
-};
-
-&i2c9 {
-	clock-frequency = <100000>;
-	status = "okay";
-
-	synaptics-rmi4-i2c@20 {
-		compatible = "syna,rmi4-i2c";
-		reg = <0x20>;
-		interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&touchscreen_default>;
-
-		vio-supply = <&ts_1p8_supply>;
-
-		syna,reset-delay-ms = <200>;
-		syna,startup-delay-ms = <200>;
-
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		rmi4-f01@1 {
-			reg = <0x01>;
-			syna,nosleep-mode = <1>;
-		};
-
-		rmi4-f12@12 {
-			reg = <0x12>;
-			touchscreen-x-mm = <62>;
-			touchscreen-y-mm = <127>;
-			syna,sensor-type = <1>;
-		};
-	};
-};
-
-&mdss {
-	status = "okay";
-};
-
-&mdss_dsi0 {
-	vdda-supply = <&vreg_l1a_1p225>;
-	status = "okay";
-
-	panel@0 {
-		compatible = "samsung,s6e3fa7-ams559nk06";
-		reg = <0>;
-
-		reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&panel_default>;
-
-		power-supply = <&vreg_l6b_3p3>;
-
-		port {
-			panel_in: endpoint {
-				remote-endpoint = <&mdss_dsi0_out>;
-			};
-		};
-	};
-};
-
-&mdss_dsi0_out {
-	remote-endpoint = <&panel_in>;
-	data-lanes = <0 1 2 3>;
+&battery {
+	charge-full-design-microamp-hours = <3000000>;
 };
 
-&mdss_dsi0_phy {
-	vdds-supply = <&vreg_l1b_0p925>;
-	status = "okay";
+&framebuffer {
+	height = <2220>;
 };
 
-&mdss_mdp {
-	status = "okay";
+&panel {
+	compatible = "samsung,s6e3fa7-ams559nk06";
 };
 
-&pm660_charger {
-	monitored-battery = <&battery>;
-	status = "okay";
-};
-
-&pm660_gpios {
-	cam_vio_pin: cam-vio-state {
-		pins = "gpio13";
-		function = "normal";
-		power-source = <0>;
-	};
-};
-
-&pm660_rradc {
-	status = "okay";
-};
-
-&pm660l_flash {
-	status = "okay";
-
-	led-0 {
-		function = LED_FUNCTION_FLASH;
-		color = <LED_COLOR_ID_WHITE>;
-		led-sources = <1>, <2>;
-		led-max-microamp = <500000>;
-		flash-max-microamp = <1500000>;
-		flash-max-timeout-us = <1280000>;
-	};
-};
-
-&pm660l_gpios {
-	cam_front_ldo_pin: cam-front-state {
-		pins = "gpio4";
-		function = "normal";
-		power-source = <0>;
-	};
-
-	vol_up_pin: vol-up-state {
-		pins = "gpio7";
-		function = "normal";
-		qcom,drive-strength = <PMIC_GPIO_STRENGTH_NO>;
-		input-enable;
-		bias-pull-up;
-	};
-};
-
-&pon_pwrkey {
-	status = "okay";
-};
-
-&pon_resin {
-	linux,code = <KEY_VOLUMEDOWN>;
-	status = "okay";
-};
-
-&qupv3_id_1 {
-	status = "okay";
-};
-
-&sdhc_1 {
-	supports-cqe;
-	mmc-hs200-1_8v;
-	mmc-hs400-1_8v;
-	mmc-ddr-1_8v;
-
-	qcom,ddr-config = <0xc3040873>;
-
-	vmmc-supply = <&vreg_l4b_2p95>;
-	vqmmc-supply = <&vreg_l8a_1p8>;
-
-	status = "okay";
-};
-
-&tlmm {
-	gpio-reserved-ranges = <0 4>, <81 4>;
-
-	panel_default: panel-default-state {
-		te-pins {
-			pins = "gpio10";
-			function = "mdp_vsync";
-			drive-strength = <2>;
-			bias-pull-down;
-		};
-
-		reset-pins {
-			pins = "gpio75";
-			function = "gpio";
-			drive-strength = <8>;
-			bias-disable;
-		};
-
-		mode-pins {
-			pins = "gpio76";
-			function = "gpio";
-			drive-strength = <8>;
-			bias-disable;
-		};
-	};
-
-	touchscreen_default: ts-default-state {
-		ts-reset-pins {
-			pins = "gpio99";
-			function = "gpio";
-			drive-strength = <2>;
-			bias-pull-up;
-			output-high;
-		};
-
-		ts-irq-pins {
-			pins = "gpio125";
-			function = "gpio";
-			drive-strength = <2>;
-			bias-disable;
-		};
-
-		ts-switch-pins {
-			pins = "gpio135";
-			function = "gpio";
-			drive-strength = <2>;
-			bias-disable;
-			output-low;
-		};
-	};
-};
-
-&usb_1_hsphy {
-	vdd-supply = <&vreg_l1b_0p925>;
-	vdda-pll-supply = <&vreg_l10a_1p8>;
-	vdda-phy-dpdm-supply = <&vreg_l7b_3p125>;
-
-	status = "okay";
-};
-
-&usb_1 {
-	qcom,select-utmi-as-pipe-clk;
-	status = "okay";
-};
-
-&usb_1_dwc3 {
-	/* Only peripheral works for now */
-	dr_mode = "peripheral";
-
-	/* Do not assume that sdm670.dtsi will never support USB 3.0 */
-	phys = <&usb_1_hsphy>;
-	phy-names = "usb2-phy";
-	maximum-speed = "high-speed";
+&rmi4_f12 {
+	touchscreen-x-mm = <62>;
+	touchscreen-y-mm = <127>;
 };
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-05-13 17:25 [PATCH v4 0/2] arm64: dts: qcom: pixel 3a xl support with tianma panel Richard Acayan
  2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-05-13 17:25 ` Richard Acayan
  2026-05-13 17:58   ` Dmitry Baryshkov
  2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
  2026-05-16  1:37 ` Claude review: arm64: dts: qcom: pixel 3a xl support with " Claude Code Review Bot
  2 siblings, 2 replies; 11+ messages in thread
From: Richard Acayan @ 2026-05-13 17:25 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree
  Cc: Neil Armstrong, Jessica Zhang, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Thierry Reding, Sam Ravnborg, dri-devel, Yifei Zhan,
	Richard Acayan

The Pixel 3a XL has variants with either a Samsung Display Controller (SDC)
panel or a Tianma panel. Add the device tree for the variant with the
Tianma panel.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Assisted-by: "Claude Code Review Bot":claude-opus-4-6
---
 arch/arm64/boot/dts/qcom/Makefile             |  1 +
 .../dts/qcom/sdm670-google-bonito-tianma.dts  | 32 +++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 169cc9b9ccd3..d909c527d848 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -303,6 +303,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm632-fairphone-fp3.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm632-motorola-ocean.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm636-sony-xperia-ganges-mermaid.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm660-xiaomi-lavender.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-bonito-tianma.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-sargo.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
 
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
new file mode 100644
index 000000000000..42ddaf53d150
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device tree for Google Pixel 3a XL with the Tianma panel.
+ *
+ * Copyright (c) Richard Acayan. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sdm670-google-common.dtsi"
+
+/ {
+	model = "Google Pixel 3a XL (with Tianma panel)";
+	compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670";
+};
+
+&battery {
+	charge-full-design-microamp-hours = <3700000>;
+};
+
+&framebuffer {
+	height = <2160>;
+};
+
+&panel {
+	compatible = "novatek,nt37700f";
+};
+
+&rmi4_f12 {
+	touchscreen-x-mm = <69>;
+	touchscreen-y-mm = <137>;
+};
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include
  2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-05-13 17:57   ` Dmitry Baryshkov
  2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-05-13 17:57 UTC (permalink / raw)
  To: Richard Acayan
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, Neil Armstrong,
	Jessica Zhang, David Airlie, Simona Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Thierry Reding, Sam Ravnborg,
	dri-devel, Yifei Zhan

On Wed, May 13, 2026 at 01:25:48PM -0400, Richard Acayan wrote:
> The Pixel 3a XL can be supported by reusing most of the device tree from
> the Pixel 3a. Move the common elements to a common device tree include
> like with other devices.
> 
> Since the original devicetree should only specify non-XL properties, it
> needs to be completely rewritten. Also change the SPDX license
> identifier from GPL-2.0 (which was deprecated as it can be
> misinterpreted as GPLv2 or later) into GPL-2.0-only.
> 
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Assisted-by: "Claude Code Review Bot":claude-opus-4-6
> ---
>  ...le-sargo.dts => sdm670-google-common.dtsi} |  22 +-
>  .../boot/dts/qcom/sdm670-google-sargo.dts     | 714 +-----------------
>  2 files changed, 20 insertions(+), 716 deletions(-)
>  copy arch/arm64/boot/dts/qcom/{sdm670-google-sargo.dts => sdm670-google-common.dtsi} (97%)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-05-13 17:25 ` [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-05-13 17:58   ` Dmitry Baryshkov
  2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-05-13 17:58 UTC (permalink / raw)
  To: Richard Acayan
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, Neil Armstrong,
	Jessica Zhang, David Airlie, Simona Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Thierry Reding, Sam Ravnborg,
	dri-devel, Yifei Zhan

On Wed, May 13, 2026 at 01:25:49PM -0400, Richard Acayan wrote:
> The Pixel 3a XL has variants with either a Samsung Display Controller (SDC)
> panel or a Tianma panel. Add the device tree for the variant with the
> Tianma panel.
> 
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Assisted-by: "Claude Code Review Bot":claude-opus-4-6
> ---
>  arch/arm64/boot/dts/qcom/Makefile             |  1 +
>  .../dts/qcom/sdm670-google-bonito-tianma.dts  | 32 +++++++++++++++++++
>  2 files changed, 33 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 169cc9b9ccd3..d909c527d848 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -303,6 +303,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm632-fairphone-fp3.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm632-motorola-ocean.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm636-sony-xperia-ganges-mermaid.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm660-xiaomi-lavender.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-bonito-tianma.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-sargo.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
>  

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: arm64: dts: qcom: pixel 3a xl support with tianma panel
  2026-05-13 17:25 [PATCH v4 0/2] arm64: dts: qcom: pixel 3a xl support with tianma panel Richard Acayan
  2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
  2026-05-13 17:25 ` [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-05-16  1:37 ` Claude Code Review Bot
  2 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  1:37 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: arm64: dts: qcom: pixel 3a xl support with tianma panel
Author: Richard Acayan <mailingradian@gmail.com>
Patches: 5
Reviewed: 2026-05-16T11:37:15.551397

---

This is a clean, well-structured 2-patch series that refactors the existing Pixel 3a (sargo) device tree to extract common hardware definitions into a shared `.dtsi` include, then adds a new Pixel 3a XL (bonito) variant with the Tianma/Novatek panel.

The approach follows established DT patterns for sharing code between device variants. The factoring decisions are sensible — device-specific properties (model, compatible, battery capacity, display height, panel compatible, touchscreen dimensions) are correctly moved to per-device `.dts` files, while all shared SoC/board configuration stays in the common include.

The series has Reviewed-by from Konrad Dybcio on both patches, proper AI assistance disclosure, and the cover letter is thorough. The `novatek,nt37700f` compatible already has both a binding and a driver in the tree, so the new device tree will work out of the box.

**No blocking issues.** A few minor observations below.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: arm64: dts: qcom: sdm670-google: add common device tree include
  2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
  2026-05-13 17:57   ` Dmitry Baryshkov
@ 2026-05-16  1:37   ` Claude Code Review Bot
  1 sibling, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  1:37 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Overall:** Good refactoring. The split between common and device-specific properties is logical and complete.

**Positive changes:**

- The switch from inline `reg` with a computed size to `memory-region = <&fb_mem>` in the framebuffer node is a good improvement. It avoids duplicating the address and references the already-defined reserved memory region, which is the preferred DT pattern:
```
-		framebuffer@9c000000 {
+		framebuffer: framebuffer@9c000000 {
 			compatible = "simple-framebuffer";
-			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+			memory-region = <&fb_mem>;
 			width = <1080>;
-			height = <2220>;
+			/* height is in specific device trees */
```

- Adding labels (`framebuffer:`, `fb_mem:`, `rmi4_f12:`, `panel:`) to nodes that need per-device overrides is the correct mechanism.

- The SPDX license change from `GPL-2.0` to `GPL-2.0-only` in the rewritten sargo.dts is appropriate — `GPL-2.0` is deprecated as ambiguous. The commit message explains the rationale well.

**Minor observations (non-blocking):**

1. The comment `/* compatible is provided in specific device trees */` and `/* height is in specific device trees */` are helpful for readability, since a panel node without a compatible or a framebuffer without height would otherwise look like a bug. These are good comments.

2. The `width = <1080>` and `stride = <(1080 * 4)>` are hardcoded in the common dtsi. Both current variants (sargo and bonito-tianma) share 1080px width, so this is fine. If a future variant has a different width, it would need to override these too, but that's a bridge to cross later.

3. The rewritten `sdm670-google-sargo.dts` is pleasantly minimal — just the include and five short override blocks. This validates that the common/specific split was done well.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
  2026-05-13 17:25 ` [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
  2026-05-13 17:58   ` Dmitry Baryshkov
@ 2026-05-16  1:37   ` Claude Code Review Bot
  1 sibling, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  1:37 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Overall:** Clean, minimal device tree for the new variant. Nothing to object to.

**Makefile ordering** is correct — `bonito` sorts before `sargo` alphabetically:
```
+dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-bonito-tianma.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-sargo.dtb
```

**Compatible string** follows the standard most-specific-to-least-specific pattern:
```
+	compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670";
```

**Device-specific values** all check out against known hardware specs:
- Battery: `3700000` µAh (3700mAh, correct for Pixel 3a XL)
- Height: `2160` (matches the NT37700F panel binding description: "1080x2160 AMOLED")
- Panel compatible: `novatek,nt37700f` (binding and driver exist in-tree)
- Touchscreen: `69x137` mm (reasonable for 6.0" screen vs sargo's `62x127` mm for 5.6")

**Framebuffer region sizing:** The shared `fb_mem` reserved region is `0x2400000` (37.7 MB), which comfortably fits both sargo's 1080×2220×4 = 9.6 MB and bonito's 1080×2160×4 = 9.3 MB. No issue.

**No issues found in this patch.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-05-16  1:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 17:25 [PATCH v4 0/2] arm64: dts: qcom: pixel 3a xl support with tianma panel Richard Acayan
2026-05-13 17:25 ` [PATCH v4 1/2] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
2026-05-13 17:57   ` Dmitry Baryshkov
2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
2026-05-13 17:25 ` [PATCH v4 2/2] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-05-13 17:58   ` Dmitry Baryshkov
2026-05-16  1:37   ` Claude review: " Claude Code Review Bot
2026-05-16  1:37 ` Claude review: arm64: dts: qcom: pixel 3a xl support with " Claude Code Review Bot
  -- strict thread matches above, loose matches on Subject: below --
2026-03-10  0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-03-10  0:26 ` [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-03-10  1:42   ` Claude review: " Claude Code Review Bot
2026-02-17  0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-02-17  0:08 ` [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-02-17  5:51   ` Claude review: " Claude Code Review Bot
2026-02-10  2:32 [PATCH 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-02-10  2:33 ` [PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-02-11  6:47   ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox