Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Daniel Carter" <yass27106@gmail.com>
Cc: linux-sound@vger.kernel.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v2 2/3] arm64: dts: qcom: add device tree for Asus Vivobook 16 X1607QA
Date: Fri, 13 Nov 2026 20:51:50 +0000	[thread overview]
Message-ID: <sashiko-outbox-213684@kernel.org> (raw)
In-Reply-To: <bee078b697b74da694b5f9e6f518c0fb@gmail.com>

Thank you for your contribution! Sashiko AI review found 6 potential
issues to consider:
- [High] arm64: dts: qcom: missing Signed-off-by trailer and commit
  message body
- [Medium] arm64: dts: qcom: missing /delete-node/ for pmc8380_6 on
  Purwa X1607QA
- [Medium] arm64: dts: qcom: missing qcom,dmic-sample-rate and
  vdd-micb-supply on vamacro
- [Medium] arm64: dts: qcom: missing x1-el2.dtbo target and
  &gpu_zap_shader reference
- [Medium] arm64: dts: qcom: x1p42100-asus-vivobook-16-x1607qa: wrong
  inherited eDP panel
- [Low] arm64: dts: qcom: split root node, unsorted nodes, and style
  violations in DTS
--

commit 0106da8e8437e65f83503717cf06104fc06ff24c
Author: Daniel Carter <yass27106@gmail.com>

> arm64: dts: qcom: add device tree for Asus Vivobook 16 X1607QA
> 
> arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../x1p42100-asus-vivobook-16-x1607qa.dts     | 237 ++++++++++++++++++
>  2 files changed, 238 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/x1p42100-asus-vivobook-16-x1607qa.dts

> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 88230c4e00ea9..7332a73dbac71 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -509,6 +509,7 @@ x1e80100-qcp-el2-dtbs	:= x1e80100-qcp.dtb x1-el2.dtbo
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-qcp.dtb x1e80100-qcp-el2.dtb
>  x1p42100-asus-vivobook-s15-el2-dtbs	:= x1p42100-asus-vivobook-s15.dtb x1-el2.dtbo
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1p42100-asus-vivobook-s15.dtb x1p42100-asus-vivobook-s15-el2.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= x1p42100-asus-vivobook-16-x1607qa.dtb
>  x1p42100-asus-zenbook-a14-el2-dtbs	:= x1p42100-asus-zenbook-a14.dtb x1-el2.dtbo
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1p42100-asus-zenbook-a14.dtb x1p42100-asus-zenbook-a14-el2.dtb

[Severity: Medium]
Should this also define x1p42100-asus-vivobook-16-x1607qa-el2-dtbs with
x1-el2.dtbo and build x1p42100-asus-vivobook-16-x1607qa-el2.dtb, sorted
alphabetically before x1p42100-asus-vivobook-s15?

scripts/Makefile.dtbs only passes -@ to DTC for base DTBs referenced by a
composite *-dtbs rule.  Without the -el2-dtbs rule, neither the pre-merged
-el2.dtb is built nor are __symbols__ emitted into the base DTB, so
x1-el2.dtbo cannot be applied for EL2/KVM boot.

In addition, at the bottom of x1p42100-asus-vivobook-16-x1607qa.dts, the
GPU zap-shader firmware is specified via a nested zap-shader child under
&gpu (along with a redundant status = "okay" already set in
x1-asus-vivobook-s15.dtsi) instead of referencing &gpu_zap_shader directly
as x1p42100-asus-vivobook-s15.dts and x1-el2.dtso do.

> diff --git a/arch/arm64/boot/dts/qcom/x1p42100-asus-vivobook-16-x1607qa.dts b/arch/arm64/boot/dts/qcom/x1p42100-asus-vivobook-16-x1607qa.dts
> new file mode 100644
> index 0000000000000..0092ae3db9cc8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1p42100-asus-vivobook-16-x1607qa.dts
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2026, yass27106-hue <yass27106@users.noreply.github.com>
> + */

[Severity: High]
The commit message is missing a Signed-off-by: trailer certifying the
Developer's Certificate of Origin, and its body contains a pasted git
diffstat instead of a description of the ASUS Vivobook 16 (X1607QA)
hardware and why it shares x1-asus-vivobook-s15.dtsi.  The copyright
header here also uses a GitHub handle and noreply address rather than the
author's name and routable email address.

> +
> +/dts-v1/;
> +
> +#include "purwa.dtsi"
> +#include "x1-asus-vivobook-s15.dtsi"

[Severity: Medium]
Does this need to delete &pmc8380_6, &pmc8380_6_thermal, and
&pmc8380_6_die_temp after including x1-asus-vivobook-s15.dtsi?

x1-asus-vivobook-s15.dtsi transitively includes hamoa-pmics.dtsi, which
defines pmc8380_6 (SPMI USID 6), pmc8380_6_thermal, and pmc8380_6_die_temp
for the third CPU cluster on 12-core X1E80100 SoCs.  On 8-core X1P42100
(Purwa) platforms that cluster and its companion PMIC are unpopulated, so
leaving these nodes enabled causes the SPMI PMIC, temp-alarm, GPIO, VADC,
and thermal drivers to probe and poll absent hardware, just as
x1p42100-asus-vivobook-s15.dts avoids with /delete-node/.

