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@xxxxxxxxxxxxxxx
> Fixes: 7616f006db07 ("usb: typec: ucsi: Update power_supply on power role change")
> Signed-off-by: Myrrh Periwinkle <myrrhperiwinkle@xxxxxxxxxxx>

Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
>  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