| Message ID | 1481142941-15616-6-git-send-email-loic.pallardy@st.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 12/07/2016 02:35 PM, Loic Pallardy wrote: > Low level platform specific driver has the knowledge of the different > communication link constraints like fixed or secured memory region to > use for buffer allocation. > > If virtual address is defined in virtio_rpmsg_cfg structure, a dedicated > memory pool buffer fitting platform requirements is available. > Rpmsg virtio layer should rely on it if its size is compliant with link > characteristics. > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > --- > drivers/rpmsg/virtio_rpmsg_bus.c | 54 ++++++++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 18 deletions(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > index 1a97af8..b12fe2a 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -57,6 +57,7 @@ > * @sendq: wait queue of sending contexts waiting for a tx buffers > * @sleepers: number of senders that are waiting for a tx buffer > * @ns_ept: the bus's name service endpoint > + * @ext_buffer: buffer allocated by low level driver > * > * This structure stores the rpmsg state of a given virtio remote processor > * device (there might be several virtio proc devices for each physical > @@ -76,6 +77,7 @@ struct virtproc_info { > wait_queue_head_t sendq; > atomic_t sleepers; > struct rpmsg_endpoint *ns_ept; > + bool ext_buffer; > }; > > /* The feature bitmap for virtio rpmsg */ > @@ -904,6 +906,17 @@ static int virtio_rpmsg_get_config(struct virtio_device *vdev) > ret = -ENOMEM; > goto out; > } > + > + /* Level platform specific buffer driver ? */ > + if (virtio_cfg.va != -1) { > + vrp->ext_buffer = true; > + /* half of the buffers is dedicated for RX */ > + vrp->rbufs = (void *)(uintptr_t)virtio_cfg.va; > + > + /* and half is dedicated for TX */ > + vrp->sbufs = (void *)(uintptr_t) virtio_cfg.va + total_buf_space / 2; > + } > + > return !ret; > } > out: > @@ -960,24 +973,27 @@ static int rpmsg_probe(struct virtio_device *vdev) > if (err < 0) > goto free_vrp; > > - total_buf_space = vrp->num_bufs * vrp->buf_size; > + /* Allocate buffer if none provided by low level platform driver */ > + if (!vrp->ext_buffer) { > + total_buf_space = vrp->num_bufs * vrp->buf_size; > > - bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, > - total_buf_space, &vrp->bufs_dma, > - GFP_KERNEL); > - if (!bufs_va) { > - err = -ENOMEM; > - goto vqs_del; > - } > + bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, > + total_buf_space, &vrp->bufs_dma, > + GFP_KERNEL); Can you run checkpatch with --strict when you resubmit the series, I believe a few have been introduced that weren't there before. regards Suman > + if (!bufs_va) { > + err = -ENOMEM; > + goto vqs_del; > + } > > - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > - bufs_va, &vrp->bufs_dma); > + dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > + bufs_va, &vrp->bufs_dma); > > - /* half of the buffers is dedicated for RX */ > - vrp->rbufs = bufs_va; > + /* half of the buffers is dedicated for RX */ > + vrp->rbufs = bufs_va; > > - /* and half is dedicated for TX */ > - vrp->sbufs = bufs_va + total_buf_space / 2; > + /* and half is dedicated for TX */ > + vrp->sbufs = bufs_va + total_buf_space / 2; > + } > > /* set up the receive buffers */ > for (i = 0; i < vrp->num_bufs / 2; i++) { > @@ -1028,8 +1044,9 @@ static int rpmsg_probe(struct virtio_device *vdev) > return 0; > > free_coherent: > - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > - bufs_va, vrp->bufs_dma); > + if (!vrp->ext_buffer) > + dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > + bufs_va, vrp->bufs_dma); > vqs_del: > vdev->config->del_vqs(vrp->vdev); > free_vrp: > @@ -1063,8 +1080,9 @@ static void rpmsg_remove(struct virtio_device *vdev) > > vdev->config->del_vqs(vrp->vdev); > > - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > - vrp->rbufs, vrp->bufs_dma); > + if (!vrp->ext_buffer) > + dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > + vrp->rbufs, vrp->bufs_dma); > > kfree(vrp); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Suman Anna [mailto:s-anna@ti.com] > Sent: Saturday, January 14, 2017 3:38 AM > To: Loic PALLARDY <loic.pallardy@st.com>; bjorn.andersson@linaro.org; > ohad@wizery.com; lee.jones@linaro.org; Patrice CHOTARD > <patrice.chotard@st.com> > Cc: linux-remoteproc@vger.kernel.org; kernel@stlinux.com > Subject: Re: [PATCH v1 5/6] rpmsg: virtio_rpmsg: don't allocate buffer if > provided by low level driver > > On 12/07/2016 02:35 PM, Loic Pallardy wrote: > > Low level platform specific driver has the knowledge of the different > > communication link constraints like fixed or secured memory region to > > use for buffer allocation. > > > > If virtual address is defined in virtio_rpmsg_cfg structure, a > > dedicated memory pool buffer fitting platform requirements is available. > > Rpmsg virtio layer should rely on it if its size is compliant with > > link characteristics. > > > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > > --- > > drivers/rpmsg/virtio_rpmsg_bus.c | 54 > > ++++++++++++++++++++++++++-------------- > > 1 file changed, 36 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c > > b/drivers/rpmsg/virtio_rpmsg_bus.c > > index 1a97af8..b12fe2a 100644 > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > > @@ -57,6 +57,7 @@ > > * @sendq: wait queue of sending contexts waiting for a tx buffers > > * @sleepers: number of senders that are waiting for a tx buffer > > * @ns_ept: the bus's name service endpoint > > + * @ext_buffer: buffer allocated by low level driver > > * > > * This structure stores the rpmsg state of a given virtio remote processor > > * device (there might be several virtio proc devices for each > > physical @@ -76,6 +77,7 @@ struct virtproc_info { > > wait_queue_head_t sendq; > > atomic_t sleepers; > > struct rpmsg_endpoint *ns_ept; > > + bool ext_buffer; > > }; > > > > /* The feature bitmap for virtio rpmsg */ @@ -904,6 +906,17 @@ static > > int virtio_rpmsg_get_config(struct virtio_device *vdev) > > ret = -ENOMEM; > > goto out; > > } > > + > > + /* Level platform specific buffer driver ? */ > > + if (virtio_cfg.va != -1) { > > + vrp->ext_buffer = true; > > + /* half of the buffers is dedicated for RX */ > > + vrp->rbufs = (void *)(uintptr_t)virtio_cfg.va; > > + > > + /* and half is dedicated for TX */ > > + vrp->sbufs = (void *)(uintptr_t) virtio_cfg.va + > total_buf_space / 2; > > + } > > + > > return !ret; > > } > > out: > > @@ -960,24 +973,27 @@ static int rpmsg_probe(struct virtio_device > *vdev) > > if (err < 0) > > goto free_vrp; > > > > - total_buf_space = vrp->num_bufs * vrp->buf_size; > > + /* Allocate buffer if none provided by low level platform driver */ > > + if (!vrp->ext_buffer) { > > + total_buf_space = vrp->num_bufs * vrp->buf_size; > > > > - bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, > > - total_buf_space, &vrp->bufs_dma, > > - GFP_KERNEL); > > - if (!bufs_va) { > > - err = -ENOMEM; > > - goto vqs_del; > > - } > > + bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, > > + total_buf_space, &vrp->bufs_dma, > > + GFP_KERNEL); > > Can you run checkpatch with --strict when you resubmit the series, I believe > a few have been introduced that weren't there before. Sure I will. Regards, Loic > > regards > Suman > > > + if (!bufs_va) { > > + err = -ENOMEM; > > + goto vqs_del; > > + } > > > > - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > > - bufs_va, &vrp->bufs_dma); > > + dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > > + bufs_va, &vrp->bufs_dma); > > > > - /* half of the buffers is dedicated for RX */ > > - vrp->rbufs = bufs_va; > > + /* half of the buffers is dedicated for RX */ > > + vrp->rbufs = bufs_va; > > > > - /* and half is dedicated for TX */ > > - vrp->sbufs = bufs_va + total_buf_space / 2; > > + /* and half is dedicated for TX */ > > + vrp->sbufs = bufs_va + total_buf_space / 2; > > + } > > > > /* set up the receive buffers */ > > for (i = 0; i < vrp->num_bufs / 2; i++) { @@ -1028,8 +1044,9 @@ > > static int rpmsg_probe(struct virtio_device *vdev) > > return 0; > > > > free_coherent: > > - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > > - bufs_va, vrp->bufs_dma); > > + if (!vrp->ext_buffer) > > + dma_free_coherent(vdev->dev.parent->parent, > total_buf_space, > > + bufs_va, vrp->bufs_dma); > > vqs_del: > > vdev->config->del_vqs(vrp->vdev); > > free_vrp: > > @@ -1063,8 +1080,9 @@ static void rpmsg_remove(struct virtio_device > > *vdev) > > > > vdev->config->del_vqs(vrp->vdev); > > > > - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, > > - vrp->rbufs, vrp->bufs_dma); > > + if (!vrp->ext_buffer) > > + dma_free_coherent(vdev->dev.parent->parent, > total_buf_space, > > + vrp->rbufs, vrp->bufs_dma); > > > > kfree(vrp); > > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index 1a97af8..b12fe2a 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -57,6 +57,7 @@ * @sendq: wait queue of sending contexts waiting for a tx buffers * @sleepers: number of senders that are waiting for a tx buffer * @ns_ept: the bus's name service endpoint + * @ext_buffer: buffer allocated by low level driver * * This structure stores the rpmsg state of a given virtio remote processor * device (there might be several virtio proc devices for each physical @@ -76,6 +77,7 @@ struct virtproc_info { wait_queue_head_t sendq; atomic_t sleepers; struct rpmsg_endpoint *ns_ept; + bool ext_buffer; }; /* The feature bitmap for virtio rpmsg */ @@ -904,6 +906,17 @@ static int virtio_rpmsg_get_config(struct virtio_device *vdev) ret = -ENOMEM; goto out; } + + /* Level platform specific buffer driver ? */ + if (virtio_cfg.va != -1) { + vrp->ext_buffer = true; + /* half of the buffers is dedicated for RX */ + vrp->rbufs = (void *)(uintptr_t)virtio_cfg.va; + + /* and half is dedicated for TX */ + vrp->sbufs = (void *)(uintptr_t) virtio_cfg.va + total_buf_space / 2; + } + return !ret; } out: @@ -960,24 +973,27 @@ static int rpmsg_probe(struct virtio_device *vdev) if (err < 0) goto free_vrp; - total_buf_space = vrp->num_bufs * vrp->buf_size; + /* Allocate buffer if none provided by low level platform driver */ + if (!vrp->ext_buffer) { + total_buf_space = vrp->num_bufs * vrp->buf_size; - bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, - total_buf_space, &vrp->bufs_dma, - GFP_KERNEL); - if (!bufs_va) { - err = -ENOMEM; - goto vqs_del; - } + bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, + total_buf_space, &vrp->bufs_dma, + GFP_KERNEL); + if (!bufs_va) { + err = -ENOMEM; + goto vqs_del; + } - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", - bufs_va, &vrp->bufs_dma); + dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", + bufs_va, &vrp->bufs_dma); - /* half of the buffers is dedicated for RX */ - vrp->rbufs = bufs_va; + /* half of the buffers is dedicated for RX */ + vrp->rbufs = bufs_va; - /* and half is dedicated for TX */ - vrp->sbufs = bufs_va + total_buf_space / 2; + /* and half is dedicated for TX */ + vrp->sbufs = bufs_va + total_buf_space / 2; + } /* set up the receive buffers */ for (i = 0; i < vrp->num_bufs / 2; i++) { @@ -1028,8 +1044,9 @@ static int rpmsg_probe(struct virtio_device *vdev) return 0; free_coherent: - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, - bufs_va, vrp->bufs_dma); + if (!vrp->ext_buffer) + dma_free_coherent(vdev->dev.parent->parent, total_buf_space, + bufs_va, vrp->bufs_dma); vqs_del: vdev->config->del_vqs(vrp->vdev); free_vrp: @@ -1063,8 +1080,9 @@ static void rpmsg_remove(struct virtio_device *vdev) vdev->config->del_vqs(vrp->vdev); - dma_free_coherent(vdev->dev.parent->parent, total_buf_space, - vrp->rbufs, vrp->bufs_dma); + if (!vrp->ext_buffer) + dma_free_coherent(vdev->dev.parent->parent, total_buf_space, + vrp->rbufs, vrp->bufs_dma); kfree(vrp); }
Low level platform specific driver has the knowledge of the different communication link constraints like fixed or secured memory region to use for buffer allocation. If virtual address is defined in virtio_rpmsg_cfg structure, a dedicated memory pool buffer fitting platform requirements is available. Rpmsg virtio layer should rely on it if its size is compliant with link characteristics. Signed-off-by: Loic Pallardy <loic.pallardy@st.com> --- drivers/rpmsg/virtio_rpmsg_bus.c | 54 ++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 18 deletions(-)