[PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Michael Grzeschik]

For dmabuf import users to be able to use the vaddr from another
videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
vb2_dma_sg_dmabuf_ops_vmap callback.

This patch adds vmap on map if buf->vaddr was not set, and also
adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
afterwards.

Cc: stable@kernel.org
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v2 -> v3: resend as a single patch
v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram

 .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 28f3fdfe23a298..05b43edda94a7e 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
 {
 	struct vb2_dma_sg_buf *buf = dbuf->priv;
 
+	if (!buf->vaddr)
+		buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
+				  PAGE_KERNEL);
+
 	iosys_map_set_vaddr(map, buf->vaddr);
 
 	return 0;
 }
 
+static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
+				      struct iosys_map *map)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+
+	if (buf->vaddr)
+		vunmap(buf->vaddr);
+
+	buf->vaddr = NULL;
+}
+
 static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
 	struct vm_area_struct *vma)
 {
@@ -508,6 +523,7 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
 	.begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access,
 	.end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access,
 	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
+	.vunmap = vb2_dma_sg_dmabuf_ops_vunmap,
 	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
 	.release = vb2_dma_sg_dmabuf_ops_release,
 };
-- 
2.39.2

Re: [PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Tomasz Figa]

Hi Michael,


On Wed, Nov 1, 2023 at 8:04 AM Michael Grzeschik
<m.grzeschik@pengutronix.de> wrote:
>
> For dmabuf import users to be able to use the vaddr from another
> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> vb2_dma_sg_dmabuf_ops_vmap callback.
>
> This patch adds vmap on map if buf->vaddr was not set, and also
> adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
> afterwards.
>
> Cc: stable@kernel.org
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>
> ---
> v2 -> v3: resend as a single patch
> v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
>

First of all, thanks for the patch!

Please check my comments inline.

>  .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 28f3fdfe23a298..05b43edda94a7e 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>  {
>         struct vb2_dma_sg_buf *buf = dbuf->priv;
>
> +       if (!buf->vaddr)
> +               buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
> +                                 PAGE_KERNEL);
> +

Would it make sense to use vb2_dma_sg_vaddr() instead of open coding
the mapping again?

>         iosys_map_set_vaddr(map, buf->vaddr);
>
>         return 0;
>  }
>
> +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
> +                                     struct iosys_map *map)
> +{
> +       struct vb2_dma_sg_buf *buf = dbuf->priv;
> +
> +       if (buf->vaddr)
> +               vunmap(buf->vaddr);
> +
> +       buf->vaddr = NULL;
> +}

This could be potentially dangerous. Please consider the situation
when the exporting V4L2 driver needs the CPU mapping for its own
usage. It would call vb2_dma_sg_vaddr(), which would return the
mapping. Then the importer could call vunmap, which would destroy the
mapping that is still in use by the exporter internally.

The idea of the current implementation is that we just create a kernel
mapping when it's needed first and just keep it around until the
entire buffer is destroyed.

Best regards,
Tomasz

Re: [PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Hans Verkuil]

Tomasz,

Can you review this?

Michael, I have one comment below:

On 01/11/2023 00:04, Michael Grzeschik wrote:
> For dmabuf import users to be able to use the vaddr from another
> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> vb2_dma_sg_dmabuf_ops_vmap callback.
> 
> This patch adds vmap on map if buf->vaddr was not set, and also
> adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
> afterwards.
> 
> Cc: stable@kernel.org
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> ---
> v2 -> v3: resend as a single patch
> v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
> 
>  .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 28f3fdfe23a298..05b43edda94a7e 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>  {
>  	struct vb2_dma_sg_buf *buf = dbuf->priv;
>  
> +	if (!buf->vaddr)
> +		buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
> +				  PAGE_KERNEL);

Shouldn't this check for success and return an error code if it fails?

Regards,

	Hans

> +
>  	iosys_map_set_vaddr(map, buf->vaddr);
>  
>  	return 0;
>  }
>  
> +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
> +				      struct iosys_map *map)
> +{
> +	struct vb2_dma_sg_buf *buf = dbuf->priv;
> +
> +	if (buf->vaddr)
> +		vunmap(buf->vaddr);
> +
> +	buf->vaddr = NULL;
> +}
> +
>  static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
>  	struct vm_area_struct *vma)
>  {
> @@ -508,6 +523,7 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
>  	.begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access,
>  	.end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access,
>  	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
> +	.vunmap = vb2_dma_sg_dmabuf_ops_vunmap,
>  	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
>  	.release = vb2_dma_sg_dmabuf_ops_release,
>  };

Re: [PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Tomasz Figa]

Hi Hans,


On Mon, Nov 20, 2023 at 10:27 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Tomasz,
>
> Can you review this?

I did review it a few weeks ago, with some comments pending:
https://patchwork.kernel.org/project/linux-media/patch/20231031230435.3356103-1-m.grzeschik@pengutronix.de/#25577798

