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: Add Microsoft Surface Pro 12in Date: Sat, 16 May 2026 09:57:54 +1000 Message-ID: In-Reply-To: <8ac29ee38ba80a3fbde8bfe43b74b9b936b31cb1.1778822464.git.harrison.vanderbyl@gmail.com> References: <8ac29ee38ba80a3fbde8bfe43b74b9b936b31cb1.1778822464.git.harrison.vanderbyl@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review This is the main board DTS file (1201 lines). The overall structure follows= the established pattern for x1p42100 boards. A few style issues: **Issue 1 =E2=80=94 Indentation in `&pm8550_gpios` uses spaces instead of t= abs:** ```dts +&pm8550_gpios { + vol_up_n_default: vol-up-n-state { + pins =3D "gpio8"; ``` and: ```dts + vol_down_n_default: vol-down-n-state { + pins =3D "gpio6"; ``` The `vol_up_n_default` and `vol_down_n_default` lines use 4 spaces for inde= ntation, while the rest of the file (and DTS convention) uses tabs. This sh= ould be fixed. **Issue 2 =E2=80=94 C++ style comment in i2c9:** ```dts +&i2c9 { + clock-frequency =3D <400000>; + + // NFC @28, commercial devices only ``` DTS files conventionally use C-style `/* */` comments. Some DT maintainers = flag `//` comments. The `i2c4` section correctly uses `/* */` style comment= s for similar device annotations. **Issue 3 =E2=80=94 Extra whitespace in ts0_default closing brace:** ```dts + ts0_default: ts0-default-state { + ... + reset-n-pins { + ... + }; + }; ``` The closing `};` has an extra space before it (`\t }`), making it misaligne= d compared to the opening. **Minor observations (non-blocking):** - The `chassis-type =3D "tablet"` is correct for this form factor. - The two USB-C connectors are defined with correct pmic-glink wiring and S= BU mux configuration. - The `reserved-memory` CMA region at 512MB is typical for Qualcomm ARM64 b= oards. - The regulator chain `vreg_panel_en` -> `vreg_edp_3p3` -> backlight/panel = is logical with the 150ms enable ramp delay. - The `vreg_l7b_2p8` has `regulator-always-on` which should be verified as = intentional. - The WSA8845 speakers both share the same `reset-gpios` (lpass_tlmm gpio12= ), which is the expected configuration for paired speakers. - Firmware paths use `qcom/x1p42100/Microsoft/Surface12/` which is a sensib= le naming convention. **Summary for patch 7:** Three style issues (spaces vs tabs, comment style,= extra whitespace) should be fixed before merge. The functional content of = the DTS looks correct. --- Generated by Claude Code Patch Reviewer