Re: [PATCH v2 3/3] arm64: dts: qcom: Add Microsoft Surface Laptop 8th Edition

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: qcom: SAM compatible mismatch breaks ssam_platform_hub_probe()
- [Medium] arm64: dts: qcom: stdout-path points to Surface SAM EC UART on uart22
- [Medium] arm64: dts: qcom: missing qcom,uefi-rtc-info property on pmk8850_rtc
--

commit dc92104026c28877c07bc41ec0441b8b4cc8661a
Author: Gopikrishna Garmidi <gopikrishna.garmidi@oss.qualcomm.com>

arm64: dts: qcom: Add Microsoft Surface Laptop 8th Edition

This commit adds the device tree for the Microsoft Surface Laptop 13.8" 8th
Edition based on the Qualcomm Mahua SoC.

> diff --git a/arch/arm64/boot/dts/qcom/mahua-microsoft-surface-laptop-8-13p8.dts b/arch/arm64/boot/dts/qcom/mahua-microsoft-surface-laptop-8-13p8.dts
> new file mode 100644
> index 0000000000000..9acba318ab52b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/mahua-microsoft-surface-laptop-8-13p8.dts
> @@ -0,0 +1,1176 @@

[ ... ]

> +/ {
> +	model = "Microsoft Surface Laptop 8th Edition (13.8 inch)";
> +	compatible = "microsoft,surface-laptop-8-13p8", "qcom,mahua";

[Severity: High]
Will ssam_platform_hub_probe() fail to match this root compatible string?

When ssam_serial_hub_probe() probes the microsoft,surface-sam node on
uart22, it registers surface_aggregator_platform_hub. In
drivers/platform/surface/surface_aggregator_registry.c,
ssam_platform_hub_probe() looks up the board's software_node group against
ssam_platform_hub_of_match:

drivers/platform/surface/surface_aggregator_registry.c:
static const struct of_device_id ssam_platform_hub_of_match[] = {
	...
	/* Surface Laptop 8th Edition (13.8") */
	{ .compatible = "microsoft,surface-laptop-8-13in", (void *)ssam_node_group_sl7 },
	{ },
};

static int ssam_platform_hub_probe(struct platform_device *pdev)
{
	...
	nodes = (const struct software_node **)acpi_device_get_match_data(&pdev->dev);
	if (!nodes) {
		nodes = (const struct software_node **)of_machine_get_match_data(ssam_platform_hub_of_match);
		if (!nodes)
			return -ENODEV;
	}

Because ssam_platform_hub_of_match checks for
microsoft,surface-laptop-8-13in while the root node here uses
microsoft,surface-laptop-8-13p8, of_machine_get_match_data() returns NULL
and ssam_platform_hub_probe() fails with -ENODEV, leaving the keyboard, fan,
and platform profile unregistered.

> +
> +	aliases {
> +		i2c0 = &i2c0;
> +		i2c4 = &i2c4;
> +		i2c5 = &i2c5;
> +		i2c8 = &i2c8;
> +		i2c10 = &i2c10;
> +
> +		serial0 = &uart22;
> +		serial1 = &uart14;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};

[Severity: Medium]
Should stdout-path be omitted here since serial0 points to uart22?

In this device tree, uart21 (the debug UART) is disabled and uart22 is
dedicated to the Surface Aggregator Module embedded controller at 4000000
baud:

&uart22 {
	status = "okay";

	embedded-controller {
		compatible = "microsoft,surface-sam";
		...
		current-speed = <4000000>;
		...
	};
};

Setting stdout-path to serial0:115200n8 causes of_alias_scan() to resolve
of_stdout to uart22 and directs any earlycon, bootloader, or userspace
console consumer honoring /chosen/stdout-path to reconfigure uart22 to
115200 8N1 and write console output to the binary SAM EC link. Other
Qualcomm-based Surface device trees omit stdout-path.

[ ... ]

> +&pmk8850_rtc {
> +	qcom,no-alarm;
> +};

[Severity: Medium]
Is qcom,uefi-rtc-info missing from pmk8850_rtc?

Commit 823c3b6a6743 ("firmware: qcom: scm: Allow QSEECOM on Surface Laptop
8th Edition") added microsoft,surface-laptop-8-13p8 to
qcom_scm_qseecom_allowlist so the RTC driver can access the RTC offset
stored in UEFI variables.

However, pmk8850.dtsi does not set qcom,uefi-rtc-info on pmk8850_rtc, and
pmk8850_rtc here only sets qcom,no-alarm without nvmem-cells or
qcom,uefi-rtc-info. In drivers/rtc/rtc-pm8xxx.c,
pm8xxx_rtc_probe() -> pm8xxx_rtc_probe_offset() checks:

	/* Use UEFI storage as fallback if available */
	rtc_dd->use_uefi = of_property_read_bool(rtc_dd->dev->of_node,
						 "qcom,uefi-rtc-info");
	if (!rtc_dd->use_uefi)
		return 0;

Without qcom,uefi-rtc-info, rtc_dd->use_uefi remains false,
pm8xxx_rtc_read_time() returns the unadjusted PMIC counter value, and
pm8xxx_rtc_set_time() -> pm8xxx_rtc_update_offset() fails with -ENODEV.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-mahua-surface-laptop-v2-0-7dd903a12d22@oss.qualcomm.com?part=3