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

[Gopikrishna Garmidi <gopikrishna.garmidi@oss.qualcomm.com>]

On 10/27/2026 1:17 PM, sashiko-bot@kernel.org wrote:
> 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 },
> 	{ },
> };
> 
> [...]
> 
> 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.
> 

Thanks for the review.

The wrong SAM compatible in surface_aggregator_registry.c is fixed
separately here:
https://lore.kernel.org/all/20261027-surface-laptop-8-sam-compatible-v1-1-022cc782b0f4@oss.qualcomm.com/

>> +
>> [...]
>> +
>> +	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:
> 
> [...]
> 
> 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.
> 
> [ ... ]
> 

Right, uart22 is the SAM link, not a debug console. I'll drop 
stdout-path in v3, same as the other Surface DTs.

>> +&pmk8850_rtc {
>> +	qcom,no-alarm;
>> +};
> 
> [Severity: Medium]
> Is qcom,uefi-rtc-info missing from pmk8850_rtc?
> 
> [...]
> 
> 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.
> 

Agreed, I'll add qcom,uefi-rtc-info to pmk8850_rtc in v3 so the RTC
picks up the UEFI offset, like the other Glymur/Mahua laptops.


Thanks,
Gopikrishna