From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: arm64: dts: qcom: sdm670-google: add common device tree include Date: Tue, 17 Feb 2026 15:51:44 +1000 Message-ID: In-Reply-To: <20260217000854.131242-6-mailingradian@gmail.com> References: <20260217000854.131242-1-mailingradian@gmail.com> <20260217000854.131242-6-mailingradian@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This is the main refactoring patch. The approach of extracting common elements into `sdm670-google-common.dtsi` and making per-device DTS files thin overlays is clean. > - charge-full-design-microamp-hours = <3000000>; The `charge-full-design-microamp-hours` property is removed from the common include and added back in the sargo-specific DTS with the same value. This makes sense since the 3a XL has a different battery capacity (3700 mAh vs 3000 mAh). > + framebuffer: framebuffer@9c000000 { > + compatible = "simple-framebuffer"; > + /* dimensions and size are in specific device trees */ > + format = "a8r8g8b8"; > + status = "disabled"; > }; The framebuffer is disabled in the common include and re-enabled with device-specific dimensions in each board DTS. However, `simple-framebuffer` nodes without `reg`, `width`, `height`, and `stride` properties are incomplete. Since the node is `status = "disabled"`, this is fine at runtime, but it may generate DT validation warnings. The `reg` property is also omitted from the common include but provided in the per-device overrides. > +&rmi4_f12 { > + touchscreen-x-mm = <63>; > + touchscreen-y-mm = <127>; > +}; The original sargo DTS had `touchscreen-x-mm = <62>`, but the new sargo DTS sets it to `<63>`. Is this an intentional correction, or a typo introduced during the refactoring? The commit message doesn't mention this change. If intentional, it would be worth noting in the commit message. > - panel: panel@0 { > - compatible = "samsung,s6e3fa7-ams559nk06"; > + panel: panel@0 { > + /* compatible is provided in specific device trees */ > reg = <0>; The panel node is left without a `compatible` and with `status = "disabled"` in the common include. This is correct -- the per-device DTS provides both the compatible and `status = "okay"`. > -// SPDX-License-Identifier: GPL-2.0 > +// SPDX-License-Identifier: GPL-2.0-only The SPDX license identifier changed from `GPL-2.0` to `GPL-2.0-only` in the new sargo DTS. While `GPL-2.0-only` is more precise, this is a license change that should probably be noted in the commit message or handled as a separate patch, as it changes the licensing terms. --- Generated by Claude Code Patch Reviewer