* [PATCH V1 1/6] rpmsg: glink: fix destroy channel endpoint logic
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 14:47 ` Bjorn Andersson
2020-07-30 5:18 ` [PATCH V1 2/6] rpmsg: glink: Deny intent request if reusable intent fits Deepak Kumar Singh
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Konstantin Dorfman, Deepak Kumar Singh
From: Konstantin Dorfman <kdorfman@codeaurora.org>
When rpmsg client driver destroys last channel endpoint, remove rpmsg
device is triggered. In both cases (destroy endpoint and remove device)
a glink close command sent to the remote peer.
This change, when for removing rpmsg device endpoint already destroyed
will avoid sending second glink close command.
Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 1995f5b..2668c66 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1210,6 +1210,10 @@ static void qcom_glink_destroy_ept(struct rpmsg_endpoint *ept)
unsigned long flags;
spin_lock_irqsave(&channel->recv_lock, flags);
+ if (!channel->ept.cb) {
+ spin_unlock_irqrestore(&channel->recv_lock, flags);
+ return;
+ }
channel->ept.cb = NULL;
spin_unlock_irqrestore(&channel->recv_lock, flags);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V1 1/6] rpmsg: glink: fix destroy channel endpoint logic
2020-07-30 5:18 ` [PATCH V1 1/6] rpmsg: glink: fix destroy channel endpoint logic Deepak Kumar Singh
@ 2021-10-15 14:47 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 14:47 UTC (permalink / raw)
To: Deepak Kumar Singh
Cc: clew, mathieu.poirier, linux-arm-msm, linux-remoteproc,
linux-kernel, Konstantin Dorfman
On Thu 30 Jul 00:18 CDT 2020, Deepak Kumar Singh wrote:
> From: Konstantin Dorfman <kdorfman@codeaurora.org>
>
> When rpmsg client driver destroys last channel endpoint, remove rpmsg
> device is triggered. In both cases (destroy endpoint and remove device)
> a glink close command sent to the remote peer.
>
> This change, when for removing rpmsg device endpoint already destroyed
> will avoid sending second glink close command.
>
Should it really be considered valid to rpmsg_destroy_ept() the
rpmsg_device's primary endpoint?
Do you have a use case where this makes sense?
Also, I think this has a potential to hide a problems of clients doing a
"double free" on the ept.
Regards,
Bjorn
> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---
> drivers/rpmsg/qcom_glink_native.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 1995f5b..2668c66 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -1210,6 +1210,10 @@ static void qcom_glink_destroy_ept(struct rpmsg_endpoint *ept)
> unsigned long flags;
>
> spin_lock_irqsave(&channel->recv_lock, flags);
> + if (!channel->ept.cb) {
> + spin_unlock_irqrestore(&channel->recv_lock, flags);
> + return;
> + }
> channel->ept.cb = NULL;
> spin_unlock_irqrestore(&channel->recv_lock, flags);
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 2/6] rpmsg: glink: Deny intent request if reusable intent fits
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
2020-07-30 5:18 ` [PATCH V1 1/6] rpmsg: glink: fix destroy channel endpoint logic Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 15:14 ` Bjorn Andersson
2020-07-30 5:18 ` [PATCH V1 3/6] rpmsg: glink: Add TX_DATA_CONT command while sending Deepak Kumar Singh
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Deepak Kumar Singh
From: Chris Lew <clew@codeaurora.org>
In high traffic scenarios a remote may request extra intents to send
data faster. If the work thread that handles these intent requests is
starved of cpu time, then these requests can build up. Some remote
procs may not be able to handle this burst of built up intent requests.
In order to prevent intent build up, deny intent requests that can be
fulfilled by default intents that are reusable.
Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 2668c66..df3c608 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -734,9 +734,11 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink,
static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
u32 cid, size_t size)
{
- struct glink_core_rx_intent *intent;
+ struct glink_core_rx_intent *intent = NULL;
+ struct glink_core_rx_intent *tmp;
struct glink_channel *channel;
unsigned long flags;
+ int iid;
spin_lock_irqsave(&glink->idr_lock, flags);
channel = idr_find(&glink->rcids, cid);
@@ -747,6 +749,19 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
return;
}
+ spin_lock_irqsave(&channel->intent_lock, flags);
+ idr_for_each_entry(&channel->liids, tmp, iid) {
+ if (tmp->size >= size && tmp->reuse) {
+ intent = tmp;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(&channel->intent_lock, flags);
+ if (intent) {
+ qcom_glink_send_intent_req_ack(glink, channel, !!intent);
+ return;
+ }
+
intent = qcom_glink_alloc_intent(glink, channel, size, false);
if (intent)
qcom_glink_advertise_intent(glink, channel, intent);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V1 2/6] rpmsg: glink: Deny intent request if reusable intent fits
2020-07-30 5:18 ` [PATCH V1 2/6] rpmsg: glink: Deny intent request if reusable intent fits Deepak Kumar Singh
@ 2021-10-15 15:14 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 15:14 UTC (permalink / raw)
To: Deepak Kumar Singh
Cc: clew, mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel
On Thu 30 Jul 00:18 CDT 2020, Deepak Kumar Singh wrote:
> From: Chris Lew <clew@codeaurora.org>
>
> In high traffic scenarios a remote may request extra intents to send
> data faster. If the work thread that handles these intent requests is
> starved of cpu time, then these requests can build up. Some remote
> procs may not be able to handle this burst of built up intent requests.
>
> In order to prevent intent build up, deny intent requests that can be
> fulfilled by default intents that are reusable.
>
> Signed-off-by: Chris Lew <clew@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---
> drivers/rpmsg/qcom_glink_native.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 2668c66..df3c608 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -734,9 +734,11 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink,
> static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
> u32 cid, size_t size)
> {
> - struct glink_core_rx_intent *intent;
> + struct glink_core_rx_intent *intent = NULL;
> + struct glink_core_rx_intent *tmp;
> struct glink_channel *channel;
> unsigned long flags;
> + int iid;
>
> spin_lock_irqsave(&glink->idr_lock, flags);
> channel = idr_find(&glink->rcids, cid);
> @@ -747,6 +749,19 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
> return;
> }
>
> + spin_lock_irqsave(&channel->intent_lock, flags);
> + idr_for_each_entry(&channel->liids, tmp, iid) {
> + if (tmp->size >= size && tmp->reuse) {
> + intent = tmp;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(&channel->intent_lock, flags);
> + if (intent) {
> + qcom_glink_send_intent_req_ack(glink, channel, !!intent);
This looks reasonable, but you know that !!intent is true in this block,
and looking more at it I think the end result would look better as:
/* Try to find a reusable intent to fit the request */
spin_lock()
intent = findit();
spin_unlock()
/* Allocate a new intent if none was found */
if (!intent) {
intent = qcom_glink_alloc_intent(glink, channel, size, false);
if (intent)
qcom_glink_advertise_intent(glink, channel, intent);
}
qcom_glink_send_intent_req_ack(glink, channel, !!intent);
Regards,
Bjorn
> + return;
> + }
> +
> intent = qcom_glink_alloc_intent(glink, channel, size, false);
> if (intent)
> qcom_glink_advertise_intent(glink, channel, intent);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 3/6] rpmsg: glink: Add TX_DATA_CONT command while sending
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
2020-07-30 5:18 ` [PATCH V1 1/6] rpmsg: glink: fix destroy channel endpoint logic Deepak Kumar Singh
2020-07-30 5:18 ` [PATCH V1 2/6] rpmsg: glink: Deny intent request if reusable intent fits Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 17:22 ` (subset) " Bjorn Andersson
2020-07-30 5:18 ` [PATCH V1 4/6] rpmsg: glink: Remove the rpmsg dev in close_ack Deepak Kumar Singh
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Arun Kumar Neelakantam, Deepak Kumar Singh
From: Arun Kumar Neelakantam <aneela@codeaurora.org>
With current design the transport can send packets of size upto
FIFO_SIZE which is 16k and return failure for all packets above 16k.
Add TX_DATA_CONT command to send packets greater than 16k by splitting
into 8K chunks.
Signed-off-by: Arun Kumar Neelakantam <aneela@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 38 ++++++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index df3c608..ac179b1 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1289,6 +1289,8 @@ static int __qcom_glink_send(struct glink_channel *channel,
} __packed req;
int ret;
unsigned long flags;
+ int chunk_size = len;
+ int left_size = 0;
if (!glink->intentless) {
while (!intent) {
@@ -1322,18 +1324,46 @@ static int __qcom_glink_send(struct glink_channel *channel,
iid = intent->id;
}
+ if (wait && (chunk_size > SZ_8K)) {
+ chunk_size = SZ_8K;
+ left_size = len - chunk_size;
+ }
req.msg.cmd = cpu_to_le16(RPM_CMD_TX_DATA);
req.msg.param1 = cpu_to_le16(channel->lcid);
req.msg.param2 = cpu_to_le32(iid);
- req.chunk_size = cpu_to_le32(len);
- req.left_size = cpu_to_le32(0);
+ req.chunk_size = cpu_to_le32(chunk_size);
+ req.left_size = cpu_to_le32(left_size);
- ret = qcom_glink_tx(glink, &req, sizeof(req), data, len, wait);
+ ret = qcom_glink_tx(glink, &req, sizeof(req), data, chunk_size, wait);
/* Mark intent available if we failed */
- if (ret && intent)
+ if (ret && intent) {
intent->in_use = false;
+ return ret;
+ }
+ while (left_size > 0) {
+ data = (void *)((char *)data + chunk_size);
+ chunk_size = left_size;
+ if (chunk_size > SZ_8K)
+ chunk_size = SZ_8K;
+ left_size -= chunk_size;
+
+ req.msg.cmd = cpu_to_le16(RPM_CMD_TX_DATA_CONT);
+ req.msg.param1 = cpu_to_le16(channel->lcid);
+ req.msg.param2 = cpu_to_le32(iid);
+ req.chunk_size = cpu_to_le32(chunk_size);
+ req.left_size = cpu_to_le32(left_size);
+
+ ret = qcom_glink_tx(glink, &req, sizeof(req), data,
+ chunk_size, wait);
+
+ /* Mark intent available if we failed */
+ if (ret && intent) {
+ intent->in_use = false;
+ break;
+ }
+ }
return ret;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH V1 3/6] rpmsg: glink: Add TX_DATA_CONT command while sending
2020-07-30 5:18 ` [PATCH V1 3/6] rpmsg: glink: Add TX_DATA_CONT command while sending Deepak Kumar Singh
@ 2021-10-15 17:22 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 17:22 UTC (permalink / raw)
To: clew, Deepak Kumar Singh
Cc: mathieu.poirier, linux-remoteproc, Arun Kumar Neelakantam,
linux-kernel, linux-arm-msm
On Thu, 30 Jul 2020 10:48:13 +0530, Deepak Kumar Singh wrote:
> From: Arun Kumar Neelakantam <aneela@codeaurora.org>
>
> With current design the transport can send packets of size upto
> FIFO_SIZE which is 16k and return failure for all packets above 16k.
>
> Add TX_DATA_CONT command to send packets greater than 16k by splitting
> into 8K chunks.
>
> [...]
Applied, thanks!
[3/6] rpmsg: glink: Add TX_DATA_CONT command while sending
commit: 8956927faed366b60b0355f4a4317a10e281ced7
Best regards,
--
Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 4/6] rpmsg: glink: Remove the rpmsg dev in close_ack
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
` (2 preceding siblings ...)
2020-07-30 5:18 ` [PATCH V1 3/6] rpmsg: glink: Add TX_DATA_CONT command while sending Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 17:22 ` (subset) " Bjorn Andersson
2020-07-30 5:18 ` [PATCH V1 5/6] rpmsg: glink: Remove channel decouple from rpdev release Deepak Kumar Singh
2020-07-30 5:18 ` [PATCH V1 6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case Deepak Kumar Singh
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Arun Kumar Neelakantam, Deepak Kumar Singh
From: Arun Kumar Neelakantam <aneela@codeaurora.org>
Un-register and register of rpmsg driver is sending invalid open_ack
on closed channel.
To avoid sending invalid open_ack case unregister the rpmsg device
after receiving the local_close_ack from remote side.
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
signed-off-by: Arun Kumar Neelakantam <aneela@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index ac179b1..031bc1d 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1526,6 +1526,7 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid)
rpmsg_unregister_device(glink->dev, &chinfo);
}
+ channel->rpdev = NULL;
qcom_glink_send_close_ack(glink, channel->rcid);
@@ -1539,6 +1540,7 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid)
static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)
{
+ struct rpmsg_channel_info chinfo;
struct glink_channel *channel;
unsigned long flags;
@@ -1553,6 +1555,16 @@ static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)
channel->lcid = 0;
spin_unlock_irqrestore(&glink->idr_lock, flags);
+ /* Decouple the potential rpdev from the channel */
+ if (channel->rpdev) {
+ strlcpy(chinfo.name, channel->name, sizeof(chinfo.name));
+ chinfo.src = RPMSG_ADDR_ANY;
+ chinfo.dst = RPMSG_ADDR_ANY;
+
+ rpmsg_unregister_device(glink->dev, &chinfo);
+ }
+ channel->rpdev = NULL;
+
kref_put(&channel->refcount, qcom_glink_channel_release);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH V1 4/6] rpmsg: glink: Remove the rpmsg dev in close_ack
2020-07-30 5:18 ` [PATCH V1 4/6] rpmsg: glink: Remove the rpmsg dev in close_ack Deepak Kumar Singh
@ 2021-10-15 17:22 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 17:22 UTC (permalink / raw)
To: clew, Deepak Kumar Singh
Cc: mathieu.poirier, linux-remoteproc, Arun Kumar Neelakantam,
linux-kernel, linux-arm-msm
On Thu, 30 Jul 2020 10:48:14 +0530, Deepak Kumar Singh wrote:
> From: Arun Kumar Neelakantam <aneela@codeaurora.org>
>
> Un-register and register of rpmsg driver is sending invalid open_ack
> on closed channel.
>
> To avoid sending invalid open_ack case unregister the rpmsg device
> after receiving the local_close_ack from remote side.
>
> [...]
Applied, thanks!
[4/6] rpmsg: glink: Remove the rpmsg dev in close_ack
commit: c7c182d4447e172f87e37d6c04879b94b8635b37
Best regards,
--
Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 5/6] rpmsg: glink: Remove channel decouple from rpdev release
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
` (3 preceding siblings ...)
2020-07-30 5:18 ` [PATCH V1 4/6] rpmsg: glink: Remove the rpmsg dev in close_ack Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 17:22 ` (subset) " Bjorn Andersson
2020-07-30 5:18 ` [PATCH V1 6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case Deepak Kumar Singh
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Deepak Kumar Singh
From: Chris Lew <clew@codeaurora.org>
If a channel is being rapidly restarting and the kobj release worker
is busy, there is a chance the the rpdev_release function will run
after the channel struct itself has been released.
There should not be a need to decouple the channel from rpdev in the
rpdev release since that should only happen from the close commands.
Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 031bc1d..efaf32d 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1419,9 +1419,7 @@ static const struct rpmsg_endpoint_ops glink_endpoint_ops = {
static void qcom_glink_rpdev_release(struct device *dev)
{
struct rpmsg_device *rpdev = to_rpmsg_device(dev);
- struct glink_channel *channel = to_glink_channel(rpdev->ept);
- channel->rpdev = NULL;
kfree(rpdev);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH V1 5/6] rpmsg: glink: Remove channel decouple from rpdev release
2020-07-30 5:18 ` [PATCH V1 5/6] rpmsg: glink: Remove channel decouple from rpdev release Deepak Kumar Singh
@ 2021-10-15 17:22 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 17:22 UTC (permalink / raw)
To: clew, Deepak Kumar Singh
Cc: mathieu.poirier, linux-arm-msm, linux-kernel, linux-remoteproc
On Thu, 30 Jul 2020 10:48:15 +0530, Deepak Kumar Singh wrote:
> From: Chris Lew <clew@codeaurora.org>
>
> If a channel is being rapidly restarting and the kobj release worker
> is busy, there is a chance the the rpdev_release function will run
> after the channel struct itself has been released.
>
> There should not be a need to decouple the channel from rpdev in the
> rpdev release since that should only happen from the close commands.
>
> [...]
Applied, thanks!
[5/6] rpmsg: glink: Remove channel decouple from rpdev release
commit: 343ba27b6f9d473ec3e602cc648300eb03a7fa05
Best regards,
--
Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case
2020-07-30 5:18 [PATCH V1 0/6] Glink native fixes upstreaming Deepak Kumar Singh
` (4 preceding siblings ...)
2020-07-30 5:18 ` [PATCH V1 5/6] rpmsg: glink: Remove channel decouple from rpdev release Deepak Kumar Singh
@ 2020-07-30 5:18 ` Deepak Kumar Singh
2021-10-15 17:22 ` (subset) " Bjorn Andersson
5 siblings, 1 reply; 13+ messages in thread
From: Deepak Kumar Singh @ 2020-07-30 5:18 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel,
Arun Kumar Neelakantam, Deepak Kumar Singh
From: Arun Kumar Neelakantam <aneela@codeaurora.org>
The current design sleeps unconditionally in TX FIFO full case and
wakeup only after sleep timer expires which adds random delays in
clients TX path.
Avoid sleep and use READ_NOTIFY command so that writer can be woken up
when remote notifies about read completion by sending IRQ.
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
Signed-off-by: Arun Kumar Neelakantam <aneela@codeaurora.org>
---
drivers/rpmsg/qcom_glink_native.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index efaf32d..098039d 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -92,6 +92,8 @@ struct glink_core_rx_intent {
* @rcids: idr of all channels with a known remote channel id
* @features: remote features
* @intentless: flag to indicate that there is no intent
+ * @tx_avail_notify: Waitqueue for pending tx tasks
+ * @sent_read_notify: flag to check cmd sent or not
*/
struct qcom_glink {
struct device *dev;
@@ -118,6 +120,8 @@ struct qcom_glink {
unsigned long features;
bool intentless;
+ wait_queue_head_t tx_avail_notify;
+ bool sent_read_notify;
};
enum {
@@ -301,6 +305,20 @@ static void qcom_glink_tx_write(struct qcom_glink *glink,
glink->tx_pipe->write(glink->tx_pipe, hdr, hlen, data, dlen);
}
+static void qcom_glink_send_read_notify(struct qcom_glink *glink)
+{
+ struct glink_msg msg;
+
+ msg.cmd = cpu_to_le16(RPM_CMD_READ_NOTIF);
+ msg.param1 = 0;
+ msg.param2 = 0;
+
+ qcom_glink_tx_write(glink, &msg, sizeof(msg), NULL, 0);
+
+ mbox_send_message(glink->mbox_chan, NULL);
+ mbox_client_txdone(glink->mbox_chan, 0);
+}
+
static int qcom_glink_tx(struct qcom_glink *glink,
const void *hdr, size_t hlen,
const void *data, size_t dlen, bool wait)
@@ -321,12 +339,21 @@ static int qcom_glink_tx(struct qcom_glink *glink,
goto out;
}
+ if (!glink->sent_read_notify) {
+ glink->sent_read_notify = true;
+ qcom_glink_send_read_notify(glink);
+ }
+
/* Wait without holding the tx_lock */
spin_unlock_irqrestore(&glink->tx_lock, flags);
- usleep_range(10000, 15000);
+ wait_event_timeout(glink->tx_avail_notify,
+ qcom_glink_tx_avail(glink) >= tlen, 10 * HZ);
spin_lock_irqsave(&glink->tx_lock, flags);
+
+ if (qcom_glink_tx_avail(glink) >= tlen)
+ glink->sent_read_notify = false;
}
qcom_glink_tx_write(glink, hdr, hlen, data, dlen);
@@ -1000,6 +1027,9 @@ static irqreturn_t qcom_glink_native_intr(int irq, void *data)
unsigned int cmd;
int ret = 0;
+ /* To wakeup any blocking writers */
+ wake_up_all(&glink->tx_avail_notify);
+
for (;;) {
avail = qcom_glink_rx_avail(glink);
if (avail < sizeof(msg))
@@ -1542,6 +1572,9 @@ static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)
struct glink_channel *channel;
unsigned long flags;
+ /* To wakeup any blocking writers */
+ wake_up_all(&glink->tx_avail_notify);
+
spin_lock_irqsave(&glink->idr_lock, flags);
channel = idr_find(&glink->lcids, lcid);
if (WARN(!channel, "close ack on unknown channel\n")) {
@@ -1658,6 +1691,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
spin_lock_init(&glink->rx_lock);
INIT_LIST_HEAD(&glink->rx_queue);
INIT_WORK(&glink->rx_work, qcom_glink_work);
+ init_waitqueue_head(&glink->tx_avail_notify);
spin_lock_init(&glink->idr_lock);
idr_init(&glink->lcids);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH V1 6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case
2020-07-30 5:18 ` [PATCH V1 6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case Deepak Kumar Singh
@ 2021-10-15 17:22 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2021-10-15 17:22 UTC (permalink / raw)
To: clew, Deepak Kumar Singh
Cc: mathieu.poirier, linux-remoteproc, Arun Kumar Neelakantam,
linux-kernel, linux-arm-msm
On Thu, 30 Jul 2020 10:48:16 +0530, Deepak Kumar Singh wrote:
> From: Arun Kumar Neelakantam <aneela@codeaurora.org>
>
> The current design sleeps unconditionally in TX FIFO full case and
> wakeup only after sleep timer expires which adds random delays in
> clients TX path.
>
> Avoid sleep and use READ_NOTIFY command so that writer can be woken up
> when remote notifies about read completion by sending IRQ.
>
> [...]
Applied, thanks!
[6/6] rpmsg: glink: Send READ_NOTIFY command in FIFO full case
commit: b16a37e1846c9573a847a56fa2f31ba833dae45a
Best regards,
--
Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread