[PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[David Vornholt]

From: David Vornholt <david@vornholt.online>

On a Lenovo ThinkPad X1 Carbon Gen 13 (BIOS N4BET78W 1.48, EC N4BHT61W
1.41, UCSI 1.0) with an HP E273m monitor connected, the PPM keeps
signaling a Negotiated Power Level Change although the power contract
stays the same, and signals it again right after it has been
acknowledged:

  ucsi_connector_change: port1 status: change=0040, opmode=3,
    connected=1, sourcing=0, partner_flags=2, partner_type=4,
    request_data_obj=1304b12c, BC status=2

This repeats about seven times a second for as long as the monitor stays
connected. Since commit a0d4618788f2 ("usb: typec: ucsi: Workaround for
missed op_mode change"), every Negotiated Power Level Change is handled
like a power operation mode change, so the driver reads the source PDOs
again, checks the alternate modes and the connector capability, and
registers the partner's PDOs again each time. Every UCSI command takes
about 50 ms of EC transactions on this machine. The EC GPE fires about
680 times a second, and irq/9-acpi and the kworkers keep a CPU busy.
Skipping that work for an unchanged contract only brings the rate down
to about 360 a second, because the PPM still signals every change and
the driver still reads the connector status and acknowledges it.

Keep handling the notification as before when the power operation mode
or the RDO changed, which the workaround for Dell's firmware relies on,
or when the source PDOs changed with it. Once the PPM has signaled it
ten times in a row with the same PD contract, each within a second of
the previous one and without a power operation mode change, disable the
notification with SET_NOTIFICATION_ENABLE and log it, and enable it
again when the partner on that connector is gone. The mask applies to
all connectors, so until then a contract change on another connector is
only handled with the next connector change that it does signal. This
PPM still reports the bit in the connector status change field. If the
PPM signals the notification while it is disabled, ignore it for an
unchanged contract, and send the mask again after another ten.

Resume and the reset after a timed-out role swap send ucsi->ntfy to the
PPM too. Update ucsi->ntfy and send it in one helper that holds
ppm_lock, so that none of them can send an outdated mask after another
one has changed it. Keep the new mask, and enable the notification again
later, even if the command fails, as the PPM may have applied it anyway.
Once ucsi_unregister() has disabled the notifications, don't send a mask
anymore.

Also forget the RDO when the partner goes away, so that the first
contract with the next partner counts as a new one.

With this, the PPM signals ten changes after the monitor is connected,
the driver disables the notification and the GPE rate drops to zero.
The ports, the partner and its 5 V/3 A power supply stay registered.

Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface")
Cc: stable@vger.kernel.org
Signed-off-by: David Vornholt <david@vornholt.online>
---

Notes:
    Changes in v2:
    - Enable the notification again when the partner on the connector that
      it was disabled for goes away, instead of leaving it disabled until
      the driver is unbound (Sashiko review).
    - Update ucsi->ntfy and send it to the PPM in one helper that holds
      ppm_lock, and use it on resume and in the reset path of
      ucsi_role_cmd() too, so that they can't send an outdated mask
      (Sashiko review).
    - No longer decide from ucsi->ntfy whether to ignore the notification.
      While it is disabled, ignore it for an unchanged contract, and send
      the mask again after another ten.
    - Keep the new mask, and enable the notification again later, even if
      SET_NOTIFICATION_ENABLE fails, and don't send a mask anymore once
      ucsi_unregister() has disabled the notifications (Codex review).
    - Start counting again after a power operation mode change, and don't
      count a power level change that comes with a source PDO change
      (Codex review).
    - v1: https://lore.kernel.org/r/20261010121026.84844-1-dv@etik.com
    
    Claude Code, an AI coding assistant, found the cause while I was looking
    into why irq/9-acpi kept a CPU busy, and wrote this patch and its
    description. Codex with GPT-6 Astra reviewed v2 before I sent it, and
    this version addresses its findings.
    
    Tested: v1 ported to v7.2.9 on the X1 Carbon Gen 13 with the HP E273m.
    The ucsi trace events show ten connector changes about 100 ms apart,
    one SET_NOTIFICATION_ENABLE and the warning, and no connector changes
    after that. GPE 0x6E went from about 680/s to 0/s.
    
    This version, ported to v7.2.9 on the same machine, with the monitor on
    the other port (con1): after the monitor is plugged in, the PPM signals
    the power level change about every 5 s, which doesn't count, and after
    about 50 s every 110 ms. After ten of those, the warning appears and
    SET_NOTIFICATION_ENABLE is sent once. The PPM still signals a power
    level change about every 5 to 15 s for another 45 s, which the driver
    ignores (no GET_PDOS), and then stops. GPE 0x6E goes to about 1/s.
    Unplugging the monitor sends SET_NOTIFICATION_ENABLE again, and after
    plugging it back in, the next storm is caught the same way. I did that
    twice.
    
    Against usb-linus, this version is only compile-tested (x86_64 defconfig
    with TYPEC_UCSI=m and UCSI_ACPI=m, W=1) and passes checkpatch --strict.
    
    Not tested: this version on con2; other PPMs, in particular Dell's that
    rely on a0d4618788f2; other partners on this laptop; suspend and resume
    with the notification disabled; the reset path of ucsi_role_cmd(); a
    failing SET_NOTIFICATION_ENABLE; unbinding while connector work runs;
    sparse (the version available to me was too old).
    
    If enabling the notification again fails, it's only retried when the
    next partner on that connector goes away, or sent again on resume.
    
    The patch applies as-is from v7.2 on. Older stable trees need a
    backport, in particular for the ucsi_run_command() call in
    ucsi_update_notifications().

 drivers/usb/typec/ucsi/ucsi.c | 119 +++++++++++++++++++++++++++++++---
 drivers/usb/typec/ucsi/ucsi.h |   5 ++
 2 files changed, 115 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index c1450639c..0d543f777 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -274,6 +274,37 @@ int ucsi_send_command(struct ucsi *ucsi, u64 command,
 }
 EXPORT_SYMBOL_GPL(ucsi_send_command);
 
+/*
+ * Update the notification enable mask and send it to the PPM. Holding ppm_lock
+ * across both keeps ucsi->ntfy the mask that the PPM was sent last. Keep it
+ * even if the command fails, as the PPM may have applied the mask anyway.
+ */
+static int ucsi_update_notifications(struct ucsi *ucsi, u64 clear, u64 set)
+{
+	u64 command;
+	u32 cci;
+	int ret;
+
+	mutex_lock(&ucsi->ppm_lock);
+
+	if (ucsi->unregistering) {
+		ret = -ESHUTDOWN;
+		goto out_unlock;
+	}
+
+	ucsi->ntfy = (ucsi->ntfy & ~clear) | set;
+	command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
+	ret = ucsi_run_command(ucsi, command, &cci, NULL, 0, NULL, 0, false);
+	if (cci & UCSI_CCI_ERROR)
+		ret = ucsi_read_error(ucsi, 0);
+
+	trace_ucsi_run_command(command, ret);
+
+out_unlock:
+	mutex_unlock(&ucsi->ppm_lock);
+	return ret;
+}
+
 int ucsi_write_message_out_command(struct ucsi *ucsi, u64 command,
 				   void *data, size_t size, void *msg_out,
 				   size_t msg_out_size)
@@ -1254,6 +1285,68 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
 	}
 }
 