Best,
Tomasz

>
> Michael, I have one comment below:
>
> On 01/11/2023 00:04, Michael Grzeschik wrote:
> > For dmabuf import users to be able to use the vaddr from another
> > videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> > vb2_dma_sg_dmabuf_ops_vmap callback.
> >
> > This patch adds vmap on map if buf->vaddr was not set, and also
> > adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
> > afterwards.
> >
> > Cc: stable@kernel.org
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> >
> > ---
> > v2 -> v3: resend as a single patch
> > v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
> >
> >  .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index 28f3fdfe23a298..05b43edda94a7e 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
> >  {
> >       struct vb2_dma_sg_buf *buf = dbuf->priv;
> >
> > +     if (!buf->vaddr)
> > +             buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
> > +                               PAGE_KERNEL);
>
> Shouldn't this check for success and return an error code if it fails?
>
> Regards,
>
>         Hans
>
> > +
> >       iosys_map_set_vaddr(map, buf->vaddr);
> >
> >       return 0;
> >  }
> >
> > +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
> > +                                   struct iosys_map *map)
> > +{
> > +     struct vb2_dma_sg_buf *buf = dbuf->priv;
> > +
> > +     if (buf->vaddr)
> > +             vunmap(buf->vaddr);
> > +
> > +     buf->vaddr = NULL;
> > +}
> > +
> >  static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
> >       struct vm_area_struct *vma)
> >  {
> > @@ -508,6 +523,7 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
> >       .begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access,
> >       .end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access,
> >       .vmap = vb2_dma_sg_dmabuf_ops_vmap,
> > +     .vunmap = vb2_dma_sg_dmabuf_ops_vunmap,
> >       .mmap = vb2_dma_sg_dmabuf_ops_mmap,
> >       .release = vb2_dma_sg_dmabuf_ops_release,
> >  };
>

Re: [PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Michael Grzeschik]

[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]

On Wed, Nov 01, 2023 at 12:48:21PM +0900, Tomasz Figa wrote:
>Hi Michael,
>
>
>On Wed, Nov 1, 2023 at 8:04 AM Michael Grzeschik
><m.grzeschik@pengutronix.de> wrote:
>>
>> For dmabuf import users to be able to use the vaddr from another
>> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
>> vb2_dma_sg_dmabuf_ops_vmap callback.
>>
>> This patch adds vmap on map if buf->vaddr was not set, and also
>> adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
>> afterwards.
>>
>> Cc: stable@kernel.org
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>
>> ---
>> v2 -> v3: resend as a single patch
>> v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
>>
>
>First of all, thanks for the patch!
>
>Please check my comments inline.
>
>>  .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>> index 28f3fdfe23a298..05b43edda94a7e 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>> @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>>  {
>>         struct vb2_dma_sg_buf *buf = dbuf->priv;
>>
>> +       if (!buf->vaddr)
>> +               buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
>> +                                 PAGE_KERNEL);
>> +
>
>Would it make sense to use vb2_dma_sg_vaddr() instead of open coding
>the mapping again?
>

So in the end the whole patch would look like this:

	buf->vaddr = vb2_dma_sg_vaddr(NULL, buf);
	if (!buf->vaddr)
		return -ENOMEM;

>>         iosys_map_set_vaddr(map, buf->vaddr);
>>
>>         return 0;
>>  }
>>
>> +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
>> +                                     struct iosys_map *map)
>> +{
>> +       struct vb2_dma_sg_buf *buf = dbuf->priv;
>> +
>> +       if (buf->vaddr)
>> +               vunmap(buf->vaddr);
>> +
>> +       buf->vaddr = NULL;
>> +}
>
>This could be potentially dangerous. Please consider the situation
>when the exporting V4L2 driver needs the CPU mapping for its own
>usage. It would call vb2_dma_sg_vaddr(), which would return the
>mapping. Then the importer could call vunmap, which would destroy the
>mapping that is still in use by the exporter internally.
>
>The idea of the current implementation is that we just create a kernel
>mapping when it's needed first and just keep it around until the
>entire buffer is destroyed.

In this patch I will drop the while vunmap callback then.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Tomasz Figa]

