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

From 92cfab5bd1d9a1e2b88a17a27e6e06b771a2fb88 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:23:53 +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
documenation. Additionally, introduce an enum containing all currently
known peers, i.e. targets and sources.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.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 +-
 include/linux/surface_aggregator/controller.h |  4 +-
 include/linux/surface_aggregator/serial_hub.h | 40 +++++++++++++------
 6 files changed, 58 insertions(+), 42 deletions(-)

diff --git a/Documentation/driver-api/surface_aggregator/client.rst b/Documentation/driver-api/surface_aggregator/client.rst
index 27f95abdbe99..9d7411223a84 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 bf007d6c9873..18fd0f0aee84 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 43e765199137..54bfb1f3d2dc 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);
@@ -1777,35 +1777,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 f3ecad92eefd..438873e06098 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/include/linux/surface_aggregator/controller.h b/include/linux/surface_aggregator/controller.h
index d11a1c6e3186..8932bc0bae18 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 45501b6e54e8..5c4ae1a26183 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.38.1

From 6bc6ddf3cd9bb505b2b5c2d9180471a6674e428f Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:30:58 +0100
Subject: [PATCH] platform/surface: aggregator: Ignore command messages not
 intended for us

It is possible that we (the host/kernel driver) receive command messages
that are not intended for us. Ignore those for now.

The whole story is a bit more complicated: It is possible to enable
debug output on SAM, which is sent via SSH command messages. By default
this output is sent to a debug connector, with its own target ID
(TID=0x03). It is possible to override the target of the debug output
and set it to the host/kernel driver. This, however, does not change the
original target ID of the message. Meaning, we receive messages with
TID=0x03 (debug) but expect to only receive messages with TID=0x00
(host).

The problem is that the different target ID also comes with a different
scope of request IDs. In particular, these do not follow the standard
event rules (i.e. do not fall into a set of small reserved values).
Therefore, current message handling interprets them as responses to
pending requests and tries to match them up via the request ID. However,
these debug output messages are not in fact responses, and therefore
this will at best fail to find the request and at worst pass on the
wrong data as response for a request.

Therefore ignore any command messages not intended for us (host) for
now. We can implement support for the debug messages once we have a
better understanding of them.

Note that this may also provide a bit more stability and avoid some
driver confusion in case any other targets want to talk to us in the
future, since we don't yet know what to do with those as well. A warning
for the dropped messages should suffice for now and also give us a
chance of discovering new targets if they come along without any
potential for bugs/instabilities.

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

diff --git a/drivers/platform/surface/aggregator/ssh_request_layer.c b/drivers/platform/surface/aggregator/ssh_request_layer.c
index f5565570f16c..90634dcacabf 100644
--- a/drivers/platform/surface/aggregator/ssh_request_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_request_layer.c
@@ -916,6 +916,21 @@ static void ssh_rtl_rx_command(struct ssh_ptl *p, const struct ssam_span *data)
 	if (sshp_parse_command(dev, data, &command, &command_data))
 		return;
 
+	/*
+	 * 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 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 != SSAM_SSH_TID_HOST) {
+		rtl_warn(rtl, "rtl: dropping message not intended for us (tid = %#04x)\n",
+			 command->tid);
+		return;
+	}
+
 	if (ssh_rqid_is_event(get_unaligned_le16(&command->rqid)))
 		ssh_rtl_rx_event(rtl, command, &command_data);
 	else
-- 
2.38.1

From ed86c2fd06a16439c0f943e3e11eaafabdebc944 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:42:19 +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>
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 2a2c17771d01..55cc61bba1da 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.38.1

From 955e918a080d74ce3b3e6ab236a04ff55923e4a4 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:32:16 +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>
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 43061514be38..62f27cdb6ca8 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.38.1

From 0816b11bbcdc1762aa2ee4bb252d374f3b1c5e03 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:33:26 +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>
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 27d95a6a7851..bd8cd453c393 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -247,7 +247,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,
 });
@@ -371,7 +371,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;
@@ -430,7 +430,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.38.1

From b306ce20065c9e8a60cc1122d36a34e2c409ef7e Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:34:06 +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>
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 ed36944467f9..0de76a784a35 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.38.1

From e97bfd1c74489d65c9c900d17bfb1efe9d145ea1 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:40:06 +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>
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 0635341bc517..42933bf3e925 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.38.1

From e0eb3c174f1cf81bd38655c8871735ce2d709355 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:45:28 +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>
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 d4aa8c81903a..aa80d83a83d1 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 62f27cdb6ca8..6abd1efe2088 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 bd8cd453c393..6147aa887939 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -510,8 +510,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 0de76a784a35..30cbde278c59 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 fbf2e11fd6ce..f433a13c3689 100644
--- a/drivers/platform/surface/surface_platform_profile.c
+++ b/drivers/platform/surface/surface_platform_profile.c
@@ -169,7 +169,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 540707882bb0..19d2f8834e56 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 59182d55742d..cabdd8da12d0 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 46c45d1b6368..4da20b7a0ee5 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.38.1

From f769515c00411c0c7d4ebe1aeedaa3c0658f8d36 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 5 Nov 2022 21:46:37 +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.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.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 6abd1efe2088..8b8b80228c14 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 585911020cea..3afe293ac114 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.38.1

From f1274adec1f3f91fad86eaed1566c9d043faa685 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Wed, 9 Nov 2022 13:13:12 +0100
Subject: [PATCH] wip! platform/surface: aggregator_registry: Add preliminary
 support for Surface Pro 9

Adds support for battery and charger status, as well as platform profile
and internal HID devices. Support for type-cover HID devices is still
missing, requiring validation.

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 3afe293ac114..443fe7647315 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -268,6 +268,7 @@ static const struct software_node *ssam_node_group_sp7[] = {
 	NULL,
 };
 
+/* Devices for Surface Pro 8 */
 static const struct software_node *ssam_node_group_sp8[] = {
 	&ssam_node_root,
 	&ssam_node_hub_kip,
@@ -284,6 +285,19 @@ static const struct software_node *ssam_node_group_sp8[] = {
 	NULL,
 };
 
+/* Devices for Surface Pro 9 */
+static const struct software_node *ssam_node_group_sp9[] = {
+	&ssam_node_root,
+	&ssam_node_hub_kip,
+	&ssam_node_bat_ac,
+	&ssam_node_bat_main,
+	&ssam_node_tmp_pprof,
+	/* TODO: typecover/KIP devices, we can likely reuse the SP8 node but need validation */
+	&ssam_node_hid_sam_sensors,
+	&ssam_node_hid_sam_ucm_ucsi,
+	NULL,
+};
+
 
 /* -- SSAM platform/meta-hub driver. ---------------------------------------- */
 
@@ -303,6 +317,9 @@ static const struct acpi_device_id ssam_platform_hub_match[] = {
 	/* Surface Pro 8 */
 	{ "MSHW0263", (unsigned long)ssam_node_group_sp8 },
 
+	/* Surface Pro 9 */
+	{ "MSHW0343", (unsigned long)ssam_node_group_sp9 },
+
 	/* Surface Book 2 */
 	{ "MSHW0107", (unsigned long)ssam_node_group_gen5 },
 
-- 
2.38.1

From 86721e97861259dadbbd998549bbc3339f4973bf Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sat, 12 Nov 2022 22:51:26 +0100
Subject: [PATCH] wip! platform/surface: aggregator_registry: Add preliminary
 typecover support for Surface Pro 9

Still missing: Tablet mode switch.

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

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 443fe7647315..58ad4cad445c 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -292,7 +292,11 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_tmp_pprof,
-	/* TODO: typecover/KIP devices, we can likely reuse the SP8 node but need validation */
+	/* TODO: POS tablet mode switch */
+	&ssam_node_hid_kip_keyboard,
+	&ssam_node_hid_kip_penstash,
+	&ssam_node_hid_kip_touchpad,
+	&ssam_node_hid_kip_fwupd,
 	&ssam_node_hid_sam_sensors,
 	&ssam_node_hid_sam_ucm_ucsi,
 	NULL,
-- 
2.38.1

From e585a9fde47a9f2418d160f5da97e274f3319482 Mon Sep 17 00:00:00 2001
From: Maximilian Luz <luzmaximilian@gmail.com>
Date: Sun, 13 Nov 2022 11:38:58 +0100
Subject: [PATCH] wip! platform/surface: aggregator: Do not check for repeated
 unsequenced packets

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

diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c b/drivers/platform/surface/aggregator/ssh_packet_layer.c
index 6748fe4ac5d5..e0d5b18ade0b 100644
--- a/drivers/platform/surface/aggregator/ssh_packet_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c
@@ -1596,16 +1596,33 @@ static void ssh_ptl_timeout_reap(struct work_struct *work)
 		ssh_ptl_tx_wakeup_packet(ptl);
 }
 
-static bool ssh_ptl_rx_retransmit_check(struct ssh_ptl *ptl, u8 seq)
+static bool ssh_ptl_rx_retransmit_check(struct ssh_ptl *ptl,
+					const struct ssh_frame *frame)
 {
 	int i;
 
+	/*
+	 * Ignore unsequenced packets. On some devices (notably Surface Pro 9),
+	 * unsequenced events will always be sent with SEQ=0x00. Attempting to
+	 * detect re-transmission would thus just block all events.
+	 * 
+	 * While sequence numbers would also allow detection of re-transmitted
+	 * packets in unsequenced communication, they have only ever been used
+	 * to cover edge-cases in sequenced transmission. In particular, the
+	 * only instance of packets being retransmitted (that we are aware of)
+	 * is due to an ACK timeout. As this does not happen in unsequenced
+	 * communication, skip the re-transmission check for those packets
+	 * entirely. 
+	 */
+	if (frame->type == SSH_FRAME_TYPE_DATA_NSQ)
+		return false;
+
 	/*
 	 * Check if SEQ has been seen recently (i.e. packet was
 	 * re-transmitted and we should ignore it).
 	 */
 	for (i = 0; i < ARRAY_SIZE(ptl->rx.blocked.seqs); i++) {
-		if (likely(ptl->rx.blocked.seqs[i] != seq))
+		if (likely(ptl->rx.blocked.seqs[i] != frame->seq))
 			continue;
 
 		ptl_dbg(ptl, "ptl: ignoring repeated data packet\n");
@@ -1613,7 +1630,7 @@ static bool ssh_ptl_rx_retransmit_check(struct ssh_ptl *ptl, u8 seq)
 	}
 
 	/* Update list of blocked sequence IDs. */
-	ptl->rx.blocked.seqs[ptl->rx.blocked.offset] = seq;
+	ptl->rx.blocked.seqs[ptl->rx.blocked.offset] = frame->seq;
 	ptl->rx.blocked.offset = (ptl->rx.blocked.offset + 1)
 				  % ARRAY_SIZE(ptl->rx.blocked.seqs);
 
@@ -1624,7 +1641,7 @@ static void ssh_ptl_rx_dataframe(struct ssh_ptl *ptl,
 				 const struct ssh_frame *frame,
 				 const struct ssam_span *payload)
 {
-	if (ssh_ptl_rx_retransmit_check(ptl, frame->seq))
+	if (ssh_ptl_rx_retransmit_check(ptl, frame))
 		return;
 
 	ptl->ops.data_received(ptl, payload);
-- 
2.38.1