Re: [PATCH 1/6] platform/surface: Set up Surface Aggregator device registry

From: Maximilian Luz
Date: Mon Feb 08 2021 - 19:32:04 EST




On 2/8/21 8:35 PM, Maximilian Luz wrote:
The Surface System Aggregator Module (SSAM) subsystem provides various
functionalities, which are separated by spreading them across multiple
devices and corresponding drivers. Parts of that functionality / some of
those devices, however, can (as far as we currently know) not be
auto-detected by conventional means. While older (specifically 5th- and
6th-)generation models do advertise most of their functionality via
standard platform devices in ACPI, newer generations do not.

As we are currently also not aware of any feasible way to query said
functionalities dynamically, this poses a problem. There is, however, a
device in ACPI that seems to be used by Windows for identifying
different Surface models: The Windows Surface Integration Device (WSID).
This device seems to have a HID corresponding to the overall set of
functionalities SSAM provides for the associated model.

This commit introduces a registry providing non-detectable device
information via software nodes. In addition, a SSAM platform hub driver
is introduced, which takes care of creating and managing the SSAM
devices specified in this registry. This approach allows for a
hierarchical setup akin to ACPI and is easily extendable, e.g. via
firmware node properties.

Note that this commit only provides the basis for the platform hub and
registry, and does not add any content to it. The registry will be
expanded in subsequent commits.

Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx>
---
MAINTAINERS | 1 +
drivers/platform/surface/Kconfig | 26 ++
drivers/platform/surface/Makefile | 1 +
.../surface/surface_aggregator_registry.c | 284 ++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 drivers/platform/surface/surface_aggregator_registry.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4fcf3df517a8..000a82f59c76 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11826,6 +11826,7 @@ F: Documentation/driver-api/surface_aggregator/
F: drivers/platform/surface/aggregator/
F: drivers/platform/surface/surface_acpi_notify.c
F: drivers/platform/surface/surface_aggregator_cdev.c
+F: drivers/platform/surface/surface_aggregator_registry.c
F: include/linux/surface_acpi_notify.h
F: include/linux/surface_aggregator/
F: include/uapi/linux/surface_aggregator/
diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 0847b2dc97bf..1cd37c041710 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -77,6 +77,32 @@ config SURFACE_AGGREGATOR_CDEV
The provided interface is intended for debugging and development only,
and should not be used otherwise.
+config SURFACE_AGGREGATOR_REGISTRY
+ tristate "Surface System Aggregator Module Device Registry"
+ depends on SURFACE_AGGREGATOR_BUS

Just noticed that there should also be a

depends on SURFACE_AGGREGATOR

here as SURFACE_AGGREGATOR_BUS is a bool. Without that, one could set

CONFIG_SURFACE_AGGREGATOR=m
CONFIG_SURFACE_AGGREGATOR_BUS=y
CONFIG_SURFACE_AGGREGATOR_REGISTRY=y

which will probably cause the compiler to complain that it can't find
the functions from the Aggregator module.

I'll leave it up as-is for a bit longer to maybe gather a couple more
comments before I send out a v2 with this fixed.