+/*
+ * Some PPMs keep signaling a Negotiated Power Level Change although the power
+ * contract stays the same, and signal it again as soon as it has been
+ * acknowledged, so the PPM and the driver keep each other busy for as long as
+ * the partner stays connected. Disable the notification once the PPM has
+ * signaled it this many times in a row, each within a second of the previous
+ * one, without a new contract, and enable it again when the partner is gone.
+ * The notification can only be disabled for all connectors at once. A change
+ * of the source PDOs can come with an unchanged RDO, so it doesn't count.
+ */
+#define UCSI_PWR_LEVEL_UNCHANGED_MAX	10
+
+static void ucsi_pwr_level_change(struct ucsi_connector *con)
+{
+	struct ucsi *ucsi = con->ucsi;
+
+	if (UCSI_CONSTAT(con, PWR_OPMODE) != UCSI_CONSTAT_PWR_OPMODE_PD ||
+	    UCSI_CONSTAT(con, RDO) != con->rdo ||
+	    (UCSI_CONSTAT(con, CHANGE) & UCSI_CONSTAT_PDOS_CHANGE)) {
+		con->pwr_level_unchanged = 0;
+		ucsi_pwr_opmode_change(con);
+		return;
+	}
+
+	if (time_after(jiffies, con->pwr_level_change_time + HZ))
+		con->pwr_level_unchanged = 0;
+	con->pwr_level_change_time = jiffies;
+
+	if (++con->pwr_level_unchanged < UCSI_PWR_LEVEL_UNCHANGED_MAX) {
+		/* Signaled before the notification was disabled, or despite it */
+		if (!con->pwr_level_ntfy_disabled)
+			ucsi_pwr_opmode_change(con);
+		return;
+	}
+
+	/*
+	 * Enable it again when the partner is gone even if the command fails,
+	 * as the PPM may have disabled it anyway.
+	 */
+	if (!con->pwr_level_ntfy_disabled) {
+		con->pwr_level_ntfy_disabled = true;
+		dev_warn(ucsi->dev,
+			 "con%d: Firmware bug: power level changes without a new contract, disabling the notification\n",
+			 con->num);
+	}
+
+	if (ucsi_update_notifications(ucsi, UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE, 0) >= 0)
+		con->pwr_level_unchanged = 0;
+}
+
+/* The partner that the notification was disabled for is gone */
+static void ucsi_pwr_level_ntfy_restore(struct ucsi_connector *con)
+{
+	if (!con->pwr_level_ntfy_disabled)
+		return;
+
+	if (ucsi_update_notifications(con->ucsi, 0, UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE) < 0)
+		return;
+
+	con->pwr_level_ntfy_disabled = false;
+}
+
 static int ucsi_register_partner(struct ucsi_connector *con)
 {
 	u8 pwr_opmode = UCSI_CONSTAT(con, PWR_OPMODE);
@@ -1324,6 +1417,8 @@ static void ucsi_unregister_partner(struct ucsi_connector *con)
 	typec_unregister_partner(con->partner);
 	memset(&con->partner_identity, 0, sizeof(con->partner_identity));
 	con->partner = NULL;
+	con->rdo = 0;
+	con->pwr_level_unchanged = 0;
 }
 
 static void ucsi_partner_change(struct ucsi_connector *con)
