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@xxxxxxxxxxx>
> ---
> 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
>
