Merge tag 'thunderbolt-for-v6.12-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt into usb-next

Mika writes:

thunderbolt: Changes for v6.12 merge window

This includes following USB4/Thunderbolt changes for the v6.12 merge
window:

  - Improvements for software receiver lane margining
  - Enable support for optional voltage offset range for receiver lane
    margining.

All these have been in linux-next with no reported issues.

* tag 'thunderbolt-for-v6.12-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt:
  thunderbolt: Improve software receiver lane margining
  thunderbolt: Add optional voltage offset range for receiver lane margining
  thunderbolt: Consolidate margining parameters into a structure
  thunderbolt: Add missing usb4_port_sb_read() to usb4_port_sw_margin()

4 files changed, 452 insertions, 52 deletions

diff --git a/drivers/thunderbolt/debugfs.c b/drivers/thunderbolt/debugfs.c
index 9ed4bb2e8d05..350310bd0fee 100644
--- a/drivers/thunderbolt/debugfs.c
+++ b/drivers/thunderbolt/debugfs.c
@@ -9,6 +9,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/debugfs.h>
+#include <linux/delay.h>
 #include <linux/pm_runtime.h>
 #include <linux/uaccess.h>
 
@@ -34,6 +35,14 @@
 
 #define COUNTER_SET_LEN		3
 
+/*
+ * USB4 spec doesn't specify dwell range, the range of 100 ms to 500 ms
+ * probed to give good results.
+ */
+#define MIN_DWELL_TIME		100 /* ms */
+#define MAX_DWELL_TIME		500 /* ms */
+#define DWELL_SAMPLE_INTERVAL	10
+
 /* Sideband registers and their sizes as defined in the USB4 spec */
 struct sb_reg {
 	unsigned int reg;
@@ -394,8 +403,15 @@ out:
  * @ber_level: Current BER level contour value
  * @voltage_steps: Number of mandatory voltage steps
  * @max_voltage_offset: Maximum mandatory voltage offset (in mV)
+ * @voltage_steps_optional_range: Number of voltage steps for optional range
+ * @max_voltage_offset_optional_range: Maximum voltage offset for the optional
+ *					range (in mV).
  * @time_steps: Number of time margin steps
  * @max_time_offset: Maximum time margin offset (in mUI)
+ * @voltage_time_offset: Offset for voltage / time for software margining
+ * @dwell_time: Dwell time for software margining (in ms)
+ * @error_counter: Error counter operation for software margining
+ * @optional_voltage_offset_range: Enable optional extended voltage range
  * @software: %true if software margining is used instead of hardware
  * @time: %true if time margining is used instead of voltage
  * @right_high: %false if left/low margin test is performed, %true if
@@ -414,13 +430,37 @@ struct tb_margining {
 	unsigned int ber_level;
 	unsigned int voltage_steps;
 	unsigned int max_voltage_offset;
+	unsigned int voltage_steps_optional_range;
+	unsigned int max_voltage_offset_optional_range;
 	unsigned int time_steps;
 	unsigned int max_time_offset;
+	unsigned int voltage_time_offset;
+	unsigned int dwell_time;
+	enum usb4_margin_sw_error_counter error_counter;
+	bool optional_voltage_offset_range;
 	bool software;
 	bool time;
 	bool right_high;
 };
 
+static int margining_modify_error_counter(struct tb_margining *margining,
+	u32 lanes, enum usb4_margin_sw_error_counter error_counter)
+{
+	struct usb4_port_margining_params params = { 0 };
+	struct tb_port *port = margining->port;
+	u32 result;
+
+	if (error_counter != USB4_MARGIN_SW_ERROR_COUNTER_CLEAR &&
+	    error_counter != USB4_MARGIN_SW_ERROR_COUNTER_STOP)
+		return -EOPNOTSUPP;
+
+	params.error_counter = error_counter;
+	params.lanes = lanes;
+
+	return usb4_port_sw_margin(port, margining->target, margining->index,
+				   &params, &result);
+}
+
 static bool supports_software(const struct tb_margining *margining)
 {
 	return margining->caps[0] & USB4_MARGIN_CAP_0_MODES_SW;
@@ -454,6 +494,12 @@ independent_time_margins(const struct tb_margining *margining)
 	return FIELD_GET(USB4_MARGIN_CAP_1_TIME_INDP_MASK, margining->caps[1]);
 }
 
+static bool
+supports_optional_voltage_offset_range(const struct tb_margining *margining)
+{
+	return margining->caps[0] & USB4_MARGIN_CAP_0_OPT_VOLTAGE_SUPPORT;
+}
+
 static ssize_t
 margining_ber_level_write(struct file *file, const char __user *user_buf,
 			   size_t count, loff_t *ppos)
@@ -553,6 +599,14 @@ static int margining_caps_show(struct seq_file *s, void *not_used)
 		   margining->voltage_steps);
 	seq_printf(s, "# maximum voltage offset: %u mV\n",
 		   margining->max_voltage_offset);
+	seq_printf(s, "# optional voltage offset range support: %s\n",
+		   str_yes_no(supports_optional_voltage_offset_range(margining)));
+	if (supports_optional_voltage_offset_range(margining)) {
+		seq_printf(s, "# voltage margin steps, optional range: %u\n",
+			   margining->voltage_steps_optional_range);
+		seq_printf(s, "# maximum voltage offset, optional range: %u mV\n",
+			   margining->max_voltage_offset_optional_range);
+	}
 
 	switch (independent_voltage_margins(margining)) {
 	case USB4_MARGIN_CAP_0_VOLTAGE_MIN:
@@ -667,6 +721,198 @@ static int margining_lanes_show(struct seq_file *s, void *not_used)
 }
 DEBUGFS_ATTR_RW(margining_lanes);
 
+static ssize_t
+margining_voltage_time_offset_write(struct file *file,
+				    const char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+	unsigned int max_margin;
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint_from_user(user_buf, count, 10, &val);
+	if (ret)
+		return ret;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		if (margining->time)
+			max_margin = margining->time_steps;
+		else
+			if (margining->optional_voltage_offset_range)
+				max_margin = margining->voltage_steps_optional_range;
+			else
+				max_margin = margining->voltage_steps;
+
+		margining->voltage_time_offset = clamp(val, 0, max_margin);
+	}
+
+	return count;
+}
+
+static int margining_voltage_time_offset_show(struct seq_file *s,
+					      void *not_used)
+{
+	const struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		seq_printf(s, "%d\n", margining->voltage_time_offset);
+	}
+
+	return 0;
+}
+DEBUGFS_ATTR_RW(margining_voltage_time_offset);
+
+static ssize_t
+margining_error_counter_write(struct file *file, const char __user *user_buf,
+			      size_t count, loff_t *ppos)
+{
+	enum usb4_margin_sw_error_counter error_counter;
+	struct seq_file *s = file->private_data;
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+	char *buf;
+
+	buf = validate_and_copy_from_user(user_buf, &count);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	buf[count - 1] = '\0';
+
+	if (!strcmp(buf, "nop"))
+		error_counter = USB4_MARGIN_SW_ERROR_COUNTER_NOP;
+	else if (!strcmp(buf, "clear"))
+		error_counter = USB4_MARGIN_SW_ERROR_COUNTER_CLEAR;
+	else if (!strcmp(buf, "start"))
+		error_counter = USB4_MARGIN_SW_ERROR_COUNTER_START;
+	else if (!strcmp(buf, "stop"))
+		error_counter = USB4_MARGIN_SW_ERROR_COUNTER_STOP;
+	else
+		return -EINVAL;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		margining->error_counter = error_counter;
+	}
+
+	return count;
+}
+
+static int margining_error_counter_show(struct seq_file *s, void *not_used)
+{
+	const struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		switch (margining->error_counter) {
+		case USB4_MARGIN_SW_ERROR_COUNTER_NOP:
+			seq_puts(s, "[nop] clear start stop\n");
+			break;
+		case USB4_MARGIN_SW_ERROR_COUNTER_CLEAR:
+			seq_puts(s, "nop [clear] start stop\n");
+			break;
+		case USB4_MARGIN_SW_ERROR_COUNTER_START:
+			seq_puts(s, "nop clear [start] stop\n");
+			break;
+		case USB4_MARGIN_SW_ERROR_COUNTER_STOP:
+			seq_puts(s, "nop clear start [stop]\n");
+			break;
+		}
+	}
+
+	return 0;
+}
+DEBUGFS_ATTR_RW(margining_error_counter);
+
+static ssize_t
+margining_dwell_time_write(struct file *file, const char __user *user_buf,
+			   size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint_from_user(user_buf, count, 10, &val);
+	if (ret)
+		return ret;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		margining->dwell_time = clamp(val, MIN_DWELL_TIME, MAX_DWELL_TIME);
+	}
+
+	return count;
+}
+
+static int margining_dwell_time_show(struct seq_file *s, void *not_used)
+{
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		if (!margining->software)
+			return -EOPNOTSUPP;
+
+		seq_printf(s, "%d\n", margining->dwell_time);
+	}
+
+	return 0;
+}
+DEBUGFS_ATTR_RW(margining_dwell_time);
+
+static ssize_t
+margining_optional_voltage_offset_write(struct file *file, const char __user *user_buf,
+					size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+	bool val;
+	int ret;
+
+	ret = kstrtobool_from_user(user_buf, count, &val);
+	if (ret)
+		return ret;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		margining->optional_voltage_offset_range = val;
+	}
+
+	return count;
+}
+
+static int margining_optional_voltage_offset_show(struct seq_file *s,
+						  void *not_used)
+{
+	struct tb_margining *margining = s->private;
+	struct tb *tb = margining->port->sw->tb;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
+		seq_printf(s, "%u\n", margining->optional_voltage_offset_range);
+	}
+
+	return 0;
+}
+DEBUGFS_ATTR_RW(margining_optional_voltage_offset);
+
 static ssize_t margining_mode_write(struct file *file,
 				   const char __user *user_buf,
 				   size_t count, loff_t *ppos)
@@ -739,6 +985,51 @@ static int margining_mode_show(struct seq_file *s, void *not_used)
 }
 DEBUGFS_ATTR_RW(margining_mode);
 
+static int margining_run_sw(struct tb_margining *margining,
+			    struct usb4_port_margining_params *params)
+{
+	u32 nsamples = margining->dwell_time / DWELL_SAMPLE_INTERVAL;
+	int ret, i;
+
+	ret = usb4_port_sw_margin(margining->port, margining->target, margining->index,
+				  params, margining->results);
+	if (ret)
+		goto out_stop;
+
+	for (i = 0; i <= nsamples; i++) {
+		u32 errors = 0;
+
+		ret = usb4_port_sw_margin_errors(margining->port, margining->target,
+						 margining->index, &margining->results[1]);
+		if (ret)
+			break;
+
+		if (margining->lanes == USB4_MARGIN_SW_LANE_0)
+			errors = FIELD_GET(USB4_MARGIN_SW_ERR_COUNTER_LANE_0_MASK,
+					   margining->results[1]);
+		else if (margining->lanes == USB4_MARGIN_SW_LANE_1)
+			errors = FIELD_GET(USB4_MARGIN_SW_ERR_COUNTER_LANE_1_MASK,
+					   margining->results[1]);
+		else if (margining->lanes == USB4_MARGIN_SW_ALL_LANES)
+			errors = margining->results[1];
+
+		/* Any errors stop the test */
+		if (errors)
+			break;
+
+		fsleep(DWELL_SAMPLE_INTERVAL * USEC_PER_MSEC);
+	}
+
+out_stop:
+	/*
+	 * Stop the counters but don't clear them to allow the
+	 * different error counter configurations.
+	 */
+	margining_modify_error_counter(margining, margining->lanes,
+				       USB4_MARGIN_SW_ERROR_COUNTER_STOP);
+	return ret;
+}
+
 static int margining_run_write(void *data, u64 val)
 {
 	struct tb_margining *margining = data;
@@ -779,36 +1070,43 @@ static int margining_run_write(void *data, u64 val)
 		clx = ret;
 	}
 
+	/* Clear the results */
+	memset(margining->results, 0, sizeof(margining->results));
+
 	if (margining->software) {
+		struct usb4_port_margining_params params = {
+			.error_counter = USB4_MARGIN_SW_ERROR_COUNTER_CLEAR,
+			.lanes = margining->lanes,
+			.time = margining->time,
+			.voltage_time_offset = margining->voltage_time_offset,
+			.right_high = margining->right_high,
+			.optional_voltage_offset_range = margining->optional_voltage_offset_range,
+		};
+
 		tb_port_dbg(port,
 			    "running software %s lane margining for %s lanes %u\n",
 			    margining->time ? "time" : "voltage", dev_name(dev),
 			    margining->lanes);
-		ret = usb4_port_sw_margin(port, margining->target, margining->index,
-					  margining->lanes, margining->time,
-					  margining->right_high,
-					  USB4_MARGIN_SW_COUNTER_CLEAR);
-		if (ret)
-			goto out_clx;
 
-		ret = usb4_port_sw_margin_errors(port, margining->target,
-						 margining->index,
-						 &margining->results[0]);
+		ret = margining_run_sw(margining, &params);
 	} else {
+		struct usb4_port_margining_params params = {
+			.ber_level = margining->ber_level,
+			.lanes = margining->lanes,
+			.time = margining->time,
+			.right_high = margining->right_high,
+			.optional_voltage_offset_range = margining->optional_voltage_offset_range,
+		};
+
 		tb_port_dbg(port,
 			    "running hardware %s lane margining for %s lanes %u\n",
 			    margining->time ? "time" : "voltage", dev_name(dev),
 			    margining->lanes);
-		/* Clear the results */
-		margining->results[0] = 0;
-		margining->results[1] = 0;
-		ret = usb4_port_hw_margin(port, margining->target, margining->index,
-					  margining->lanes, margining->ber_level,
-					  margining->time, margining->right_high,
+
+		ret = usb4_port_hw_margin(port, margining->target, margining->index, &params,
 					  margining->results);
 	}
 
-out_clx:
 	if (down_sw)
 		tb_switch_clx_enable(down_sw, clx);
 out_unlock:
@@ -837,6 +1135,13 @@ static ssize_t margining_results_write(struct file *file,
 	margining->results[0] = 0;
 	margining->results[1] = 0;
 
+	if (margining->software) {
+		/* Clear the error counters */
+		margining_modify_error_counter(margining,
+					       USB4_MARGIN_SW_ALL_LANES,
+					       USB4_MARGIN_SW_ERROR_COUNTER_CLEAR);
+	}
+
 	mutex_unlock(&tb->lock);
 	return count;
 }
@@ -852,6 +1157,8 @@ static void voltage_margin_show(struct seq_file *s,
 	if (val & USB4_MARGIN_HW_RES_1_EXCEEDS)
 		seq_puts(s, " exceeds maximum");
 	seq_puts(s, "\n");
+	if (margining->optional_voltage_offset_range)
+		seq_puts(s, " optional voltage offset range enabled\n");
 }
 
 static void time_margin_show(struct seq_file *s,
@@ -924,6 +1231,24 @@ static int margining_results_show(struct seq_file *s, void *not_used)
 				voltage_margin_show(s, margining, val);
 			}
 		}
+	} else {
+		u32 lane_errors, result;
+
+		seq_printf(s, "0x%08x\n", margining->results[1]);
+		result = FIELD_GET(USB4_MARGIN_SW_LANES_MASK, margining->results[0]);
+
+		if (result == USB4_MARGIN_SW_LANE_0 ||
+		    result == USB4_MARGIN_SW_ALL_LANES) {
+			lane_errors = FIELD_GET(USB4_MARGIN_SW_ERR_COUNTER_LANE_0_MASK,
+						margining->results[1]);
+			seq_printf(s, "# lane 0 errors: %u\n", lane_errors);
+		}
+		if (result == USB4_MARGIN_SW_LANE_1 ||
+		    result == USB4_MARGIN_SW_ALL_LANES) {
+			lane_errors = FIELD_GET(USB4_MARGIN_SW_ERR_COUNTER_LANE_1_MASK,
+						margining->results[1]);
+			seq_printf(s, "# lane 1 errors: %u\n", lane_errors);
+		}
 	}
 
 	mutex_unlock(&tb->lock);
@@ -1091,6 +1416,15 @@ static struct tb_margining *margining_alloc(struct tb_port *port,
 	val = FIELD_GET(USB4_MARGIN_CAP_0_MAX_VOLTAGE_OFFSET_MASK, margining->caps[0]);
 	margining->max_voltage_offset = 74 + val * 2;
 
+	if (supports_optional_voltage_offset_range(margining)) {
+		val = FIELD_GET(USB4_MARGIN_CAP_0_VOLT_STEPS_OPT_MASK,
+				margining->caps[0]);
+		margining->voltage_steps_optional_range = val;
+		val = FIELD_GET(USB4_MARGIN_CAP_1_MAX_VOLT_OFS_OPT_MASK,
+				margining->caps[1]);
+		margining->max_voltage_offset_optional_range = 74 + val * 2;
+	}
+
 	if (supports_time(margining)) {
 		val = FIELD_GET(USB4_MARGIN_CAP_1_TIME_STEPS_MASK, margining->caps[1]);
 		margining->time_steps = val;
@@ -1127,6 +1461,22 @@ static struct tb_margining *margining_alloc(struct tb_port *port,
 	     independent_time_margins(margining) == USB4_MARGIN_CAP_1_TIME_LR))
 		debugfs_create_file("margin", 0600, dir, margining,
 				    &margining_margin_fops);
+
+	margining->error_counter = USB4_MARGIN_SW_ERROR_COUNTER_CLEAR;
+	margining->dwell_time = MIN_DWELL_TIME;
+
+	if (supports_optional_voltage_offset_range(margining))
+		debugfs_create_file("optional_voltage_offset", DEBUGFS_MODE, dir, margining,
+				    &margining_optional_voltage_offset_fops);
+
+	if (supports_software(margining)) {
+		debugfs_create_file("voltage_time_offset", DEBUGFS_MODE, dir, margining,
+				    &margining_voltage_time_offset_fops);
+		debugfs_create_file("error_counter", DEBUGFS_MODE, dir, margining,
+				    &margining_error_counter_fops);
+		debugfs_create_file("dwell_time", DEBUGFS_MODE, dir, margining,
+				    &margining_dwell_time_fops);
+	}
 	return margining;
 }
 
diff --git a/drivers/thunderbolt/sb_regs.h b/drivers/thunderbolt/sb_regs.h
index 2a88edfc97b2..dbcad25ead50 100644
--- a/drivers/thunderbolt/sb_regs.h
+++ b/drivers/thunderbolt/sb_regs.h
@@ -57,6 +57,9 @@ enum usb4_sb_opcode {
 #define USB4_MARGIN_CAP_0_TIME			BIT(5)
 #define USB4_MARGIN_CAP_0_VOLTAGE_STEPS_MASK	GENMASK(12, 6)
 #define USB4_MARGIN_CAP_0_MAX_VOLTAGE_OFFSET_MASK GENMASK(18, 13)
+#define USB4_MARGIN_CAP_0_OPT_VOLTAGE_SUPPORT	BIT(19)
+#define USB4_MARGIN_CAP_0_VOLT_STEPS_OPT_MASK	GENMASK(26, 20)
+#define USB4_MARGIN_CAP_1_MAX_VOLT_OFS_OPT_MASK GENMASK(7, 0)
 #define USB4_MARGIN_CAP_1_TIME_DESTR		BIT(8)
 #define USB4_MARGIN_CAP_1_TIME_INDP_MASK	GENMASK(10, 9)
 #define USB4_MARGIN_CAP_1_TIME_MIN		0x0
@@ -72,6 +75,7 @@ enum usb4_sb_opcode {
 #define USB4_MARGIN_HW_RH			BIT(4)
 #define USB4_MARGIN_HW_BER_MASK			GENMASK(9, 5)
 #define USB4_MARGIN_HW_BER_SHIFT		5
+#define USB4_MARGIN_HW_OPT_VOLTAGE		BIT(10)
 
 /* Applicable to all margin values */
 #define USB4_MARGIN_HW_RES_1_MARGIN_MASK	GENMASK(6, 0)
@@ -82,13 +86,17 @@ enum usb4_sb_opcode {
 #define USB4_MARGIN_HW_RES_1_L1_LL_MARGIN_SHIFT	24
 
 /* USB4_SB_OPCODE_RUN_SW_LANE_MARGINING */
+#define USB4_MARGIN_SW_LANES_MASK		GENMASK(2, 0)
+#define USB4_MARGIN_SW_LANE_0			0x0
+#define USB4_MARGIN_SW_LANE_1			0x1
+#define USB4_MARGIN_SW_ALL_LANES		0x7
 #define USB4_MARGIN_SW_TIME			BIT(3)
 #define USB4_MARGIN_SW_RH			BIT(4)
+#define USB4_MARGIN_SW_OPT_VOLTAGE		BIT(5)
+#define USB4_MARGIN_SW_VT_MASK			GENMASK(12, 6)
 #define USB4_MARGIN_SW_COUNTER_MASK		GENMASK(14, 13)
-#define USB4_MARGIN_SW_COUNTER_SHIFT		13
-#define USB4_MARGIN_SW_COUNTER_NOP		0x0
-#define USB4_MARGIN_SW_COUNTER_CLEAR		0x1
-#define USB4_MARGIN_SW_COUNTER_START		0x2
-#define USB4_MARGIN_SW_COUNTER_STOP		0x3
+
+#define USB4_MARGIN_SW_ERR_COUNTER_LANE_0_MASK	GENMASK(3, 0)
+#define USB4_MARGIN_SW_ERR_COUNTER_LANE_1_MASK	GENMASK(7, 4)
 
 #endif
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index b47f7873c847..6737188f2581 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1353,14 +1353,48 @@ int usb4_port_sb_read(struct tb_port *port, enum usb4_sb_target target, u8 index
 int usb4_port_sb_write(struct tb_port *port, enum usb4_sb_target target,
 		       u8 index, u8 reg, const void *buf, u8 size);
 
+/**
+ * enum usb4_margin_sw_error_counter - Software margining error counter operation
+ * @USB4_MARGIN_SW_ERROR_COUNTER_NOP: No change in counter setup
+ * @USB4_MARGIN_SW_ERROR_COUNTER_CLEAR: Set the error counter to 0, enable counter
+ * @USB4_MARGIN_SW_ERROR_COUNTER_START: Start counter, count from last value
+ * @USB4_MARGIN_SW_ERROR_COUNTER_STOP: Stop counter, do not clear value
+ */
+enum usb4_margin_sw_error_counter {
+	USB4_MARGIN_SW_ERROR_COUNTER_NOP,
+	USB4_MARGIN_SW_ERROR_COUNTER_CLEAR,
+	USB4_MARGIN_SW_ERROR_COUNTER_START,
+	USB4_MARGIN_SW_ERROR_COUNTER_STOP,
+};
+
+/**
+ * struct usb4_port_margining_params - USB4 margining parameters
+ * @error_counter: Error counter operation for software margining
+ * @ber_level: Current BER level contour value
+ * @lanes: %0, %1 or %7 (all)
+ * @voltage_time_offset: Offset for voltage / time for software margining
+ * @optional_voltage_offset_range: Enable optional extended voltage range
+ * @right_high: %false if left/low margin test is performed, %true if right/high
+ * @time: %true if time margining is used instead of voltage
+ */
+struct usb4_port_margining_params {
+	enum usb4_margin_sw_error_counter error_counter;
+	u32 ber_level;
+	u32 lanes;
+	u32 voltage_time_offset;
+	bool optional_voltage_offset_range;
+	bool right_high;
+	bool time;
+};
+
 int usb4_port_margining_caps(struct tb_port *port, enum usb4_sb_target target,
 			     u8 index, u32 *caps);
 int usb4_port_hw_margin(struct tb_port *port, enum usb4_sb_target target,
-			u8 index, unsigned int lanes, unsigned int ber_level,
-			bool timing, bool right_high, u32 *results);
+			u8 index, const struct usb4_port_margining_params *params,
+			u32 *results);
 int usb4_port_sw_margin(struct tb_port *port, enum usb4_sb_target target,
-			u8 index, unsigned int lanes, bool timing,
-			bool right_high, u32 counter);
+			u8 index, const struct usb4_port_margining_params *params,
+			u32 *results);
 int usb4_port_sw_margin_errors(struct tb_port *port, enum usb4_sb_target target,
 			       u8 index, u32 *errors);
 
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 4d83b65afb5b..0a9b4aeb3fa1 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -1653,31 +1653,31 @@ int usb4_port_margining_caps(struct tb_port *port, enum usb4_sb_target target,
  * @port: USB4 port
  * @target: Sideband target
  * @index: Retimer index if taget is %USB4_SB_TARGET_RETIMER
- * @lanes: Which lanes to run (must match the port capabilities). Can be
- *	   %0, %1 or %7.
- * @ber_level: BER level contour value
- * @timing: Perform timing margining instead of voltage
- * @right_high: Use Right/high margin instead of left/low
+ * @params: Parameters for USB4 hardware margining
  * @results: Array with at least two elements to hold the results
  *
  * Runs hardware lane margining on USB4 port and returns the result in
  * @results.
  */
 int usb4_port_hw_margin(struct tb_port *port, enum usb4_sb_target target,
-			u8 index, unsigned int lanes, unsigned int ber_level,
-			bool timing, bool right_high, u32 *results)
+			u8 index, const struct usb4_port_margining_params *params,
+			u32 *results)
 {
 	u32 val;
 	int ret;
 
-	val = lanes;
-	if (timing)
+	if (WARN_ON_ONCE(!params))
+		return -EINVAL;
+
+	val = params->lanes;
+	if (params->time)
 		val |= USB4_MARGIN_HW_TIME;
-	if (right_high)
+	if (params->right_high)
 		val |= USB4_MARGIN_HW_RH;
-	if (ber_level)
-		val |= (ber_level << USB4_MARGIN_HW_BER_SHIFT) &
-			USB4_MARGIN_HW_BER_MASK;
+	if (params->ber_level)
+		val |= FIELD_PREP(USB4_MARGIN_HW_BER_MASK, params->ber_level);
+	if (params->optional_voltage_offset_range)
+		val |= USB4_MARGIN_HW_OPT_VOLTAGE;
 
 	ret = usb4_port_sb_write(port, target, index, USB4_SB_METADATA, &val,
 				 sizeof(val));
@@ -1698,38 +1698,46 @@ int usb4_port_hw_margin(struct tb_port *port, enum usb4_sb_target target,
  * @port: USB4 port
  * @target: Sideband target
  * @index: Retimer index if taget is %USB4_SB_TARGET_RETIMER
- * @lanes: Which lanes to run (must match the port capabilities). Can be
- *	   %0, %1 or %7.
- * @timing: Perform timing margining instead of voltage
- * @right_high: Use Right/high margin instead of left/low
- * @counter: What to do with the error counter
+ * @params: Parameters for USB4 software margining
+ * @results: Data word for the operation completion data
  *
  * Runs software lane margining on USB4 port. Read back the error
  * counters by calling usb4_port_sw_margin_errors(). Returns %0 in
  * success and negative errno otherwise.
  */
 int usb4_port_sw_margin(struct tb_port *port, enum usb4_sb_target target,
-			u8 index, unsigned int lanes, bool timing,
-			bool right_high, u32 counter)
+			u8 index, const struct usb4_port_margining_params *params,
+			u32 *results)
 {
 	u32 val;
 	int ret;
 
-	val = lanes;
-	if (timing)
+	if (WARN_ON_ONCE(!params))
+		return -EINVAL;
+
+	val = params->lanes;
+	if (params->time)
 		val |= USB4_MARGIN_SW_TIME;
-	if (right_high)
+	if (params->optional_voltage_offset_range)
+		val |= USB4_MARGIN_SW_OPT_VOLTAGE;
+	if (params->right_high)
 		val |= USB4_MARGIN_SW_RH;
-	val |= (counter << USB4_MARGIN_SW_COUNTER_SHIFT) &
-		USB4_MARGIN_SW_COUNTER_MASK;
+	val |= FIELD_PREP(USB4_MARGIN_SW_COUNTER_MASK, params->error_counter);
+	val |= FIELD_PREP(USB4_MARGIN_SW_VT_MASK, params->voltage_time_offset);
 
 	ret = usb4_port_sb_write(port, target, index, USB4_SB_METADATA, &val,
 				 sizeof(val));
 	if (ret)
 		return ret;
 
-	return usb4_port_sb_op(port, target, index,
-			       USB4_SB_OPCODE_RUN_SW_LANE_MARGINING, 2500);
+	ret = usb4_port_sb_op(port, target, index,
+			      USB4_SB_OPCODE_RUN_SW_LANE_MARGINING, 2500);
+	if (ret)
+		return ret;
+
+	return usb4_port_sb_read(port, target, index, USB4_SB_DATA, results,
+				 sizeof(*results));
+
 }
 
 /**