linux-surface/patches/6.1/0006-surface-sam.patch

From b485239f375af4576846cfeb3d3de9338d7c7fca Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:20 +0100
Subject: [PATCH] platform/surface: aggregator: Improve documentation and
 handling of message target and source IDs

The `tid_in` and `tid_out` fields of the serial hub protocol command
struct (struct ssh_command) are actually source and target IDs,
indicating the peer from which the message originated and the peer for
which it is intended.

Change the naming of those fields accordingly and improve the protocol
documentation. Additionally, introduce an enum containing all currently
known peers, i.e. targets and sources.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-3-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 .../driver-api/surface_aggregator/client.rst  |  4 +-
 .../driver-api/surface_aggregator/ssh.rst     | 36 +++++++++--------
 .../platform/surface/aggregator/controller.c  | 12 +++---
 .../platform/surface/aggregator/ssh_msgb.h    |  4 +-
 .../surface/aggregator/ssh_request_layer.c    | 11 ++---
 include/linux/surface_aggregator/controller.h |  4 +-
 include/linux/surface_aggregator/serial_hub.h | 40 +++++++++++++------
 7 files changed, 64 insertions(+), 47 deletions(-)

diff --git a/Documentation/driver-api/surface_aggregator/client.rst b/Documentation/driver-api/surface_aggregator/client.rst
index 27f95abdbe997..9d7411223a848 100644
--- a/Documentation/driver-api/surface_aggregator/client.rst
+++ b/Documentation/driver-api/surface_aggregator/client.rst
@@ -191,7 +191,7 @@ data received from it is converted from little-endian to host endianness.
             *       they do not correspond to an actual SAM/EC request.
             */
            rqst.target_category = SSAM_SSH_TC_SAM;
-           rqst.target_id = 0x01;
+           rqst.target_id = SSAM_SSH_TID_SAM;
            rqst.command_id = 0x02;
            rqst.instance_id = 0x03;
            rqst.flags = SSAM_REQUEST_HAS_RESPONSE;
@@ -241,7 +241,7 @@ one of the generator macros, for example via:
 
    SSAM_DEFINE_SYNC_REQUEST_W(__ssam_tmp_perf_mode_set, __le32, {
            .target_category = SSAM_SSH_TC_TMP,
-           .target_id       = 0x01,
+           .target_id       = SSAM_SSH_TID_SAM,
            .command_id      = 0x03,
            .instance_id     = 0x00,
    });
diff --git a/Documentation/driver-api/surface_aggregator/ssh.rst b/Documentation/driver-api/surface_aggregator/ssh.rst
index bf007d6c98732..18fd0f0aee84b 100644
--- a/Documentation/driver-api/surface_aggregator/ssh.rst
+++ b/Documentation/driver-api/surface_aggregator/ssh.rst
@@ -13,6 +13,7 @@
 .. |DATA_NSQ| replace:: ``DATA_NSQ``
 .. |TC| replace:: ``TC``
 .. |TID| replace:: ``TID``
+.. |SID| replace:: ``SID``
 .. |IID| replace:: ``IID``
 .. |RQID| replace:: ``RQID``
 .. |CID| replace:: ``CID``
@@ -219,13 +220,13 @@ following fields, packed together and in order:
      - |u8|
      - Target category.
 
-   * - |TID| (out)
+   * - |TID|
      - |u8|
-     - Target ID for outgoing (host to EC) commands.
+     - Target ID for commands/messages.
 
-   * - |TID| (in)
+   * - |SID|
      - |u8|
-     - Target ID for incoming (EC to host) commands.
+     - Source ID for commands/messages.
 
    * - |IID|
      - |u8|
@@ -286,19 +287,20 @@ general, however, a single target category should map to a single reserved
 event request ID.
 
 Furthermore, requests, responses, and events have an associated target ID
-(``TID``). This target ID is split into output (host to EC) and input (EC to
-host) fields, with the respecting other field (e.g. output field on incoming
-messages) set to zero. Two ``TID`` values are known: Primary (``0x01``) and
-secondary (``0x02``). In general, the response to a request should have the
-same ``TID`` value, however, the field (output vs. input) should be used in
-accordance to the direction in which the response is sent (i.e. on the input
-field, as responses are generally sent from the EC to the host).
-
-Note that, even though requests and events should be uniquely identifiable
-by target category and command ID alone, the EC may require specific
-target ID and instance ID values to accept a command. A command that is
-accepted for ``TID=1``, for example, may not be accepted for ``TID=2``
-and vice versa.
+(``TID``) and source ID (``SID``). These two fields indicate where a message
+originates from (``SID``) and what the intended target of the message is
+(``TID``). Note that a response to a specific request therefore has the source
+and target IDs swapped when compared to the original request (i.e. the request
+target is the response source and the request source is the response target).
+See (:c:type:`enum ssh_request_id <ssh_request_id>`) for possible values of
+both.
+
+Note that, even though requests and events should be uniquely identifiable by
+target category and command ID alone, the EC may require specific target ID and
+instance ID values to accept a command. A command that is accepted for
+``TID=1``, for example, may not be accepted for ``TID=2`` and vice versa. While
+this may not always hold in reality, you can think of different target/source
+IDs indicating different physical ECs with potentially different feature sets.
 
 
 Limitations and Observations
