Re: [PATCH 2/2] arm64: dts: qcom: Add HP OmniBook X 16-cp1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: missing data-lanes on usb_2_qmpphy in qmp_combo_probe()
--

commit 445e95f688e19265eba79721f7454f1310f9d5f6
Author: Jeff Roach <jeff.roach0@gmail.com>

arm64: dts: qcom: Add HP OmniBook X 16-cp1

This commit adds a device tree for the HP OmniBook X 16-cp1 laptop based on
the Qualcomm Mahua (Snapdragon X2 Elite) SoC, enabling keyboard, touchpad,
touchscreen, PCIe NVMe and Wi-Fi, USB-C and USB-A ports, eDP panel, remoteprocs,
and audio.

> diff --git a/arch/arm64/boot/dts/qcom/mahua-hp-omnibook-x16.dts b/arch/arm64/boot/dts/qcom/mahua-hp-omnibook-x16.dts
> new file mode 100644
> index 0000000000000..94882596d83e4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/mahua-hp-omnibook-x16.dts

[ ... ]

> +/* USB-A port on the right side */
> +&usb_2 {
> +	dr_mode = "host";
> +
> +	/delete-property/ usb-role-switch;
> +
> +	status = "okay";
> +};
> +
> +&usb_2_hsphy {
> +	vdd-supply = <&vreg_l4c_e1_0p91>;
> +	vdda12-supply = <&vreg_l4f_e1_1p2>;
> +
> +	status = "okay";
> +};
> +
> +&usb_2_qmpphy {
> +	vdda-phy-supply = <&vreg_l4c_e1_0p91>;
> +	vdda-pll-supply = <&vreg_l4f_e1_1p2>;
> +	refgen-supply = <&vreg_l1c_e1_0p91>;
> +
> +	/delete-property/ mode-switch;
> +	/delete-property/ orientation-switch;
> +
> +	status = "okay";
> +};

[Severity: Medium]
Should usb_2_qmpphy also define a static USB3 data-lanes mapping under
ports/port@0/endpoint@1 when mode-switch and orientation-switch are deleted?

In arch/arm64/boot/dts/qcom/glymur.dtsi, usb_2_qmpphy only defines an empty
unnumbered endpoint under port@0 without data-lanes:

	usb_2_qmpphy: phy@88e1000 {
		...
		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;

				usb_2_qmpphy_out: endpoint {
				};
			};

When neither mode-switch nor orientation-switch is present,
drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_probe() falls back to
reading static lane mappings from port@0/endpoint@0 (for DP) and
port@0/endpoint@1 (for USB3):

	if (of_property_present(dev->of_node, "mode-switch") ||
	    of_property_present(dev->of_node, "orientation-switch")) {
		ret = qmp_combo_typec_register(qmp);
		if (ret)
			goto err_node_put;
	} else {
		enum typec_orientation dp_orientation = TYPEC_ORIENTATION_NONE;
		enum typec_orientation usb3_orientation = TYPEC_ORIENTATION_NONE;

		ret = qmp_combo_get_dt_dp_orientation(dev, &dp_orientation);
		if (ret)
			goto err_node_put;

		ret = qmp_combo_get_dt_usb3_orientation(dev, &usb3_orientation);
		if (ret)
			goto err_node_put;
		...
		} else {
			dev_warn(dev, "unable to determine orientation & mode from data-lanes");
		}
	}

Without replacing usb_2_qmpphy_out with endpoint@1 containing data-lanes
(such as <1 0> or <2 3>), both dp_orientation and usb3_orientation remain
TYPEC_ORIENTATION_NONE. Does this cause qmp_combo_probe() to emit the warning
above, leave qmp->qmpphy_mode at QMPPHY_MODE_USB3DP instead of
QMPPHY_MODE_USB3_ONLY, and leave qmp->orientation unset?

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