| Message ID | CAK=WgbaCtACsvt9cqQmGqXsADFb_3bfnNKeWq9iijR2vkYVuQQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Ohad Ben-Cohen [mailto:ohad@wizery.com] > Sent: Sunday, April 07, 2013 5:51 AM > > On Fri, Mar 29, 2013 at 4:41 AM, Robert Tivy <rtivy@ti.com> wrote: > > Change virtqueue callback function rpmsg_recv_done() to process all > > available messages instead of just one message. > > > > Signed-off-by: Robert Tivy <rtivy@ti.com> > > I'm thinking instead of adding an indentation level, let's split the > _recv function. > > This is what I have in mind - let me know if it works for you - thanks: > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c > b/drivers/rpmsg/virtio_rpmsg_bus.c > index a59684b..42b9872 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -776,22 +776,11 @@ out: > } > EXPORT_SYMBOL(rpmsg_send_offchannel_raw); > > -/* called when an rx buffer is used, and it's time to digest a message > */ > -static void rpmsg_recv_done(struct virtqueue *rvq) > +static int rpmsg_recv_single(struct virtproc_info *vrp, struct device > *dev, > + struct rpmsg_hdr *msg, unsigned int len) > { > - struct rpmsg_hdr *msg; > - unsigned int len; > struct rpmsg_endpoint *ept; > struct scatterlist sg; > - struct virtproc_info *vrp = rvq->vdev->priv; > - struct device *dev = &rvq->vdev->dev; > - int err; > - > - msg = virtqueue_get_buf(rvq, &len); > - if (!msg) { > - dev_err(dev, "uhm, incoming signal, but no used buffer > ?\n"); > - return; > - } > > dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d, > Reserved: %d\n", > msg->src, msg->dst, msg->len, > @@ -806,7 +795,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq) > if (len > RPMSG_BUF_SIZE || > msg->len > (len - sizeof(struct rpmsg_hdr))) { > dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, > msg->len); > - return; > + return -EINVAL; > } > > /* use the dst addr to fetch the callback of the appropriate > user */ > @@ -842,9 +831,39 @@ static void rpmsg_recv_done(struct virtqueue *rvq) > err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, msg, GFP_KERNEL); > if (err < 0) { > dev_err(dev, "failed to add a virtqueue buffer: %d\n", > err); > + return err; > + } > + > + return 0; > +} > + > +/* called when an rx buffer is used, and it's time to digest a message > */ > +static void rpmsg_recv_done(struct virtqueue *rvq) > +{ > + struct virtproc_info *vrp = rvq->vdev->priv; > + struct device *dev = &rvq->vdev->dev; > + struct rpmsg_hdr *msg; > + unsigned int len, msgs_received = 0; > + int err; > + > + msg = virtqueue_get_buf(rvq, &len); > + if (!msg) { > + dev_err(dev, "uhm, incoming signal, but no used buffer > ?\n"); > return; > } > > + while (msg) { > + err = rpmsg_recv_single(vrp, dev, msg, len); > + if (err) > + break; > + > + msgs_received++; > + > + msg = virtqueue_get_buf(rvq, &len); > + }; > + > + dev_dbg(dev, "Received %u messages\n", msgs_received); > + > /* tell the remote processor we added another available rx > buffer */ > virtqueue_kick(vrp->rvq); > } Shouldn't the virtqueue_kick() be called only when we successfully added a buffer with virtqueue_add_buf()? If so, we should make it: if (msgs_received) /* tell the remote processor we added another available rx buffer */ virtqueue_kick(vrp->rvp); } Thanks for the rewrite, looks better. I'll give it a try and let you know how it goes. Regards, - Rob
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index a59684b..42b9872 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -776,22 +776,11 @@ out: } EXPORT_SYMBOL(rpmsg_send_offchannel_raw); -/* called when an rx buffer is used, and it's time to digest a message */ -static void rpmsg_recv_done(struct virtqueue *rvq) +static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev, + struct rpmsg_hdr *msg, unsigned int len) { - struct rpmsg_hdr *msg; - unsigned int len; struct rpmsg_endpoint *ept; struct scatterlist sg; - struct virtproc_info *vrp = rvq->vdev->priv; - struct device *dev = &rvq->vdev->dev; - int err; - - msg = virtqueue_get_buf(rvq, &len); - if (!msg) { - dev_err(dev, "uhm, incoming signal, but no used buffer ?\n"); - return; - } dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n", msg->src, msg->dst, msg->len, @@ -806,7 +795,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq) if (len > RPMSG_BUF_SIZE || msg->len > (len - sizeof(struct rpmsg_hdr))) { dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg->len); - return; + return -EINVAL; } /* use the dst addr to fetch the callback of the appropriate user */ @@ -842,9 +831,39 @@ static void rpmsg_recv_done(struct virtqueue *rvq) err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, msg, GFP_KERNEL); if (err < 0) { dev_err(dev, "failed to add a virtqueue buffer: %d\n", err); + return err; + } + + return 0; +} + +/* called when an rx buffer is used, and it's time to digest a message */ +static void rpmsg_recv_done(struct virtqueue *rvq) +{ + struct virtproc_info *vrp = rvq->vdev->priv; + struct device *dev = &rvq->vdev->dev; + struct rpmsg_hdr *msg; + unsigned int len, msgs_received = 0; + int err; + + msg = virtqueue_get_buf(rvq, &len); + if (!msg) { + dev_err(dev, "uhm, incoming signal, but no used buffer ?\n"); return; } + while (msg) { + err = rpmsg_recv_single(vrp, dev, msg, len); + if (err) + break; + + msgs_received++; + + msg = virtqueue_get_buf(rvq, &len); + }; + + dev_dbg(dev, "Received %u messages\n", msgs_received); + /* tell the remote processor we added another available rx buffer */ virtqueue_kick(vrp->rvq); }