@@ -1400,6 +1495,7 @@ static int ucsi_check_connection(struct ucsi_connector *con)
 		ucsi_partner_change(con);
 		ucsi_port_psy_changed(con);
 		ucsi_unregister_partner(con);
+		ucsi_pwr_level_ntfy_restore(con);
 	}
 
 	return 0;
@@ -1511,11 +1607,16 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 			}
 		} else {
 			ucsi_unregister_partner(con);
+			ucsi_pwr_level_ntfy_restore(con);
 		}
 	}
 
-	if (change & (UCSI_CONSTAT_POWER_OPMODE_CHANGE | UCSI_CONSTAT_POWER_LEVEL_CHANGE))
+	if (change & UCSI_CONSTAT_POWER_OPMODE_CHANGE) {
+		con->pwr_level_unchanged = 0;
 		ucsi_pwr_opmode_change(con);
+	} else if (change & UCSI_CONSTAT_POWER_LEVEL_CHANGE) {
+		ucsi_pwr_level_change(con);
+	}
 
 	if (con->partner && (change & UCSI_CONSTAT_PARTNER_CHANGE)) {
 		ucsi_partner_change(con);
@@ -1685,13 +1786,10 @@ static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
 
 	ret = ucsi_send_command(con->ucsi, command, NULL, 0);
 	if (ret == -ETIMEDOUT) {
-		u64 c;
-
 		/* PPM most likely stopped responding. Resetting everything. */
 		ucsi_reset_ppm(con->ucsi);
 
-		c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy;
-		ucsi_send_command(con->ucsi, c, NULL, 0);
+		ucsi_update_notifications(con->ucsi, 0, 0);
 
 		ucsi_reset_connector(con, true);
 	}
@@ -2171,12 +2269,10 @@ static void ucsi_resume_work(struct work_struct *work)
 {
 	struct ucsi *ucsi = container_of(work, struct ucsi, resume_work);
 	struct ucsi_connector *con;
-	u64 command;
 	int ret;
 
 	/* Restore UCSI notification enable mask after system resume */
-	command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
-	ret = ucsi_send_command(ucsi, command, NULL, 0);
+	ret = ucsi_update_notifications(ucsi, 0, 0);
 	if (ret < 0) {
 		dev_err(ucsi->dev, "failed to re-enable notifications (%d)\n", ret);
 		return;
@@ -2347,6 +2443,8 @@ int ucsi_register(struct ucsi *ucsi)
 	if (!ucsi->version)
 		return -ENODEV;
 
+	ucsi->unregistering = false;
+
 	/*
 	 * Version format is JJ.M.N (JJ = Major version, M = Minor version,
 	 * N = sub-minor version).
@@ -2380,7 +2478,10 @@ void ucsi_unregister(struct ucsi *ucsi)
 
 	ucsi_debugfs_unregister(ucsi);
 
-	/* Disable notifications */
+	/* Disable notifications, and keep connector work from enabling them */
+	mutex_lock(&ucsi->ppm_lock);
+	ucsi->unregistering = true;
+	mutex_unlock(&ucsi->ppm_lock);
 	ucsi->ops->async_control(ucsi, cmd);
 
 	if (!ucsi->connector)
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index dc594388d..137a83357 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -496,6 +496,8 @@ struct ucsi {
 
 	/* The latest "Notification Enable" bits (SET_NOTIFICATION_ENABLE) */
 	u64 ntfy;
+	/* Set by ucsi_unregister(), protected by ppm_lock */
+	bool unregistering;
 
 	/* PPM communication flags */
 	unsigned long flags;
@@ -554,6 +556,9 @@ struct ucsi_connector {
 	struct power_supply *psy;
 	struct power_supply_desc psy_desc;
 	u32 rdo;
+	unsigned long pwr_level_change_time;
+	unsigned int pwr_level_unchanged;
+	bool pwr_level_ntfy_disabled;
 	u32 src_pdos[PDO_MAX_OBJECTS];
 	int num_pdos;
 

base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
-- 
2.54.0

Re: [PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[Heikki Krogerus]

Hi David,

On Sat, Oct 10, 2026 at 08:39:17PM +0200, David Vornholt wrote:
> From: David Vornholt <david@vornholt.online>
> 
> On a Lenovo ThinkPad X1 Carbon Gen 13 (BIOS N4BET78W 1.48, EC N4BHT61W
> 1.41, UCSI 1.0) with an HP E273m monitor connected, the PPM keeps
> signaling a Negotiated Power Level Change although the power contract
> stays the same, and signals it again right after it has been
> acknowledged:
> 
>   ucsi_connector_change: port1 status: change=0040, opmode=3,
>     connected=1, sourcing=0, partner_flags=2, partner_type=4,
>     request_data_obj=1304b12c, BC status=2

Okay, so the PPM really should not report the power level changes
since there are not power level changes, right? Have you reported this
to Lenovo?

> This repeats about seven times a second for as long as the monitor stays
> connected. Since commit a0d4618788f2 ("usb: typec: ucsi: Workaround for
> missed op_mode change"), every Negotiated Power Level Change is handled
> like a power operation mode change, so the driver reads the source PDOs
> again, checks the alternate modes and the connector capability, and
> registers the partner's PDOs again each time. Every UCSI command takes
> about 50 ms of EC transactions on this machine. The EC GPE fires about
> 680 times a second, and irq/9-acpi and the kworkers keep a CPU busy.
> Skipping that work for an unchanged contract only brings the rate down
> to about 360 a second, because the PPM still signals every change and
> the driver still reads the connector status and acknowledges it.
> 
> Keep handling the notification as before when the power operation mode
> or the RDO changed, which the workaround for Dell's firmware relies on,
> or when the source PDOs changed with it. Once the PPM has signaled it
> ten times in a row with the same PD contract, each within a second of
> the previous one and without a power operation mode change, disable the
> notification with SET_NOTIFICATION_ENABLE and log it, and enable it
> again when the partner on that connector is gone. The mask applies to
> all connectors, so until then a contract change on another connector is
> only handled with the next connector change that it does signal. This
> PPM still reports the bit in the connector status change field. If the
> PPM signals the notification while it is disabled, ignore it for an
> unchanged contract, and send the mask again after another ten.
> 
> Resume and the reset after a timed-out role swap send ucsi->ntfy to the
> PPM too. Update ucsi->ntfy and send it in one helper that holds
> ppm_lock, so that none of them can send an outdated mask after another
> one has changed it. Keep the new mask, and enable the notification again
> later, even if the command fails, as the PPM may have applied it anyway.
> Once ucsi_unregister() has disabled the notifications, don't send a mask
> anymore.
> 
> Also forget the RDO when the partner goes away, so that the first
> contract with the next partner counts as a new one.
> 
> With this, the PPM signals ten changes after the monitor is connected,
> the driver disables the notification and the GPE rate drops to zero.
> The ports, the partner and its 5 V/3 A power supply stay registered.
> 
> Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Vornholt <david@vornholt.online>
> ---
> 
> Notes:
>     Changes in v2:
>     - Enable the notification again when the partner on the connector that
>       it was disabled for goes away, instead of leaving it disabled until
>       the driver is unbound (Sashiko review).
>     - Update ucsi->ntfy and send it to the PPM in one helper that holds
>       ppm_lock, and use it on resume and in the reset path of
>       ucsi_role_cmd() too, so that they can't send an outdated mask
>       (Sashiko review).
>     - No longer decide from ucsi->ntfy whether to ignore the notification.
>       While it is disabled, ignore it for an unchanged contract, and send
>       the mask again after another ten.
>     - Keep the new mask, and enable the notification again later, even if
>       SET_NOTIFICATION_ENABLE fails, and don't send a mask anymore once
>       ucsi_unregister() has disabled the notifications (Codex review).
>     - Start counting again after a power operation mode change, and don't
>       count a power level change that comes with a source PDO change
>       (Codex review).
>     - v1: https://lore.kernel.org/r/20261010121026.84844-1-dv@etik.com
>     
>     Claude Code, an AI coding assistant, found the cause while I was looking
>     into why irq/9-acpi kept a CPU busy, and wrote this patch and its
>     description. Codex with GPT-6 Astra reviewed v2 before I sent it, and
>     this version addresses its findings.
>     
>     Tested: v1 ported to v7.2.9 on the X1 Carbon Gen 13 with the HP E273m.
>     The ucsi trace events show ten connector changes about 100 ms apart,
>     one SET_NOTIFICATION_ENABLE and the warning, and no connector changes
>     after that. GPE 0x6E went from about 680/s to 0/s.
>     
>     This version, ported to v7.2.9 on the same machine, with the monitor on
>     the other port (con1): after the monitor is plugged in, the PPM signals
>     the power level change about every 5 s, which doesn't count, and after
>     about 50 s every 110 ms. After ten of those, the warning appears and
>     SET_NOTIFICATION_ENABLE is sent once. The PPM still signals a power
>     level change about every 5 to 15 s for another 45 s, which the driver
>     ignores (no GET_PDOS), and then stops. GPE 0x6E goes to about 1/s.
>     Unplugging the monitor sends SET_NOTIFICATION_ENABLE again, and after
>     plugging it back in, the next storm is caught the same way. I did that
>     twice.
>     
>     Against usb-linus, this version is only compile-tested (x86_64 defconfig
>     with TYPEC_UCSI=m and UCSI_ACPI=m, W=1) and passes checkpatch --strict.
>     
>     Not tested: this version on con2; other PPMs, in particular Dell's that
>     rely on a0d4618788f2; other partners on this laptop; suspend and resume
>     with the notification disabled; the reset path of ucsi_role_cmd(); a
>     failing SET_NOTIFICATION_ENABLE; unbinding while connector work runs;
>     sparse (the version available to me was too old).
>     
>     If enabling the notification again fails, it's only retried when the
>     next partner on that connector goes away, or sent again on resume.
>     
>     The patch applies as-is from v7.2 on. Older stable trees need a
>     backport, in particular for the ucsi_run_command() call in
>     ucsi_update_notifications().

This problem may only affect your system, so we really have to isolate
it somehow. I'm almost certain that like this there will be problems
on some other systems.

Something that I did not quite understand from the above is, if you
revert commit a0d4618788f2, do you still have a problem (I understand
that you still get the repeated notifications, but do they cause other
problems then)?

Thanks,

>  drivers/usb/typec/ucsi/ucsi.c | 119 +++++++++++++++++++++++++++++++---
>  drivers/usb/typec/ucsi/ucsi.h |   5 ++
>  2 files changed, 115 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index c1450639c..0d543f777 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -274,6 +274,37 @@ int ucsi_send_command(struct ucsi *ucsi, u64 command,
>  }
>  EXPORT_SYMBOL_GPL(ucsi_send_command);
>  
> +/*
> + * Update the notification enable mask and send it to the PPM. Holding ppm_lock
> + * across both keeps ucsi->ntfy the mask that the PPM was sent last. Keep it
> + * even if the command fails, as the PPM may have applied the mask anyway.
> + */
> +static int ucsi_update_notifications(struct ucsi *ucsi, u64 clear, u64 set)
> +{
> +	u64 command;
> +	u32 cci;
> +	int ret;
> +
> +	mutex_lock(&ucsi->ppm_lock);
> +
> +	if (ucsi->unregistering) {
> +		ret = -ESHUTDOWN;
> +		goto out_unlock;
> +	}
> +
> +	ucsi->ntfy = (ucsi->ntfy & ~clear) | set;
> +	command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
> +	ret = ucsi_run_command(ucsi, command, &cci, NULL, 0, NULL, 0, false);
> +	if (cci & UCSI_CCI_ERROR)
> +		ret = ucsi_read_error(ucsi, 0);
> +
> +	trace_ucsi_run_command(command, ret);
> +
> +out_unlock:
> +	mutex_unlock(&ucsi->ppm_lock);
> +	return ret;
> +}
> +
>  int ucsi_write_message_out_command(struct ucsi *ucsi, u64 command,
>  				   void *data, size_t size, void *msg_out,
>  				   size_t msg_out_size)
> @@ -1254,6 +1285,68 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
>  	}
>  }
>  
> +/*
> + * Some PPMs keep signaling a Negotiated Power Level Change although the power
> + * contract stays the same, and signal it again as soon as it has been
> + * acknowledged, so the PPM and the driver keep each other busy for as long as
> + * the partner stays connected. Disable the notification once the PPM has
> + * signaled it this many times in a row, each within a second of the previous
> + * one, without a new contract, and enable it again when the partner is gone.
> + * The notification can only be disabled for all connectors at once. A change
> + * of the source PDOs can come with an unchanged RDO, so it doesn't count.
> + */
> +#define UCSI_PWR_LEVEL_UNCHANGED_MAX	10
> +
> +static void ucsi_pwr_level_change(struct ucsi_connector *con)
> +{
> +	struct ucsi *ucsi = con->ucsi;
> +
> +	if (UCSI_CONSTAT(con, PWR_OPMODE) != UCSI_CONSTAT_PWR_OPMODE_PD ||
> +	    UCSI_CONSTAT(con, RDO) != con->rdo ||
> +	    (UCSI_CONSTAT(con, CHANGE) & UCSI_CONSTAT_PDOS_CHANGE)) {
> +		con->pwr_level_unchanged = 0;
> +		ucsi_pwr_opmode_change(con);
> +		return;
> +	}
> +
> +	if (time_after(jiffies, con->pwr_level_change_time + HZ))
> +		con->pwr_level_unchanged = 0;
> +	con->pwr_level_change_time = jiffies;
> +
> +	if (++con->pwr_level_unchanged < UCSI_PWR_LEVEL_UNCHANGED_MAX) {
> +		/* Signaled before the notification was disabled, or despite it */
> +		if (!con->pwr_level_ntfy_disabled)
> +			ucsi_pwr_opmode_change(con);
> +		return;
> +	}
> +
> +	/*
> +	 * Enable it again when the partner is gone even if the command fails,
> +	 * as the PPM may have disabled it anyway.
> +	 */
> +	if (!con->pwr_level_ntfy_disabled) {
> +		con->pwr_level_ntfy_disabled = true;
> +		dev_warn(ucsi->dev,
> +			 "con%d: Firmware bug: power level changes without a new contract, disabling the notification\n",
> +			 con->num);
> +	}
> +
> +	if (ucsi_update_notifications(ucsi, UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE, 0) >= 0)
> +		con->pwr_level_unchanged = 0;
> +}
> +
> +/* The partner that the notification was disabled for is gone */
> +static void ucsi_pwr_level_ntfy_restore(struct ucsi_connector *con)
> +{
> +	if (!con->pwr_level_ntfy_disabled)
> +		return;
> +
> +	if (ucsi_update_notifications(con->ucsi, 0, UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE) < 0)
> +		return;
> +
> +	con->pwr_level_ntfy_disabled = false;
> +}
> +
>  static int ucsi_register_partner(struct ucsi_connector *con)
>  {
>  	u8 pwr_opmode = UCSI_CONSTAT(con, PWR_OPMODE);
> @@ -1324,6 +1417,8 @@ static void ucsi_unregister_partner(struct ucsi_connector *con)
>  	typec_unregister_partner(con->partner);
>  	memset(&con->partner_identity, 0, sizeof(con->partner_identity));
>  	con->partner = NULL;
> +	con->rdo = 0;
> +	con->pwr_level_unchanged = 0;
>  }
>  
>  static void ucsi_partner_change(struct ucsi_connector *con)
> @@ -1400,6 +1495,7 @@ static int ucsi_check_connection(struct ucsi_connector *con)
>  		ucsi_partner_change(con);
>  		ucsi_port_psy_changed(con);
>  		ucsi_unregister_partner(con);
> +		ucsi_pwr_level_ntfy_restore(con);
>  	}
>  
>  	return 0;
> @@ -1511,11 +1607,16 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>  			}
>  		} else {
>  			ucsi_unregister_partner(con);
> +			ucsi_pwr_level_ntfy_restore(con);
>  		}
>  	}
>  
> -	if (change & (UCSI_CONSTAT_POWER_OPMODE_CHANGE | UCSI_CONSTAT_POWER_LEVEL_CHANGE))
> +	if (change & UCSI_CONSTAT_POWER_OPMODE_CHANGE) {
> +		con->pwr_level_unchanged = 0;
>  		ucsi_pwr_opmode_change(con);
> +	} else if (change & UCSI_CONSTAT_POWER_LEVEL_CHANGE) {
> +		ucsi_pwr_level_change(con);
> +	}
>  
>  	if (con->partner && (change & UCSI_CONSTAT_PARTNER_CHANGE)) {
>  		ucsi_partner_change(con);
> @@ -1685,13 +1786,10 @@ static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
>  
>  	ret = ucsi_send_command(con->ucsi, command, NULL, 0);
>  	if (ret == -ETIMEDOUT) {
> -		u64 c;
> -
>  		/* PPM most likely stopped responding. Resetting everything. */
>  		ucsi_reset_ppm(con->ucsi);
>  
> -		c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy;
> -		ucsi_send_command(con->ucsi, c, NULL, 0);
> +		ucsi_update_notifications(con->ucsi, 0, 0);
>  
>  		ucsi_reset_connector(con, true);
>  	}
> @@ -2171,12 +2269,10 @@ static void ucsi_resume_work(struct work_struct *work)
>  {
>  	struct ucsi *ucsi = container_of(work, struct ucsi, resume_work);
>  	struct ucsi_connector *con;
> -	u64 command;
>  	int ret;
>  
>  	/* Restore UCSI notification enable mask after system resume */
> -	command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
> -	ret = ucsi_send_command(ucsi, command, NULL, 0);
> +	ret = ucsi_update_notifications(ucsi, 0, 0);
>  	if (ret < 0) {
>  		dev_err(ucsi->dev, "failed to re-enable notifications (%d)\n", ret);
>  		return;
> @@ -2347,6 +2443,8 @@ int ucsi_register(struct ucsi *ucsi)
>  	if (!ucsi->version)
>  		return -ENODEV;
>  
> +	ucsi->unregistering = false;
> +
>  	/*
>  	 * Version format is JJ.M.N (JJ = Major version, M = Minor version,
>  	 * N = sub-minor version).
> @@ -2380,7 +2478,10 @@ void ucsi_unregister(struct ucsi *ucsi)
>  
>  	ucsi_debugfs_unregister(ucsi);
>  
> -	/* Disable notifications */
> +	/* Disable notifications, and keep connector work from enabling them */
> +	mutex_lock(&ucsi->ppm_lock);
> +	ucsi->unregistering = true;
> +	mutex_unlock(&ucsi->ppm_lock);
>  	ucsi->ops->async_control(ucsi, cmd);
>  
>  	if (!ucsi->connector)
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index dc594388d..137a83357 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -496,6 +496,8 @@ struct ucsi {
>  
>  	/* The latest "Notification Enable" bits (SET_NOTIFICATION_ENABLE) */
>  	u64 ntfy;
> +	/* Set by ucsi_unregister(), protected by ppm_lock */
> +	bool unregistering;
>  
>  	/* PPM communication flags */
>  	unsigned long flags;
> @@ -554,6 +556,9 @@ struct ucsi_connector {
>  	struct power_supply *psy;
>  	struct power_supply_desc psy_desc;
>  	u32 rdo;
> +	unsigned long pwr_level_change_time;
> +	unsigned int pwr_level_unchanged;
> +	bool pwr_level_ntfy_disabled;
>  	u32 src_pdos[PDO_MAX_OBJECTS];
>  	int num_pdos;
>  
> 
> base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
> -- 
> 2.54.0

-- 
heikki

Re: [PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[David Vornholt]

Hi Heikki,

On Mon, Oct 26, 2026 at 10:58:04AM +0100, Heikki Krogerus wrote:
> On Sat, Oct 10, 2026 at 08:39:17PM +0200, David Vornholt wrote:
> > On a Lenovo ThinkPad X1 Carbon Gen 13 (BIOS N4BET78W 1.48, EC N4BHT61W
> > 1.41, UCSI 1.0) with an HP E273m monitor connected, the PPM keeps
> > signaling a Negotiated Power Level Change although the power contract
> > stays the same, and signals it again right after it has been
> > acknowledged:
> >
> >   ucsi_connector_change: port1 status: change=0040, opmode=3,
> >     connected=1, sourcing=0, partner_flags=2, partner_type=4,
> >     request_data_obj=1304b12c, BC status=2
>
> Okay, so the PPM really should not report the power level changes
> since there are not power level changes, right? Have you reported this
> to Lenovo?

Right. The connector status is the same every time, with the same RDO,
and the Negotiated Power Level Change bit is the only one set in the
change field.

I haven't reported it to Lenovo yet, so I've added Mark to Cc. Mark,
this is a ThinkPad X1 Carbon Gen 13 (21NT) with BIOS N4BET78W (1.48)
and EC 1.41, which is still the latest firmware on LVFS. With an HP
E273m connected over USB-C, the EC keeps signaling a Negotiated Power
Level Change for an unchanged contract, about seven times a second,
and GPE 0x6E fires about 680 times a second. Could you pass this on to
the firmware team? The details are in the patch:
https://lore.kernel.org/r/20261010183917.146821-1-dv@etik.com

> This problem may only affect your system, so we really have to isolate
> it somehow. I'm almost certain that like this there will be problems
> on some other systems.

OK. How about a quirk for this machine only? ucsi_acpi would match it
by DMI (LENOVO, "ThinkPad X1 Carbon Gen 13") and set a new quirk flag,
and ucsi_get_supported_notifications() would then leave
UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE out of the mask. That replaces all of
the detection logic in v2 and changes nothing on other systems.

I've tested the effect of that on this machine, with a test switch that
kept the notification disabled from the start. With the monitor
connected, the PPM signaled no connector changes at all, and GPE 0x6E
dropped to about 5/s, from about 680/s with the stock driver. The cost
is that on this machine the driver won't notice a real power level
change until the PPM signals some other connector change. The EC
handles charging on its own.

> Something that I did not quite understand from the above is, if you
> revert commit a0d4618788f2, do you still have a problem (I understand
> that you still get the repeated notifications, but do they cause other
> problems then)?

Yes, though only about half as bad. I didn't test a plain revert, but
my first attempt did the same for these notifications: it skipped
ucsi_pwr_opmode_change() when the RDO hadn't changed. That's where the
"about 360 a second" in the commit message comes from. The PPM still
signaled the next change right after each ACK, so the driver read the
connector status and acknowledged it about nine times a second, back to
back. At about 50 ms of EC transactions per command, the connector work
and the EC were still busy almost all the time. Only disabling the
notification stopped that.

I didn't measure CPU time or power draw with that version. If numbers
for a plain revert would help, I can get them.

If the quirk is OK with you, I'll send a v3 with it.

Thanks,
David

Re: [PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[Mark Pearson]

On Mon, Oct 26, 2026, at 2:09 PM, David Vornholt wrote:
> Hi Heikki,
>
> On Mon, Oct 26, 2026 at 10:58:04AM +0100, Heikki Krogerus wrote:
>> On Sat, Oct 10, 2026 at 08:39:17PM +0200, David Vornholt wrote:
>> > On a Lenovo ThinkPad X1 Carbon Gen 13 (BIOS N4BET78W 1.48, EC N4BHT61W
>> > 1.41, UCSI 1.0) with an HP E273m monitor connected, the PPM keeps
>> > signaling a Negotiated Power Level Change although the power contract
>> > stays the same, and signals it again right after it has been
>> > acknowledged:
>> >
>> >   ucsi_connector_change: port1 status: change=0040, opmode=3,
>> >     connected=1, sourcing=0, partner_flags=2, partner_type=4,
>> >     request_data_obj=1304b12c, BC status=2
>>
>> Okay, so the PPM really should not report the power level changes
>> since there are not power level changes, right? Have you reported this
>> to Lenovo?
>
> Right. The connector status is the same every time, with the same RDO,
> and the Negotiated Power Level Change bit is the only one set in the
> change field.
>
> I haven't reported it to Lenovo yet, so I've added Mark to Cc. Mark,
> this is a ThinkPad X1 Carbon Gen 13 (21NT) with BIOS N4BET78W (1.48)
> and EC 1.41, which is still the latest firmware on LVFS. With an HP
> E273m connected over USB-C, the EC keeps signaling a Negotiated Power
> Level Change for an unchanged contract, about seven times a second,
> and GPE 0x6E fires about 680 times a second. Could you pass this on to
> the firmware team? The details are in the patch:
> https://lore.kernel.org/r/20261010183917.146821-1-dv@etik.com
>

I need to read through the patch properly - but the X1 Carbon is quite popular and used by lots of people. Have we been able to rule out it being the E273m on another machine?

I've created internal ticket LO-4728 so we can see if we can reproduce and get input from the FW team on why so many notifications would be generated - generated so many seems quite wrong.

>> This problem may only affect your system, so we really have to isolate
>> it somehow. I'm almost certain that like this there will be problems
>> on some other systems.
>
> OK. How about a quirk for this machine only? ucsi_acpi would match it
> by DMI (LENOVO, "ThinkPad X1 Carbon Gen 13") and set a new quirk flag,
> and ucsi_get_supported_notifications() would then leave
> UCSI_ENABLE_NTFY_PWR_LEVEL_CHANGE out of the mask. That replaces all of
> the detection logic in v2 and changes nothing on other systems.
>

I've only skim read the proposed patch, but I'm also hesitant to say this is X1 C13 specific.

As anote, if you do end doing a quirk then that field is bad as there are two different variants of the X1 Carbon 13 - ARL and LNL. Better to use DMI checks on the BOARD_NAME - there are two per platform, and I believe you're on Lunarlake so would be 21NS and 21NT.

Mark

Re: [PATCH v2] usb: typec: ucsi: Stop a power level change notification storm

[David Vornholt]

Hi Mark,

On Mon, Oct 26, 2026 at 04:01:32PM -0400, Mark Pearson wrote:
> I need to read through the patch properly - but the X1 Carbon is
> quite popular and used by lots of people. Have we been able to rule
> out it being the E273m on another machine?

No, I haven't tried the E273m on another machine.

On this X1, though, I've only seen it with the E273m. The X1 has been
running v2 since the 10th, and up to the 16th its warning fired 14
times, every time with the E273m connected, on either port. The
monitor hasn't been connected since the 16th, and the warning hasn't
fired since, although the X1 has been charging over USB-C on most
days.

Right now the same port has a 20 V PD contract (the source offers up
to 20 V/5 A and PPS) with the notification enabled. The warning
hasn't fired, and GPE 0x6E stays at a few per second.

So it seems to take this EC and the E273m together. From the UCSI
side I can't tell what the EC is reacting to.

> I've created internal ticket LO-4728 so we can see if we can
> reproduce and get input from the FW team on why so many
> notifications would be generated - generated so many seems quite
> wrong.

Thank you. If it would help the FW team, I can capture the ucsi trace
events or an acpidump with the monitor connected.

> I've only skim read the proposed patch, but I'm also hesitant to say
> this is X1 C13 specific.

Agreed. This X1 is the only machine I've tested. If LO-4728 shows
that other platforms have the same problem, their board names could
go into the quirk too.

> As anote, if you do end doing a quirk then that field is bad as
> there are two different variants of the X1 Carbon 13 - ARL and LNL.
> Better to use DMI checks on the BOARD_NAME - there are two per
> platform, and I believe you're on Lunarlake so would be 21NS and
> 21NT.

Thanks. Yes, the board name here is 21NTS4EE00. If we go with the
quirk, v3 will match "21NS" and "21NT" on DMI_BOARD_NAME.

Thanks,
David