linux-surface/patches/5.14/0005-surface-sam.patch

From f6611d8d35d47d43b7aae454a36d69232c430958 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Thu, 28 Oct 2021 03:28:45 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Add initial support
 for Surface Pro 8

Add preliminary support for the Surface Pro 8 to the Surface Aggregator
registry. This includes battery/charger status and platform profile
support.

In contrast to earlier Surface Pro generations, the keyboard cover is
now also connected via the Surface Aggregator Module (whereas it was
previously connected via USB or HID-over-I2C). To properly support the
HID devices of that cover, however, more changes regarding hot-removal
of Surface Aggregator client devices as well as a new device hub driver
are required. We will address those things in a follow-up series, so do
not add any HID device IDs just yet.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20211028012845.1887219-1-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 .../platform/surface/surface_aggregator_registry.c   | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 1679811eff50..e70f4c63554e 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -228,6 +228,15 @@ static const struct software_node *ssam_node_group_sp7[] = {
 	NULL,
 };
 
+static const struct software_node *ssam_node_group_sp8[] = {
+	&ssam_node_root,
+	&ssam_node_bat_ac,
+	&ssam_node_bat_main,
+	&ssam_node_tmp_pprof,
+	/* TODO: Add support for keyboard cover. */
+	NULL,
+};
+
 
 /* -- Device registry helper functions. ------------------------------------- */
 
@@ -534,6 +543,9 @@ static const struct acpi_device_id ssam_platform_hub_match[] = {
 	/* Surface Pro 7+ */
 	{ "MSHW0119", (unsigned long)ssam_node_group_sp7 },
 
+	/* Surface Pro 8 */
+	{ "MSHW0263", (unsigned long)ssam_node_group_sp8 },
+
 	/* Surface Book 2 */
 	{ "MSHW0107", (unsigned long)ssam_node_group_gen5 },
 
-- 
2.34.0

From 11a1289287c02cfec4efbdd63f85dea6d68284f2 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 2 Jun 2021 03:34:06 +0200
Subject: [PATCH] platform/surface: aggregator: Make client device removal more
 generic

Currently, there are similar functions defined in the Aggregator
Registry and the controller core.

Make client device removal more generic and export it. We can then use
this function later on to remove client devices from device hubs as well
as the controller and avoid re-defining similar things.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/aggregator/bus.c  | 24 ++++++++--------------
 drivers/platform/surface/aggregator/bus.h  |  3 ---
 drivers/platform/surface/aggregator/core.c |  3 ++-
 include/linux/surface_aggregator/device.h  |  9 ++++++++
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/surface/aggregator/bus.c b/drivers/platform/surface/aggregator/bus.c
index 0169677c243e..eaa8626baead 100644
--- a/drivers/platform/surface/aggregator/bus.c
+++ b/drivers/platform/surface/aggregator/bus.c
@@ -376,27 +376,19 @@ static int ssam_remove_device(struct device *dev, void *_data)
 }
 
 /**
- * ssam_controller_remove_clients() - Remove SSAM client devices registered as
- * direct children under the given controller.
- * @ctrl: The controller to remove all direct clients for.
+ * ssam_remove_clients() - Remove SSAM client devices registered as direct
+ * children under the given parent device.
+ * @dev: The (parent) device to remove all direct clients for.
  *
- * Remove all SSAM client devices registered as direct children under the
- * given controller. Note that this only accounts for direct children of the
- * controller device. This does not take care of any client devices where the
- * parent device has been manually set before calling ssam_device_add. Refer
- * to ssam_device_add()/ssam_device_remove() for more details on those cases.
- *
- * To avoid new devices being added in parallel to this call, the main
- * controller lock (not statelock) must be held during this (and if
- * necessary, any subsequent deinitialization) call.
+ * Remove all SSAM client devices registered as direct children under the given
+ * device. Note that this only accounts for direct children of the device.
+ * Refer to ssam_device_add()/ssam_device_remove() for more details.
  */
-void ssam_controller_remove_clients(struct ssam_controller *ctrl)
+void ssam_remove_clients(struct device *dev)
 {
-	struct device *dev;
-
-	dev = ssam_controller_device(ctrl);
 	device_for_each_child_reverse(dev, NULL, ssam_remove_device);
 }
+EXPORT_SYMBOL_GPL(ssam_remove_clients);
 
 /**
  * ssam_bus_register() - Register and set-up the SSAM client device bus.
diff --git a/drivers/platform/surface/aggregator/bus.h b/drivers/platform/surface/aggregator/bus.h
index ed032c2cbdb2..6964ee84e79c 100644
--- a/drivers/platform/surface/aggregator/bus.h
+++ b/drivers/platform/surface/aggregator/bus.h
@@ -12,14 +12,11 @@
 
 #ifdef CONFIG_SURFACE_AGGREGATOR_BUS
 
-void ssam_controller_remove_clients(struct ssam_controller *ctrl);
-
 int ssam_bus_register(void);
 void ssam_bus_unregister(void);
 
 #else /* CONFIG_SURFACE_AGGREGATOR_BUS */
 
-static inline void ssam_controller_remove_clients(struct ssam_controller *ctrl) {}
 static inline int ssam_bus_register(void) { return 0; }
 static inline void ssam_bus_unregister(void) {}
 
diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c
index 279d9df19c01..124df0100a9f 100644
--- a/drivers/platform/surface/aggregator/core.c
+++ b/drivers/platform/surface/aggregator/core.c
@@ -22,6 +22,7 @@
 #include <linux/sysfs.h>
 
 #include <linux/surface_aggregator/controller.h>
+#include <linux/surface_aggregator/device.h>
 
 #include "bus.h"
 #include "controller.h"
@@ -742,7 +743,7 @@ static void ssam_serial_hub_remove(struct serdev_device *serdev)
 	ssam_controller_lock(ctrl);
 
 	/* Remove all client devices. */
-	ssam_controller_remove_clients(ctrl);
+	ssam_remove_clients(&serdev->dev);
 
 	/* Act as if suspending to silence events. */
 	status = ssam_ctrl_notif_display_off(ctrl);
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h
index f636c5310321..cc257097eb05 100644
--- a/include/linux/surface_aggregator/device.h
+++ b/include/linux/surface_aggregator/device.h
@@ -319,6 +319,15 @@ void ssam_device_driver_unregister(struct ssam_device_driver *d);
 		      ssam_device_driver_unregister)
 
 
+/* -- Helpers for controller and hub devices. ------------------------------- */
+
+#ifdef CONFIG_SURFACE_AGGREGATOR_BUS
+void ssam_remove_clients(struct device *dev);
+#else /* CONFIG_SURFACE_AGGREGATOR_BUS */
+static inline void ssam_remove_clients(struct device *dev) {}
+#endif /* CONFIG_SURFACE_AGGREGATOR_BUS */
+
+
 /* -- Helpers for client-device requests. ----------------------------------- */
 
 /**
-- 
2.34.0

From 4a84f3d00f6fa73d2d423f737b31e574b5a5f30a Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 27 Oct 2021 02:06:38 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Use generic client
 removal function

Use generic client removal function introduced in the previous commit
instead of defining our own one.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_registry.c     | 24 ++++---------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index e70f4c63554e..f6c639342b9d 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -258,20 +258,6 @@ static int ssam_uid_from_string(const char *str, struct ssam_device_uid *uid)
 	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)
 {
@@ -317,7 +303,7 @@ static int ssam_hub_add_devices(struct device *parent, struct ssam_controller *c
 
 	return 0;
 err:
-	ssam_hub_remove_devices(parent);
+	ssam_remove_clients(parent);
 	return status;
 }
 
@@ -414,7 +400,7 @@ static void ssam_base_hub_update_workfn(struct work_struct *work)
 	if (hub->state == SSAM_BASE_HUB_CONNECTED)
 		status = ssam_hub_add_devices(&hub->sdev->dev, hub->sdev->ctrl, node);
 	else
-		ssam_hub_remove_devices(&hub->sdev->dev);
+		ssam_remove_clients(&hub->sdev->dev);
 
 	if (status)
 		dev_err(&hub->sdev->dev, "failed to update base-hub devices: %d\n", status);
@@ -496,7 +482,7 @@ static int ssam_base_hub_probe(struct ssam_device *sdev)
 err:
 	ssam_notifier_unregister(sdev->ctrl, &hub->notif);
 	cancel_delayed_work_sync(&hub->update_work);
-	ssam_hub_remove_devices(&sdev->dev);
+	ssam_remove_clients(&sdev->dev);
 	return status;
 }
 
@@ -508,7 +494,7 @@ static void ssam_base_hub_remove(struct ssam_device *sdev)
 
 	ssam_notifier_unregister(sdev->ctrl, &hub->notif);
 	cancel_delayed_work_sync(&hub->update_work);
-	ssam_hub_remove_devices(&sdev->dev);
+	ssam_remove_clients(&sdev->dev);
 }
 
 static const struct ssam_device_id ssam_base_hub_match[] = {
@@ -625,7 +611,7 @@ 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);
+	ssam_remove_clients(&pdev->dev);
 	set_secondary_fwnode(&pdev->dev, NULL);
 	software_node_unregister_node_group(nodes);
 	return 0;
-- 
2.34.0

From a801735c964f387be196a2d63d22b030667d592e Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 27 Oct 2021 02:07:33 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Rename device
 registration function

Rename the device registration function to better align names with the
newly introduced device removal function.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_registry.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index f6c639342b9d..ce2bd88feeaa 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -283,8 +283,8 @@ static int ssam_hub_add_device(struct device *parent, struct ssam_controller *ct
 	return status;
 }
 
-static int ssam_hub_add_devices(struct device *parent, struct ssam_controller *ctrl,
-				struct fwnode_handle *node)
+static int ssam_hub_register_clients(struct device *parent, struct ssam_controller *ctrl,
+				     struct fwnode_handle *node)
 {
 	struct fwnode_handle *child;
 	int status;
@@ -398,7 +398,7 @@ static void ssam_base_hub_update_workfn(struct work_struct *work)
 	hub->state = state;
 
 	if (hub->state == SSAM_BASE_HUB_CONNECTED)
-		status = ssam_hub_add_devices(&hub->sdev->dev, hub->sdev->ctrl, node);
+		status = ssam_hub_register_clients(&hub->sdev->dev, hub->sdev->ctrl, node);
 	else
 		ssam_remove_clients(&hub->sdev->dev);
 
@@ -597,7 +597,7 @@ static int ssam_platform_hub_probe(struct platform_device *pdev)
 
 	set_secondary_fwnode(&pdev->dev, root);
 
-	status = ssam_hub_add_devices(&pdev->dev, ctrl, root);
+	status = ssam_hub_register_clients(&pdev->dev, ctrl, root);
 	if (status) {
 		set_secondary_fwnode(&pdev->dev, NULL);
 		software_node_unregister_node_group(nodes);
-- 
2.34.0

From b18657a4239bad3b786edb87311b130403a5a1fd Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 8 Jun 2021 00:24:47 +0200
Subject: [PATCH] platform/surface: aggregator: Allow devices to be marked as
 hot-removed

Some SSAM devices, notably the keyboard cover (keyboard and touchpad) on
the Surface Pro 8, can be hot-removed. When this occurs, communication
with the device may fail and time out. This timeout can unnecessarily
block and slow down device removal and even cause issues when the
devices are detached and re-attached quickly. Thus, communication should
generally be avoided once hot-removal is detected.

While we already remove a device as soon as we detect its (hot-)removal,
the corresponding device driver may still attempt to communicate with
the device during teardown. This is especially critical as communication
failure may also extend to disabling of events, which is typically done
at that stage.

Add a flag to allow marking devices as hot-removed. This can then be
used during client driver teardown to check if any communication
attempts should be avoided.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/aggregator/bus.c |  3 ++
 include/linux/surface_aggregator/device.h | 48 +++++++++++++++++++++--
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/surface/aggregator/bus.c b/drivers/platform/surface/aggregator/bus.c
index eaa8626baead..736cc43c9104 100644
--- a/drivers/platform/surface/aggregator/bus.c
+++ b/drivers/platform/surface/aggregator/bus.c
@@ -390,6 +390,9 @@ void ssam_remove_clients(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(ssam_remove_clients);
 
+
+/* -- Bus registration. ----------------------------------------------------- */
+
 /**
  * ssam_bus_register() - Register and set-up the SSAM client device bus.
  */
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h
index cc257097eb05..491aa7e9f4bc 100644
--- a/include/linux/surface_aggregator/device.h
+++ b/include/linux/surface_aggregator/device.h
@@ -148,17 +148,30 @@ struct ssam_device_uid {
 #define SSAM_SDEV(cat, tid, iid, fun) \
 	SSAM_DEVICE(SSAM_DOMAIN_SERIALHUB, SSAM_SSH_TC_##cat, tid, iid, fun)
 
+/*
+ * enum ssam_device_flags - Flags for SSAM client devices.
+ * @SSAM_DEVICE_HOT_REMOVED_BIT:
+ *	The device has been hot-removed. Further communication with it may time
+ *	out and should be avoided.
+ */
+enum ssam_device_flags {
+	SSAM_DEVICE_HOT_REMOVED_BIT = 0,
+};
+
 /**
  * struct ssam_device - SSAM client device.
- * @dev:  Driver model representation of the device.
- * @ctrl: SSAM controller managing this device.
- * @uid:  UID identifying the device.
+ * @dev:   Driver model representation of the device.
+ * @ctrl:  SSAM controller managing this device.
+ * @uid:   UID identifying the device.
+ * @flags: Device state flags, see &enum ssam_device_flags.
  */
 struct ssam_device {
 	struct device dev;
 	struct ssam_controller *ctrl;
 
 	struct ssam_device_uid uid;
+
+	unsigned long flags;
 };
 
 /**
@@ -240,6 +253,35 @@ struct ssam_device *ssam_device_alloc(struct ssam_controller *ctrl,
 int ssam_device_add(struct ssam_device *sdev);
 void ssam_device_remove(struct ssam_device *sdev);
 
+/**
+ * ssam_device_mark_hot_removed() - Mark the given device as hot-removed.
+ * @sdev: The device to mark as hot-removed.
+ *
+ * Mark the device as having been hot-removed. This signals drivers using the
+ * device that communication with the device should be avoided and may lead to
+ * timeouts.
+ */
+static inline void ssam_device_mark_hot_removed(struct ssam_device *sdev)
+{
+	dev_dbg(&sdev->dev, "marking device as hot-removed\n");
+	set_bit(SSAM_DEVICE_HOT_REMOVED_BIT, &sdev->flags);
+}
+
+/**
+ * ssam_device_is_hot_removed() - Check if the given device has been
+ * hot-removed.
+ * @sdev: The device to check.
+ *
+ * Checks if the given device has been marked as hot-removed. See
+ * ssam_device_mark_hot_removed() for more details.
+ *
+ * Return: Returns ``true`` if the device has been marked as hot-removed.
+ */
+static inline bool ssam_device_is_hot_removed(struct ssam_device *sdev)
+{
+	return test_bit(SSAM_DEVICE_HOT_REMOVED_BIT, &sdev->flags);
+}
+
 /**
  * ssam_device_get() - Increment reference count of SSAM client device.
  * @sdev: The device to increment the reference count of.
-- 
2.34.0

From f95d6ac5874db1bb881ffa482abc35f94bb13f5e Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 8 Jun 2021 00:48:22 +0200
Subject: [PATCH] platform/surface: aggregator: Allow notifiers to avoid
 communication on unregistering

When SSAM client devices have been (physically) hot-removed,
communication attempts with those devices may fail and time out. This
can even extend to event notifiers, due to which timeouts may occur
during device removal, slowing down that process.

Add a flag to the notifier unregister function that allows skipping
communication with the EC to prevent this. Furthermore, add wrappers for
registering and unregistering notifiers belonging to SSAM client devices
that automatically check if the device has been marked as hot-removed
and communication should be avoided.

Note that non-SSAM client devices can generally not be hot-removed, so
also add a convenience wrapper for those, defaulting to allow
communication.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../driver-api/surface_aggregator/client.rst  |  6 +-
 .../platform/surface/aggregator/controller.c  | 53 ++++++++++------
 include/linux/surface_aggregator/controller.h | 24 +++++++-
 include/linux/surface_aggregator/device.h     | 60 +++++++++++++++++++
 4 files changed, 122 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/surface_aggregator/client.rst b/Documentation/driver-api/surface_aggregator/client.rst
index e519d374c378..27f95abdbe99 100644
--- a/Documentation/driver-api/surface_aggregator/client.rst
+++ b/Documentation/driver-api/surface_aggregator/client.rst
@@ -17,6 +17,8 @@
 .. |SSAM_DEVICE| replace:: :c:func:`SSAM_DEVICE`
 .. |ssam_notifier_register| replace:: :c:func:`ssam_notifier_register`
 .. |ssam_notifier_unregister| replace:: :c:func:`ssam_notifier_unregister`
+.. |ssam_device_notifier_register| replace:: :c:func:`ssam_device_notifier_register`
+.. |ssam_device_notifier_unregister| replace:: :c:func:`ssam_device_notifier_unregister`
 .. |ssam_request_sync| replace:: :c:func:`ssam_request_sync`
 .. |ssam_event_mask| replace:: :c:type:`enum ssam_event_mask <ssam_event_mask>`
 
@@ -312,7 +314,9 @@ Handling Events
 To receive events from the SAM EC, an event notifier must be registered for
 the desired event via |ssam_notifier_register|. The notifier must be
 unregistered via |ssam_notifier_unregister| once it is not required any
-more.
+more. For |ssam_device| type clients, the |ssam_device_notifier_register| and
+|ssam_device_notifier_unregister| wrappers should be preferred as they properly
+handle hot-removal of client devices.
 
 Event notifiers are registered by providing (at minimum) a callback to call
 in case an event has been received, the registry specifying how the event
diff --git a/drivers/platform/surface/aggregator/controller.c b/drivers/platform/surface/aggregator/controller.c
index b8c377b3f932..6de834b52b63 100644
--- a/drivers/platform/surface/aggregator/controller.c
+++ b/drivers/platform/surface/aggregator/controller.c
@@ -2199,16 +2199,26 @@ static int ssam_nf_refcount_enable(struct ssam_controller *ctrl,
 }
 
 /**
- * ssam_nf_refcount_disable_free() - Disable event for reference count entry if it is
- * no longer in use and free the corresponding entry.
+ * ssam_nf_refcount_disable_free() - Disable event for reference count entry if
+ * it is no longer in use and free the corresponding entry.
  * @ctrl:  The controller to disable the event on.
  * @entry: The reference count entry for the event to be disabled.
  * @flags: The flags used for enabling the event on the EC.
+ * @ec:    Flag specifying if the event should actually be disabled on the EC.
  *
- * If the reference count equals zero, i.e. the event is no longer requested by
- * any client, the event will be disabled and the corresponding reference count
- * entry freed. The reference count entry must not be used any more after a
- * call to this function.
+ * If ``ec`` equals ``true`` and the reference count equals zero (i.e. the
+ * event is no longer requested by any client), the specified event will be
+ * disabled on the EC via the corresponding request.
+ *
+ * If ``ec`` equals ``false``, no request will be sent to the EC and the event
+ * can be considered in a detached state (i.e. no longer used but still
+ * enabled). Disabling an event via this method may be required for
+ * hot-removable devices, where event disable requests may time out after the
+ * device has been physically removed.
+ *
+ * In both cases, if the reference count equals zero, the corresponding
+ * reference count entry will be freed. The reference count entry must not be
+ * used any more after a call to this function.
  *
  * Also checks if the flags used for disabling the event match the flags used
  * for enabling the event and warns if they do not (regardless of reference
@@ -2223,7 +2233,7 @@ static int ssam_nf_refcount_enable(struct ssam_controller *ctrl,
  * returns the status of the event-enable EC command.
  */
 static int ssam_nf_refcount_disable_free(struct ssam_controller *ctrl,
-					 struct ssam_nf_refcount_entry *entry, u8 flags)
+					 struct ssam_nf_refcount_entry *entry, u8 flags, bool ec)
 {
 	const struct ssam_event_registry reg = entry->key.reg;
 	const struct ssam_event_id id = entry->key.id;
@@ -2232,8 +2242,9 @@ static int ssam_nf_refcount_disable_free(struct ssam_controller *ctrl,
 
 	lockdep_assert_held(&nf->lock);
 
-	ssam_dbg(ctrl, "disabling event (reg: %#04x, tc: %#04x, iid: %#04x, rc: %d)\n",
-		 reg.target_category, id.target_category, id.instance, entry->refcount);
+	ssam_dbg(ctrl, "%s event (reg: %#04x, tc: %#04x, iid: %#04x, rc: %d)\n",
+		 ec ? "disabling" : "detaching", reg.target_category, id.target_category,
+		 id.instance, entry->refcount);
 
 	if (entry->flags != flags) {
 		ssam_warn(ctrl,
@@ -2242,7 +2253,7 @@ static int ssam_nf_refcount_disable_free(struct ssam_controller *ctrl,
 			  id.instance);
 	}
 
-	if (entry->refcount == 0) {
+	if (ec && entry->refcount == 0) {
 		status = ssam_ssh_event_disable(ctrl, reg, id, flags);
 		kfree(entry);
 	}
@@ -2322,20 +2333,26 @@ int ssam_notifier_register(struct ssam_controller *ctrl, struct ssam_event_notif
 EXPORT_SYMBOL_GPL(ssam_notifier_register);
 
 /**
- * ssam_notifier_unregister() - Unregister an event notifier.
- * @ctrl: The controller the notifier has been registered on.
- * @n:    The event notifier to unregister.
+ * __ssam_notifier_unregister() - Unregister an event notifier.
+ * @ctrl:    The controller the notifier has been registered on.
+ * @n:       The event notifier to unregister.
+ * @disable: Whether to disable the corresponding event on the EC.
  *
  * Unregister an event notifier. Decrement the usage counter of the associated
  * SAM event if the notifier is not marked as an observer. If the usage counter
- * reaches zero, the event will be disabled.
+ * reaches zero and ``disable`` equals ``true``, the event will be disabled.
+ *
+ * Useful for hot-removable devices, where communication may fail once the
+ * device has been physically removed. In that case, specifying ``disable`` as
+ * ``false`` avoids communication with the EC.
  *
  * Return: Returns zero on success, %-ENOENT if the given notifier block has
  * not been registered on the controller. If the given notifier block was the
  * last one associated with its specific event, returns the status of the
  * event-disable EC-command.
  */
-int ssam_notifier_unregister(struct ssam_controller *ctrl, struct ssam_event_notifier *n)
+int __ssam_notifier_unregister(struct ssam_controller *ctrl, struct ssam_event_notifier *n,
+			       bool disable)
 {
 	u16 rqid = ssh_tc_to_rqid(n->event.id.target_category);
 	struct ssam_nf_refcount_entry *entry;
@@ -2373,7 +2390,7 @@ int ssam_notifier_unregister(struct ssam_controller *ctrl, struct ssam_event_not
 			goto remove;
 		}
 
-		status = ssam_nf_refcount_disable_free(ctrl, entry, n->event.flags);
+		status = ssam_nf_refcount_disable_free(ctrl, entry, n->event.flags, disable);
 	}
 
 remove:
@@ -2383,7 +2400,7 @@ int ssam_notifier_unregister(struct ssam_controller *ctrl, struct ssam_event_not
 
 	return status;
 }
-EXPORT_SYMBOL_GPL(ssam_notifier_unregister);
+EXPORT_SYMBOL_GPL(__ssam_notifier_unregister);
 
 /**
  * ssam_controller_event_enable() - Enable the specified event.
@@ -2477,7 +2494,7 @@ int ssam_controller_event_disable(struct ssam_controller *ctrl,
 		return -ENOENT;
 	}
 
-	status = ssam_nf_refcount_disable_free(ctrl, entry, flags);
+	status = ssam_nf_refcount_disable_free(ctrl, entry, flags, true);
 
 	mutex_unlock(&nf->lock);
 	return status;
diff --git a/include/linux/surface_aggregator/controller.h b/include/linux/surface_aggregator/controller.h
index 74bfdffaf7b0..50a2b4926c06 100644
--- a/include/linux/surface_aggregator/controller.h
+++ b/include/linux/surface_aggregator/controller.h
@@ -835,8 +835,28 @@ struct ssam_event_notifier {
 int ssam_notifier_register(struct ssam_controller *ctrl,
 			   struct ssam_event_notifier *n);
 
-int ssam_notifier_unregister(struct ssam_controller *ctrl,
-			     struct ssam_event_notifier *n);
+int __ssam_notifier_unregister(struct ssam_controller *ctrl,
+			       struct ssam_event_notifier *n, bool disable);
+
+/**
+ * ssam_notifier_unregister() - Unregister an event notifier.
+ * @ctrl:    The controller the notifier has been registered on.
+ * @n:       The event notifier to unregister.
+ *
+ * Unregister an event notifier. Decrement the usage counter of the associated
+ * SAM event if the notifier is not marked as an observer. If the usage counter
+ * reaches zero, the event will be disabled.
+ *
+ * Return: Returns zero on success, %-ENOENT if the given notifier block has
+ * not been registered on the controller. If the given notifier block was the
+ * last one associated with its specific event, returns the status of the
+ * event-disable EC-command.
+ */
+static inline int ssam_notifier_unregister(struct ssam_controller *ctrl,
+					   struct ssam_event_notifier *n)
+{
+	return __ssam_notifier_unregister(ctrl, n, true);
+}
 
 int ssam_controller_event_enable(struct ssam_controller *ctrl,
 				 struct ssam_event_registry reg,
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h
index 491aa7e9f4bc..16816c34da3e 100644
--- a/include/linux/surface_aggregator/device.h
+++ b/include/linux/surface_aggregator/device.h
@@ -472,4 +472,64 @@ static inline void ssam_remove_clients(struct device *dev) {}
 				    sdev->uid.instance, ret);		\
 	}
 
+
+/* -- Helpers for client-device notifiers. ---------------------------------- */
+
+/**
+ * ssam_device_notifier_register() - Register an event notifier for the
+ * specified client device.
+ * @sdev: The device the notifier should be registered on.
+ * @n:    The event notifier to register.
+ *
+ * Register an event notifier. Increment the usage counter of the associated
+ * SAM event if the notifier is not marked as an observer. If the event is not
+ * marked as an observer and is currently not enabled, it will be enabled
+ * during this call. If the notifier is marked as an observer, no attempt will
+ * be made at enabling any event and no reference count will be modified.
+ *
+ * Notifiers marked as observers do not need to be associated with one specific
+ * event, i.e. as long as no event matching is performed, only the event target
+ * category needs to be set.
+ *
+ * Return: Returns zero on success, %-ENOSPC if there have already been
+ * %INT_MAX notifiers for the event ID/type associated with the notifier block
+ * registered, %-ENOMEM if the corresponding event entry could not be
+ * allocated, %-ENODEV if the device is marked as hot-removed. If this is the
+ * first time that a notifier block is registered for the specific associated
+ * event, returns the status of the event-enable EC-command.
+ */
+static inline int ssam_device_notifier_register(struct ssam_device *sdev,
+						struct ssam_event_notifier *n)
+{
+	if (ssam_device_is_hot_removed(sdev))
+		return -ENODEV;
+
+	return ssam_notifier_register(sdev->ctrl, n);
+}
+
+/**
+ * ssam_device_notifier_unregister() - Unregister an event notifier for the
+ * specified client device.
+ * @sdev: The device the notifier has been registered on.
+ * @n:    The event notifier to unregister.
+ *
+ * Unregister an event notifier. Decrement the usage counter of the associated
+ * SAM event if the notifier is not marked as an observer. If the usage counter
+ * reaches zero, the event will be disabled.
+ *
+ * In case the device has been marked as hot-removed, the event will not be
+ * disabled on the EC, as in those cases any attempt at doing so may time out.
+ *
+ * Return: Returns zero on success, %-ENOENT if the given notifier block has
+ * not been registered on the controller. If the given notifier block was the
+ * last one associated with its specific event, returns the status of the
+ * event-disable EC-command.
+ */
+static inline int ssam_device_notifier_unregister(struct ssam_device *sdev,
+						  struct ssam_event_notifier *n)
+{
+	return __ssam_notifier_unregister(sdev->ctrl, n,
+					  !ssam_device_is_hot_removed(sdev));
+}
+
 #endif /* _LINUX_SURFACE_AGGREGATOR_DEVICE_H */
-- 
2.34.0

From fe2e44cf86dee61dc84b56f76d432e8b87c2710a Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 8 Jun 2021 01:20:49 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Use client device
 wrappers for notifier registration

Use newly introduced client device wrapper functions for notifier
registration and unregistration.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_registry.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index ce2bd88feeaa..9f630e890ff7 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -468,7 +468,7 @@ static int ssam_base_hub_probe(struct ssam_device *sdev)
 
 	ssam_device_set_drvdata(sdev, hub);
 
-	status = ssam_notifier_register(sdev->ctrl, &hub->notif);
+	status = ssam_device_notifier_register(sdev, &hub->notif);
 	if (status)
 		return status;
 
@@ -480,7 +480,7 @@ static int ssam_base_hub_probe(struct ssam_device *sdev)
 	return 0;
 
 err:
-	ssam_notifier_unregister(sdev->ctrl, &hub->notif);
+	ssam_device_notifier_unregister(sdev, &hub->notif);
 	cancel_delayed_work_sync(&hub->update_work);
 	ssam_remove_clients(&sdev->dev);
 	return status;
@@ -492,7 +492,7 @@ static void ssam_base_hub_remove(struct ssam_device *sdev)
 
 	sysfs_remove_group(&sdev->dev.kobj, &ssam_base_hub_group);
 
-	ssam_notifier_unregister(sdev->ctrl, &hub->notif);
+	ssam_device_notifier_unregister(sdev, &hub->notif);
 	cancel_delayed_work_sync(&hub->update_work);
 	ssam_remove_clients(&sdev->dev);
 }
-- 
2.34.0

From 05ded3f8005f37297e5c47821d43c24dc243250c Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Thu, 28 Oct 2021 03:37:06 +0200
Subject: [PATCH] power/supply: surface_charger: Use client device wrappers for
 notifier registration

Use newly introduced client device wrapper functions for notifier
registration and unregistration.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/power/supply/surface_charger.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/surface_charger.c b/drivers/power/supply/surface_charger.c
index a060c36c7766..59182d55742d 100644
--- a/drivers/power/supply/surface_charger.c
+++ b/drivers/power/supply/surface_charger.c
@@ -216,7 +216,7 @@ static int spwr_ac_register(struct spwr_ac_device *ac)
 	if (IS_ERR(ac->psy))
 		return PTR_ERR(ac->psy);
 
-	return ssam_notifier_register(ac->sdev->ctrl, &ac->notif);
+	return ssam_device_notifier_register(ac->sdev, &ac->notif);
 }
 
 
@@ -251,7 +251,7 @@ static void surface_ac_remove(struct ssam_device *sdev)
 {
 	struct spwr_ac_device *ac = ssam_device_get_drvdata(sdev);
 
-	ssam_notifier_unregister(sdev->ctrl, &ac->notif);
+	ssam_device_notifier_unregister(sdev, &ac->notif);
 }
 
 static const struct spwr_psy_properties spwr_psy_props_adp1 = {
-- 
2.34.0

From 18b5e32f9cf968307c604677143eea7dec7a8533 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Thu, 28 Oct 2021 03:38:09 +0200
Subject: [PATCH] power/supply: surface_battery: Use client device wrappers for
 notifier registration

Use newly introduced client device wrapper functions for notifier
registration and unregistration.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/power/supply/surface_battery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/surface_battery.c b/drivers/power/supply/surface_battery.c
index 5ec2e6bb2465..540707882bb0 100644
--- a/drivers/power/supply/surface_battery.c
+++ b/drivers/power/supply/surface_battery.c
@@ -802,7 +802,7 @@ static int spwr_battery_register(struct spwr_battery_device *bat)
 	if (IS_ERR(bat->psy))
 		return PTR_ERR(bat->psy);
 
-	return ssam_notifier_register(bat->sdev->ctrl, &bat->notif);
+	return ssam_device_notifier_register(bat->sdev, &bat->notif);
 }
 
 
@@ -837,7 +837,7 @@ static void surface_battery_remove(struct ssam_device *sdev)
 {
 	struct spwr_battery_device *bat = ssam_device_get_drvdata(sdev);
 
-	ssam_notifier_unregister(sdev->ctrl, &bat->notif);
+	ssam_device_notifier_unregister(sdev, &bat->notif);
 	cancel_delayed_work_sync(&bat->update_work);
 }
 
-- 
2.34.0

From 43c9f03affd1b3e0ecf04a370efe3ffa463a26c9 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 8 Jun 2021 01:33:02 +0200
Subject: [PATCH] HID: surface-hid: Add support for hot-removal

Add support for hot-removal of SSAM HID client devices.

Once a device has been hot-removed, further communication with it should
be avoided as it may fail and time out. While the device will be removed
as soon as we detect hot-removal, communication may still occur during
teardown, especially when unregistering notifiers.

While hot-removal is a surprise event that can happen any time, try to
avoid communication as much as possible once it has been detected to
prevent timeouts that can slow down device removal and cause issues,
e.g. when quickly re-attaching the device.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/hid/surface-hid/surface_hid_core.c | 38 +++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/surface-hid/surface_hid_core.c b/drivers/hid/surface-hid/surface_hid_core.c
index 5571e74abe91..d2e695e942b6 100644
--- a/drivers/hid/surface-hid/surface_hid_core.c
+++ b/drivers/hid/surface-hid/surface_hid_core.c
@@ -19,12 +19,30 @@
 #include "surface_hid_core.h"
 
 
+/* -- Utility functions. ---------------------------------------------------- */
+
+static bool surface_hid_is_hot_removed(struct surface_hid_device *shid)
+{
+	/*
+	 * Non-ssam client devices, i.e. platform client devices, cannot be
+	 * hot-removed.
+	 */
+	if (!is_ssam_device(shid->dev))
+		return false;
+
+	return ssam_device_is_hot_removed(to_ssam_device(shid->dev));
+}
+
+
 /* -- Device descriptor access. --------------------------------------------- */
 
 static int surface_hid_load_hid_descriptor(struct surface_hid_device *shid)
 {
 	int status;
 
+	if (surface_hid_is_hot_removed(shid))
+		return -ENODEV;
+
 	status = shid->ops.get_descriptor(shid, SURFACE_HID_DESC_HID,
 			(u8 *)&shid->hid_desc, sizeof(shid->hid_desc));
 	if (status)
@@ -61,6 +79,9 @@ static int surface_hid_load_device_attributes(struct surface_hid_device *shid)
 {
 	int status;
 
+	if (surface_hid_is_hot_removed(shid))
+		return -ENODEV;
+
 	status = shid->ops.get_descriptor(shid, SURFACE_HID_DESC_ATTRS,
 			(u8 *)&shid->attrs, sizeof(shid->attrs));
 	if (status)
@@ -88,9 +109,18 @@ static int surface_hid_start(struct hid_device *hid)
 static void surface_hid_stop(struct hid_device *hid)
 {
 	struct surface_hid_device *shid = hid->driver_data;
+	bool hot_removed;
+
+	/*
+	 * Communication may fail for devices that have been hot-removed. This
+	 * also includes unregistration of HID events, so we need to check this
+	 * here. Only if the device has not been marked as hot-removed, we can
+	 * safely disable events.
+	 */
+	hot_removed = surface_hid_is_hot_removed(shid);
 
 	/* Note: This call will log errors for us, so ignore them here. */
-	ssam_notifier_unregister(shid->ctrl, &shid->notif);
+	__ssam_notifier_unregister(shid->ctrl, &shid->notif, !hot_removed);
 }
 
 static int surface_hid_open(struct hid_device *hid)
@@ -109,6 +139,9 @@ static int surface_hid_parse(struct hid_device *hid)
 	u8 *buf;
 	int status;
 
+	if (surface_hid_is_hot_removed(shid))
+		return -ENODEV;
+
 	buf = kzalloc(len, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
@@ -126,6 +159,9 @@ static int surface_hid_raw_request(struct hid_device *hid, unsigned char reportn
 {
 	struct surface_hid_device *shid = hid->driver_data;
 
+	if (surface_hid_is_hot_removed(shid))
+		return -ENODEV;
+
 	if (rtype == HID_OUTPUT_REPORT && reqtype == HID_REQ_SET_REPORT)
 		return shid->ops.output_report(shid, reportnum, buf, len);
 
-- 
2.34.0

From 4b49b3f485689ddf1e9fbb4280c7ab4a8eb405ff Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 31 Oct 2021 12:34:08 +0100
Subject: [PATCH] platform/surface: aggregator: Add comment for KIP subsystem
 category

The KIP subsystem (full name unknown, abbreviation has been obtained
through reverse engineering) handles detachable peripherals such as the
keyboard cover on the Surface Pro X and Surface Pro 8.

It is currently not entirely clear what this subsystem entails, but at
the very least it provides event notifications for when the keyboard
cover on the Surface Pro X and Surface Pro 8 have been detached or
re-attached, as well as the state that the keyboard cover is currently
in (e.g. folded-back, folded laptop-like, closed, etc.).

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 include/linux/surface_aggregator/serial_hub.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/surface_aggregator/serial_hub.h b/include/linux/surface_aggregator/serial_hub.h
index c3de43edcffa..d1efac85caf1 100644
--- a/include/linux/surface_aggregator/serial_hub.h
+++ b/include/linux/surface_aggregator/serial_hub.h
@@ -306,7 +306,7 @@ enum ssam_ssh_tc {
 	SSAM_SSH_TC_LPC = 0x0b,
 	SSAM_SSH_TC_TCL = 0x0c,
 	SSAM_SSH_TC_SFL = 0x0d,
-	SSAM_SSH_TC_KIP = 0x0e,
+	SSAM_SSH_TC_KIP = 0x0e, /* Manages detachable peripherals (Pro X/8 keyboard cover) */
 	SSAM_SSH_TC_EXT = 0x0f,
 	SSAM_SSH_TC_BLD = 0x10,
 	SSAM_SSH_TC_BAS = 0x11,	/* Detachment system (Surface Book 2/3). */
-- 
2.34.0

From e0933caec88e7140b057a8ce85a21b9797fd567c Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 10 Oct 2021 23:56:23 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Add KIP device hub

Add a Surface System Aggregator Module (SSAM) client device hub for
hot-removable devices managed via the KIP subsystem.

The KIP subsystem (full name unknown, abbreviation has been obtained
through reverse engineering) is a subsystem that manages hot-removable
SSAM client devices. Specifically, it manages HID input devices
contained in the detachable keyboard cover of the Surface Pro 8 and
Surface Pro X.

To properly handle detachable devices, we need to remove their kernel
representation when the physical device has been detached and (re-)add
and (re-)initialize said representation when the physical device has
been (re-)attached. Note that we need to hot-remove those devices, as
communication (especially during event notifier unregistration) may time
out when the physical device is no longer present, which would lead to
an unnecessary delay. This delay might become problematic when devices
are detached and re-attached quickly.

The KIP subsystem handles a single group of devices (e.g. all devices
contained in the keyboard cover) and cannot handle devices individually.
Thus we model it as a client device hub, which removes all devices
contained under it once removal of the hub (e.g. keyboard cover) has
been detected and (re-)adds all devices once the physical hub device has
been (re-)attached.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_registry.c     | 247 +++++++++++++++++-
 1 file changed, 245 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 9f630e890ff7..4838ce6519a6 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -514,6 +514,237 @@ static struct ssam_device_driver ssam_base_hub_driver = {
 };
 
 
+/* -- SSAM KIP-subsystem hub driver. ---------------------------------------- */
+
+/*
+ * Some devices may need a bit of time to be fully usable after being
+ * (re-)connected. This delay has been determined via experimentation.
+ */
+#define SSAM_KIP_UPDATE_CONNECT_DELAY		msecs_to_jiffies(250)
+
+#define SSAM_EVENT_KIP_CID_CONNECTION		0x2c
+
+enum ssam_kip_hub_state {
+	SSAM_KIP_HUB_UNINITIALIZED,
+	SSAM_KIP_HUB_CONNECTED,
+	SSAM_KIP_HUB_DISCONNECTED,
+};
+
+struct ssam_kip_hub {
+	struct ssam_device *sdev;
+
+	enum ssam_kip_hub_state state;
+	struct delayed_work update_work;
+
+	struct ssam_event_notifier notif;
+};
+
+SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_get_connection_state, u8, {
+	.target_category = SSAM_SSH_TC_KIP,
+	.target_id       = 0x01,
+	.command_id      = 0x2c,
+	.instance_id     = 0x00,
+});
+
+static int ssam_kip_get_connection_state(struct ssam_kip_hub *hub, enum ssam_kip_hub_state *state)
+{
+	int status;
+	u8 connected;
+
+	status = ssam_retry(__ssam_kip_get_connection_state, hub->sdev->ctrl, &connected);
+	if (status < 0) {
+		dev_err(&hub->sdev->dev, "failed to query KIP connection state: %d\n", status);
+		return status;
+	}
+
+	*state = connected ? SSAM_KIP_HUB_CONNECTED : SSAM_KIP_HUB_DISCONNECTED;
+	return 0;
+}
+
+static ssize_t ssam_kip_hub_state_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct ssam_kip_hub *hub = dev_get_drvdata(dev);
+	const char *state;
+
+	switch (hub->state) {
+	case SSAM_KIP_HUB_UNINITIALIZED:
+		state = "uninitialized";
+		break;
+
+	case SSAM_KIP_HUB_CONNECTED:
+		state = "connected";
+		break;
+
+	case SSAM_KIP_HUB_DISCONNECTED:
+		state = "disconnected";
+		break;
+
+	default:
+		/*
+		 * Any value not handled in the above cases is invalid and
+		 * should never have been set. Thus this case should be
+		 * impossible to reach.
+		 */
+		WARN(1, "invalid KIP hub state: %d\n", hub->state);
+		state = "<invalid>";
+		break;
+	}
+
+	return sysfs_emit(buf, "%s\n", state);
+}
+
+static struct device_attribute ssam_kip_hub_attr_state =
+	__ATTR(state, 0444, ssam_kip_hub_state_show, NULL);
+
+static struct attribute *ssam_kip_hub_attrs[] = {
+	&ssam_kip_hub_attr_state.attr,
+	NULL,
+};
+
+static const struct attribute_group ssam_kip_hub_group = {
+	.attrs = ssam_kip_hub_attrs,
+};
+
+static int ssam_kip_hub_mark_hot_removed(struct device *dev, void *_data)
+{
+	struct ssam_device *sdev = to_ssam_device(dev);
+
+	if (is_ssam_device(dev))
+		ssam_device_mark_hot_removed(sdev);
+
+	return 0;
+}
+
+static void ssam_kip_hub_update_workfn(struct work_struct *work)
+{
+	struct ssam_kip_hub *hub = container_of(work, struct ssam_kip_hub, update_work.work);
+	struct fwnode_handle *node = dev_fwnode(&hub->sdev->dev);
+	enum ssam_kip_hub_state state;
+	int status = 0;
+
+	status = ssam_kip_get_connection_state(hub, &state);
+	if (status)
+		return;
+
+	if (hub->state == state)
+		return;
+	hub->state = state;
+
+	if (hub->state == SSAM_KIP_HUB_CONNECTED)
+		status = ssam_hub_register_clients(&hub->sdev->dev, hub->sdev->ctrl, node);
+	else
+		ssam_remove_clients(&hub->sdev->dev);
+
+	if (status)
+		dev_err(&hub->sdev->dev, "failed to update KIP-hub devices: %d\n", status);
+}
+
+static u32 ssam_kip_hub_notif(struct ssam_event_notifier *nf, const struct ssam_event *event)
+{
+	struct ssam_kip_hub *hub = container_of(nf, struct ssam_kip_hub, notif);
+	unsigned long delay;
+
+	if (event->command_id != SSAM_EVENT_KIP_CID_CONNECTION)
+		return 0;	/* Return "unhandled". */
+
+	if (event->length < 1) {
+		dev_err(&hub->sdev->dev, "unexpected payload size: %u\n", event->length);
+		return 0;
+	}
+
+	/* Mark devices as hot-removed before we remove any */
+	if (!event->data[0])
+		device_for_each_child_reverse(&hub->sdev->dev, NULL, ssam_kip_hub_mark_hot_removed);
+
+	/*
+	 * Delay update when KIP devices are being connected to give devices/EC
+	 * some time to set up.
+	 */
+	delay = event->data[0] ? SSAM_KIP_UPDATE_CONNECT_DELAY : 0;
+
+	schedule_delayed_work(&hub->update_work, delay);
+	return SSAM_NOTIF_HANDLED;
+}
+
+static int __maybe_unused ssam_kip_hub_resume(struct device *dev)
+{
+	struct ssam_kip_hub *hub = dev_get_drvdata(dev);
+
+	schedule_delayed_work(&hub->update_work, 0);
+	return 0;
+}
+static SIMPLE_DEV_PM_OPS(ssam_kip_hub_pm_ops, NULL, ssam_kip_hub_resume);
+
+static int ssam_kip_hub_probe(struct ssam_device *sdev)
+{
+	struct ssam_kip_hub *hub;
+	int status;
+
+	hub = devm_kzalloc(&sdev->dev, sizeof(*hub), GFP_KERNEL);
+	if (!hub)
+		return -ENOMEM;
+
+	hub->sdev = sdev;
+	hub->state = SSAM_KIP_HUB_UNINITIALIZED;
+
+	hub->notif.base.priority = INT_MAX;  /* This notifier should run first. */
+	hub->notif.base.fn = ssam_kip_hub_notif;
+	hub->notif.event.reg = SSAM_EVENT_REGISTRY_SAM;
+	hub->notif.event.id.target_category = SSAM_SSH_TC_KIP,
+	hub->notif.event.id.instance = 0,
+	hub->notif.event.mask = SSAM_EVENT_MASK_TARGET;
+	hub->notif.event.flags = SSAM_EVENT_SEQUENCED;
+
+	INIT_DELAYED_WORK(&hub->update_work, ssam_kip_hub_update_workfn);
+
+	ssam_device_set_drvdata(sdev, hub);
+
+	status = ssam_device_notifier_register(sdev, &hub->notif);
+	if (status)
+		return status;
+
+	status = sysfs_create_group(&sdev->dev.kobj, &ssam_kip_hub_group);
+	if (status)
+		goto err;
+
+	schedule_delayed_work(&hub->update_work, 0);
+	return 0;
+
+err:
+	ssam_device_notifier_unregister(sdev, &hub->notif);
+	cancel_delayed_work_sync(&hub->update_work);
+	ssam_remove_clients(&sdev->dev);
+	return status;
+}
+
+static void ssam_kip_hub_remove(struct ssam_device *sdev)
+{
+	struct ssam_kip_hub *hub = ssam_device_get_drvdata(sdev);
+
+	sysfs_remove_group(&sdev->dev.kobj, &ssam_kip_hub_group);
+
+	ssam_device_notifier_unregister(sdev, &hub->notif);
+	cancel_delayed_work_sync(&hub->update_work);
+	ssam_remove_clients(&sdev->dev);
+}
+
+static const struct ssam_device_id ssam_kip_hub_match[] = {
+	{ SSAM_SDEV(KIP, 0x01, 0x00, 0x00) },
+	{ },
+};
+
+static struct ssam_device_driver ssam_kip_hub_driver = {
+	.probe = ssam_kip_hub_probe,
+	.remove = ssam_kip_hub_remove,
+	.match_table = ssam_kip_hub_match,
+	.driver = {
+		.name = "surface_kip_hub",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.pm = &ssam_kip_hub_pm_ops,
+	},
+};
+
+
 /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
 
 static const struct acpi_device_id ssam_platform_hub_match[] = {
@@ -636,18 +867,30 @@ static int __init ssam_device_hub_init(void)
 
 	status = platform_driver_register(&ssam_platform_hub_driver);
 	if (status)
-		return status;
+		goto err_platform;
 
 	status = ssam_device_driver_register(&ssam_base_hub_driver);
 	if (status)
-		platform_driver_unregister(&ssam_platform_hub_driver);
+		goto err_base;
+
+	status = ssam_device_driver_register(&ssam_kip_hub_driver);
+	if (status)
+		goto err_kip;
 
+	return 0;
+
+err_kip:
+	ssam_device_driver_unregister(&ssam_base_hub_driver);
+err_base:
+	platform_driver_unregister(&ssam_platform_hub_driver);
+err_platform:
 	return status;
 }
 module_init(ssam_device_hub_init);
 
 static void __exit ssam_device_hub_exit(void)
 {
+	ssam_device_driver_unregister(&ssam_kip_hub_driver);
 	ssam_device_driver_unregister(&ssam_base_hub_driver);
 	platform_driver_unregister(&ssam_platform_hub_driver);
 }
-- 
2.34.0

From 0fc4fd278cac0d5f3274656187d706f75ef48008 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 27 Oct 2021 22:33:03 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Add support for
 keyboard cover on Surface Pro 8

Add support for the detachable keyboard cover on the Surface Pro 8.

The keyboard cover on the Surface Pro 8 is, unlike the keyboard covers
of earlier Surface Pro generations, handled via the Surface System
Aggregator Module (SSAM). The keyboard and touchpad (as well as other
HID input devices) of this cover are standard SSAM HID client devices
(just like keyboard and touchpad on e.g. the Surface Laptop 3 and 4),
however, some care needs to be taken as they can be physically detached
(similarly to the Surface Book 3). Specifically, the respective SSAM
client devices need to be removed when the keyboard cover has been
detached and (re-)initialized when the keyboard cover has been
(re-)attached.

On the Surface Pro 8, detachment of the keyboard cover (and by extension
its devices) is managed via the KIP subsystem. Therefore, said devices
need to be registered under the KIP device hub, which in turn will
remove and re-create/re-initialize those devices as needed.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_registry.c     | 37 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 4838ce6519a6..c0e29c0514df 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -47,6 +47,12 @@ static const struct software_node ssam_node_hub_base = {
 	.parent = &ssam_node_root,
 };
 
+/* KIP device hub (connects keyboard cover devices on Surface Pro 8). */
+static const struct software_node ssam_node_hub_kip = {
+	.name = "ssam:01:0e:01:00:00",
+	.parent = &ssam_node_root,
+};
+
 /* AC adapter. */
 static const struct software_node ssam_node_bat_ac = {
 	.name = "ssam:01:02:01:01:01",
@@ -155,6 +161,30 @@ static const struct software_node ssam_node_hid_base_iid6 = {
 	.parent = &ssam_node_hub_base,
 };
 
+/* HID keyboard (KIP hub). */
+static const struct software_node ssam_node_hid_kip_keyboard = {
+	.name = "ssam:01:15:02:01:00",
+	.parent = &ssam_node_hub_kip,
+};
+
+/* HID pen stash (KIP hub; pen taken / stashed away evens). */
+static const struct software_node ssam_node_hid_kip_penstash = {
+	.name = "ssam:01:15:02:02:00",
+	.parent = &ssam_node_hub_kip,
+};
+
+/* HID touchpad (KIP hub). */
+static const struct software_node ssam_node_hid_kip_touchpad = {
+	.name = "ssam:01:15:02:03:00",
+	.parent = &ssam_node_hub_kip,
+};
+
+/* HID device instance 5 (KIP hub, unknown HID device). */
+static const struct software_node ssam_node_hid_kip_iid5 = {
+	.name = "ssam:01:15:02:05:00",
+	.parent = &ssam_node_hub_kip,
+};
+
 /*
  * Devices for 5th- and 6th-generations models:
  * - Surface Book 2,
@@ -230,10 +260,15 @@ static const struct software_node *ssam_node_group_sp7[] = {
 
 static const struct software_node *ssam_node_group_sp8[] = {
 	&ssam_node_root,
+	&ssam_node_hub_kip,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
-	/* TODO: Add support for keyboard cover. */
+	&ssam_node_hid_kip_keyboard,
+	&ssam_node_hid_kip_penstash,
+	&ssam_node_hid_kip_touchpad,
+	&ssam_node_hid_kip_iid5,
+	/* TODO: Add support for tablet mode switch. */
 	NULL,
 };
 
-- 
2.34.0

From 50b107f14ef9e33f8510d726630138f69f2cf480 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 8 Jun 2021 03:19:20 +0200
Subject: [PATCH] platform/surface: Add KIP tablet-mode switch

Add a driver providing a tablet-mode switch input device for Surface
models using the KIP subsystem to manage detachable peripherals.

The Surface Pro 8 has a detachable keyboard cover. Unlike the keyboard
covers of previous generation Surface Pro models, this cover is fully
handled by the Surface System Aggregator Module (SSAM). The SSAM KIP
subsystem (full name unknown, abbreviation found through reverse
engineering) provides notifications for mode changes of the cover.
Specifically, it allows us to know when the cover has been folded back,
detached, or whether it is in laptop mode.

The driver introduced with this change captures these events and
translates them to standard SW_TABLET_MODE input events.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 MAINTAINERS                                   |   6 +
 drivers/platform/surface/Kconfig              |  22 ++
 drivers/platform/surface/Makefile             |   1 +
 .../surface/surface_kip_tablet_switch.c       | 245 ++++++++++++++++++
 4 files changed, 274 insertions(+)
 create mode 100644 drivers/platform/surface/surface_kip_tablet_switch.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d7b4f32875a9..5713585d060d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12357,6 +12357,12 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/surface/surface_hotplug.c
 
+MICROSOFT SURFACE KIP TABLET-MODE SWITCH
+M:	Maximilian Luz <luzmaximilian@gmail.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/surface/surface_kip_tablet_switch.c
+
 MICROSOFT SURFACE PLATFORM PROFILE DRIVER
 M:	Maximilian Luz <luzmaximilian@gmail.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 3105f651614f..3c0ee0cdaef5 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -152,6 +152,28 @@ config SURFACE_HOTPLUG
 	  Select M or Y here, if you want to (fully) support hot-plugging of
 	  dGPU devices on the Surface Book 2 and/or 3 during D3cold.
 
+config SURFACE_KIP_TABLET_SWITCH
+	tristate "Surface KIP Tablet-Mode Switch Driver"
+	depends on SURFACE_AGGREGATOR
+	depends on SURFACE_AGGREGATOR_BUS
+	depends on INPUT
+	help
+	  Provides a tablet-mode switch input device on Microsoft Surface models
+	  using the KIP subsystem for detachable keyboards (e.g. keyboard
+	  covers).
+
+	  The KIP subsystem is used on newer Surface generations to handle
+	  detachable input peripherals, specifically the keyboard cover
+	  (containing keyboard and touchpad) on the Surface Pro 8. This module
+	  provides a driver to let user-space know when the device should be
+	  considered in tablet-mode due to the keyboard cover being detached or
+	  folded back (essentially signaling when the keyboard is not available
+	  for input). It does so by creating a tablet-mode switch input device,
+	  sending the standard SW_TABLET_MODE event on mode change.
+
+	  Select M or Y here, if you want to provide tablet-mode switch input
+	  events on the Surface Pro 8.
+
 config SURFACE_PLATFORM_PROFILE
 	tristate "Surface Platform Profile Driver"
 	depends on SURFACE_AGGREGATOR_REGISTRY
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 32889482de55..6d9291c993c4 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_SURFACE_AGGREGATOR_REGISTRY) += surface_aggregator_registry.o
 obj-$(CONFIG_SURFACE_DTX)		+= surface_dtx.o
 obj-$(CONFIG_SURFACE_GPE)		+= surface_gpe.o
 obj-$(CONFIG_SURFACE_HOTPLUG)		+= surface_hotplug.o
+obj-$(CONFIG_SURFACE_KIP_TABLET_SWITCH)	+= surface_kip_tablet_switch.o
 obj-$(CONFIG_SURFACE_PLATFORM_PROFILE)	+= surface_platform_profile.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
diff --git a/drivers/platform/surface/surface_kip_tablet_switch.c b/drivers/platform/surface/surface_kip_tablet_switch.c
new file mode 100644
index 000000000000..458470067579
--- /dev/null
+++ b/drivers/platform/surface/surface_kip_tablet_switch.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Surface System Aggregator Module (SSAM) tablet mode switch via KIP
+ * subsystem.
+ *
+ * Copyright (C) 2021 Maximilian Luz <luzmaximilian@gmail.com>
+ */
+
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include <linux/surface_aggregator/controller.h>
+#include <linux/surface_aggregator/device.h>
+
+#define SSAM_EVENT_KIP_CID_LID_STATE		0x1d
+
+enum ssam_kip_lid_state {
+	SSAM_KIP_LID_STATE_DISCONNECTED   = 0x01,
+	SSAM_KIP_LID_STATE_CLOSED         = 0x02,
+	SSAM_KIP_LID_STATE_LAPTOP         = 0x03,
+	SSAM_KIP_LID_STATE_FOLDED_CANVAS  = 0x04,
+	SSAM_KIP_LID_STATE_FOLDED_BACK    = 0x05,
+};
+
+struct ssam_kip_sw {
+	struct ssam_device *sdev;
+
+	enum ssam_kip_lid_state state;
+	struct work_struct update_work;
+	struct input_dev *mode_switch;
+
+	struct ssam_event_notifier notif;
+};
+
+SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_get_lid_state, u8, {
+	.target_category = SSAM_SSH_TC_KIP,
+	.target_id       = 0x01,
+	.command_id      = 0x1d,
+	.instance_id     = 0x00,
+});
+
+static int ssam_kip_get_lid_state(struct ssam_kip_sw *sw, enum ssam_kip_lid_state *state)
+{
+	int status;
+	u8 raw;
+
+	status = ssam_retry(__ssam_kip_get_lid_state, sw->sdev->ctrl, &raw);
+	if (status < 0) {
+		dev_err(&sw->sdev->dev, "failed to query KIP lid state: %d\n", status);
+		return status;
+	}
+
+	*state = raw;
+	return 0;
+}
+
+static ssize_t state_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct ssam_kip_sw *sw = dev_get_drvdata(dev);
+	const char *state;
+
+	switch (sw->state) {
+	case SSAM_KIP_LID_STATE_DISCONNECTED:
+		state = "disconnected";
+		break;
+
+	case SSAM_KIP_LID_STATE_CLOSED:
+		state = "closed";
+		break;
+
+	case SSAM_KIP_LID_STATE_LAPTOP:
+		state = "laptop";
+		break;
+
+	case SSAM_KIP_LID_STATE_FOLDED_CANVAS:
+		state = "folded-canvas";
+		break;
+
+	case SSAM_KIP_LID_STATE_FOLDED_BACK:
+		state = "folded-back";
+		break;
+
+	default:
+		state = "<unknown>";
+		dev_warn(dev, "unknown KIP lid state: %d\n", sw->state);
+		break;
+	}
+
+	return sysfs_emit(buf, "%s\n", state);
+}
+static DEVICE_ATTR_RO(state);
+
+static struct attribute *ssam_kip_sw_attrs[] = {
+	&dev_attr_state.attr,
+	NULL,
+};
+
+static const struct attribute_group ssam_kip_sw_group = {
+	.attrs = ssam_kip_sw_attrs,
+};
+
+static void ssam_kip_sw_update_workfn(struct work_struct *work)
+{
+	struct ssam_kip_sw *sw = container_of(work, struct ssam_kip_sw, update_work);
+	enum ssam_kip_lid_state state;
+	int tablet, status;
+
+	status = ssam_kip_get_lid_state(sw, &state);
+	if (status)
+		return;
+
+	if (sw->state == state)
+		return;
+	sw->state = state;
+
+	/* Send SW_TABLET_MODE event. */
+	tablet = state != SSAM_KIP_LID_STATE_LAPTOP;
+	input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);
+	input_sync(sw->mode_switch);
+}
+
+static u32 ssam_kip_sw_notif(struct ssam_event_notifier *nf, const struct ssam_event *event)
+{
+	struct ssam_kip_sw *sw = container_of(nf, struct ssam_kip_sw, notif);
+
+	if (event->command_id != SSAM_EVENT_KIP_CID_LID_STATE)
+		return 0;	/* Return "unhandled". */
+
+	if (event->length < 1) {
+		dev_err(&sw->sdev->dev, "unexpected payload size: %u\n", event->length);
+		return 0;
+	}
+
+	schedule_work(&sw->update_work);
+	return SSAM_NOTIF_HANDLED;
+}
+
+static int __maybe_unused ssam_kip_sw_resume(struct device *dev)
+{
+	struct ssam_kip_sw *sw = dev_get_drvdata(dev);
+
+	schedule_work(&sw->update_work);
+	return 0;
+}
+static SIMPLE_DEV_PM_OPS(ssam_kip_sw_pm_ops, NULL, ssam_kip_sw_resume);
+
+static int ssam_kip_sw_probe(struct ssam_device *sdev)
+{
+	struct ssam_kip_sw *sw;
+	int tablet, status;
+
+	sw = devm_kzalloc(&sdev->dev, sizeof(*sw), GFP_KERNEL);
+	if (!sw)
+		return -ENOMEM;
+
+	sw->sdev = sdev;
+	INIT_WORK(&sw->update_work, ssam_kip_sw_update_workfn);
+
+	ssam_device_set_drvdata(sdev, sw);
+
+	/* Get initial state. */
+	status = ssam_kip_get_lid_state(sw, &sw->state);
+	if (status)
+		return status;
+
+	/* Set up tablet mode switch. */
+	sw->mode_switch = devm_input_allocate_device(&sdev->dev);
+	if (!sw->mode_switch)
+		return -ENOMEM;
+
+	sw->mode_switch->name = "Microsoft Surface KIP Tablet Mode Switch";
+	sw->mode_switch->phys = "ssam/01:0e:01:00:01/input0";
+	sw->mode_switch->id.bustype = BUS_HOST;
+	sw->mode_switch->dev.parent = &sdev->dev;
+
+	tablet = sw->state != SSAM_KIP_LID_STATE_LAPTOP;
+	input_set_capability(sw->mode_switch, EV_SW, SW_TABLET_MODE);
+	input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);
+
+	status = input_register_device(sw->mode_switch);
+	if (status)
+		return status;
+
+	/* Set up notifier. */
+	sw->notif.base.priority = 0;
+	sw->notif.base.fn = ssam_kip_sw_notif;
+	sw->notif.event.reg = SSAM_EVENT_REGISTRY_SAM;
+	sw->notif.event.id.target_category = SSAM_SSH_TC_KIP,
+	sw->notif.event.id.instance = 0,
+	sw->notif.event.mask = SSAM_EVENT_MASK_TARGET;
+	sw->notif.event.flags = SSAM_EVENT_SEQUENCED;
+
+	status = ssam_device_notifier_register(sdev, &sw->notif);
+	if (status)
+		return status;
+
+	status = sysfs_create_group(&sdev->dev.kobj, &ssam_kip_sw_group);
+	if (status)
+		goto err;
+
+	/* We might have missed events during setup, so check again. */
+	schedule_work(&sw->update_work);
+	return 0;
+
+err:
+	ssam_device_notifier_unregister(sdev, &sw->notif);
+	cancel_work_sync(&sw->update_work);
+	return status;
+}
+
+static void ssam_kip_sw_remove(struct ssam_device *sdev)
+{
+	struct ssam_kip_sw *sw = ssam_device_get_drvdata(sdev);
+
+	sysfs_remove_group(&sdev->dev.kobj, &ssam_kip_sw_group);
+
+	ssam_device_notifier_unregister(sdev, &sw->notif);
+	cancel_work_sync(&sw->update_work);
+}
+
+static const struct ssam_device_id ssam_kip_sw_match[] = {
+	{ SSAM_SDEV(KIP, 0x01, 0x00, 0x01) },
+	{ },
+};
+MODULE_DEVICE_TABLE(ssam, ssam_kip_sw_match);
+
+static struct ssam_device_driver ssam_kip_sw_driver = {
+	.probe = ssam_kip_sw_probe,
+	.remove = ssam_kip_sw_remove,
+	.match_table = ssam_kip_sw_match,
+	.driver = {
+		.name = "surface_kip_tablet_mode_switch",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.pm = &ssam_kip_sw_pm_ops,
+	},
+};
+module_ssam_device_driver(ssam_kip_sw_driver);
+
+MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
+MODULE_DESCRIPTION("Tablet mode switch driver for Surface devices using KIP subsystem");
+MODULE_LICENSE("GPL");
-- 
2.34.0

From 2b1685c0a906769031590c7bfc0793bcc6fb9e98 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 27 Oct 2021 22:33:03 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Add support for tablet
 mode switch on Surface Pro 8

Add a KIP subsystem tablet-mode switch device for the Surface Pro 8.
The respective driver for this device provides SW_TABLET_MODE input
events for user-space based on the state of the keyboard cover (e.g.
detached, folded-back, normal/laptop mode).

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_registry.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index c0e29c0514df..eaf0054627a5 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -77,6 +77,12 @@ static const struct software_node ssam_node_tmp_pprof = {
 	.parent = &ssam_node_root,
 };
 
+/* Tablet-mode switch via KIP subsystem. */
+static const struct software_node ssam_node_kip_tablet_switch = {
+	.name = "ssam:01:0e:01:00:01",
+	.parent = &ssam_node_root,
+};
+
 /* DTX / detachment-system device (Surface Book 3). */
 static const struct software_node ssam_node_bas_dtx = {
 	.name = "ssam:01:11:01:00:00",
@@ -264,11 +270,11 @@ static const struct software_node *ssam_node_group_sp8[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
+	&ssam_node_kip_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
 	&ssam_node_hid_kip_penstash,
 	&ssam_node_hid_kip_touchpad,
 	&ssam_node_hid_kip_iid5,
-	/* TODO: Add support for tablet mode switch. */
 	NULL,
 };
 
-- 
2.34.0

From fea397d2b114521a7e4cbc13998e451ebd9796ad Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Thu, 28 Oct 2021 03:40:22 +0200
Subject: [PATCH] power/supply: surface_battery: Add support for hot-removal

In cases of hot-removal, further communication with the device should be
avoided whenever possible, as it may fail and time out.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/power/supply/surface_battery.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/surface_battery.c b/drivers/power/supply/surface_battery.c
index 540707882bb0..ebf1f96e9a89 100644
--- a/drivers/power/supply/surface_battery.c
+++ b/drivers/power/supply/surface_battery.c
@@ -156,6 +156,9 @@ static bool spwr_battery_present(struct spwr_battery_device *bat)
 {
 	lockdep_assert_held(&bat->lock);
 
+	if (ssam_device_is_hot_removed(bat->sdev))
+		return false;
+
 	return le32_to_cpu(bat->sta) & SAM_BATTERY_STA_PRESENT;
 }
 
@@ -245,6 +248,9 @@ static int spwr_battery_update_bix_unlocked(struct spwr_battery_device *bat)
 
 	lockdep_assert_held(&bat->lock);
 
+	if (ssam_device_is_hot_removed(bat->sdev))
+		return 0;
+
 	status = spwr_battery_load_sta(bat);
 	if (status)
 		return status;
-- 
2.34.0

From 1610b9bfd7295dfdde50ffc777a7d88c6a7e9531 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 31 Oct 2021 18:07:39 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Use KIP hub for
 Surface Book 3 base devices

It turns out that the Surface Book 3 manages the devices contained in
its detachable base via the KIP hub as well, similarly to the Surface
Pro 8 and Surface Pro X. So move them over to the KIP hub.

Right now, we (mis-)use the detachment subsystem (DTX), which is
designed for handling detachment requests and physical locking of the
base, to properly remove and re-attach Surface System Aggregator Module
(SSAM) client devices contained in the base. This system does not seem
to be intended for managing the (sub-)devices contained in the base,
which may need some time to be set up properly.

The KIP subsystem seems to be the intended subsystem for managing those
devices, thus let's use that one instead.

Note that this also changes the way in which devices on the Surface Book
3 are removed when they have been detached, specifically from normal
removal to hot-removal (avoiding further communication with the embedded
controller). It seems that the "communication timeout after device
removal" issue does also occur on the Surface Book 3, but has so far
been missed as it does not happen reliably every time. Switching to
hot-removal fixes this issue and should not have any noticable
drawbacks.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_registry.c     | 56 ++++++-------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index eaf0054627a5..d17f656b2dad 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -41,13 +41,7 @@ static const struct software_node ssam_node_root = {
 	.name = "ssam_platform_hub",
 };
 
-/* Base device hub (devices attached to Surface Book 3 base). */
-static const struct software_node ssam_node_hub_base = {
-	.name = "ssam:00:00:02:00:00",
-	.parent = &ssam_node_root,
-};
-
-/* KIP device hub (connects keyboard cover devices on Surface Pro 8). */
+/* KIP device hub (connects detachable keyboard/touchpad on Surface Pro 8 and Book 3). */
 static const struct software_node ssam_node_hub_kip = {
 	.name = "ssam:01:0e:01:00:00",
 	.parent = &ssam_node_root,
@@ -65,10 +59,10 @@ static const struct software_node ssam_node_bat_main = {
 	.parent = &ssam_node_root,
 };
 
-/* Secondary battery (Surface Book 3). */
-static const struct software_node ssam_node_bat_sb3base = {
+/* Secondary battery (Surface Book 3, managed via KIP hub). */
+static const struct software_node ssam_node_bat_kip = {
 	.name = "ssam:01:02:02:01:00",
-	.parent = &ssam_node_hub_base,
+	.parent = &ssam_node_hub_kip,
 };
 
 /* Platform profile / performance-mode device. */
@@ -143,30 +137,6 @@ static const struct software_node ssam_node_hid_main_iid5 = {
 	.parent = &ssam_node_root,
 };
 
-/* HID keyboard (base hub). */
-static const struct software_node ssam_node_hid_base_keyboard = {
-	.name = "ssam:01:15:02:01:00",
-	.parent = &ssam_node_hub_base,
-};
-
-/* HID touchpad (base hub). */
-static const struct software_node ssam_node_hid_base_touchpad = {
-	.name = "ssam:01:15:02:03:00",
-	.parent = &ssam_node_hub_base,
-};
-
-/* HID device instance 5 (unknown HID device, base hub). */
-static const struct software_node ssam_node_hid_base_iid5 = {
-	.name = "ssam:01:15:02:05:00",
-	.parent = &ssam_node_hub_base,
-};
-
-/* HID device instance 6 (unknown HID device, base hub). */
-static const struct software_node ssam_node_hid_base_iid6 = {
-	.name = "ssam:01:15:02:06:00",
-	.parent = &ssam_node_hub_base,
-};
-
 /* HID keyboard (KIP hub). */
 static const struct software_node ssam_node_hid_kip_keyboard = {
 	.name = "ssam:01:15:02:01:00",
@@ -191,6 +161,12 @@ static const struct software_node ssam_node_hid_kip_iid5 = {
 	.parent = &ssam_node_hub_kip,
 };
 
+/* HID device instance 6 (KIP hub, unknown HID device). */
+static const struct software_node ssam_node_hid_kip_iid6 = {
+	.name = "ssam:01:15:02:06:00",
+	.parent = &ssam_node_hub_kip,
+};
+
 /*
  * Devices for 5th- and 6th-generations models:
  * - Surface Book 2,
@@ -206,16 +182,16 @@ static const struct software_node *ssam_node_group_gen5[] = {
 /* Devices for Surface Book 3. */
 static const struct software_node *ssam_node_group_sb3[] = {
 	&ssam_node_root,
-	&ssam_node_hub_base,
+	&ssam_node_hub_kip,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_bat_sb3base,
+	&ssam_node_bat_kip,
 	&ssam_node_tmp_pprof,
 	&ssam_node_bas_dtx,
-	&ssam_node_hid_base_keyboard,
-	&ssam_node_hid_base_touchpad,
-	&ssam_node_hid_base_iid5,
-	&ssam_node_hid_base_iid6,
+	&ssam_node_hid_kip_keyboard,
+	&ssam_node_hid_kip_touchpad,
+	&ssam_node_hid_kip_iid5,
+	&ssam_node_hid_kip_iid6,
 	NULL,
 };
 
-- 
2.34.0

From 236abbbef8f8a528621e839383e357f50591cb7c Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 31 Oct 2021 18:09:53 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Remove base hub driver

The base hub was a virtual device hub for Surface System Aggregator
Module (SSAM) client devices contained in the detachable Surface Book 3
base. Remove it as it is no longer needed.

In the previous change, we have moved all devices from the base hub to
the KIP hub. That change has also removed the only base-hub-device that
ever existed, as it was essentially replaced by the KIP-hub-device and
thus was no longer needed. This means that there is no remaining
hub-device against which the base hub driver can load, i.e. it is now
essentially dead code. So remove the driver as well.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_registry.c     | 213 ------------------
 1 file changed, 213 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index d17f656b2dad..590473220e9d 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -325,212 +325,6 @@ static int ssam_hub_register_clients(struct device *parent, struct ssam_controll
 }
 
 
-/* -- SSAM base-hub driver. ------------------------------------------------- */
-
-/*
- * Some devices (especially battery) may need a bit of time to be fully usable
- * after being (re-)connected. This delay has been determined via
- * experimentation.
- */
-#define SSAM_BASE_UPDATE_CONNECT_DELAY		msecs_to_jiffies(2500)
-
-enum ssam_base_hub_state {
-	SSAM_BASE_HUB_UNINITIALIZED,
-	SSAM_BASE_HUB_CONNECTED,
-	SSAM_BASE_HUB_DISCONNECTED,
-};
-
-struct ssam_base_hub {
-	struct ssam_device *sdev;
-
-	enum ssam_base_hub_state state;
-	struct delayed_work update_work;
-
-	struct ssam_event_notifier notif;
-};
-
-SSAM_DEFINE_SYNC_REQUEST_R(ssam_bas_query_opmode, u8, {
-	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
-	.command_id      = 0x0d,
-	.instance_id     = 0x00,
-});
-
-#define SSAM_BAS_OPMODE_TABLET		0x00
-#define SSAM_EVENT_BAS_CID_CONNECTION	0x0c
-
-static int ssam_base_hub_query_state(struct ssam_base_hub *hub, enum ssam_base_hub_state *state)
-{
-	u8 opmode;
-	int status;
-
-	status = ssam_retry(ssam_bas_query_opmode, hub->sdev->ctrl, &opmode);
-	if (status < 0) {
-		dev_err(&hub->sdev->dev, "failed to query base state: %d\n", status);
-		return status;
-	}
-
-	if (opmode != SSAM_BAS_OPMODE_TABLET)
-		*state = SSAM_BASE_HUB_CONNECTED;
-	else
-		*state = SSAM_BASE_HUB_DISCONNECTED;
-
-	return 0;
-}
-
-static ssize_t ssam_base_hub_state_show(struct device *dev, struct device_attribute *attr,
-					char *buf)
-{
-	struct ssam_base_hub *hub = dev_get_drvdata(dev);
-	bool connected = hub->state == SSAM_BASE_HUB_CONNECTED;
-
-	return sysfs_emit(buf, "%d\n", connected);
-}
-
-static struct device_attribute ssam_base_hub_attr_state =
-	__ATTR(state, 0444, ssam_base_hub_state_show, NULL);
-
-static struct attribute *ssam_base_hub_attrs[] = {
-	&ssam_base_hub_attr_state.attr,
-	NULL,
-};
-
-static const struct attribute_group ssam_base_hub_group = {
-	.attrs = ssam_base_hub_attrs,
-};
-
-static void ssam_base_hub_update_workfn(struct work_struct *work)
-{
-	struct ssam_base_hub *hub = container_of(work, struct ssam_base_hub, update_work.work);
-	struct fwnode_handle *node = dev_fwnode(&hub->sdev->dev);
-	enum ssam_base_hub_state state;
-	int status = 0;
-
-	status = ssam_base_hub_query_state(hub, &state);
-	if (status)
-		return;
-
-	if (hub->state == state)
-		return;
-	hub->state = state;
-
-	if (hub->state == SSAM_BASE_HUB_CONNECTED)
-		status = ssam_hub_register_clients(&hub->sdev->dev, hub->sdev->ctrl, node);
-	else
-		ssam_remove_clients(&hub->sdev->dev);
-
-	if (status)
-		dev_err(&hub->sdev->dev, "failed to update base-hub devices: %d\n", status);
-}
-
-static u32 ssam_base_hub_notif(struct ssam_event_notifier *nf, const struct ssam_event *event)
-{
-	struct ssam_base_hub *hub = container_of(nf, struct ssam_base_hub, notif);
-	unsigned long delay;
-
-	if (event->command_id != SSAM_EVENT_BAS_CID_CONNECTION)
-		return 0;
-
-	if (event->length < 1) {
-		dev_err(&hub->sdev->dev, "unexpected payload size: %u\n", event->length);
-		return 0;
-	}
-
-	/*
-	 * Delay update when the base is being connected to give devices/EC
-	 * some time to set up.
-	 */
-	delay = event->data[0] ? SSAM_BASE_UPDATE_CONNECT_DELAY : 0;
-
-	schedule_delayed_work(&hub->update_work, delay);
-
-	/*
-	 * Do not return SSAM_NOTIF_HANDLED: The event should be picked up and
-	 * consumed by the detachment system driver. We're just a (more or less)
-	 * silent observer.
-	 */
-	return 0;
-}
-
-static int __maybe_unused ssam_base_hub_resume(struct device *dev)
-{
-	struct ssam_base_hub *hub = dev_get_drvdata(dev);
-
-	schedule_delayed_work(&hub->update_work, 0);
-	return 0;
-}
-static SIMPLE_DEV_PM_OPS(ssam_base_hub_pm_ops, NULL, ssam_base_hub_resume);
-
-static int ssam_base_hub_probe(struct ssam_device *sdev)
-{
-	struct ssam_base_hub *hub;
-	int status;
-
-	hub = devm_kzalloc(&sdev->dev, sizeof(*hub), GFP_KERNEL);
-	if (!hub)
-		return -ENOMEM;
-
-	hub->sdev = sdev;
-	hub->state = SSAM_BASE_HUB_UNINITIALIZED;
-
-	hub->notif.base.priority = INT_MAX;  /* This notifier should run first. */
-	hub->notif.base.fn = ssam_base_hub_notif;
-	hub->notif.event.reg = SSAM_EVENT_REGISTRY_SAM;
-	hub->notif.event.id.target_category = SSAM_SSH_TC_BAS,
-	hub->notif.event.id.instance = 0,
-	hub->notif.event.mask = SSAM_EVENT_MASK_NONE;
-	hub->notif.event.flags = SSAM_EVENT_SEQUENCED;
-
-	INIT_DELAYED_WORK(&hub->update_work, ssam_base_hub_update_workfn);
-
-	ssam_device_set_drvdata(sdev, hub);
-
-	status = ssam_device_notifier_register(sdev, &hub->notif);
-	if (status)
-		return status;
-
-	status = sysfs_create_group(&sdev->dev.kobj, &ssam_base_hub_group);
-	if (status)
-		goto err;
-
-	schedule_delayed_work(&hub->update_work, 0);
-	return 0;
-
-err:
-	ssam_device_notifier_unregister(sdev, &hub->notif);
-	cancel_delayed_work_sync(&hub->update_work);
-	ssam_remove_clients(&sdev->dev);
-	return status;
-}
-
-static void ssam_base_hub_remove(struct ssam_device *sdev)
-{
-	struct ssam_base_hub *hub = ssam_device_get_drvdata(sdev);
-
-	sysfs_remove_group(&sdev->dev.kobj, &ssam_base_hub_group);
-
-	ssam_device_notifier_unregister(sdev, &hub->notif);
-	cancel_delayed_work_sync(&hub->update_work);
-	ssam_remove_clients(&sdev->dev);
-}
-
-static const struct ssam_device_id ssam_base_hub_match[] = {
-	{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
-	{ },
-};
-
-static struct ssam_device_driver ssam_base_hub_driver = {
-	.probe = ssam_base_hub_probe,
-	.remove = ssam_base_hub_remove,
-	.match_table = ssam_base_hub_match,
-	.driver = {
-		.name = "surface_aggregator_base_hub",
-		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
-		.pm = &ssam_base_hub_pm_ops,
-	},
-};
-
-
 /* -- SSAM KIP-subsystem hub driver. ---------------------------------------- */
 
 /*
@@ -886,10 +680,6 @@ static int __init ssam_device_hub_init(void)
 	if (status)
 		goto err_platform;
 
-	status = ssam_device_driver_register(&ssam_base_hub_driver);
-	if (status)
-		goto err_base;
-
 	status = ssam_device_driver_register(&ssam_kip_hub_driver);
 	if (status)
 		goto err_kip;
@@ -897,8 +687,6 @@ static int __init ssam_device_hub_init(void)
 	return 0;
 
 err_kip:
-	ssam_device_driver_unregister(&ssam_base_hub_driver);
-err_base:
 	platform_driver_unregister(&ssam_platform_hub_driver);
 err_platform:
 	return status;
@@ -908,7 +696,6 @@ module_init(ssam_device_hub_init);
 static void __exit ssam_device_hub_exit(void)
 {
 	ssam_device_driver_unregister(&ssam_kip_hub_driver);
-	ssam_device_driver_unregister(&ssam_base_hub_driver);
 	platform_driver_unregister(&ssam_platform_hub_driver);
 }
 module_exit(ssam_device_hub_exit);
-- 
2.34.0