usb: typec: ucsi: Move unregister out of atomic section

Commit '9329933699b3 ("soc: qcom: pmic_glink: Make client-lock
non-sleeping")' moved the pmic_glink client list under a spinlock, as it
is accessed by the rpmsg/glink callback, which in turn is invoked from
IRQ context.

This means that ucsi_unregister() is now called from atomic context,
which isn't feasible as it's expecting a sleepable context. An effort is
under way to get GLINK to invoke its callbacks in a sleepable context,
but until then lets schedule the unregistration.

A side effect of this is that ucsi_unregister() can now happen
after the remote processor, and thereby the communication link with it, is
gone. pmic_glink_send() is amended with a check to avoid the resulting NULL
pointer dereference.
This does however result in the user being informed about this error by
the following entry in the kernel log:

  ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: failed to send UCSI write request: -5

Fixes: 9329933699b3 ("soc: qcom: pmic_glink: Make client-lock non-sleeping")
Cc: stable@vger.kernel.org
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Tested-by: Amit Pundir <amit.pundir@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Link: https://lore.kernel.org/r/20240820-pmic-glink-v6-11-races-v3-2-eec53c750a04@quicinc.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>

diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 53b176d04fbdfeb60e07085db0c83b293ef58e35..b218460219b76face0b3b68d3a04a6947b47a96c 100644 (file)
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -112,8 +112,16 @@ EXPORT_SYMBOL_GPL(pmic_glink_client_register);
 int pmic_glink_send(struct pmic_glink_client *client, void *data, size_t len)
 {
        struct pmic_glink *pg = client->pg;
+       int ret;
 
-       return rpmsg_send(pg->ept, data, len);
+       mutex_lock(&pg->state_lock);
+       if (!pg->ept)
+               ret = -ECONNRESET;
+       else
+               ret = rpmsg_send(pg->ept, data, len);
+       mutex_unlock(&pg->state_lock);
+
+       return ret;
 }
 EXPORT_SYMBOL_GPL(pmic_glink_send);
 

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index f6f4fae40399344a3f582a11c4175a17850afa59..6aace19d595bcd0d6647215a384fc34932d7bb0e 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -68,6 +68,9 @@ struct pmic_glink_ucsi {
 
        struct work_struct notify_work;
        struct work_struct register_work;
+       spinlock_t state_lock;
+       bool ucsi_registered;
+       bool pd_running;
 
        u8 read_buf[UCSI_BUF_SIZE];
 };
@@ -244,8 +247,20 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
 static void pmic_glink_ucsi_register(struct work_struct *work)
 {
        struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, register_work);
+       unsigned long flags;
+       bool pd_running;
 
-       ucsi_register(ucsi->ucsi);
+       spin_lock_irqsave(&ucsi->state_lock, flags);
+       pd_running = ucsi->pd_running;
+       spin_unlock_irqrestore(&ucsi->state_lock, flags);
+
+       if (!ucsi->ucsi_registered && pd_running) {
+               ucsi_register(ucsi->ucsi);
+               ucsi->ucsi_registered = true;
+       } else if (ucsi->ucsi_registered && !pd_running) {
+               ucsi_unregister(ucsi->ucsi);
+               ucsi->ucsi_registered = false;
+       }
 }
 
 static void pmic_glink_ucsi_callback(const void *data, size_t len, void *priv)
@@ -269,11 +284,12 @@ static void pmic_glink_ucsi_callback(const void *data, size_t len, void *priv)
 static void pmic_glink_ucsi_pdr_notify(void *priv, int state)
 {
        struct pmic_glink_ucsi *ucsi = priv;
+       unsigned long flags;
 
-       if (state == SERVREG_SERVICE_STATE_UP)
-               schedule_work(&ucsi->register_work);
-       else if (state == SERVREG_SERVICE_STATE_DOWN)
-               ucsi_unregister(ucsi->ucsi);
+       spin_lock_irqsave(&ucsi->state_lock, flags);
+       ucsi->pd_running = (state == SERVREG_SERVICE_STATE_UP);
+       spin_unlock_irqrestore(&ucsi->state_lock, flags);
+       schedule_work(&ucsi->register_work);
 }
 
 static void pmic_glink_ucsi_destroy(void *data)
@@ -320,6 +336,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
        INIT_WORK(&ucsi->register_work, pmic_glink_ucsi_register);
        init_completion(&ucsi->read_ack);
        init_completion(&ucsi->write_ack);
+       spin_lock_init(&ucsi->state_lock);
        mutex_init(&ucsi->lock);
 
        ucsi->ucsi = ucsi_create(dev, &pmic_glink_ucsi_ops);