> +#include "x1-asus-vivobook-s15.dtsi"

[Severity: Medium]
Does the 16-inch Vivobook 16 (X1607QA) really use the 15.6-inch Samsung
ATNA56AC03 panel inherited from x1-asus-vivobook-s15.dtsi?

x1-asus-vivobook-s15.dtsi configures the mdss_dp3 aux-bus panel node with:

arch/arm64/boot/dts/qcom/x1-asus-vivobook-s15.dtsi:
	aux-bus {
		panel {
			compatible = "samsung,atna56ac03", "samsung,atna33xc20";
			enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
			power-supply = <&vreg_edp_3p3>;

Without overriding the panel node for this 16-inch (16:10) laptop, the
board binds the panel-samsung-atna33xc20 driver for a 15.6-inch 2880x1620
(16:9) AMOLED panel with Samsung EL_ON3 power sequencing and DP AUX-only
backlight control instead of the actual panel and backlight configuration.

[ ... ]

> +&lpass_vamacro {
> +	pinctrl-0 = <&dmic01_default>;
> +	pinctrl-names = "default";
> +};

[Severity: Medium]
Are qcom,dmic-sample-rate and vdd-micb-supply missing on &lpass_vamacro
(along with the vreg_l1b_1p8 LDO1 definition in &apps_rsc regulators-0)?

va_macro_probe() logs "qcom,dmic-sample-rate dt entry missing" when
qcom,dmic-sample-rate is omitted, and its "vdd-micb" DAPM regulator widget
falls back to a dummy regulator without enabling the 1.8V LDO1 DMIC micbias
rail unless vdd-micb-supply = <&vreg_l1b_1p8> is provided as on other X1
boards.

> +
> +/ {
> +	wcd938x: audio-codec {

[ ... ]

> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-ASUS-Vivobook-16";
> +		audio-routing = "SpkrLeft IN", "WSA WSA_SPK1 OUT",
> +						"SpkrRight IN", "WSA WSA_SPK2 OUT",

[ ... ]

> +&swr0 {
> +	status = "okay";
> +
> +	pinctrl-0 = <&wsa_swr_active>, <&spkr_01_sd_n_active>;
> +	pinctrl-names = "default";

[Severity: Low]
This isn't a bug, but several parts of this file diverge from the
devicetree coding style:

- The root node / { ... }; is split into two blocks with phandle references
  in between instead of a single root node block at the top of the file,
  and chassis-type = "laptop" duplicates x1-asus-vivobook-s15.dtsi.
- The top-level &label references, the DAI-link child nodes under sound,
  and the cpu/codec subnodes in wcd-playback-dai-link and
  wcd-capture-dai-link are not sorted alphabetically.
- In &swr0, status = "okay" is placed before pinctrl-0 and pinctrl-names
  instead of as the last property before child nodes.
- Multi-line property continuations in audio-routing, wsa-dai-link
  sound-dai, and the remoteproc firmware-name properties are aligned
  assuming 4-space tabs rather than 8-column kernel tabs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2c4a807496c04047b6d5735e3946dd16@gmail.com?part=2

  parent reply	other threads:[~2026-11-13 20:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 13:24 [PATCH 0/2] qcom: add Asus Vivobook 16 X1607QA DT + LPASS audio clock retry yass27106
2026-11-13 13:24 ` [PATCH 1/2] arm64: dts: qcom: add device tree for Asus Vivobook 16 X1607QA yass27106
2026-11-13 13:24   ` [PATCH 2/2] ASoC: qcom: lpass macro / soundwire: retry deferred clock acquisition yass27106
2026-11-13 13:43     ` sashiko-bot
2026-11-13 15:04     ` Mark Brown
2026-11-13 13:39   ` [PATCH 1/2] arm64: dts: qcom: add device tree for Asus Vivobook 16 X1607QA sashiko-bot
2026-11-13 15:44   ` Krzysztof Kozlowski
2026-11-13 20:33     ` [PATCH 1/2] arm64: dts: qcom: add device tree for Asus Vivobook 16 X1607QA — v2 series Daniel Carter
2026-11-13 20:33       ` [PATCH v2 1/3] dt-bindings: qcom: document Asus Vivobook 16 X1607QA Daniel Carter
2026-11-13 20:33         ` [PATCH v2 2/3] arm64: dts: qcom: add device tree for " Daniel Carter
2026-11-13 20:33           ` [PATCH v2 3/3] ASoC: qcom: lpass macro / soundwire: retry deferred clock acquisition Daniel Carter
2026-11-13 20:47             ` sashiko-bot
2026-11-13 20:51           ` sashiko-bot [this message]
2026-11-13 20:39         ` [PATCH v2 1/3] dt-bindings: qcom: document Asus Vivobook 16 X1607QA sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-213684@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yass27106@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox