Re: [PATCH v4 3/4] arm64: dts: qcom: Add HONOR MagicBook Art 14 device tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Mismatch between touchpad interrupt GPIO and its pinctrl configuration.
- [Medium] Type-C SBU (Sideband Use) routing is omitted from the USB-C connectors in `pmic-glink`, breaking the DisplayPort Alt Mode AUX channel.
- [Medium] QMP DisplayPort PHY is directly connected to `hdmi-connector` without an intervening DP-to-HDMI bridge node.
- [Medium] Both WooferRight and TweeterRight are routed to the same 'WSA2 WSA_SPK2 OUT' source widget.
- [Medium] The touchscreen reset pin (gpio48) is configured in pinctrl but never driven to an active output state.
--

commit 856d8e361f3fb8f4f8c9dce853782183cccf8f2e
Author: Konstantin Shabanov <mail@etehtsea.me>

arm64: dts: qcom: Add HONOR MagicBook Art 14 device tree
    
Introduce support for the HONOR MagicBook Art 14 laptop.
This version is based on the initial work by Kirill A. Korinsky
and Valentin Manea.

Link: https://lore.kernel.org/all/871px910m1.wl-kirill@korins.ky/
Link: https://github.com/vamanea/linux-magicbook/blob/x1e80100-magicbook-6.19/arch/arm64/boot/dts/qcom/x1e80100-honor-magicbook-art-14.dts
Link: https://github.com/alsa-project/alsa-ucm-conf/pull/755

> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-honor-magicbook-art-14.dts b/arch/arm64/boot/dts/qcom/x1e80100-honor-magicbook-art-14.dts
> new file mode 100644
> index 0000000000000..b3113a0ee9283
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-honor-magicbook-art-14.dts
[ ... ]
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		pinctrl-0 = <&hdmi_hpd_default>;
> +		pinctrl-names = "default";
> +
> +		port {
> +			hdmi_con: endpoint {
> +				remote-endpoint = <&usb_1_ss2_qmpphy_out_dp>;
> +			};
> +		};
> +	};

[Severity: Medium]
Is this missing an active DP-to-HDMI bridge node? The Snapdragon QMP DP PHY
outputs native DisplayPort signaling, but the remote-endpoint is directly
connected to an hdmi-connector node here.

Because the SoC does not support native TMDS (HDMI) output, this bypasses the
necessary hardware bridge, which can leave the bridge unmanaged by the kernel.

[ ... ]
> +	pmic-glink {
> +		compatible = "qcom,x1e80100-pmic-glink",
[ ... ]
> +		/* Left-side port, closer to the screen */
> +		connector@0 {
> +			compatible = "usb-c-connector";
> +			reg = <0>;
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
[ ... ]
> +				port@1 {
[ ... ]
> +			};
> +		};

[Severity: Medium]
Does this routing omit the Type-C SBU (Sideband Use) ports? DisplayPort Alt
Mode requires the SBU lines for the AUX channel.

Without a port@2 node for SBU routing, the orientation switch cannot correctly
flip the AUX channel lines, which will likely break DisplayPort link training.

[ ... ]
> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-HONOR-MagicBook-Art-14-Snapdragon";
> +		audio-routing = "WooferLeft IN", "WSA WSA_SPK1 OUT",
> +				"TweeterLeft IN", "WSA WSA_SPK2 OUT",
> +				"WooferRight IN", "WSA2 WSA_SPK2 OUT",
> +				"TweeterRight IN", "WSA2 WSA_SPK2 OUT",

[Severity: Medium]
Are both the WooferRight and TweeterRight meant to map to the exact same
output source widget (WSA2 WSA_SPK2 OUT)? This leaves WSA2 WSA_SPK1 OUT
unused and might prevent the right audio channels from mixing and playing
properly.

[ ... ]
> +&i2c1 {
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	touchpad@5d {
> +		compatible = "hid-over-i2c";
> +		reg = <0x5d>;
> +
> +		hid-descr-addr = <0x1>;
> +		interrupts-extended = <&tlmm 24 IRQ_TYPE_LEVEL_LOW>;
[ ... ]
> +		pinctrl-0 = <&tpad_default>;
> +		pinctrl-names = "default";

[Severity: High]
Could there be a mismatch between the interrupt GPIO and the pinctrl
configuration here?

The interrupts-extended property declares the interrupt on gpio24, but the
tpad_default node configured below targets gpio3:

[ ... ]
> +&tlmm {
[ ... ]
> +	tpad_default: tpad-default-state {
> +		pins = "gpio3";
> +		function = "gpio";
> +		bias-disable;
> +	};

This might lead to an improperly biased interrupt line for the touchpad,
resulting in spurious interrupts or failure of touchpad input.

[ ... ]
> +	ts0_default: ts0-default-state {
> +		reset-n-pins {
> +			pins = "gpio48";
> +			function = "gpio";
> +			drive-strength = <16>;
> +		};

[Severity: Medium]
Should this reset pin be actively driven high, or managed by the hid-over-i2c
touchscreen driver?

The pin is configured in pinctrl but lacks an explicit state (like output-high)
and is not assigned to a reset-gpios property in the touchscreen node. This
could leave the GPIO floating or in a bootloader-assigned state, potentially
causing the touchscreen to fail initialization or sporadically reset.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628094031.54452-1-mail@etehtsea.me?part=3