[PATCH 0/6] Support for the Pixel 3a XL with the Tianma panel

[PATCH 0/6] Support for the Pixel 3a XL with the Tianma panel

[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.

Richard Acayan (6):
  dt-bindings: arm: qcom: document google,bonito-tianma board
  dt-bindings: panel-simple-dsi: add nt37700f compatible
  drm/panel: Add Novatek/Tianma NT37700F panel
  arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display
    labels
  arm64: dts: qcom: sdm670-google: add common device tree include
  arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

 .../devicetree/bindings/arm/qcom.yaml         |   6 +
 .../display/panel/panel-simple-dsi.yaml       |   2 +
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../dts/qcom/sdm670-google-bonito-tianma.dts  |  38 +
 ...le-sargo.dts => sdm670-google-common.dtsi} |  23 +-
 .../boot/dts/qcom/sdm670-google-sargo.dts     | 710 +-----------------
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-novatek-nt37700f.c    | 294 ++++++++
 9 files changed, 374 insertions(+), 710 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%)
 create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt37700f.c

-- 
2.53.0

[PATCH 1/6] dt-bindings: arm: qcom: document google, bonito-tianma board

[Richard Acayan]

The google,bonito-sdc board compatible represents the Google Pixel 3a XL
with a Tianma/Novatek NT37700F panel. Document it in the bindings.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index d84bd3bca201..59118faffb02 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -831,6 +831,12 @@ properties:
               - google,sargo
           - const: qcom,sdm670
 
+      - items:
+          - enum:
+              - google,bonito-tianma
+          - const: google,bonito
+          - const: qcom,sdm670
+
       - items:
           - enum:
               - qcom,sdx55-mtp
-- 
2.53.0

Re: [PATCH 1/6] dt-bindings: arm: qcom: document google, bonito-tianma board

[Krzysztof Kozlowski]

On 10/02/2026 03:32, Richard Acayan wrote:
> The google,bonito-sdc board compatible represents the Google Pixel 3a XL

I do not see sdc anywhere in the diff.

> with a Tianma/Novatek NT37700F panel. Document it in the bindings.

What is bonito? Why bonito is compatible with sdm670?

Best regards,
Krzysztof

Claude review: dt-bindings: arm: qcom: document google, bonito-tianma board

[Claude Code Review Bot]

Patch Review

**Commit Message Issues (CRITICAL):**

```
The google,bonito-sdc board compatible represents the Google Pixel 3a XL
with a Tianma/Novatek NT37700F panel. Document it in the bindings.
```

The message refers to "bonito-sdc" but the actual compatible added is "bonito-tianma". This appears to be a copy-paste error from downstream sources.

**Technical Review:**

```yaml
+      - items:
+          - enum:
+              - google,bonito-tianma
+          - const: google,bonito
+          - const: qcom,sdm670
```

**ISSUE:** This creates a compatibility hierarchy `bonito-tianma -> bonito -> sdm670`, but the base "google,bonito" compatible is not documented anywhere in this series. As Krzysztof Kozlowski noted in review, this needs clarification about what "bonito" represents and why it's compatible with sdm670.

**Required Changes:**
1. Fix commit message to say "bonito-tianma" not "bonito-sdc"
2. Either add base "google,bonito" compatible in a separate patch, or flatten to just two levels if no base device exists

**Rating:** NEEDS REVISION

---

---
Generated by Claude Code Patch Reviewer

