CV-Bowen commented on code in PR #21381:
URL: https://github.com/apache/nuttx/pull/21381#discussion_r4472348884
##########
drivers/rpmsg/rpmsg_virtio.c:
##########
@@ -472,8 +527,129 @@ static void rpmsg_virtio_rx_callback(FAR struct virtqueue
*vq)
RPMSG_VIRTIO_INVALIDATE(rvq->vq_ring.avail->idx);
priv->headrx = rvq->vq_ring.avail->idx;
}
+}
+
+static void rpmsg_virtio_update_tx(FAR struct rpmsg_virtio_priv_s *priv)
+{
+ FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev;
+ FAR struct virtqueue *svq = rvdev->svq;
+
+ if (rpmsg_virtio_get_role(rvdev) == RPMSG_HOST)
+ {
+ RPMSG_VIRTIO_INVALIDATE(svq->vq_ring.avail->idx);
+ priv->headtx = svq->vq_ring.avail->idx;
+ }
+ else
+ {
+ RPMSG_VIRTIO_INVALIDATE(svq->vq_ring.used->idx);
+ priv->headtx = svq->vq_ring.used->idx;
+ }
+}
+
+/****************************************************************************
+ * Name: rpmsg_virtio_rx_enqueue
+ ****************************************************************************/
+
+static void rpmsg_virtio_rx_enqueue(FAR struct rpmsg_virtio_priv_s *priv,
+ FAR struct rpmsg_hdr *hdr)
+{
+ FAR struct rpmsg_virtio_node_s *node;
+ FAR struct metal_list *tmp;
+
+ tmp = metal_list_first(&priv->freerx);
+ DEBUGASSERT(tmp != NULL);
+ metal_list_del(tmp);
+ node = metal_container_of(tmp, struct rpmsg_virtio_node_s, node);
+ node->hdr = hdr;
+ metal_list_add_tail(&priv->readyrx, &node->node);
+}
+
+/****************************************************************************
+ * Name: rpmsg_virtio_rx_dispatch
+ ****************************************************************************/
+
+static void rpmsg_virtio_rx_dispatch(FAR struct rpmsg_virtio_priv_s *priv)
+{
+ FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev;
+ FAR struct rpmsg_device *rdev = &rvdev->rdev;
+ FAR struct rpmsg_endpoint *ept;
+ FAR struct rpmsg_hdr *hdr;
+ bool release = false;
+ bool last = false;
+ uint32_t len;
+ uint16_t idx;
+ int status;
+
+ metal_mutex_acquire(&rdev->lock);
Review Comment:
rdev->lock is an rmutex on master, so it cannot be taken in the interrupt. I
dropped the interrupt dispatch commit from this PR; it will come back in a
later PR together with the change that makes the libmetal mutex a spinlock.
With it gone, rx is handled by the thread again and the interrupt only updates
headrx and wakes the thread.
##########
drivers/rpmsg/rpmsg_virtio.c:
##########
@@ -472,8 +527,129 @@ static void rpmsg_virtio_rx_callback(FAR struct virtqueue
*vq)
RPMSG_VIRTIO_INVALIDATE(rvq->vq_ring.avail->idx);
priv->headrx = rvq->vq_ring.avail->idx;
}
+}
+
+static void rpmsg_virtio_update_tx(FAR struct rpmsg_virtio_priv_s *priv)
+{
+ FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev;
+ FAR struct virtqueue *svq = rvdev->svq;
+
+ if (rpmsg_virtio_get_role(rvdev) == RPMSG_HOST)
+ {
+ RPMSG_VIRTIO_INVALIDATE(svq->vq_ring.avail->idx);
+ priv->headtx = svq->vq_ring.avail->idx;
+ }
+ else
+ {
+ RPMSG_VIRTIO_INVALIDATE(svq->vq_ring.used->idx);
+ priv->headtx = svq->vq_ring.used->idx;
+ }
+}
+
+/****************************************************************************
+ * Name: rpmsg_virtio_rx_enqueue
+ ****************************************************************************/
+
+static void rpmsg_virtio_rx_enqueue(FAR struct rpmsg_virtio_priv_s *priv,
+ FAR struct rpmsg_hdr *hdr)
+{
+ FAR struct rpmsg_virtio_node_s *node;
+ FAR struct metal_list *tmp;
+
+ tmp = metal_list_first(&priv->freerx);
+ DEBUGASSERT(tmp != NULL);
+ metal_list_del(tmp);
+ node = metal_container_of(tmp, struct rpmsg_virtio_node_s, node);
+ node->hdr = hdr;
+ metal_list_add_tail(&priv->readyrx, &node->node);
+}
+
+/****************************************************************************
+ * Name: rpmsg_virtio_rx_dispatch
+ ****************************************************************************/
+
+static void rpmsg_virtio_rx_dispatch(FAR struct rpmsg_virtio_priv_s *priv)
+{
+ FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev;
+ FAR struct rpmsg_device *rdev = &rvdev->rdev;
+ FAR struct rpmsg_endpoint *ept;
+ FAR struct rpmsg_hdr *hdr;
+ bool release = false;
+ bool last = false;
+ uint32_t len;
+ uint16_t idx;
+ int status;
+
+ metal_mutex_acquire(&rdev->lock);
+ while (!last)
+ {
+ /* Process the received data from remote node */
+
+ hdr = rpmsg_virtio_get_rx_buffer(rvdev, &len, &idx, &last);
Review Comment:
With the interrupt dispatch commit dropped, the rx callback no longer
touches rvdev->rvq. That later commit will gate the rx callback on an init flag
set after the first dispatch in rpmsg_virtio_start().
##########
drivers/rpmsg/rpmsg_virtio.c:
##########
@@ -805,6 +1013,10 @@ void rpmsg_virtio_remove(FAR struct virtio_device *vdev)
rpmsg_deinit_vdev(&priv->rvdev);
+ /* Free noderx */
+
+ kmm_free(priv->noderx);
Review Comment:
noderx is not added in this PR any more. In the later commit it is freed
after kthread_delete().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
