commit e613904fa419 causes suspend regression
Subsystems (2)
Hi,
We've identified 6.6.y stable commit e613904fa419 (usb: typec: ucsi:
Update power_supply on power role change) (commit 7616f006db07017 upstream) as a root-cause of suspend failures on some of our laptops. It seems that ucsi_port_psy_changed() causes:
[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2
which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Greg Kroah-Hartman
maintainerQuoted 7 lines · Sergey Senozhatsky
Hi,
We've identified 6.6.y stable commit
e613904fa419(usb: typec: ucsi:
Updatepower_supplyon power role change) (commit7616f006db07017upstream) as a root-cause of suspend failures on some of our laptops. It seems thatucsi_port_psy_changed()causes:
Quoted 6 lines
[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Ok, and what should we do? Is this issue also in 7.0.y?
thanks,
greg k-h
Quoted 5 lines · Greg Kroah-Hartman
Quoted 5 lines
We've identified 6.6.y stable commit
e613904fa419(usb: typec: ucsi:
Updatepower_supplyon power role change) (commit7616f006db07017upstream) as a root-cause of suspend failures on some of our laptops. It seems thatucsi_port_psy_changed()causes:
Quoted 8 lines
Quoted 6 lines
[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Ok, and what should we do?
I suppose what you usually do with stable regressions?
Quoted 1 line
Is this issue also in 7.0.y?
We don't have any boards on 7.0.y unfortunately. 6.6.y is the most recent we use, but I'd expect 7.0 to regress as well.
Greg Kroah-Hartman
maintainerQuoted 9 lines · Sergey Senozhatsky
Quoted 8 lines · Greg Kroah-Hartman
Quoted 8 lines
We've identified 6.6.y stable commit
e613904fa419(usb: typec: ucsi:
Updatepower_supplyon power role change) (commit7616f006db07017upstream) as a root-cause of suspend failures on some of our laptops. It seems thatucsi_port_psy_changed()causes:[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Ok, and what should we do?
I suppose what you usually do with stable regressions?
Get the reporter to fix them, great! :)
Quoted 4 lines
Quoted 1 line
Is this issue also in 7.0.y?
We don't have any boards on 7.0.y unfortunately. 6.6.y is the most recent we use, but I'd expect 7.0 to regress as well.
Wonderful, please send a revert for mainline and we will be glad to apply that and backport it properly to the needed stable kernels.
thanks,
greg k-h
(re-sending as text/plain as the previous one got rejected by the mailing list)
Hello,
Quoted 7 lines · Sergey Senozhatsky
Hi,
We've identified 6.6.y stable commit
e613904fa419(usb: typec: ucsi:
Updatepower_supplyon power role change) (commit7616f006db07017upstream) as a root-cause of suspend failures on some of our laptops. It seems thatucsi_port_psy_changed()causes:
Quoted 5 lines
[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Can you check if this patch series fixes your issue? https://lore.kernel.org/all/[email protected]/
Hi Myrrh,
Quoted 13 lines · Myrrh Periwinkle
(re-sending as text/plain as the previous one got rejected by the mailing list)
Hello,
Quoted 7 lines · Sergey Senozhatsky
Hi,
We've identified 6.6.y stable commit
e613904fa419(usb: typec: ucsi:
Updatepower_supplyon power role change) (commit7616f006db07017upstream) as a root-cause of suspend failures on some of our laptops. It seems thatucsi_port_psy_changed()causes:
Quoted 7 lines
Quoted 5 lines
[ 309.858915] PM: last active wakeup source: ucsi-source-psy-cros_ec_ucsi.3.auto2 [ 309.858917] PM: PM: Last active Wakeup Source: ucsi-source-psy-cros_ec_ucsi.3.auto2which prevent laptop from entering suspend. Reverting the commit in question fixes the issue.
Can you check if this patch series fixes your issue? https://lore.kernel.org/all/[email protected]/
Sure, thanks a ton for the patches! I backported them to 6.6.y
--- drivers/usb/typec/ucsi/ucsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/usb/typec/ucsi/ucsi.c +4 -2
@@ -1224,12 +1224,14 @@ static void ucsi_handle_connector_change(struct work_struct *work) struct ucsi_connector *con = container_of(work, struct ucsi_connector, work);struct ucsi *ucsi = con->ucsi; -enum typec_role role; +enum typec_role role, prev_role;u64 command;int ret;mutex_lock (&con->lock); + prev_role = !!(con->status.flags &UCSI_CONSTAT_PWR_DIR ); + command =UCSI_GET_CONNECTOR_STATUS |UCSI_CONNECTOR_NUMBER (con->num); ret = ucsi_send_command_common(ucsi, command, &con->status,
@@ -1248,7 +1250,7 @@ static void ucsi_handle_connector_change(struct work_struct *work) role = !!(con->status.flags &UCSI_CONSTAT_PWR_DIR ); -if (con->status.change &UCSI_CONSTAT_POWER_DIR_CHANGE ) { +if ((con->status.change &UCSI_CONSTAT_POWER_DIR_CHANGE ) && role != prev_role) {typec_set_pwr_role (con->port, role);ucsi_port_psy_changed (con);
Signature · 8 lines
2.54.0.631.ge1b05301d1-goog and --- drivers/usb/typec/ucsi/ucsi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi.c +6 -1
@@ -1252,7 +1252,12 @@ static void ucsi_handle_connector_change(struct work_struct *work)if ((con->status.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 (con->status.flags &UCSI_CONSTAT_CONNECTED ) +ucsi_port_psy_changed (con);/* Complete pending power role swap */ if (!completion_done (&con->complete))
Signature · 4 lines
2.54.0.631.ge1b05301d1-goog I started the tests, need prob around a day or so to verify.