[PATCH 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible

[Richard Acayan]

Add the compatible for the NT37700F panel found on the Pixel 3a XL.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 .../devicetree/bindings/display/panel/panel-simple-dsi.yaml     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 8d668979b62d..e56865d2771b 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -49,6 +49,8 @@ properties:
       - lg,lh500wx1-sd03
         # Lincoln LCD197 5" 1080x1920 LCD panel
       - lincolntech,lcd197
+        # Novatek NT37700F 1080x2160 AMOLED panel
+      - novatek,nt37700f
         # One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
       - osddisplays,osd101t2587-53ts
         # Panasonic 10" WUXGA TFT LCD panel
-- 
2.53.0

Re: [PATCH 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible

[Krzysztof Kozlowski]

On 10/02/2026 03:32, Richard Acayan wrote:
> Add the compatible for the NT37700F panel found on the Pixel 3a XL.
> 
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> ---
>  .../devicetree/bindings/display/panel/panel-simple-dsi.yaml     | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Claude review: dt-bindings: panel-simple-dsi: add nt37700f compatible

[Claude Code Review Bot]

Patch Review

**CRITICAL ISSUE - Wrong Binding File:**

```yaml
+        # Novatek NT37700F 1080x2160 AMOLED panel
+      - novatek,nt37700f
```

This adds NT37700F to `panel-simple-dsi.yaml`, which is for panels driven by standard DCS commands. However, examining Patch 3's driver reveals:

```c
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x80);
```

These are vendor-specific register sequences, NOT standard DCS commands. The panel requires:
- Custom initialization sequences
- Custom backlight operations  
- Vendor-specific register access

**This panel needs its own binding document:**
`Documentation/devicetree/bindings/display/panel/novatek,nt37700f.yaml`

The binding should document required properties: reset-gpios, power-supply, port, and note the embedded backlight support.

**Rating:** NACK - Must create dedicated binding file, remove from panel-simple-dsi.yaml

---

---
Generated by Claude Code Patch Reviewer

[PATCH 3/6] drm/panel: Add Novatek/Tianma NT37700F panel

[Richard Acayan]

Some Pixel 3a XL devices have a Tianma panel. Add support for it, with
the aid of linux-mdss-dsi-panel-driver-generator.

Link: https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-novatek-nt37700f.c    | 294 ++++++++++++++++++
 3 files changed, 304 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt37700f.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 76f6af819037..138d617e8195 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -553,6 +553,15 @@ config DRM_PANEL_NOVATEK_NT36672E
 	  LCD panel module. The panel has a resolution of 1080x2408 and uses 24 bit
 	  RGB per pixel.
 
+config DRM_PANEL_NOVATEK_NT37700F
+	tristate "Novatek NT37700F DSI panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Novatek NT37700F DSI
+	  panel module. The panel has a resolution of 1080x2160.
+
 config DRM_PANEL_NOVATEK_NT37801
 	tristate "Novatek NT37801/NT37810 AMOLED DSI panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index b9562a6fdcb3..9218a7d7ff34 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36523) += panel-novatek-nt36523.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36672A) += panel-novatek-nt36672a.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36672E) += panel-novatek-nt36672e.o
+obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37700F) += panel-novatek-nt37700f.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37801) += panel-novatek-nt37801.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
 obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt37700f.c b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
new file mode 100644
index 000000000000..491f1f30ce41
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, The Linux Foundation. All rights reserved.
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
+ *   Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct nt37700f_tianma {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct nt37700f_tianma *to_nt37700f_tianma(struct drm_panel *panel)
+{
+	return container_of(panel, struct nt37700f_tianma, panel);
+}
+
+static void nt37700f_tianma_reset(struct nt37700f_tianma *ctx)
+{
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	usleep_range(10000, 11000);
+}
+
+static int nt37700f_tianma_on(struct nt37700f_tianma *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x06);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb5, 0x2b, 0x1a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x04, 0x82);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcc, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x80);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x55);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x56);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x81);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x07);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x07);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x05);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x25);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x01);
+
+	mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 0x0437);
+	mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 0x086f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x04);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd0, 0x11, 0x64);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x09);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb1, 0x20);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+	return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_disable(struct drm_panel *panel)
+{
+	struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display off: %d\n", ret);
+		return ret;
+	}
+	msleep(50);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(100);
+
+	return 0;
+}
+
+static int nt37700f_tianma_prepare(struct drm_panel *panel)
+{
+	struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+	struct device *dev = &ctx->dsi->dev;
+	int ret;
+
+	nt37700f_tianma_reset(ctx);
+
+	ret = nt37700f_tianma_on(ctx);
+	if (ret < 0) {
+		dev_err(dev, "Failed to initialize panel: %d\n", ret);
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int nt37700f_tianma_unprepare(struct drm_panel *panel)
+{
+	struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
+	return 0;
+}
+
+static const struct drm_display_mode nt37700f_tianma_mode = {
+	.clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
+	.hdisplay = 1080,
+	.hsync_start = 1080 + 32,
+	.hsync_end = 1080 + 32 + 32,
+	.htotal = 1080 + 32 + 32 + 98,
+	.vdisplay = 2160,
+	.vsync_start = 2160 + 32,
+	.vsync_end = 2160 + 32 + 4,
+	.vtotal = 2160 + 32 + 4 + 98,
+	.width_mm = 69,
+	.height_mm = 137,
+	.type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int nt37700f_tianma_get_modes(struct drm_panel *panel,
+				     struct drm_connector *connector)
+{
+	return drm_connector_helper_get_modes_fixed(connector, &nt37700f_tianma_mode);
+}
+
+static const struct drm_panel_funcs nt37700f_tianma_panel_funcs = {
+	.prepare = nt37700f_tianma_prepare,
+	.unprepare = nt37700f_tianma_unprepare,
+	.disable = nt37700f_tianma_disable,
+	.get_modes = nt37700f_tianma_get_modes,
+};
+
+static int nt37700f_tianma_bl_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	u16 brightness = backlight_get_brightness(bl);
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return 0;
+}
+
+// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
+// correct values. If not, remove this function.
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	u16 brightness;
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return brightness;
+}
+
+static const struct backlight_ops nt37700f_tianma_bl_ops = {
+	.update_status = nt37700f_tianma_bl_update_status,
+	.get_brightness = nt37700f_tianma_bl_get_brightness,
+};
+
+static struct backlight_device *
+nt37700f_tianma_create_backlight(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	const struct backlight_properties props = {
+		.type = BACKLIGHT_RAW,
+		.brightness = 2047,
+		.max_brightness = 2047,
+	};
+
+	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+					      &nt37700f_tianma_bl_ops, &props);
+}
+
+static int nt37700f_tianma_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct nt37700f_tianma *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+				     "Failed to get reset-gpios\n");
+
+	ctx->dsi = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
+
+	drm_panel_init(&ctx->panel, dev, &nt37700f_tianma_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	ctx->panel.prepare_prev_first = true;
+
+	ctx->panel.backlight = nt37700f_tianma_create_backlight(dsi);
+	if (IS_ERR(ctx->panel.backlight))
+		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+				     "Failed to create backlight\n");
+
+	drm_panel_add(&ctx->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		drm_panel_remove(&ctx->panel);
+		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+	}
+
+	return 0;
+}
+
+static void nt37700f_tianma_remove(struct mipi_dsi_device *dsi)
+{
+	struct nt37700f_tianma *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+	drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+	{ .compatible = "novatek,nt37700f" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nt37700f_tianma_of_match);
+
+static struct mipi_dsi_driver nt37700f_tianma_driver = {
+	.probe = nt37700f_tianma_probe,
+	.remove = nt37700f_tianma_remove,
+	.driver = {
+		.name = "panel-novatek-nt37700f",
+		.of_match_table = nt37700f_tianma_of_match,
+	},
+};
+module_mipi_dsi_driver(nt37700f_tianma_driver);
+
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
+MODULE_LICENSE("GPL");
-- 
2.53.0

Claude review: drm/panel: Add Novatek/Tianma NT37700F panel

[Claude Code Review Bot]

Patch Review

**Code Quality Issues:**

**1. TODO in Production Code (CRITICAL):**

```c
+// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
+// correct values. If not, remove this function.
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
```

Production code submitted to mainline must not contain TODO comments. Either test the functionality or remove the function. The backlight core doesn't require `get_brightness` - it's optional.

**2. Compatible String Issue (CRITICAL):**

```c
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+	{ .compatible = "novatek,nt37700f" },
```

The driver is named `nt37700f_tianma` (Tianma-specific), but uses generic compatible `novatek,nt37700f`. The cover letter states: "The Pixel 3a XL has two variants with panels from Samsung or from Tianma/Novatek."

This confirms multiple vendors use NT37700F with potentially different init sequences. The compatible MUST be vendor-specific:

```c
{ .compatible = "tianma,nt37700f" },
```

**3. Module Description Error:**

```c
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
```

But the driver sets:

```c
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
```

This is VIDEO_BURST mode, not command mode. Fix:

```c
MODULE_DESCRIPTION("DRM driver for Tianma NT37700F DSI video mode panel");
```

**4. Undocumented Magic Values:**

```c
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
```

Add a comment explaining the source of these sequences (downstream DT, datasheet, or reverse-engineered).

**5. Clock Calculation:**

```c
+	.clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
```

While correct, add a comment indicating where porch values originate.

**Positive Aspects:**
- Proper use of `mipi_dsi_multi_context` for error accumulation
- Correct GPIO handling with `gpiod_set_value_cansleep`
- Good error handling with `dev_err_probe()`
- Proper resource cleanup
- Correct use of `DRM_MODE_CONNECTOR_DSI`

**Rating:** NEEDS REVISION - Remove TODO, fix compatible string, fix module description

---

---
Generated by Claude Code Patch Reviewer

[PATCH 4/6] arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display labels

[Richard Acayan]

The touchscreen and framebuffer can have different resolutions on the
Pixel 3a and 3a XL. Label them so the resolutions can be modified, in
anticipation of the addition of the Pixel 3a XL.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index e925cba0381f..2e86bed9ea8c 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -48,7 +48,7 @@ chosen {
 		#size-cells = <2>;
 		ranges;
 
-		framebuffer@9c000000 {
+		framebuffer: framebuffer@9c000000 {
 			compatible = "simple-framebuffer";
 			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
 			width = <1080>;
@@ -516,7 +516,7 @@ 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>;
@@ -533,7 +533,7 @@ &mdss_dsi0 {
 	vdda-supply = <&vreg_l1a_1p225>;
 	status = "okay";
 
-	panel@0 {
+	panel: panel@0 {
 		compatible = "samsung,s6e3fa7-ams559nk06";
 		reg = <0>;
 
-- 
2.53.0

Claude review: arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display labels

[Claude Code Review Bot]

Patch Review

**Technical Review:**

```diff
-		framebuffer@9c000000 {
+		framebuffer: framebuffer@9c000000 {
...
-		rmi4-f12@12 {
+		rmi4_f12: rmi4-f12@12 {
...
-	panel@0 {
+	panel: panel@0 {
```

**Minor Style Issue:**

```c
+		rmi4_f12: rmi4-f12@12 {
```

Label uses underscore while node name uses hyphen. While both valid, consider consistency. However, underscores in labels are common practice, so this is acceptable.

**Commit Message:**

```
The touchscreen and framebuffer can have different resolutions on the
Pixel 3a and 3a XL. Label them so the resolutions can be modified, in
anticipation of the addition of the Pixel 3a XL.
```

Good explanation but doesn't mention the panel label addition. Suggested improvement:

```
arm64: dts: qcom: sdm670-google-sargo: add labels for device-variant nodes

The Pixel 3a and 3a XL share most hardware but differ in display panel,
framebuffer size, and touchscreen dimensions. Add labels to these nodes
to enable device-specific overrides in variant DT files.

This is preparatory work for adding Pixel 3a XL support.
```

**Rating:** ACCEPTABLE (minor commit message improvement suggested)

---

---
Generated by Claude Code Patch Reviewer

[PATCH 5/6] arm64: dts: qcom: sdm670-google: add common device tree include

[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.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 ...le-sargo.dts => sdm670-google-common.dtsi} |  17 +-
 .../boot/dts/qcom/sdm670-google-sargo.dts     | 710 +-----------------
 2 files changed, 20 insertions(+), 707 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 2e86bed9ea8c..29efff444845 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 {
@@ -50,11 +44,8 @@ chosen {
 
 		framebuffer: framebuffer@9c000000 {
 			compatible = "simple-framebuffer";
-			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
-			width = <1080>;
-			height = <2220>;
-			stride = <(1080 * 4)>;
 			format = "a8r8g8b8";
+			status = "disabled";
 		};
 	};
 
@@ -518,8 +509,6 @@ rmi4-f01@1 {
 
 		rmi4_f12: rmi4-f12@12 {
 			reg = <0x12>;
-			touchscreen-x-mm = <62>;
-			touchscreen-y-mm = <127>;
 			syna,sensor-type = <1>;
 		};
 	};
@@ -534,7 +523,7 @@ &mdss_dsi0 {
 	status = "okay";
 
 	panel: panel@0 {
-		compatible = "samsung,s6e3fa7-ams559nk06";
+		// compatible is provided in specific device trees
 		reg = <0>;
 
 		reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
@@ -544,6 +533,8 @@ panel: panel@0 {
 
 		power-supply = <&vreg_l6b_3p3>;
 
+		status = "disabled";
+
 		port {
 			panel_in: endpoint {
 				remote-endpoint = <&mdss_dsi0_out>;
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index 2e86bed9ea8c..7a3e24997232 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -1,715 +1,37 @@
-// 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) 2023, 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: 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>;
-
-		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 {
-				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: rmi4-f12@12 {
-			reg = <0x12>;
-			touchscreen-x-mm = <62>;
-			touchscreen-y-mm = <127>;
-			syna,sensor-type = <1>;
-		};
-	};
 };
 
-&mdss {
-	status = "okay";
+&battery {
+	charge-full-design-microamp-hours = <3000000>;
 };
 
-&mdss_dsi0 {
-	vdda-supply = <&vreg_l1a_1p225>;
+&framebuffer {
+	reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+	width = <1080>;
+	height = <2220>;
+	stride = <(1080 * 4)>;
 	status = "okay";
-
-	panel: 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>;
-};
-
-&mdss_dsi0_phy {
-	vdds-supply = <&vreg_l1b_0p925>;
-	status = "okay";
-};
-
-&mdss_mdp {
+&panel {
+	compatible = "samsung,s6e3fa7-ams559nk06";
 	status = "okay";
 };
 
-&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 = <63>;
+	touchscreen-y-mm = <127>;
 };
-- 
2.53.0

Claude review: arm64: dts: qcom: sdm670-google: add common device tree include

[Claude Code Review Bot]

Patch Review

**CRITICAL - License Change:**

```diff
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0-only
```

The original is GPL-2.0 (v2 or later), the new file is GPL-2.0-only (ONLY v2). This is a **license restriction** that requires:
- Explicit justification in commit message
- Legal review
- Permission from copyright holders

If unintentional, keep GPL-2.0. If intentional, explain why in the commit message.

**CRITICAL - Undocumented Functional Change:**

```diff
 		rmi4_f12: rmi4-f12@12 {
 			reg = <0x12>;
-			touchscreen-x-mm = <62>;
-			touchscreen-y-mm = <127>;
 			syna,sensor-type = <1>;
 		};
```

Then in new sargo.dts:

```diff
+&rmi4_f12 {
+	touchscreen-x-mm = <63>;
+	touchscreen-y-mm = <127>;
+};
```

The X dimension changed from **62mm to 63mm**. This is a functional change buried in a refactoring patch. Either:
- Explain in commit message why this changed
- Put in separate patch
- Verify this isn't a typo

**Style Issue - Comment Format:**

```diff
 	panel: panel@0 {
-		compatible = "samsung,s6e3fa7-ams559nk06";
+		// compatible is provided in specific device trees
```

Kernel device tree files should use C-style comments:

```c
/* compatible is provided in specific device trees */
```

**Copyright Year:**

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

Submitted in 2026 but claims 2023. Should be "2022-2026" or similar.

**Positive Aspects:**
- Logical refactoring that moves ~700 lines of common code
- Correct separation of device-specific (battery capacity, panel type, dimensions) from common (regulators, peripherals)
- Framebuffer properly disabled in common and enabled in device-specific

**Rating:** NEEDS REVISION - License change needs justification, touchscreen change needs explanation, use C-style comments

---

---
Generated by Claude Code Patch Reviewer

Re: [PATCH 5/6] arm64: dts: qcom: sdm670-google: add common device tree include

[Konrad Dybcio]

On 2/10/26 3:32 AM, 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.
> 
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> ---

[...]

>  
>  		framebuffer: framebuffer@9c000000 {
>  			compatible = "simple-framebuffer";
> -			reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
> -			width = <1080>;
> -			height = <2220>;
> -			stride = <(1080 * 4)>;
>  			format = "a8r8g8b8";
> +			status = "disabled";

why?

[...]

>  	panel: panel@0 {
> -		compatible = "samsung,s6e3fa7-ams559nk06";
> +		// compatible is provided in specific device trees

/* C-style comment, please */

lg otherwise

Konrad

[PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

[Richard Acayan]

Some Pixel 3a XL devices have a Tianma panel. Add the separate device
tree for this to support these other devices.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |  1 +
 .../dts/qcom/sdm670-google-bonito-tianma.dts  | 38 +++++++++++++++++++
 2 files changed, 39 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 6f34d5ed331c..6237aeb0e36d 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -245,6 +245,7 @@ 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-sargo.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sdm670-google-bonito-tianma.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
 
 sdm845-db845c-navigation-mezzanine-dtbs	:= sdm845-db845c.dtb sdm845-db845c-navigation-mezzanine.dtbo
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..da309434b511
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device tree for Google Pixel 3a XL with the panel connected to the Samsung
+ * Display Controller.
+ *
+ * Copyright (c) 2023-2024, 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 {
+	reg = <0 0x9c000000 0 (1080 * 2160 * 4)>;
+	width = <1080>;
+	height = <2160>;
+	stride = <(1080 * 4)>;
+	status = "okay";
+};
+
+&panel {
+	compatible = "novatek,nt37700f";
+	status = "okay";
+};
+
+&rmi4_f12 {
+	touchscreen-x-mm = <69>;
+	touchscreen-y-mm = <137>;
+};
-- 
2.53.0

Re: [PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

[Konrad Dybcio]

On 2/10/26 3:33 AM, Richard Acayan wrote:
> Some Pixel 3a XL devices have a Tianma panel. Add the separate device
> tree for this to support these other devices.

There's not a device upstream for any kind of Pixel 3a XL - should
we anticipate a non-Tianma-panel one too (i.e. are you sure those
are out in the wild)?

Konrad

Re: [PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

[Richard Acayan]

On Tue, Feb 10, 2026 at 10:10:46AM +0100, Konrad Dybcio wrote:
> On 2/10/26 3:33 AM, Richard Acayan wrote:
> > Some Pixel 3a XL devices have a Tianma panel. Add the separate device
> > tree for this to support these other devices.
> 
> There's not a device upstream for any kind of Pixel 3a XL - should
> we anticipate a non-Tianma-panel one too (i.e. are you sure those
> are out in the wild)?

Yes, some postmarketOS community members self-declared as owning the
variant with the SDC (Samsung) panel[1].

[1] https://wiki.postmarketos.org/index.php?title=Google_Pixel_3a_XL_(google-bonito)&oldid=89946#Users_owning_this_device

Re: [PATCH 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

[Konrad Dybcio]

On 2/11/26 8:33 PM, Richard Acayan wrote:
> On Tue, Feb 10, 2026 at 10:10:46AM +0100, Konrad Dybcio wrote:
>> On 2/10/26 3:33 AM, Richard Acayan wrote:
>>> Some Pixel 3a XL devices have a Tianma panel. Add the separate device
>>> tree for this to support these other devices.
>>
>> There's not a device upstream for any kind of Pixel 3a XL - should
>> we anticipate a non-Tianma-panel one too (i.e. are you sure those
>> are out in the wild)?
> 
> Yes, some postmarketOS community members self-declared as owning the
> variant with the SDC (Samsung) panel[1].
> 
> [1] https://wiki.postmarketos.org/index.php?title=Google_Pixel_3a_XL_(google-bonito)&oldid=89946#Users_owning_this_device
> 

Ok, please mention in the commit message that the panel is dual-sourced
and the other variant has a samsung display then

Konrad

Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel

[Claude Code Review Bot]

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

Claude review: Support for the Pixel 3a XL with the Tianma panel

[Claude Code Review Bot]

Overall Series Review

Subject: Support for the Pixel 3a XL with the Tianma panel
Author: Richard Acayan <mailingradian@gmail.com>
Patches: 10
Reviewed: 2026-02-11T16:47:57.512044

---

This 6-patch series adds support for the Google Pixel 3a XL (Bonito) with the Tianma/Novatek NT37700F AMOLED panel to the Linux kernel. The series refactors existing Pixel 3a (Sargo) device trees into a common include file, adds a new panel driver, and creates device-specific configurations.

**HIGH-LEVEL ASSESSMENT:**

**CRITICAL ISSUES:**
1. **Wrong binding file** - Patch 2 adds NT37700F to panel-simple-dsi.yaml, but Patch 3 implements a full custom driver with vendor-specific commands, not a simple-dsi panel. Needs dedicated binding documentation.
2. **Commit message error** - Patch 1 describes "bonito-sdc" but adds "bonito-tianma" compatible string
3. **TODO in production code** - Panel driver contains unresolved TODO comment that must be removed
4. **License change** - Patch 5 changes from GPL-2.0 to GPL-2.0-only without explanation
5. **Compatible string issue** - Should be vendor-specific "tianma,nt37700f" not generic "novatek,nt37700f"

**MAJOR ISSUES:**
1. Missing base "google,bonito" compatible documentation
2. Touchscreen dimension change (62mm→63mm) buried in refactoring without explanation
3. Copyright years (2023-2024) don't match submission date (2026)
4. Module description says "cmd mode" but driver uses video mode
5. C++ style comments in device tree files

**POSITIVE ASPECTS:**
- Clean driver implementation using modern kernel APIs (mipi_dsi_multi_context)
- Logical device tree refactoring approach
- Proper error handling in most paths
- Good use of DRM panel helper functions

**OVERALL RECOMMENDATION:** **NACK** - Series requires significant revision before merging.

---

---
Generated by Claude Code Patch Reviewer