On Wed, 27 Mar 2024 at 14:39, Diogo Ivo <diogo.ivo@xxxxxxxxxxxxxxxxxx> wrote: > > The UCSI specification mentions that when the OPM is notified by the > PPM of an asynchronous event it should first send all the commands it > needs to get the details of the event and only after acknowledge it by > sending ACK_CC_CI with the Connector Change bit set, while the current > code sends this ack immediately after scheduling all the partner_tasks. > Move this ACK_CC_CI command to the end of all partner_tasks. > > This fixes a problem with some LG Gram laptops where the PPM sometimes > notifies the OPM with the Connector Change Indicator field set in the > CCI after an ACK_CC_CI command is sent, causing the UCSI stack to check > the connector status indefinitely since the EVENT_PENDING bit is already > cleared. This causes an interrupt storm and an artificial high load on > these platforms. > > It would also be interesting to see if we could take this approach and > implement the discussion in [1] regarding sending an ACK_CC_CI command > that acks both the command completion and the connector change. > > Signed-off-by: Diogo Ivo <diogo.ivo@xxxxxxxxxxxxxxxxxx> > > [1]: https://lore.kernel.org/all/20240320073927.1641788-1-lk@xxxxxxx/ We had similar issue, see https://lore.kernel.org/linux-arm-msm/20240313-qcom-ucsi-fixes-v1-1-74d90cb48a00@xxxxxxxxxx/ > --- > drivers/usb/typec/ucsi/ucsi.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index 0c8f3b3a99d6..b8b39e43aba8 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -69,6 +69,23 @@ static int ucsi_acknowledge_connector_change(struct ucsi *ucsi) > return ucsi->ops->sync_write(ucsi, UCSI_CONTROL, &ctrl, sizeof(ctrl)); > } > > +static void ucsi_handle_ack_connector_change(struct ucsi_connector *con) > +{ > + struct ucsi *ucsi = con->ucsi; > + int ret; > + > + if (list_empty(&con->partner_tasks)) { > + mutex_lock(&ucsi->ppm_lock); > + ret = ucsi_acknowledge_connector_change(ucsi); > + mutex_unlock(&ucsi->ppm_lock); > + > + if (ret) > + dev_err(ucsi->dev, "%s: ACK failed (%d)", __func__, ret); > + > + clear_bit(EVENT_PENDING, &ucsi->flags); > + } > +} > + > static int ucsi_exec_command(struct ucsi *ucsi, u64 command); > > static int ucsi_read_error(struct ucsi *ucsi) > @@ -222,6 +239,7 @@ static void ucsi_poll_worker(struct work_struct *work) > list_del(&uwork->node); > mutex_unlock(&con->lock); > kfree(uwork); > + ucsi_handle_ack_connector_change(con); > return; > } > > @@ -232,6 +250,7 @@ static void ucsi_poll_worker(struct work_struct *work) > } else { > list_del(&uwork->node); > kfree(uwork); > + ucsi_handle_ack_connector_change(con); > } > > mutex_unlock(&con->lock); > @@ -1215,13 +1234,7 @@ static void ucsi_handle_connector_change(struct work_struct *work) > if (con->status.change & UCSI_CONSTAT_CAM_CHANGE) > ucsi_partner_task(con, ucsi_check_altmodes, 1, 0); > > - clear_bit(EVENT_PENDING, &con->ucsi->flags); > - > - mutex_lock(&ucsi->ppm_lock); > - ret = ucsi_acknowledge_connector_change(ucsi); > - mutex_unlock(&ucsi->ppm_lock); > - if (ret) > - dev_err(ucsi->dev, "%s: ACK failed (%d)", __func__, ret); > + ucsi_handle_ack_connector_change(con); > > out_unlock: > mutex_unlock(&con->lock); > -- > 2.44.0 > -- With best wishes Dmitry
