* [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
@ 2018-01-10 13:16 Anup Patel
2018-01-23 13:46 ` Anup Patel
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Anup Patel @ 2018-01-10 13:16 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson
Cc: linux-remoteproc, linux-kernel, Anup Patel, stable
When virtio-rpmsg device is provided via virtio-mmio transport, the
dma_alloc_coherent() (called by rpmsg_probe()) fails on ARM/ARM64
systems because "vdev->dev.parent->parent" device is used as parameter
to dma_alloc_coherent().
The "vdev->dev.parent->parent" device represents underlying remoteproc
platform device when virtio-rpmsg device is provided via virtio-remoteproc
transport. When virtio-rpmsg device is provided via virtio-mmio transport,
the "vdev->dev.parent->parent" device represents the parent device of
virtio-mmio platform device and dma_alloc_coherent() fails for this device
because generally there is no corresponding platform device and dma_ops
are not setup for "vdev->dev.parent->parent".
This patch fixes dma_alloc_coherent() usage in rpmsg_probe() by trying
dma_alloc_coherent() with "vdev->dev.parent" device when it fails with
"vdev->dev.parent->parent" device.
Fixes: b5ab5e24e960 ("remoteproc: maintain a generic child device for
each rproc")
Signed-off-by: Anup Patel <anup@brainfault.org>
Cc: stable@vger.kernel.org
---
drivers/rpmsg/virtio_rpmsg_bus.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 82b8300..7f8710a 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -64,6 +64,7 @@
struct virtproc_info {
struct virtio_device *vdev;
struct virtqueue *rvq, *svq;
+ struct device *bufs_dev;
void *rbufs, *sbufs;
unsigned int num_bufs;
unsigned int buf_size;
@@ -924,9 +925,16 @@ static int rpmsg_probe(struct virtio_device *vdev)
total_buf_space, &vrp->bufs_dma,
GFP_KERNEL);
if (!bufs_va) {
- err = -ENOMEM;
- goto vqs_del;
- }
+ bufs_va = dma_alloc_coherent(vdev->dev.parent,
+ total_buf_space, &vrp->bufs_dma,
+ GFP_KERNEL);
+ if (!bufs_va) {
+ err = -ENOMEM;
+ goto vqs_del;
+ } else
+ vrp->bufs_dev = vdev->dev.parent;
+ } else
+ vrp->bufs_dev = vdev->dev.parent->parent;
dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
bufs_va, &vrp->bufs_dma);
@@ -988,7 +996,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
return 0;
free_coherent:
- dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+ dma_free_coherent(vrp->bufs_dev, total_buf_space,
bufs_va, vrp->bufs_dma);
vqs_del:
vdev->config->del_vqs(vrp->vdev);
@@ -1023,7 +1031,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
vdev->config->del_vqs(vrp->vdev);
- dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+ dma_free_coherent(vrp->bufs_dev, total_buf_space,
vrp->rbufs, vrp->bufs_dma);
kfree(vrp);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
2018-01-10 13:16 [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport Anup Patel
@ 2018-01-23 13:46 ` Anup Patel
2018-02-06 4:44 ` Anup Patel
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2018-01-23 13:46 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson
Cc: linux-remoteproc, linux-kernel@vger.kernel.org List, Anup Patel, stable
On Wed, Jan 10, 2018 at 6:46 PM, Anup Patel <anup@brainfault.org> wrote:
> When virtio-rpmsg device is provided via virtio-mmio transport, the
> dma_alloc_coherent() (called by rpmsg_probe()) fails on ARM/ARM64
> systems because "vdev->dev.parent->parent" device is used as parameter
> to dma_alloc_coherent().
>
> The "vdev->dev.parent->parent" device represents underlying remoteproc
> platform device when virtio-rpmsg device is provided via virtio-remoteproc
> transport. When virtio-rpmsg device is provided via virtio-mmio transport,
> the "vdev->dev.parent->parent" device represents the parent device of
> virtio-mmio platform device and dma_alloc_coherent() fails for this device
> because generally there is no corresponding platform device and dma_ops
> are not setup for "vdev->dev.parent->parent".
>
> This patch fixes dma_alloc_coherent() usage in rpmsg_probe() by trying
> dma_alloc_coherent() with "vdev->dev.parent" device when it fails with
> "vdev->dev.parent->parent" device.
>
> Fixes: b5ab5e24e960 ("remoteproc: maintain a generic child device for
> each rproc")
>
> Signed-off-by: Anup Patel <anup@brainfault.org>
> Cc: stable@vger.kernel.org
> ---
> drivers/rpmsg/virtio_rpmsg_bus.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 82b8300..7f8710a 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -64,6 +64,7 @@
> struct virtproc_info {
> struct virtio_device *vdev;
> struct virtqueue *rvq, *svq;
> + struct device *bufs_dev;
> void *rbufs, *sbufs;
> unsigned int num_bufs;
> unsigned int buf_size;
> @@ -924,9 +925,16 @@ static int rpmsg_probe(struct virtio_device *vdev)
> total_buf_space, &vrp->bufs_dma,
> GFP_KERNEL);
> if (!bufs_va) {
> - err = -ENOMEM;
> - goto vqs_del;
> - }
> + bufs_va = dma_alloc_coherent(vdev->dev.parent,
> + total_buf_space, &vrp->bufs_dma,
> + GFP_KERNEL);
> + if (!bufs_va) {
> + err = -ENOMEM;
> + goto vqs_del;
> + } else
> + vrp->bufs_dev = vdev->dev.parent;
> + } else
> + vrp->bufs_dev = vdev->dev.parent->parent;
>
> dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
> bufs_va, &vrp->bufs_dma);
> @@ -988,7 +996,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
> return 0;
>
> free_coherent:
> - dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
> + dma_free_coherent(vrp->bufs_dev, total_buf_space,
> bufs_va, vrp->bufs_dma);
> vqs_del:
> vdev->config->del_vqs(vrp->vdev);
> @@ -1023,7 +1031,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
>
> vdev->config->del_vqs(vrp->vdev);
>
> - dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
> + dma_free_coherent(vrp->bufs_dev, total_buf_space,
> vrp->rbufs, vrp->bufs_dma);
>
> kfree(vrp);
> --
> 2.7.4
>
Hi All,
Any comments on this??
Regards,
Anup
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
2018-01-10 13:16 [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport Anup Patel
2018-01-23 13:46 ` Anup Patel
@ 2018-02-06 4:44 ` Anup Patel
2018-02-21 11:40 ` Anup Patel
2018-03-18 22:47 ` Bjorn Andersson
3 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2018-02-06 4:44 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson
Cc: linux-remoteproc, linux-kernel@vger.kernel.org List, Anup Patel, stable
Hi Bjorn,
Can you please have a look at this patch?
Regards,
Anup
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
2018-01-10 13:16 [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport Anup Patel
2018-01-23 13:46 ` Anup Patel
2018-02-06 4:44 ` Anup Patel
@ 2018-02-21 11:40 ` Anup Patel
2018-03-18 22:47 ` Bjorn Andersson
3 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2018-02-21 11:40 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson
Cc: linux-remoteproc, linux-kernel@vger.kernel.org List, Anup Patel, stable
On Wed, Jan 10, 2018 at 6:46 PM, Anup Patel <anup@brainfault.org> wrote:
> When virtio-rpmsg device is provided via virtio-mmio transport, the
> dma_alloc_coherent() (called by rpmsg_probe()) fails on ARM/ARM64
> systems because "vdev->dev.parent->parent" device is used as parameter
> to dma_alloc_coherent().
>
> The "vdev->dev.parent->parent" device represents underlying remoteproc
> platform device when virtio-rpmsg device is provided via virtio-remoteproc
> transport. When virtio-rpmsg device is provided via virtio-mmio transport,
> the "vdev->dev.parent->parent" device represents the parent device of
> virtio-mmio platform device and dma_alloc_coherent() fails for this device
> because generally there is no corresponding platform device and dma_ops
> are not setup for "vdev->dev.parent->parent".
>
> This patch fixes dma_alloc_coherent() usage in rpmsg_probe() by trying
> dma_alloc_coherent() with "vdev->dev.parent" device when it fails with
> "vdev->dev.parent->parent" device.
>
> Fixes: b5ab5e24e960 ("remoteproc: maintain a generic child device for
> each rproc")
>
> Signed-off-by: Anup Patel <anup@brainfault.org>
> Cc: stable@vger.kernel.org
> ---
> drivers/rpmsg/virtio_rpmsg_bus.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 82b8300..7f8710a 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -64,6 +64,7 @@
> struct virtproc_info {
> struct virtio_device *vdev;
> struct virtqueue *rvq, *svq;
> + struct device *bufs_dev;
> void *rbufs, *sbufs;
> unsigned int num_bufs;
> unsigned int buf_size;
> @@ -924,9 +925,16 @@ static int rpmsg_probe(struct virtio_device *vdev)
> total_buf_space, &vrp->bufs_dma,
> GFP_KERNEL);
> if (!bufs_va) {
> - err = -ENOMEM;
> - goto vqs_del;
> - }
> + bufs_va = dma_alloc_coherent(vdev->dev.parent,
> + total_buf_space, &vrp->bufs_dma,
> + GFP_KERNEL);
> + if (!bufs_va) {
> + err = -ENOMEM;
> + goto vqs_del;
> + } else
> + vrp->bufs_dev = vdev->dev.parent;
> + } else
> + vrp->bufs_dev = vdev->dev.parent->parent;
>
> dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
> bufs_va, &vrp->bufs_dma);
> @@ -988,7 +996,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
> return 0;
>
> free_coherent:
> - dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
> + dma_free_coherent(vrp->bufs_dev, total_buf_space,
> bufs_va, vrp->bufs_dma);
> vqs_del:
> vdev->config->del_vqs(vrp->vdev);
> @@ -1023,7 +1031,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
>
> vdev->config->del_vqs(vrp->vdev);
>
> - dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
> + dma_free_coherent(vrp->bufs_dev, total_buf_space,
> vrp->rbufs, vrp->bufs_dma);
>
> kfree(vrp);
> --
> 2.7.4
>
Hi Bjorn,
Can you please review this patch?
Regards,
Anup
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
2018-01-10 13:16 [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport Anup Patel
` (2 preceding siblings ...)
2018-02-21 11:40 ` Anup Patel
@ 2018-03-18 22:47 ` Bjorn Andersson
2018-03-21 8:04 ` Anup Patel
3 siblings, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2018-03-18 22:47 UTC (permalink / raw)
To: Anup Patel; +Cc: Ohad Ben-Cohen, linux-remoteproc, linux-kernel, stable
On Wed 10 Jan 05:16 PST 2018, Anup Patel wrote:
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
[..]
> @@ -924,9 +925,16 @@ static int rpmsg_probe(struct virtio_device *vdev)
> total_buf_space, &vrp->bufs_dma,
> GFP_KERNEL);
> if (!bufs_va) {
> - err = -ENOMEM;
> - goto vqs_del;
> - }
> + bufs_va = dma_alloc_coherent(vdev->dev.parent,
> + total_buf_space, &vrp->bufs_dma,
> + GFP_KERNEL);
> + if (!bufs_va) {
> + err = -ENOMEM;
> + goto vqs_del;
> + } else
> + vrp->bufs_dev = vdev->dev.parent;
> + } else
> + vrp->bufs_dev = vdev->dev.parent->parent;
I really don't fancy the idea of us allocating on behalf of our
grandparent here, as you show it's not certain that our grandparent is
what someone originally expected it to be.
With the purpose of being able to control these allocations there is an
ongoing discussion related to this, which I believe will result in this
being changed to at least vdev->dev.parent..
I do expect that this discussion will be brought up during Linaro
Connect the coming week.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport
2018-03-18 22:47 ` Bjorn Andersson
@ 2018-03-21 8:04 ` Anup Patel
0 siblings, 0 replies; 6+ messages in thread
From: Anup Patel @ 2018-03-21 8:04 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ohad Ben-Cohen, linux-remoteproc,
linux-kernel@vger.kernel.org List, stable
On Mon, Mar 19, 2018 at 4:17 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 10 Jan 05:16 PST 2018, Anup Patel wrote:
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> [..]
>> @@ -924,9 +925,16 @@ static int rpmsg_probe(struct virtio_device *vdev)
>> total_buf_space, &vrp->bufs_dma,
>> GFP_KERNEL);
>> if (!bufs_va) {
>> - err = -ENOMEM;
>> - goto vqs_del;
>> - }
>> + bufs_va = dma_alloc_coherent(vdev->dev.parent,
>> + total_buf_space, &vrp->bufs_dma,
>> + GFP_KERNEL);
>> + if (!bufs_va) {
>> + err = -ENOMEM;
>> + goto vqs_del;
>> + } else
>> + vrp->bufs_dev = vdev->dev.parent;
>> + } else
>> + vrp->bufs_dev = vdev->dev.parent->parent;
>
> I really don't fancy the idea of us allocating on behalf of our
> grandparent here, as you show it's not certain that our grandparent is
> what someone originally expected it to be.
>
> With the purpose of being able to control these allocations there is an
> ongoing discussion related to this, which I believe will result in this
> being changed to at least vdev->dev.parent..
>
>
> I do expect that this discussion will be brought up during Linaro
> Connect the coming week.
>
Currently, rpmsg_probe() is broken for virtio-mmio transport
hence I send this patch as a stable fix.
In general, I am fine if we are eventually going towards
vdev->dev.parent usage.
Regards,
Anup
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-21 8:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 13:16 [PATCH RESEND] rpmsg: virtio_rpmsg_bus: fix rpmsg_probe() for virtio-mmio transport Anup Patel
2018-01-23 13:46 ` Anup Patel
2018-02-06 4:44 ` Anup Patel
2018-02-21 11:40 ` Anup Patel
2018-03-18 22:47 ` Bjorn Andersson
2018-03-21 8:04 ` Anup Patel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.