+ help
+ Device-registry and device-hubs for Surface System Aggregator Module
+ (SSAM) devices.
+
+ Provides a module and driver which act as a device-registry for SSAM
+ client devices that cannot be detected automatically, e.g. via ACPI.
+ Such devices are instead provided via this registry and attached via
+ device hubs, also provided in this module.
+
+ Devices provided via this registry are:
+ - Platform profile (performance-/cooling-mode) device (5th- and later
+ generations).
+ - Battery/AC devices (7th-generation).
+ - HID input devices (7th-generation).
+
+ Select M (recommended) or Y here if you want support for the above
+ mentioned devices on the corresponding Surface models. Without this
+ module, the respective devices will not be instantiated and thus any
+ functionality provided by them will be missing, even when drivers for
+ these devices are present. In other words, this module only provides
+ the respective client devices. Drivers for these devices still need to
+ be selected via the other options.
+
config SURFACE_GPE
tristate "Surface GPE/Lid Support Driver"
depends on DMI
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 990424c5f0c9..80035ee540bf 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_SURFACE_3_POWER_OPREGION) += surface3_power.o
obj-$(CONFIG_SURFACE_ACPI_NOTIFY) += surface_acpi_notify.o
obj-$(CONFIG_SURFACE_AGGREGATOR) += aggregator/
obj-$(CONFIG_SURFACE_AGGREGATOR_CDEV) += surface_aggregator_cdev.o
+obj-$(CONFIG_SURFACE_AGGREGATOR_REGISTRY) += surface_aggregator_registry.o
obj-$(CONFIG_SURFACE_GPE) += surface_gpe.o
obj-$(CONFIG_SURFACE_HOTPLUG) += surface_hotplug.o
obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o
diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
new file mode 100644
index 000000000000..a051d941ad96
--- /dev/null
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Surface System Aggregator Module (SSAM) client device registry.
+ *
+ * Registry for non-platform/non-ACPI SSAM client devices, i.e. devices that
+ * cannot be auto-detected. Provides device-hubs and performs instantiation
+ * for these devices.
+ *
+ * Copyright (C) 2020-2021 Maximilian Luz <luzmaximilian@xxxxxxxxx>
+ */
+
+#include <linux/acpi.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#include <linux/surface_aggregator/controller.h>
+#include <linux/surface_aggregator/device.h>
+
+
+/* -- Device registry. ------------------------------------------------------ */
+
+/*
+ * SSAM device names follow the SSAM module alias, meaning they are prefixed
+ * with 'ssam:', followed by domain, category, target ID, instance ID, and
+ * function, each encoded as two-digit hexadecimal, separated by ':'. In other
+ * words, it follows the scheme
+ *
+ * ssam:dd:cc:tt:ii:ff
+ *
+ * Where, 'dd', 'cc', 'tt', 'ii', and 'ff' are the two-digit hexadecimal
+ * values mentioned above, respectively.
+ */
+
+/* Root node. */
+static const struct software_node ssam_node_root = {
+ .name = "ssam_platform_hub",
+};
+
+/* Devices for Surface Book 2. */
+static const struct software_node *ssam_node_group_sb2[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Book 3. */
+static const struct software_node *ssam_node_group_sb3[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Laptop 1. */
+static const struct software_node *ssam_node_group_sl1[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Laptop 2. */
+static const struct software_node *ssam_node_group_sl2[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Laptop 3. */
+static const struct software_node *ssam_node_group_sl3[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Laptop Go. */
+static const struct software_node *ssam_node_group_slg1[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Pro 5. */
+static const struct software_node *ssam_node_group_sp5[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Pro 6. */
+static const struct software_node *ssam_node_group_sp6[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+/* Devices for Surface Pro 7. */
+static const struct software_node *ssam_node_group_sp7[] = {
+ &ssam_node_root,
+ NULL,
+};
+
+
+/* -- Device registry helper functions. ------------------------------------- */
+
+static int ssam_uid_from_string(const char *str, struct ssam_device_uid *uid)
+{
+ u8 d, tc, tid, iid, fn;
+ int n;
+
+ n = sscanf(str, "ssam:%hhx:%hhx:%hhx:%hhx:%hhx", &d, &tc, &tid, &iid, &fn);
+ if (n != 5)
+ return -EINVAL;
+
+ uid->domain = d;
+ uid->category = tc;
+ uid->target = tid;
+ uid->instance = iid;
+ uid->function = fn;
+
+ return 0;
+}
+
+static int ssam_hub_remove_devices_fn(struct device *dev, void *data)
+{
+ if (!is_ssam_device(dev))
+ return 0;
+
+ ssam_device_remove(to_ssam_device(dev));
+ return 0;
+}
+
+static void ssam_hub_remove_devices(struct device *parent)
+{
+ device_for_each_child_reverse(parent, NULL, ssam_hub_remove_devices_fn);
+}
+
+static int ssam_hub_add_device(struct device *parent, struct ssam_controller *ctrl,
+ struct fwnode_handle *node)
+{
+ struct ssam_device_uid uid;
+ struct ssam_device *sdev;
+ int status;
+
+ status = ssam_uid_from_string(fwnode_get_name(node), &uid);
+ if (status)
+ return status;
+
+ sdev = ssam_device_alloc(ctrl, uid);
+ if (!sdev)
+ return -ENOMEM;
+
+ sdev->dev.parent = parent;
+ sdev->dev.fwnode = node;
+
+ status = ssam_device_add(sdev);
+ if (status)
+ ssam_device_put(sdev);
+
+ return status;
+}
+
+static int ssam_hub_add_devices(struct device *parent, struct ssam_controller *ctrl,
+ struct fwnode_handle *node)
+{
+ struct fwnode_handle *child;
+ int status;
+
+ fwnode_for_each_child_node(node, child) {
+ /*
+ * Try to add the device specified in the firmware node. If
+ * this fails with -EINVAL, the node does not specify any SSAM
+ * device, so ignore it and continue with the next one.
+ */
+
+ status = ssam_hub_add_device(parent, ctrl, child);
+ if (status && status != -EINVAL)
+ goto err;
+ }
+
+ return 0;
+err:
+ ssam_hub_remove_devices(parent);
+ return status;
+}
+
+
+/* -- SSAM platform/meta-hub driver. ---------------------------------------- */
+
+static const struct acpi_device_id ssam_platform_hub_match[] = {
+ /* Surface Pro 4, 5, and 6 (OMBR < 0x10) */
+ { "MSHW0081", (unsigned long)ssam_node_group_sp5 },
+
+ /* Surface Pro 6 (OMBR >= 0x10) */
+ { "MSHW0111", (unsigned long)ssam_node_group_sp6 },
+
+ /* Surface Pro 7 */
+ { "MSHW0116", (unsigned long)ssam_node_group_sp7 },
+
+ /* Surface Book 2 */
+ { "MSHW0107", (unsigned long)ssam_node_group_sb2 },
+
+ /* Surface Book 3 */
+ { "MSHW0117", (unsigned long)ssam_node_group_sb3 },
+
+ /* Surface Laptop 1 */
+ { "MSHW0086", (unsigned long)ssam_node_group_sl1 },
+
+ /* Surface Laptop 2 */
+ { "MSHW0112", (unsigned long)ssam_node_group_sl2 },
+
+ /* Surface Laptop 3 (13", Intel) */
+ { "MSHW0114", (unsigned long)ssam_node_group_sl3 },
+
+ /* Surface Laptop 3 (15", AMD) */
+ { "MSHW0110", (unsigned long)ssam_node_group_sl3 },
+
+ /* Surface Laptop Go 1 */
+ { "MSHW0118", (unsigned long)ssam_node_group_slg1 },
+
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, ssam_platform_hub_match);
+
+static int ssam_platform_hub_probe(struct platform_device *pdev)
+{
+ const struct software_node **nodes;
+ struct ssam_controller *ctrl;
+ struct fwnode_handle *root;
+ int status;
+
+ nodes = (const struct software_node **)acpi_device_get_match_data(&pdev->dev);
+ if (!nodes)
+ return -ENODEV;
+
+ /*
+ * As we're adding the SSAM client devices as children under this device
+ * and not the SSAM controller, we need to add a device link to the
+ * controller to ensure that we remove all of our devices before the
+ * controller is removed. This also guarantees proper ordering for
+ * suspend/resume of the devices on this hub.
+ */
+ ctrl = ssam_client_bind(&pdev->dev);
+ if (IS_ERR(ctrl))
+ return PTR_ERR(ctrl) == -ENODEV ? -EPROBE_DEFER : PTR_ERR(ctrl);
+
+ status = software_node_register_node_group(nodes);
+ if (status)
+ return status;
+
+ root = software_node_fwnode(&ssam_node_root);
+ if (!root) {
+ software_node_unregister_node_group(nodes);
+ return -ENOENT;
+ }
+
+ set_secondary_fwnode(&pdev->dev, root);
+
+ status = ssam_hub_add_devices(&pdev->dev, ctrl, root);
+ if (status) {
+ set_secondary_fwnode(&pdev->dev, NULL);
+ software_node_unregister_node_group(nodes);
+ }
+
+ platform_set_drvdata(pdev, nodes);
+ return status;
+}
+
+static int ssam_platform_hub_remove(struct platform_device *pdev)
+{
+ const struct software_node **nodes = platform_get_drvdata(pdev);
+
+ ssam_hub_remove_devices(&pdev->dev);
+ set_secondary_fwnode(&pdev->dev, NULL);
+ software_node_unregister_node_group(nodes);
+ return 0;
+}
+
+static struct platform_driver ssam_platform_hub_driver = {
+ .probe = ssam_platform_hub_probe,
+ .remove = ssam_platform_hub_remove,
+ .driver = {
+ .name = "surface_aggregator_platform_hub",
+ .acpi_match_table = ssam_platform_hub_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+};
+module_platform_driver(ssam_platform_hub_driver);
+
+MODULE_AUTHOR("Maximilian Luz <luzmaximilian@xxxxxxxxx>");
+MODULE_DESCRIPTION("Device-registry for Surface System Aggregator Module");
+MODULE_LICENSE("GPL");