| Message ID | 20180213190404.25026-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
Hey Bjorn, Minor issue with the glink patch. Thanks, Chris On 2/13/2018 11:04 AM, Bjorn Andersson wrote: [..] > @@ -288,15 +288,14 @@ static int qcom_glink_tx(struct qcom_glink *glink, > const void *data, size_t dlen, bool wait) > { > unsigned int tlen = hlen + dlen; > + unsigned long flags; > int ret; > > /* Reject packets that are too big */ > if (tlen >= glink->tx_pipe->length) > return -EINVAL; > > - ret = mutex_lock_interruptible(&glink->tx_lock); > - if (ret) > - return ret; > + spin_lock_irqsave(&glink->tx_lock, flags); > qcom_glink_tx will return an uninitialized ret value with removal of mutex_lock_interruptible.
On 14/02/18 03:04, Bjorn Andersson wrote: > Switch the tx_lock to a spinlock we allow clients to use rpmsg_trysend() > from atomic context. > > In order to allow clients to sleep while waiting for space in the FIFO > we release the lock temporarily around the delay; which should be > replaced by sending a READ_NOTIF and waiting for the remote to signal > us that space has been made available. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- with ret set to 0 in qcom_glink_tx() Tested-by: Srinivas kandagatla <srinivas.kandagatla@linaro.org> > drivers/rpmsg/qcom_glink_native.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c > index e0f31ed096a5..6e950e9afa82 100644 > --- a/drivers/rpmsg/qcom_glink_native.c > +++ b/drivers/rpmsg/qcom_glink_native.c > @@ -113,7 +113,7 @@ struct qcom_glink { > spinlock_t rx_lock; > struct list_head rx_queue; > > - struct mutex tx_lock; > + spinlock_t tx_lock; > > spinlock_t idr_lock; > struct idr lcids; > @@ -288,15 +288,14 @@ static int qcom_glink_tx(struct qcom_glink *glink, > const void *data, size_t dlen, bool wait) > { > unsigned int tlen = hlen + dlen; > + unsigned long flags; > int ret; > > /* Reject packets that are too big */ > if (tlen >= glink->tx_pipe->length) > return -EINVAL; > > - ret = mutex_lock_interruptible(&glink->tx_lock); > - if (ret) > - return ret; > + spin_lock_irqsave(&glink->tx_lock, flags); > > while (qcom_glink_tx_avail(glink) < tlen) { > if (!wait) { > @@ -304,7 +303,12 @@ static int qcom_glink_tx(struct qcom_glink *glink, > goto out; > } > > + /* Wait without holding the tx_lock */ > + spin_unlock_irqrestore(&glink->tx_lock, flags); > + > usleep_range(10000, 15000); > + > + spin_lock_irqsave(&glink->tx_lock, flags); > } > > qcom_glink_tx_write(glink, hdr, hlen, data, dlen); > @@ -313,7 +317,7 @@ static int qcom_glink_tx(struct qcom_glink *glink, > mbox_client_txdone(glink->mbox_chan, 0); > > out: > - mutex_unlock(&glink->tx_lock); > + spin_unlock_irqrestore(&glink->tx_lock, flags); > > return ret; > } > @@ -1567,7 +1571,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev, > glink->features = features; > glink->intentless = intentless; > > - mutex_init(&glink->tx_lock); > + spin_lock_init(&glink->tx_lock); > spin_lock_init(&glink->rx_lock); > INIT_LIST_HEAD(&glink->rx_queue); > INIT_WORK(&glink->rx_work, qcom_glink_work); > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index e0f31ed096a5..6e950e9afa82 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -113,7 +113,7 @@ struct qcom_glink { spinlock_t rx_lock; struct list_head rx_queue; - struct mutex tx_lock; + spinlock_t tx_lock; spinlock_t idr_lock; struct idr lcids; @@ -288,15 +288,14 @@ static int qcom_glink_tx(struct qcom_glink *glink, const void *data, size_t dlen, bool wait) { unsigned int tlen = hlen + dlen; + unsigned long flags; int ret; /* Reject packets that are too big */ if (tlen >= glink->tx_pipe->length) return -EINVAL; - ret = mutex_lock_interruptible(&glink->tx_lock); - if (ret) - return ret; + spin_lock_irqsave(&glink->tx_lock, flags); while (qcom_glink_tx_avail(glink) < tlen) { if (!wait) { @@ -304,7 +303,12 @@ static int qcom_glink_tx(struct qcom_glink *glink, goto out; } + /* Wait without holding the tx_lock */ + spin_unlock_irqrestore(&glink->tx_lock, flags); + usleep_range(10000, 15000); + + spin_lock_irqsave(&glink->tx_lock, flags); } qcom_glink_tx_write(glink, hdr, hlen, data, dlen); @@ -313,7 +317,7 @@ static int qcom_glink_tx(struct qcom_glink *glink, mbox_client_txdone(glink->mbox_chan, 0); out: - mutex_unlock(&glink->tx_lock); + spin_unlock_irqrestore(&glink->tx_lock, flags); return ret; } @@ -1567,7 +1571,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev, glink->features = features; glink->intentless = intentless; - mutex_init(&glink->tx_lock); + spin_lock_init(&glink->tx_lock); spin_lock_init(&glink->rx_lock); INIT_LIST_HEAD(&glink->rx_queue); INIT_WORK(&glink->rx_work, qcom_glink_work);
Switch the tx_lock to a spinlock we allow clients to use rpmsg_trysend() from atomic context. In order to allow clients to sleep while waiting for space in the FIFO we release the lock temporarily around the delay; which should be replaced by sending a READ_NOTIF and waiting for the remote to signal us that space has been made available. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/rpmsg/qcom_glink_native.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)