diff mbox series

[4/4] rpmsg: glink: Add support for rpmsg_rx_done

Message ID 1654651005-15475-5-git-send-email-quic_clew@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series Introduction of rpmsg_rx_done | expand

Commit Message

Chris Lew June 8, 2022, 1:16 a.m. UTC
Add the implementation for the hooks of rpmsg_rx_done. If a client
signals they want to hold onto a buffer with RPMSG_DEFER in the rx_cb,
glink will move that intent to a deferred cleanup list. On the new
rpmsg rx_done call, the glink transport will search this deferred
cleanup list for the matching buffer and release the intent.

Signed-off-by: Chris Lew <quic_clew@quicinc.com>
---
 drivers/rpmsg/qcom_glink_native.c | 54 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)

Comments

Mathieu Poirier July 27, 2022, 5:21 p.m. UTC | #1
On Tue, Jun 07, 2022 at 06:16:45PM -0700, Chris Lew wrote:
> Add the implementation for the hooks of rpmsg_rx_done. If a client
> signals they want to hold onto a buffer with RPMSG_DEFER in the rx_cb,
> glink will move that intent to a deferred cleanup list. On the new
> rpmsg rx_done call, the glink transport will search this deferred
> cleanup list for the matching buffer and release the intent.
> 
> Signed-off-by: Chris Lew <quic_clew@quicinc.com>
> ---
>  drivers/rpmsg/qcom_glink_native.c | 54 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 799e602113a1..db0dcc04f393 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -146,6 +146,7 @@ enum {
>   * @riids:	idr of all remote intents
>   * @intent_work: worker responsible for transmitting rx_done packets
>   * @done_intents: list of intents that needs to be announced rx_done
> + * @defer_intents: list of intents held by the client released by rpmsg_rx_done
>   * @buf:	receive buffer, for gathering fragments
>   * @buf_offset:	write offset in @buf
>   * @buf_size:	size of current @buf
> @@ -174,6 +175,7 @@ struct glink_channel {
>  	struct idr riids;
>  	struct work_struct intent_work;
>  	struct list_head done_intents;
> +	struct list_head defer_intents;
>  
>  	struct glink_core_rx_intent *buf;
>  	int buf_offset;
> @@ -232,6 +234,7 @@ static struct glink_channel *qcom_glink_alloc_channel(struct qcom_glink *glink,
>  	init_completion(&channel->intent_req_comp);
>  
>  	INIT_LIST_HEAD(&channel->done_intents);
> +	INIT_LIST_HEAD(&channel->defer_intents);
>  	INIT_WORK(&channel->intent_work, qcom_glink_rx_done_work);
>  
>  	idr_init(&channel->liids);
> @@ -261,6 +264,12 @@ static void qcom_glink_channel_release(struct kref *ref)
>  			kfree(intent);
>  		}
>  	}
> +	list_for_each_entry_safe(intent, tmp, &channel->defer_intents, node) {
> +		if (!intent->reuse) {
> +			kfree(intent->data);
> +			kfree(intent);
> +		}
> +	}
>  
>  	idr_for_each_entry(&channel->liids, tmp, iid) {
>  		kfree(tmp->data);
> @@ -549,9 +558,10 @@ static void qcom_glink_rx_done_work(struct work_struct *work)
>  	spin_unlock_irqrestore(&channel->intent_lock, flags);
>  }
>  
> -static void qcom_glink_rx_done(struct qcom_glink *glink,
> +static void __qcom_glink_rx_done(struct qcom_glink *glink,
>  			       struct glink_channel *channel,
> -			       struct glink_core_rx_intent *intent)
> +			       struct glink_core_rx_intent *intent,
> +			       bool defer)
>  {
>  	int ret = -EAGAIN;
>  
> @@ -569,6 +579,14 @@ static void qcom_glink_rx_done(struct qcom_glink *glink,
>  		spin_unlock(&channel->intent_lock);
>  	}
>  
> +	/* Move intent to defer list until client calls rpmsg_rx_done */
> +	if (defer) {
> +		spin_lock(&channel->intent_lock);
> +		list_add_tail(&intent->node, &channel->defer_intents);
> +		spin_unlock(&channel->intent_lock);
> +		return;
> +	}
> +
>  	/* Schedule the sending of a rx_done indication */
>  	spin_lock(&channel->intent_lock);
>  	if (list_empty(&channel->done_intents))
> @@ -581,6 +599,28 @@ static void qcom_glink_rx_done(struct qcom_glink *glink,
>  	spin_unlock(&channel->intent_lock);
>  }
>  
> +static int qcom_glink_rx_done(struct rpmsg_endpoint *ept, void *data)
> +{
> +	struct glink_channel *channel = to_glink_channel(ept);
> +	struct qcom_glink *glink = channel->glink;
> +	struct glink_core_rx_intent *intent, *tmp;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&channel->intent_lock, flags);
> +	list_for_each_entry_safe(intent, tmp, &channel->defer_intents, node) {
> +		if (intent->data == data) {
> +			list_del(&intent->node);
> +			spin_unlock_irqrestore(&channel->intent_lock, flags);
> +
> +			qcom_glink_send_rx_done(glink, channel, intent, true);
> +			return 0;
> +		}
> +	}
> +	spin_unlock_irqrestore(&channel->intent_lock, flags);
> +
> +	return -EINVAL;
> +}
> +
>  /**
>   * qcom_glink_receive_version() - receive version/features from remote system
>   *
> @@ -841,6 +881,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
>  	} __packed hdr;
>  	unsigned int chunk_size;
>  	unsigned int left_size;
> +	bool rx_done_defer;
>  	unsigned int rcid;
>  	unsigned int liid;
>  	int ret = 0;
> @@ -935,7 +976,12 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
>  		intent->offset = 0;
>  		channel->buf = NULL;
>  
> -		qcom_glink_rx_done(glink, channel, intent);
> +		if (channel->ept.rx_done && ret == RPMSG_DEFER)

I don't see where @ret could be set to RPMSG_DEFER in this function...

Thanks,
Mathieu


> +			rx_done_defer = true;
> +		else
> +			rx_done_defer = false;
> +
> +		__qcom_glink_rx_done(glink, channel, intent, rx_done_defer);
>  	}
>  
>  advance_rx:
> @@ -1212,6 +1258,7 @@ static struct rpmsg_endpoint *qcom_glink_create_ept(struct rpmsg_device *rpdev,
>  	ept->cb = cb;
>  	ept->priv = priv;
>  	ept->ops = &glink_endpoint_ops;
> +	ept->rx_done = true;
>  
>  	return ept;
>  }
> @@ -1462,6 +1509,7 @@ static const struct rpmsg_endpoint_ops glink_endpoint_ops = {
>  	.sendto = qcom_glink_sendto,
>  	.trysend = qcom_glink_trysend,
>  	.trysendto = qcom_glink_trysendto,
> +	.rx_done = qcom_glink_rx_done,
>  };
>  
>  static void qcom_glink_rpdev_release(struct device *dev)
> -- 
> 2.7.4
>
diff mbox series

Patch

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 799e602113a1..db0dcc04f393 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -146,6 +146,7 @@  enum {
  * @riids:	idr of all remote intents
  * @intent_work: worker responsible for transmitting rx_done packets
  * @done_intents: list of intents that needs to be announced rx_done
+ * @defer_intents: list of intents held by the client released by rpmsg_rx_done
  * @buf:	receive buffer, for gathering fragments
  * @buf_offset:	write offset in @buf
  * @buf_size:	size of current @buf
@@ -174,6 +175,7 @@  struct glink_channel {
 	struct idr riids;
 	struct work_struct intent_work;
 	struct list_head done_intents;
+	struct list_head defer_intents;
 
 	struct glink_core_rx_intent *buf;
 	int buf_offset;
@@ -232,6 +234,7 @@  static struct glink_channel *qcom_glink_alloc_channel(struct qcom_glink *glink,
 	init_completion(&channel->intent_req_comp);
 
 	INIT_LIST_HEAD(&channel->done_intents);
+	INIT_LIST_HEAD(&channel->defer_intents);
 	INIT_WORK(&channel->intent_work, qcom_glink_rx_done_work);
 
 	idr_init(&channel->liids);
@@ -261,6 +264,12 @@  static void qcom_glink_channel_release(struct kref *ref)
 			kfree(intent);
 		}
 	}
+	list_for_each_entry_safe(intent, tmp, &channel->defer_intents, node) {
+		if (!intent->reuse) {
+			kfree(intent->data);
+			kfree(intent);
+		}
+	}
 
 	idr_for_each_entry(&channel->liids, tmp, iid) {
 		kfree(tmp->data);
@@ -549,9 +558,10 @@  static void qcom_glink_rx_done_work(struct work_struct *work)
 	spin_unlock_irqrestore(&channel->intent_lock, flags);
 }
 
-static void qcom_glink_rx_done(struct qcom_glink *glink,
+static void __qcom_glink_rx_done(struct qcom_glink *glink,
 			       struct glink_channel *channel,
-			       struct glink_core_rx_intent *intent)
+			       struct glink_core_rx_intent *intent,
+			       bool defer)
 {
 	int ret = -EAGAIN;
 
@@ -569,6 +579,14 @@  static void qcom_glink_rx_done(struct qcom_glink *glink,
 		spin_unlock(&channel->intent_lock);
 	}
 
+	/* Move intent to defer list until client calls rpmsg_rx_done */
+	if (defer) {
+		spin_lock(&channel->intent_lock);
+		list_add_tail(&intent->node, &channel->defer_intents);
+		spin_unlock(&channel->intent_lock);
+		return;
+	}
+
 	/* Schedule the sending of a rx_done indication */
 	spin_lock(&channel->intent_lock);
 	if (list_empty(&channel->done_intents))
@@ -581,6 +599,28 @@  static void qcom_glink_rx_done(struct qcom_glink *glink,
 	spin_unlock(&channel->intent_lock);
 }
 
+static int qcom_glink_rx_done(struct rpmsg_endpoint *ept, void *data)
+{
+	struct glink_channel *channel = to_glink_channel(ept);
+	struct qcom_glink *glink = channel->glink;
+	struct glink_core_rx_intent *intent, *tmp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&channel->intent_lock, flags);
+	list_for_each_entry_safe(intent, tmp, &channel->defer_intents, node) {
+		if (intent->data == data) {
+			list_del(&intent->node);
+			spin_unlock_irqrestore(&channel->intent_lock, flags);
+
+			qcom_glink_send_rx_done(glink, channel, intent, true);
+			return 0;
+		}
+	}
+	spin_unlock_irqrestore(&channel->intent_lock, flags);
+
+	return -EINVAL;
+}
+
 /**
  * qcom_glink_receive_version() - receive version/features from remote system
  *
@@ -841,6 +881,7 @@  static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
 	} __packed hdr;
 	unsigned int chunk_size;
 	unsigned int left_size;
+	bool rx_done_defer;
 	unsigned int rcid;
 	unsigned int liid;
 	int ret = 0;
@@ -935,7 +976,12 @@  static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
 		intent->offset = 0;
 		channel->buf = NULL;
 
-		qcom_glink_rx_done(glink, channel, intent);
+		if (channel->ept.rx_done && ret == RPMSG_DEFER)
+			rx_done_defer = true;
+		else
+			rx_done_defer = false;
+
+		__qcom_glink_rx_done(glink, channel, intent, rx_done_defer);
 	}
 
 advance_rx:
@@ -1212,6 +1258,7 @@  static struct rpmsg_endpoint *qcom_glink_create_ept(struct rpmsg_device *rpdev,
 	ept->cb = cb;
 	ept->priv = priv;
 	ept->ops = &glink_endpoint_ops;
+	ept->rx_done = true;
 
 	return ept;
 }
@@ -1462,6 +1509,7 @@  static const struct rpmsg_endpoint_ops glink_endpoint_ops = {
 	.sendto = qcom_glink_sendto,
 	.trysend = qcom_glink_trysend,
 	.trysendto = qcom_glink_trysendto,
+	.rx_done = qcom_glink_rx_done,
 };
 
 static void qcom_glink_rpdev_release(struct device *dev)