Re: [PATCH v5 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 3 potential issue(s) to consider:
- [Medium] DP Alt Mode over USB-C is claimed as supported but lacks SBU mux, pmic-glink SBU endpoints, and DP PHY connections.
- [Medium] The touchscreen reset line is configured in pinctrl but lacks an active drive level, potentially leaving the device stuck in reset.
- [Medium] The pinctrl configuration for the touchpad interrupt incorrectly targets gpio3 instead of the interrupt's actual GPIO (gpio24).
--

commit 6ee378ed5847ae927be3c89dca68d6810e9c3f3e
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 based on the initial
work by Kirill A. Korinsky and Valentin Manea. The commit adds device tree
configurations for hardware components including display, audio, and USB.

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..79e49ba55e4b8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-honor-magicbook-art-14.dts

[ ... ]

> +	pmic-glink {
> +		compatible = "qcom,x1e80100-pmic-glink",
> +			     "qcom,sm8550-pmic-glink",
> +			     "qcom,pmic-glink";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
> +				    <&tlmm 123 GPIO_ACTIVE_HIGH>;
> +
> +		/* 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 {
> +					reg = <0>;
> +
> +					pmic_glink_ss0_hs_in: endpoint {
> +						remote-endpoint = <&usb_1_ss0_dwc3_hs>;
> +					};
> +				};

[Severity: Medium]
The commit message indicates DP over USB-C is supported, but does this
pmic-glink connector configuration omit the SBU endpoints needed for the
DisplayPort AUX channel?

A standard DP over USB-C implementation typically requires a port@2 mapping
to an SBU mux to handle link training.

Without the SBU routing, will DisplayPort Alt Mode fail to negotiate?

[ ... ]

> +&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>;

[Severity: Medium]
This sets the touchpad interrupt to GPIO 24, but the tpad_default pinctrl
state configured below targets GPIO 3 instead.

Is there a mismatch here that will leave GPIO 24 unconfigured and
potentially cause missing or spurious touchpad interrupts?

[ ... ]

> +&mdss_dp0 {
> +	status = "okay";
> +};
> +
> +&mdss_dp0_out {
> +	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> +};

[Severity: Medium]
The mdss_dp0 and mdss_dp1 outputs appear to be isolated here.

Should they include endpoint connections linking them to the QMP PHYs
(such as usb_1_ss0_qmpphy)? Without these remote-endpoint links, it seems the
DisplayPort outputs are structurally disconnected.

[ ... ]

> +&tlmm {
> +	gpio-reserved-ranges = <34 2>, /* Unused */
> +			       <44 4>; /* SPI (TPM) */
> +
> +	tpad_default: tpad-default-state {
> +		pins = "gpio3";
> +		function = "gpio";
> +		bias-disable;
> +	};

[Severity: Medium]
Does this pinctrl state incorrectly target GPIO 3 instead of the touchpad's
actual interrupt line, which is declared as GPIO 24 in the touchpad@5d node
above?

[ ... ]

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

[Severity: Medium]
Should this reset pin configuration include an output-high; or bias-pull-up;
property?

Since hid-over-i2c does not natively manage reset-gpios, relying entirely
on this pinctrl state without actively driving the line high might leave
the touchscreen stuck in reset.

[ ... ]

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