diff --git a/drivers/platform/surface/aggregator/controller.c b/drivers/platform/surface/aggregator/controller.c
index 30cea324ff95f..cb86760622762 100644
--- a/drivers/platform/surface/aggregator/controller.c
+++ b/drivers/platform/surface/aggregator/controller.c
@@ -994,7 +994,7 @@ static void ssam_handle_event(struct ssh_rtl *rtl,
 
 	item->rqid = get_unaligned_le16(&cmd->rqid);
 	item->event.target_category = cmd->tc;
-	item->event.target_id = cmd->tid_in;
+	item->event.target_id = cmd->sid;
 	item->event.command_id = cmd->cid;
 	item->event.instance_id = cmd->iid;
 	memcpy(&item->event.data[0], data->ptr, data->len);
@@ -1779,35 +1779,35 @@ EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer);
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, {
 	.target_category = SSAM_SSH_TC_SAM,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x13,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, {
 	.target_category = SSAM_SSH_TC_SAM,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x15,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, {
 	.target_category = SSAM_SSH_TC_SAM,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x16,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_exit, u8, {
 	.target_category = SSAM_SSH_TC_SAM,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x33,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_entry, u8, {
 	.target_category = SSAM_SSH_TC_SAM,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x34,
 	.instance_id     = 0x00,
 });
diff --git a/drivers/platform/surface/aggregator/ssh_msgb.h b/drivers/platform/surface/aggregator/ssh_msgb.h
index f3ecad92eefd8..438873e060986 100644
--- a/drivers/platform/surface/aggregator/ssh_msgb.h
+++ b/drivers/platform/surface/aggregator/ssh_msgb.h
@@ -189,8 +189,8 @@ static inline void msgb_push_cmd(struct msgbuf *msgb, u8 seq, u16 rqid,
 
 	__msgb_push_u8(msgb, SSH_PLD_TYPE_CMD);		/* Payload type. */
 	__msgb_push_u8(msgb, rqst->target_category);	/* Target category. */
-	__msgb_push_u8(msgb, rqst->target_id);		/* Target ID (out). */
-	__msgb_push_u8(msgb, 0x00);			/* Target ID (in). */
+	__msgb_push_u8(msgb, rqst->target_id);		/* Target ID. */
+	__msgb_push_u8(msgb, SSAM_SSH_TID_HOST);	/* Source ID. */
 	__msgb_push_u8(msgb, rqst->instance_id);	/* Instance ID. */
 	__msgb_push_u16(msgb, rqid);			/* Request ID. */
 	__msgb_push_u8(msgb, rqst->command_id);		/* Command ID. */
diff --git a/drivers/platform/surface/aggregator/ssh_request_layer.c b/drivers/platform/surface/aggregator/ssh_request_layer.c
index 69132976d297e..90634dcacabf2 100644
--- a/drivers/platform/surface/aggregator/ssh_request_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_request_layer.c
@@ -920,13 +920,14 @@ static void ssh_rtl_rx_command(struct ssh_ptl *p, const struct ssam_span *data)
 	 * Check if the message was intended for us. If not, drop it.
 	 *
 	 * Note: We will need to change this to handle debug messages. On newer
-	 * generation devices, these seem to be sent to tid_out=0x03. We as
-	 * host can still receive them as they can be forwarded via an override
-	 * option on SAM, but doing so does not change tid_out=0x00.
+	 * generation devices, these seem to be sent to SSAM_SSH_TID_DEBUG. We
+	 * as host can still receive them as they can be forwarded via an
+	 * override option on SAM, but doing so does not change the target ID
+	 * to SSAM_SSH_TID_HOST.
 	 */
-	if (command->tid_out != 0x00) {
+	if (command->tid != SSAM_SSH_TID_HOST) {
 		rtl_warn(rtl, "rtl: dropping message not intended for us (tid = %#04x)\n",
-			 command->tid_out);
+			 command->tid);
 		return;
 	}
 
diff --git a/include/linux/surface_aggregator/controller.h b/include/linux/surface_aggregator/controller.h
index d11a1c6e3186a..8932bc0bae187 100644
--- a/include/linux/surface_aggregator/controller.h
+++ b/include/linux/surface_aggregator/controller.h
@@ -912,10 +912,10 @@ enum ssam_event_mask {
 	})
 
 #define SSAM_EVENT_REGISTRY_SAM	\
-	SSAM_EVENT_REGISTRY(SSAM_SSH_TC_SAM, 0x01, 0x0b, 0x0c)
+	SSAM_EVENT_REGISTRY(SSAM_SSH_TC_SAM, SSAM_SSH_TID_SAM, 0x0b, 0x0c)
 
 #define SSAM_EVENT_REGISTRY_KIP	\
-	SSAM_EVENT_REGISTRY(SSAM_SSH_TC_KIP, 0x02, 0x27, 0x28)
+	SSAM_EVENT_REGISTRY(SSAM_SSH_TC_KIP, SSAM_SSH_TID_KIP, 0x27, 0x28)
 
 #define SSAM_EVENT_REGISTRY_REG(tid)\
 	SSAM_EVENT_REGISTRY(SSAM_SSH_TC_REG, tid, 0x01, 0x02)
diff --git a/include/linux/surface_aggregator/serial_hub.h b/include/linux/surface_aggregator/serial_hub.h
index 45501b6e54e8a..5c4ae1a261831 100644
--- a/include/linux/surface_aggregator/serial_hub.h
+++ b/include/linux/surface_aggregator/serial_hub.h
@@ -83,23 +83,21 @@ enum ssh_payload_type {
 
 /**
  * struct ssh_command - Payload of a command-type frame.
- * @type:    The type of the payload. See &enum ssh_payload_type. Should be
- *           SSH_PLD_TYPE_CMD for this struct.
- * @tc:      Command target category.
- * @tid_out: Output target ID. Should be zero if this an incoming (EC to host)
- *           message.
- * @tid_in:  Input target ID. Should be zero if this is an outgoing (host to
- *           EC) message.
- * @iid:     Instance ID.
- * @rqid:    Request ID. Used to match requests with responses and differentiate
- *           between responses and events.
- * @cid:     Command ID.
+ * @type: The type of the payload. See &enum ssh_payload_type. Should be
+ *        SSH_PLD_TYPE_CMD for this struct.
+ * @tc:   Command target category.
+ * @tid:  Target ID. Indicates the target of the message.
+ * @sid:  Source ID. Indicates the source of the message.
+ * @iid:  Instance ID.
+ * @rqid: Request ID. Used to match requests with responses and differentiate
+ *        between responses and events.
+ * @cid:  Command ID.
  */
 struct ssh_command {
 	u8 type;
 	u8 tc;
-	u8 tid_out;
-	u8 tid_in;
+	u8 tid;
+	u8 sid;
 	u8 iid;
 	__le16 rqid;
 	u8 cid;
@@ -280,6 +278,22 @@ struct ssam_span {
 	size_t len;
 };
 
+/**
+ * enum ssam_ssh_tid - Target/source IDs for Serial Hub messages.
+ * @SSAM_SSH_TID_HOST:     We as the kernel Serial Hub driver.
+ * @SSAM_SSH_TID_SAM:      The Surface Aggregator EC.
+ * @SSAM_SSH_TID_KIP:      Keyboard and perihperal controller.
+ * @SSAM_SSH_TID_DEBUG:    Debug connector.
+ * @SSAM_SSH_TID_SURFLINK: SurfLink connector.
+ */
+enum ssam_ssh_tid {
+	SSAM_SSH_TID_HOST     = 0x00,
+	SSAM_SSH_TID_SAM      = 0x01,
+	SSAM_SSH_TID_KIP      = 0x02,
+	SSAM_SSH_TID_DEBUG    = 0x03,
+	SSAM_SSH_TID_SURFLINK = 0x04,
+};
+
 /*
  * Known SSH/EC target categories.
  *
-- 
2.43.0

From 69c2a8e68aca66851c845903701871b9f71aa97c Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:21 +0100
Subject: [PATCH] platform/surface: aggregator: Add target and source IDs to
 command trace events

Add command source and target IDs to trace events.

Tracing support for the Surface Aggregator driver was originally
implemented at a time when only two peers were known: Host and SAM. We
now know that there are at least five, with three actively being used
(Host, SAM, KIP; four with Debug if you want to count manually enabling
that interface). So it makes sense to also explicitly name the peers
involved when tracing.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-4-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 drivers/platform/surface/aggregator/trace.h | 73 +++++++++++++++++++--
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/surface/aggregator/trace.h b/drivers/platform/surface/aggregator/trace.h
index 2a2c17771d014..55cc61bba1da6 100644
--- a/drivers/platform/surface/aggregator/trace.h
+++ b/drivers/platform/surface/aggregator/trace.h
@@ -96,6 +96,7 @@ TRACE_DEFINE_ENUM(SSAM_SSH_TC_POS);
 #define SSAM_SEQ_NOT_APPLICABLE		((u16)-1)
 #define SSAM_RQID_NOT_APPLICABLE	((u32)-1)
 #define SSAM_SSH_TC_NOT_APPLICABLE	0
+#define SSAM_SSH_TID_NOT_APPLICABLE	((u8)-1)
 
 #ifndef _SURFACE_AGGREGATOR_TRACE_HELPERS
 #define _SURFACE_AGGREGATOR_TRACE_HELPERS
@@ -150,12 +151,44 @@ static inline u32 ssam_trace_get_request_id(const struct ssh_packet *p)
 	return get_unaligned_le16(&p->data.ptr[SSH_MSGOFFSET_COMMAND(rqid)]);
 }
 
+/**
+ * ssam_trace_get_request_tid() - Read the packet's request target ID.
+ * @p: The packet.
+ *
+ * Return: Returns the packet's request target ID (TID) field if the packet
+ * represents a request with command data, or %SSAM_SSH_TID_NOT_APPLICABLE
+ * if not (e.g. flush request, control packet).
+ */
+static inline u32 ssam_trace_get_request_tid(const struct ssh_packet *p)
+{
+	if (!p->data.ptr || p->data.len < SSH_COMMAND_MESSAGE_LENGTH(0))
+		return SSAM_SSH_TID_NOT_APPLICABLE;
+
+	return get_unaligned_le16(&p->data.ptr[SSH_MSGOFFSET_COMMAND(tid)]);
+}
+
+/**
+ * ssam_trace_get_request_sid() - Read the packet's request source ID.
+ * @p: The packet.
+ *
+ * Return: Returns the packet's request source ID (SID) field if the packet
+ * represents a request with command data, or %SSAM_SSH_TID_NOT_APPLICABLE
+ * if not (e.g. flush request, control packet).
+ */
+static inline u32 ssam_trace_get_request_sid(const struct ssh_packet *p)
+{
+	if (!p->data.ptr || p->data.len < SSH_COMMAND_MESSAGE_LENGTH(0))
+		return SSAM_SSH_TID_NOT_APPLICABLE;
+
+	return get_unaligned_le16(&p->data.ptr[SSH_MSGOFFSET_COMMAND(sid)]);
+}
+
 /**
  * ssam_trace_get_request_tc() - Read the packet's request target category.
  * @p: The packet.
  *
  * Return: Returns the packet's request target category (TC) field if the
- * packet represents a request with command data, or %SSAM_TC_NOT_APPLICABLE
+ * packet represents a request with command data, or %SSAM_SSH_TC_NOT_APPLICABLE
  * if not (e.g. flush request, control packet).
  */
 static inline u32 ssam_trace_get_request_tc(const struct ssh_packet *p)
@@ -232,8 +265,18 @@ static inline u32 ssam_trace_get_request_tc(const struct ssh_packet *p)
 		{ SSAM_RQID_NOT_APPLICABLE,		"N/A" }		\
 	)
 
-#define ssam_show_ssh_tc(rqid)						\
-	__print_symbolic(rqid,						\
+#define ssam_show_ssh_tid(tid)						\
+	__print_symbolic(tid,						\
+		{ SSAM_SSH_TID_NOT_APPLICABLE,		"N/A"      },	\
+		{ SSAM_SSH_TID_HOST,			"Host"     },	\
+		{ SSAM_SSH_TID_SAM,			"SAM"      },	\
+		{ SSAM_SSH_TID_KIP,			"KIP"      },	\
+		{ SSAM_SSH_TID_DEBUG,			"Debug"    },	\
+		{ SSAM_SSH_TID_SURFLINK,		"SurfLink" }	\
+	)
+
+#define ssam_show_ssh_tc(tc)						\
+	__print_symbolic(tc,						\
 		{ SSAM_SSH_TC_NOT_APPLICABLE,		"N/A"  },	\
 		{ SSAM_SSH_TC_SAM,			"SAM"  },	\
 		{ SSAM_SSH_TC_BAT,			"BAT"  },	\
@@ -313,6 +356,8 @@ DECLARE_EVENT_CLASS(ssam_command_class,
 	TP_STRUCT__entry(
 		__field(u16, rqid)
 		__field(u16, len)
+		__field(u8, tid)
+		__field(u8, sid)
 		__field(u8, tc)
 		__field(u8, cid)
 		__field(u8, iid)
@@ -320,14 +365,18 @@ DECLARE_EVENT_CLASS(ssam_command_class,
 
 	TP_fast_assign(
 		__entry->rqid = get_unaligned_le16(&cmd->rqid);
+		__entry->tid = cmd->tid;
+		__entry->sid = cmd->sid;
 		__entry->tc = cmd->tc;
 		__entry->cid = cmd->cid;
 		__entry->iid = cmd->iid;
 		__entry->len = len;
 	),
 
-	TP_printk("rqid=%#06x, tc=%s, cid=%#04x, iid=%#04x, len=%u",
+	TP_printk("rqid=%#06x, tid=%s, sid=%s, tc=%s, cid=%#04x, iid=%#04x, len=%u",
 		__entry->rqid,
+		ssam_show_ssh_tid(__entry->tid),
+		ssam_show_ssh_tid(__entry->sid),
 		ssam_show_ssh_tc(__entry->tc),
 		__entry->cid,
 		__entry->iid,
@@ -430,6 +479,8 @@ DECLARE_EVENT_CLASS(ssam_request_class,
 		__field(u8, tc)
 		__field(u16, cid)
 		__field(u16, iid)
+		__field(u8, tid)
+		__field(u8, sid)
 	),
 
 	TP_fast_assign(
@@ -439,16 +490,20 @@ DECLARE_EVENT_CLASS(ssam_request_class,
 		__entry->state = READ_ONCE(request->state);
 		__entry->rqid = ssam_trace_get_request_id(p);
 		ssam_trace_ptr_uid(p, __entry->uid);
+		__entry->tid = ssam_trace_get_request_tid(p);
+		__entry->sid = ssam_trace_get_request_sid(p);
 		__entry->tc = ssam_trace_get_request_tc(p);
 		__entry->cid = ssam_trace_get_command_field_u8(p, cid);
 		__entry->iid = ssam_trace_get_command_field_u8(p, iid);
 	),
 
-	TP_printk("uid=%s, rqid=%s, ty=%s, sta=%s, tc=%s, cid=%s, iid=%s",
+	TP_printk("uid=%s, rqid=%s, ty=%s, sta=%s, tid=%s, sid=%s, tc=%s, cid=%s, iid=%s",
 		__entry->uid,
 		ssam_show_request_id(__entry->rqid),
 		ssam_show_request_type(__entry->state),
 		ssam_show_request_state(__entry->state),
+		ssam_show_ssh_tid(__entry->tid),
+		ssam_show_ssh_tid(__entry->sid),
 		ssam_show_ssh_tc(__entry->tc),
 		ssam_show_generic_u8_field(__entry->cid),
 		ssam_show_generic_u8_field(__entry->iid)
@@ -474,6 +529,8 @@ DECLARE_EVENT_CLASS(ssam_request_status_class,
 		__field(u8, tc)
 		__field(u16, cid)
 		__field(u16, iid)
+		__field(u8, tid)
+		__field(u8, sid)
 	),
 
 	TP_fast_assign(
@@ -484,16 +541,20 @@ DECLARE_EVENT_CLASS(ssam_request_status_class,
 		__entry->rqid = ssam_trace_get_request_id(p);
 		__entry->status = status;
 		ssam_trace_ptr_uid(p, __entry->uid);
+		__entry->tid = ssam_trace_get_request_tid(p);
+		__entry->sid = ssam_trace_get_request_sid(p);
 		__entry->tc = ssam_trace_get_request_tc(p);
 		__entry->cid = ssam_trace_get_command_field_u8(p, cid);
 		__entry->iid = ssam_trace_get_command_field_u8(p, iid);
 	),
 
-	TP_printk("uid=%s, rqid=%s, ty=%s, sta=%s, tc=%s, cid=%s, iid=%s, status=%d",
+	TP_printk("uid=%s, rqid=%s, ty=%s, sta=%s, tid=%s, sid=%s, tc=%s, cid=%s, iid=%s, status=%d",
 		__entry->uid,
 		ssam_show_request_id(__entry->rqid),
 		ssam_show_request_type(__entry->state),
 		ssam_show_request_state(__entry->state),
+		ssam_show_ssh_tid(__entry->tid),
+		ssam_show_ssh_tid(__entry->sid),
 		ssam_show_ssh_tc(__entry->tc),
 		ssam_show_generic_u8_field(__entry->cid),
 		ssam_show_generic_u8_field(__entry->iid),
-- 
2.43.0

From 2c39117451ac4ff8c8cfbf8145d75476bb2049da Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:22 +0100
Subject: [PATCH] platform/surface: aggregator_hub: Use target-ID enum instead
 of hard-coding values

Instead of hard-coding the target ID, use the respective enum
ssam_ssh_tid value.

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

diff --git a/drivers/platform/surface/surface_aggregator_hub.c b/drivers/platform/surface/surface_aggregator_hub.c
index 43061514be382..62f27cdb6ca8f 100644
--- a/drivers/platform/surface/surface_aggregator_hub.c
+++ b/drivers/platform/surface/surface_aggregator_hub.c
@@ -214,7 +214,7 @@ static void ssam_hub_remove(struct ssam_device *sdev)
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_bas_query_opmode, u8, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x0d,
 	.instance_id     = 0x00,
 });
@@ -292,7 +292,7 @@ static const struct ssam_hub_desc base_hub = {
 
 SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_query_state, u8, {
 	.target_category = SSAM_SSH_TC_KIP,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x2c,
 	.instance_id     = 0x00,
 });
-- 
2.43.0

From 557688c8fb79ccab9336084de3e42256bc0bba71 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:23 +0100
Subject: [PATCH] platform/surface: aggregator_tabletsw: Use target-ID enum
 instead of hard-coding values

Instead of hard-coding the target ID, use the respective enum
ssam_ssh_tid value.

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

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index af8b547cffdc6..c8ecbdbb516c5 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -252,7 +252,7 @@ static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 s
 
 SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_get_cover_state, u8, {
 	.target_category = SSAM_SSH_TC_KIP,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x1d,
 	.instance_id     = 0x00,
 });
@@ -376,7 +376,7 @@ static int ssam_pos_get_sources_list(struct ssam_tablet_sw *sw, struct ssam_sour
 	int status;
 
 	rqst.target_category = SSAM_SSH_TC_POS;
-	rqst.target_id = 0x01;
+	rqst.target_id = SSAM_SSH_TID_SAM;
 	rqst.command_id = 0x01;
 	rqst.instance_id = 0x00;
 	rqst.flags = SSAM_REQUEST_HAS_RESPONSE;
@@ -435,7 +435,7 @@ static int ssam_pos_get_source(struct ssam_tablet_sw *sw, u32 *source_id)
 
 SSAM_DEFINE_SYNC_REQUEST_WR(__ssam_pos_get_posture_for_source, __le32, __le32, {
 	.target_category = SSAM_SSH_TC_POS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x02,
 	.instance_id     = 0x00,
 });
-- 
2.43.0

From fde3b7b3d4239a851fcbd2f0c6bf2cc52d8f8b0f Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:24 +0100
Subject: [PATCH] platform/surface: dtx: Use target-ID enum instead of
 hard-coding values

Instead of hard-coding the target ID, use the respective enum
ssam_ssh_tid value.

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

diff --git a/drivers/platform/surface/surface_dtx.c b/drivers/platform/surface/surface_dtx.c
index ed36944467f9f..0de76a784a35f 100644
--- a/drivers/platform/surface/surface_dtx.c
+++ b/drivers/platform/surface/surface_dtx.c
@@ -71,63 +71,63 @@ static_assert(sizeof(struct ssam_bas_base_info) == 2);
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_lock, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x06,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_unlock, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x07,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_request, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x08,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_confirm, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x09,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_heartbeat, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x0a,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_N(ssam_bas_latch_cancel, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x0b,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_bas_get_base, struct ssam_bas_base_info, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x0c,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_bas_get_device_mode, u8, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x0d,
 	.instance_id     = 0x00,
 });
 
 SSAM_DEFINE_SYNC_REQUEST_R(ssam_bas_get_latch_status, u8, {
 	.target_category = SSAM_SSH_TC_BAS,
-	.target_id       = 0x01,
+	.target_id       = SSAM_SSH_TID_SAM,
 	.command_id      = 0x11,
 	.instance_id     = 0x00,
 });
-- 
2.43.0

From 62cba06b475782a9d876e239b7a4a9e7320fabda Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:25 +0100
Subject: [PATCH] HID: surface-hid: Use target-ID enum instead of hard-coding
 values

Instead of hard-coding the target ID, use the respective enum
ssam_ssh_tid value.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-8-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 drivers/hid/surface-hid/surface_kbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/surface-hid/surface_kbd.c b/drivers/hid/surface-hid/surface_kbd.c
index 0635341bc5174..42933bf3e925f 100644
--- a/drivers/hid/surface-hid/surface_kbd.c
+++ b/drivers/hid/surface-hid/surface_kbd.c
@@ -250,7 +250,7 @@ static int surface_kbd_probe(struct platform_device *pdev)
 
 	shid->uid.domain = SSAM_DOMAIN_SERIALHUB;
 	shid->uid.category = SSAM_SSH_TC_KBD;
-	shid->uid.target = 2;
+	shid->uid.target = SSAM_SSH_TID_KIP;
 	shid->uid.instance = 0;
 	shid->uid.function = 0;
 
-- 
2.43.0

From b652f52830fed086d0d8c1a16d7aa8e48743188d Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:26 +0100
Subject: [PATCH] platform/surface: aggregator: Enforce use of target-ID enum
 in device ID macros

Similar to the target category (TC), the target ID (TID) can be one
value out of a small number of choices, given in enum ssam_ssh_tid.

In the device ID macros, SSAM_SDEV() and SSAM_VDEV() we already use text
expansion to, both, remove some textual clutter for the target category
values and enforce that the value belongs to the known set. Now that we
know the names for the target IDs, use the same trick for them as well.

Also rename the SSAM_ANY_x macros to SSAM_SSH_x_ANY to better fit in.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Link: https://lore.kernel.org/r/20221202223327.690880-9-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 drivers/hid/surface-hid/surface_hid.c         |  2 +-
 .../platform/surface/surface_aggregator_hub.c |  4 +-
 .../surface/surface_aggregator_tabletsw.c     |  4 +-
 drivers/platform/surface/surface_dtx.c        |  2 +-
 .../surface/surface_platform_profile.c        |  2 +-
 drivers/power/supply/surface_battery.c        |  4 +-
 drivers/power/supply/surface_charger.c        |  2 +-
 include/linux/surface_aggregator/device.h     | 50 +++++++++----------
 8 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/surface-hid/surface_hid.c b/drivers/hid/surface-hid/surface_hid.c
index d4aa8c81903ae..aa80d83a83d1b 100644
--- a/drivers/hid/surface-hid/surface_hid.c
+++ b/drivers/hid/surface-hid/surface_hid.c
@@ -230,7 +230,7 @@ static void surface_hid_remove(struct ssam_device *sdev)
 }
 
 static const struct ssam_device_id surface_hid_match[] = {
-	{ SSAM_SDEV(HID, SSAM_ANY_TID, SSAM_ANY_IID, 0x00) },
+	{ SSAM_SDEV(HID, ANY, SSAM_SSH_IID_ANY, 0x00) },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, surface_hid_match);
diff --git a/drivers/platform/surface/surface_aggregator_hub.c b/drivers/platform/surface/surface_aggregator_hub.c
index 62f27cdb6ca8f..6abd1efe20883 100644
--- a/drivers/platform/surface/surface_aggregator_hub.c
+++ b/drivers/platform/surface/surface_aggregator_hub.c
@@ -348,8 +348,8 @@ static const struct ssam_hub_desc kip_hub = {
 /* -- Driver registration. -------------------------------------------------- */
 
 static const struct ssam_device_id ssam_hub_match[] = {
-	{ SSAM_VDEV(HUB, 0x01, SSAM_SSH_TC_KIP, 0x00), (unsigned long)&kip_hub  },
-	{ SSAM_VDEV(HUB, 0x02, SSAM_SSH_TC_BAS, 0x00), (unsigned long)&base_hub },
+	{ SSAM_VDEV(HUB, SAM, SSAM_SSH_TC_KIP, 0x00), (unsigned long)&kip_hub  },
+	{ SSAM_VDEV(HUB, KIP, SSAM_SSH_TC_BAS, 0x00), (unsigned long)&base_hub },
 	{ }
 };
 MODULE_DEVICE_TABLE(ssam, ssam_hub_match);
diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index c8ecbdbb516c5..6917e86062bc8 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -515,8 +515,8 @@ static const struct ssam_tablet_sw_desc ssam_pos_sw_desc = {
 /* -- Driver registration. -------------------------------------------------- */
 
 static const struct ssam_device_id ssam_tablet_sw_match[] = {
-	{ SSAM_SDEV(KIP, 0x01, 0x00, 0x01), (unsigned long)&ssam_kip_sw_desc },
-	{ SSAM_SDEV(POS, 0x01, 0x00, 0x01), (unsigned long)&ssam_pos_sw_desc },
+	{ SSAM_SDEV(KIP, SAM, 0x00, 0x01), (unsigned long)&ssam_kip_sw_desc },
+	{ SSAM_SDEV(POS, SAM, 0x00, 0x01), (unsigned long)&ssam_pos_sw_desc },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, ssam_tablet_sw_match);
diff --git a/drivers/platform/surface/surface_dtx.c b/drivers/platform/surface/surface_dtx.c
index 0de76a784a35f..30cbde278c599 100644
--- a/drivers/platform/surface/surface_dtx.c
+++ b/drivers/platform/surface/surface_dtx.c
@@ -1214,7 +1214,7 @@ static void surface_dtx_ssam_remove(struct ssam_device *sdev)
 }
 
 static const struct ssam_device_id surface_dtx_ssam_match[] = {
-	{ SSAM_SDEV(BAS, 0x01, 0x00, 0x00) },
+	{ SSAM_SDEV(BAS, SAM, 0x00, 0x00) },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, surface_dtx_ssam_match);
diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
index 37c761f577149..a5a3941b3f43a 100644
--- a/drivers/platform/surface/surface_platform_profile.c
+++ b/drivers/platform/surface/surface_platform_profile.c
@@ -168,7 +168,7 @@ static void surface_platform_profile_remove(struct ssam_device *sdev)
 }
 
 static const struct ssam_device_id ssam_platform_profile_match[] = {
-	{ SSAM_SDEV(TMP, 0x01, 0x00, 0x01) },
+	{ SSAM_SDEV(TMP, SAM, 0x00, 0x01) },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, ssam_platform_profile_match);
diff --git a/drivers/power/supply/surface_battery.c b/drivers/power/supply/surface_battery.c
index 540707882bb0a..19d2f8834e56d 100644
--- a/drivers/power/supply/surface_battery.c
+++ b/drivers/power/supply/surface_battery.c
@@ -852,8 +852,8 @@ static const struct spwr_psy_properties spwr_psy_props_bat2_sb3 = {
 };
 
 static const struct ssam_device_id surface_battery_match[] = {
-	{ SSAM_SDEV(BAT, 0x01, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat1     },
-	{ SSAM_SDEV(BAT, 0x02, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat2_sb3 },
+	{ SSAM_SDEV(BAT, SAM, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat1     },
+	{ SSAM_SDEV(BAT, KIP, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat2_sb3 },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, surface_battery_match);
diff --git a/drivers/power/supply/surface_charger.c b/drivers/power/supply/surface_charger.c
index 59182d55742d0..cabdd8da12d08 100644
--- a/drivers/power/supply/surface_charger.c
+++ b/drivers/power/supply/surface_charger.c
@@ -260,7 +260,7 @@ static const struct spwr_psy_properties spwr_psy_props_adp1 = {
 };
 
 static const struct ssam_device_id surface_ac_match[] = {
-	{ SSAM_SDEV(BAT, 0x01, 0x01, 0x01), (unsigned long)&spwr_psy_props_adp1 },
+	{ SSAM_SDEV(BAT, SAM, 0x01, 0x01), (unsigned long)&spwr_psy_props_adp1 },
 	{ },
 };
 MODULE_DEVICE_TABLE(ssam, surface_ac_match);
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h
index 46c45d1b63682..4da20b7a0ee5e 100644
--- a/include/linux/surface_aggregator/device.h
+++ b/include/linux/surface_aggregator/device.h
@@ -68,9 +68,9 @@ struct ssam_device_uid {
  * match_flags member of the device ID structure. Do not use them directly
  * with struct ssam_device_id or struct ssam_device_uid.
  */
-#define SSAM_ANY_TID		0xffff
-#define SSAM_ANY_IID		0xffff
-#define SSAM_ANY_FUN		0xffff
+#define SSAM_SSH_TID_ANY	0xffff
+#define SSAM_SSH_IID_ANY	0xffff
+#define SSAM_SSH_FUN_ANY	0xffff
 
 /**
  * SSAM_DEVICE() - Initialize a &struct ssam_device_id with the given
@@ -83,25 +83,25 @@ struct ssam_device_uid {
  *
  * Initializes a &struct ssam_device_id with the given parameters. See &struct
  * ssam_device_uid for details regarding the parameters. The special values
- * %SSAM_ANY_TID, %SSAM_ANY_IID, and %SSAM_ANY_FUN can be used to specify that
+ * %SSAM_SSH_TID_ANY, %SSAM_SSH_IID_ANY, and %SSAM_SSH_FUN_ANY can be used to specify that
  * matching should ignore target ID, instance ID, and/or sub-function,
  * respectively. This macro initializes the ``match_flags`` field based on the
  * given parameters.
  *
  * Note: The parameters @d and @cat must be valid &u8 values, the parameters
- * @tid, @iid, and @fun must be either valid &u8 values or %SSAM_ANY_TID,
- * %SSAM_ANY_IID, or %SSAM_ANY_FUN, respectively. Other non-&u8 values are not
+ * @tid, @iid, and @fun must be either valid &u8 values or %SSAM_SSH_TID_ANY,
+ * %SSAM_SSH_IID_ANY, or %SSAM_SSH_FUN_ANY, respectively. Other non-&u8 values are not
  * allowed.
  */
 #define SSAM_DEVICE(d, cat, tid, iid, fun)					\
-	.match_flags = (((tid) != SSAM_ANY_TID) ? SSAM_MATCH_TARGET : 0)	\
-		     | (((iid) != SSAM_ANY_IID) ? SSAM_MATCH_INSTANCE : 0)	\
-		     | (((fun) != SSAM_ANY_FUN) ? SSAM_MATCH_FUNCTION : 0),	\
+	.match_flags = (((tid) != SSAM_SSH_TID_ANY) ? SSAM_MATCH_TARGET : 0)	\
+		     | (((iid) != SSAM_SSH_IID_ANY) ? SSAM_MATCH_INSTANCE : 0)	\
+		     | (((fun) != SSAM_SSH_FUN_ANY) ? SSAM_MATCH_FUNCTION : 0),	\
 	.domain   = d,								\
 	.category = cat,							\
-	.target   = __builtin_choose_expr((tid) != SSAM_ANY_TID, (tid), 0),	\
-	.instance = __builtin_choose_expr((iid) != SSAM_ANY_IID, (iid), 0),	\
-	.function = __builtin_choose_expr((fun) != SSAM_ANY_FUN, (fun), 0)
+	.target   = __builtin_choose_expr((tid) != SSAM_SSH_TID_ANY, (tid), 0),	\
+	.instance = __builtin_choose_expr((iid) != SSAM_SSH_IID_ANY, (iid), 0),	\
+	.function = __builtin_choose_expr((fun) != SSAM_SSH_FUN_ANY, (fun), 0)
 
 /**
  * SSAM_VDEV() - Initialize a &struct ssam_device_id as virtual device with
@@ -113,18 +113,18 @@ struct ssam_device_uid {
  *
  * Initializes a &struct ssam_device_id with the given parameters in the
  * virtual domain. See &struct ssam_device_uid for details regarding the
- * parameters. The special values %SSAM_ANY_TID, %SSAM_ANY_IID, and
- * %SSAM_ANY_FUN can be used to specify that matching should ignore target ID,
+ * parameters. The special values %SSAM_SSH_TID_ANY, %SSAM_SSH_IID_ANY, and
+ * %SSAM_SSH_FUN_ANY can be used to specify that matching should ignore target ID,
  * instance ID, and/or sub-function, respectively. This macro initializes the
  * ``match_flags`` field based on the given parameters.
  *
  * Note: The parameter @cat must be a valid &u8 value, the parameters @tid,
- * @iid, and @fun must be either valid &u8 values or %SSAM_ANY_TID,
- * %SSAM_ANY_IID, or %SSAM_ANY_FUN, respectively. Other non-&u8 values are not
+ * @iid, and @fun must be either valid &u8 values or %SSAM_SSH_TID_ANY,
+ * %SSAM_SSH_IID_ANY, or %SSAM_SSH_FUN_ANY, respectively. Other non-&u8 values are not
  * allowed.
  */
 #define SSAM_VDEV(cat, tid, iid, fun) \
-	SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
+	SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, SSAM_SSH_TID_##tid, iid, fun)
 
 /**
  * SSAM_SDEV() - Initialize a &struct ssam_device_id as physical SSH device
@@ -136,18 +136,18 @@ struct ssam_device_uid {
  *
  * Initializes a &struct ssam_device_id with the given parameters in the SSH
  * domain. See &struct ssam_device_uid for details regarding the parameters.
- * The special values %SSAM_ANY_TID, %SSAM_ANY_IID, and %SSAM_ANY_FUN can be
- * used to specify that matching should ignore target ID, instance ID, and/or
- * sub-function, respectively. This macro initializes the ``match_flags``
- * field based on the given parameters.
+ * The special values %SSAM_SSH_TID_ANY, %SSAM_SSH_IID_ANY, and
+ * %SSAM_SSH_FUN_ANY can be used to specify that matching should ignore target
+ * ID, instance ID, and/or sub-function, respectively. This macro initializes
+ * the ``match_flags`` field based on the given parameters.
  *
  * Note: The parameter @cat must be a valid &u8 value, the parameters @tid,
- * @iid, and @fun must be either valid &u8 values or %SSAM_ANY_TID,
- * %SSAM_ANY_IID, or %SSAM_ANY_FUN, respectively. Other non-&u8 values are not
- * allowed.
+ * @iid, and @fun must be either valid &u8 values or %SSAM_SSH_TID_ANY,
+ * %SSAM_SSH_IID_ANY, or %SSAM_SSH_FUN_ANY, respectively. Other non-&u8 values
+ * are not allowed.
  */
 #define SSAM_SDEV(cat, tid, iid, fun) \
-	SSAM_DEVICE(SSAM_DOMAIN_SERIALHUB, SSAM_SSH_TC_##cat, tid, iid, fun)
+	SSAM_DEVICE(SSAM_DOMAIN_SERIALHUB, SSAM_SSH_TC_##cat, SSAM_SSH_TID_##tid, iid, fun)
 
 /*
  * enum ssam_device_flags - Flags for SSAM client devices.
-- 
2.43.0

From b16cc48e61fb290cbc81fd7e45d2a66770459e6a Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Fri, 2 Dec 2022 23:33:27 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Fix target-ID of
 base-hub

The target ID of the base hub is currently set to KIP (keyboard/
peripherals). However, even though it manages such devices with the KIP
target ID, the base hub itself is actually accessed via the SAM target
ID. So set it accordingly.

Note that the target ID of the hub can be chosen arbitrarily and does
not directly correspond to any physical or virtual component of the EC.
This change is only a code improvement intended for consistency and
clarity, it does not fix an actual bug.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221202223327.690880-10-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_hub.c      | 2 +-
 drivers/platform/surface/surface_aggregator_registry.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_hub.c b/drivers/platform/surface/surface_aggregator_hub.c
index 6abd1efe20883..8b8b80228c147 100644
--- a/drivers/platform/surface/surface_aggregator_hub.c
+++ b/drivers/platform/surface/surface_aggregator_hub.c
@@ -349,7 +349,7 @@ static const struct ssam_hub_desc kip_hub = {
 
 static const struct ssam_device_id ssam_hub_match[] = {
 	{ SSAM_VDEV(HUB, SAM, SSAM_SSH_TC_KIP, 0x00), (unsigned long)&kip_hub  },
-	{ SSAM_VDEV(HUB, KIP, SSAM_SSH_TC_BAS, 0x00), (unsigned long)&base_hub },
+	{ SSAM_VDEV(HUB, SAM, SSAM_SSH_TC_BAS, 0x00), (unsigned long)&base_hub },
 	{ }
 };
 MODULE_DEVICE_TABLE(ssam, ssam_hub_match);
diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 023f126121d7d..296f72d52e6a6 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -46,7 +46,7 @@ static const struct software_node ssam_node_hub_kip = {
 
 /* Base device hub (devices attached to Surface Book 3 base). */
 static const struct software_node ssam_node_hub_base = {
-	.name = "ssam:00:00:02:11:00",
+	.name = "ssam:00:00:01:11:00",
 	.parent = &ssam_node_root,
 };
 
-- 
2.43.0

From f7eb5245e8e253df7ec399d45b5c44d47904c01b Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Tue, 20 Dec 2022 18:56:08 +0100
Subject: [PATCH] platform/surface: aggregator: Rename top-level request
 functions to avoid ambiguities

We currently have a struct ssam_request_sync and a function
ssam_request_sync(). While this is valid C, there are some downsides to
it.

One of these is that current Sphinx versions (>= 3.0) cannot
disambiguate between the two (see disucssion and pull request linked
below). It instead emits a "WARNING: Duplicate C declaration" and links
for the struct and function in the resulting documentation link to the
same entry (i.e. both to either function or struct documentation)
instead of their respective own entries.

While we could just ignore that and wait for a fix, there's also a point
to be made that the current naming can be somewhat confusing when
searching (e.g. via grep) or trying to understand the levels of
abstraction at play:

We currently have struct ssam_request_sync and associated functions
ssam_request_sync_[alloc|free|init|wait|...]() operating on this struct.
However, function ssam_request_sync() is one abstraction level above
this. Similarly, ssam_request_sync_with_buffer() is not a function
operating on struct ssam_request_sync, but rather a sibling to
ssam_request_sync(), both using the struct under the hood.

Therefore, rename the top level request functions:

  ssam_request_sync() -> ssam_request_do_sync()
  ssam_request_sync_with_buffer() -> ssam_request_do_sync_with_buffer()
  ssam_request_sync_onstack() -> ssam_request_do_sync_onstack()

Link: https://lore.kernel.org/all/085e0ada65c11da9303d07e70c510dc45f21315b.1656756450.git.mchehab@kernel.org/
Link: https://github.com/sphinx-doc/sphinx/pull/8313
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20221220175608.1436273-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 .../driver-api/surface_aggregator/client.rst  |  8 +--
 drivers/hid/surface-hid/surface_hid.c         |  6 +-
 drivers/hid/surface-hid/surface_kbd.c         |  6 +-
 drivers/platform/surface/aggregator/bus.c     |  6 +-
 .../platform/surface/aggregator/controller.c  | 32 +++++------
 .../platform/surface/surface_acpi_notify.c    |  2 +-
 .../surface/surface_aggregator_cdev.c         |  6 +-
 .../surface/surface_aggregator_tabletsw.c     |  2 +-
 include/linux/surface_aggregator/controller.h | 56 +++++++++----------
 include/linux/surface_aggregator/device.h     |  8 +--
 10 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/Documentation/driver-api/surface_aggregator/client.rst b/Documentation/driver-api/surface_aggregator/client.rst
index 9d7411223a848..e100ab0a24cc4 100644
--- a/Documentation/driver-api/surface_aggregator/client.rst
+++ b/Documentation/driver-api/surface_aggregator/client.rst
@@ -19,7 +19,7 @@
 .. |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_request_do_sync| replace:: :c:func:`ssam_request_do_sync`
 .. |ssam_event_mask| replace:: :c:type:`enum ssam_event_mask <ssam_event_mask>`
 
 
@@ -209,12 +209,12 @@ data received from it is converted from little-endian to host endianness.
             * with the SSAM_REQUEST_HAS_RESPONSE flag set in the specification
             * above.
             */
-           status = ssam_request_sync(ctrl, &rqst, &resp);
+           status = ssam_request_do_sync(ctrl, &rqst, &resp);
 
            /*
             * Alternatively use
             *
-            *   ssam_request_sync_onstack(ctrl, &rqst, &resp, sizeof(arg_le));
+            *   ssam_request_do_sync_onstack(ctrl, &rqst, &resp, sizeof(arg_le));
             *
             * to perform the request, allocating the message buffer directly
             * on the stack as opposed to allocation via kzalloc().
@@ -230,7 +230,7 @@ data received from it is converted from little-endian to host endianness.
            return status;
    }
 
-Note that |ssam_request_sync| in its essence is a wrapper over lower-level
+Note that |ssam_request_do_sync| in its essence is a wrapper over lower-level
 request primitives, which may also be used to perform requests. Refer to its
 implementation and documentation for more details.
 
diff --git a/drivers/hid/surface-hid/surface_hid.c b/drivers/hid/surface-hid/surface_hid.c
index aa80d83a83d1b..61e5814b0ad7d 100644
--- a/drivers/hid/surface-hid/surface_hid.c
+++ b/drivers/hid/surface-hid/surface_hid.c
@@ -80,7 +80,7 @@ static int ssam_hid_get_descriptor(struct surface_hid_device *shid, u8 entry, u8
 
 		rsp.length = 0;
 
-		status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp,
+		status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp,
 				    sizeof(*slice));
 		if (status)
 			return status;
@@ -131,7 +131,7 @@ static int ssam_hid_set_raw_report(struct surface_hid_device *shid, u8 rprt_id,
 
 	buf[0] = rprt_id;
 
-	return ssam_retry(ssam_request_sync, shid->ctrl, &rqst, NULL);
+	return ssam_retry(ssam_request_do_sync, shid->ctrl, &rqst, NULL);
 }
 
 static int ssam_hid_get_raw_report(struct surface_hid_device *shid, u8 rprt_id, u8 *buf, size_t len)
@@ -151,7 +151,7 @@ static int ssam_hid_get_raw_report(struct surface_hid_device *shid, u8 rprt_id,
 	rsp.length = 0;
 	rsp.pointer = buf;
 
-	return ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(rprt_id));
+	return ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(rprt_id));
 }
 
 static u32 ssam_hid_event_fn(struct ssam_event_notifier *nf, const struct ssam_event *event)
diff --git a/drivers/hid/surface-hid/surface_kbd.c b/drivers/hid/surface-hid/surface_kbd.c
index 42933bf3e925f..4fbce201db6a1 100644
--- a/drivers/hid/surface-hid/surface_kbd.c
+++ b/drivers/hid/surface-hid/surface_kbd.c
@@ -49,7 +49,7 @@ static int ssam_kbd_get_descriptor(struct surface_hid_device *shid, u8 entry, u8
 	rsp.length = 0;
 	rsp.pointer = buf;
 
-	status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(entry));
+	status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(entry));
 	if (status)
 		return status;
 
@@ -75,7 +75,7 @@ static int ssam_kbd_set_caps_led(struct surface_hid_device *shid, bool value)
 	rqst.length = sizeof(value_u8);
 	rqst.payload = &value_u8;
 
-	return ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, NULL, sizeof(value_u8));
+	return ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, NULL, sizeof(value_u8));
 }
 
 static int ssam_kbd_get_feature_report(struct surface_hid_device *shid, u8 *buf, size_t len)
@@ -97,7 +97,7 @@ static int ssam_kbd_get_feature_report(struct surface_hid_device *shid, u8 *buf,
 	rsp.length = 0;
 	rsp.pointer = buf;
 
-	status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(payload));
+	status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(payload));
 	if (status)
 		return status;
 
diff --git a/drivers/platform/surface/aggregator/bus.c b/drivers/platform/surface/aggregator/bus.c
index b501a79f2a08a..da0baba5ed51f 100644
--- a/drivers/platform/surface/aggregator/bus.c
+++ b/drivers/platform/surface/aggregator/bus.c
@@ -136,9 +136,9 @@ int ssam_device_add(struct ssam_device *sdev)
 	 * is always valid and can be used for requests as long as the client
 	 * device we add here is registered as child under it. This essentially
 	 * guarantees that the client driver can always expect the preconditions
-	 * for functions like ssam_request_sync (controller has to be started
-	 * and is not suspended) to hold and thus does not have to check for
-	 * them.
+	 * for functions like ssam_request_do_sync() (controller has to be
+	 * started and is not suspended) to hold and thus does not have to check
+	 * for them.
 	 *
 	 * Note that for this to work, the controller has to be a parent device.
 	 * If it is not a direct parent, care has to be taken that the device is
diff --git a/drivers/platform/surface/aggregator/controller.c b/drivers/platform/surface/aggregator/controller.c
index cb86760622762..7fc602e01487d 100644
--- a/drivers/platform/surface/aggregator/controller.c
+++ b/drivers/platform/surface/aggregator/controller.c
@@ -1674,7 +1674,7 @@ int ssam_request_sync_submit(struct ssam_controller *ctrl,
 EXPORT_SYMBOL_GPL(ssam_request_sync_submit);
 
 /**
- * ssam_request_sync() - Execute a synchronous request.
+ * ssam_request_do_sync() - Execute a synchronous request.
  * @ctrl: The controller via which the request will be submitted.
  * @spec: The request specification and payload.
  * @rsp:  The response buffer.
@@ -1686,9 +1686,9 @@ EXPORT_SYMBOL_GPL(ssam_request_sync_submit);
  *
  * Return: Returns the status of the request or any failure during setup.
  */
-int ssam_request_sync(struct ssam_controller *ctrl,
-		      const struct ssam_request *spec,
-		      struct ssam_response *rsp)
+int ssam_request_do_sync(struct ssam_controller *ctrl,
+			 const struct ssam_request *spec,
+			 struct ssam_response *rsp)
 {
 	struct ssam_request_sync *rqst;
 	struct ssam_span buf;
@@ -1722,10 +1722,10 @@ int ssam_request_sync(struct ssam_controller *ctrl,
 	ssam_request_sync_free(rqst);
 	return status;
 }
-EXPORT_SYMBOL_GPL(ssam_request_sync);
+EXPORT_SYMBOL_GPL(ssam_request_do_sync);
 
 /**
- * ssam_request_sync_with_buffer() - Execute a synchronous request with the
+ * ssam_request_do_sync_with_buffer() - Execute a synchronous request with the
  * provided buffer as back-end for the message buffer.
  * @ctrl: The controller via which the request will be submitted.
  * @spec: The request specification and payload.
@@ -1738,17 +1738,17 @@ EXPORT_SYMBOL_GPL(ssam_request_sync);
  * SSH_COMMAND_MESSAGE_LENGTH() macro can be used to compute the required
  * message buffer size.
  *
- * This function does essentially the same as ssam_request_sync(), but instead
- * of dynamically allocating the request and message data buffer, it uses the
- * provided message data buffer and stores the (small) request struct on the
- * heap.
+ * This function does essentially the same as ssam_request_do_sync(), but
+ * instead of dynamically allocating the request and message data buffer, it
+ * uses the provided message data buffer and stores the (small) request struct
+ * on the heap.
  *
  * Return: Returns the status of the request or any failure during setup.
  */
-int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
-				  const struct ssam_request *spec,
-				  struct ssam_response *rsp,
-				  struct ssam_span *buf)
+int ssam_request_do_sync_with_buffer(struct ssam_controller *ctrl,
+				     const struct ssam_request *spec,
+				     struct ssam_response *rsp,
+				     struct ssam_span *buf)
 {
 	struct ssam_request_sync rqst;
 	ssize_t len;
@@ -1772,7 +1772,7 @@ int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
 
 	return status;
 }
-EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer);
+EXPORT_SYMBOL_GPL(ssam_request_do_sync_with_buffer);
 
 
 /* -- Internal SAM requests. ------------------------------------------------ */
@@ -1864,7 +1864,7 @@ static int __ssam_ssh_event_request(struct ssam_controller *ctrl,
 	result.length = 0;
 	result.pointer = &buf;
 
-	status = ssam_retry(ssam_request_sync_onstack, ctrl, &rqst, &result,
+	status = ssam_retry(ssam_request_do_sync_onstack, ctrl, &rqst, &result,
 			    sizeof(params));
 
 	return status < 0 ? status : buf;
diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c
index 50500e562963d..897cdd9c3aae8 100644
--- a/drivers/platform/surface/surface_acpi_notify.c
+++ b/drivers/platform/surface/surface_acpi_notify.c
@@ -590,7 +590,7 @@ static acpi_status san_rqst(struct san_data *d, struct gsb_buffer *buffer)
 		return san_rqst_fixup_suspended(d, &rqst, buffer);
 	}
 
-	status = __ssam_retry(ssam_request_sync_onstack, SAN_REQUEST_NUM_TRIES,
+	status = __ssam_retry(ssam_request_do_sync_onstack, SAN_REQUEST_NUM_TRIES,
 			      d->ctrl, &rqst, &rsp, SAN_GSB_MAX_RQSX_PAYLOAD);
 
 	if (!status) {
diff --git a/drivers/platform/surface/surface_aggregator_cdev.c b/drivers/platform/surface/surface_aggregator_cdev.c
index 492c82e691827..07f0ed658369b 100644
--- a/drivers/platform/surface/surface_aggregator_cdev.c
+++ b/drivers/platform/surface/surface_aggregator_cdev.c
@@ -302,8 +302,8 @@ static long ssam_cdev_request(struct ssam_cdev_client *client, struct ssam_cdev_
 		 * theoretical maximum (SSH_COMMAND_MAX_PAYLOAD_SIZE) of the
 		 * underlying protocol (note that nothing remotely this size
 		 * should ever be allocated in any normal case). This size is
-		 * validated later in ssam_request_sync(), for allocation the
-		 * bound imposed by u16 should be enough.
+		 * validated later in ssam_request_do_sync(), for allocation
+		 * the bound imposed by u16 should be enough.
 		 */
 		spec.payload = kzalloc(spec.length, GFP_KERNEL);
 		if (!spec.payload) {
@@ -342,7 +342,7 @@ static long ssam_cdev_request(struct ssam_cdev_client *client, struct ssam_cdev_
 	}
 
 	/* Perform request. */
-	status = ssam_request_sync(client->cdev->ctrl, &spec, &rsp);
+	status = ssam_request_do_sync(client->cdev->ctrl, &spec, &rsp);
 	if (status)
 		goto out;
 
diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index 6917e86062bc8..a18e9fc7896b3 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -387,7 +387,7 @@ static int ssam_pos_get_sources_list(struct ssam_tablet_sw *sw, struct ssam_sour
 	rsp.length = 0;
 	rsp.pointer = (u8 *)sources;
 
-	status = ssam_retry(ssam_request_sync_onstack, sw->sdev->ctrl, &rqst, &rsp, 0);
+	status = ssam_retry(ssam_request_do_sync_onstack, sw->sdev->ctrl, &rqst, &rsp, 0);
 	if (status)
 		return status;
 
diff --git a/include/linux/surface_aggregator/controller.h b/include/linux/surface_aggregator/controller.h
index 8932bc0bae187..cb7980805920a 100644
--- a/include/linux/surface_aggregator/controller.h
+++ b/include/linux/surface_aggregator/controller.h
@@ -207,17 +207,17 @@ static inline int ssam_request_sync_wait(struct ssam_request_sync *rqst)
 	return rqst->status;
 }
 
-int ssam_request_sync(struct ssam_controller *ctrl,
-		      const struct ssam_request *spec,
-		      struct ssam_response *rsp);
+int ssam_request_do_sync(struct ssam_controller *ctrl,
+			 const struct ssam_request *spec,
+			 struct ssam_response *rsp);
 
-int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
-				  const struct ssam_request *spec,
-				  struct ssam_response *rsp,
-				  struct ssam_span *buf);
+int ssam_request_do_sync_with_buffer(struct ssam_controller *ctrl,
+				     const struct ssam_request *spec,
+				     struct ssam_response *rsp,
+				     struct ssam_span *buf);
 
 /**
- * ssam_request_sync_onstack - Execute a synchronous request on the stack.
+ * ssam_request_do_sync_onstack - Execute a synchronous request on the stack.
  * @ctrl: The controller via which the request is submitted.
  * @rqst: The request specification.
  * @rsp:  The response buffer.
@@ -227,7 +227,7 @@ int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
  * fully initializes it via the provided request specification, submits it,
  * and finally waits for its completion before returning its status. This
  * helper macro essentially allocates the request message buffer on the stack
- * and then calls ssam_request_sync_with_buffer().
+ * and then calls ssam_request_do_sync_with_buffer().
  *
  * Note: The @payload_len parameter specifies the maximum payload length, used
  * for buffer allocation. The actual payload length may be smaller.
@@ -235,12 +235,12 @@ int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
  * Return: Returns the status of the request or any failure during setup, i.e.
  * zero on success and a negative value on failure.
  */
-#define ssam_request_sync_onstack(ctrl, rqst, rsp, payload_len)			\
+#define ssam_request_do_sync_onstack(ctrl, rqst, rsp, payload_len)		\
 	({									\
 		u8 __data[SSH_COMMAND_MESSAGE_LENGTH(payload_len)];		\
 		struct ssam_span __buf = { &__data[0], ARRAY_SIZE(__data) };	\
 										\
-		ssam_request_sync_with_buffer(ctrl, rqst, rsp, &__buf);		\
+		ssam_request_do_sync_with_buffer(ctrl, rqst, rsp, &__buf);	\
 	})
 
 /**
@@ -349,7 +349,7 @@ struct ssam_request_spec_md {
  * zero on success and negative on failure. The ``ctrl`` parameter is the
  * controller via which the request is being sent.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_N(name, spec...)				\
@@ -366,7 +366,7 @@ struct ssam_request_spec_md {
 		rqst.length = 0;						\
 		rqst.payload = NULL;						\
 										\
-		return ssam_request_sync_onstack(ctrl, &rqst, NULL, 0);		\
+		return ssam_request_do_sync_onstack(ctrl, &rqst, NULL, 0);	\
 	}
 
 /**
@@ -389,7 +389,7 @@ struct ssam_request_spec_md {
  * parameter is the controller via which the request is sent. The request
  * argument is specified via the ``arg`` pointer.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_W(name, atype, spec...)			\
@@ -406,8 +406,8 @@ struct ssam_request_spec_md {
 		rqst.length = sizeof(atype);					\
 		rqst.payload = (u8 *)arg;					\
 										\
-		return ssam_request_sync_onstack(ctrl, &rqst, NULL,		\
-						 sizeof(atype));		\
+		return ssam_request_do_sync_onstack(ctrl, &rqst, NULL,		\
+						    sizeof(atype));		\
 	}
 
 /**
@@ -430,7 +430,7 @@ struct ssam_request_spec_md {
  * the controller via which the request is sent. The request's return value is
  * written to the memory pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_R(name, rtype, spec...)			\
@@ -453,7 +453,7 @@ struct ssam_request_spec_md {
 		rsp.length = 0;							\
 		rsp.pointer = (u8 *)ret;					\
 										\
-		status = ssam_request_sync_onstack(ctrl, &rqst, &rsp, 0);	\
+		status = ssam_request_do_sync_onstack(ctrl, &rqst, &rsp, 0);	\
 		if (status)							\
 			return status;						\
 										\
@@ -491,7 +491,7 @@ struct ssam_request_spec_md {
  * request argument is specified via the ``arg`` pointer. The request's return
  * value is written to the memory pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_WR(name, atype, rtype, spec...)		\
@@ -514,7 +514,7 @@ struct ssam_request_spec_md {
 		rsp.length = 0;							\
 		rsp.pointer = (u8 *)ret;					\
 										\
-		status = ssam_request_sync_onstack(ctrl, &rqst, &rsp, sizeof(atype)); \
+		status = ssam_request_do_sync_onstack(ctrl, &rqst, &rsp, sizeof(atype)); \
 		if (status)							\
 			return status;						\
 										\
@@ -550,7 +550,7 @@ struct ssam_request_spec_md {
  * parameter is the controller via which the request is sent, ``tid`` the
  * target ID for the request, and ``iid`` the instance ID.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_MD_N(name, spec...)				\
@@ -567,7 +567,7 @@ struct ssam_request_spec_md {
 		rqst.length = 0;						\
 		rqst.payload = NULL;						\
 										\
-		return ssam_request_sync_onstack(ctrl, &rqst, NULL, 0);		\
+		return ssam_request_do_sync_onstack(ctrl, &rqst, NULL, 0);	\
 	}
 
 /**
@@ -592,7 +592,7 @@ struct ssam_request_spec_md {
  * ``tid`` the target ID for the request, and ``iid`` the instance ID. The
  * request argument is specified via the ``arg`` pointer.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_MD_W(name, atype, spec...)			\
@@ -609,7 +609,7 @@ struct ssam_request_spec_md {
 		rqst.length = sizeof(atype);					\
 		rqst.payload = (u8 *)arg;					\
 										\
-		return ssam_request_sync_onstack(ctrl, &rqst, NULL,		\
+		return ssam_request_do_sync_onstack(ctrl, &rqst, NULL,		\
 						 sizeof(atype));		\
 	}
 
@@ -635,7 +635,7 @@ struct ssam_request_spec_md {
  * the target ID for the request, and ``iid`` the instance ID. The request's
  * return value is written to the memory pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_MD_R(name, rtype, spec...)			\
@@ -658,7 +658,7 @@ struct ssam_request_spec_md {
 		rsp.length = 0;							\
 		rsp.pointer = (u8 *)ret;					\
 										\
-		status = ssam_request_sync_onstack(ctrl, &rqst, &rsp, 0);	\
+		status = ssam_request_do_sync_onstack(ctrl, &rqst, &rsp, 0);	\
 		if (status)							\
 			return status;						\
 										\
@@ -698,7 +698,7 @@ struct ssam_request_spec_md {
  * The request argument is specified via the ``arg`` pointer. The request's
  * return value is written to the memory pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_MD_WR(name, atype, rtype, spec...)		\
@@ -722,7 +722,7 @@ struct ssam_request_spec_md {
 		rsp.length = 0;							\
 		rsp.pointer = (u8 *)ret;					\
 										\
-		status = ssam_request_sync_onstack(ctrl, &rqst, &rsp, sizeof(atype)); \
+		status = ssam_request_do_sync_onstack(ctrl, &rqst, &rsp, sizeof(atype)); \
 		if (status)							\
 			return status;						\
 										\
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h
index 4da20b7a0ee5e..1545e5567b152 100644
--- a/include/linux/surface_aggregator/device.h
+++ b/include/linux/surface_aggregator/device.h
@@ -456,7 +456,7 @@ static inline int ssam_device_register_clients(struct ssam_device *sdev)
  * device of the request and by association the controller via which the
  * request is sent.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_CL_N(name, spec...)			\
@@ -490,7 +490,7 @@ static inline int ssam_device_register_clients(struct ssam_device *sdev)
  * which the request is sent. The request's argument is specified via the
  * ``arg`` pointer.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_CL_W(name, atype, spec...)		\
@@ -524,7 +524,7 @@ static inline int ssam_device_register_clients(struct ssam_device *sdev)
  * the request is sent. The request's return value is written to the memory
  * pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_CL_R(name, rtype, spec...)		\
@@ -560,7 +560,7 @@ static inline int ssam_device_register_clients(struct ssam_device *sdev)
  * specified via the ``arg`` pointer. The request's return value is written to
  * the memory pointed to by the ``ret`` parameter.
  *
- * Refer to ssam_request_sync_onstack() for more details on the behavior of
+ * Refer to ssam_request_do_sync_onstack() for more details on the behavior of
  * the generated function.
  */
 #define SSAM_DEFINE_SYNC_REQUEST_CL_WR(name, atype, rtype, spec...)		\
-- 
2.43.0

From 7b63e1f2a5cfa18d867a206b08e6cd0cbf321e8d Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Wed, 18 Jan 2023 11:38:23 +0200
Subject: [PATCH] platform/surface: Switch to use acpi_evaluate_dsm_typed()

The acpi_evaluate_dsm_typed() provides a way to check the type of the
object evaluated by _DSM call. Use it instead of open coded variant.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20230118093823.39679-1-andriy.shevchenko@linux.intel.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_hotplug.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/surface/surface_hotplug.c b/drivers/platform/surface/surface_hotplug.c
index f004a24952013..7b6d887dccdbf 100644
--- a/drivers/platform/surface/surface_hotplug.c
+++ b/drivers/platform/surface/surface_hotplug.c
@@ -101,18 +101,12 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
 	param.type = ACPI_TYPE_INTEGER;
 	param.integer.value = value;
 
-	result = acpi_evaluate_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
-				   shps_dsm_fn_for_irq(type), &param);
-
+	result = acpi_evaluate_dsm_typed(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
+					 shps_dsm_fn_for_irq(type), &param, ACPI_TYPE_BUFFER);
 	if (!result) {
 		dev_err(&pdev->dev, "IRQ notification via DSM failed (irq=%d, gpio=%d)\n",
 			type, value);
 
-	} else if (result->type != ACPI_TYPE_BUFFER) {
-		dev_err(&pdev->dev,
-			"IRQ notification via DSM failed: unexpected result type (irq=%d, gpio=%d)\n",
-			type, value);
-
 	} else if (result->buffer.length != 1 || result->buffer.pointer[0] != 0) {
 		dev_err(&pdev->dev,
 			"IRQ notification via DSM failed: unexpected result value (irq=%d, gpio=%d)\n",
@@ -121,8 +115,7 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
 
 	mutex_unlock(&sdev->lock[type]);
 
-	if (result)
-		ACPI_FREE(result);
+	ACPI_FREE(result);
 }
 
 static irqreturn_t shps_handle_irq(int irq, void *data)
-- 
2.43.0

From 0f5c1efd939ca9d0d7442aedb1b067bdfd701107 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 4 Mar 2023 20:09:36 +0100
Subject: [PATCH] platform/surface: aggregator_tabletsw: Properly handle
 different posture source IDs

The device posture subsystem (POS) can provide different posture
sources. Different sources can provide different posture states and
sources can be identified by their ID.

For example, screen posture of the Surface Laptop Studio (SLS), which is
currently the only supported source, uses a source ID of 0x03. The
Surface Pro 9 uses the same subsystem for its Type-Cover, however,
provides different states for that under the ID 0x00.

To eventually support the Surface Pro 9 and potential future devices, we
need to properly disambiguate between source IDs. Therefore, add the
source ID to the state we carry and determine the tablet-mode state (as
well as state names) based on that.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 .../surface/surface_aggregator_tabletsw.c     | 123 ++++++++++++------
 1 file changed, 84 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index a18e9fc7896b3..f9a58db6afde4 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -20,16 +20,23 @@
 
 struct ssam_tablet_sw;
 
+struct ssam_tablet_sw_state {
+	u32 source;
+	u32 state;
+};
+
 struct ssam_tablet_sw_ops {
-	int (*get_state)(struct ssam_tablet_sw *sw, u32 *state);
-	const char *(*state_name)(struct ssam_tablet_sw *sw, u32 state);
-	bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw, u32 state);
+	int (*get_state)(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state);
+	const char *(*state_name)(struct ssam_tablet_sw *sw,
+				  const struct ssam_tablet_sw_state *state);
+	bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw,
+				     const struct ssam_tablet_sw_state *state);
 };
 
 struct ssam_tablet_sw {
 	struct ssam_device *sdev;
 
-	u32 state;
+	struct ssam_tablet_sw_state state;
 	struct work_struct update_work;
 	struct input_dev *mode_switch;
 
@@ -45,9 +52,11 @@ struct ssam_tablet_sw_desc {
 
 	struct {
 		u32 (*notify)(struct ssam_event_notifier *nf, const struct ssam_event *event);
-		int (*get_state)(struct ssam_tablet_sw *sw, u32 *state);
-		const char *(*state_name)(struct ssam_tablet_sw *sw, u32 state);
-		bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw, u32 state);
+		int (*get_state)(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state);
+		const char *(*state_name)(struct ssam_tablet_sw *sw,
+					  const struct ssam_tablet_sw_state *state);
+		bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw,
+					     const struct ssam_tablet_sw_state *state);
 	} ops;
 
 	struct {
@@ -61,7 +70,7 @@ struct ssam_tablet_sw_desc {
 static ssize_t state_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct ssam_tablet_sw *sw = dev_get_drvdata(dev);
-	const char *state = sw->ops.state_name(sw, sw->state);
+	const char *state = sw->ops.state_name(sw, &sw->state);
 
 	return sysfs_emit(buf, "%s\n", state);
 }
@@ -79,19 +88,19 @@ static const struct attribute_group ssam_tablet_sw_group = {
 static void ssam_tablet_sw_update_workfn(struct work_struct *work)
 {
 	struct ssam_tablet_sw *sw = container_of(work, struct ssam_tablet_sw, update_work);
+	struct ssam_tablet_sw_state state;
 	int tablet, status;
-	u32 state;
 
 	status = sw->ops.get_state(sw, &state);
 	if (status)
 		return;
 
-	if (sw->state == state)
+	if (sw->state.source == state.source && sw->state.state == state.state)
 		return;
 	sw->state = state;
 
 	/* Send SW_TABLET_MODE event. */
-	tablet = sw->ops.state_is_tablet_mode(sw, state);
+	tablet = sw->ops.state_is_tablet_mode(sw, &state);
 	input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);
 	input_sync(sw->mode_switch);
 }
@@ -146,7 +155,7 @@ static int ssam_tablet_sw_probe(struct ssam_device *sdev)
 	sw->mode_switch->id.bustype = BUS_HOST;
 	sw->mode_switch->dev.parent = &sdev->dev;
 
-	tablet = sw->ops.state_is_tablet_mode(sw, sw->state);
+	tablet = sw->ops.state_is_tablet_mode(sw, &sw->state);
 	input_set_capability(sw->mode_switch, EV_SW, SW_TABLET_MODE);
 	input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);
 
@@ -204,9 +213,10 @@ enum ssam_kip_cover_state {
 	SSAM_KIP_COVER_STATE_BOOK          = 0x06,
 };
 
-static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw,
+					     const struct ssam_tablet_sw_state *state)
 {
-	switch (state) {
+	switch (state->state) {
 	case SSAM_KIP_COVER_STATE_DISCONNECTED:
 		return "disconnected";
 
@@ -226,14 +236,15 @@ static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw, u32 stat
 		return "book";
 
 	default:
-		dev_warn(&sw->sdev->dev, "unknown KIP cover state: %u\n", state);
+		dev_warn(&sw->sdev->dev, "unknown KIP cover state: %u\n", state->state);
 		return "<unknown>";
 	}
 }
 
-static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 state)
+static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw,
+						const struct ssam_tablet_sw_state *state)
 {
-	switch (state) {
+	switch (state->state) {
 	case SSAM_KIP_COVER_STATE_DISCONNECTED:
 	case SSAM_KIP_COVER_STATE_FOLDED_CANVAS:
 	case SSAM_KIP_COVER_STATE_FOLDED_BACK:
@@ -245,7 +256,7 @@ static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 s
 		return false;
 
 	default:
-		dev_warn(&sw->sdev->dev, "unknown KIP cover state: %d\n", sw->state);
+		dev_warn(&sw->sdev->dev, "unknown KIP cover state: %d\n", state->state);
 		return true;
 	}
 }
@@ -257,7 +268,7 @@ SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_get_cover_state, u8, {
 	.instance_id     = 0x00,
 });
 
-static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, u32 *state)
+static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state)
 {
 	int status;
 	u8 raw;
@@ -268,7 +279,8 @@ static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, u32 *state)
 		return status;
 	}
 
-	*state = raw;
+	state->source = 0;	/* Unused for KIP switch. */
+	state->state = raw;
 	return 0;
 }
 
@@ -317,11 +329,15 @@ MODULE_PARM_DESC(tablet_mode_in_slate_state, "Enable tablet mode in slate device
 #define SSAM_EVENT_POS_CID_POSTURE_CHANGED	0x03
 #define SSAM_POS_MAX_SOURCES			4
 
-enum ssam_pos_state {
-	SSAM_POS_POSTURE_LID_CLOSED = 0x00,
-	SSAM_POS_POSTURE_LAPTOP     = 0x01,
-	SSAM_POS_POSTURE_SLATE      = 0x02,
-	SSAM_POS_POSTURE_TABLET     = 0x03,
+enum ssam_pos_source_id {
+	SSAM_POS_SOURCE_SLS   = 0x03,
+};
+
+enum ssam_pos_state_sls {
+	SSAM_POS_SLS_LID_CLOSED = 0x00,
+	SSAM_POS_SLS_LAPTOP     = 0x01,
+	SSAM_POS_SLS_SLATE      = 0x02,
+	SSAM_POS_SLS_TABLET     = 0x03,
 };
 
 struct ssam_sources_list {
@@ -329,42 +345,68 @@ struct ssam_sources_list {
 	__le32 id[SSAM_POS_MAX_SOURCES];
 } __packed;
 
-static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_pos_state_name_sls(struct ssam_tablet_sw *sw, u32 state)
 {
 	switch (state) {
-	case SSAM_POS_POSTURE_LID_CLOSED:
+	case SSAM_POS_SLS_LID_CLOSED:
 		return "closed";
 
-	case SSAM_POS_POSTURE_LAPTOP:
+	case SSAM_POS_SLS_LAPTOP:
 		return "laptop";
 
-	case SSAM_POS_POSTURE_SLATE:
+	case SSAM_POS_SLS_SLATE:
 		return "slate";
 
-	case SSAM_POS_POSTURE_TABLET:
+	case SSAM_POS_SLS_TABLET:
 		return "tablet";
 
 	default:
-		dev_warn(&sw->sdev->dev, "unknown device posture: %u\n", state);
+		dev_warn(&sw->sdev->dev, "unknown device posture for SLS: %u\n", state);
 		return "<unknown>";
 	}
 }
 
-static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
+				       const struct ssam_tablet_sw_state *state)
+{
+	switch (state->source) {
+	case SSAM_POS_SOURCE_SLS:
+		return ssam_pos_state_name_sls(sw, state->state);
+
+	default:
+		dev_warn(&sw->sdev->dev, "unknown device posture source: %u\n", state->source);
+		return "<unknown>";
+	}
+}
+
+static bool ssam_pos_state_is_tablet_mode_sls(struct ssam_tablet_sw *sw, u32 state)
 {
 	switch (state) {
-	case SSAM_POS_POSTURE_LAPTOP:
-	case SSAM_POS_POSTURE_LID_CLOSED:
+	case SSAM_POS_SLS_LAPTOP:
+	case SSAM_POS_SLS_LID_CLOSED:
 		return false;
 
-	case SSAM_POS_POSTURE_SLATE:
+	case SSAM_POS_SLS_SLATE:
 		return tablet_mode_in_slate_state;
 
-	case SSAM_POS_POSTURE_TABLET:
+	case SSAM_POS_SLS_TABLET:
 		return true;
 
 	default:
-		dev_warn(&sw->sdev->dev, "unknown device posture: %u\n", state);
+		dev_warn(&sw->sdev->dev, "unknown device posture for SLS: %u\n", state);
+		return true;
+	}
+}
+
+static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw,
+					  const struct ssam_tablet_sw_state *state)
+{
+	switch (state->source) {
+	case SSAM_POS_SOURCE_SLS:
+		return ssam_pos_state_is_tablet_mode_sls(sw, state->state);
+
+	default:
+		dev_warn(&sw->sdev->dev, "unknown device posture source: %u\n", state->source);
 		return true;
 	}
 }
@@ -455,9 +497,10 @@ static int ssam_pos_get_posture_for_source(struct ssam_tablet_sw *sw, u32 source
 	return 0;
 }
 
-static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, u32 *state)
+static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state)
 {
 	u32 source_id;
+	u32 source_state;
 	int status;
 
 	status = ssam_pos_get_source(sw, &source_id);
@@ -466,13 +509,15 @@ static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, u32 *state)
 		return status;
 	}
 
-	status = ssam_pos_get_posture_for_source(sw, source_id, state);
+	status = ssam_pos_get_posture_for_source(sw, source_id, &source_state);
 	if (status) {
 		dev_err(&sw->sdev->dev, "failed to get posture value for source %u: %d\n",
 			source_id, status);
 		return status;
 	}
 
+	state->source = source_id;
+	state->state = source_state;
 	return 0;
 }
 
-- 
2.43.0

From 89dc1d5972b9c2001086ca9aa1982fd669258763 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 19 Feb 2023 23:33:43 +0100
Subject: [PATCH] platform/surface: aggregator_tabletsw: Add support for
 Type-Cover posture source

Implement support for the Type-Cover posture source (ID 0x00), found on
the Surface Pro 9.

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

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index f9a58db6afde4..4a029f5db20a9 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -330,9 +330,18 @@ MODULE_PARM_DESC(tablet_mode_in_slate_state, "Enable tablet mode in slate device
 #define SSAM_POS_MAX_SOURCES			4
 
 enum ssam_pos_source_id {
+	SSAM_POS_SOURCE_COVER = 0x00,
 	SSAM_POS_SOURCE_SLS   = 0x03,
 };
 
+enum ssam_pos_state_cover {
+	SSAM_POS_COVER_DISCONNECTED  = 0x01,
+	SSAM_POS_COVER_CLOSED        = 0x02,
+	SSAM_POS_COVER_LAPTOP        = 0x03,
+	SSAM_POS_COVER_FOLDED_CANVAS = 0x04,
+	SSAM_POS_COVER_FOLDED_BACK   = 0x05,
+};
+
 enum ssam_pos_state_sls {
 	SSAM_POS_SLS_LID_CLOSED = 0x00,
 	SSAM_POS_SLS_LAPTOP     = 0x01,
@@ -345,6 +354,30 @@ struct ssam_sources_list {
 	__le32 id[SSAM_POS_MAX_SOURCES];
 } __packed;
 
+static const char *ssam_pos_state_name_cover(struct ssam_tablet_sw *sw, u32 state)
+{
+	switch (state) {
+	case SSAM_POS_COVER_DISCONNECTED:
+		return "disconnected";
+
+	case SSAM_POS_COVER_CLOSED:
+		return "closed";
+
+	case SSAM_POS_COVER_LAPTOP:
+		return "laptop";
+
+	case SSAM_POS_COVER_FOLDED_CANVAS:
+		return "folded-canvas";
+
+	case SSAM_POS_COVER_FOLDED_BACK:
+		return "folded-back";
+
+	default:
+		dev_warn(&sw->sdev->dev, "unknown device posture for type-cover: %u\n", state);
+		return "<unknown>";
+	}
+}
+
 static const char *ssam_pos_state_name_sls(struct ssam_tablet_sw *sw, u32 state)
 {
 	switch (state) {
@@ -370,6 +403,9 @@ static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
 				       const struct ssam_tablet_sw_state *state)
 {
 	switch (state->source) {
+	case SSAM_POS_SOURCE_COVER:
+		return ssam_pos_state_name_cover(sw, state->state);
+
 	case SSAM_POS_SOURCE_SLS:
 		return ssam_pos_state_name_sls(sw, state->state);
 
@@ -379,6 +415,24 @@ static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
 	}
 }
 
+static bool ssam_pos_state_is_tablet_mode_cover(struct ssam_tablet_sw *sw, u32 state)
+{
+	switch (state) {
+	case SSAM_POS_COVER_DISCONNECTED:
+	case SSAM_POS_COVER_FOLDED_CANVAS:
+	case SSAM_POS_COVER_FOLDED_BACK:
+		return true;
+
+	case SSAM_POS_COVER_CLOSED:
+	case SSAM_POS_COVER_LAPTOP:
+		return false;
+
+	default:
+		dev_warn(&sw->sdev->dev, "unknown device posture for type-cover: %u\n", state);
+		return true;
+	}
+}
+
 static bool ssam_pos_state_is_tablet_mode_sls(struct ssam_tablet_sw *sw, u32 state)
 {
 	switch (state) {
@@ -402,6 +456,9 @@ static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw,
 					  const struct ssam_tablet_sw_state *state)
 {
 	switch (state->source) {
+	case SSAM_POS_SOURCE_COVER:
+		return ssam_pos_state_is_tablet_mode_cover(sw, state->state);
+
 	case SSAM_POS_SOURCE_SLS:
 		return ssam_pos_state_is_tablet_mode_sls(sw, state->state);
 
-- 
2.43.0

From 0ef185523ee176cfa2fd105fcc89d3f2f617eeaf Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Thu, 18 May 2023 22:57:17 +0200
Subject: [PATCH] platform/surface: aggregator_tabletsw: Add support for book
 mode in POS subsystem

Devices with a type-cover have an additional "book" mode, deactivating
type-cover input and turning off its backlight. This is currently
unsupported, leading to the warning

  surface_aggregator_tablet_mode_switch 01:26:01:00:01: unknown device posture for type-cover: 6

Therefore, add support for this state and map it to enable tablet-mode.

Fixes: 37ff64cd81ff ("platform/surface: aggregator_tabletsw: Add support for Type-Cover posture source")
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_tabletsw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index 4a029f5db20a9..c0a1a5869246e 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -340,6 +340,7 @@ enum ssam_pos_state_cover {
 	SSAM_POS_COVER_LAPTOP        = 0x03,
 	SSAM_POS_COVER_FOLDED_CANVAS = 0x04,
 	SSAM_POS_COVER_FOLDED_BACK   = 0x05,
+	SSAM_POS_COVER_BOOK          = 0x06,
 };
 
 enum ssam_pos_state_sls {
@@ -372,6 +373,9 @@ static const char *ssam_pos_state_name_cover(struct ssam_tablet_sw *sw, u32 stat
 	case SSAM_POS_COVER_FOLDED_BACK:
 		return "folded-back";
 
+	case SSAM_POS_COVER_BOOK:
+		return "book";
+
 	default:
 		dev_warn(&sw->sdev->dev, "unknown device posture for type-cover: %u\n", state);
 		return "<unknown>";
@@ -421,6 +425,7 @@ static bool ssam_pos_state_is_tablet_mode_cover(struct ssam_tablet_sw *sw, u32 s
 	case SSAM_POS_COVER_DISCONNECTED:
 	case SSAM_POS_COVER_FOLDED_CANVAS:
 	case SSAM_POS_COVER_FOLDED_BACK:
+	case SSAM_POS_COVER_BOOK:
 		return true;
 
 	case SSAM_POS_COVER_CLOSED:
-- 
2.43.0

From dcb71bedc807baae6102c9a8d2534d0477af51cd Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 19 Feb 2023 23:41:18 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Add support for
 tablet-mode switch on Surface Pro 9

Add support for the POS-subsystem tablet-mode switch used on the Surface
Pro 9.

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 296f72d52e6a6..0fe5be5396525 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -305,7 +305,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
-	/* TODO: Tablet mode switch (via POS subsystem) */
+	&ssam_node_pos_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
 	&ssam_node_hid_kip_penstash,
 	&ssam_node_hid_kip_touchpad,
-- 
2.43.0

From 776b9d87b6b47f6e83688766418965349b9e75fc Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 22 Oct 2023 14:57:11 +0200
Subject: [PATCH] platform/surface: aggregator_registry: Add support for
 Surface Laptop Go 3

Add SAM client device nodes for the Surface Laptop Go 3. It seems to use
the same SAM client devices as the Surface Laptop Go 1 and 2, so re-use
their node group.

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 0fe5be5396525..0d8c8395c5886 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -367,6 +367,9 @@ static const struct acpi_device_id ssam_platform_hub_match[] = {
 	/* Surface Laptop Go 2 */
 	{ "MSHW0290", (unsigned long)ssam_node_group_slg1 },
 
+	/* Surface Laptop Go 3 */
+	{ "MSHW0440", (unsigned long)ssam_node_group_slg1 },
+
 	/* Surface Laptop Studio */
 	{ "MSHW0123", (unsigned long)ssam_node_group_sls },
 
-- 
2.43.0

From c650a4d3042c1cb708251d910fbf048c84f40682 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Mon, 20 Nov 2023 19:47:00 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Add support for
 Surface Laptop Studio 2

Add SAM client device nodes for the Surface Laptop Studio 2 (SLS2). The
SLS2 is quite similar to the SLS1, but it does not provide the touchpad
as a SAM-HID device. Therefore, add a new node group for the SLS2 and
update the comments accordingly

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 0d8c8395c5886..530db4db71aba 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -247,8 +247,8 @@ static const struct software_node *ssam_node_group_sl5[] = {
 	NULL,
 };
 
-/* Devices for Surface Laptop Studio. */
-static const struct software_node *ssam_node_group_sls[] = {
+/* Devices for Surface Laptop Studio 1. */
+static const struct software_node *ssam_node_group_sls1[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
@@ -263,6 +263,20 @@ static const struct software_node *ssam_node_group_sls[] = {
 	NULL,
 };
 
+/* Devices for Surface Laptop Studio 2. */
+static const struct software_node *ssam_node_group_sls2[] = {
+	&ssam_node_root,
+	&ssam_node_bat_ac,
+	&ssam_node_bat_main,
+	&ssam_node_tmp_pprof,
+	&ssam_node_pos_tablet_switch,
+	&ssam_node_hid_sam_keyboard,
+	&ssam_node_hid_sam_penstash,
+	&ssam_node_hid_sam_sensors,
+	&ssam_node_hid_sam_ucm_ucsi,
+	NULL,
+};
+
 /* Devices for Surface Laptop Go. */
 static const struct software_node *ssam_node_group_slg1[] = {
 	&ssam_node_root,
@@ -370,8 +384,11 @@ static const struct acpi_device_id ssam_platform_hub_match[] = {
 	/* Surface Laptop Go 3 */
 	{ "MSHW0440", (unsigned long)ssam_node_group_slg1 },
 
-	/* Surface Laptop Studio */
-	{ "MSHW0123", (unsigned long)ssam_node_group_sls },
+	/* Surface Laptop Studio 1 */
+	{ "MSHW0123", (unsigned long)ssam_node_group_sls1 },
+
+	/* Surface Laptop Studio 2 */
+	{ "MSHW0360", (unsigned long)ssam_node_group_sls2 },
 
 	{ },
 };
-- 
2.43.0

From ee3c245f0f101a020ec8df9f961cc0354b21ef64 Mon Sep 17 00:00:00 2001
From: Ivor Wanders <ivor@iwanders.net>
Date: Mon, 18 Dec 2023 19:21:32 -0500
Subject: [PATCH] platform/surface: aggregator_registry: add entry for fan
 speed

Add an entry for the fan speed function.
Add this new entry to the Surface Pro 9 group.

Signed-off-by: Ivor Wanders <ivor@iwanders.net>
Link: https://github.com/linux-surface/kernel/pull/144
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/platform/surface/surface_aggregator_registry.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 530db4db71aba..b0db25886c996 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -74,6 +74,12 @@ static const struct software_node ssam_node_tmp_pprof = {
 	.parent = &ssam_node_root,
 };
 
+/* Fan speed function. */
+static const struct software_node ssam_node_fan_speed = {
+	.name = "ssam:01:05:01:01:01",
+	.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",
@@ -319,6 +325,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
+	&ssam_node_fan_speed,
 	&ssam_node_pos_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
 	&ssam_node_hid_kip_penstash,
-- 
2.43.0

From 0aecdee1605a40edddb894fa823a9bc568f05f3c Mon Sep 17 00:00:00 2001
From: Ivor Wanders <ivor@iwanders.net>
Date: Thu, 30 Nov 2023 20:20:24 -0500
Subject: [PATCH] hwmon: add fan speed monitoring driver for Surface devices

Adds a driver that provides read only access to the fan speed for Microsoft
Surface Pro devices. The fan speed is always regulated by the EC and cannot
be influenced directly.

Signed-off-by: Ivor Wanders <ivor@iwanders.net>
Link: https://github.com/linux-surface/kernel/pull/144
Patchset: surface-sam
---
 Documentation/hwmon/index.rst       |  1 +
 Documentation/hwmon/surface_fan.rst | 25 ++++++++
 MAINTAINERS                         |  8 +++
 drivers/hwmon/Kconfig               | 13 ++++
 drivers/hwmon/Makefile              |  1 +
 drivers/hwmon/surface_fan.c         | 93 +++++++++++++++++++++++++++++
 6 files changed, 141 insertions(+)
 create mode 100644 Documentation/hwmon/surface_fan.rst
 create mode 100644 drivers/hwmon/surface_fan.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index c1d11cf13eef1..0e52bb60dc6d0 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -192,6 +192,7 @@ Hardware Monitoring Kernel Drivers
    smsc47m1
    sparx5-temp
    stpddc60
+   surface_fan
    sy7636a-hwmon
    tc654
    tc74
diff --git a/Documentation/hwmon/surface_fan.rst b/Documentation/hwmon/surface_fan.rst
new file mode 100644
index 0000000000000..07942574c4f0c
--- /dev/null
+++ b/Documentation/hwmon/surface_fan.rst
@@ -0,0 +1,25 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver surface_fan
+=========================
+
+Supported Devices:
+
+  * Microsoft Surface Pro 9
+
+Author: Ivor Wanders <ivor@iwanders.net>
+
+Description
+-----------
+
+This provides monitoring of the fan found in some Microsoft Surface Pro devices,
+like the Surface Pro 9. The fan is always controlled by the onboard controller.
+
+Sysfs interface
+---------------
+
+======================= ======= =========================================
+Name                    Perm    Description
+======================= ======= =========================================
+``fan1_input``          RO      Current fan speed in RPM.
+======================= ======= =========================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 13d1078808bb5..9f7702a82842d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13700,6 +13700,14 @@ F:	Documentation/driver-api/surface_aggregator/clients/dtx.rst
 F:	drivers/platform/surface/surface_dtx.c
 F:	include/uapi/linux/surface_aggregator/dtx.h
 
+MICROSOFT SURFACE SENSOR FAN DRIVER
+M:	Maximilian Luz <luzmaximilian@gmail.com>
+M:	Ivor Wanders <ivor@iwanders.net>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/surface_fan.rst
+F:	drivers/hwmon/surface_fan.c
+
 MICROSOFT SURFACE GPE LID SUPPORT DRIVER
 M:	Maximilian Luz <luzmaximilian@gmail.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index a5143d01b95f8..4a5f0ffd13e7a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1925,6 +1925,19 @@ config SENSORS_SMM665
 	  This driver can also be built as a module. If so, the module will
 	  be called smm665.
 
+config SENSORS_SURFACE_FAN
+	tristate "Surface Fan Driver"
+	depends on SURFACE_AGGREGATOR
+	help
+	  Driver that provides monitoring of the fan on Surface Pro devices that
+	  have a fan, like the Surface Pro 9.
+
+	  This makes the fan's current speed accessible through the hwmon
+	  system. It does not provide control over the fan, the firmware is
+	  responsible for that, this driver merely provides monitoring.
+
+	  Select M or Y here, if you want to be able to read the fan's speed.
+
 config SENSORS_ADC128D818
 	tristate "Texas Instruments ADC128D818"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 11d076cad8a2d..4481011816074 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -192,6 +192,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
 obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.o
+obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o
 obj-$(CONFIG_SENSORS_SY7636A)	+= sy7636a-hwmon.o
 obj-$(CONFIG_SENSORS_AMC6821)	+= amc6821.o
 obj-$(CONFIG_SENSORS_TC74)	+= tc74.o
diff --git a/drivers/hwmon/surface_fan.c b/drivers/hwmon/surface_fan.c
new file mode 100644
index 0000000000000..7c2e3ae3eb40e
--- /dev/null
+++ b/drivers/hwmon/surface_fan.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Surface Fan driver for Surface System Aggregator Module. It provides access
+ * to the fan's rpm through the hwmon system.
+ *
+ * Copyright (C) 2023 Ivor Wanders <ivor@iwanders.net>
+ */
+
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/surface_aggregator/device.h>
+#include <linux/types.h>
+
+// SSAM
+SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_fan_rpm_get, __le16, {
+	.target_category = SSAM_SSH_TC_FAN,
+	.command_id      = 0x01,
+});
+
+// hwmon
+umode_t surface_fan_hwmon_is_visible(const void *drvdata,
+				     enum hwmon_sensor_types type, u32 attr,
+				     int channel)
+{
+	return 0444;
+}
+
+static int surface_fan_hwmon_read(struct device *dev,
+				  enum hwmon_sensor_types type, u32 attr,
+				  int channel, long *val)
+{
+	struct ssam_device *sdev = dev_get_drvdata(dev);
+	int ret;
+	__le16 value;
+
+	ret = __ssam_fan_rpm_get(sdev, &value);
+	if (ret)
+		return ret;
+
+	*val = le16_to_cpu(value);
+
+	return ret;
+}
+
+static const struct hwmon_channel_info *const surface_fan_info[] = {
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops surface_fan_hwmon_ops = {
+	.is_visible = surface_fan_hwmon_is_visible,
+	.read = surface_fan_hwmon_read,
+};
+
+static const struct hwmon_chip_info surface_fan_chip_info = {
+	.ops = &surface_fan_hwmon_ops,
+	.info = surface_fan_info,
+};
+
+static int surface_fan_probe(struct ssam_device *sdev)
+{
+	struct device *hdev;
+
+	hdev = devm_hwmon_device_register_with_info(&sdev->dev,
+						    "surface_fan", sdev,
+						    &surface_fan_chip_info,
+						    NULL);
+	if (IS_ERR(hdev))
+		return PTR_ERR(hdev);
+
+	return 0;
+}
+
+static const struct ssam_device_id ssam_fan_match[] = {
+	{ SSAM_SDEV(FAN, SAM, 0x01, 0x01) },
+	{},
+};
+MODULE_DEVICE_TABLE(ssam, ssam_fan_match);
+
+static struct ssam_device_driver surface_fan = {
+	.probe = surface_fan_probe,
+	.match_table = ssam_fan_match,
+	.driver = {
+		.name = "surface_fan",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+module_ssam_device_driver(surface_fan);
+
+MODULE_AUTHOR("Ivor Wanders <ivor@iwanders.net>");
+MODULE_DESCRIPTION("Fan Driver for Surface System Aggregator Module");
+MODULE_LICENSE("GPL");
-- 
2.43.0

From 75ed426ff1c6b08c6198be537e9b22724a86fc5d Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 30 Dec 2023 18:07:54 +0100
Subject: [PATCH] hwmon: Add thermal sensor driver for Surface Aggregator
 Module

Some of the newer Microsoft Surface devices (such as the Surface Book
3 and Pro 9) have thermal sensors connected via the Surface Aggregator
Module (the embedded controller on those devices). Add a basic driver
to read out the temperature values of those sensors.

Link: https://github.com/linux-surface/surface-aggregator-module/issues/59
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/hwmon/Kconfig        |  10 +++
 drivers/hwmon/Makefile       |   1 +
 drivers/hwmon/surface_temp.c | 165 +++++++++++++++++++++++++++++++++++
 3 files changed, 176 insertions(+)
 create mode 100644 drivers/hwmon/surface_temp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 4a5f0ffd13e7a..be7243e7ccb23 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1938,6 +1938,16 @@ config SENSORS_SURFACE_FAN
 
 	  Select M or Y here, if you want to be able to read the fan's speed.
 
+config SENSORS_SURFACE_TEMP
+	tristate "Microsoft Surface Thermal Sensor Driver"
+	depends on SURFACE_AGGREGATOR
+	help
+	  Driver for monitoring thermal sensors connected via the Surface
+	  Aggregator Module (embedded controller) on Microsoft Surface devices.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called surface_temp.
+
 config SENSORS_ADC128D818
 	tristate "Texas Instruments ADC128D818"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4481011816074..c22cfad4ca112 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -193,6 +193,7 @@ obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
 obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.o
 obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o
+obj-$(CONFIG_SENSORS_SURFACE_TEMP)+= surface_temp.o
 obj-$(CONFIG_SENSORS_SY7636A)	+= sy7636a-hwmon.o
 obj-$(CONFIG_SENSORS_AMC6821)	+= amc6821.o
 obj-$(CONFIG_SENSORS_TC74)	+= tc74.o
diff --git a/drivers/hwmon/surface_temp.c b/drivers/hwmon/surface_temp.c
new file mode 100644
index 0000000000000..48c3e826713f6
--- /dev/null
+++ b/drivers/hwmon/surface_temp.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Thermal sensor subsystem driver for Surface System Aggregator Module (SSAM).
+ *
+ * Copyright (C) 2022-2023 Maximilian Luz <luzmaximilian@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include <linux/surface_aggregator/controller.h>
+#include <linux/surface_aggregator/device.h>
+
+
+/* -- SAM interface. -------------------------------------------------------- */
+
+SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_get_available_sensors, __le16, {
+	.target_category = SSAM_SSH_TC_TMP,
+	.command_id      = 0x04,
+});
+
+SSAM_DEFINE_SYNC_REQUEST_MD_R(__ssam_tmp_get_temperature, __le16, {
+	.target_category = SSAM_SSH_TC_TMP,
+	.command_id      = 0x01,
+});
+
+static int ssam_tmp_get_available_sensors(struct ssam_device *sdev, s16 *sensors)
+{
+	__le16 sensors_le;
+	int status;
+
+	status = __ssam_tmp_get_available_sensors(sdev, &sensors_le);
+	if (status)
+		return status;
+
+	*sensors = le16_to_cpu(sensors_le);
+	return 0;
+}
+
+static int ssam_tmp_get_temperature(struct ssam_device *sdev, u8 iid, long *temperature)
+{
+	__le16 temp_le;
+	int status;
+
+	status = __ssam_tmp_get_temperature(sdev->ctrl, sdev->uid.target, iid, &temp_le);
+	if (status)
+		return status;
+
+	/* Convert 1/10 °K to 1/1000 °C */
+	*temperature = (le16_to_cpu(temp_le) - 2731) * 100L;
+	return 0;
+}
+
+
+/* -- Driver.---------------------------------------------------------------- */
+
+struct ssam_temp {
+	struct ssam_device *sdev;
+	s16 sensors;
+};
+
+static umode_t ssam_temp_hwmon_is_visible(const void *data,
+					  enum hwmon_sensor_types type,
+					  u32 attr, int channel)
+{
+	const struct ssam_temp *ssam_temp = data;
+
+	if (!(ssam_temp->sensors & BIT(channel)))
+		return 0;
+
+	return 0444;
+}
+
+static int ssam_temp_hwmon_read(struct device *dev,
+				enum hwmon_sensor_types type,
+				u32 attr, int channel, long *value)
+{
+	const struct ssam_temp *ssam_temp = dev_get_drvdata(dev);
+
+	return ssam_tmp_get_temperature(ssam_temp->sdev, channel + 1, value);
+}
+
+static const struct hwmon_channel_info * const ssam_temp_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip,
+			   HWMON_C_REGISTER_TZ),
+	/* We have at most 16 thermal sensor channels. */
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops ssam_temp_hwmon_ops = {
+	.is_visible = ssam_temp_hwmon_is_visible,
+	.read = ssam_temp_hwmon_read,
+};
+
+static const struct hwmon_chip_info ssam_temp_hwmon_chip_info = {
+	.ops = &ssam_temp_hwmon_ops,
+	.info = ssam_temp_hwmon_info,
+};
+
+static int ssam_temp_probe(struct ssam_device *sdev)
+{
+	struct ssam_temp *ssam_temp;
+	struct device *hwmon_dev;
+	s16 sensors;
+	int status;
+
+	status = ssam_tmp_get_available_sensors(sdev, &sensors);
+	if (status)
+		return status;
+
+	ssam_temp = devm_kzalloc(&sdev->dev, sizeof(*ssam_temp), GFP_KERNEL);
+	if (!ssam_temp)
+		return -ENOMEM;
+
+	ssam_temp->sdev = sdev;
+	ssam_temp->sensors = sensors;
+
+	hwmon_dev = devm_hwmon_device_register_with_info(&sdev->dev,
+			"surface_thermal", ssam_temp, &ssam_temp_hwmon_chip_info,
+			NULL);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	return 0;
+}
+
+static const struct ssam_device_id ssam_temp_match[] = {
+	{ SSAM_SDEV(TMP, SAM, 0x00, 0x02) },
+	{ },
+};
+MODULE_DEVICE_TABLE(ssam, ssam_temp_match);
+
+static struct ssam_device_driver ssam_temp = {
+	.probe = ssam_temp_probe,
+	.match_table = ssam_temp_match,
+	.driver = {
+		.name = "surface_temp",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+module_ssam_device_driver(ssam_temp);
+
+MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
+MODULE_DESCRIPTION("Thermal sensor subsystem driver for Surface System Aggregator Module");
+MODULE_LICENSE("GPL");
-- 
2.43.0

From aef610f92f0370d3b11472e302da555edca811dd Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 30 Dec 2023 18:12:23 +0100
Subject: [PATCH] hwmon: surface_temp: Add support for sensor names

The thermal subsystem of the Surface Aggregator Module allows us to
query the names of the respective thermal sensors. Forward those to
userspace.

Signed-off-by: Ivor Wanders <ivor@iwanders.net>
Co-Developed-by: Maximilian Luz <luzmaximilian@gmail.com>
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Patchset: surface-sam
---
 drivers/hwmon/surface_temp.c | 113 +++++++++++++++++++++++++++++------
 1 file changed, 96 insertions(+), 17 deletions(-)

diff --git a/drivers/hwmon/surface_temp.c b/drivers/hwmon/surface_temp.c
index 48c3e826713f6..4c08926139dbf 100644
--- a/drivers/hwmon/surface_temp.c
+++ b/drivers/hwmon/surface_temp.c
@@ -17,6 +17,27 @@
 
 /* -- SAM interface. -------------------------------------------------------- */
 
+/*
+ * Available sensors are indicated by a 16-bit bitfield, where a 1 marks the
+ * presence of a sensor. So we have at most 16 possible sensors/channels.
+ */
+#define SSAM_TMP_SENSOR_MAX_COUNT 16
+
+/*
+ * All names observed so far are 6 characters long, but there's only
+ * zeros after the name, so perhaps they can be longer. This number reflects
+ * the maximum zero-padded space observed in the returned buffer.
+ */
+#define SSAM_TMP_SENSOR_NAME_LENGTH 18
+
+struct ssam_tmp_get_name_rsp {
+	__le16 unknown1;
+	char unknown2;
+	char name[SSAM_TMP_SENSOR_NAME_LENGTH];
+} __packed;
+
+static_assert(sizeof(struct ssam_tmp_get_name_rsp) == 21);
+
 SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_get_available_sensors, __le16, {
 	.target_category = SSAM_SSH_TC_TMP,
 	.command_id      = 0x04,
@@ -27,6 +48,11 @@ SSAM_DEFINE_SYNC_REQUEST_MD_R(__ssam_tmp_get_temperature, __le16, {
 	.command_id      = 0x01,
 });
 
+SSAM_DEFINE_SYNC_REQUEST_MD_R(__ssam_tmp_get_name, struct ssam_tmp_get_name_rsp, {
+	.target_category = SSAM_SSH_TC_TMP,
+	.command_id      = 0x0e,
+});
+
 static int ssam_tmp_get_available_sensors(struct ssam_device *sdev, s16 *sensors)
 {
 	__le16 sensors_le;
@@ -54,12 +80,37 @@ static int ssam_tmp_get_temperature(struct ssam_device *sdev, u8 iid, long *temp
 	return 0;
 }
 
+static int ssam_tmp_get_name(struct ssam_device *sdev, u8 iid, char *buf, size_t buf_len)
+{
+	struct ssam_tmp_get_name_rsp name_rsp;
+	int status;
+
+	status =  __ssam_tmp_get_name(sdev->ctrl, sdev->uid.target, iid, &name_rsp);
+	if (status)
+		return status;
+
+	/*
+	 * This should not fail unless the name in the returned struct is not
+	 * null-terminated or someone changed something in the struct
+	 * definitions above, since our buffer and struct have the same
+	 * capacity by design. So if this fails blow this up with a warning.
+	 * Since the more likely cause is that the returned string isn't
+	 * null-terminated, we might have received garbage (as opposed to just
+	 * an incomplete string), so also fail the function.
+	 */
+	status = strscpy(buf, name_rsp.name, buf_len);
+	WARN_ON(status < 0);
+
+	return status < 0 ? status : 0;
+}
+
 
 /* -- Driver.---------------------------------------------------------------- */
 
 struct ssam_temp {
 	struct ssam_device *sdev;
 	s16 sensors;
+	char names[SSAM_TMP_SENSOR_MAX_COUNT][SSAM_TMP_SENSOR_NAME_LENGTH];
 };
 
 static umode_t ssam_temp_hwmon_is_visible(const void *data,
@@ -83,33 +134,47 @@ static int ssam_temp_hwmon_read(struct device *dev,
 	return ssam_tmp_get_temperature(ssam_temp->sdev, channel + 1, value);
 }
 
+static int ssam_temp_hwmon_read_string(struct device *dev,
+				       enum hwmon_sensor_types type,
+				       u32 attr, int channel, const char **str)
+{
+	const struct ssam_temp *ssam_temp = dev_get_drvdata(dev);
+
+	*str = ssam_temp->names[channel];
+	return 0;
+}
+
 static const struct hwmon_channel_info * const ssam_temp_hwmon_info[] = {
 	HWMON_CHANNEL_INFO(chip,
 			   HWMON_C_REGISTER_TZ),
-	/* We have at most 16 thermal sensor channels. */
+	/*
+	 * We have at most SSAM_TMP_SENSOR_MAX_COUNT = 16 thermal sensor
+	 * channels.
+	 */
 	HWMON_CHANNEL_INFO(temp,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT,
-			   HWMON_T_INPUT),
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
 	NULL
 };
 
 static const struct hwmon_ops ssam_temp_hwmon_ops = {
 	.is_visible = ssam_temp_hwmon_is_visible,
 	.read = ssam_temp_hwmon_read,
+	.read_string = ssam_temp_hwmon_read_string,
 };
 
 static const struct hwmon_chip_info ssam_temp_hwmon_chip_info = {
@@ -122,6 +187,7 @@ static int ssam_temp_probe(struct ssam_device *sdev)
 	struct ssam_temp *ssam_temp;
 	struct device *hwmon_dev;
 	s16 sensors;
+	int channel;
 	int status;
 
 	status = ssam_tmp_get_available_sensors(sdev, &sensors);
@@ -135,6 +201,19 @@ static int ssam_temp_probe(struct ssam_device *sdev)
 	ssam_temp->sdev = sdev;
 	ssam_temp->sensors = sensors;
 
+	/* Retrieve the name for each available sensor. */
+	for (channel = 0; channel < SSAM_TMP_SENSOR_MAX_COUNT; channel++)
+	{
+		if (!(sensors & BIT(channel)))
+			continue;
+
+		status = ssam_tmp_get_name(sdev, channel + 1,
+					   ssam_temp->names[channel],
+					   SSAM_TMP_SENSOR_NAME_LENGTH);
+		if (status)
+			return status;
+	}
+
 	hwmon_dev = devm_hwmon_device_register_with_info(&sdev->dev,
 			"surface_thermal", ssam_temp, &ssam_temp_hwmon_chip_info,
 			NULL);
-- 
2.43.0

From e2cecbfd921d8e15791f549b26465aef5dabc001 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 30 Dec 2023 18:21:12 +0100
Subject: [PATCH] platform/surface: aggregator_registry: Add support for
 thermal sensors on the Surface Pro 9

The Surface Pro 9 has thermal sensors connected via the Surface
Aggregator Module. Add a device node to support those.

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index b0db25886c996..797802d16f319 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -74,6 +74,12 @@ static const struct software_node ssam_node_tmp_pprof = {
 	.parent = &ssam_node_root,
 };
 
+/* Thermal sensors. */
+static const struct software_node ssam_node_tmp_sensors = {
+	.name = "ssam:01:03:01:00:02",
+	.parent = &ssam_node_root,
+};
+
 /* Fan speed function. */
 static const struct software_node ssam_node_fan_speed = {
 	.name = "ssam:01:05:01:01:01",
@@ -325,6 +331,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_sensors,
 	&ssam_node_fan_speed,
 	&ssam_node_pos_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
-- 
2.43.0