Re: [PATCH v3 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 6 potential issue(s) to consider:
- [High] Missing SBU routing and multiplexers for DisplayPort Alternate Mode over USB-C.
- [High] Missing DisplayPort and HDMI audio routing in the sound card.
- [High] Duplicate audio routing for Right Woofer and Right Tweeter breaks channel separation.
- [Medium] QMP DP PHY output directly connected to hdmi-connector without an HDMI bridge.
- [Low] Invalid `vddl-supply` property in generic `hid-over-i2c` nodes.
- [Low] Invalid property placement: `dr_mode = "host"` placed on DWC3 wrapper instead of core.
--

commit c44972cad74464a55d079d27d560d3a648a72db6
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 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..c472d844429b3
> --- /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]
The QMP combo PHY natively outputs DisplayPort signaling, but this
hdmi-connector node strictly expects TMDS signaling.

Since the X1E80100 DP blocks lack DP++ support, does this direct connection
violate DRM DT graph bindings by omitting the active DP-to-HDMI bridge
needed to convert the signal?

[ ... ]

> +	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>;
> +					};
> +				};
> +
> +				port@1 {

[Severity: High]
Is it possible that the SBU routing is missing here?

The commit claims support for DisplayPort over USB-C, but this configuration
omits port@2 for the SBU endpoints, and there are no SBU multiplexers or
retimers defined.

Will this prevent the UCSI Alt Mode driver from routing SBU and break
DisplayPort Alternate Mode?

[ ... ]

> +	sound {
> +		compatible = "qcom,x1e80100-sndcard";
> +		model = "X1E80100-HONOR-MagicBook-Art-14-Snapdragon";

[Severity: High]
The commit mentions support for HDMI and DP over USB-C, but this sound node
omits the displayport-X-dai-link blocks.

Without these DAI links, won't audio routing to external monitors over
DisplayPort and HDMI fail?

> +		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: High]
Does mapping both WooferRight and TweeterRight to WSA2 WSA_SPK2 OUT
accidentally leave the WSA2 WSA_SPK1 OUT widget unused?

If the first channel of the WSA2 macro is never powered on, could this
break channel separation or cause the right woofer to fail?

[ ... ]

> +	touchpad@5d {
> +		compatible = "hid-over-i2c";
> +		reg = <0x5d>;
> +
> +		hid-descr-addr = <0x1>;
> +		interrupts-extended = <&tlmm 24 IRQ_TYPE_LEVEL_LOW>;
> +
> +		vdd-supply = <&vreg_misc_3p3>;
> +		vddl-supply = <&vreg_l12b_1p2>;

[Severity: Low]
According to the hid-over-i2c dt-bindings, the driver parses vddio-supply
rather than vddl-supply.

Will this trigger dtbs_check errors and cause the upstream i2c-hid-of
driver to ignore the supply? Should this be renamed to vddio-supply?

[ ... ]

> +	keyboard@3a {
> +		compatible = "hid-over-i2c";
> +		reg = <0x3a>;
> +
> +		hid-descr-addr = <0x1>;
> +		interrupts-extended = <&tlmm 67 IRQ_TYPE_LEVEL_LOW>;
> +
> +		vdd-supply = <&vreg_misc_3p3>;
> +		vddl-supply = <&vreg_l12b_1p2>;

[Severity: Low]
Similarly, should vddl-supply be renamed to vddio-supply here for the keyboard
to conform to the hid-over-i2c dt-bindings?

[ ... ]

> +&usb_1_ss0 {
> +	dr_mode = "host";
> +
> +	status = "okay";
> +};

[Severity: Low]
The dr_mode property is placed on the DWC3 wrapper node rather than the
core node (e.g., usb_1_ss0_dwc3).

Since the core DWC3 driver parses dr_mode exclusively from its own node,
won't this be ignored and cause the core to fall back to OTG mode?

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