On Tue, Nov 21, 2023 at 10:30 PM Michael Grzeschik <mgr@pengutronix.de> wrote:
>
> On Wed, Nov 01, 2023 at 12:48:21PM +0900, Tomasz Figa wrote:
> >Hi Michael,
> >
> >
> >On Wed, Nov 1, 2023 at 8:04 AM Michael Grzeschik
> ><m.grzeschik@pengutronix.de> wrote:
> >>
> >> For dmabuf import users to be able to use the vaddr from another
> >> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> >> vb2_dma_sg_dmabuf_ops_vmap callback.
> >>
> >> This patch adds vmap on map if buf->vaddr was not set, and also
> >> adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
> >> afterwards.
> >>
> >> Cc: stable@kernel.org
> >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> >>
> >> ---
> >> v2 -> v3: resend as a single patch
> >> v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
> >>
> >
> >First of all, thanks for the patch!
> >
> >Please check my comments inline.
> >
> >>  .../media/common/videobuf2/videobuf2-dma-sg.c    | 16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> >> index 28f3fdfe23a298..05b43edda94a7e 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> >> @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
> >>  {
> >>         struct vb2_dma_sg_buf *buf = dbuf->priv;
> >>
> >> +       if (!buf->vaddr)
> >> +               buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
> >> +                                 PAGE_KERNEL);
> >> +
> >
> >Would it make sense to use vb2_dma_sg_vaddr() instead of open coding
> >the mapping again?
> >
>
> So in the end the whole patch would look like this:
>
>         buf->vaddr = vb2_dma_sg_vaddr(NULL, buf);
>         if (!buf->vaddr)
>                 return -ENOMEM;

Yes, but buf->vaddr is already updated by vb2_dma_sg_vaddr(), so we
should just save the return value to a local variable.

>
> >>         iosys_map_set_vaddr(map, buf->vaddr);
> >>
> >>         return 0;
> >>  }
> >>
> >> +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
> >> +                                     struct iosys_map *map)
> >> +{
> >> +       struct vb2_dma_sg_buf *buf = dbuf->priv;
> >> +
> >> +       if (buf->vaddr)
> >> +               vunmap(buf->vaddr);
> >> +
> >> +       buf->vaddr = NULL;
> >> +}
> >
> >This could be potentially dangerous. Please consider the situation
> >when the exporting V4L2 driver needs the CPU mapping for its own
> >usage. It would call vb2_dma_sg_vaddr(), which would return the
> >mapping. Then the importer could call vunmap, which would destroy the
> >mapping that is still in use by the exporter internally.
> >
> >The idea of the current implementation is that we just create a kernel
> >mapping when it's needed first and just keep it around until the
> >entire buffer is destroyed.
>
> In this patch I will drop the while vunmap callback then.

Ack.

I think in the long term we may want to rework vb2_plane_vaddr()
semantics so that the mapping can be reference counted, but that would
require quite a bit of a change around the vb2 framework and existing
drivers. (Has been on my list for a while...)

Best regards,
Tomasz

[PATCH v4] media: videobuf2-dma-sg: fix vmap callback

[Michael Grzeschik]

For dmabuf import users to be able to use the vaddr from another
videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
vb2_dma_sg_dmabuf_ops_vmap callback. This patch adds vmap on map if
buf->vaddr was not set.

Cc: stable@kernel.org
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v3 -> v4: drop vunmap function and make use of vb2_dma_sg_vaddr in vmap callback
v2 -> v3: resend as a single patch
v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram

 drivers/media/common/videobuf2/videobuf2-dma-sg.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 28f3fdfe23a298..6975a71d740f6d 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -487,9 +487,15 @@ vb2_dma_sg_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf,
 static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
 				      struct iosys_map *map)
 {
-	struct vb2_dma_sg_buf *buf = dbuf->priv;
+	struct vb2_dma_sg_buf *buf;
+	void *vaddr;
+
+	buf = dbuf->priv;
+	vaddr = vb2_dma_sg_vaddr(buf->vb, buf);
+	if (!vaddr)
+		return -EINVAL;
 
-	iosys_map_set_vaddr(map, buf->vaddr);
+	iosys_map_set_vaddr(map, vaddr);
 
 	return 0;
 }
-- 
2.39.2

Re: [PATCH v4] media: videobuf2-dma-sg: fix vmap callback

[Tomasz Figa]

On Fri, Nov 24, 2023 at 7:32 AM Michael Grzeschik
<m.grzeschik@pengutronix.de> wrote:
>
> For dmabuf import users to be able to use the vaddr from another
> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> vb2_dma_sg_dmabuf_ops_vmap callback. This patch adds vmap on map if
> buf->vaddr was not set.
>
> Cc: stable@kernel.org
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>
> ---
> v3 -> v4: drop vunmap function and make use of vb2_dma_sg_vaddr in vmap callback
> v2 -> v3: resend as a single patch
> v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram
>
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 28f3fdfe23a298..6975a71d740f6d 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -487,9 +487,15 @@ vb2_dma_sg_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf,
>  static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>                                       struct iosys_map *map)
>  {
> -       struct vb2_dma_sg_buf *buf = dbuf->priv;
> +       struct vb2_dma_sg_buf *buf;
> +       void *vaddr;
> +
> +       buf = dbuf->priv;
> +       vaddr = vb2_dma_sg_vaddr(buf->vb, buf);
> +       if (!vaddr)
> +               return -EINVAL;
>
> -       iosys_map_set_vaddr(map, buf->vaddr);
> +       iosys_map_set_vaddr(map, vaddr);
>
>         return 0;
>  }
> --
> 2.39.2
>

Thanks for addressing the comments.

Acked-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz