* [PATCH 0/4] usb: typec: ucsi: revert broken buffer management
@ 2025-12-22 15:22 Johan Hovold
2025-12-22 15:22 ` [PATCH 1/4] Revert "usb: typec: ucsi: Add support for SET_PDOS command" Johan Hovold
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Johan Hovold @ 2025-12-22 15:22 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel,
Johan Hovold
The new buffer management code has not been tested or reviewed properly
and breaks boot of machines like the Lenovo ThinkPad X13s.
Fixing this will require designing a proper interface for managing these
transactions, something which most likely involves reverting most of the
offending commit anyway.
Revert the broken code to fix the regression and let Intel come up with
a properly tested implementation for a later kernel.
Johan
Johan Hovold (4):
Revert "usb: typec: ucsi: Add support for SET_PDOS command"
Revert "usb: typec: ucsi: Enable debugfs for message_out data
structure"
Revert "usb: typec: ucsi: Add support for message out data structure"
Revert "usb: typec: ucsi: Update UCSI structure to have message in and
message out fields"
drivers/usb/typec/ucsi/cros_ec_ucsi.c | 5 +-
drivers/usb/typec/ucsi/debugfs.c | 36 +-------
drivers/usb/typec/ucsi/displayport.c | 11 +--
drivers/usb/typec/ucsi/ucsi.c | 118 ++++++++----------------
drivers/usb/typec/ucsi/ucsi.h | 22 ++---
drivers/usb/typec/ucsi/ucsi_acpi.c | 25 +----
drivers/usb/typec/ucsi/ucsi_ccg.c | 11 ++-
drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 15 +--
8 files changed, 71 insertions(+), 172 deletions(-)
--
2.51.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] Revert "usb: typec: ucsi: Add support for SET_PDOS command"
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
@ 2025-12-22 15:22 ` Johan Hovold
2025-12-22 15:22 ` [PATCH 2/4] Revert "usb: typec: ucsi: Enable debugfs for message_out data structure" Johan Hovold
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2025-12-22 15:22 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel,
Johan Hovold
This reverts commit 1b474ee01fbb73b1365adbf9b3067f7375e471ee.
The new buffer management code that this feature relies on is broken so
revert for now.
The interface for writing data and support for UCSI_SET_PDOS looks like
it could use some more thought as well.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/typec/ucsi/debugfs.c | 1 -
drivers/usb/typec/ucsi/ucsi.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/usb/typec/ucsi/debugfs.c b/drivers/usb/typec/ucsi/debugfs.c
index 174f4d53b777..90d11b79d2c0 100644
--- a/drivers/usb/typec/ucsi/debugfs.c
+++ b/drivers/usb/typec/ucsi/debugfs.c
@@ -37,7 +37,6 @@ static int ucsi_cmd(void *data, u64 val)
case UCSI_SET_USB:
case UCSI_SET_POWER_LEVEL:
case UCSI_READ_POWER_LEVEL:
- case UCSI_SET_PDOS:
ucsi->message_in_size = 0;
ret = ucsi_send_command(ucsi, val);
break;
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index f946b728c373..d01b796a8d23 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -137,7 +137,6 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num);
#define UCSI_GET_PD_MESSAGE 0x15
#define UCSI_GET_CAM_CS 0x18
#define UCSI_SET_SINK_PATH 0x1c
-#define UCSI_SET_PDOS 0x1d
#define UCSI_READ_POWER_LEVEL 0x1e
#define UCSI_SET_USB 0x21
#define UCSI_GET_LPM_PPM_INFO 0x22
--
2.51.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] Revert "usb: typec: ucsi: Enable debugfs for message_out data structure"
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
2025-12-22 15:22 ` [PATCH 1/4] Revert "usb: typec: ucsi: Add support for SET_PDOS command" Johan Hovold
@ 2025-12-22 15:22 ` Johan Hovold
2025-12-22 15:22 ` [PATCH 3/4] Revert "usb: typec: ucsi: Add support for message out " Johan Hovold
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2025-12-22 15:22 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel,
Johan Hovold
This reverts commit 775fae520e6ae62c393a8daf42dc534f09692f3f.
The new buffer management code that this relies on is broken so revert
for now.
It also looks like the error handling needs some more thought as the
message out size is not reset on errors.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/typec/ucsi/debugfs.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/usb/typec/ucsi/debugfs.c b/drivers/usb/typec/ucsi/debugfs.c
index 90d11b79d2c0..924f93027553 100644
--- a/drivers/usb/typec/ucsi/debugfs.c
+++ b/drivers/usb/typec/ucsi/debugfs.c
@@ -110,30 +110,6 @@ static int ucsi_vbus_volt_show(struct seq_file *m, void *v)
}
DEFINE_SHOW_ATTRIBUTE(ucsi_vbus_volt);
-static ssize_t ucsi_message_out_write(struct file *file,
- const char __user *data, size_t count, loff_t *ppos)
-{
- struct ucsi *ucsi = file->private_data;
- int ret;
-
- char *buf __free(kfree) = memdup_user_nul(data, count);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
-
- ucsi->message_out_size = min(count / 2, UCSI_MAX_MESSAGE_OUT_LENGTH);
- ret = hex2bin(ucsi->message_out, buf, ucsi->message_out_size);
- if (ret)
- return ret;
-
- return count;
-}
-
-static const struct file_operations ucsi_message_out_fops = {
- .open = simple_open,
- .write = ucsi_message_out_write,
- .llseek = generic_file_llseek,
-};
-
void ucsi_debugfs_register(struct ucsi *ucsi)
{
ucsi->debugfs = kzalloc(sizeof(*ucsi->debugfs), GFP_KERNEL);
@@ -146,8 +122,6 @@ void ucsi_debugfs_register(struct ucsi *ucsi)
debugfs_create_file("peak_current", 0400, ucsi->debugfs->dentry, ucsi, &ucsi_peak_curr_fops);
debugfs_create_file("avg_current", 0400, ucsi->debugfs->dentry, ucsi, &ucsi_avg_curr_fops);
debugfs_create_file("vbus_voltage", 0400, ucsi->debugfs->dentry, ucsi, &ucsi_vbus_volt_fops);
- debugfs_create_file("message_out", 0200, ucsi->debugfs->dentry, ucsi,
- &ucsi_message_out_fops);
}
void ucsi_debugfs_unregister(struct ucsi *ucsi)
--
2.51.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] Revert "usb: typec: ucsi: Add support for message out data structure"
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
2025-12-22 15:22 ` [PATCH 1/4] Revert "usb: typec: ucsi: Add support for SET_PDOS command" Johan Hovold
2025-12-22 15:22 ` [PATCH 2/4] Revert "usb: typec: ucsi: Enable debugfs for message_out data structure" Johan Hovold
@ 2025-12-22 15:22 ` Johan Hovold
2025-12-22 15:22 ` [PATCH 4/4] Revert "usb: typec: ucsi: Update UCSI structure to have message in and message out fields" Johan Hovold
2025-12-22 22:15 ` [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Katiyar, Pooja
4 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2025-12-22 15:22 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel,
Johan Hovold
This reverts commit db0028637cc832add6d87564fcc2ebb12781b046.
The new buffer management code that this feature relies on is broken so
revert for now.
As for the in buffer, nothing prevents the out message size and buffer
from being modified while the message is being processed due to lack of
serialisation.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/typec/ucsi/ucsi.c | 14 --------------
drivers/usb/typec/ucsi/ucsi.h | 2 --
drivers/usb/typec/ucsi/ucsi_acpi.c | 16 ----------------
3 files changed, 32 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 9b3df776137a..819540713150 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -67,20 +67,6 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci)
reinit_completion(&ucsi->complete);
- if (ucsi->message_out_size > 0) {
- if (!ucsi->ops->write_message_out) {
- ucsi->message_out_size = 0;
- ret = -EOPNOTSUPP;
- goto out_clear_bit;
- }
-
- ret = ucsi->ops->write_message_out(ucsi, ucsi->message_out,
- ucsi->message_out_size);
- ucsi->message_out_size = 0;
- if (ret)
- goto out_clear_bit;
- }
-
ret = ucsi->ops->async_control(ucsi, command);
if (ret)
goto out_clear_bit;
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index d01b796a8d23..479bf1f69c72 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -69,7 +69,6 @@ struct dentry;
* @read_cci: Read CCI register
* @poll_cci: Read CCI register while polling with notifications disabled
* @read_message_in: Read message data from UCSI
- * @write_message_out: Write message data to UCSI
* @sync_control: Blocking control operation
* @async_control: Non-blocking control operation
* @update_altmodes: Squashes duplicate DP altmodes
@@ -85,7 +84,6 @@ struct ucsi_operations {
int (*read_cci)(struct ucsi *ucsi, u32 *cci);
int (*poll_cci)(struct ucsi *ucsi, u32 *cci);
int (*read_message_in)(struct ucsi *ucsi, void *val, size_t val_len);
- int (*write_message_out)(struct ucsi *ucsi, void *data, size_t data_len);
int (*sync_control)(struct ucsi *ucsi, u64 command, u32 *cci);
int (*async_control)(struct ucsi *ucsi, u64 command);
bool (*update_altmodes)(struct ucsi *ucsi, u8 recipient,
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index f9beeb835238..f1d1f6917b09 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -86,21 +86,6 @@ static int ucsi_acpi_read_message_in(struct ucsi *ucsi, void *val, size_t val_le
return 0;
}
-static int ucsi_acpi_write_message_out(struct ucsi *ucsi, void *data, size_t data_len)
-{
- struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
-
- if (!data || !data_len)
- return -EINVAL;
-
- if (ucsi->version <= UCSI_VERSION_1_2)
- memcpy(ua->base + UCSI_MESSAGE_OUT, data, data_len);
- else
- memcpy(ua->base + UCSIv2_MESSAGE_OUT, data, data_len);
-
- return 0;
-}
-
static int ucsi_acpi_async_control(struct ucsi *ucsi, u64 command)
{
struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
@@ -116,7 +101,6 @@ static const struct ucsi_operations ucsi_acpi_ops = {
.read_cci = ucsi_acpi_read_cci,
.poll_cci = ucsi_acpi_poll_cci,
.read_message_in = ucsi_acpi_read_message_in,
- .write_message_out = ucsi_acpi_write_message_out,
.sync_control = ucsi_sync_control_common,
.async_control = ucsi_acpi_async_control
};
--
2.51.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] Revert "usb: typec: ucsi: Update UCSI structure to have message in and message out fields"
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
` (2 preceding siblings ...)
2025-12-22 15:22 ` [PATCH 3/4] Revert "usb: typec: ucsi: Add support for message out " Johan Hovold
@ 2025-12-22 15:22 ` Johan Hovold
2025-12-22 22:15 ` [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Katiyar, Pooja
4 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2025-12-22 15:22 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel,
Johan Hovold
This reverts commit 3e082978c33151d576694deac8abde021ea669a8.
The new buffer management code has not been tested or reviewed properly
and breaks boot of machines like the Lenovo ThinkPad X13s:
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000000
CPU: 0 UID: 0 PID: 813 Comm: kworker/0:3 Not tainted 6.19.0-rc2 #26 PREEMPT
Hardware name: LENOVO 21BYZ9SRUS/21BYZ9SRUS, BIOS N3HET87W (1.59 ) 12/05/2023
Workqueue: events ucsi_handle_connector_change [typec_ucsi]
Call trace:
ucsi_sync_control_common+0xe4/0x1ec [typec_ucsi] (P)
ucsi_run_command+0xcc/0x194 [typec_ucsi]
ucsi_send_command_common+0x84/0x2a0 [typec_ucsi]
ucsi_get_connector_status+0x48/0x78 [typec_ucsi]
ucsi_handle_connector_change+0x5c/0x4f4 [typec_ucsi]
process_one_work+0x208/0x60c
worker_thread+0x244/0x388
The new code completely ignores concurrency so that the message length
can be updated while a transaction is ongoing. In the above case, the
length ends up being modified by another thread while processing an ack
so that the NULL cci pointer is dereferenced.
Fixing this will require designing a proper interface for managing these
transactions, something which most likely involves reverting most of the
offending commit anyway.
Revert the broken code to fix the regression and let Intel come up with
a properly tested implementation for a later kernel.
Fixes: 3e082978c331 ("usb: typec: ucsi: Update UCSI structure to have message in and message out fields")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/typec/ucsi/cros_ec_ucsi.c | 5 +-
drivers/usb/typec/ucsi/debugfs.c | 9 +-
drivers/usb/typec/ucsi/displayport.c | 11 +--
drivers/usb/typec/ucsi/ucsi.c | 104 ++++++++----------------
drivers/usb/typec/ucsi/ucsi.h | 19 ++---
drivers/usb/typec/ucsi/ucsi_acpi.c | 9 +-
drivers/usb/typec/ucsi/ucsi_ccg.c | 11 +--
drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 15 ++--
8 files changed, 71 insertions(+), 112 deletions(-)
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
index d753f2188e25..eed2a7d0ebc6 100644
--- a/drivers/usb/typec/ucsi/cros_ec_ucsi.c
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
@@ -105,12 +105,13 @@ static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
return 0;
}
-static int cros_ucsi_sync_control(struct ucsi *ucsi, u64 cmd, u32 *cci)
+static int cros_ucsi_sync_control(struct ucsi *ucsi, u64 cmd, u32 *cci,
+ void *data, size_t size)
{
struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
int ret;
- ret = ucsi_sync_control_common(ucsi, cmd, cci);
+ ret = ucsi_sync_control_common(ucsi, cmd, cci, data, size);
switch (ret) {
case -EBUSY:
/* EC may return -EBUSY if CCI.busy is set.
diff --git a/drivers/usb/typec/ucsi/debugfs.c b/drivers/usb/typec/ucsi/debugfs.c
index 924f93027553..f3684ab787fe 100644
--- a/drivers/usb/typec/ucsi/debugfs.c
+++ b/drivers/usb/typec/ucsi/debugfs.c
@@ -37,8 +37,7 @@ static int ucsi_cmd(void *data, u64 val)
case UCSI_SET_USB:
case UCSI_SET_POWER_LEVEL:
case UCSI_READ_POWER_LEVEL:
- ucsi->message_in_size = 0;
- ret = ucsi_send_command(ucsi, val);
+ ret = ucsi_send_command(ucsi, val, NULL, 0);
break;
case UCSI_GET_CAPABILITY:
case UCSI_GET_CONNECTOR_CAPABILITY:
@@ -53,9 +52,9 @@ static int ucsi_cmd(void *data, u64 val)
case UCSI_GET_ATTENTION_VDO:
case UCSI_GET_CAM_CS:
case UCSI_GET_LPM_PPM_INFO:
- ucsi->message_in_size = sizeof(ucsi->debugfs->response);
- ret = ucsi_send_command(ucsi, val);
- memcpy(&ucsi->debugfs->response, ucsi->message_in, sizeof(ucsi->debugfs->response));
+ ret = ucsi_send_command(ucsi, val,
+ &ucsi->debugfs->response,
+ sizeof(ucsi->debugfs->response));
break;
default:
ret = -EOPNOTSUPP;
diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index a09b4900ec76..8aae80b457d7 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -67,14 +67,11 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
}
command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(dp->con->num);
- ucsi->message_in_size = sizeof(cur);
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, &cur, sizeof(cur));
if (ret < 0) {
if (ucsi->version > 0x0100)
goto err_unlock;
cur = 0xff;
- } else {
- memcpy(&cur, ucsi->message_in, ucsi->message_in_size);
}
if (cur != 0xff) {
@@ -129,8 +126,7 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
}
command = UCSI_CMD_SET_NEW_CAM(dp->con->num, 0, dp->offset, 0);
- dp->con->ucsi->message_in_size = 0;
- ret = ucsi_send_command(dp->con->ucsi, command);
+ ret = ucsi_send_command(dp->con->ucsi, command, NULL, 0);
if (ret < 0)
goto out_unlock;
@@ -197,8 +193,7 @@ static int ucsi_displayport_configure(struct ucsi_dp *dp)
command = UCSI_CMD_SET_NEW_CAM(dp->con->num, 1, dp->offset, pins);
- dp->con->ucsi->message_in_size = 0;
- return ucsi_send_command(dp->con->ucsi, command);
+ return ucsi_send_command(dp->con->ucsi, command, NULL, 0);
}
static int ucsi_displayport_vdm(struct typec_altmode *alt,
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 819540713150..a7b388dc7fa0 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -55,7 +55,8 @@ void ucsi_notify_common(struct ucsi *ucsi, u32 cci)
}
EXPORT_SYMBOL_GPL(ucsi_notify_common);
-int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci)
+int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size)
{
bool ack = UCSI_COMMAND(command) == UCSI_ACK_CC_CI;
int ret;
@@ -83,10 +84,9 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci)
if (!ret && cci)
ret = ucsi->ops->read_cci(ucsi, cci);
- if (!ret && ucsi->message_in_size > 0 &&
+ if (!ret && data &&
(*cci & UCSI_CCI_COMMAND_COMPLETE))
- ret = ucsi->ops->read_message_in(ucsi, ucsi->message_in,
- ucsi->message_in_size);
+ ret = ucsi->ops->read_message_in(ucsi, data, size);
return ret;
}
@@ -103,25 +103,23 @@ static int ucsi_acknowledge(struct ucsi *ucsi, bool conn_ack)
ctrl |= UCSI_ACK_CONNECTOR_CHANGE;
}
- ucsi->message_in_size = 0;
- return ucsi->ops->sync_control(ucsi, ctrl, NULL);
+ return ucsi->ops->sync_control(ucsi, ctrl, NULL, NULL, 0);
}
-static int ucsi_run_command(struct ucsi *ucsi, u64 command, u32 *cci, bool conn_ack)
+static int ucsi_run_command(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size, bool conn_ack)
{
int ret, err;
*cci = 0;
- if (ucsi->message_in_size > UCSI_MAX_DATA_LENGTH(ucsi))
+ if (size > UCSI_MAX_DATA_LENGTH(ucsi))
return -EINVAL;
- ret = ucsi->ops->sync_control(ucsi, command, cci);
+ ret = ucsi->ops->sync_control(ucsi, command, cci, data, size);
- if (*cci & UCSI_CCI_BUSY) {
- ucsi->message_in_size = 0;
- return ucsi_run_command(ucsi, UCSI_CANCEL, cci, false) ?: -EBUSY;
- }
+ if (*cci & UCSI_CCI_BUSY)
+ return ucsi_run_command(ucsi, UCSI_CANCEL, cci, NULL, 0, false) ?: -EBUSY;
if (ret)
return ret;
@@ -153,13 +151,10 @@ static int ucsi_read_error(struct ucsi *ucsi, u8 connector_num)
int ret;
command = UCSI_GET_ERROR_STATUS | UCSI_CONNECTOR_NUMBER(connector_num);
- ucsi->message_in_size = sizeof(error);
- ret = ucsi_run_command(ucsi, command, &cci, false);
+ ret = ucsi_run_command(ucsi, command, &cci, &error, sizeof(error), false);
if (ret < 0)
return ret;
- memcpy(&error, ucsi->message_in, sizeof(error));
-
switch (error) {
case UCSI_ERROR_INCOMPATIBLE_PARTNER:
return -EOPNOTSUPP;
@@ -205,7 +200,8 @@ static int ucsi_read_error(struct ucsi *ucsi, u8 connector_num)
return -EIO;
}
-static int ucsi_send_command_common(struct ucsi *ucsi, u64 cmd, bool conn_ack)
+static int ucsi_send_command_common(struct ucsi *ucsi, u64 cmd,
+ void *data, size_t size, bool conn_ack)
{
u8 connector_num;
u32 cci;
@@ -233,7 +229,7 @@ static int ucsi_send_command_common(struct ucsi *ucsi, u64 cmd, bool conn_ack)
mutex_lock(&ucsi->ppm_lock);
- ret = ucsi_run_command(ucsi, cmd, &cci, conn_ack);
+ ret = ucsi_run_command(ucsi, cmd, &cci, data, size, conn_ack);
if (cci & UCSI_CCI_ERROR)
ret = ucsi_read_error(ucsi, connector_num);
@@ -242,9 +238,10 @@ static int ucsi_send_command_common(struct ucsi *ucsi, u64 cmd, bool conn_ack)
return ret;
}
-int ucsi_send_command(struct ucsi *ucsi, u64 command)
+int ucsi_send_command(struct ucsi *ucsi, u64 command,
+ void *data, size_t size)
{
- return ucsi_send_command_common(ucsi, command, false);
+ return ucsi_send_command_common(ucsi, command, data, size, false);
}
EXPORT_SYMBOL_GPL(ucsi_send_command);
@@ -322,8 +319,7 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
int i;
command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(con->num);
- con->ucsi->message_in_size = sizeof(cur);
- ret = ucsi_send_command(con->ucsi, command);
+ ret = ucsi_send_command(con->ucsi, command, &cur, sizeof(cur));
if (ret < 0) {
if (con->ucsi->version > 0x0100) {
dev_err(con->ucsi->dev,
@@ -331,8 +327,6 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
return;
}
cur = 0xff;
- } else {
- memcpy(&cur, con->ucsi->message_in, sizeof(cur));
}
if (cur < UCSI_MAX_ALTMODES)
@@ -516,8 +510,7 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i);
- ucsi->message_in_size = sizeof(alt);
- len = ucsi_send_command(con->ucsi, command);
+ len = ucsi_send_command(con->ucsi, command, &alt, sizeof(alt));
/*
* We are collecting all altmodes first and then registering.
* Some type-C device will return zero length data beyond last
@@ -526,8 +519,6 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
if (len < 0)
return len;
- memcpy(&alt, ucsi->message_in, sizeof(alt));
-
/* We got all altmodes, now break out and register them */
if (!len || !alt.svid)
break;
@@ -595,15 +586,12 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i);
- con->ucsi->message_in_size = sizeof(alt);
- len = ucsi_send_command(con->ucsi, command);
+ len = ucsi_send_command(con->ucsi, command, alt, sizeof(alt));
if (len == -EBUSY)
continue;
if (len <= 0)
return len;
- memcpy(&alt, con->ucsi->message_in, sizeof(alt));
-
/*
* This code is requesting one alt mode at a time, but some PPMs
* may still return two. If that happens both alt modes need be
@@ -671,9 +659,7 @@ static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack)
UCSI_MAX_DATA_LENGTH(con->ucsi));
int ret;
- con->ucsi->message_in_size = size;
- ret = ucsi_send_command_common(con->ucsi, command, conn_ack);
- memcpy(&con->status, con->ucsi->message_in, size);
+ ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack);
return ret < 0 ? ret : 0;
}
@@ -696,9 +682,8 @@ static int ucsi_read_pdos(struct ucsi_connector *con,
command |= UCSI_GET_PDOS_PDO_OFFSET(offset);
command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1);
command |= is_source(role) ? UCSI_GET_PDOS_SRC_PDOS : 0;
- ucsi->message_in_size = num_pdos * sizeof(u32);
- ret = ucsi_send_command(ucsi, command);
- memcpy(pdos + offset, ucsi->message_in, num_pdos * sizeof(u32));
+ ret = ucsi_send_command(ucsi, command, pdos + offset,
+ num_pdos * sizeof(u32));
if (ret < 0 && ret != -ETIMEDOUT)
dev_err(ucsi->dev, "UCSI_GET_PDOS failed (%d)\n", ret);
@@ -785,9 +770,7 @@ static int ucsi_get_pd_message(struct ucsi_connector *con, u8 recipient,
command |= UCSI_GET_PD_MESSAGE_BYTES(len);
command |= UCSI_GET_PD_MESSAGE_TYPE(type);
- con->ucsi->message_in_size = len;
- ret = ucsi_send_command(con->ucsi, command);
- memcpy(data + offset, con->ucsi->message_in, len);
+ ret = ucsi_send_command(con->ucsi, command, data + offset, len);
if (ret < 0)
return ret;
}
@@ -952,9 +935,7 @@ static int ucsi_register_cable(struct ucsi_connector *con)
int ret;
command = UCSI_GET_CABLE_PROPERTY | UCSI_CONNECTOR_NUMBER(con->num);
- con->ucsi->message_in_size = sizeof(cable_prop);
- ret = ucsi_send_command(con->ucsi, command);
- memcpy(&cable_prop, con->ucsi->message_in, sizeof(cable_prop));
+ ret = ucsi_send_command(con->ucsi, command, &cable_prop, sizeof(cable_prop));
if (ret < 0) {
dev_err(con->ucsi->dev, "GET_CABLE_PROPERTY failed (%d)\n", ret);
return ret;
@@ -1015,9 +996,7 @@ static int ucsi_check_connector_capability(struct ucsi_connector *con)
return 0;
command = UCSI_GET_CONNECTOR_CAPABILITY | UCSI_CONNECTOR_NUMBER(con->num);
- con->ucsi->message_in_size = sizeof(con->cap);
- ret = ucsi_send_command(con->ucsi, command);
- memcpy(&con->cap, con->ucsi->message_in, sizeof(con->cap));
+ ret = ucsi_send_command(con->ucsi, command, &con->cap, sizeof(con->cap));
if (ret < 0) {
dev_err(con->ucsi->dev, "GET_CONNECTOR_CAPABILITY failed (%d)\n", ret);
return ret;
@@ -1401,8 +1380,7 @@ static int ucsi_reset_connector(struct ucsi_connector *con, bool hard)
else if (con->ucsi->version >= UCSI_VERSION_2_0)
command |= hard ? 0 : UCSI_CONNECTOR_RESET_DATA_VER_2_0;
- con->ucsi->message_in_size = 0;
- return ucsi_send_command(con->ucsi, command);
+ return ucsi_send_command(con->ucsi, command, NULL, 0);
}
static int ucsi_reset_ppm(struct ucsi *ucsi)
@@ -1483,8 +1461,7 @@ static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
{
int ret;
- con->ucsi->message_in_size = 0;
- ret = ucsi_send_command(con->ucsi, command);
+ ret = ucsi_send_command(con->ucsi, command, NULL, 0);
if (ret == -ETIMEDOUT) {
u64 c;
@@ -1492,8 +1469,7 @@ static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
ucsi_reset_ppm(con->ucsi);
c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy;
- con->ucsi->message_in_size = 0;
- ucsi_send_command(con->ucsi, c);
+ ucsi_send_command(con->ucsi, c, NULL, 0);
ucsi_reset_connector(con, true);
}
@@ -1646,13 +1622,10 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
/* Get connector capability */
command = UCSI_GET_CONNECTOR_CAPABILITY;
command |= UCSI_CONNECTOR_NUMBER(con->num);
- ucsi->message_in_size = sizeof(con->cap);
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, &con->cap, sizeof(con->cap));
if (ret < 0)
goto out_unlock;
- memcpy(&con->cap, ucsi->message_in, sizeof(con->cap));
-
if (UCSI_CONCAP(con, OPMODE_DRP))
cap->data = TYPEC_PORT_DRD;
else if (UCSI_CONCAP(con, OPMODE_DFP))
@@ -1849,20 +1822,17 @@ static int ucsi_init(struct ucsi *ucsi)
/* Enable basic notifications */
ntfy = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR;
command = UCSI_SET_NOTIFICATION_ENABLE | ntfy;
- ucsi->message_in_size = 0;
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, NULL, 0);
if (ret < 0)
goto err_reset;
/* Get PPM capabilities */
command = UCSI_GET_CAPABILITY;
- ucsi->message_in_size = BITS_TO_BYTES(UCSI_GET_CAPABILITY_SIZE);
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, &ucsi->cap,
+ BITS_TO_BYTES(UCSI_GET_CAPABILITY_SIZE));
if (ret < 0)
goto err_reset;
- memcpy(&ucsi->cap, ucsi->message_in, BITS_TO_BYTES(UCSI_GET_CAPABILITY_SIZE));
-
if (!ucsi->cap.num_connectors) {
ret = -ENODEV;
goto err_reset;
@@ -1892,8 +1862,7 @@ static int ucsi_init(struct ucsi *ucsi)
/* Enable all supported notifications */
ntfy = ucsi_get_supported_notifications(ucsi);
command = UCSI_SET_NOTIFICATION_ENABLE | ntfy;
- ucsi->message_in_size = 0;
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, NULL, 0);
if (ret < 0)
goto err_unregister;
@@ -1944,8 +1913,7 @@ static void ucsi_resume_work(struct work_struct *work)
/* Restore UCSI notification enable mask after system resume */
command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
- ucsi->message_in_size = 0;
- ret = ucsi_send_command(ucsi, command);
+ ret = ucsi_send_command(ucsi, command, NULL, 0);
if (ret < 0) {
dev_err(ucsi->dev, "failed to re-enable notifications (%d)\n", ret);
return;
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 479bf1f69c72..410389ef173a 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -29,10 +29,6 @@ struct dentry;
#define UCSI_MESSAGE_OUT 32
#define UCSIv2_MESSAGE_OUT 272
-/* Define maximum lengths for message buffers */
-#define UCSI_MAX_MESSAGE_IN_LENGTH 256
-#define UCSI_MAX_MESSAGE_OUT_LENGTH 256
-
/* UCSI versions */
#define UCSI_VERSION_1_0 0x0100
#define UCSI_VERSION_1_1 0x0110
@@ -84,7 +80,8 @@ struct ucsi_operations {
int (*read_cci)(struct ucsi *ucsi, u32 *cci);
int (*poll_cci)(struct ucsi *ucsi, u32 *cci);
int (*read_message_in)(struct ucsi *ucsi, void *val, size_t val_len);
- int (*sync_control)(struct ucsi *ucsi, u64 command, u32 *cci);
+ int (*sync_control)(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size);
int (*async_control)(struct ucsi *ucsi, u64 command);
bool (*update_altmodes)(struct ucsi *ucsi, u8 recipient,
struct ucsi_altmode *orig,
@@ -496,12 +493,6 @@ struct ucsi {
unsigned long quirks;
#define UCSI_NO_PARTNER_PDOS BIT(0) /* Don't read partner's PDOs */
#define UCSI_DELAY_DEVICE_PDOS BIT(1) /* Reading PDOs fails until the parter is in PD mode */
-
- /* Fixed-size buffers for incoming and outgoing messages */
- u8 message_in[UCSI_MAX_MESSAGE_IN_LENGTH];
- size_t message_in_size;
- u8 message_out[UCSI_MAX_MESSAGE_OUT_LENGTH];
- size_t message_out_size;
};
#define UCSI_MAX_DATA_LENGTH(u) (((u)->version < UCSI_VERSION_2_0) ? 0x10 : 0xff)
@@ -564,13 +555,15 @@ struct ucsi_connector {
struct usb_pd_identity cable_identity;
};
-int ucsi_send_command(struct ucsi *ucsi, u64 command);
+int ucsi_send_command(struct ucsi *ucsi, u64 command,
+ void *retval, size_t size);
void ucsi_altmode_update_active(struct ucsi_connector *con);
int ucsi_resume(struct ucsi *ucsi);
void ucsi_notify_common(struct ucsi *ucsi, u32 cci);
-int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci);
+int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size);
#if IS_ENABLED(CONFIG_POWER_SUPPLY)
int ucsi_register_port_psy(struct ucsi_connector *con);
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index f1d1f6917b09..6b92f296e985 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -105,14 +105,15 @@ static const struct ucsi_operations ucsi_acpi_ops = {
.async_control = ucsi_acpi_async_control
};
-static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci)
+static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *val, size_t len)
{
u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE |
UCSI_CONSTAT_PDOS_CHANGE;
struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
int ret;
- ret = ucsi_sync_control_common(ucsi, command, cci);
+ ret = ucsi_sync_control_common(ucsi, command, cci, val, len);
if (ret < 0)
return ret;
@@ -124,8 +125,8 @@ static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci)
if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS &&
ua->check_bogus_event) {
/* Clear the bogus change */
- if (*(u16 *)ucsi->message_in == bogus_change)
- *(u16 *)ucsi->message_in = 0;
+ if (*(u16 *)val == bogus_change)
+ *(u16 *)val = 0;
ua->check_bogus_event = false;
}
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index ead1b2a25c79..d83a0051c737 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -606,7 +606,8 @@ static int ucsi_ccg_async_control(struct ucsi *ucsi, u64 command)
return ccg_write(uc, reg, (u8 *)&command, sizeof(command));
}
-static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci)
+static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size)
{
struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
struct ucsi_connector *con;
@@ -628,16 +629,16 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci)
ucsi_ccg_update_set_new_cam_cmd(uc, con, &command);
}
- ret = ucsi_sync_control_common(ucsi, command, cci);
+ ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
switch (UCSI_COMMAND(command)) {
case UCSI_GET_CURRENT_CAM:
if (uc->has_multiple_dp)
- ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)ucsi->message_in);
+ ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)data);
break;
case UCSI_GET_ALTERNATE_MODES:
if (UCSI_ALTMODE_RECIPIENT(command) == UCSI_RECIPIENT_SOP) {
- struct ucsi_altmode *alt = (struct ucsi_altmode *)ucsi->message_in;
+ struct ucsi_altmode *alt = data;
if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID)
ucsi_ccg_nvidia_altmode(uc, alt, command);
@@ -645,7 +646,7 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci)
break;
case UCSI_GET_CAPABILITY:
if (uc->fw_build == CCG_FW_BUILD_NVIDIA_TEGRA) {
- struct ucsi_capability *cap = (struct ucsi_capability *)ucsi->message_in;
+ struct ucsi_capability *cap = data;
cap->features &= ~UCSI_CAP_ALT_MODE_DETAILS;
}
diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
index 299081444caa..0187c1c4b21a 100644
--- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
@@ -88,7 +88,8 @@ static int yoga_c630_ucsi_async_control(struct ucsi *ucsi, u64 command)
static int yoga_c630_ucsi_sync_control(struct ucsi *ucsi,
u64 command,
- u32 *cci)
+ u32 *cci,
+ void *data, size_t size)
{
int ret;
@@ -106,8 +107,8 @@ static int yoga_c630_ucsi_sync_control(struct ucsi *ucsi,
};
dev_dbg(ucsi->dev, "faking DP altmode for con1\n");
- memset(ucsi->message_in, 0, ucsi->message_in_size);
- memcpy(ucsi->message_in, &alt, min(sizeof(alt), ucsi->message_in_size));
+ memset(data, 0, size);
+ memcpy(data, &alt, min(sizeof(alt), size));
*cci = UCSI_CCI_COMMAND_COMPLETE | UCSI_SET_CCI_LENGTH(sizeof(alt));
return 0;
}
@@ -120,18 +121,18 @@ static int yoga_c630_ucsi_sync_control(struct ucsi *ucsi,
if (UCSI_COMMAND(command) == UCSI_GET_ALTERNATE_MODES &&
UCSI_GET_ALTMODE_GET_CONNECTOR_NUMBER(command) == 2) {
dev_dbg(ucsi->dev, "ignoring altmodes for con2\n");
- memset(ucsi->message_in, 0, ucsi->message_in_size);
+ memset(data, 0, size);
*cci = UCSI_CCI_COMMAND_COMPLETE;
return 0;
}
- ret = ucsi_sync_control_common(ucsi, command, cci);
+ ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
if (ret < 0)
return ret;
/* UCSI_GET_CURRENT_CAM is off-by-one on all ports */
- if (UCSI_COMMAND(command) == UCSI_GET_CURRENT_CAM && ucsi->message_in_size > 0)
- ucsi->message_in[0]--;
+ if (UCSI_COMMAND(command) == UCSI_GET_CURRENT_CAM && data)
+ ((u8 *)data)[0]--;
return ret;
}
--
2.51.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] usb: typec: ucsi: revert broken buffer management
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
` (3 preceding siblings ...)
2025-12-22 15:22 ` [PATCH 4/4] Revert "usb: typec: ucsi: Update UCSI structure to have message in and message out fields" Johan Hovold
@ 2025-12-22 22:15 ` Katiyar, Pooja
2025-12-23 14:04 ` Johan Hovold
4 siblings, 1 reply; 9+ messages in thread
From: Katiyar, Pooja @ 2025-12-22 22:15 UTC (permalink / raw)
To: Johan Hovold, Heikki Krogerus, Greg Kroah-Hartman
Cc: Pooja Katiyar, Dmitry Baryshkov, linux-usb, linux-kernel
Hello Jonah,
On Mon, Dec 22, 2025 at 07:22:00AM -0800, Johan Hovold wrote:
> The new buffer management code has not been tested or reviewed properly
> and breaks boot of machines like the Lenovo ThinkPad X13s.
>
> Fixing this will require designing a proper interface for managing these
> transactions, something which most likely involves reverting most of the
> offending commit anyway.
>
> Revert the broken code to fix the regression and let Intel come up with
> a properly tested implementation for a later kernel.
>
Thanks! A fix patch addressing the race condition has been identified and
is being tested right now. It will be submitted for review shortly.
Here’s the discussion on same -
https://lore.kernel.org/all/349e1f70-7e40-4e3e-b078-6e001bbb5f1a@oss.qualcomm.com/
> Johan
>
>
> Johan Hovold (4):
> Revert "usb: typec: ucsi: Add support for SET_PDOS command"
> Revert "usb: typec: ucsi: Enable debugfs for message_out data
> structure"
> Revert "usb: typec: ucsi: Add support for message out data structure"
> Revert "usb: typec: ucsi: Update UCSI structure to have message in and
> message out fields"
>
> drivers/usb/typec/ucsi/cros_ec_ucsi.c | 5 +-
> drivers/usb/typec/ucsi/debugfs.c | 36 +-------
> drivers/usb/typec/ucsi/displayport.c | 11 +--
> drivers/usb/typec/ucsi/ucsi.c | 118 ++++++++----------------
> drivers/usb/typec/ucsi/ucsi.h | 22 ++---
> drivers/usb/typec/ucsi/ucsi_acpi.c | 25 +----
> drivers/usb/typec/ucsi/ucsi_ccg.c | 11 ++-
> drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 15 +--
> 8 files changed, 71 insertions(+), 172 deletions(-)
>
Regards,
Pooja
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] usb: typec: ucsi: revert broken buffer management
2025-12-22 22:15 ` [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Katiyar, Pooja
@ 2025-12-23 14:04 ` Johan Hovold
2025-12-23 14:24 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Johan Hovold @ 2025-12-23 14:04 UTC (permalink / raw)
To: Katiyar, Pooja
Cc: Heikki Krogerus, Greg Kroah-Hartman, Pooja Katiyar,
Dmitry Baryshkov, linux-usb, linux-kernel
On Mon, Dec 22, 2025 at 02:15:10PM -0800, Katiyar, Pooja wrote:
> On Mon, Dec 22, 2025 at 07:22:00AM -0800, Johan Hovold wrote:
> > The new buffer management code has not been tested or reviewed properly
> > and breaks boot of machines like the Lenovo ThinkPad X13s.
> >
> > Fixing this will require designing a proper interface for managing these
> > transactions, something which most likely involves reverting most of the
> > offending commit anyway.
> >
> > Revert the broken code to fix the regression and let Intel come up with
> > a properly tested implementation for a later kernel.
> >
>
> Thanks! A fix patch addressing the race condition has been identified and
> is being tested right now. It will be submitted for review shortly.
>
> Here’s the discussion on same -
> https://lore.kernel.org/all/349e1f70-7e40-4e3e-b078-6e001bbb5f1a@oss.qualcomm.com/
Yes, I'm aware that discussion and I still think this needs to be
reverted. Then you can propose a redesigned and tested implementation
that we can help you review as that kind of work is not something that
should be done as part of rc stabilisation.
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] usb: typec: ucsi: revert broken buffer management
2025-12-23 14:04 ` Johan Hovold
@ 2025-12-23 14:24 ` Greg Kroah-Hartman
2025-12-23 20:21 ` Katiyar, Pooja
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2025-12-23 14:24 UTC (permalink / raw)
To: Johan Hovold
Cc: Katiyar, Pooja, Heikki Krogerus, Pooja Katiyar, Dmitry Baryshkov,
linux-usb, linux-kernel
On Tue, Dec 23, 2025 at 03:04:49PM +0100, Johan Hovold wrote:
> On Mon, Dec 22, 2025 at 02:15:10PM -0800, Katiyar, Pooja wrote:
> > On Mon, Dec 22, 2025 at 07:22:00AM -0800, Johan Hovold wrote:
> > > The new buffer management code has not been tested or reviewed properly
> > > and breaks boot of machines like the Lenovo ThinkPad X13s.
> > >
> > > Fixing this will require designing a proper interface for managing these
> > > transactions, something which most likely involves reverting most of the
> > > offending commit anyway.
> > >
> > > Revert the broken code to fix the regression and let Intel come up with
> > > a properly tested implementation for a later kernel.
> > >
> >
> > Thanks! A fix patch addressing the race condition has been identified and
> > is being tested right now. It will be submitted for review shortly.
> >
> > Here’s the discussion on same -
> > https://lore.kernel.org/all/349e1f70-7e40-4e3e-b078-6e001bbb5f1a@oss.qualcomm.com/
>
> Yes, I'm aware that discussion and I still think this needs to be
> reverted. Then you can propose a redesigned and tested implementation
> that we can help you review as that kind of work is not something that
> should be done as part of rc stabilisation.
I agree, I don't see a submitted patch yet so I'll go take your reverts
at this point in time. That way people have more time to get this
correct instead of being rushed this time of the year.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] usb: typec: ucsi: revert broken buffer management
2025-12-23 14:24 ` Greg Kroah-Hartman
@ 2025-12-23 20:21 ` Katiyar, Pooja
0 siblings, 0 replies; 9+ messages in thread
From: Katiyar, Pooja @ 2025-12-23 20:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Johan Hovold
Cc: Heikki Krogerus, Pooja Katiyar, Dmitry Baryshkov, linux-usb,
linux-kernel
Hello Johan and Greg,
On Tue, Dec 23, 2025 at 06:24:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 23, 2025 at 03:04:49PM +0100, Johan Hovold wrote:
>> On Mon, Dec 22, 2025 at 02:15:10PM -0800, Katiyar, Pooja wrote:
>>> On Mon, Dec 22, 2025 at 07:22:00AM -0800, Johan Hovold wrote:
>>>> The new buffer management code has not been tested or reviewed properly
>>>> and breaks boot of machines like the Lenovo ThinkPad X13s.
>>>>
>>>> Fixing this will require designing a proper interface for managing these
>>>> transactions, something which most likely involves reverting most of the
>>>> offending commit anyway.
>>>>
>>>> Revert the broken code to fix the regression and let Intel come up with
>>>> a properly tested implementation for a later kernel.
>>>>
>>>
>>> Thanks! A fix patch addressing the race condition has been identified and
>>> is being tested right now. It will be submitted for review shortly.
>>>
>>> Here’s the discussion on same -
>>> https://lore.kernel.org/all/349e1f70-7e40-4e3e-b078-6e001bbb5f1a@oss.qualcomm.com/
>>
>> Yes, I'm aware that discussion and I still think this needs to be
>> reverted. Then you can propose a redesigned and tested implementation
>> that we can help you review as that kind of work is not something that
>> should be done as part of rc stabilisation.
>
> I agree, I don't see a submitted patch yet so I'll go take your reverts
> at this point in time. That way people have more time to get this
> correct instead of being rushed this time of the year.
>
> thanks,
>
> greg k-h
>
Thank you for your feedback! We will work on a redesign for clearer implementation.
Regards,
Pooja
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-12-23 20:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-22 15:22 [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Johan Hovold
2025-12-22 15:22 ` [PATCH 1/4] Revert "usb: typec: ucsi: Add support for SET_PDOS command" Johan Hovold
2025-12-22 15:22 ` [PATCH 2/4] Revert "usb: typec: ucsi: Enable debugfs for message_out data structure" Johan Hovold
2025-12-22 15:22 ` [PATCH 3/4] Revert "usb: typec: ucsi: Add support for message out " Johan Hovold
2025-12-22 15:22 ` [PATCH 4/4] Revert "usb: typec: ucsi: Update UCSI structure to have message in and message out fields" Johan Hovold
2025-12-22 22:15 ` [PATCH 0/4] usb: typec: ucsi: revert broken buffer management Katiyar, Pooja
2025-12-23 14:04 ` Johan Hovold
2025-12-23 14:24 ` Greg Kroah-Hartman
2025-12-23 20:21 ` Katiyar, Pooja
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).