From a1fcc455899ace544d7130c6eace471d69d2f96a Mon Sep 17 00:00:00 2001 From: Vasyl Gomonovych Date: Tue, 28 Nov 2017 23:32:40 +0100 Subject: remoteproc: qcom: Use PTR_ERR_OR_ZERO() in glink prob Fix ptr_ret.cocci warnings: drivers/remoteproc/qcom_common.c:60:8-14: WARNING: PTR_ERR_OR_ZERO can be used Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR Generated by: scripts/coccinelle/api/ptr_ret.cocci Signed-off-by: Vasyl Gomonovych Signed-off-by: Bjorn Andersson --- drivers/remoteproc/qcom_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c index d487040b528b..e9b5f2a98c4d 100644 --- a/drivers/remoteproc/qcom_common.c +++ b/drivers/remoteproc/qcom_common.c @@ -57,7 +57,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev) glink->edge = qcom_glink_smem_register(glink->dev, glink->node); - return IS_ERR(glink->edge) ? PTR_ERR(glink->edge) : 0; + return PTR_ERR_OR_ZERO(glink->edge); } static void glink_subdev_remove(struct rproc_subdev *subdev) -- cgit From ed608eb0be578472ba1b23bca92f7dec61b24053 Mon Sep 17 00:00:00 2001 From: Pravin Shedge Date: Wed, 6 Dec 2017 22:32:41 +0530 Subject: drivers: rpmsg: remove duplicate includes These duplicate includes have been found with scripts/checkincludes.pl but they have been removed manually to avoid removing false positives. Signed-off-by: Pravin Shedge Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_glink_smem.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c index 5cdaa5f8fb61..057528e23d3a 100644 --- a/drivers/rpmsg/qcom_glink_smem.c +++ b/drivers/rpmsg/qcom_glink_smem.c @@ -29,8 +29,6 @@ #include #include -#include -#include #include #include "qcom_glink_native.h" -- cgit From 9d32497361ff89d2fc8306407de6f04b2bfb2836 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 14 Dec 2017 12:15:46 -0800 Subject: rpmsg: glink: smem: Ensure ordering during tx Ensure the ordering of the fifo write and the update of the write index, so that the index is not updated before the data has landed in the fifo. Acked-By: Chris Lew Reported-by: Arun Kumar Neelakantam Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_glink_smem.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c index 057528e23d3a..892f2b92a4d8 100644 --- a/drivers/rpmsg/qcom_glink_smem.c +++ b/drivers/rpmsg/qcom_glink_smem.c @@ -183,6 +183,9 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe, if (head >= pipe->native.length) head -= pipe->native.length; + /* Ensure ordering of fifo and head update */ + wmb(); + *pipe->head = cpu_to_le32(head); } -- cgit From 268105fbc0f82e1daa44b57112ef3fd81f69a174 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Tue, 12 Dec 2017 15:58:53 -0800 Subject: rpmsg: smd: Perform handshake during open Validate the the remote side is opening the channel that we've found by performing a handshake when opening the channel. Tested-by: Will Newton Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index b01774e9fac0..58dd44493420 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -200,6 +200,7 @@ struct qcom_smd_channel { char *name; enum smd_channel_state state; enum smd_channel_state remote_state; + wait_queue_head_t state_change_event; struct smd_channel_info_pair *info; struct smd_channel_info_word_pair *info_word; @@ -570,6 +571,8 @@ static bool qcom_smd_channel_intr(struct qcom_smd_channel *channel) if (remote_state != channel->remote_state) { channel->remote_state = remote_state; need_state_scan = true; + + wake_up_interruptible_all(&channel->state_change_event); } /* Indicate that we have seen any state change */ SET_RX_CHANNEL_FLAG(channel, fSTATE, 0); @@ -786,7 +789,9 @@ out: static int qcom_smd_channel_open(struct qcom_smd_channel *channel, rpmsg_rx_cb_t cb) { + struct qcom_smd_edge *edge = channel->edge; size_t bb_size; + int ret; /* * Packets are maximum 4k, but reduce if the fifo is smaller @@ -798,9 +803,33 @@ static int qcom_smd_channel_open(struct qcom_smd_channel *channel, qcom_smd_channel_set_callback(channel, cb); qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENING); + + /* Wait for remote to enter opening or opened */ + ret = wait_event_interruptible_timeout(channel->state_change_event, + channel->remote_state == SMD_CHANNEL_OPENING || + channel->remote_state == SMD_CHANNEL_OPENED, + HZ); + if (!ret) { + dev_err(&edge->dev, "remote side did not enter opening state\n"); + goto out_close_timeout; + } + qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENED); + /* Wait for remote to enter opened */ + ret = wait_event_interruptible_timeout(channel->state_change_event, + channel->remote_state == SMD_CHANNEL_OPENED, + HZ); + if (!ret) { + dev_err(&edge->dev, "remote side did not enter open state\n"); + goto out_close_timeout; + } + return 0; + +out_close_timeout: + qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSED); + return -ETIMEDOUT; } /* @@ -1055,6 +1084,7 @@ static struct qcom_smd_channel *qcom_smd_create_channel(struct qcom_smd_edge *ed mutex_init(&channel->tx_lock); spin_lock_init(&channel->recv_lock); init_waitqueue_head(&channel->fblockread_event); + init_waitqueue_head(&channel->state_change_event); info = qcom_smem_get(edge->remote_pid, smem_info_item, &info_size); if (IS_ERR(info)) { -- cgit From c12fc4519f607f83b6874a5388bb4df0759f687c Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Tue, 12 Dec 2017 15:58:54 -0800 Subject: rpmsg: smd: Create device for all channels Rather than selectively creating devices only for the channels that the remote have moved to "opening" state let's create devices for all channels found. The driver model will match drivers to the ones we care about and attempt to open these. The one case where this fails is if the user loads a firmware that lacks a particular channel of the previous firmware that was running, in which case we would find the old channel and attempt to probe it. The channel opening handshake will ensure this will result in a graceful failure. The result of this patch is that we will actively open the RPM channel even though it's left in a state other than "opening" after the boot loader's closing of the channel. Tested-by: Will Newton Reported-by: Jeremy McNicoll Reported-by: Will Newton Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index 58dd44493420..1beddea6f087 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -1225,11 +1225,6 @@ static void qcom_channel_state_worker(struct work_struct *work) if (channel->state != SMD_CHANNEL_CLOSED) continue; - remote_state = GET_RX_CHANNEL_INFO(channel, state); - if (remote_state != SMD_CHANNEL_OPENING && - remote_state != SMD_CHANNEL_OPENED) - continue; - if (channel->registered) continue; -- cgit From eb114f27fd3a6fd5005e07818043993bda67d8d1 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Tue, 12 Dec 2017 15:58:55 -0800 Subject: rpmsg: smd: Wake up all waiters It's possible to have multiple contexts waiting for new channel events and with an upcoming change it's possible to have multiple contexts waiting for a full FIFO. As such we need to wake them all up. Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index 1beddea6f087..0993e95bf0f5 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -579,7 +579,7 @@ static bool qcom_smd_channel_intr(struct qcom_smd_channel *channel) /* Signal waiting qcom_smd_send() about the interrupt */ if (!GET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR)) - wake_up_interruptible(&channel->fblockread_event); + wake_up_interruptible_all(&channel->fblockread_event); /* Don't consume any data until we've opened the channel */ if (channel->state != SMD_CHANNEL_OPENED) @@ -1191,7 +1191,7 @@ static void qcom_channel_scan_worker(struct work_struct *work) dev_dbg(&edge->dev, "new channel found: '%s'\n", channel->name); set_bit(i, edge->allocated[tbl]); - wake_up_interruptible(&edge->new_channel_event); + wake_up_interruptible_all(&edge->new_channel_event); } } -- cgit From b2c932e7991ca7e3995457463b72fc34e64477a0 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Tue, 12 Dec 2017 15:58:56 -0800 Subject: rpmsg: smd: Fail send on a closed channel Move the check for a closed channel out from the tx-full loop to fail any send request on a non-open channel. Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index 0993e95bf0f5..ed167ab52a68 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -743,17 +743,13 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, if (ret) return ret; - while (qcom_smd_get_tx_avail(channel) < tlen) { + while (qcom_smd_get_tx_avail(channel) < tlen && + channel->state == SMD_CHANNEL_OPENED) { if (!wait) { ret = -EAGAIN; goto out; } - if (channel->state != SMD_CHANNEL_OPENED) { - ret = -EPIPE; - goto out; - } - SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 0); ret = wait_event_interruptible(channel->fblockread_event, @@ -765,6 +761,12 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 1); } + /* Fail if the channel was closed */ + if (channel->state != SMD_CHANNEL_OPENED) { + ret = -EPIPE; + goto out; + } + SET_TX_CHANNEL_FLAG(channel, fTAIL, 0); qcom_smd_write_fifo(channel, hdr, sizeof(hdr)); -- cgit From 178f3f75bb4ef7a29bf5c175eb33794ac9ae9bce Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Tue, 12 Dec 2017 15:58:57 -0800 Subject: rpmsg: smd: Don't hold the tx lock during wait Holding the tx lock while waiting for tx-drain events from the remote side blocks try_send requests from failing quickly, so temporarily drop the tx lock while waiting. While this allows try_send to fail quickly it also could allow a subsequent send to succeed putting a smaller packet in the FIFO while we're waiting for room for our large packet. But as this lock is per channel we expect that clients with ordering concerns implements their own ordering mechanism. Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index ed167ab52a68..10870189c0c8 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -752,12 +752,19 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 0); + /* Wait without holding the tx_lock */ + mutex_unlock(&channel->tx_lock); + ret = wait_event_interruptible(channel->fblockread_event, qcom_smd_get_tx_avail(channel) >= tlen || channel->state != SMD_CHANNEL_OPENED); if (ret) goto out; + ret = mutex_lock_interruptible(&channel->tx_lock); + if (ret) + goto out; + SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 1); } -- cgit From fb416f69900773d5a6030c909114099f92d07ab9 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 2 Jan 2018 11:47:03 +0000 Subject: rpmsg: glink: Fix missing mutex_init() in qcom_glink_alloc_channel() qcom_glink_alloc_channel() allocates the mutex but not initialize it. Use mutex_init() on it to initialize it correctly. This is detected by Coccinelle semantic patch. Signed-off-by: Wei Yongjun Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_glink_native.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index 40d76d2a5eff..e0f31ed096a5 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -221,6 +221,7 @@ static struct glink_channel *qcom_glink_alloc_channel(struct qcom_glink *glink, /* Setup glink internal glink_channel data */ spin_lock_init(&channel->recv_lock); spin_lock_init(&channel->intent_lock); + mutex_init(&channel->intent_req_lock); channel->glink = glink; channel->name = kstrdup(name, GFP_KERNEL); -- cgit From c3388a075c8ac568f892c40bec919ba8ac4077f0 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 19 Jan 2018 16:22:36 +0300 Subject: rpmsg: smd: Fix double unlock in __qcom_smd_send() We're not holding the lock here, so we shouldn't unlock. Fixes: 178f3f75bb4e ("rpmsg: smd: Don't hold the tx lock during wait") Signed-off-by: Dan Carpenter [bjorn: renamed "out" label to further distinguish the two exit paths] Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_smd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index 10870189c0c8..e92fd0129658 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -747,7 +747,7 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, channel->state == SMD_CHANNEL_OPENED) { if (!wait) { ret = -EAGAIN; - goto out; + goto out_unlock; } SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 0); @@ -759,11 +759,11 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, qcom_smd_get_tx_avail(channel) >= tlen || channel->state != SMD_CHANNEL_OPENED); if (ret) - goto out; + return ret; ret = mutex_lock_interruptible(&channel->tx_lock); if (ret) - goto out; + return ret; SET_TX_CHANNEL_FLAG(channel, fBLOCKREADINTR, 1); } @@ -771,7 +771,7 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, /* Fail if the channel was closed */ if (channel->state != SMD_CHANNEL_OPENED) { ret = -EPIPE; - goto out; + goto out_unlock; } SET_TX_CHANNEL_FLAG(channel, fTAIL, 0); @@ -786,7 +786,7 @@ static int __qcom_smd_send(struct qcom_smd_channel *channel, const void *data, qcom_smd_signal_channel(channel); -out: +out_unlock: mutex_unlock(&channel->tx_lock); return ret; -- cgit