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 <[email protected]>
> ---
> 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 <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +MODULE_DESCRIPTION("Device-registry for Surface System Aggregator Module");
> +MODULE_LICENSE("GPL");
>