Re: [PATCH v2 3/7] platform/surface: SAM: Add support for Surface Pro 12in

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect software node used for the pen stash device, breaking pen stash events.
--

commit 66a330e4ed95d1a03b167abb4b106aaef584a66f
Author: Harrison Vanderbyl <harrison.vanderbyl@gmail.com>

platform/surface: SAM: Add support for Surface Pro 12in

This commit adds a SAM client device node group and registry entry for the
Microsoft Surface Pro 12-inch. It specifically mentions enabling the cover
keyboard, cover touchpad, and pen stash events.

> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> index 0599d5adf02e6..8840499614154 100644
> --- a/drivers/platform/surface/surface_aggregator_registry.c
> +++ b/drivers/platform/surface/surface_aggregator_registry.c
[ ... ]
> @@ -422,6 +422,19 @@ static const struct software_node *ssam_node_group_sp11[] = {
>  	NULL,
>  };
>  
> +/* Devices for Surface Pro 12" first edition (ARM/QCOM) */
> +static const struct software_node *ssam_node_group_sp12in[] = {
> +	&ssam_node_root,
> +	&ssam_node_hub_kip,
> +	&ssam_node_tmp_sensors,
> +	&ssam_node_hid_kip_keyboard,
> +	&ssam_node_hid_sam_penstash,

Will this software node work correctly for the Surface Pro pen stash events?

Since the pen is typically stashed inside the detachable cover keyboard on
Surface Pro devices, its events are usually routed through the Keyboard
Interface Protocol (KIP) hub.

Other Surface Pro models, like the SP11, use ssam_node_hid_kip_penstash
instead. Should this use the KIP variant here as well? Using the SAM
variant might cause the driver to listen on the wrong target ID and miss
the pen stash events entirely.

> +	&ssam_node_hid_kip_touchpad,
> +	&ssam_node_hid_kip_fwupd,
> +	&ssam_node_pos_tablet_switch,
> +	NULL,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778822464.git.harrison.vanderbyl@gmail.com?part=3