| Message ID | 20240908223505.21011-4-jerome.debretagne@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Microsoft Surface Pro 9 5G support | expand |
Looks good. Two very small nit-picks below, if this goes for a v3: On 9/9/24 12:35 AM, Jérôme de Bretagne wrote: > Add SAM client device nodes for the Surface Pro 9 5G, with the usual > battery/AC and HID nodes for keyboard and touchpad support. > > Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com> > --- > .../surface/surface_aggregator_registry.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c > index 25c8aa2131d6..8b34d7e465c2 100644 > --- a/drivers/platform/surface/surface_aggregator_registry.c > +++ b/drivers/platform/surface/surface_aggregator_registry.c > @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = { > NULL, > }; > > +/* Devices for Surface Pro 9 5G. */ Would be nice if you could change the comment on the SP9 node group to "Surface Pro 9 (Intel/x86)" and the comment here to "Surface Pro 9 5G (ARM/QCOM)" or something along those lines to make things a bit more clear. > +static const struct software_node *ssam_node_group_sp9_5G[] = { (This is really just me being a bit obsessive:) It would be nice to have all-lowercase variable names (regarding the 5G). > + &ssam_node_root, > + &ssam_node_hub_kip, > + &ssam_node_bat_ac, > + &ssam_node_bat_main, > + &ssam_node_tmp_sensors, > + &ssam_node_hid_kip_keyboard, > + &ssam_node_hid_kip_penstash, > + &ssam_node_hid_kip_touchpad, > + &ssam_node_hid_kip_fwupd, > + &ssam_node_hid_sam_sensors, > + &ssam_node_kip_tablet_switch, > + NULL, > +}; > > /* -- SSAM platform/meta-hub driver. ---------------------------------------- */ > > @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = { > MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match); > > static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = { > + /* Surface Pro 9 5G */ > + { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G }, > /* Surface Laptop 7 */ > { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 }, > { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 }, Thanks! Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> On a related note: I'm wondering whether we should also already add the ACPI IDs for the as-of-now ARM-only devices. Technically, things won't work with ACPI right now, but it's not because of the SAM driver stack. Or maybe at least add them to the comments as a way of documenting it. Best regards, Max
diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c index 25c8aa2131d6..8b34d7e465c2 100644 --- a/drivers/platform/surface/surface_aggregator_registry.c +++ b/drivers/platform/surface/surface_aggregator_registry.c @@ -390,6 +390,21 @@ static const struct software_node *ssam_node_group_sp9[] = { NULL, }; +/* Devices for Surface Pro 9 5G. */ +static const struct software_node *ssam_node_group_sp9_5G[] = { + &ssam_node_root, + &ssam_node_hub_kip, + &ssam_node_bat_ac, + &ssam_node_bat_main, + &ssam_node_tmp_sensors, + &ssam_node_hid_kip_keyboard, + &ssam_node_hid_kip_penstash, + &ssam_node_hid_kip_touchpad, + &ssam_node_hid_kip_fwupd, + &ssam_node_hid_sam_sensors, + &ssam_node_kip_tablet_switch, + NULL, +}; /* -- SSAM platform/meta-hub driver. ---------------------------------------- */ @@ -462,6 +477,8 @@ static const struct acpi_device_id ssam_platform_hub_acpi_match[] = { MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_acpi_match); static const struct of_device_id ssam_platform_hub_of_match[] __maybe_unused = { + /* Surface Pro 9 5G */ + { .compatible = "microsoft,arcata", (void *)ssam_node_group_sp9_5G }, /* Surface Laptop 7 */ { .compatible = "microsoft,romulus13", (void *)ssam_node_group_sl7 }, { .compatible = "microsoft,romulus15", (void *)ssam_node_group_sl7 },
Add SAM client device nodes for the Surface Pro 9 5G, with the usual battery/AC and HID nodes for keyboard and touchpad support. Signed-off-by: Jérôme de Bretagne <jerome.debretagne@gmail.com> --- .../surface/surface_aggregator_registry.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)