[PATCH 0/2] usb: typec: ucsi: Additional checks for power role change handling

[PATCH 0/2] usb: typec: ucsi: Additional checks for power role change handling

[Myrrh Periwinkle]

Fixes a regression preventing system suspend on some Chromebooks.

Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>
---
Myrrh Periwinkle (2):
      usb: typec: ucsi: Check if power role change actually happened before handling
      usb: typec: ucsi: Don't update power_supply on power role change if not connected

 drivers/usb/typec/ucsi/ucsi.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
---
base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf
change-id: 20260519-ucsi-fix-2-1330c1695d1e

Best regards,
-- 
Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>

[PATCH 1/2] usb: typec: ucsi: Check if power role change actually happened before handling

[Myrrh Periwinkle]

The CrOS EC may send a connector status change event with the power
direction changed flag set even if the power direction hasn't actually
changed after initiating a SET_PDR command internally [1]. In practice
this happens on every system suspend due to other changes performed by
the EC [2][3][4], causing suspend to fail.

Fix this by checking if the power role change actually happened before
handling it.

[1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=1689;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
[2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=3923;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
[3]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=5094;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
[4]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=2229;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794

Cc: stable@vger.kernel.org
Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>
---
 drivers/usb/typec/ucsi/ucsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 5b7ad9e99cb9..e19b656609e4 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1277,7 +1277,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 						  work);
 	struct ucsi *ucsi = con->ucsi;
 	u8 curr_scale, volt_scale;
-	enum typec_role role;
+	enum typec_role role, prev_role;
 	u16 change;
 	int ret;
 	u32 val;
@@ -1288,6 +1288,8 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 		dev_err_once(ucsi->dev, "%s entered without EVENT_PENDING\n",
 			     __func__);
 
+	prev_role = UCSI_CONSTAT(con, PWR_DIR);
+
 	ret = ucsi_get_connector_status(con, true);
 	if (ret) {
 		dev_err(ucsi->dev, "%s: GET_CONNECTOR_STATUS failed (%d)\n",
@@ -1304,7 +1306,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 	change = UCSI_CONSTAT(con, CHANGE);
 	role = UCSI_CONSTAT(con, PWR_DIR);
 
-	if (change & UCSI_CONSTAT_POWER_DIR_CHANGE) {
+	if ((change & UCSI_CONSTAT_POWER_DIR_CHANGE) && role != prev_role) {
 		typec_set_pwr_role(con->port, role);
 		ucsi_port_psy_changed(con);
 

-- 
2.54.0

[PATCH 2/2] usb: typec: ucsi: Don't update power_supply on power role change if not connected

[Myrrh Periwinkle]

We only need to update the power_supply on power role change if the port
is connected, because otherwise the online status should be the same for
both cases.

Cc: stable@vger.kernel.org
Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>
---
 drivers/usb/typec/ucsi/ucsi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index e19b656609e4..c59c4d8ee076 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1308,7 +1308,12 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 
 	if ((change & UCSI_CONSTAT_POWER_DIR_CHANGE) && role != prev_role) {
 		typec_set_pwr_role(con->port, role);
-		ucsi_port_psy_changed(con);
+
+		/* Some power_supply properties vary depending on the power direction when
+		 * connected
+		 */
+		if (UCSI_CONSTAT(con, CONNECTED))
+			ucsi_port_psy_changed(con);
 
 		/* Complete pending power role swap */
 		if (!completion_done(&con->complete))

-- 
2.54.0

Re: [PATCH 1/2] usb: typec: ucsi: Check if power role change actually happened before handling

[Heikki Krogerus]

On Tue, May 19, 2026 at 06:41:39PM +0700, Myrrh Periwinkle wrote:
> The CrOS EC may send a connector status change event with the power
> direction changed flag set even if the power direction hasn't actually
> changed after initiating a SET_PDR command internally [1]. In practice
> this happens on every system suspend due to other changes performed by
> the EC [2][3][4], causing suspend to fail.
> 
> Fix this by checking if the power role change actually happened before
> handling it.
> 
> [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=1689;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=3923;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [3]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=5094;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [4]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=2229;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> 
> Cc: stable@vger.kernel.org
> Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
> Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/ucsi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 5b7ad9e99cb9..e19b656609e4 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1277,7 +1277,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>  						  work);
>  	struct ucsi *ucsi = con->ucsi;
>  	u8 curr_scale, volt_scale;
> -	enum typec_role role;
> +	enum typec_role role, prev_role;
>  	u16 change;
>  	int ret;
>  	u32 val;
> @@ -1288,6 +1288,8 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>  		dev_err_once(ucsi->dev, "%s entered without EVENT_PENDING\n",
>  			     __func__);
>  
> +	prev_role = UCSI_CONSTAT(con, PWR_DIR);
> +
>  	ret = ucsi_get_connector_status(con, true);
>  	if (ret) {
>  		dev_err(ucsi->dev, "%s: GET_CONNECTOR_STATUS failed (%d)\n",
> @@ -1304,7 +1306,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>  	change = UCSI_CONSTAT(con, CHANGE);
>  	role = UCSI_CONSTAT(con, PWR_DIR);
>  
> -	if (change & UCSI_CONSTAT_POWER_DIR_CHANGE) {
> +	if ((change & UCSI_CONSTAT_POWER_DIR_CHANGE) && role != prev_role) {
>  		typec_set_pwr_role(con->port, role);
>  		ucsi_port_psy_changed(con);
>  
> 
> -- 
> 2.54.0

-- 
heikki

Re: [PATCH 1/2] usb: typec: ucsi: Check if power role change actually happened before handling

[Heikki Krogerus]

On Tue, May 19, 2026 at 05:14:15PM +0300, Heikki Krogerus wrote:
> On Tue, May 19, 2026 at 06:41:39PM +0700, Myrrh Periwinkle wrote:
> > The CrOS EC may send a connector status change event with the power
> > direction changed flag set even if the power direction hasn't actually
> > changed after initiating a SET_PDR command internally [1]. In practice
> > this happens on every system suspend due to other changes performed by
> > the EC [2][3][4], causing suspend to fail.
> > 
> > Fix this by checking if the power role change actually happened before
> > handling it.
> > 
> > [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=1689;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> > [2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=3923;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> > [3]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=5094;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> > [4]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=2229;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
> > Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>
> 
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Hold on. Shouldn't this actually be fixed in that EC code?

thanks,

-- 
heikki

Re: [PATCH 1/2] usb: typec: ucsi: Check if power role change actually happened before handling

[Myrrh Periwinkle]

Hello,

On 5/19/26 9:22 PM, Heikki Krogerus wrote:
> On Tue, May 19, 2026 at 05:14:15PM +0300, Heikki Krogerus wrote:
>> On Tue, May 19, 2026 at 06:41:39PM +0700, Myrrh Periwinkle wrote:
>>> The CrOS EC may send a connector status change event with the power
>>> direction changed flag set even if the power direction hasn't actually
>>> changed after initiating a SET_PDR command internally [1]. In practice
>>> this happens on every system suspend due to other changes performed by
>>> the EC [2][3][4], causing suspend to fail.
>>>
>>> Fix this by checking if the power role change actually happened before
>>> handling it.
>>>
>>> [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=1689;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
>>> [2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=3923;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
>>> [3]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=5094;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
>>> [4]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=2229;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
>>> Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>
>> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Hold on. Shouldn't this actually be fixed in that EC code?
>
> thanks,
>
I'm not responsible for the CrOS EC and the only alternative proposed 
thus far is a full revert: 
https://lore.kernel.org/all/2026051935-flashing-relearn-8444@gregkh/

Re: [PATCH 1/2] usb: typec: ucsi: Check if power role change actually happened before handling

[Sergey Senozhatsky]

On (26/05/19 18:41), Myrrh Periwinkle wrote:
> The CrOS EC may send a connector status change event with the power
> direction changed flag set even if the power direction hasn't actually
> changed after initiating a SET_PDR command internally [1]. In practice
> this happens on every system suspend due to other changes performed by
> the EC [2][3][4], causing suspend to fail.
> 
> Fix this by checking if the power role change actually happened before
> handling it.
> 
> [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=1689;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=3923;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [3]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=5094;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> [4]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/subsys/pd_controller/pdc_power_mgmt.c;l=2229;drc=2d5a1cffce4e5ac8a39442cb3b764d2d5e1cf794
> 
> Cc: stable@vger.kernel.org
> Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
> Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>

Reported-and-tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Re: [PATCH 2/2] usb: typec: ucsi: Don't update power_supply on power role change if not connected

[Sergey Senozhatsky]

On (26/05/19 18:41), Myrrh Periwinkle wrote:
> We only need to update the power_supply on power role change if the port
> is connected, because otherwise the online status should be the same for
> both cases.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
> Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>

Reported-and-tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>