cron job: media_tree daily build: ERRORS

2018-03-23 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sat Mar 24 05:00:13 CET 2018
media-tree git hash:6ccd228e0cfce2a4f44558422d25c60fcb1a6710
media_build git hash:   e95b7e6bfea396f9dfb1ff7d4d6b95ecacd53d3d
v4l-utils git hash: 098e402950fd45b5a572cccfe1d103661d418417
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.101-i686: ERRORS
linux-3.0.101-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.100-i686: ERRORS
linux-3.2.100-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.113-i686: ERRORS
linux-3.4.113-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.10-i686: ERRORS
linux-3.7.10-x86_64: ERRORS
linux-3.8.13-i686: ERRORS
linux-3.8.13-x86_64: ERRORS
linux-3.9.11-i686: ERRORS
linux-3.9.11-x86_64: ERRORS
linux-3.10.108-i686: ERRORS
linux-3.10.108-x86_64: ERRORS
linux-3.11.10-i686: ERRORS
linux-3.11.10-x86_64: ERRORS
linux-3.12.74-i686: ERRORS
linux-3.12.74-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.79-i686: ERRORS
linux-3.14.79-x86_64: ERRORS
linux-3.15.10-i686: ERRORS
linux-3.15.10-x86_64: ERRORS
linux-3.16.55-i686: ERRORS
linux-3.16.55-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.100-i686: ERRORS
linux-3.18.100-x86_64: ERRORS
linux-3.19.8-i686: ERRORS
linux-3.19.8-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.50-i686: ERRORS
linux-4.1.50-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.99-i686: ERRORS
linux-4.4.99-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.10-i686: ERRORS
linux-4.7.10-x86_64: ERRORS
linux-4.8.17-i686: ERRORS
linux-4.8.17-x86_64: ERRORS
linux-4.9.87-i686: ERRORS
linux-4.9.87-x86_64: ERRORS
linux-4.10.17-i686: ERRORS
linux-4.10.17-x86_64: ERRORS
linux-4.11.12-i686: ERRORS
linux-4.11.12-x86_64: ERRORS
linux-4.12.14-i686: ERRORS
linux-4.12.14-x86_64: ERRORS
linux-4.13.16-i686: WARNINGS
linux-4.13.16-x86_64: WARNINGS
linux-4.14.27-i686: WARNINGS
linux-4.14.27-x86_64: WARNINGS
linux-4.15.10-i686: WARNINGS
linux-4.15.10-x86_64: WARNINGS
linux-4.16-rc5-i686: WARNINGS
linux-4.16-rc5-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH] [media] vcodec: fix error return value from mtk_jpeg_clk_init()

2018-03-23 Thread Rick Chang
On Fri, 2018-03-23 at 11:44 +0800, Ryder Lee wrote:
> The error return value should be fixed as it may return EPROBE_DEFER.
> 
> Cc: Rick Chang 
> Cc: Bin Liu 
> Signed-off-by: Ryder Lee 
> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 226f908..af17aaa 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -1081,11 +1081,11 @@ static int mtk_jpeg_clk_init(struct
> mtk_jpeg_dev *jpeg)
>  
>   jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
>   if (IS_ERR(jpeg->clk_jdec))
> - return -EINVAL;
> + return PTR_ERR(jpeg->clk_jdec);
>  
>   jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi");
>   if (IS_ERR(jpeg->clk_jdec_smi))
> - return -EINVAL;
> + return PTR_ERR(jpeg->clk_jdec_smi);
>  
>   return 0;
>  }

Acked-by: Rick Chang 


Re: [PATCH v2] venus: vdec: fix format enumeration

2018-03-23 Thread Stanimir Varbanov

Hi Hans,

Could you take this patch it not too late.

On 20.03.2018 15:42, Stanimir Varbanov wrote:

Hi Alex,

Thanks!

On 03/19/2018 11:32 AM, Alexandre Courbot wrote:

find_format_by_index() stops enumerating formats as soon as the index
matches, and returns NULL if venus_helper_check_codec() finds out that
the format is not supported. This prevents formats to be properly
enumerated if a non-supported format is present, as the enumeration will
end with it.

Fix this by moving the call to venus_helper_check_codec() into the loop,
and keep enumerating when it fails.

Fixes: 29f0133ec6 media: venus: use helper function to check supported codecs

Signed-off-by: Alexandre Courbot 
---
  drivers/media/platform/qcom/venus/vdec.c | 13 +++--
  drivers/media/platform/qcom/venus/venc.c | 13 +++--
  2 files changed, 14 insertions(+), 12 deletions(-)


Acked-by: Stanimir Varbanov 



regards,
Stan


Re: [RFCv2] Request API

2018-03-23 Thread Sakari Ailus
Hi Hans,

Thanks for writing this. I generally agree with the RFC to the level of
detail available here; a few comments below.

On Fri, Mar 23, 2018 at 09:46:00AM +0100, Hans Verkuil wrote:
> RFC Request API, version 2
> --
> 
> This document proposes the public API for handling requests.
> 
> There has been some confusion about how to do this, so this summarizes the
> current approach based on conversations with the various stakeholders today
> (Sakari, Alexandre Courbot, Thomasz Figa and myself).
> 
> 1) Additions to the media API
> 
>Allocate an empty request object:
> 
>#define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)
> 
>This will return a file descriptor representing the request or an error
>if it can't allocate the request.
> 
>If the pointer argument is NULL, then this will just return 0 (if this 
> ioctl
>is implemented) or -ENOTTY otherwise. This can be used to test whether this
>ioctl is supported or not without actually having to allocate a request.
> 
> 2) Operations on the request fd
> 
>You can queue or reinit a request by calling these ioctls on the request 
> fd:
> 
>#define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
>#define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)
> 
>With REINIT you reset the state of the request as if you had just allocated
>it.
> 
>You can poll the request fd to wait for it to complete.

I'm not sure I like having to release a reference (fd) to a request to know
whether or not it succeeded. I put this as an open question on my RFC
patchset.

> 
>To free a request you close the request fd. Note that it may still be in
>use internally, so the internal datastructures have to be refcounted.
> 
>For this initial implementation only buffers and controls are contained
>in a request. This is needed to implement stateless codecs. Supporting
>complex camera pipelines will require more work.
> 
>Requests only contain the changes to the state at request queue time
>relative to the previously queued request(s) or the current hardware state
>if no other requests were queued.
> 
>Once a request is completed it will retain the state at completion
>time.
> 
> 3) To associate a v4l2 buffer with a request the 'reserved' field in struct
>v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
>until the request is queued since the request may contain information that
>is needed to prepare the buffer.
> 
>To indicate that request_fd should be used this flag should be set by
>userspace at QBUF time:
> 
> #define V4L2_BUF_FLAG_REQUEST 0x0080
> 
>This flag will also be returned by the driver to indicate that the buffer
>is associated with a request.
> 
>TBD: what should vb2 return as request_fd value if this flag is set?
>This should act the same as the fence patch series and this is still
>being tweaked so let's wait for that to be merged first, then we can
>finalize this.
> 
> 4) To associate v4l2 controls with a request we take the first of the
>'reserved[2]' array elements in struct v4l2_ext_controls and use it to 
> store
>the request fd.
> 
>We also add a new WHICH value:
> 
> #define V4L2_CTRL_WHICH_REQUEST   0x0f01
> 
>This tells the control framework to get/set controls from the given
>request fd.
> 
>When querying a control value from a request it will return the newest
>value in the list of pending requests, or the current hardware value if
>is not set in any of the pending requests.
> 
>When a request is completed the controls will no longer change. A copy
>will be made of volatile controls at the time of completion (actually
>it will be up to the driver to decide when to do that).
> 
>Volatile controls and requests:
> 
>- If you set a volatile control in a request, then that will be ignored,
>  unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.
> 
>- If you get a volatile control from a request then:
>  1) If the request is completed it will return the value of the volatile
> control at completion time.
>  2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
> set in a request, then that value is returned.
>  3) Otherwise: return the current value from the hardware (i.e. normal
> behavior).
> 
>Read-only controls and requests:
> 
>- If you get a read-only control from a request then:
>  1) If the request is completed it will return the value of the read-only
> control at completion time.
>  2) Otherwise it will get the current value from the driver (i.e. normal
> behavior).
> 
>Open issue: should we receive control events if a control in a request is
>added/changed? Currently there are no plans to support control events for
>requests. I don't see a clear use-case and neither do I see an easy way
>of impl

Re: [PATCH 08/30] media: v4l2-ioctl: fix some "too small" warnings

2018-03-23 Thread Sakari Ailus
Hi Mauro,

On Fri, Mar 23, 2018 at 07:56:54AM -0400, Mauro Carvalho Chehab wrote:
> While the code there is right, it produces three false positives:
>   drivers/media/v4l2-core/v4l2-ioctl.c:2868 video_usercopy() error: 
> copy_from_user() 'parg' too small (128 vs 16383)
>   drivers/media/v4l2-core/v4l2-ioctl.c:2868 video_usercopy() error: 
> copy_from_user() 'parg' too small (128 vs 16383)
>   drivers/media/v4l2-core/v4l2-ioctl.c:2876 video_usercopy() error: 
> memset() 'parg' too small (128 vs 16383)
> 
> Store the ioctl size on a cache var, in order to suppress those.

I have to say I'm not a big fan of changing perfectly fine code in order to
please static analysers. What's this, smatch? I wonder if it could be fixed
instead of changing the code. That'd be presumably a lot more work though.

On naming --- "size" is rather more generic, but it's not a long function
either. I'd be a bit more specific, e.g. ioc_size or arg_size.

> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 672ab22ccd96..a5dab16ff2d2 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2833,14 +2833,15 @@ video_usercopy(struct file *file, unsigned int cmd, 
> unsigned long arg,
>   size_t  array_size = 0;
>   void __user *user_ptr = NULL;
>   void**kernel_ptr = NULL;
> + size_t  size = _IOC_SIZE(cmd);
>  
>   /*  Copy arguments into temp kernel buffer  */
>   if (_IOC_DIR(cmd) != _IOC_NONE) {
> - if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
> + if (size <= sizeof(sbuf)) {
>   parg = sbuf;
>   } else {
>   /* too big to allocate from stack */
> - mbuf = kvmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
> + mbuf = kvmalloc(size, GFP_KERNEL);
>   if (NULL == mbuf)
>   return -ENOMEM;
>   parg = mbuf;
> @@ -2848,7 +2849,7 @@ video_usercopy(struct file *file, unsigned int cmd, 
> unsigned long arg,
>  
>   err = -EFAULT;
>   if (_IOC_DIR(cmd) & _IOC_WRITE) {
> - unsigned int n = _IOC_SIZE(cmd);
> + unsigned int n = size;
>  
>   /*
>* In some cases, only a few fields are used as input,
> @@ -2869,11 +2870,11 @@ video_usercopy(struct file *file, unsigned int cmd, 
> unsigned long arg,
>   goto out;
>  
>   /* zero out anything we don't copy from userspace */
> - if (n < _IOC_SIZE(cmd))
> - memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
> + if (n < size)
> + memset((u8 *)parg + n, 0, size - n);
>   } else {
>   /* read-only ioctl */
> - memset(parg, 0, _IOC_SIZE(cmd));
> + memset(parg, 0, size);
>   }
>   }
>  
> @@ -2931,7 +2932,7 @@ video_usercopy(struct file *file, unsigned int cmd, 
> unsigned long arg,
>   switch (_IOC_DIR(cmd)) {
>   case _IOC_READ:
>   case (_IOC_WRITE | _IOC_READ):
> - if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
> + if (copy_to_user((void __user *)arg, parg, size))
>   err = -EFAULT;
>   break;
>   }

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH 06/30] media: ov5670: get rid of a series of __be warnings

2018-03-23 Thread Sakari Ailus
On Fri, Mar 23, 2018 at 07:56:52AM -0400, Mauro Carvalho Chehab wrote:
> There are some troubles on this driver with respect to the usage
> of __be16 and __b32 macros:
> 
>   drivers/media/i2c/ov5670.c:1857:27: warning: incorrect type in 
> initializer (different base types)
>   drivers/media/i2c/ov5670.c:1857:27:expected unsigned short 
> [unsigned] [usertype] reg_addr_be
>   drivers/media/i2c/ov5670.c:1857:27:got restricted __be16 [usertype] 
> 
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
>   drivers/media/i2c/ov5670.c:1901:13: warning: incorrect type in 
> assignment (different base types)
>   drivers/media/i2c/ov5670.c:1901:13:expected unsigned int [unsigned] 
> [usertype] val
>   drivers/media/i2c/ov5670.c:1901:13:got restricted __be32 [usertype] 
> 
> 
> Fix them.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


[RFC v2 06/10] vb2: Add support for requests

2018-03-23 Thread Sakari Ailus
Associate a buffer to a request when it is queued and disassociate when it
is done.

Signed-off-by: Sakari Ailus 
---
 drivers/media/common/videobuf2/videobuf2-core.c | 43 -
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 40 ++-
 include/media/videobuf2-core.h  | 19 +++
 include/media/videobuf2-v4l2.h  | 28 
 4 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index d3f7bb3..b8535de 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -346,6 +346,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
break;
}
 
+   if (q->class)
+   media_request_object_init(q->class, &vb->req_obj);
+
vb->state = VB2_BUF_STATE_DEQUEUED;
vb->vb2_queue = q;
vb->num_planes = num_planes;
@@ -520,7 +523,10 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
int buffers)
/* Free videobuf buffers */
for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
 ++buffer) {
-   kfree(q->bufs[buffer]);
+   if (q->class)
+   media_request_object_put(&q->bufs[buffer]->req_obj);
+   else
+   kfree(q->bufs[buffer]);
q->bufs[buffer] = NULL;
}
 
@@ -944,6 +950,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
default:
/* Inform any processes that may be waiting for buffers */
wake_up(&q->done_wq);
+   if (vb->req_ref) {
+   media_request_ref_complete(vb->req_ref);
+   vb->req_ref = NULL;
+   }
break;
}
 }
@@ -1249,6 +1259,32 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
return -EIO;
}
 
+   if (vb->request_fd) {
+   struct media_request *req;
+   struct media_request_ref *ref;
+
+   if (!q->class) {
+   dprintk(1, "requests not enabled for the queue\n");
+   return -EINVAL;
+   }
+
+   req = media_request_find(q->class->mdev, vb->request_fd);
+   if (IS_ERR(req)) {
+   dprintk(1, "no request found for fd %d (%ld)\n",
+   vb->request_fd, PTR_ERR(req));
+   return PTR_ERR(req);
+   }
+
+   ref = media_request_object_bind(req,
+   &q->bufs[vb->index]->req_obj);
+   media_request_put(req);
+
+   if (IS_ERR(ref))
+   return PTR_ERR(ref);
+
+   vb->req_ref = ref;
+   }
+
vb->state = VB2_BUF_STATE_PREPARING;
 
switch (q->memory) {
@@ -1269,6 +1305,8 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
if (ret) {
dprintk(1, "buffer preparation failed: %d\n", ret);
vb->state = VB2_BUF_STATE_DEQUEUED;
+   media_request_ref_unbind(vb->req_ref);
+   vb->req_ref = NULL;
return ret;
}
 
@@ -2037,6 +2075,9 @@ void vb2_core_queue_release(struct vb2_queue *q)
mutex_lock(&q->mmap_lock);
__vb2_queue_free(q, q->num_buffers);
mutex_unlock(&q->mmap_lock);
+   media_request_class_unregister(q->class);
+   kfree(q->class);
+   q->class = NULL;
 }
 EXPORT_SYMBOL_GPL(vb2_core_queue_release);
 
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 6d4d184..30047b9 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -196,6 +196,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->index = vb->index;
b->type = vb->type;
b->memory = vb->memory;
+   b->request_fd = vb->request_fd;
b->bytesused = 0;
 
b->flags = vbuf->flags;
@@ -203,7 +204,6 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->timestamp = ns_to_timeval(vb->timestamp);
b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
-   b->request_fd = 0;
b->reserved = 0;
 
if (q->is_multiplanar) {
@@ -319,6 +319,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
return -EINVAL;
}
vb->timestamp = 0;
+   vb->request_fd = b->request_fd;
vbuf->sequence = 0;
 
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
@@ -667,6 +668,43 @@ int vb2_queue_init(struct vb2_queue *q)
 }
 EXPORT_SYMBOL_GPL(vb2_queue_

[RFC v2 09/10] vim2m: Register V4L2 video device after V4L2 mem2mem init

2018-03-23 Thread Sakari Ailus
Initialise the V4L2 mem2mem framework before creating the video device.
Referencing ctx->m2m_dev isn't allowed before that as it is NULL.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/vim2m.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 065483e..9b6b456 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -1008,6 +1008,16 @@ static int vim2m_probe(struct platform_device *pdev)
atomic_set(&dev->num_inst, 0);
mutex_init(&dev->dev_mutex);
 
+   timer_setup(&dev->timer, device_isr, 0);
+   platform_set_drvdata(pdev, dev);
+
+   dev->m2m_dev = v4l2_m2m_init(&m2m_ops);
+   if (IS_ERR(dev->m2m_dev)) {
+   v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem device\n");
+   ret = PTR_ERR(dev->m2m_dev);
+   goto err_unreg_v4l2_dev;
+   }
+
dev->vfd = vim2m_videodev;
vfd = &dev->vfd;
vfd->lock = &dev->dev_mutex;
@@ -1016,7 +1026,7 @@ static int vim2m_probe(struct platform_device *pdev)
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
if (ret) {
v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
-   goto unreg_dev;
+   goto err_unreg_vdev;
}
 
video_set_drvdata(vfd, dev);
@@ -1024,22 +1034,13 @@ static int vim2m_probe(struct platform_device *pdev)
v4l2_info(&dev->v4l2_dev,
"Device registered as /dev/video%d\n", vfd->num);
 
-   timer_setup(&dev->timer, device_isr, 0);
-   platform_set_drvdata(pdev, dev);
-
-   dev->m2m_dev = v4l2_m2m_init(&m2m_ops);
-   if (IS_ERR(dev->m2m_dev)) {
-   v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem device\n");
-   ret = PTR_ERR(dev->m2m_dev);
-   goto err_m2m;
-   }
-
return 0;
 
-err_m2m:
-   v4l2_m2m_release(dev->m2m_dev);
+err_unreg_vdev:
video_unregister_device(&dev->vfd);
-unreg_dev:
+   v4l2_m2m_release(dev->m2m_dev);
+
+err_unreg_v4l2_dev:
v4l2_device_unregister(&dev->v4l2_dev);
 
return ret;
-- 
2.7.4



[RFC v2 02/10] media: Add request API

2018-03-23 Thread Sakari Ailus
From: Laurent Pinchart 

The request API allows bundling media device parameters with request
objects and queueing them to be executed atomically.

Signed-off-by: Laurent Pinchart 
Signed-off-by: Sakari Ailus 
Signed-off-by: Alexandre Courbot 
---
 drivers/media/Makefile|   3 +-
 drivers/media/media-device.c  |  15 +
 drivers/media/media-request.c | 650 ++
 include/media/media-device.h  |  19 +-
 include/media/media-request.h | 301 +++
 include/uapi/linux/media.h|   8 +
 6 files changed, 994 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/media-request.c
 create mode 100644 include/media/media-request.h

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 594b462..985d35e 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -3,7 +3,8 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
-media-objs := media-device.o media-devnode.o media-entity.o
+media-objs := media-device.o media-devnode.o media-entity.o \
+  media-request.o
 
 #
 # I2C drivers should come before other drivers, otherwise they'll fail
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index da63da1..cc579ce 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 
@@ -366,6 +367,16 @@ static long media_device_get_topology(struct media_device 
*mdev,
return ret;
 }
 
+static long media_device_request_alloc(struct media_device *mdev,
+  struct media_request_new *new)
+{
+   if (!mdev->ops || !mdev->ops->req_alloc || !mdev->ops->req_free ||
+   !mdev->ops->req_queue)
+   return -ENOTTY;
+
+   return media_request_alloc(mdev, new);
+}
+
 static long copy_arg_from_user(void *karg, void __user *uarg, unsigned int cmd)
 {
/* All media IOCTLs are _IOWR() */
@@ -428,6 +439,7 @@ static const struct media_ioctl_info ioctl_info[] = {
MEDIA_IOC(ENUM_LINKS, media_device_enum_links, 
MEDIA_IOC_FL_GRAPH_MUTEX),
MEDIA_IOC(SETUP_LINK, media_device_setup_link, 
MEDIA_IOC_FL_GRAPH_MUTEX),
MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, 
MEDIA_IOC_FL_GRAPH_MUTEX),
+   MEDIA_IOC(REQUEST_ALLOC, media_device_request_alloc, 0),
 };
 
 #define MASK_IOC_SIZE(cmd) \
@@ -739,6 +751,9 @@ void media_device_init(struct media_device *mdev)
INIT_LIST_HEAD(&mdev->pads);
INIT_LIST_HEAD(&mdev->links);
INIT_LIST_HEAD(&mdev->entity_notify);
+   INIT_LIST_HEAD(&mdev->classes);
+   spin_lock_init(&mdev->req_lock);
+
mutex_init(&mdev->graph_mutex);
ida_init(&mdev->entity_internal_idx);
 
diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
new file mode 100644
index 000..af108a7
--- /dev/null
+++ b/drivers/media/media-request.c
@@ -0,0 +1,650 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Media device request objects
+ *
+ * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018, The Chromium OS Authors.  All rights reserved.
+ *
+ * Author: Sakari Ailus 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+static const char *__request_state[] = {
+   "IDLE",
+   "QUEUEING",
+   "QUEUED",
+   "COMPLETE",
+   "REINIT",
+};
+
+const char *
+media_request_state_str(enum media_request_state state)
+{
+   if (state < ARRAY_SIZE(__request_state))
+   return __request_state[state];
+
+   return "UNKNOWN";
+}
+
+static void media_request_clean(struct media_request *req)
+{
+   struct media_request_ref *ref, *ref_safe;
+
+   list_for_each_entry_safe(ref, ref_safe, &req->obj_refs, req_list) {
+   media_request_ref_unbind(ref);
+   kfree(ref);
+   }
+}
+
+void media_request_release(struct kref *kref)
+{
+   struct media_request *req =
+   container_of(kref, struct media_request, kref);
+   struct media_device *mdev = req->mdev;
+
+   dev_dbg(mdev->dev, "request: release %s\n", req->debug_str);
+
+   media_request_clean(req);
+
+   mdev->ops->req_free(req);
+}
+EXPORT_SYMBOL_GPL(media_request_release);
+
+static unsigned int media_request_poll(struct file *filp,
+  struct poll_table_struct *wait)
+{
+   struct media_request *req = filp->private_data;
+   struct media_device *mdev = req->mdev;
+   unsigned int poll_events = poll_requested_events(wait);
+   int ret = 0;
+
+   if (poll_events & (POLLIN | POLLOUT))
+   return POLLERR;
+
+   if (poll_events & POLLPRI) {
+   unsigned long flags;
+   bool complete;
+
+   spin_lock_irqsave(&mdev->req_lock, flags);
+   complete = req->state == MEDIA_REQUEST_STATE_COMPLETE;
+   spin_unlock_irqrestore(&mdev->req_lock, flags);
+

[RFC v2 10/10] vim2m: Request support

2018-03-23 Thread Sakari Ailus
Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/vim2m.c | 49 --
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 9b6b456..a8fe3ea 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -24,6 +24,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -138,6 +139,7 @@ static struct vim2m_fmt *find_format(struct v4l2_format *f)
 }
 
 struct vim2m_dev {
+   struct media_device mdev;
struct v4l2_device  v4l2_dev;
struct video_device vfd;
 
@@ -200,6 +202,7 @@ static struct vim2m_q_data *get_q_data(struct vim2m_ctx 
*ctx,
 
 
 static int device_process(struct vim2m_ctx *ctx,
+ struct v4l2_m2m_request *vreq,
  struct vb2_v4l2_buffer *in_vb,
  struct vb2_v4l2_buffer *out_vb)
 {
@@ -377,12 +380,21 @@ static void device_run(void *priv)
 {
struct vim2m_ctx *ctx = priv;
struct vim2m_dev *dev = ctx->dev;
-   struct vb2_v4l2_buffer *src_buf, *dst_buf;
-
-   src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
-   dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+   struct vb2_v4l2_buffer *src_buf = NULL, *dst_buf = NULL;
+   struct v4l2_m2m_request *vreq;
+
+   vreq = v4l2_m2m_next_req(ctx->fh.m2m_ctx);
+   if (vreq) {
+   src_buf = to_vb2_v4l2_buffer(
+   media_request_object_to_vb2_buffer(vreq->out_ref->new));
+   dst_buf = to_vb2_v4l2_buffer(
+   media_request_object_to_vb2_buffer(vreq->cap_ref->new));
+   } else {
+   src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+   dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+   }
 
-   device_process(ctx, src_buf, dst_buf);
+   device_process(ctx, vreq, src_buf, dst_buf);
 
/* Run a timer, which simulates a hardware irq  */
schedule_irq(dev, ctx->transtime);
@@ -989,6 +1001,12 @@ static const struct v4l2_m2m_ops m2m_ops = {
.job_abort  = job_abort,
 };
 
+static const struct media_device_ops vim2m_mdev_ops = {
+   .req_alloc = v4l2_m2m_req_alloc,
+   .req_free = v4l2_m2m_req_free,
+   .req_queue = v4l2_m2m_req_queue,
+};
+
 static int vim2m_probe(struct platform_device *pdev)
 {
struct vim2m_dev *dev;
@@ -1001,9 +1019,17 @@ static int vim2m_probe(struct platform_device *pdev)
 
spin_lock_init(&dev->irqlock);
 
+   dev->mdev.dev = &pdev->dev;
+   dev->mdev.ops = &vim2m_mdev_ops;
+   strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
+   snprintf(dev->mdev.bus_info, sizeof(dev->mdev.bus_info), "platform:%s",
+MEM2MEM_NAME);
+   media_device_init(&dev->mdev);
+
+   dev->v4l2_dev.mdev = &dev->mdev;
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret)
-   return ret;
+   goto err_cleanup_mdev;
 
atomic_set(&dev->num_inst, 0);
mutex_init(&dev->dev_mutex);
@@ -1018,6 +1044,8 @@ static int vim2m_probe(struct platform_device *pdev)
goto err_unreg_v4l2_dev;
}
 
+   v4l2_m2m_allow_requests(dev->m2m_dev, &dev->mdev);
+
dev->vfd = vim2m_videodev;
vfd = &dev->vfd;
vfd->lock = &dev->dev_mutex;
@@ -1034,6 +1062,10 @@ static int vim2m_probe(struct platform_device *pdev)
v4l2_info(&dev->v4l2_dev,
"Device registered as /dev/video%d\n", vfd->num);
 
+   ret = media_device_register(&dev->mdev);
+   if (ret)
+   goto err_unreg_vdev;
+
return 0;
 
 err_unreg_vdev:
@@ -1043,6 +1075,9 @@ static int vim2m_probe(struct platform_device *pdev)
 err_unreg_v4l2_dev:
v4l2_device_unregister(&dev->v4l2_dev);
 
+err_cleanup_mdev:
+   media_device_cleanup(&dev->mdev);
+
return ret;
 }
 
@@ -1055,6 +1090,8 @@ static int vim2m_remove(struct platform_device *pdev)
del_timer_sync(&dev->timer);
video_unregister_device(&dev->vfd);
v4l2_device_unregister(&dev->v4l2_dev);
+   media_device_unregister(&dev->mdev);
+   media_device_cleanup(&dev->mdev);
 
return 0;
 }
-- 
2.7.4



[RFC v2 03/10] videodev2.h: Add request_fd field to v4l2_buffer

2018-03-23 Thread Sakari Ailus
From: Hans Verkuil 

When queuing buffers allow for passing the request that should
be associated with this buffer.

Signed-off-by: Hans Verkuil 
[acour...@chromium.org: make request ID 32-bit]
Signed-off-by: Alexandre Courbot 
[Sakari Ailus: requests fds are int; use assign_in_user in get_v4l2_buffer]
Signed-off-by: Sakari Ailus 
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 +-
 drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c   | 7 ---
 drivers/media/v4l2-core/v4l2-ioctl.c| 4 ++--
 include/uapi/linux/videodev2.h  | 3 ++-
 5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 886a2d8..6d4d184 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->timestamp = ns_to_timeval(vb->timestamp);
b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
-   b->reserved2 = 0;
+   b->request_fd = 0;
b->reserved = 0;
 
if (q->is_multiplanar) {
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c 
b/drivers/media/usb/cpia2/cpia2_v4l.c
index 99f106b..af42ce3 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -948,7 +948,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct 
v4l2_buffer *buf)
buf->sequence = cam->buffers[buf->index].seq;
buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
buf->length = cam->frame_size;
-   buf->reserved2 = 0;
+   buf->request_fd = 0;
buf->reserved = 0;
memset(&buf->timecode, 0, sizeof(buf->timecode));
 
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 5198c9e..61a8bd4 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -386,7 +386,7 @@ struct v4l2_buffer32 {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __s32   request_fd;
__u32   reserved;
 };
 
@@ -500,7 +500,8 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp,
get_user(memory, &up->memory) ||
put_user(memory, &kp->memory) ||
get_user(length, &up->length) ||
-   put_user(length, &kp->length))
+   put_user(length, &kp->length) ||
+   assign_in_user(&kp->request_fd, &up->request_fd))
return -EFAULT;
 
if (V4L2_TYPE_IS_OUTPUT(type))
@@ -604,7 +605,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp,
assign_in_user(&up->timestamp.tv_usec, &kp->timestamp.tv_usec) ||
copy_in_user(&up->timecode, &kp->timecode, sizeof(kp->timecode)) ||
assign_in_user(&up->sequence, &kp->sequence) ||
-   assign_in_user(&up->reserved2, &kp->reserved2) ||
+   assign_in_user(&up->request_fd, &kp->request_fd) ||
assign_in_user(&up->reserved, &kp->reserved) ||
get_user(length, &kp->length) ||
put_user(length, &up->length))
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 672ab22..c2671de 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -437,13 +437,13 @@ static void v4l_print_buffer(const void *arg, bool 
write_only)
const struct v4l2_plane *plane;
int i;
 
-   pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, flags=0x%08x, 
field=%s, sequence=%d, memory=%s",
+   pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request_fd=%d, 
flags=0x%08x, field=%s, sequence=%d, memory=%s",
p->timestamp.tv_sec / 3600,
(int)(p->timestamp.tv_sec / 60) % 60,
(int)(p->timestamp.tv_sec % 60),
(long)p->timestamp.tv_usec,
p->index,
-   prt_names(p->type, v4l2_type_names),
+   prt_names(p->type, v4l2_type_names), p->request_fd,
p->flags, prt_names(p->field, v4l2_field_names),
p->sequence, prt_names(p->memory, v4l2_memory_names));
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877b..d39932d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -910,6 +910,7 @@ struct v4l2_plane {
  * @length:size in bytes of the buffer (NOT its payload) for single-plane
  * buffers (when type != *_MPLANE); number of elements in the
  * planes array for multi-plane buffers
+ * @request_fd: fd of the request that this buffer should

[RFC v2 00/10] Preparing the request API

2018-03-23 Thread Sakari Ailus
Hi folks,

This preliminary RFC patchset prepares for the request API. What's new
here is support for binding arbitrary configuration or resources to
requests.

There are a few new concepts here:

Class --- a type of configuration or resource a driver (or e.g. the V4L2
framework) can attach to a resource. E.g. a video buffer queue would be a
class.

Object --- an instance of the class. This may be either configuration (in
which case the setting will stay until changed, e.g. V4L2 format on a
video node) or a resource (such as a video buffer).

Reference --- a reference to an object. If a configuration is not changed
in a request, instead of allocating a new object, a reference to an
existing object is used. This saves time and memory.

I expect Laurent to comment on aligning the concept names between the
request API and DRM. As far as I understand, the respective DRM names for
"class" and "object" used in this set would be "object" and "state".

The drivers will need to interact with the requests in three ways:

- Allocate new configurations or resources. Drivers are free to store
  their own data into request objects as well. These callbacks are
  specific to classes.

- Validate and queue callbacks. These callbacks are used to try requests
  (validate only) as well as queue them (validate and queue). These
  callbacks are media device wide, at least for now.

The lifetime of the objects related to requests is based on refcounting
both requests and request objects. This fits well for existing use cases
whether or not based on refcounting; what still needs most of the
attention is likely that the number of gets and puts matches once the
object is no longer needed.

Configuration can be bound to the request the usual way (V4L2 IOCTLs with
the request_fd field set to the request). Once queued, request completion
is signalled through polling the request file handle (POLLPRI).

I'm posting this as an RFC because it's not complete yet. The code
compiles but no testing has been done yet.

Todo list:

- Testing! (And fixing the bugs.)

- Request support in a few drivers as well as the control framework.

- Request support for V4L2 formats?

In the future, support for changing e.g. Media controller link state or
V4L2 sub-device formats will need to be added. Those should receive more
attention when the core is in a good shape and the more simple use cases
are already functional.

Comments and questions are welcome.

since v1:

- Provide an iterator helper for request objects in a request.

- Remove the request lists in the media device (they were not used)

- Move request queing to request fd and add reinit (Alexandre's patchset);
  this roughly corresponds to Request API RFC v2 from Hans.
  (MEDIA_IOC_REQUEST_ALLOC argument is a struct pointer instead of an
  __s32 pointer.)

- Provide a way to unbind request objects from an unqueued request
  (reinit, closing request fd).

- v4l2-mem2mem + vivid implementation without control support.

- More states for requests. In order to take a spinlock (or a mutex) for
  an extended period of time, add a "QUEUEING" and "REINIT" states.

- Move non-IOCTL code to media-request.c, remove extra filp argument that
  was added in v1.

- SPDX license header, other small changes.

Open questions:

- How to tell at complete time whether a request failed? Return error code
  on release? What's the behaviour with reinit then --- fail on error? Add
  another IOCTL to ask for completion status?


Alexandre Courbot (1):
  videodev2.h: add request_fd field to v4l2_ext_controls

Hans Verkuil (1):
  videodev2.h: Add request_fd field to v4l2_buffer

Laurent Pinchart (1):
  media: Add request API

Sakari Ailus (7):
  media: Support variable size IOCTL arguments
  staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack
  vb2: Add support for requests
  v4l: m2m: Simplify exiting the function in v4l2_m2m_try_schedule
  v4l: m2m: Support requests with video buffers
  vim2m: Register V4L2 video device after V4L2 mem2mem init
  vim2m: Request support

 drivers/media/Makefile |   3 +-
 drivers/media/common/videobuf2/videobuf2-core.c|  43 +-
 drivers/media/common/videobuf2/videobuf2-v4l2.c|  40 +-
 drivers/media/media-device.c   |  80 ++-
 drivers/media/media-request.c  | 650 +
 drivers/media/platform/vim2m.c |  76 ++-
 drivers/media/usb/cpia2/cpia2_v4l.c|   2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c  |  16 +-
 drivers/media/v4l2-core/v4l2-ioctl.c   |   6 +-
 drivers/media/v4l2-core/v4l2-mem2mem.c | 131 -
 .../media/atomisp/pci/atomisp2/atomisp_ioctl.c |  17 +-
 include/media/media-device.h   |  19 +-
 include/media/media-request.h  | 301 ++
 include/media/v4l2-mem2mem.h   |  28 +
 include/media/videobuf2-core.h |  19 +
 include/me

[RFC v2 04/10] videodev2.h: add request_fd field to v4l2_ext_controls

2018-03-23 Thread Sakari Ailus
From: Alexandre Courbot 

Allow to specify a request to be used with the S_EXT_CTRLS and
G_EXT_CTRLS operations.

Signed-off-by: Alexandre Courbot 
[Sakari Ailus: reserved no longer an array, add compat32 code]
Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 ++---
 drivers/media/v4l2-core/v4l2-ioctl.c  | 2 +-
 include/uapi/linux/videodev2.h| 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 61a8bd4..9adb367 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -733,7 +733,8 @@ struct v4l2_ext_controls32 {
__u32 which;
__u32 count;
__u32 error_idx;
-   __u32 reserved[2];
+   __s32 request_fd;
+   __u32 reserved;
compat_caddr_t controls; /* actually struct v4l2_ext_control32 * */
 };
 
@@ -808,7 +809,8 @@ static int get_v4l2_ext_controls32(struct file *file,
get_user(count, &up->count) ||
put_user(count, &kp->count) ||
assign_in_user(&kp->error_idx, &up->error_idx) ||
-   copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
+   assign_in_user(&kp->request_fd, &up->request_fd) ||
+   assign_in_user(&kp->reserved, &up->reserved))
return -EFAULT;
 
if (count == 0)
@@ -866,7 +868,8 @@ static int put_v4l2_ext_controls32(struct file *file,
get_user(count, &kp->count) ||
put_user(count, &up->count) ||
assign_in_user(&up->error_idx, &kp->error_idx) ||
-   copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved)) ||
+   assign_in_user(&up->request_fd, &kp->request_fd) ||
+   assign_in_user(&up->reserved, &kp->reserved) ||
get_user(kcontrols, &kp->controls))
return -EFAULT;
 
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index c2671de..85c4bb9 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -870,7 +870,7 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int 
allow_priv)
__u32 i;
 
/* zero the reserved fields */
-   c->reserved[0] = c->reserved[1] = 0;
+   c->reserved = 0;
for (i = 0; i < c->count; i++)
c->controls[i].reserved2[0] = 0;
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index d39932d..e6e68a5 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1593,7 +1593,8 @@ struct v4l2_ext_controls {
};
__u32 count;
__u32 error_idx;
-   __u32 reserved[2];
+   __s32 request_fd;
+   __u32 reserved;
struct v4l2_ext_control *controls;
 };
 
-- 
2.7.4



[RFC v2 05/10] staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack

2018-03-23 Thread Sakari Ailus
The atomisp driver used to use the reserved2 field in struct v4l2_buffer
for picking a particular ISP configuration for the buffer. As reserved2
field will have new use soon, remove this hack.

Signed-off-by: Sakari Ailus 
---
 .../staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c  | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
index 5c84dd6..182bb70 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
@@ -1283,16 +1283,7 @@ static int atomisp_qbuf(struct file *file, void *fh, 
struct v4l2_buffer *buf)
if (!((buf->flags & NOFLUSH_FLAGS) == NOFLUSH_FLAGS))
wbinvd();
 
-   if (!atomisp_is_vf_pipe(pipe) &&
-   (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) {
-   /* this buffer will have a per-frame parameter */
-   pipe->frame_request_config_id[buf->index] = buf->reserved2 &
-   ~ATOMISP_BUFFER_HAS_PER_FRAME_SETTING;
-   dev_dbg(isp->dev, "This buffer requires per_frame setting which 
has isp_config_id %d\n",
-   pipe->frame_request_config_id[buf->index]);
-   } else {
-   pipe->frame_request_config_id[buf->index] = 0;
-   }
+   pipe->frame_request_config_id[buf->index] = 0;
 
pipe->frame_params[buf->index] = NULL;
 
@@ -1470,12 +1461,10 @@ static int atomisp_dqbuf(struct file *file, void *fh, 
struct v4l2_buffer *buf)
buf->reserved &= 0x;
if (!(buf->flags & V4L2_BUF_FLAG_ERROR))
buf->reserved |= __get_frame_exp_id(pipe, buf) << 16;
-   buf->reserved2 = pipe->frame_config_id[buf->index];
rt_mutex_unlock(&isp->mutex);
 
-   dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d, 
isp_config_id %d\n",
-   buf->index, vdev->name, asd->index, buf->reserved >> 16,
-   buf->reserved2);
+   dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d\n",
+   buf->index, vdev->name, asd->index, buf->reserved >> 16);
return 0;
 }
 
-- 
2.7.4



[RFC v2 01/10] media: Support variable size IOCTL arguments

2018-03-23 Thread Sakari Ailus
Maintain a list of supported IOCTL argument sizes and allow only those in
the list.

As an additional bonus, IOCTL handlers will be able to check whether the
caller actually set (using the argument size) the field vs. assigning it
to zero. Separate macro can be provided for that.

This will be easier for applications as well since there is no longer the
problem of setting the reserved fields zero, or at least it is a lesser
problem.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c | 65 
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 35e81f7..da63da1 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -387,22 +387,36 @@ static long copy_arg_to_user(void __user *uarg, void 
*karg, unsigned int cmd)
 /* Do acquire the graph mutex */
 #define MEDIA_IOC_FL_GRAPH_MUTEX   BIT(0)
 
-#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
+#define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user)  \
[_IOC_NR(MEDIA_IOC_##__cmd)] = {\
.cmd = MEDIA_IOC_##__cmd,   \
.fn = (long (*)(struct media_device *, void *))func,\
.flags = fl,\
+   .alt_arg_sizes = alt_sz,\
.arg_from_user = from_user, \
.arg_to_user = to_user, \
}
 
-#define MEDIA_IOC(__cmd, func, fl) \
-   MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
+#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
+   MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
+
+#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz)  \
+   MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz,   \
+copy_arg_from_user, copy_arg_to_user)
+
+#define MEDIA_IOC(__cmd, func, fl) \
+   MEDIA_IOC_ARG(__cmd, func, fl,  \
+ copy_arg_from_user, copy_arg_to_user)
 
 /* the table is indexed by _IOC_NR(cmd) */
 struct media_ioctl_info {
unsigned int cmd;
unsigned short flags;
+   /*
+* Sizes of the alternative arguments. If there are none, this
+* pointer is NULL.
+*/
+   const unsigned short *alt_arg_sizes;
long (*fn)(struct media_device *dev, void *arg);
long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
@@ -416,6 +430,42 @@ static const struct media_ioctl_info ioctl_info[] = {
MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, 
MEDIA_IOC_FL_GRAPH_MUTEX),
 };
 
+#define MASK_IOC_SIZE(cmd) \
+   ((cmd) & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
+
+static inline long is_valid_ioctl(unsigned int cmd)
+{
+   const struct media_ioctl_info *info = ioctl_info;
+   const unsigned short *alt_arg_sizes;
+
+   if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
+   return -ENOIOCTLCMD;
+
+   info += _IOC_NR(cmd);
+
+   if (info->cmd == cmd)
+   return 0;
+
+   /*
+* Verify that the size-dependent patch of the IOCTL command
+* matches and that the size does not exceed the principal
+* argument size.
+*/
+   if (MASK_IOC_SIZE(info->cmd) != MASK_IOC_SIZE(cmd)
+   || _IOC_SIZE(info->cmd) < _IOC_SIZE(cmd))
+   return -ENOIOCTLCMD;
+
+   alt_arg_sizes = info->alt_arg_sizes;
+   if (!alt_arg_sizes)
+   return -ENOIOCTLCMD;
+
+   for (; *alt_arg_sizes; alt_arg_sizes++)
+   if (_IOC_SIZE(cmd) == *alt_arg_sizes)
+   return 0;
+
+   return -ENOIOCTLCMD;
+}
+
 static long media_device_ioctl(struct file *filp, unsigned int cmd,
   unsigned long __arg)
 {
@@ -426,9 +476,9 @@ static long media_device_ioctl(struct file *filp, unsigned 
int cmd,
char __karg[256], *karg = __karg;
long ret;
 
-   if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
-   || ioctl_info[_IOC_NR(cmd)].cmd != cmd)
-   return -ENOIOCTLCMD;
+   ret = is_valid_ioctl(cmd);
+   if (ret)
+   return ret;
 
info = &ioctl_info[_IOC_NR(cmd)];
 
@@ -444,6 +494,9 @@ static long media_device_ioctl(struct file *filp, unsigned 
int cmd,
goto out_free;
}
 
+   /* Set the rest of the argument struct to zero */
+   memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
+
if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
mutex_lock(&dev->graph_mutex);
 
-- 
2.

[RFC v2 08/10] v4l: m2m: Support requests with video buffers

2018-03-23 Thread Sakari Ailus
Enable supporting requests on V4L2 buffer queues on M2M devices. This
requires Media controller.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 109 +
 include/media/v4l2-mem2mem.h   |  28 +
 2 files changed, 137 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 9fbf778..effdd15 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -57,6 +58,8 @@ module_param(debug, bool, 0644);
  * @job_queue: instances queued to run
  * @job_spinlock:  protects job_queue
  * @m2m_ops:   driver callbacks
+ * @mdev:  media device; optional
+ * @allow_requests:whether requests are allowed on the M2M device
  */
 struct v4l2_m2m_dev {
struct v4l2_m2m_ctx *curr_ctx;
@@ -65,6 +68,9 @@ struct v4l2_m2m_dev {
spinlock_t  job_spinlock;
 
const struct v4l2_m2m_ops *m2m_ops;
+
+   struct media_device *mdev;
+   boolallow_requests;
 };
 
 static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx *m2m_ctx,
@@ -89,6 +95,78 @@ struct vb2_queue *v4l2_m2m_get_vq(struct v4l2_m2m_ctx 
*m2m_ctx,
 }
 EXPORT_SYMBOL(v4l2_m2m_get_vq);
 
+struct media_request *v4l2_m2m_req_alloc(struct media_device *mdev)
+{
+   struct v4l2_m2m_request *vreq;
+
+   vreq = kzalloc(sizeof(*vreq), GFP_KERNEL);
+   if (!vreq)
+   return NULL;
+
+   return &vreq->req;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_req_alloc);
+
+void v4l2_m2m_req_free(struct media_request *req)
+{
+   struct v4l2_m2m_request *vreq =
+   container_of(req, struct v4l2_m2m_request, req);
+
+   kfree(vreq);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_req_free);
+
+int v4l2_m2m_req_queue(struct media_request *req)
+{
+   struct v4l2_m2m_request *vreq =
+   container_of(req, struct v4l2_m2m_request, req);
+   struct v4l2_m2m_ctx *m2m_ctx = vreq->ctx;
+   struct v4l2_m2m_dev *m2m_dev = m2m_ctx->m2m_dev;
+   struct media_request_ref *iter;
+   unsigned long flags;
+
+   media_request_for_each_ref(iter, req)
+   if (iter->new->class == m2m_ctx->cap_q_ctx.q.class)
+   vreq->cap_ref = iter;
+   else if (iter->new->class == m2m_ctx->out_q_ctx.q.class)
+   vreq->out_ref = iter;
+   else
+   return -EINVAL;
+
+   if (!vreq->out_ref || !vreq->cap_ref)
+   return -EINVAL;
+
+   spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
+   list_add(&vreq->queue_list, &m2m_ctx->request_queue);
+   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
+
+   v4l2_m2m_try_schedule(m2m_ctx);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_req_queue);
+
+struct v4l2_m2m_request *v4l2_m2m_next_req(struct v4l2_m2m_ctx *m2m_ctx)
+{
+   struct v4l2_m2m_dev *m2m_dev = m2m_ctx->m2m_dev;
+   struct v4l2_m2m_request *vreq;
+   unsigned long flags;
+
+   spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
+   if (list_empty(&m2m_ctx->request_queue)) {
+   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
+   return NULL;
+   }
+
+   vreq = list_first_entry(&m2m_ctx->request_queue,
+   struct v4l2_m2m_request, queue_list);
+
+   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
+
+   return vreq;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_next_req);
+
 void *v4l2_m2m_next_buf(struct v4l2_m2m_queue_ctx *q_ctx)
 {
struct v4l2_m2m_buffer *b;
@@ -239,6 +317,11 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
goto out_unlock;
}
 
+   if (m2m_dev->allow_requests && list_empty(&m2m_ctx->request_queue)) {
+   dprintk("No requests queued\n");
+   goto out_unlock;
+   }
+
spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue)
&& !m2m_ctx->out_q_ctx.buffered) {
@@ -393,6 +476,9 @@ int v4l2_m2m_qbuf(struct file *file, struct v4l2_m2m_ctx 
*m2m_ctx,
struct vb2_queue *vq;
int ret;
 
+   if (m2m_ctx->m2m_dev->allow_requests && !buf->request_fd)
+   return -EINVAL;
+
vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
ret = vb2_qbuf(vq, buf);
if (!ret)
@@ -618,6 +704,17 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct 
v4l2_m2m_ops *m2m_ops)
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_init);
 
+void v4l2_m2m_allow_requests(struct v4l2_m2m_dev *m2m_dev,
+struct media_device *mdev)
+{
+   if (WARN_ON(!mdev))
+   return;
+
+   m2m_dev->mdev = mdev;
+   m2m_dev->allow_requests = true;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_allow_requests);
+
 void v4l2_m2m_releas

[RFC v2 07/10] v4l: m2m: Simplify exiting the function in v4l2_m2m_try_schedule

2018-03-23 Thread Sakari Ailus
The v4l2_m2m_try_schedule function acquires and releases multiple
spinlocks; simplify unlocking the job lock by adding a label to unlock the
job lock and exit the function.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index c4f963d..9fbf778 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -230,15 +230,13 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
 
/* If the context is aborted then don't schedule it */
if (m2m_ctx->job_flags & TRANS_ABORT) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("Aborted context\n");
-   return;
+   goto out_unlock;
}
 
if (m2m_ctx->job_flags & TRANS_QUEUED) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("On job queue already\n");
-   return;
+   goto out_unlock;
}
 
spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
@@ -246,9 +244,8 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
&& !m2m_ctx->out_q_ctx.buffered) {
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
flags_out);
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("No input buffers available\n");
-   return;
+   goto out_unlock;
}
spin_lock_irqsave(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue)
@@ -257,18 +254,16 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
flags_cap);
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
flags_out);
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("No output buffers available\n");
-   return;
+   goto out_unlock;
}
spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
 
if (m2m_dev->m2m_ops->job_ready
&& (!m2m_dev->m2m_ops->job_ready(m2m_ctx->priv))) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("Driver not ready\n");
-   return;
+   goto out_unlock;
}
 
list_add_tail(&m2m_ctx->queue, &m2m_dev->job_queue);
@@ -277,6 +272,13 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
 
v4l2_m2m_try_run(m2m_dev);
+
+   return;
+
+out_unlock:
+   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
+
+   return;
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_try_schedule);
 
-- 
2.7.4



Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

2018-03-23 Thread Kieran Bingham
Hi Simon,

On 23/03/18 08:51, Simon Horman wrote:
> On Thu, Mar 22, 2018 at 09:30:40PM +, Kieran Bingham wrote:
>> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
>> bus, however in low power mode the ADV7513 will reset it's slave maps to
>> use the hardware defined default addresses.
>>
>> The ADV7511 driver was adapted to allow the two devices to be registered
>> correctly - but it did not take into account the fault whereby the
>> devices reset the addresses.
>>
>> This results in an address conflict between the device using the default
>> addresses, and the other device if it is in low-power-mode.
>>
>> Repair this issue by moving both devices away from the default address
>> definitions.
> 
> Hi Kierean,
> 
> as this is a fix
> a) Does it warrant a fixes tag?
>Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> b) Does it warrant being posted as a fix for v4.16;
> c) or v4.17?

Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue if
the corresponding driver updates to allow secondary addresses to be parsed are
also backported.

It should be safe to back port the dts fix without the driver updates, but the
addresses specified by this patch will simply be ignored.

Thus if this is marked with the fixes tag the corresponding patch "drm: adv7511:
Add support for i2c_new_secondary_device" should also be marked.

It looks like that patch has yet to be picked up by the DRM subsystem, so how
about I bundle both of these two patches together in a repost along with the
fixes tag.

In fact, I don't think the ADV7511 dt-bindings update has made any progress
either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
addresses). The media tree variants for the adv7604 have already been picked up
by Mauro I believe though.

I presume it would be acceptable for this dts patch (or rather all three patches
mentioned) to get integrated through the DRM tree ?

--
Kieran


[PATCH] media: em28xx-cards: output regular messages as info

2018-03-23 Thread Chris Mayo
Messages expected during device probe were being marked as errors.

Signed-off-by: Chris Mayo 
---
 drivers/media/usb/em28xx/em28xx-cards.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c
index 6e0e67d23..8977c2be3 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3736,7 +3736,7 @@ static int em28xx_usb_probe(struct usb_interface *intf,
speed = "unknown";
}
 
-   dev_err(&intf->dev,
+   dev_info(&intf->dev,
"New device %s %s @ %s Mbps (%04x:%04x, interface %d, class 
%d)\n",
udev->manufacturer ? udev->manufacturer : "",
udev->product ? udev->product : "",
@@ -3771,7 +3771,7 @@ static int em28xx_usb_probe(struct usb_interface *intf,
dev->dev_next = NULL;
 
if (has_vendor_audio) {
-   dev_err(&intf->dev,
+   dev_info(&intf->dev,
"Audio interface %i found (Vendor Class)\n", ifnum);
dev->usb_audio_type = EM28XX_USB_AUDIO_VENDOR;
}
@@ -3790,12 +3790,12 @@ static int em28xx_usb_probe(struct usb_interface *intf,
}
 
if (has_video)
-   dev_err(&intf->dev, "Video interface %i found:%s%s\n",
+   dev_info(&intf->dev, "Video interface %i found:%s%s\n",
ifnum,
dev->analog_ep_bulk ? " bulk" : "",
dev->analog_ep_isoc ? " isoc" : "");
if (has_dvb)
-   dev_err(&intf->dev, "DVB interface %i found:%s%s\n",
+   dev_info(&intf->dev, "DVB interface %i found:%s%s\n",
ifnum,
dev->dvb_ep_bulk ? " bulk" : "",
dev->dvb_ep_isoc ? " isoc" : "");
@@ -3837,13 +3837,13 @@ static int em28xx_usb_probe(struct usb_interface *intf,
if (has_video) {
if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk))
dev->analog_xfer_bulk = 1;
-   dev_err(&intf->dev, "analog set to %s mode.\n",
+   dev_info(&intf->dev, "analog set to %s mode.\n",
dev->analog_xfer_bulk ? "bulk" : "isoc");
}
if (has_dvb) {
if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk))
dev->dvb_xfer_bulk = 1;
-   dev_err(&intf->dev, "dvb set to %s mode.\n",
+   dev_info(&intf->dev, "dvb set to %s mode.\n",
dev->dvb_xfer_bulk ? "bulk" : "isoc");
}
 
-- 
2.16.1



[GIT PULL for 4.17] Add array size to v4l2_find_nearest_size

2018-03-23 Thread Sakari Ailus
Hi Mauro,

Just one patch here, to add the size of the array of the modes for
v4l2_find_nearest_size.

Please pull.


The following changes since commit 238f694e1b7f8297f1256c57e41f69c39576c9b4:

  media: v4l2-common: fix a compilation breakage (2018-03-21 16:07:01 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git v4l2-common-size2

for you to fetch changes up to ba46ac7f7047f21823bdf6cca5fd8c5ded0babdf:

  v4l: Bring back array_size parameter to v4l2_find_nearest_size (2018-03-23 
17:33:33 +0200)


Sakari Ailus (1):
  v4l: Bring back array_size parameter to v4l2_find_nearest_size

 drivers/media/i2c/ov13858.c  | 4 +++-
 drivers/media/i2c/ov5670.c   | 4 +++-
 drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
 include/media/v4l2-common.h  | 5 +++--
 4 files changed, 12 insertions(+), 6 deletions(-)

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH] [media] vcodec: fix error return value from mtk_jpeg_clk_init()

2018-03-23 Thread Matthias Brugger


On 03/23/2018 04:44 AM, Ryder Lee wrote:
> The error return value should be fixed as it may return EPROBE_DEFER.
> 
> Cc: Rick Chang 
> Cc: Bin Liu 
> Signed-off-by: Ryder Lee 

Reviewed-by: Matthias Brugger 

> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c 
> b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 226f908..af17aaa 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -1081,11 +1081,11 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev 
> *jpeg)
>  
>   jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
>   if (IS_ERR(jpeg->clk_jdec))
> - return -EINVAL;
> + return PTR_ERR(jpeg->clk_jdec);
>  
>   jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi");
>   if (IS_ERR(jpeg->clk_jdec_smi))
> - return -EINVAL;
> + return PTR_ERR(jpeg->clk_jdec_smi);
>  
>   return 0;
>  }
> 


Re: [PATCH 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Mauro Carvalho Chehab
Em Fri, 23 Mar 2018 17:31:53 +0200
Sakari Ailus  escreveu:

> On Fri, Mar 23, 2018 at 11:08:55AM -0300, Mauro Carvalho Chehab wrote:
> > Em Fri, 23 Mar 2018 11:07:42 -0300
> > Mauro Carvalho Chehab  escreveu:
> >   
> > > Em Fri, 23 Mar 2018 15:48:41 +0200
> > > Sakari Ailus  escreveu:
> > >   
> > > > An older version of the driver patches were merged accidentally which
> > > > resulted in missing the array_size parameter that tells the length of 
> > > > the
> > > > array that contains the different supported sizes.
> > > > 
> > > > Bring it back to v4l2_find_nearest size and make the corresponding 
> > > > change
> > > > for the drivers using it as well.
> > > > 
> > > > Signed-off-by: Sakari Ailus 
> > > > ---
> > > > Hi Mauro,
> > > > 
> > > > Here's the patch I mentioned. It restores the intended state of the
> > > > v4l2_find_nearest_size() API as it was reviewed and acked (by Hans).
> > > > 
> > > > This time the exact patch is tested for vivid.
> > > > 
> > > >  drivers/media/i2c/ov13858.c  | 5 +++--
> > > >  drivers/media/i2c/ov5670.c   | 5 +++--
> > > >  drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
> > > >  include/media/v4l2-common.h  | 5 +++--
> > > >  4 files changed, 12 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> > > > index 30ee9f71bf0d..420af1e32d4e 100644
> > > > --- a/drivers/media/i2c/ov13858.c
> > > > +++ b/drivers/media/i2c/ov13858.c
> > > > @@ -1375,8 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
> > > > if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
> > > > fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > > >  
> > > > -   mode = v4l2_find_nearest_size(supported_modes, width, height,
> > > > - fmt->format.width, 
> > > > fmt->format.height);
> > > > +   mode = v4l2_find_nearest_size(
> > > > +   supported_modes, ARRAY_SIZE(supported_modes), width, 
> > > > height,
> > > > +   fmt->format.width, fmt->format.height);
> > > 
> > > 
> > > Nitpick... I find ugly and arder to mentally parse things like the above, 
> > >  
> 
> Ok, I'll send v2.

Thanks!
> 
> > 
> > "arder" -> "harder"
> > 
> > My keyboard sometimes is losing keystrokes. It seems it is approaching
> > the time to replace it again.  
> 
> Perhaps an IBM model M? I once tried one but my fingers started to ache.

I used one for a while... Works fine, but yeah, keys are havier.

> :-9 So I'm still using my Keytronic keyboard from 1994. :-D

Good for you!

I'm using a Logitech K520 wireless keyboard here. That's my second one.
It works great when it is new, but after too much usage, keys start to
glue on its back :-(

Yet, I loved the idea of having a wireless one, and use the same
mini-stick for both mouse and kbd. Makes easy when I need to plug on
some other device.

Anyway, I can't buy keyboards in BR, as it will come with a "weird"
layout.

Thanks,
Mauro


[PATCH v2 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Sakari Ailus
An older version of the driver patches were merged accidentally which
resulted in missing the array_size parameter that tells the length of the
array that contains the different supported sizes.

Bring it back to v4l2_find_nearest size and make the corresponding change
for the drivers using it as well.

Signed-off-by: Sakari Ailus 
---
since v1:

- Rewrap v4l2_find_nearest_size() calls in the two ov sensor drivers.

 drivers/media/i2c/ov13858.c  | 4 +++-
 drivers/media/i2c/ov5670.c   | 4 +++-
 drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
 include/media/v4l2-common.h  | 5 +++--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 30ee9f71bf0d..3e9ff8205991 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1375,7 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
-   mode = v4l2_find_nearest_size(supported_modes, width, height,
+   mode = v4l2_find_nearest_size(supported_modes,
+ ARRAY_SIZE(supported_modes),
+ width, height,
  fmt->format.width, fmt->format.height);
ov13858_update_pad_format(mode, fmt);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 556a95c30781..4bf25f527324 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -2229,7 +2229,9 @@ static int ov5670_set_pad_format(struct v4l2_subdev *sd,
 
fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
-   mode = v4l2_find_nearest_size(supported_modes, width, height,
+   mode = v4l2_find_nearest_size(supported_modes,
+ ARRAY_SIZE(supported_modes),
+ width, height,
  fmt->format.width, fmt->format.height);
ov5670_update_pad_format(mode, fmt);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c 
b/drivers/media/platform/vivid/vivid-vid-cap.c
index 01c703683657..1599159f2574 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -561,8 +561,9 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
mp->field = vivid_field_cap(dev, mp->field);
if (vivid_is_webcam(dev)) {
const struct v4l2_frmsize_discrete *sz =
-   v4l2_find_nearest_size(webcam_sizes, width, height,
-  mp->width, mp->height);
+   v4l2_find_nearest_size(webcam_sizes,
+  VIVID_WEBCAM_SIZES, width,
+  height, mp->width, mp->height);
 
w = sz->width;
h = sz->height;
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 54b689247937..160bca96d524 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -320,6 +320,7 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  * set of resolutions contained in an array of a driver specific struct.
  *
  * @array: a driver specific array of image sizes
+ * @array_size: the length of the driver specific array of image sizes
  * @width_field: the name of the width field in the driver specific struct
  * @height_field: the name of the height field in the driver specific struct
  * @width: desired width.
@@ -332,13 +333,13 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  *
  * Returns the best match or NULL if the length of the array is zero.
  */
-#define v4l2_find_nearest_size(array, width_field, height_field, \
+#define v4l2_find_nearest_size(array, array_size, width_field, height_field, \
   width, height)   \
({  \
BUILD_BUG_ON(sizeof((array)->width_field) != sizeof(u32) || \
 sizeof((array)->height_field) != sizeof(u32)); \
(typeof(&(*(array__v4l2_find_nearest_size(  \
-   (array), ARRAY_SIZE(array), sizeof(*(array)),   \
+   (array), array_size, sizeof(*(array)),  \
offsetof(typeof(*(array)), width_field),\
offsetof(typeof(*(array)), height_field),   \
width, height); \
-- 
2.11.0



Re: [PATCH 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Sakari Ailus
On Fri, Mar 23, 2018 at 11:08:55AM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 23 Mar 2018 11:07:42 -0300
> Mauro Carvalho Chehab  escreveu:
> 
> > Em Fri, 23 Mar 2018 15:48:41 +0200
> > Sakari Ailus  escreveu:
> > 
> > > An older version of the driver patches were merged accidentally which
> > > resulted in missing the array_size parameter that tells the length of the
> > > array that contains the different supported sizes.
> > > 
> > > Bring it back to v4l2_find_nearest size and make the corresponding change
> > > for the drivers using it as well.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > ---
> > > Hi Mauro,
> > > 
> > > Here's the patch I mentioned. It restores the intended state of the
> > > v4l2_find_nearest_size() API as it was reviewed and acked (by Hans).
> > > 
> > > This time the exact patch is tested for vivid.
> > > 
> > >  drivers/media/i2c/ov13858.c  | 5 +++--
> > >  drivers/media/i2c/ov5670.c   | 5 +++--
> > >  drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
> > >  include/media/v4l2-common.h  | 5 +++--
> > >  4 files changed, 12 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> > > index 30ee9f71bf0d..420af1e32d4e 100644
> > > --- a/drivers/media/i2c/ov13858.c
> > > +++ b/drivers/media/i2c/ov13858.c
> > > @@ -1375,8 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
> > >   if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
> > >   fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > >  
> > > - mode = v4l2_find_nearest_size(supported_modes, width, height,
> > > -   fmt->format.width, fmt->format.height);
> > > + mode = v4l2_find_nearest_size(
> > > + supported_modes, ARRAY_SIZE(supported_modes), width, height,
> > > + fmt->format.width, fmt->format.height);  
> > 
> > 
> > Nitpick... I find ugly and arder to mentally parse things like the above,

Ok, I'll send v2.

> 
>   "arder" -> "harder"
> 
> My keyboard sometimes is losing keystrokes. It seems it is approaching
> the time to replace it again.

Perhaps an IBM model M? I once tried one but my fingers started to ache.
:-9 So I'm still using my Keytronic keyboard from 1994. :-D

-- 
Cheers,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [RFCv2] Request API

2018-03-23 Thread Tomasz Figa
Hi Hans,

On Fri, Mar 23, 2018 at 5:46 PM, Hans Verkuil  wrote:
> RFC Request API, version 2
> --
>
> This document proposes the public API for handling requests.
>
> There has been some confusion about how to do this, so this summarizes the
> current approach based on conversations with the various stakeholders today
> (Sakari, Alexandre Courbot, Thomasz Figa and myself).
>
> 1) Additions to the media API
>
>Allocate an empty request object:
>
>#define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)
>
>This will return a file descriptor representing the request or an error
>if it can't allocate the request.
>
>If the pointer argument is NULL, then this will just return 0 (if this 
> ioctl
>is implemented) or -ENOTTY otherwise. This can be used to test whether this
>ioctl is supported or not without actually having to allocate a request.
>
> 2) Operations on the request fd
>
>You can queue or reinit a request by calling these ioctls on the request 
> fd:
>
>#define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
>#define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)
>
>With REINIT you reset the state of the request as if you had just allocated
>it.
>
>You can poll the request fd to wait for it to complete.
>
>To free a request you close the request fd. Note that it may still be in
>use internally, so the internal datastructures have to be refcounted.
>
>For this initial implementation only buffers and controls are contained
>in a request. This is needed to implement stateless codecs. Supporting
>complex camera pipelines will require more work.
>
>Requests only contain the changes to the state at request queue time
>relative to the previously queued request(s) or the current hardware state
>if no other requests were queued.
>
>Once a request is completed it will retain the state at completion
>time.
>
> 3) To associate a v4l2 buffer with a request the 'reserved' field in struct
>v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
>until the request is queued since the request may contain information that
>is needed to prepare the buffer.
>
>To indicate that request_fd should be used this flag should be set by
>userspace at QBUF time:
>
> #define V4L2_BUF_FLAG_REQUEST   0x0080
>
>This flag will also be returned by the driver to indicate that the buffer
>is associated with a request.
>
>TBD: what should vb2 return as request_fd value if this flag is set?
>This should act the same as the fence patch series and this is still
>being tweaked so let's wait for that to be merged first, then we can
>finalize this.
>
> 4) To associate v4l2 controls with a request we take the first of the
>'reserved[2]' array elements in struct v4l2_ext_controls and use it to 
> store
>the request fd.
>
>We also add a new WHICH value:
>
> #define V4L2_CTRL_WHICH_REQUEST   0x0f01
>
>This tells the control framework to get/set controls from the given
>request fd.
>
>When querying a control value from a request it will return the newest
>value in the list of pending requests, or the current hardware value if
>is not set in any of the pending requests.
>
>When a request is completed the controls will no longer change. A copy
>will be made of volatile controls at the time of completion (actually
>it will be up to the driver to decide when to do that).
>
>Volatile controls and requests:
>
>- If you set a volatile control in a request, then that will be ignored,
>  unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.
>
>- If you get a volatile control from a request then:
>  1) If the request is completed it will return the value of the volatile
> control at completion time.
>  2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
> set in a request, then that value is returned.
>  3) Otherwise: return the current value from the hardware (i.e. normal
> behavior).
>
>Read-only controls and requests:
>
>- If you get a read-only control from a request then:
>  1) If the request is completed it will return the value of the read-only
> control at completion time.
>  2) Otherwise it will get the current value from the driver (i.e. normal
> behavior).
>
>Open issue: should we receive control events if a control in a request is
>added/changed? Currently there are no plans to support control events for
>requests. I don't see a clear use-case and neither do I see an easy way
>of implementing this (which fd would receive those events?).
>
> Notes:
>
> - Earlier versions of this API had a TRY command as well to validate the
>   request. I'm not sure that is useful so I dropped it, but it can easily
>   be added if there is a good use-case for it. Traditionally within V4L the
>   TRY ioctl will 

Re: [RFC] Request API

2018-03-23 Thread Tomasz Figa
On Fri, Mar 23, 2018 at 6:33 PM, Sakari Ailus  wrote:
>> >>> An alternative, maybe a bit crazy, idea would be to allow adding
>> >>> MEDIA_REQUEST_IOC_QUEUE ioctl to the request itself. This would be
>> >>> similar to the idea of indirect command buffers in the graphics (GPU)
>> >>> land. It could for example look like this:
>> >>>
>> >>> // One time initialization
>> >>> bulk_fd = ioctl(..., MEDIA_IOC_REQUEST_ALLOC, ...);
>> >>> for (i = 0; i < N; ++i) {
>> >>> fd[i] = ioctl(..., MEDIA_IOC_REQUEST_ALLOC, ...);
>> >>> // Add some state
>> >>> ioctl(fd[i], MEDIA_IOC_REQUEST_QUEUE, { .request = bulk_fd });
>> >>> }
>> >>>
>> >>> // Do some work
>> >>>
>> >>> ioctl(bulk_fd, MEDIA_IOC_REQUEST_QUEUE); // Queues all the requests at 
>> >>> once
>> >>
>> >> That doesn't reduce the number of ioctl calls :-)
>> >
>> > Depends on what cases we are talking about. If we have cases of
>> > queuing the same (big) sets of requests multiple time, then only for
>> > the first time all the ioctls would be needed. Next time, one only
>> > needs to queue the bulk_fd.
>> >
>> > Personally, I don't have any good use case that would involve queuing
>> > many requests instantly and would be affected by the number of ioctls,
>> > though.
>>
>> Sakari gave the example of 10 cameras running at 60 fps, thus generating
>> a very large amount of request ioctls.
>>
>> But this doesn't apply to stateless codecs, so for now this is something
>> for the future.
>
> I think get real benefit from being able to queue multiple requests at a
> time only when you don't need a ton of IOCTLs to construct those requests
> first. I.e. that means moving constructing the requests to Media device as
> well. That's something for the future indeed, and I don't think it's
> realistic to think we'd be there any time soon.

That wouldn't be even Media. Constructing the requests would have to
be moved to userspace, with requests being more like command buffers,
arrays of state objects.

Best regards,
Tomasz


Re: [PATCH v9.1] media: imx258: Add imx258 camera sensor driver

2018-03-23 Thread Sakari Ailus
On Fri, Mar 23, 2018 at 11:08:11PM +0900, Tomasz Figa wrote:
> On Fri, Mar 23, 2018 at 10:50 PM, Sakari Ailus
>  wrote:
> > Hi Tomasz,
> >
> > On Fri, Mar 23, 2018 at 08:43:50PM +0900, Tomasz Figa wrote:
> >> Hi Andy,
> >>
> >> Some issues found when reviewing cherry pick of this patch to Chrome
> >> OS kernel. Please see inline.
> >>
> >> On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh  wrote:
> >>
> >> [snip]
> >>
> >> > +   case V4L2_CID_VBLANK:
> >> > +   /*
> >> > +* Auto Frame Length Line Control is enabled by default.
> >> > +* Not need control Vblank Register.
> >> > +*/
> >>
> >> What is the meaning of this control then? Should it be read-only?
> >
> > The read-only flag is for the uAPI; the control framework still passes
> > through changes to the control value done using kAPI to the driver.
> 
> The read-only flag is not even set in current code.

Ah, you're right, it's just hblank... but if the driver doesn't support
setting this control, then it should most likely be read-only. It would
seem like that the driver just updates the control to convey the value to
the user.

> 
> Also, I'm not sure about the control framework setting read-only
> control. According to the code, it doesn't:
> https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-ctrls.c#L2477

If you set the control using e.g. v4l2_ctrl_s_ctrl(), it should end up to
the driver's s_ctrl callback.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Mauro Carvalho Chehab
Em Fri, 23 Mar 2018 11:07:42 -0300
Mauro Carvalho Chehab  escreveu:

> Em Fri, 23 Mar 2018 15:48:41 +0200
> Sakari Ailus  escreveu:
> 
> > An older version of the driver patches were merged accidentally which
> > resulted in missing the array_size parameter that tells the length of the
> > array that contains the different supported sizes.
> > 
> > Bring it back to v4l2_find_nearest size and make the corresponding change
> > for the drivers using it as well.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> > Hi Mauro,
> > 
> > Here's the patch I mentioned. It restores the intended state of the
> > v4l2_find_nearest_size() API as it was reviewed and acked (by Hans).
> > 
> > This time the exact patch is tested for vivid.
> > 
> >  drivers/media/i2c/ov13858.c  | 5 +++--
> >  drivers/media/i2c/ov5670.c   | 5 +++--
> >  drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
> >  include/media/v4l2-common.h  | 5 +++--
> >  4 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> > index 30ee9f71bf0d..420af1e32d4e 100644
> > --- a/drivers/media/i2c/ov13858.c
> > +++ b/drivers/media/i2c/ov13858.c
> > @@ -1375,8 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
> > if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
> > fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
> >  
> > -   mode = v4l2_find_nearest_size(supported_modes, width, height,
> > - fmt->format.width, fmt->format.height);
> > +   mode = v4l2_find_nearest_size(
> > +   supported_modes, ARRAY_SIZE(supported_modes), width, height,
> > +   fmt->format.width, fmt->format.height);  
> 
> 
> Nitpick... I find ugly and arder to mentally parse things like the above,

"arder" -> "harder"

My keyboard sometimes is losing keystrokes. It seems it is approaching
the time to replace it again.


Thanks,
Mauro


Re: [PATCH v9.1] media: imx258: Add imx258 camera sensor driver

2018-03-23 Thread Tomasz Figa
On Fri, Mar 23, 2018 at 10:50 PM, Sakari Ailus
 wrote:
> Hi Tomasz,
>
> On Fri, Mar 23, 2018 at 08:43:50PM +0900, Tomasz Figa wrote:
>> Hi Andy,
>>
>> Some issues found when reviewing cherry pick of this patch to Chrome
>> OS kernel. Please see inline.
>>
>> On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh  wrote:
>>
>> [snip]
>>
>> > +   case V4L2_CID_VBLANK:
>> > +   /*
>> > +* Auto Frame Length Line Control is enabled by default.
>> > +* Not need control Vblank Register.
>> > +*/
>>
>> What is the meaning of this control then? Should it be read-only?
>
> The read-only flag is for the uAPI; the control framework still passes
> through changes to the control value done using kAPI to the driver.

The read-only flag is not even set in current code.

Also, I'm not sure about the control framework setting read-only
control. According to the code, it doesn't:
https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-ctrls.c#L2477

Best regards,
Tomasz


Re: [PATCH 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Mauro Carvalho Chehab
Em Fri, 23 Mar 2018 15:48:41 +0200
Sakari Ailus  escreveu:

> An older version of the driver patches were merged accidentally which
> resulted in missing the array_size parameter that tells the length of the
> array that contains the different supported sizes.
> 
> Bring it back to v4l2_find_nearest size and make the corresponding change
> for the drivers using it as well.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi Mauro,
> 
> Here's the patch I mentioned. It restores the intended state of the
> v4l2_find_nearest_size() API as it was reviewed and acked (by Hans).
> 
> This time the exact patch is tested for vivid.
> 
>  drivers/media/i2c/ov13858.c  | 5 +++--
>  drivers/media/i2c/ov5670.c   | 5 +++--
>  drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
>  include/media/v4l2-common.h  | 5 +++--
>  4 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> index 30ee9f71bf0d..420af1e32d4e 100644
> --- a/drivers/media/i2c/ov13858.c
> +++ b/drivers/media/i2c/ov13858.c
> @@ -1375,8 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
>   if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
>   fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
>  
> - mode = v4l2_find_nearest_size(supported_modes, width, height,
> -   fmt->format.width, fmt->format.height);
> + mode = v4l2_find_nearest_size(
> + supported_modes, ARRAY_SIZE(supported_modes), width, height,
> + fmt->format.width, fmt->format.height);


Nitpick... I find ugly and arder to mentally parse things like the above,
where all arguments are on the next line. Also, it doesn't follow the
coding style, as the parameters should be aligned with the parenthesis.

It would be better coded as:

mode = v4l2_find_nearest_size(supported_modes, 
  ARRAY_SIZE(supported_modes),
  width, height,
  fmt->format.width, fmt->format.height);

Same applies to the other occurrences of it.

>   ov13858_update_pad_format(mode, fmt);
>   if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>   framefmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index 556a95c30781..028abc860387 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -2229,8 +2229,9 @@ static int ov5670_set_pad_format(struct v4l2_subdev *sd,
>  
>   fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
>  
> - mode = v4l2_find_nearest_size(supported_modes, width, height,
> -   fmt->format.width, fmt->format.height);
> + mode = v4l2_find_nearest_size(
> + supported_modes, ARRAY_SIZE(supported_modes), width, height,
> + fmt->format.width, fmt->format.height);
>   ov5670_update_pad_format(mode, fmt);
>   if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>   *v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c 
> b/drivers/media/platform/vivid/vivid-vid-cap.c
> index 01c703683657..1599159f2574 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -561,8 +561,9 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>   mp->field = vivid_field_cap(dev, mp->field);
>   if (vivid_is_webcam(dev)) {
>   const struct v4l2_frmsize_discrete *sz =
> - v4l2_find_nearest_size(webcam_sizes, width, height,
> -mp->width, mp->height);
> + v4l2_find_nearest_size(webcam_sizes,
> +VIVID_WEBCAM_SIZES, width,
> +height, mp->width, mp->height);
>  
>   w = sz->width;
>   h = sz->height;
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 54b689247937..160bca96d524 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -320,6 +320,7 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
> int wmin,
>   *   set of resolutions contained in an array of a driver specific struct.
>   *
>   * @array: a driver specific array of image sizes
> + * @array_size: the length of the driver specific array of image sizes
>   * @width_field: the name of the width field in the driver specific struct
>   * @height_field: the name of the height field in the driver specific struct
>   * @width: desired width.
> @@ -332,13 +333,13 @@ void v4l_bound_align_image(unsigned int *width, 
> unsigned int wmin,
>   *
>   * Returns the best match or NULL if the length of the array is zero.
>   */
> -#define v4l2_find_nearest_size(array, width_field, height_field,

Re: [PATCH v9.1] media: imx258: Add imx258 camera sensor driver

2018-03-23 Thread Sakari Ailus
Hi Tomasz,

On Fri, Mar 23, 2018 at 08:43:50PM +0900, Tomasz Figa wrote:
> Hi Andy,
> 
> Some issues found when reviewing cherry pick of this patch to Chrome
> OS kernel. Please see inline.
> 
> On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh  wrote:
> 
> [snip]
> 
> > +   case V4L2_CID_VBLANK:
> > +   /*
> > +* Auto Frame Length Line Control is enabled by default.
> > +* Not need control Vblank Register.
> > +*/
> 
> What is the meaning of this control then? Should it be read-only?

The read-only flag is for the uAPI; the control framework still passes
through changes to the control value done using kAPI to the driver.

> 
> > +   break;
> > +   default:
> > +   dev_info(&client->dev,
> > +"ctrl(id:0x%x,val:0x%x) is not handled\n",
> > +ctrl->id, ctrl->val);
> > +   ret = -EINVAL;
> > +   break;
> > +   }
> > +
> > +   pm_runtime_put(&client->dev);
> > +
> > +   return ret;
> > +
> 
> [snip]
> 
> > +   v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx258_ctrl_ops,
> > +   V4L2_CID_TEST_PATTERN,
> > +   ARRAY_SIZE(imx258_test_pattern_menu) - 1,
> > +   0, 0, imx258_test_pattern_menu);
> 
> There is no code for handling this control in imx258_s_ctrl(). It's
> not a correct behavior to register a control, which isn't handled by
> the driver. Please either implement the control completely or remove
> it.

Indeed.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


[PATCH 1/1] v4l: Bring back array_size parameter to v4l2_find_nearest_size

2018-03-23 Thread Sakari Ailus
An older version of the driver patches were merged accidentally which
resulted in missing the array_size parameter that tells the length of the
array that contains the different supported sizes.

Bring it back to v4l2_find_nearest size and make the corresponding change
for the drivers using it as well.

Signed-off-by: Sakari Ailus 
---
Hi Mauro,

Here's the patch I mentioned. It restores the intended state of the
v4l2_find_nearest_size() API as it was reviewed and acked (by Hans).

This time the exact patch is tested for vivid.

 drivers/media/i2c/ov13858.c  | 5 +++--
 drivers/media/i2c/ov5670.c   | 5 +++--
 drivers/media/platform/vivid/vivid-vid-cap.c | 5 +++--
 include/media/v4l2-common.h  | 5 +++--
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 30ee9f71bf0d..420af1e32d4e 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1375,8 +1375,9 @@ ov13858_set_pad_format(struct v4l2_subdev *sd,
if (fmt->format.code != MEDIA_BUS_FMT_SGRBG10_1X10)
fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
-   mode = v4l2_find_nearest_size(supported_modes, width, height,
- fmt->format.width, fmt->format.height);
+   mode = v4l2_find_nearest_size(
+   supported_modes, ARRAY_SIZE(supported_modes), width, height,
+   fmt->format.width, fmt->format.height);
ov13858_update_pad_format(mode, fmt);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
framefmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 556a95c30781..028abc860387 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -2229,8 +2229,9 @@ static int ov5670_set_pad_format(struct v4l2_subdev *sd,
 
fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
-   mode = v4l2_find_nearest_size(supported_modes, width, height,
- fmt->format.width, fmt->format.height);
+   mode = v4l2_find_nearest_size(
+   supported_modes, ARRAY_SIZE(supported_modes), width, height,
+   fmt->format.width, fmt->format.height);
ov5670_update_pad_format(mode, fmt);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
*v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c 
b/drivers/media/platform/vivid/vivid-vid-cap.c
index 01c703683657..1599159f2574 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -561,8 +561,9 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
mp->field = vivid_field_cap(dev, mp->field);
if (vivid_is_webcam(dev)) {
const struct v4l2_frmsize_discrete *sz =
-   v4l2_find_nearest_size(webcam_sizes, width, height,
-  mp->width, mp->height);
+   v4l2_find_nearest_size(webcam_sizes,
+  VIVID_WEBCAM_SIZES, width,
+  height, mp->width, mp->height);
 
w = sz->width;
h = sz->height;
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 54b689247937..160bca96d524 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -320,6 +320,7 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  * set of resolutions contained in an array of a driver specific struct.
  *
  * @array: a driver specific array of image sizes
+ * @array_size: the length of the driver specific array of image sizes
  * @width_field: the name of the width field in the driver specific struct
  * @height_field: the name of the height field in the driver specific struct
  * @width: desired width.
@@ -332,13 +333,13 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  *
  * Returns the best match or NULL if the length of the array is zero.
  */
-#define v4l2_find_nearest_size(array, width_field, height_field, \
+#define v4l2_find_nearest_size(array, array_size, width_field, height_field, \
   width, height)   \
({  \
BUILD_BUG_ON(sizeof((array)->width_field) != sizeof(u32) || \
 sizeof((array)->height_field) != sizeof(u32)); \
(typeof(&(*(array__v4l2_find_nearest_size(  \
-   (array), ARRAY_SIZE(array), sizeof(*(array)),   \
+   (array), array_size, sizeof(*(array)),  \
offsetof(typeof(*(array)), width_field),\
offsetof(typeof(*(array

Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

2018-03-23 Thread jacopo mondi
Hi Todor,
   thanks for the patch.

When running checkpatch --strict I see a few warning you can easily
close (braces indentation mostly, and one additional empty line at
line 1048).

A few more nits below.

On Fri, Mar 23, 2018 at 12:14:20PM +0800, Todor Tomov wrote:
> The ov7251 sensor is a 1/7.5-Inch B&W VGA (640x480) CMOS Digital Image
> Sensor from Omnivision.
>
> The driver supports the following modes:
> - 640x480 30fps
> - 640x480 60fps
> - 640x480 90fps
>
> Output format is MIPI RAW 10.
>
> The driver supports configuration via user controls for:
> - exposure and gain;
> - horizontal and vertical flip;
> - test pattern.
>
> Signed-off-by: Todor Tomov 
> ---
>  drivers/media/i2c/Kconfig  |   13 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov7251.c | 1494 
> 
>  3 files changed, 1508 insertions(+)
>  create mode 100644 drivers/media/i2c/ov7251.c
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28..89aecb3 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -688,6 +688,19 @@ config VIDEO_OV5695
> To compile this driver as a module, choose M here: the
> module will be called ov5695.
>
> +config VIDEO_OV7251
> + tristate "OmniVision OV7251 sensor support"
> + depends on OF
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + select V4L2_FWNODE
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV7251 camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov7251.
> +
>  config VIDEO_OV772X
>   tristate "OmniVision OV772x sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee..c8585b1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV5695) += ov5695.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV7251) += ov7251.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
> new file mode 100644
> index 000..7b401a9
> --- /dev/null
> +++ b/drivers/media/i2c/ov7251.c
> @@ -0,0 +1,1494 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the OV7251 camera sensor.
> + *
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017-2018, Linaro Ltd.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define OV7251_VOLTAGE_ANALOG   280
> +#define OV7251_VOLTAGE_DIGITAL_CORE 150
> +#define OV7251_VOLTAGE_DIGITAL_IO   180
> +
> +#define OV7251_SC_MODE_SELECT0x0100
> +#define OV7251_SC_MODE_SELECT_SW_STANDBY 0x0
> +#define OV7251_SC_MODE_SELECT_STREAMING  0x1
> +
> +#define OV7251_CHIP_ID_HIGH  0x300a
> +#define OV7251_CHIP_ID_HIGH_BYTE 0x77
> +#define OV7251_CHIP_ID_LOW   0x300b
> +#define OV7251_CHIP_ID_LOW_BYTE  0x50
> +#define OV7251_SC_GP_IO_IN1  0x3029
> +#define OV7251_AEC_EXPO_00x3500
> +#define OV7251_AEC_EXPO_10x3501
> +#define OV7251_AEC_EXPO_20x3502
> +#define OV7251_AEC_AGC_ADJ_0 0x350a
> +#define OV7251_AEC_AGC_ADJ_1 0x350b
> +#define OV7251_TIMING_FORMAT10x3820
> +#define OV7251_TIMING_FORMAT1_VFLIP  BIT(2)
> +#define OV7251_TIMING_FORMAT20x3821
> +#define OV7251_TIMING_FORMAT2_MIRROR BIT(2)
> +#define OV7251_PRE_ISP_000x5e00
> +#define OV7251_PRE_ISP_00_TEST_PATTERN   BIT(7)
> +
> +struct reg_value {
> + u16 reg;
> + u8 val;
> +};
> +
> +struct ov7251_mode_info {
> + u32 width;
> + u32 height;
> + const struct reg_value *data;
> + u32 data_size;
> + u32 pixel_clock;
> + u32 link_freq;
> + u16 exposure_max;
> + u16 exposure_def;
> + struct v4l2_fract timeperframe;
> +};
> +
> +struct ov7251 {
> + struct i2c_client *i2c_client;
> + struct device *dev;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct v4l2_fwnode_endpoint ep;
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_rect crop;
> + struct clk *xclk;
> +
> + struct regulator *io_regulator;
> + struct regulator *core_regulator;
> + struct regulator *analog_regulator;
> +
> + const struct ov7251_mode_info *current_mode;
> +
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *pixel_clock;
> + stru

[GIT PULL for v4.16-rc7] media fixes

2018-03-23 Thread Mauro Carvalho Chehab
Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.16-4


For 3 fixes:
  - dvb: fix a Kconfig typo on a help text
  - tegra-cec: reset rx_buf_cnt when start bit detected
  - rc: lirc does not use LIRC_CAN_SEND_SCANCODE feature

Thanks!
Mauro

-

The following changes since commit 7dbdd16a79a9d27d7dca0a49029fc8966dcfecc5:

  media: vb2: Makefile: place vb2-trace together with vb2-core (2018-02-26 
11:39:04 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.16-4

for you to fetch changes up to 2c27476e398bfd9fa2572ff80a0de16f0becc900:

  media: dvb: fix a Kconfig typo (2018-03-05 07:57:41 -0500)


media fixes for v4.16-rc7


Hans Verkuil (1):
  media: tegra-cec: reset rx_buf_cnt when start bit detected

Michael Ira Krufky (1):
  media: dvb: fix a Kconfig typo

Sean Young (1):
  media: rc: lirc does not use LIRC_CAN_SEND_SCANCODE feature

 drivers/media/Kconfig|  2 +-
 drivers/media/platform/tegra-cec/tegra_cec.c | 17 +++--
 include/uapi/linux/lirc.h|  1 -
 3 files changed, 8 insertions(+), 12 deletions(-)



Re: [PATCH v3 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

2018-03-23 Thread Hans Verkuil
On 03/23/18 14:03, Fabio Estevam wrote:
> Hi Mauro and Hans,
> 
> On Sat, Mar 10, 2018 at 12:53 PM, Fabio Estevam  wrote:
>> Hi,
>>
>> On Sat, Mar 3, 2018 at 9:56 AM, Fabio Estevam  wrote:
>>> From: Fabio Estevam 
>>>
>>> Since commit 52e17089d185 ("media: imx: Don't initialize vars that
>>> won't be used") imx_csi_probe() fails to probe after propagating the
>>> devm_pinctrl_get_select_default() error.
>>>
>>> devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
>>> entry is not found, so better not to propagate the error in the -ENODEV
>>> case to avoid a regression.
>>>
>>> Suggested-by: Philipp Zabel 
>>> Signed-off-by: Fabio Estevam 
>>> Reviewed-by: Steve Longerbeam 
>>
>> A gentle ping.
>>
>> This series fixes a regression on the imx-media-csi driver.
> 
> Could you please consider applying this series that fixes the probe of
> the imx-media-csi driver?

It's delegated to Sakari.

Sakari, if you're busy then just let me know and I can take it.

Regards,

Hans


Re: [PATCH v3 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found

2018-03-23 Thread Fabio Estevam
Hi Mauro and Hans,

On Sat, Mar 10, 2018 at 12:53 PM, Fabio Estevam  wrote:
> Hi,
>
> On Sat, Mar 3, 2018 at 9:56 AM, Fabio Estevam  wrote:
>> From: Fabio Estevam 
>>
>> Since commit 52e17089d185 ("media: imx: Don't initialize vars that
>> won't be used") imx_csi_probe() fails to probe after propagating the
>> devm_pinctrl_get_select_default() error.
>>
>> devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
>> entry is not found, so better not to propagate the error in the -ENODEV
>> case to avoid a regression.
>>
>> Suggested-by: Philipp Zabel 
>> Signed-off-by: Fabio Estevam 
>> Reviewed-by: Steve Longerbeam 
>
> A gentle ping.
>
> This series fixes a regression on the imx-media-csi driver.

Could you please consider applying this series that fixes the probe of
the imx-media-csi driver?

Thanks


[PATCHv2 3/3] arm64: dts: meson-gx.dtsi: add cec-disable

2018-03-23 Thread Hans Verkuil
From: Hans Verkuil 

The meson-gx boards have two CEC controllers: the DesignWare controller
and a meson-specific controller. Disable the DW controller since the
CEC line is hooked up to the meson controller.

Signed-off-by: Hans Verkuil 
Acked-by: Neil Armstrong 
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 4ee2e7951482..4e2567012335 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -544,6 +544,7 @@
interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
+   cec-disable;
status = "disabled";
 
/* VPU VENC Input */
-- 
2.15.1



[PATCHv2 0/3] dw-hdmi: add property to disable CEC

2018-03-23 Thread Hans Verkuil
From: Hans Verkuil 

Some boards (amlogic) have two CEC controllers: the DesignWare controller
and their own CEC controller (meson ao-cec).

Since the CEC line is not hooked up to the DW controller we need a way
to disable that controller. This patch series adds the cec-disable
property for that purpose.

Regards,

Hans

Changes since v1:

- Move the dts change to meson-gx.dtsi since according to Neil it is
  valid for all meson-gx boards.
- Fix bad subject line of patch 1.

Hans Verkuil (3):
  dt-bindings: display: dw_hdmi.txt: add cec-disable property
  drm: bridge: dw-hdmi: check the cec-disable property
  arm64: dts: meson-gx.dtsi: add cec-disable

 Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi| 1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.15.1



[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

2018-03-23 Thread Hans Verkuil
From: Hans Verkuil 

Some boards have both a DesignWare and their own CEC controller.
The CEC pin is only hooked up to their own CEC controller and not
to the DW controller.

Add the cec-disable property to disable the DW CEC controller.

This particular situation happens on Amlogic boards that have their
own meson CEC controller.

Signed-off-by: Hans Verkuil 
Acked-by: Neil Armstrong 
---
 Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt 
b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
index 33bf981fbe33..4a13f4858bc0 100644
--- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
@@ -27,6 +27,9 @@ responsible for defining whether each property is required or 
optional.
   - "isfr" is the internal register configuration clock (mandatory).
   - "cec" is the HDMI CEC controller main clock (optional).
 
+- cec-disable: Do not use the DWC CEC controller since the CEC line is not
+  hooked up even though the CEC DWC IP is present.
+
 - ports: The connectivity of the DWC HDMI TX with the rest of the system is
   expressed in using ports as specified in the device graph bindings defined
   in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
-- 
2.15.1



[PATCHv2 2/3] drm: bridge: dw-hdmi: check the cec-disable property

2018-03-23 Thread Hans Verkuil
From: Hans Verkuil 

If the cec-disable property was set, then disable the DW CEC
controller. This is needed for boards that have their own
CEC controller.

Signed-off-by: Hans Verkuil 
Reviewed-by: Neil Armstrong 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a38db40ce990..597220e40541 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2508,7 +2508,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->audio = platform_device_register_full(&pdevinfo);
}
 
-   if (config0 & HDMI_CONFIG0_CEC) {
+   if ((config0 & HDMI_CONFIG0_CEC) &&
+   !of_property_read_bool(np, "cec-disable")) {
cec.hdmi = hdmi;
cec.ops = &dw_hdmi_cec_ops;
cec.irq = irq;
-- 
2.15.1



[PATCH] media: fimc-capture: get rid of two warnings

2018-03-23 Thread Mauro Carvalho Chehab
Smatch produces two warnings when building this file:
./arch/x86/include/asm/bitops.h:433:22: warning: asm output is not an 
lvalue
./arch/x86/include/asm/bitops.h:433:22: warning: asm output is not an 
lvalue

On some asm instructions.

I suspect that those asm instructions might not be producing the
right code, so, better to use two intermediate vars, get rid of
the warnings and of the risk of producing a wrong code.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/exynos4-is/fimc-capture.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c 
b/drivers/media/platform/exynos4-is/fimc-capture.c
index ed9302caa004..a3cdac188190 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -670,10 +670,13 @@ static void fimc_capture_try_selection(struct fimc_ctx 
*ctx,
return;
}
if (target == V4L2_SEL_TGT_COMPOSE) {
+   u32 tmp_min_h = ffs(sink->width) - 3;
+   u32 tmp_min_v = ffs(sink->height) - 1;
+
if (ctx->rotation != 90 && ctx->rotation != 270)
align_h = 1;
-   max_sc_h = min(SCALER_MAX_HRATIO, 1 << (ffs(sink->width) - 3));
-   max_sc_v = min(SCALER_MAX_VRATIO, 1 << (ffs(sink->height) - 1));
+   max_sc_h = min(SCALER_MAX_HRATIO, 1 << tmp_min_h);
+   max_sc_v = min(SCALER_MAX_VRATIO, 1 << tmp_min_v);
min_sz = var->min_out_pixsize;
} else {
u32 depth = fimc_get_format_depth(sink->fmt);
-- 
2.14.3



[PATCH] media: dvb-usb-v2: fix a missing dependency of I2C_MUX

2018-03-23 Thread Mauro Carvalho Chehab
Now that af9015 requires I2C_MUX, all drivers that select
it should also depend on it.

   drivers/media/dvb-frontends/af9013.o: In function `af9013_remove':
>> drivers/media/dvb-frontends/af9013.c:1560: undefined reference to 
>> `i2c_mux_del_adapters'
   drivers/media/dvb-frontends/af9013.o: In function `af9013_probe':
>> drivers/media/dvb-frontends/af9013.c:1488: undefined reference to 
>> `i2c_mux_alloc'
>> drivers/media/dvb-frontends/af9013.c:1495: undefined reference to 
>> `i2c_mux_add_adapter'
   drivers/media/dvb-frontends/af9013.c:1544: undefined reference to 
`i2c_mux_del_adapters'

Reported-by: kbuild test robot 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/dvb-usb-v2/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/dvb-usb-v2/Kconfig 
b/drivers/media/usb/dvb-usb-v2/Kconfig
index 09a52aae299a..37053477b84d 100644
--- a/drivers/media/usb/dvb-usb-v2/Kconfig
+++ b/drivers/media/usb/dvb-usb-v2/Kconfig
@@ -15,7 +15,7 @@ config DVB_USB_V2
 
 config DVB_USB_AF9015
tristate "Afatech AF9015 DVB-T USB2.0 support"
-   depends on DVB_USB_V2
+   depends on DVB_USB_V2 && I2C_MUX
select REGMAP
select DVB_AF9013
select DVB_PLL  if MEDIA_SUBDRV_AUTOSELECT
-- 
2.14.3



Re: [PATCH 07/30] media: v4l2-tpg-core: avoid buffer overflows

2018-03-23 Thread Hans Verkuil
On 03/23/18 12:56, Mauro Carvalho Chehab wrote:
> Fix the following warnings:
>   drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1146 gen_twopix() error: 
> buffer overflow 'buf[1]' 8 <= 8
>   drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1152 gen_twopix() error: 
> buffer overflow 'buf[1]' 8 <= 8
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c 
> b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> index d248d1fb9d1d..37632bc524d4 100644
> --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> @@ -1143,13 +1143,13 @@ static void gen_twopix(struct tpg_data *tpg,
>   case V4L2_PIX_FMT_NV24:
>   buf[0][offset] = r_y_h;
>   buf[1][2 * offset] = g_u_s;
> - buf[1][2 * offset + 1] = b_v;
> + buf[1][(2 * offset + 1) % 8] = b_v;
>   break;
>  
>   case V4L2_PIX_FMT_NV42:
>   buf[0][offset] = r_y_h;
>   buf[1][2 * offset] = b_v;
> - buf[1][2 * offset + 1] = g_u_s;
> + buf[1][(2 * offset + 1) %8] = g_u_s;

Space after '%'

>   break;
>  
>   case V4L2_PIX_FMT_YUYV:
> 

Nice! I always wondered how to fix this bogus error, but this will do it.

After fixing the space:

Reviewed-by: Hans Verkuil 

Thanks,

Hans


[PATCH] media: uvc: to the right check at uvc_ioctl_enum_framesizes()

2018-03-23 Thread Mauro Carvalho Chehab
While the logic there is correct, it tricks both humans and
machines, a the check if "i" var is not zero is actually to
validate if the "frames" var was initialized when the loop
ran for the first time.

That produces the following warning:
drivers/media/usb/uvc/uvc_v4l2.c:1192 uvc_ioctl_enum_framesizes() 
error: potentially dereferencing uninitialized 'frame'.

Change the logic to do the right test instead.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 818a4369a51a..bd32914259ae 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1173,7 +1173,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, 
void *fh,
struct uvc_fh *handle = fh;
struct uvc_streaming *stream = handle->stream;
struct uvc_format *format = NULL;
-   struct uvc_frame *frame;
+   struct uvc_frame *frame = NULL;
unsigned int index;
unsigned int i;
 
@@ -1189,7 +1189,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, 
void *fh,
 
/* Skip duplicate frame sizes */
for (i = 0, index = 0; i < format->nframes; i++) {
-   if (i && frame->wWidth == format->frame[i].wWidth &&
+   if (frame && frame->wWidth == format->frame[i].wWidth &&
frame->wHeight == format->frame[i].wHeight)
continue;
frame = &format->frame[i];
-- 
2.14.3



Re: [PATCH 30/30] media: cec-core: fix a bug at cec_error_inj_write()

2018-03-23 Thread Hans Verkuil
On 03/23/18 12:57, Mauro Carvalho Chehab wrote:
> If the adapter doesn't have error_inj_parse_line() ops, the
> write() logic won't return -EINVAL, but, instead, it will keep
> looping, because "count" is a non-negative number.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Reviewed-by: Hans Verkuil 

Thanks!

Hans

> ---
>  drivers/media/cec/cec-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> index ea3eccfdba15..b0c87f9ea08f 100644
> --- a/drivers/media/cec/cec-core.c
> +++ b/drivers/media/cec/cec-core.c
> @@ -209,14 +209,14 @@ static ssize_t cec_error_inj_write(struct file *file,
>   if (IS_ERR(buf))
>   return PTR_ERR(buf);
>   p = buf;
> - while (p && *p && count >= 0) {
> + while (p && *p) {
>   p = skip_spaces(p);
>   line = strsep(&p, "\n");
>   if (!*line || *line == '#')
>   continue;
>   if (!adap->ops->error_inj_parse_line(adap, line)) {
> - count = -EINVAL;
> - break;
> + kfree(buf);
> + return -EINVAL;
>   }
>   }
>   kfree(buf);
> 



Re: [PATCH 02/30] media: imx-media-utils: fix a warning

2018-03-23 Thread Dan Carpenter
On Fri, Mar 23, 2018 at 07:56:48AM -0400, Mauro Carvalho Chehab wrote:
> The logic at find_format() is a little bit confusing even for
> humans, and it tricks static code analyzers:
> 
>   drivers/staging/media/imx/imx-media-utils.c:259 find_format() error: 
> buffer overflow 'array' 14 <= 20

It's always good to simplify the code, but I have a fix for this that I
will publish very soon.

regards,
dan carpenter




[PATCH 10/30] media: tvaudio: improve error handling

2018-03-23 Thread Mauro Carvalho Chehab
The error handling logic at tvaudio is broken on several ways,
as it doesn't really check right when an error occurs.

Change it to return the proper error code from read/write
routines and fix the errors on reads.

Shuts up the following warnings:
drivers/media/i2c/tvaudio.c:222 chip_read() error: uninitialized symbol 
'buffer'.
drivers/media/i2c/tvaudio.c:223 chip_read() error: uninitialized symbol 
'buffer'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/tvaudio.c | 92 +++--
 1 file changed, 72 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index 772164b848ef..5919214a56bf 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -156,14 +156,18 @@ static int chip_write(struct CHIPSTATE *chip, int 
subaddr, int val)
struct v4l2_subdev *sd = &chip->sd;
struct i2c_client *c = v4l2_get_subdevdata(sd);
unsigned char buffer[2];
+   int rc;
 
if (subaddr < 0) {
v4l2_dbg(1, debug, sd, "chip_write: 0x%x\n", val);
chip->shadow.bytes[1] = val;
buffer[0] = val;
-   if (1 != i2c_master_send(c, buffer, 1)) {
+   rc = i2c_master_send(c, buffer, 1);
+   if (rc != 1) {
v4l2_warn(sd, "I/O error (write 0x%x)\n", val);
-   return -1;
+   if (rc < 0)
+   return rc;
+   return -EIO;
}
} else {
if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes)) {
@@ -178,10 +182,13 @@ static int chip_write(struct CHIPSTATE *chip, int 
subaddr, int val)
chip->shadow.bytes[subaddr+1] = val;
buffer[0] = subaddr;
buffer[1] = val;
-   if (2 != i2c_master_send(c, buffer, 2)) {
+   rc = i2c_master_send(c, buffer, 2);
+   if (rc != 2) {
v4l2_warn(sd, "I/O error (write reg%d=0x%x)\n",
subaddr, val);
-   return -1;
+   if (rc < 0)
+   return rc;
+   return -EIO;
}
}
return 0;
@@ -214,10 +221,14 @@ static int chip_read(struct CHIPSTATE *chip)
struct v4l2_subdev *sd = &chip->sd;
struct i2c_client *c = v4l2_get_subdevdata(sd);
unsigned char buffer;
+   int rc;
 
-   if (1 != i2c_master_recv(c, &buffer, 1)) {
+   rc = i2c_master_recv(c, &buffer, 1);
+   if (rc != 1) {
v4l2_warn(sd, "I/O error (read)\n");
-   return -1;
+   if (rc < 0)
+   return rc;
+   return -EIO;
}
v4l2_dbg(1, debug, sd, "chip_read: 0x%x\n", buffer);
return buffer;
@@ -227,6 +238,7 @@ static int chip_read2(struct CHIPSTATE *chip, int subaddr)
 {
struct v4l2_subdev *sd = &chip->sd;
struct i2c_client *c = v4l2_get_subdevdata(sd);
+   int rc;
unsigned char write[1];
unsigned char read[1];
struct i2c_msg msgs[2] = {
@@ -245,9 +257,12 @@ static int chip_read2(struct CHIPSTATE *chip, int subaddr)
 
write[0] = subaddr;
 
-   if (2 != i2c_transfer(c->adapter, msgs, 2)) {
+   rc = i2c_transfer(c->adapter, msgs, 2);
+   if (rc != 2) {
v4l2_warn(sd, "I/O error (read2)\n");
-   return -1;
+   if (rc < 0)
+   return rc;
+   return -EIO;
}
v4l2_dbg(1, debug, sd, "chip_read2: reg%d=0x%x\n",
subaddr, read[0]);
@@ -258,7 +273,7 @@ static int chip_cmd(struct CHIPSTATE *chip, char *name, 
audiocmd *cmd)
 {
struct v4l2_subdev *sd = &chip->sd;
struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int i;
+   int i, rc;
 
if (0 == cmd->count)
return 0;
@@ -284,9 +299,12 @@ static int chip_cmd(struct CHIPSTATE *chip, char *name, 
audiocmd *cmd)
printk(KERN_CONT "\n");
 
/* send data to the chip */
-   if (cmd->count != i2c_master_send(c, cmd->bytes, cmd->count)) {
+   rc = i2c_master_send(c, cmd->bytes, cmd->count);
+   if (rc != cmd->count) {
v4l2_warn(sd, "I/O error (%s)\n", name);
-   return -1;
+   if (rc < 0)
+   return rc;
+   return -EIO;
}
return 0;
 }
@@ -400,8 +418,12 @@ static int tda9840_getrxsubchans(struct CHIPSTATE *chip)
struct v4l2_subdev *sd = &chip->sd;
int val, mode;
 
-   val = chip_read(chip);
mode = V4L2_TUNER_SUB_MONO;
+
+   val = chip_read(chip);
+   if (val < 0)
+   return mode;
+
if (val & TDA9840_DS_DUAL)
mode |= V4L2_TUNER_SUB_LANG1 | V4L2_TUNER_SUB_LANG2;
if (val & TDA98

[PATCH 06/30] media: ov5670: get rid of a series of __be warnings

2018-03-23 Thread Mauro Carvalho Chehab
There are some troubles on this driver with respect to the usage
of __be16 and __b32 macros:

drivers/media/i2c/ov5670.c:1857:27: warning: incorrect type in 
initializer (different base types)
drivers/media/i2c/ov5670.c:1857:27:expected unsigned short 
[unsigned] [usertype] reg_addr_be
drivers/media/i2c/ov5670.c:1857:27:got restricted __be16 [usertype] 

drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1880:16: warning: cast to restricted __be32
drivers/media/i2c/ov5670.c:1901:13: warning: incorrect type in 
assignment (different base types)
drivers/media/i2c/ov5670.c:1901:13:expected unsigned int [unsigned] 
[usertype] val
drivers/media/i2c/ov5670.c:1901:13:got restricted __be32 [usertype] 


Fix them.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/ov5670.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 556a95c30781..d2db480da1b9 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -1853,8 +1853,8 @@ static int ov5670_read_reg(struct ov5670 *ov5670, u16 
reg, unsigned int len,
struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd);
struct i2c_msg msgs[2];
u8 *data_be_p;
-   u32 data_be = 0;
-   u16 reg_addr_be = cpu_to_be16(reg);
+   __be32 data_be = 0;
+   __be16 reg_addr_be = cpu_to_be16(reg);
int ret;
 
if (len > 4)
@@ -1891,6 +1891,7 @@ static int ov5670_write_reg(struct ov5670 *ov5670, u16 
reg, unsigned int len,
int val_i;
u8 buf[6];
u8 *val_p;
+   __be32 tmp;
 
if (len > 4)
return -EINVAL;
@@ -1898,8 +1899,8 @@ static int ov5670_write_reg(struct ov5670 *ov5670, u16 
reg, unsigned int len,
buf[0] = reg >> 8;
buf[1] = reg & 0xff;
 
-   val = cpu_to_be32(val);
-   val_p = (u8 *)&val;
+   tmp = cpu_to_be32(val);
+   val_p = (u8 *)&tmp;
buf_i = 2;
val_i = 4 - len;
 
-- 
2.14.3



[PATCH 09/30] media: sp887x: fix a warning

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/dvb-frontends/sp887x.c:179 sp887x_initial_setup() error: memcpy() 
'&buf[2]' too small (30 vs 16384)

This is actually a false alarm, but reverting the check order
makes not only for humans to review the code, but also cleans
the warning.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/sp887x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/sp887x.c 
b/drivers/media/dvb-frontends/sp887x.c
index 572a297811fe..f39d566d7d1d 100644
--- a/drivers/media/dvb-frontends/sp887x.c
+++ b/drivers/media/dvb-frontends/sp887x.c
@@ -136,7 +136,7 @@ static void sp887x_setup_agc (struct sp887x_state* state)
 static int sp887x_initial_setup (struct dvb_frontend* fe, const struct 
firmware *fw)
 {
struct sp887x_state* state = fe->demodulator_priv;
-   u8 buf [BLOCKSIZE+2];
+   u8 buf [BLOCKSIZE + 2];
int i;
int fw_size = fw->size;
const unsigned char *mem = fw->data;
@@ -144,7 +144,7 @@ static int sp887x_initial_setup (struct dvb_frontend* fe, 
const struct firmware
dprintk("%s\n", __func__);
 
/* ignore the first 10 bytes, then we expect 0x4000 bytes of firmware */
-   if (fw_size < FW_SIZE+10)
+   if (fw_size < FW_SIZE + 10)
return -ENODEV;
 
mem = fw->data + 10;
@@ -167,7 +167,7 @@ static int sp887x_initial_setup (struct dvb_frontend* fe, 
const struct firmware
int c = BLOCKSIZE;
int err;
 
-   if (i+c > FW_SIZE)
+   if (c > FW_SIZE - i)
c = FW_SIZE - i;
 
/* bit 0x8000 in address is set to enable 13bit mode */
-- 
2.14.3



[PATCH 16/30] media: videobuf-dma-sg: Fix a weird cast

2018-03-23 Thread Mauro Carvalho Chehab
Just use %p. Fixes this warning:
drivers/media/v4l2-core/videobuf-dma-sg.c:247 
videobuf_dma_init_kernel() warn: argument 2 to %08lx specifier is cast from 
pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/v4l2-core/videobuf-dma-sg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index f412429cf5ba..add2edb23eac 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -244,9 +244,8 @@ static int videobuf_dma_init_kernel(struct videobuf_dmabuf 
*dma, int direction,
goto out_free_pages;
}
 
-   dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",
-   (unsigned long)dma->vaddr,
-   nr_pages << PAGE_SHIFT);
+   dprintk(1, "vmalloc is at addr %p, size=%d\n",
+   dma->vaddr, nr_pages << PAGE_SHIFT);
 
memset(dma->vaddr, 0, nr_pages << PAGE_SHIFT);
dma->nr_pages = nr_pages;
-- 
2.14.3



[PATCH 20/30] media: ir-kbd-i2c: improve error handling code

2018-03-23 Thread Mauro Carvalho Chehab
The current I2C error handling logic makes static analyzers
confused, and it doesn't follow the coding style we're using:

drivers/media/i2c/ir-kbd-i2c.c:180 get_key_pixelview() error: 
uninitialized symbol 'b'.
drivers/media/i2c/ir-kbd-i2c.c:224 get_key_knc1() error: uninitialized 
symbol 'b'.
drivers/media/i2c/ir-kbd-i2c.c:226 get_key_knc1() error: uninitialized 
symbol 'b'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/ir-kbd-i2c.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 193020d64e51..1d11aab1817a 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -168,11 +168,15 @@ static int get_key_haup_xvr(struct IR_i2c *ir, enum 
rc_proto *protocol,
 static int get_key_pixelview(struct IR_i2c *ir, enum rc_proto *protocol,
 u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char b;
 
/* poll IR chip */
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
dev_dbg(&ir->rc->dev, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -185,11 +189,15 @@ static int get_key_pixelview(struct IR_i2c *ir, enum 
rc_proto *protocol,
 static int get_key_fusionhdtv(struct IR_i2c *ir, enum rc_proto *protocol,
  u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char buf[4];
 
/* poll IR chip */
-   if (4 != i2c_master_recv(ir->c, buf, 4)) {
+   rc = i2c_master_recv(ir->c, buf, 4);
+   if (rc != 4) {
dev_dbg(&ir->rc->dev, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -209,11 +217,15 @@ static int get_key_fusionhdtv(struct IR_i2c *ir, enum 
rc_proto *protocol,
 static int get_key_knc1(struct IR_i2c *ir, enum rc_proto *protocol,
u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char b;
 
/* poll IR chip */
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
dev_dbg(&ir->rc->dev, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
-- 
2.14.3



[PATCH 17/30] media: ivtvfb: Cleanup some warnings

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/pci/ivtv/ivtvfb.c:349 ivtvfb_prep_frame() warn: argument 3 to 
%08lx specifier is cast from pointer
drivers/media/pci/ivtv/ivtvfb.c:360 ivtvfb_prep_frame() warn: argument 3 to 
%08lx specifier is cast from pointer
drivers/media/pci/ivtv/ivtvfb.c:363 ivtvfb_prep_frame() warn: argument 4 to 
%08lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtvfb.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 621b2f613d81..8e62b8be6529 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -346,8 +346,8 @@ static int ivtvfb_prep_frame(struct ivtv *itv, int cmd, 
void __user *source,
 
/* Not fatal, but will have undesirable results */
if ((unsigned long)source & 3)
-   IVTVFB_WARN("ivtvfb_prep_frame: Source address not 32 bit 
aligned (0x%08lx)\n",
-   (unsigned long)source);
+   IVTVFB_WARN("ivtvfb_prep_frame: Source address not 32 bit 
aligned (%p)\n",
+   source);
 
if (dest_offset & 3)
IVTVFB_WARN("ivtvfb_prep_frame: Dest offset not 32 bit aligned 
(%ld)\n", dest_offset);
@@ -357,12 +357,10 @@ static int ivtvfb_prep_frame(struct ivtv *itv, int cmd, 
void __user *source,
 
/* Check Source */
if (!access_ok(VERIFY_READ, source + dest_offset, count)) {
-   IVTVFB_WARN("Invalid userspace pointer 0x%08lx\n",
-   (unsigned long)source);
+   IVTVFB_WARN("Invalid userspace pointer %p\n", source);
 
-   IVTVFB_DEBUG_WARN("access_ok() failed for offset 0x%08lx source 
0x%08lx count %d\n",
-   dest_offset, (unsigned long)source,
-   count);
+   IVTVFB_DEBUG_WARN("access_ok() failed for offset 0x%08lx source 
%p count %d\n",
+ dest_offset, source, count);
return -EINVAL;
}
 
-- 
2.14.3



[PATCH 02/30] media: imx-media-utils: fix a warning

2018-03-23 Thread Mauro Carvalho Chehab
The logic at find_format() is a little bit confusing even for
humans, and it tricks static code analyzers:

drivers/staging/media/imx/imx-media-utils.c:259 find_format() error: 
buffer overflow 'array' 14 <= 20

Rewrite the logic in a way that it makes it clearer to understand,
while prevent static analyzers to produce false positives.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/imx/imx-media-utils.c | 81 +++--
 1 file changed, 43 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index 40bcb8fb18b9..fab98fc0d6a0 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -225,58 +225,63 @@ static void init_mbus_colorimetry(struct 
v4l2_mbus_framefmt *mbus,
  mbus->ycbcr_enc);
 }
 
+static const
+struct imx_media_pixfmt *__find_format(u32 fourcc,
+  u32 code,
+  bool allow_non_mbus,
+  bool allow_bayer,
+  const struct imx_media_pixfmt *array,
+  u32 array_size)
+{
+   const struct imx_media_pixfmt *fmt;
+   int i, j;
+
+   for (i = 0; i < array_size; i++) {
+   fmt = &array[i];
+
+   if ((!allow_non_mbus && !fmt->codes[0]) ||
+   (!allow_bayer && fmt->bayer))
+   continue;
+
+   if (fourcc && fmt->fourcc == fourcc)
+   return fmt;
+
+   if (!code)
+   continue;
+
+   for (j = 0; fmt->codes[j]; j++) {
+   if (code == fmt->codes[j])
+   return fmt;
+   }
+   }
+   return NULL;
+}
+
 static const struct imx_media_pixfmt *find_format(u32 fourcc,
  u32 code,
  enum codespace_sel cs_sel,
  bool allow_non_mbus,
  bool allow_bayer)
 {
-   const struct imx_media_pixfmt *array, *fmt, *ret = NULL;
-   u32 array_size;
-   int i, j;
+   const struct imx_media_pixfmt *ret;
 
switch (cs_sel) {
case CS_SEL_YUV:
-   array_size = NUM_YUV_FORMATS;
-   array = yuv_formats;
-   break;
+   return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+yuv_formats, NUM_YUV_FORMATS);
case CS_SEL_RGB:
-   array_size = NUM_RGB_FORMATS;
-   array = rgb_formats;
-   break;
+   return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+rgb_formats, NUM_RGB_FORMATS);
case CS_SEL_ANY:
-   array_size = NUM_YUV_FORMATS + NUM_RGB_FORMATS;
-   array = yuv_formats;
-   break;
+   ret = __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+   yuv_formats, NUM_YUV_FORMATS);
+   if (ret)
+   return ret;
+   return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+rgb_formats, NUM_RGB_FORMATS);
default:
return NULL;
}
-
-   for (i = 0; i < array_size; i++) {
-   if (cs_sel == CS_SEL_ANY && i >= NUM_YUV_FORMATS)
-   fmt = &rgb_formats[i - NUM_YUV_FORMATS];
-   else
-   fmt = &array[i];
-
-   if ((!allow_non_mbus && fmt->codes[0] == 0) ||
-   (!allow_bayer && fmt->bayer))
-   continue;
-
-   if (fourcc && fmt->fourcc == fourcc) {
-   ret = fmt;
-   goto out;
-   }
-
-   for (j = 0; code && fmt->codes[j]; j++) {
-   if (code == fmt->codes[j]) {
-   ret = fmt;
-   goto out;
-   }
-   }
-   }
-
-out:
-   return ret;
 }
 
 static int enum_format(u32 *fourcc, u32 *code, u32 index,
-- 
2.14.3



[PATCH 12/30] media: solo6x10: simplify the logic at solo_p2m_dma_desc()

2018-03-23 Thread Mauro Carvalho Chehab
The logic with gets a p2m_id is more complex than needed,
causing false positives with static analyzers:

drivers/media/pci/solo6x10/solo6x10-p2m.c:81 solo_p2m_dma_desc() error: 
buffer overflow 'solo_dev->p2m_dev' 4 <= s32max

Make it simpler and use unsigned int.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/solo6x10/solo6x10-p2m.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-p2m.c 
b/drivers/media/pci/solo6x10/solo6x10-p2m.c
index 8c8484674d2f..46c30430e30b 100644
--- a/drivers/media/pci/solo6x10/solo6x10-p2m.c
+++ b/drivers/media/pci/solo6x10/solo6x10-p2m.c
@@ -69,14 +69,11 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
unsigned int timeout;
unsigned int config = 0;
int ret = 0;
-   int p2m_id = 0;
+   unsigned int p2m_id = 0;
 
/* Get next ID. According to Softlogic, 6110 has problems on !=0 P2M */
-   if (solo_dev->type != SOLO_DEV_6110 && multi_p2m) {
+   if (solo_dev->type != SOLO_DEV_6110 && multi_p2m)
p2m_id = atomic_inc_return(&solo_dev->p2m_count) % SOLO_NR_P2M;
-   if (p2m_id < 0)
-   p2m_id = -p2m_id;
-   }
 
p2m_dev = &solo_dev->p2m_dev[p2m_id];
 
-- 
2.14.3



[PATCH 04/30] media: vpss: fix annotations for vpss_regs_base2

2018-03-23 Thread Mauro Carvalho Chehab
Fix those warnings:

drivers/media/platform/davinci/vpss.c:510:25: warning: incorrect type 
in argument 1 (different address spaces)
drivers/media/platform/davinci/vpss.c:510:25:expected void volatile 
[noderef] *addr
drivers/media/platform/davinci/vpss.c:510:25:got unsigned int 
[usertype] *static [toplevel] [assigned] vpss_regs_base2
drivers/media/platform/davinci/vpss.c:520:34: warning: incorrect type 
in assignment (different address spaces)
drivers/media/platform/davinci/vpss.c:520:34:expected unsigned int 
[usertype] *static [toplevel] [assigned] vpss_regs_base2
drivers/media/platform/davinci/vpss.c:520:34:got void [noderef] 
*
drivers/media/platform/davinci/vpss.c:522:54: warning: incorrect type 
in argument 2 (different address spaces)
drivers/media/platform/davinci/vpss.c:522:54:expected void volatile 
[noderef] *addr
drivers/media/platform/davinci/vpss.c:522:54:got unsigned int 
[usertype] *static [toplevel] [assigned] vpss_regs_base2

Weird enough, vpss_regs_base0 and vpss_regs_base1 were
properly annotated.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/davinci/vpss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpss.c 
b/drivers/media/platform/davinci/vpss.c
index b73886519f4f..19cf6853411e 100644
--- a/drivers/media/platform/davinci/vpss.c
+++ b/drivers/media/platform/davinci/vpss.c
@@ -116,7 +116,7 @@ struct vpss_hw_ops {
 struct vpss_oper_config {
__iomem void *vpss_regs_base0;
__iomem void *vpss_regs_base1;
-   resource_size_t *vpss_regs_base2;
+   __iomem void *vpss_regs_base2;
enum vpss_platform_type platform;
spinlock_t vpss_lock;
struct vpss_hw_ops hw_ops;
-- 
2.14.3



[PATCH 29/30] media: tda9840: cleanup a warning

2018-03-23 Thread Mauro Carvalho Chehab
There's a false positive warning there:
drivers/media/i2c/tda9840.c:79 tda9840_status() error: uninitialized 
symbol 'byte'.

Change the code to match our coding style, in order to fix it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/tda9840.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tda9840.c b/drivers/media/i2c/tda9840.c
index f31e659588ac..0dd6ff3e6201 100644
--- a/drivers/media/i2c/tda9840.c
+++ b/drivers/media/i2c/tda9840.c
@@ -68,11 +68,15 @@ static void tda9840_write(struct v4l2_subdev *sd, u8 reg, 
u8 val)
 static int tda9840_status(struct v4l2_subdev *sd)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   int rc;
u8 byte;
 
-   if (1 != i2c_master_recv(client, &byte, 1)) {
+   rc = i2c_master_recv(client, &byte, 1);
+   if (rc != 1) {
v4l2_dbg(1, debug, sd,
"i2c_master_recv() failed\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
-- 
2.14.3



[PATCH 01/30] media: dvbdev: handle ENOMEM error at dvb_module_probe()

2018-03-23 Thread Mauro Carvalho Chehab
If allocation of struct board_info fails, return NULL from
dvb_module_probe().

Fix this warning:
drivers/media/dvb-core/dvbdev.c:958 dvb_module_probe() error: potential 
null dereference 'board_info'.  (kzalloc returns null)

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvbdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index cf747d753a79..787fe06df217 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -953,6 +953,8 @@ struct i2c_client *dvb_module_probe(const char *module_name,
struct i2c_board_info *board_info;
 
board_info = kzalloc(sizeof(*board_info), GFP_KERNEL);
+   if (!board_info)
+   return NULL;
 
if (name)
strlcpy(board_info->type, name, I2C_NAME_SIZE);
-- 
2.14.3



[PATCH 27/30] media: em28xx-input: improve error handling code

2018-03-23 Thread Mauro Carvalho Chehab
The current I2C error handling logic makes static analyzers
confused:

drivers/media/usb/em28xx/em28xx-input.c:96 em28xx_get_key_terratec() 
error: uninitialized symbol 'b'.

Change it to match the coding style we're using elsewhere.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-input.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
b/drivers/media/usb/em28xx/em28xx-input.c
index eb2ec0384b69..2dc1be00b8b8 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -82,11 +82,16 @@ struct em28xx_IR {
 static int em28xx_get_key_terratec(struct i2c_client *i2c_dev,
   enum rc_proto *protocol, u32 *scancode)
 {
+   int rc;
unsigned char b;
 
/* poll IR chip */
-   if (i2c_master_recv(i2c_dev, &b, 1) != 1)
+   rc = i2c_master_recv(i2c_dev, &b, 1);
+   if (rc != 1) {
+   if (rc < 0)
+   return rc;
return -EIO;
+   }
 
/*
 * it seems that 0xFE indicates that a button is still hold
-- 
2.14.3



[PATCH 21/30] media: ir-kbd-i2c: change the if logic to avoid a warning

2018-03-23 Thread Mauro Carvalho Chehab
While the code is correct, it produces this warning:
drivers/media/i2c/ir-kbd-i2c.c:593 zilog_ir_format() error: buffer 
overflow 'code_block->codes' 61 <= 173

As static analyzers may be tricked by arithmetic expressions on
comparisions. So, change the order, in order to shut up this
false-positive warning.

That also makes easier for humans to understand that it won't
be trying to go past buffer size.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/ir-kbd-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 1d11aab1817a..a7e23bcf845c 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -583,7 +583,7 @@ static int zilog_ir_format(struct rc_dev *rcdev, unsigned 
int *txbuf,
/* first copy any leading non-repeating */
int leading = c - rep * 3;
 
-   if (leading + rep >= ARRAY_SIZE(code_block->codes) - 3) {
+   if (leading >= ARRAY_SIZE(code_block->codes) - 3 - rep) {
dev_warn(&rcdev->dev, "IR too long, cannot transmit\n");
return -EINVAL;
}
-- 
2.14.3



[PATCH 22/30] media: zoran: don't cast pointers to print them

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/pci/zoran/zoran_driver.c:242 v4l_fbuffer_alloc() warn: argument 5 
to %lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/zoran/zoran_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/zoran/zoran_driver.c 
b/drivers/media/pci/zoran/zoran_driver.c
index 8d4e7d930a66..14f9c0e26a1c 100644
--- a/drivers/media/pci/zoran/zoran_driver.c
+++ b/drivers/media/pci/zoran/zoran_driver.c
@@ -241,8 +241,8 @@ static int v4l_fbuffer_alloc(struct zoran_fh *fh)
SetPageReserved(virt_to_page(mem + off));
dprintk(4,
KERN_INFO
-   "%s: %s - V4L frame %d mem 0x%lx (bus: 0x%llx)\n",
-   ZR_DEVNAME(zr), __func__, i, (unsigned long) mem,
+   "%s: %s - V4L frame %d mem %p (bus: 0x%llx)\n",
+   ZR_DEVNAME(zr), __func__, i, mem,
(unsigned long long)virt_to_bus(mem));
}
 
-- 
2.14.3



[PATCH 07/30] media: v4l2-tpg-core: avoid buffer overflows

2018-03-23 Thread Mauro Carvalho Chehab
Fix the following warnings:
drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1146 gen_twopix() error: 
buffer overflow 'buf[1]' 8 <= 8
drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1152 gen_twopix() error: 
buffer overflow 'buf[1]' 8 <= 8

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c 
b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
index d248d1fb9d1d..37632bc524d4 100644
--- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
+++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
@@ -1143,13 +1143,13 @@ static void gen_twopix(struct tpg_data *tpg,
case V4L2_PIX_FMT_NV24:
buf[0][offset] = r_y_h;
buf[1][2 * offset] = g_u_s;
-   buf[1][2 * offset + 1] = b_v;
+   buf[1][(2 * offset + 1) % 8] = b_v;
break;
 
case V4L2_PIX_FMT_NV42:
buf[0][offset] = r_y_h;
buf[1][2 * offset] = b_v;
-   buf[1][2 * offset + 1] = g_u_s;
+   buf[1][(2 * offset + 1) %8] = g_u_s;
break;
 
case V4L2_PIX_FMT_YUYV:
-- 
2.14.3



[PATCH 25/30] media: vivid-radio-rx: add a cast to avoid a warning

2018-03-23 Thread Mauro Carvalho Chehab
The logic at vivid_radio_rx_g_tuner() is producint an overflow
warning:

drivers/media/platform/vivid/vivid-radio-rx.c:250 
vivid_radio_rx_g_tuner() warn: potential negative subtraction from max '65535 - 
(__builtin_choose_expr( ==  ||  == , , __builtin_choose_expr( ==  ||  == , , 
__builtin_choose_expr( ==  ||  == , , __builtin_choose_expr( ==  ||  == , , 
__builtin_choose_expr( ==  ||  == , , __builtin_choose_expr( == , , (0))) * 
65535) / delta'

Add a cast to prevent that.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/vivid/vivid-radio-rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-radio-rx.c 
b/drivers/media/platform/vivid/vivid-radio-rx.c
index acbfea2cce76..1f86d7d4f72f 100644
--- a/drivers/media/platform/vivid/vivid-radio-rx.c
+++ b/drivers/media/platform/vivid/vivid-radio-rx.c
@@ -247,7 +247,7 @@ int vivid_radio_rx_g_tuner(struct file *file, void *fh, 
struct v4l2_tuner *vt)
vt->rangehigh = FM_FREQ_RANGE_HIGH;
sig_qual = dev->radio_rx_sig_qual;
vt->signal = abs(sig_qual) > delta ? 0 :
-0x - (abs(sig_qual) * 0x) / delta;
+0x - ((unsigned)abs(sig_qual) * 0x) / delta;
vt->afc = sig_qual > delta ? 0 : sig_qual;
if (abs(sig_qual) > delta)
vt->rxsubchans = 0;
-- 
2.14.3



[PATCH 15/30] soc_camera: fix a weird cast on printk

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/platform/soc_camera/soc_camera.c:790 soc_camera_mmap() warn: 
argument 4 to %08lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/soc_camera/soc_camera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c 
b/drivers/media/platform/soc_camera/soc_camera.c
index 1318512c8fe3..69f0d8e80bd8 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -787,7 +787,7 @@ static int soc_camera_mmap(struct file *file, struct 
vm_area_struct *vma)
struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
int err;
 
-   dev_dbg(icd->pdev, "mmap called, vma=0x%08lx\n", (unsigned long)vma);
+   dev_dbg(icd->pdev, "mmap called, vma=%p\n", vma);
 
if (icd->streamer != file)
return -EBUSY;
-- 
2.14.3



[PATCH 13/30] media: cx88: fix two warnings

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/pci/cx88/cx88-alsa.c:295 cx88_alsa_dma_init() warn: argument 3 to 
%08lx specifier is cast from pointer
drivers/media/pci/cx88/cx88-alsa.c:669 snd_cx88_wm8775_volume_put() warn: 
potential negative subtraction from max '65535 - (32768 * left) / right'

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/cx88/cx88-alsa.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 9740326bc93f..ab09bb55cf45 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -292,8 +292,8 @@ static int cx88_alsa_dma_init(struct cx88_audio_dev *chip, 
int nr_pages)
return -ENOMEM;
}
 
-   dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",
-   (unsigned long)buf->vaddr, nr_pages << PAGE_SHIFT);
+   dprintk(1, "vmalloc is at addr %p, size=%d\n",
+   buf->vaddr, nr_pages << PAGE_SHIFT);
 
memset(buf->vaddr, 0, nr_pages << PAGE_SHIFT);
buf->nr_pages = nr_pages;
@@ -656,8 +656,8 @@ static void snd_cx88_wm8775_volume_put(struct snd_kcontrol 
*kcontrol,
 {
struct cx88_audio_dev *chip = snd_kcontrol_chip(kcontrol);
struct cx88_core *core = chip->core;
-   int left = value->value.integer.value[0];
-   int right = value->value.integer.value[1];
+   u16 left = value->value.integer.value[0];
+   u16 right = value->value.integer.value[1];
int v, b;
 
/* Pass volume & balance onto any WM8775 */
-- 
2.14.3



[PATCH 19/30] media: saa7134-input: improve error handling

2018-03-23 Thread Mauro Carvalho Chehab
Currently, the code produces those false-positives:
drivers/media/pci/saa7134/saa7134-input.c:203 
get_key_msi_tvanywhere_plus() error: uninitialized symbol 'b'.
drivers/media/pci/saa7134/saa7134-input.c:251 get_key_kworld_pc150u() 
error: uninitialized symbol 'b'.
drivers/media/pci/saa7134/saa7134-input.c:275 get_key_purpletv() error: 
uninitialized symbol 'b'.

Improve the error handling code, making it to look like our
coding style.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/saa7134/saa7134-input.c | 46 +--
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-input.c 
b/drivers/media/pci/saa7134/saa7134-input.c
index 33ee8322895e..0e28c5021ac4 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -115,7 +115,7 @@ static int build_key(struct saa7134_dev *dev)
 static int get_key_flydvb_trio(struct IR_i2c *ir, enum rc_proto *protocol,
   u32 *scancode, u8 *toggle)
 {
-   int gpio;
+   int gpio, rc;
int attempt = 0;
unsigned char b;
 
@@ -153,8 +153,11 @@ static int get_key_flydvb_trio(struct IR_i2c *ir, enum 
rc_proto *protocol,
   attempt);
return -EIO;
}
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -169,7 +172,7 @@ static int get_key_msi_tvanywhere_plus(struct IR_i2c *ir,
   u32 *scancode, u8 *toggle)
 {
unsigned char b;
-   int gpio;
+   int gpio, rc;
 
/*  is needed to access GPIO. Used by the saa_readl macro. */
struct saa7134_dev *dev = ir->c->adapter->algo_data;
@@ -193,8 +196,11 @@ static int get_key_msi_tvanywhere_plus(struct IR_i2c *ir,
 
/* GPIO says there is a button press. Get it. */
 
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -218,6 +224,7 @@ static int get_key_kworld_pc150u(struct IR_i2c *ir, enum 
rc_proto *protocol,
 {
unsigned char b;
unsigned int gpio;
+   int rc;
 
/*  is needed to access GPIO. Used by the saa_readl macro. */
struct saa7134_dev *dev = ir->c->adapter->algo_data;
@@ -241,8 +248,11 @@ static int get_key_kworld_pc150u(struct IR_i2c *ir, enum 
rc_proto *protocol,
 
/* GPIO says there is a button press. Get it. */
 
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -263,11 +273,15 @@ static int get_key_kworld_pc150u(struct IR_i2c *ir, enum 
rc_proto *protocol,
 static int get_key_purpletv(struct IR_i2c *ir, enum rc_proto *protocol,
u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char b;
 
/* poll IR chip */
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -288,11 +302,17 @@ static int get_key_purpletv(struct IR_i2c *ir, enum 
rc_proto *protocol,
 static int get_key_hvr1110(struct IR_i2c *ir, enum rc_proto *protocol,
   u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char buf[5];
 
/* poll IR chip */
-   if (5 != i2c_master_recv(ir->c, buf, 5))
+   rc = i2c_master_recv(ir->c, buf, 5);
+   if (rc != 5) {
+   ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
+   }
 
/* Check if some key were pressed */
if (!(buf[0] & 0x80))
@@ -319,6 +339,7 @@ static int get_key_hvr1110(struct IR_i2c *ir, enum rc_proto 
*protocol,
 static int get_key_beholdm6xx(struct IR_i2c *ir, enum rc_proto *protocol,
  u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char data[12];
u32 gpio;
 
@@ -335,8 +356,11 @@ static int get_key_beholdm6xx(struct IR_i2c *ir, enum 
rc_proto *protocol,
 
ir->c->addr = 0x5a >> 1;
 
-   if (12 != i2c_master_recv(ir->c, data, 12)) {
+   rc = i2c_master_recv(ir->c, data, 12);
+   if (rc != 12) {
ir_dbg(ir, "read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
@@ -356,12 +3

[PATCH 03/30] media: dvb_frontend: add proper __user annotations

2018-03-23 Thread Mauro Carvalho Chehab
Solves those warnings:
drivers/media/dvb-core/dvb_frontend.c:2297:39: warning: incorrect type 
in argument 1 (different address spaces)
drivers/media/dvb-core/dvb_frontend.c:2297:39:expected void const 
[noderef] *
drivers/media/dvb-core/dvb_frontend.c:2297:39:got struct 
dtv_property *props
drivers/media/dvb-core/dvb_frontend.c:2331:39: warning: incorrect type 
in argument 1 (different address spaces)
drivers/media/dvb-core/dvb_frontend.c:2331:39:expected void const 
[noderef] *
drivers/media/dvb-core/dvb_frontend.c:2331:39:got struct 
dtv_property *props

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index a7ed16e0841d..21a7d4b47e1a 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2294,7 +2294,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
return -EINVAL;
 
-   tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
+   tvp = memdup_user((void __user *)tvps->props, tvps->num * 
sizeof(*tvp));
if (IS_ERR(tvp))
return PTR_ERR(tvp);
 
@@ -2328,7 +2328,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
return -EINVAL;
 
-   tvp = memdup_user(tvps->props, tvps->num * sizeof(*tvp));
+   tvp = memdup_user((void __user *)tvps->props, tvps->num * 
sizeof(*tvp));
if (IS_ERR(tvp))
return PTR_ERR(tvp);
 
-- 
2.14.3



[PATCH 26/30] media: zr364xx: avoid casting just to print pointer address

2018-03-23 Thread Mauro Carvalho Chehab
Instead of casting, just use %p.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/zr364xx/zr364xx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index 8b7c19943d46..b8886102c5ed 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -517,8 +517,7 @@ static void zr364xx_fillbuff(struct zr364xx_camera *cam,
printk(KERN_ERR KBUILD_MODNAME ": ===no frame\n");
return;
}
-   DBG("%s: Buffer 0x%08lx size= %d\n", __func__,
-   (unsigned long)vbuf, pos);
+   DBG("%s: Buffer %p size= %d\n", __func__, vbuf, pos);
/* tell v4l buffer was filled */
 
buf->vb.field_count = cam->frame_count * 2;
@@ -1277,7 +1276,7 @@ static int zr364xx_mmap(struct file *file, struct 
vm_area_struct *vma)
DBG("%s: cam == NULL\n", __func__);
return -ENODEV;
}
-   DBG("mmap called, vma=0x%08lx\n", (unsigned long)vma);
+   DBG("mmap called, vma=%p\n", vma);
 
ret = videobuf_mmap_mapper(&cam->vb_vidq, vma);
 
-- 
2.14.3



[PATCH 11/30] media: bttv-input: better handle errors at I2C transfer

2018-03-23 Thread Mauro Carvalho Chehab
The error handling logic at get_key_pv951() is a little bit
akward, with produces this false positive warning:

drivers/media/pci/bt8xx/bttv-input.c:344 get_key_pv951() error: 
uninitialized symbol 'b'.

Do a cleanup. As a side effect, it also improves its coding
style.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/bt8xx/bttv-input.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/bt8xx/bttv-input.c 
b/drivers/media/pci/bt8xx/bttv-input.c
index da49c5567db5..08266b23826e 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -332,11 +332,15 @@ static void bttv_ir_stop(struct bttv *btv)
 static int get_key_pv951(struct IR_i2c *ir, enum rc_proto *protocol,
 u32 *scancode, u8 *toggle)
 {
+   int rc;
unsigned char b;
 
/* poll IR chip */
-   if (1 != i2c_master_recv(ir->c, &b, 1)) {
+   rc = i2c_master_recv(ir->c, &b, 1);
+   if (rc != 1) {
dprintk("read error\n");
+   if (rc < 0)
+   return rc;
return -EIO;
}
 
-- 
2.14.3



[PATCH 08/30] media: v4l2-ioctl: fix some "too small" warnings

2018-03-23 Thread Mauro Carvalho Chehab
While the code there is right, it produces three false positives:
drivers/media/v4l2-core/v4l2-ioctl.c:2868 video_usercopy() error: 
copy_from_user() 'parg' too small (128 vs 16383)
drivers/media/v4l2-core/v4l2-ioctl.c:2868 video_usercopy() error: 
copy_from_user() 'parg' too small (128 vs 16383)
drivers/media/v4l2-core/v4l2-ioctl.c:2876 video_usercopy() error: 
memset() 'parg' too small (128 vs 16383)

Store the ioctl size on a cache var, in order to suppress those.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 672ab22ccd96..a5dab16ff2d2 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2833,14 +2833,15 @@ video_usercopy(struct file *file, unsigned int cmd, 
unsigned long arg,
size_t  array_size = 0;
void __user *user_ptr = NULL;
void**kernel_ptr = NULL;
+   size_t  size = _IOC_SIZE(cmd);
 
/*  Copy arguments into temp kernel buffer  */
if (_IOC_DIR(cmd) != _IOC_NONE) {
-   if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
+   if (size <= sizeof(sbuf)) {
parg = sbuf;
} else {
/* too big to allocate from stack */
-   mbuf = kvmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
+   mbuf = kvmalloc(size, GFP_KERNEL);
if (NULL == mbuf)
return -ENOMEM;
parg = mbuf;
@@ -2848,7 +2849,7 @@ video_usercopy(struct file *file, unsigned int cmd, 
unsigned long arg,
 
err = -EFAULT;
if (_IOC_DIR(cmd) & _IOC_WRITE) {
-   unsigned int n = _IOC_SIZE(cmd);
+   unsigned int n = size;
 
/*
 * In some cases, only a few fields are used as input,
@@ -2869,11 +2870,11 @@ video_usercopy(struct file *file, unsigned int cmd, 
unsigned long arg,
goto out;
 
/* zero out anything we don't copy from userspace */
-   if (n < _IOC_SIZE(cmd))
-   memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
+   if (n < size)
+   memset((u8 *)parg + n, 0, size - n);
} else {
/* read-only ioctl */
-   memset(parg, 0, _IOC_SIZE(cmd));
+   memset(parg, 0, size);
}
}
 
@@ -2931,7 +2932,7 @@ video_usercopy(struct file *file, unsigned int cmd, 
unsigned long arg,
switch (_IOC_DIR(cmd)) {
case _IOC_READ:
case (_IOC_WRITE | _IOC_READ):
-   if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
+   if (copy_to_user((void __user *)arg, parg, size))
err = -EFAULT;
break;
}
-- 
2.14.3



[PATCH 05/30] media: rca: declare formats var as static

2018-03-23 Thread Mauro Carvalho Chehab
As warned:
drivers/media/platform/rockchip/rga/rga.c:210:16: warning: symbol 
'formats' was not declared. Should it be static?

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/rockchip/rga/rga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c 
b/drivers/media/platform/rockchip/rga/rga.c
index 89296de9cf4a..d508a8ba6f89 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -207,7 +207,7 @@ static int rga_setup_ctrls(struct rga_ctx *ctx)
return 0;
 }
 
-struct rga_fmt formats[] = {
+static struct rga_fmt formats[] = {
{
.fourcc = V4L2_PIX_FMT_ARGB32,
.color_swap = RGA_COLOR_RB_SWAP,
-- 
2.14.3



[PATCH 23/30] media: solo6x10: get rid of an address space warning

2018-03-23 Thread Mauro Carvalho Chehab
Instead of using an ancillary function to avoid duplicating
a small portion of code that copies data either to kernelspace
or between userspace-kernelspace, duplicate the code,
as it prevents static analyzers to complain about it:

drivers/media/pci/solo6x10/solo6x10-g723.c:260:46: warning: cast 
removes address space of expression

The hole idea of using __user is to make sure that the code is
doing the right thing with address space, so there's no
sense on use casting.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/solo6x10/solo6x10-g723.c | 73 +-
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c 
b/drivers/media/pci/solo6x10/solo6x10-g723.c
index 81be1b8df758..2ac33b5cc454 100644
--- a/drivers/media/pci/solo6x10/solo6x10-g723.c
+++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
@@ -223,48 +223,57 @@ static snd_pcm_uframes_t snd_solo_pcm_pointer(struct 
snd_pcm_substream *ss)
return idx * G723_FRAMES_PER_PAGE;
 }
 
-static int __snd_solo_pcm_copy(struct snd_pcm_substream *ss,
-  unsigned long pos, void *dst,
-  unsigned long count, bool in_kernel)
-{
-   struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss);
-   struct solo_dev *solo_dev = solo_pcm->solo_dev;
-   int err, i;
-
-   for (i = 0; i < (count / G723_FRAMES_PER_PAGE); i++) {
-   int page = (pos / G723_FRAMES_PER_PAGE) + i;
-
-   err = solo_p2m_dma_t(solo_dev, 0, solo_pcm->g723_dma,
-SOLO_G723_EXT_ADDR(solo_dev) +
-(page * G723_PERIOD_BLOCK) +
-(ss->number * G723_PERIOD_BYTES),
-G723_PERIOD_BYTES, 0, 0);
-   if (err)
-   return err;
-
-   if (in_kernel)
-   memcpy(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES);
-   else if (copy_to_user((void __user *)dst,
- solo_pcm->g723_buf, G723_PERIOD_BYTES))
-   return -EFAULT;
-   dst += G723_PERIOD_BYTES;
-   }
-
-   return 0;
-}
-
 static int snd_solo_pcm_copy_user(struct snd_pcm_substream *ss, int channel,
  unsigned long pos, void __user *dst,
  unsigned long count)
 {
-   return __snd_solo_pcm_copy(ss, pos, (void *)dst, count, false);
+   struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss);
+   struct solo_dev *solo_dev = solo_pcm->solo_dev;
+   int err, i;
+
+   for (i = 0; i < (count / G723_FRAMES_PER_PAGE); i++) {
+   int page = (pos / G723_FRAMES_PER_PAGE) + i;
+
+   err = solo_p2m_dma_t(solo_dev, 0, solo_pcm->g723_dma,
+SOLO_G723_EXT_ADDR(solo_dev) +
+(page * G723_PERIOD_BLOCK) +
+(ss->number * G723_PERIOD_BYTES),
+G723_PERIOD_BYTES, 0, 0);
+   if (err)
+   return err;
+
+   if (copy_to_user(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES))
+   return -EFAULT;
+   dst += G723_PERIOD_BYTES;
+   }
+
+   return 0;
 }
 
 static int snd_solo_pcm_copy_kernel(struct snd_pcm_substream *ss, int channel,
unsigned long pos, void *dst,
unsigned long count)
 {
-   return __snd_solo_pcm_copy(ss, pos, dst, count, true);
+   struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss);
+   struct solo_dev *solo_dev = solo_pcm->solo_dev;
+   int err, i;
+
+   for (i = 0; i < (count / G723_FRAMES_PER_PAGE); i++) {
+   int page = (pos / G723_FRAMES_PER_PAGE) + i;
+
+   err = solo_p2m_dma_t(solo_dev, 0, solo_pcm->g723_dma,
+SOLO_G723_EXT_ADDR(solo_dev) +
+(page * G723_PERIOD_BLOCK) +
+(ss->number * G723_PERIOD_BYTES),
+G723_PERIOD_BYTES, 0, 0);
+   if (err)
+   return err;
+
+   memcpy(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES);
+   dst += G723_PERIOD_BYTES;
+   }
+
+   return 0;
 }
 
 static const struct snd_pcm_ops snd_solo_pcm_ops = {
-- 
2.14.3



[PATCH 18/30] media: s2255drv: fix a casting warning

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/usb/s2255/s2255drv.c:651 s2255_fillbuff() warn: argument 3 to 
%08lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/s2255/s2255drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index a00a15f55d37..82927eb334c4 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -648,8 +648,8 @@ static void s2255_fillbuff(struct s2255_vc *vc,
pr_err("s2255: ===no frame\n");
return;
}
-   dprintk(dev, 2, "s2255fill at : Buffer 0x%08lx size= %d\n",
-   (unsigned long)vbuf, pos);
+   dprintk(dev, 2, "s2255fill at : Buffer %p size= %d\n",
+   vbuf, pos);
 }
 
 
-- 
2.14.3



[PATCH 28/30] media: tm6000: avoid casting just to print pointer address

2018-03-23 Thread Mauro Carvalho Chehab
Instead of casting, just use %p.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/tm6000/tm6000-video.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/tm6000/tm6000-video.c 
b/drivers/media/usb/tm6000/tm6000-video.c
index 8314d3fa9241..b2399d4266da 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -1346,9 +1346,8 @@ static int __tm6000_open(struct file *file)
fh->width = dev->width;
fh->height = dev->height;
 
-   dprintk(dev, V4L2_DEBUG_OPEN, "Open: fh=0x%08lx, dev=0x%08lx, 
dev->vidq=0x%08lx\n",
-   (unsigned long)fh, (unsigned long)dev,
-   (unsigned long)&dev->vidq);
+   dprintk(dev, V4L2_DEBUG_OPEN, "Open: fh=%p, dev=%p, dev->vidq=%p\n",
+   fh, dev, &dev->vidq);
dprintk(dev, V4L2_DEBUG_OPEN, "Open: list_empty queued=%d\n",
list_empty(&dev->vidq.queued));
dprintk(dev, V4L2_DEBUG_OPEN, "Open: list_empty active=%d\n",
-- 
2.14.3



[PATCH 24/30] media: saa7134-alsa: don't use casts to print a buffer address

2018-03-23 Thread Mauro Carvalho Chehab
Change the logic there to avoid casting, solving this warning:
drivers/media/pci/saa7134/saa7134-alsa.c:276 saa7134_alsa_dma_init() 
warn: argument 3 to %08lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/saa7134/saa7134-alsa.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c 
b/drivers/media/pci/saa7134/saa7134-alsa.c
index c59b69f1af9d..72311445d13d 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -273,9 +273,8 @@ static int saa7134_alsa_dma_init(struct saa7134_dev *dev, 
int nr_pages)
return -ENOMEM;
}
 
-   pr_debug("vmalloc is at addr 0x%08lx, size=%d\n",
-   (unsigned long)dma->vaddr,
-   nr_pages << PAGE_SHIFT);
+   pr_debug("vmalloc is at addr %p, size=%d\n",
+dma->vaddr, nr_pages << PAGE_SHIFT);
 
memset(dma->vaddr, 0, nr_pages << PAGE_SHIFT);
dma->nr_pages = nr_pages;
-- 
2.14.3



[PATCH 30/30] media: cec-core: fix a bug at cec_error_inj_write()

2018-03-23 Thread Mauro Carvalho Chehab
If the adapter doesn't have error_inj_parse_line() ops, the
write() logic won't return -EINVAL, but, instead, it will keep
looping, because "count" is a non-negative number.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/cec/cec-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index ea3eccfdba15..b0c87f9ea08f 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -209,14 +209,14 @@ static ssize_t cec_error_inj_write(struct file *file,
if (IS_ERR(buf))
return PTR_ERR(buf);
p = buf;
-   while (p && *p && count >= 0) {
+   while (p && *p) {
p = skip_spaces(p);
line = strsep(&p, "\n");
if (!*line || *line == '#')
continue;
if (!adap->ops->error_inj_parse_line(adap, line)) {
-   count = -EINVAL;
-   break;
+   kfree(buf);
+   return -EINVAL;
}
}
kfree(buf);
-- 
2.14.3



[PATCH 14/30] media: cx23885: fix a warning

2018-03-23 Thread Mauro Carvalho Chehab
drivers/media/pci/cx23885/cx23885-alsa.c:92 cx23885_alsa_dma_init() warn: 
argument 3 to %08lx specifier is cast from pointer

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/cx23885/cx23885-alsa.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c 
b/drivers/media/pci/cx23885/cx23885-alsa.c
index d8c3637e492e..20b3cb17f97f 100644
--- a/drivers/media/pci/cx23885/cx23885-alsa.c
+++ b/drivers/media/pci/cx23885/cx23885-alsa.c
@@ -89,9 +89,8 @@ static int cx23885_alsa_dma_init(struct cx23885_audio_dev 
*chip, int nr_pages)
return -ENOMEM;
}
 
-   dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",
-   (unsigned long)buf->vaddr,
-   nr_pages << PAGE_SHIFT);
+   dprintk(1, "vmalloc is at addr %p, size=%d\n",
+   buf->vaddr, nr_pages << PAGE_SHIFT);
 
memset(buf->vaddr, 0, nr_pages << PAGE_SHIFT);
buf->nr_pages = nr_pages;
-- 
2.14.3



Re: [PATCH v9.1] media: imx258: Add imx258 camera sensor driver

2018-03-23 Thread Tomasz Figa
Hi Andy,

Some issues found when reviewing cherry pick of this patch to Chrome
OS kernel. Please see inline.

On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh  wrote:

[snip]

> +   case V4L2_CID_VBLANK:
> +   /*
> +* Auto Frame Length Line Control is enabled by default.
> +* Not need control Vblank Register.
> +*/

What is the meaning of this control then? Should it be read-only?

> +   break;
> +   default:
> +   dev_info(&client->dev,
> +"ctrl(id:0x%x,val:0x%x) is not handled\n",
> +ctrl->id, ctrl->val);
> +   ret = -EINVAL;
> +   break;
> +   }
> +
> +   pm_runtime_put(&client->dev);
> +
> +   return ret;
> +

[snip]

> +   v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx258_ctrl_ops,
> +   V4L2_CID_TEST_PATTERN,
> +   ARRAY_SIZE(imx258_test_pattern_menu) - 1,
> +   0, 0, imx258_test_pattern_menu);

There is no code for handling this control in imx258_s_ctrl(). It's
not a correct behavior to register a control, which isn't handled by
the driver. Please either implement the control completely or remove
it.

> +
> +   if (ctrl_hdlr->error) {
> +   ret = ctrl_hdlr->error;
> +   dev_err(&client->dev, "%s control init failed 
> (%d)\n",
> +   __func__, ret);
> +   goto error;

Something strange happening here with indentation.

Best regards,
Tomasz


Re: [PATCH v7 1/2] uvcvideo: send a control event when a Control Change interrupt arrives

2018-03-23 Thread Laurent Pinchart
Hi Guennadi,

Thank you for the patch.

On Friday, 23 March 2018 11:24:00 EET Laurent Pinchart wrote:
> From: Guennadi Liakhovetski 
> 
> UVC defines a method of handling asynchronous controls, which sends a
> USB packet over the interrupt pipe. This patch implements support for
> such packets by sending a control event to the user. Since this can
> involve USB traffic and, therefore, scheduling, this has to be done
> in a work queue.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 166 ++
>  drivers/media/usb/uvc/uvc_status.c | 111 ++---
>  drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
>  drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
>  include/uapi/linux/uvcvideo.h  |   2 +
>  5 files changed, 269 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index 4042cbdb721b..f4773c56438c 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> 
> @@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh
> *handle, {
>   struct v4l2_subscribed_event *sev;
>   struct v4l2_event ev;
> + bool autoupdate;
> 
>   if (list_empty(&mapping->ev_subs))
>   return;
> 
> + if (!handle) {

In which circumstances does this happen ? Is it when the device reports a 
control change event without an prior control set ? Have you seen that 
happening in practice ?

> + autoupdate = true;
> + sev = list_first_entry(&mapping->ev_subs,
> +struct v4l2_subscribed_event, node);
> + handle = container_of(sev->fh, struct uvc_fh, vfh);

There's a check below that guards against sev->fh being NULL. Could sev->fh be 
NULL here ?

> + } else {
> + autoupdate = false;
> + }
> +
>   uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value, changes);
> 
>   list_for_each_entry(sev, &mapping->ev_subs, node) {
>   if (sev->fh && (sev->fh != &handle->vfh ||
>   (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
> - (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
> + (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
>   v4l2_event_queue_fh(sev->fh, &ev);
>   }
>  }
> 
> -static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> - struct uvc_control *master, u32 slave_id,
> - const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
> +static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> + struct uvc_control *master, u32 slave_id)
>  {
>   struct uvc_control_mapping *mapping = NULL;
>   struct uvc_control *ctrl = NULL;
>   u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> - unsigned int i;
>   s32 val = 0;
> 
> + __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
> + if (ctrl == NULL)
> + return;
> +
> + if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
> + changes |= V4L2_EVENT_CTRL_CH_VALUE;
> +
> + uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
> +}
> +
> +static void uvc_ctrl_status_event_work(struct work_struct *work)
> +{
> + struct uvc_device *dev = container_of(work, struct uvc_device,
> +   async_ctrl.work);
> + struct uvc_video_chain *chain;
> + struct uvc_ctrl_work *w = &dev->async_ctrl;
> + struct uvc_control_mapping *mapping;
> + struct uvc_control *ctrl;
> + struct uvc_fh *handle;
> + unsigned int i;
> + u8 *data;
> +
> + spin_lock_irq(&w->lock);
> + data = w->data;
> + w->data = NULL;
> + chain = w->chain;
> + ctrl = w->ctrl;
> + handle = ctrl->handle;
> + ctrl->handle = NULL;
> + spin_unlock_irq(&w->lock);
> +
> + if (mutex_lock_interruptible(&chain->ctrl_mutex))
> + goto free;

This will result in the event being lost, which isn't very nice (see below for 
additional comments on this topic). Can't we use mutex_lock() ?

> +
> + list_for_each_entry(mapping, &ctrl->info.mappings, list) {
> + s32 value = mapping->get(mapping, UVC_GET_CUR, data);
> +
> + for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
> + if (!mapping->slave_ids[i])
> + break;
> +
> + __uvc_ctrl_send_slave_event(handle, ctrl,
> + mapping->slave_ids[i]);
> + }
> +
> + if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
> + struct uvc_menu_info *menu = mapping->menu_info;
> + unsigned int i;
> +
> + for (i = 0; i < mapping->menu_count; ++i, ++menu)
> +

[PATCH] v4l-utils: keytable: add support for imon protocol

2018-03-23 Thread Sean Young
Signed-off-by: Sean Young 
---
 utils/common/ir-encode.c  | 1 +
 utils/keytable/keytable.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/utils/common/ir-encode.c b/utils/common/ir-encode.c
index e6b65b5b..c7e319eb 100644
--- a/utils/common/ir-encode.c
+++ b/utils/common/ir-encode.c
@@ -376,6 +376,7 @@ static const struct {
[RC_PROTO_MCIR2_MSE] = { "mcir2-mse" },
[RC_PROTO_XMP] = { "xmp" },
[RC_PROTO_CEC] = { "cec" },
+   [RC_PROTO_IMON] = { "imon", 0x7fff },
 };
 
 static bool str_like(const char *a, const char *b)
diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c
index 925eab00..482fcf86 100644
--- a/utils/keytable/keytable.c
+++ b/utils/keytable/keytable.c
@@ -116,6 +116,7 @@ enum sysfs_protocols {
SYSFS_SHARP = (1 << 11),
SYSFS_XMP   = (1 << 12),
SYSFS_CEC   = (1 << 13),
+   SYSFS_IMON  = (1 << 14),
SYSFS_INVALID   = 0,
 };
 
@@ -149,6 +150,7 @@ const struct protocol_map_entry protocol_map[] = {
{ "sharp",  NULL,   SYSFS_SHARP },
{ "xmp","/xmp_decoder", SYSFS_XMP   },
{ "cec",NULL,   SYSFS_CEC   },
+   { "imon",   NULL,   SYSFS_IMON  },
{ NULL, NULL,   SYSFS_INVALID   },
 };
 
-- 
2.14.3



Re: [RFC] Request API

2018-03-23 Thread Sakari Ailus
Hi Hans and Tomasz,

On Fri, Mar 23, 2018 at 08:38:54AM +0100, Hans Verkuil wrote:
...
> >>>
> >>> Perhaps we need to consider few separate cases here:
> >>>
> >>>  1) Query control value within a request. Would return the value that
> >>> was earlier set to that particular request (and maybe -EBUSY if it
> >>> wasn't set?). Perhaps done with v4l2_ext_controls::which set to
> >>> V4L2_CTRL_WHICH_REQUEST and v4l2_ext_controls::reserved[2] to FD of
> >>> the request in question.
> >>
> >> I see no reason to use 'WHICH_REQUEST'. If request_fd is non-zero,
> >> then it automatically refers to that request.
> > 
> > Agreed, with one minor detail. Note that zero is a perfectly valid FD.
> 
> That's annoying and will actually require WHICH_REQUEST to indicate that
> the request_fd should be used. So WHICH_CUR_VAL will ignore the request_fd
> field. I'll update this in a new RFC.
> 
> For the same reason we'll need a flag for v4l2_buffers to indicate that
> request_fd should be used. This will be similar to what the fence patch
> series does.

It's valid, indeed. Normally it'd be stdin, but if you close it and then
allocate a request, you indeed could get zero.

...

> >>> An alternative, maybe a bit crazy, idea would be to allow adding
> >>> MEDIA_REQUEST_IOC_QUEUE ioctl to the request itself. This would be
> >>> similar to the idea of indirect command buffers in the graphics (GPU)
> >>> land. It could for example look like this:
> >>>
> >>> // One time initialization
> >>> bulk_fd = ioctl(..., MEDIA_IOC_REQUEST_ALLOC, ...);
> >>> for (i = 0; i < N; ++i) {
> >>> fd[i] = ioctl(..., MEDIA_IOC_REQUEST_ALLOC, ...);
> >>> // Add some state
> >>> ioctl(fd[i], MEDIA_IOC_REQUEST_QUEUE, { .request = bulk_fd });
> >>> }
> >>>
> >>> // Do some work
> >>>
> >>> ioctl(bulk_fd, MEDIA_IOC_REQUEST_QUEUE); // Queues all the requests at 
> >>> once
> >>
> >> That doesn't reduce the number of ioctl calls :-)
> > 
> > Depends on what cases we are talking about. If we have cases of
> > queuing the same (big) sets of requests multiple time, then only for
> > the first time all the ioctls would be needed. Next time, one only
> > needs to queue the bulk_fd.
> > 
> > Personally, I don't have any good use case that would involve queuing
> > many requests instantly and would be affected by the number of ioctls,
> > though.
> 
> Sakari gave the example of 10 cameras running at 60 fps, thus generating
> a very large amount of request ioctls.
> 
> But this doesn't apply to stateless codecs, so for now this is something
> for the future.

I think get real benefit from being able to queue multiple requests at a
time only when you don't need a ton of IOCTLs to construct those requests
first. I.e. that means moving constructing the requests to Media device as
well. That's something for the future indeed, and I don't think it's
realistic to think we'd be there any time soon.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


[PATCH v7 2/2] uvcvideo: handle control pipe protocol STALLs

2018-03-23 Thread Laurent Pinchart
From: Guennadi Liakhovetski 

When a command ends up in a STALL on the control pipe, use the Request
Error Code control to provide a more precise error information to the
user.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_video.c | 59 +++
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index aa0082fe5833..eb9e04a59427 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -34,15 +34,59 @@ static int __uvc_query_ctrl(struct uvc_device *dev, u8 
query, u8 unit,
u8 intfnum, u8 cs, void *data, u16 size,
int timeout)
 {
-   u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
+   u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE, tmp, error;
unsigned int pipe;
+   int ret;
 
pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
  : usb_sndctrlpipe(dev->udev, 0);
type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
 
-   return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
+   ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
unit << 8 | intfnum, data, size, timeout);
+
+   if (ret != -EPIPE)
+   return ret;
+
+   tmp = *(u8 *)data;
+
+   pipe = usb_rcvctrlpipe(dev->udev, 0);
+   type = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN;
+   ret = usb_control_msg(dev->udev, pipe, UVC_GET_CUR, type,
+ UVC_VC_REQUEST_ERROR_CODE_CONTROL << 8,
+ unit << 8 | intfnum, data, 1, timeout);
+   error = *(u8 *)data;
+   *(u8 *)data = tmp;
+
+   if (ret < 0)
+   return ret;
+
+   if (!ret)
+   return -EINVAL;
+
+   uvc_trace(UVC_TRACE_CONTROL, "Control error %u\n", error);
+
+   switch (error) {
+   case 0:
+   /* Cannot happen - we received a STALL */
+   return -EPIPE;
+   case 1: /* Not ready */
+   return -EAGAIN;
+   case 2: /* Wrong state */
+   return -EILSEQ;
+   case 3: /* Power */
+   return -EREMOTE;
+   case 4: /* Out of range */
+   return -ERANGE;
+   case 5: /* Invalid unit */
+   case 6: /* Invalid control */
+   case 7: /* Invalid Request */
+   case 8: /* Invalid value within range */
+   default: /* reserved or unknown */
+   break;
+   }
+
+   return -EINVAL;
 }
 
 static const char *uvc_query_name(u8 query)
@@ -80,7 +124,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
uvc_printk(KERN_ERR, "Failed to query (%s) UVC control %u on "
"unit %u: %d (exp. %u).\n", uvc_query_name(query), cs,
unit, ret, size);
-   return -EIO;
+   return ret < 0 ? ret : -EIO;
}
 
return 0;
@@ -203,13 +247,15 @@ static int uvc_get_video_ctrl(struct uvc_streaming 
*stream,
uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
"compliance - GET_DEF(PROBE) not supported. "
"Enabling workaround.\n");
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
} else if (ret != size) {
uvc_printk(KERN_ERR, "Failed to query (%u) UVC %s control : "
"%d (exp. %u).\n", query, probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
}
 
@@ -290,7 +336,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
uvc_printk(KERN_ERR, "Failed to set UVC %s control : "
"%d (exp. %u).\n", probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
}
 
kfree(data);
-- 
Regards,

Laurent Pinchart



[PATCH v7 1/2] uvcvideo: send a control event when a Control Change interrupt arrives

2018-03-23 Thread Laurent Pinchart
From: Guennadi Liakhovetski 

UVC defines a method of handling asynchronous controls, which sends a
USB packet over the interrupt pipe. This patch implements support for
such packets by sending a control event to the user. Since this can
involve USB traffic and, therefore, scheduling, this has to be done
in a work queue.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_ctrl.c   | 166 +
 drivers/media/usb/uvc/uvc_status.c | 111 ++---
 drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
 drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
 include/uapi/linux/uvcvideo.h  |   2 +
 5 files changed, 269 insertions(+), 29 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 4042cbdb721b..f4773c56438c 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh *handle,
 {
struct v4l2_subscribed_event *sev;
struct v4l2_event ev;
+   bool autoupdate;
 
if (list_empty(&mapping->ev_subs))
return;
 
+   if (!handle) {
+   autoupdate = true;
+   sev = list_first_entry(&mapping->ev_subs,
+  struct v4l2_subscribed_event, node);
+   handle = container_of(sev->fh, struct uvc_fh, vfh);
+   } else {
+   autoupdate = false;
+   }
+
uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value, changes);
 
list_for_each_entry(sev, &mapping->ev_subs, node) {
if (sev->fh && (sev->fh != &handle->vfh ||
(sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
-   (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
+   (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
v4l2_event_queue_fh(sev->fh, &ev);
}
 }
 
-static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
-   struct uvc_control *master, u32 slave_id,
-   const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
+static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
+   struct uvc_control *master, u32 slave_id)
 {
struct uvc_control_mapping *mapping = NULL;
struct uvc_control *ctrl = NULL;
u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
-   unsigned int i;
s32 val = 0;
 
+   __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
+   if (ctrl == NULL)
+   return;
+
+   if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
+   changes |= V4L2_EVENT_CTRL_CH_VALUE;
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
+}
+
+static void uvc_ctrl_status_event_work(struct work_struct *work)
+{
+   struct uvc_device *dev = container_of(work, struct uvc_device,
+ async_ctrl.work);
+   struct uvc_video_chain *chain;
+   struct uvc_ctrl_work *w = &dev->async_ctrl;
+   struct uvc_control_mapping *mapping;
+   struct uvc_control *ctrl;
+   struct uvc_fh *handle;
+   unsigned int i;
+   u8 *data;
+
+   spin_lock_irq(&w->lock);
+   data = w->data;
+   w->data = NULL;
+   chain = w->chain;
+   ctrl = w->ctrl;
+   handle = ctrl->handle;
+   ctrl->handle = NULL;
+   spin_unlock_irq(&w->lock);
+
+   if (mutex_lock_interruptible(&chain->ctrl_mutex))
+   goto free;
+
+   list_for_each_entry(mapping, &ctrl->info.mappings, list) {
+   s32 value = mapping->get(mapping, UVC_GET_CUR, data);
+
+   for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
+   if (!mapping->slave_ids[i])
+   break;
+
+   __uvc_ctrl_send_slave_event(handle, ctrl,
+   mapping->slave_ids[i]);
+   }
+
+   if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
+   struct uvc_menu_info *menu = mapping->menu_info;
+   unsigned int i;
+
+   for (i = 0; i < mapping->menu_count; ++i, ++menu)
+   if (menu->value == value) {
+   value = i;
+   break;
+   }
+   }
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, value,
+   V4L2_EVENT_CTRL_CH_VALUE);
+   }
+
+   mutex_unlock(&chain->ctrl_mutex);
+
+free:
+   kfree(data);
+}
+
+void uvc_ctrl_status_event(struct uvc_video_chain *chain,
+  struct uvc_control *ctrl, u8 *data, size_t len)
+{
+   struct uvc_device *dev = chain->dev;
+   struct uvc_ctrl_

[PATCH v7 0/2] uvcvideo: asynchronous controls

2018-03-23 Thread Laurent Pinchart
Hi Guennadi,

As your v7 of this patch series conflicted with the current version of
the uvcvideo driver, I thought it would be easier to rebase it myself
before reviewing, instead of asking you to do so. Here's the result,
I'll send my review comments as replies.

Guennadi Liakhovetski (2):
  uvcvideo: send a control event when a Control Change interrupt arrives
  uvcvideo: handle control pipe protocol STALLs

 drivers/media/usb/uvc/uvc_ctrl.c   | 166 +
 drivers/media/usb/uvc/uvc_status.c | 111 ++---
 drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
 drivers/media/usb/uvc/uvc_video.c  |  59 +++--
 drivers/media/usb/uvc/uvcvideo.h   |  15 +++-
 include/uapi/linux/uvcvideo.h  |   2 +
 6 files changed, 322 insertions(+), 35 deletions(-)

-- 
Regards,

Laurent Pinchart



[ragnatech:media-tree 369/405] drivers/media/dvb-frontends/af9013.c:1560: undefined reference to `i2c_mux_del_adapters'

2018-03-23 Thread kbuild test robot
Hi Mauro,

It's probably a bug fix that unveils the link errors.

tree:   git://git.ragnatech.se/linux media-tree
head:   f67449fdba3b9dbdd340d8cbf17dfa711d5bd2fb
commit: 238f694e1b7f8297f1256c57e41f69c39576c9b4 [369/405] media: v4l2-common: 
fix a compilation breakage
config: x86_64-randconfig-s4-03231521 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 238f694e1b7f8297f1256c57e41f69c39576c9b4
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/dvb-frontends/af9013.o: In function `af9013_remove':
>> drivers/media/dvb-frontends/af9013.c:1560: undefined reference to 
>> `i2c_mux_del_adapters'
   drivers/media/dvb-frontends/af9013.o: In function `af9013_probe':
>> drivers/media/dvb-frontends/af9013.c:1488: undefined reference to 
>> `i2c_mux_alloc'
>> drivers/media/dvb-frontends/af9013.c:1495: undefined reference to 
>> `i2c_mux_add_adapter'
   drivers/media/dvb-frontends/af9013.c:1544: undefined reference to 
`i2c_mux_del_adapters'

vim +1560 drivers/media/dvb-frontends/af9013.c

f458a1bc67 Antti Palosaari 2017-06-12  1443  
82d1ce3eba Antti Palosaari 2017-06-10  1444  static int af9013_probe(struct 
i2c_client *client,
82d1ce3eba Antti Palosaari 2017-06-10  1445 const struct 
i2c_device_id *id)
82d1ce3eba Antti Palosaari 2017-06-10  1446  {
82d1ce3eba Antti Palosaari 2017-06-10  1447 struct af9013_state *state;
82d1ce3eba Antti Palosaari 2017-06-10  1448 struct af9013_platform_data 
*pdata = client->dev.platform_data;
d029799b2f Antti Palosaari 2017-06-12  1449 struct dtv_frontend_properties 
*c;
82d1ce3eba Antti Palosaari 2017-06-10  1450 int ret, i;
82d1ce3eba Antti Palosaari 2017-06-10  1451 u8 firmware_version[4];
f458a1bc67 Antti Palosaari 2017-06-12  1452 static const struct regmap_bus 
regmap_bus = {
f458a1bc67 Antti Palosaari 2017-06-12  1453 .read = 
af9013_regmap_read,
f458a1bc67 Antti Palosaari 2017-06-12  1454 .write = 
af9013_regmap_write,
f458a1bc67 Antti Palosaari 2017-06-12  1455 };
f458a1bc67 Antti Palosaari 2017-06-12  1456 static const struct 
regmap_config regmap_config = {
22e59e7204 Antti Palosaari 2017-06-22  1457 /* Actual reg is 16 
bits, see i2c adapter lock */
22e59e7204 Antti Palosaari 2017-06-22  1458 .reg_bits = 24,
f458a1bc67 Antti Palosaari 2017-06-12  1459 .val_bits = 8,
f458a1bc67 Antti Palosaari 2017-06-12  1460 };
82d1ce3eba Antti Palosaari 2017-06-10  1461  
82d1ce3eba Antti Palosaari 2017-06-10  1462 state = kzalloc(sizeof(*state), 
GFP_KERNEL);
82d1ce3eba Antti Palosaari 2017-06-10  1463 if (!state) {
82d1ce3eba Antti Palosaari 2017-06-10  1464 ret = -ENOMEM;
82d1ce3eba Antti Palosaari 2017-06-10  1465 goto err;
82d1ce3eba Antti Palosaari 2017-06-10  1466 }
82d1ce3eba Antti Palosaari 2017-06-10  1467  
22e59e7204 Antti Palosaari 2017-06-22  1468 dev_dbg(&client->dev, "\n");
22e59e7204 Antti Palosaari 2017-06-22  1469  
82d1ce3eba Antti Palosaari 2017-06-10  1470 /* Setup the state */
82d1ce3eba Antti Palosaari 2017-06-10  1471 state->client = client;
82d1ce3eba Antti Palosaari 2017-06-10  1472 i2c_set_clientdata(client, 
state);
82d1ce3eba Antti Palosaari 2017-06-10  1473 state->clk = pdata->clk;
82d1ce3eba Antti Palosaari 2017-06-10  1474 state->tuner = pdata->tuner;
82d1ce3eba Antti Palosaari 2017-06-10  1475 state->if_frequency = 
pdata->if_frequency;
82d1ce3eba Antti Palosaari 2017-06-10  1476 state->ts_mode = pdata->ts_mode;
eaa455f023 Antti Palosaari 2017-06-13  1477 state->ts_output_pin = 
pdata->ts_output_pin;
82d1ce3eba Antti Palosaari 2017-06-10  1478 state->spec_inv = 
pdata->spec_inv;
82d1ce3eba Antti Palosaari 2017-06-10  1479 memcpy(&state->api_version, 
pdata->api_version, sizeof(state->api_version));
82d1ce3eba Antti Palosaari 2017-06-10  1480 memcpy(&state->gpio, 
pdata->gpio, sizeof(state->gpio));
f458a1bc67 Antti Palosaari 2017-06-12  1481 state->regmap = 
regmap_init(&client->dev, ®map_bus, client,
f458a1bc67 Antti Palosaari 2017-06-12  1482   
®map_config);
f458a1bc67 Antti Palosaari 2017-06-12  1483 if (IS_ERR(state->regmap)) {
f458a1bc67 Antti Palosaari 2017-06-12  1484 ret = 
PTR_ERR(state->regmap);
f458a1bc67 Antti Palosaari 2017-06-12  1485 goto err_kfree;
f458a1bc67 Antti Palosaari 2017-06-12  1486 }
22e59e7204 Antti Palosaari 2017-06-22  1487 /* Create mux i2c adapter */
22e59e7204 Antti Palosaari 2017-06-22 @1488 state->muxc = 
i2c_mux_alloc(client->adapter, &client->dev, 1, 0, 0,
22e59e7204 Antti Palosaari 2017-06-22  1489 
af9013_select, af9013_deselect);
22e59e7204 Antti Palosaari 2017-06-22  1490 if (!state->muxc) {
22e59e7204 Antti Palosaari 2017-06-22  1491 ret = -ENOMEM;
22e59e7204 Antti Pa

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

2018-03-23 Thread Simon Horman
On Thu, Mar 22, 2018 at 09:30:40PM +, Kieran Bingham wrote:
> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
> bus, however in low power mode the ADV7513 will reset it's slave maps to
> use the hardware defined default addresses.
> 
> The ADV7511 driver was adapted to allow the two devices to be registered
> correctly - but it did not take into account the fault whereby the
> devices reset the addresses.
> 
> This results in an address conflict between the device using the default
> addresses, and the other device if it is in low-power-mode.
> 
> Repair this issue by moving both devices away from the default address
> definitions.

Hi Kierean,

as this is a fix
a) Does it warrant a fixes tag?
   Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
b) Does it warrant being posted as a fix for v4.16;
c) or v4.17?


[RFCv2] Request API

2018-03-23 Thread Hans Verkuil
RFC Request API, version 2
--

This document proposes the public API for handling requests.

There has been some confusion about how to do this, so this summarizes the
current approach based on conversations with the various stakeholders today
(Sakari, Alexandre Courbot, Thomasz Figa and myself).

1) Additions to the media API

   Allocate an empty request object:

   #define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)

   This will return a file descriptor representing the request or an error
   if it can't allocate the request.

   If the pointer argument is NULL, then this will just return 0 (if this ioctl
   is implemented) or -ENOTTY otherwise. This can be used to test whether this
   ioctl is supported or not without actually having to allocate a request.

2) Operations on the request fd

   You can queue or reinit a request by calling these ioctls on the request fd:

   #define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
   #define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)

   With REINIT you reset the state of the request as if you had just allocated
   it.

   You can poll the request fd to wait for it to complete.

   To free a request you close the request fd. Note that it may still be in
   use internally, so the internal datastructures have to be refcounted.

   For this initial implementation only buffers and controls are contained
   in a request. This is needed to implement stateless codecs. Supporting
   complex camera pipelines will require more work.

   Requests only contain the changes to the state at request queue time
   relative to the previously queued request(s) or the current hardware state
   if no other requests were queued.

   Once a request is completed it will retain the state at completion
   time.

3) To associate a v4l2 buffer with a request the 'reserved' field in struct
   v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
   until the request is queued since the request may contain information that
   is needed to prepare the buffer.

   To indicate that request_fd should be used this flag should be set by
   userspace at QBUF time:

#define V4L2_BUF_FLAG_REQUEST   0x0080

   This flag will also be returned by the driver to indicate that the buffer
   is associated with a request.

   TBD: what should vb2 return as request_fd value if this flag is set?
   This should act the same as the fence patch series and this is still
   being tweaked so let's wait for that to be merged first, then we can
   finalize this.

4) To associate v4l2 controls with a request we take the first of the
   'reserved[2]' array elements in struct v4l2_ext_controls and use it to store
   the request fd.

   We also add a new WHICH value:

#define V4L2_CTRL_WHICH_REQUEST   0x0f01

   This tells the control framework to get/set controls from the given
   request fd.

   When querying a control value from a request it will return the newest
   value in the list of pending requests, or the current hardware value if
   is not set in any of the pending requests.

   When a request is completed the controls will no longer change. A copy
   will be made of volatile controls at the time of completion (actually
   it will be up to the driver to decide when to do that).

   Volatile controls and requests:

   - If you set a volatile control in a request, then that will be ignored,
 unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.

   - If you get a volatile control from a request then:
 1) If the request is completed it will return the value of the volatile
control at completion time.
 2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
set in a request, then that value is returned.
 3) Otherwise: return the current value from the hardware (i.e. normal
behavior).

   Read-only controls and requests:

   - If you get a read-only control from a request then:
 1) If the request is completed it will return the value of the read-only
control at completion time.
 2) Otherwise it will get the current value from the driver (i.e. normal
behavior).

   Open issue: should we receive control events if a control in a request is
   added/changed? Currently there are no plans to support control events for
   requests. I don't see a clear use-case and neither do I see an easy way
   of implementing this (which fd would receive those events?).

Notes:

- Earlier versions of this API had a TRY command as well to validate the
  request. I'm not sure that is useful so I dropped it, but it can easily
  be added if there is a good use-case for it. Traditionally within V4L the
  TRY ioctl will also update wrong values to something that works, but that
  is not the intention here as far as I understand it. So the validation
  step can also be done when the request is queued and, if it fails, it will
  just return an error.

- If due to performance reasons we wil

Re: [RFC] Request API

2018-03-23 Thread Hans Verkuil
On 03/23/2018 05:02 AM, Tomasz Figa wrote:
> On Fri, Mar 23, 2018 at 1:28 AM, Hans Verkuil  wrote:
>> On 03/22/18 15:47, Tomasz Figa wrote:
>>> Hi Hans,
>>>
>>> On Thu, Mar 22, 2018 at 11:18 PM, Hans Verkuil  wrote:

Requests only contain the changes since the previously queued request or
since the current hardware state if no other requests are queued.
>>>
>>> Does this mean that a request is tied to certain hardware state from
>>> some point of time? (allocation? queue?) How does it affect the
>>> ability to create multiple requests beforehand, without queuing any?
>>>
>>> My original understanding was that a request would be more like a set
>>> of state change actions (S_CTRL, QBUF), that would apply over any
>>> state that is there at request execution time. As such, requests could
>>> be prepared beforehand (or in parallel), without worrying about
>>> current driver state, and then possibly used in a cyclic fashion.
>>
>> Almost correct, except that it applies over any state that is there at
>> request queue time. Easiest it probably to give an example:
>>
>> Say you have two controls: C1 and C2. They are initialized in the driver
>> to 0.
>>
>> You allocate three requests: R1, R2 and R3.
>>
>> You set control C1 to 1 in R1 and to 3 in R3.
>> You never set control C2 in the request.
>>
>> Now you queue requests R1, R2 and R3.
>>
>> When R1 is applied it sets C1 to 1. When R2 is applied it doesn't change
>> anything. When R3 is applied it sets C1 to 3.
>>
>> Calling VIDIOC_G_EXT_CTRLS for R1 will give C1 = 1 and C2 = 0. R2 returns
>> C1 = 1 and C2 = 0 and R3 returns C1 = 3 and C2 = 0.
>>
>> So each request just has the diff against the previously queued request or
>> (if the control in question has never been set in any of the queued requests)
>> it returns the current HW value.
> 
> I think this matches my original understanding actually, was just
> confused by the wording. Could we perhaps reword your sentence a bit,
> to avoid indicating any dependencies on other requests or hardware
> state at the time of request creation (i.e. allocation and setting the
> state, e.g. S_CTRL, QBUF)? How about the following?
> 
> Requests only contain changes to the state, which would be applied
> at request
> queue time on top of the previously queued request or current
> hardware state,
> if no other requests are queued.

Sounds good.

> 
>>
>> Note that this is not yet implemented in the control framework. I have a good
>> idea how to do this, but I'm waiting for a stable patch series to work on.
>>
>> I believe today in the v4 patch series all the controls are just cloned when
>> you allocate a request, right? That's just a temporary solution that will be
>> replaced later using the approach detailed above.
> 
> I don't think cloning all the controls was the intention, but maybe I
> missed something.

I believe it was an 'intentional temporary measure' :-)

Since the control framework doesn't yet support the mechanism described
above it was the easiest way to get things up and running.

> 
>>
>>>

 3) To associate a v4l2 buffer with a request the 'reserved' field in struct
v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
until the request is queued since the request may contain information 
 that
is needed to prepare the buffer.

Queuing a buffer without a request after a buffer with a request is 
 equivalent
to queuing a request containing just that buffer and nothing else. I.e. 
 it will
just use whatever values the hardware has at the time of processing.

 4) To associate v4l2 controls with a request we take the first of the
'reserved[2]' array elements in struct v4l2_ext_controls and use it to 
 store
the request fd.

When querying a control value from a request it will return the newest
value in the list of pending requests, or the current hardware value if
is not set in any of the pending requests.
>>>
>>> What does it mean to "query a control value from a request"? The first
>>> thing that comes to my mind is checking the value that was earlier set
>>> to that particular request (and maybe -EBUSY if it wasn't set?).
>>
>> Calling VIDIOC_G_EXT_CTRLS with a non-zero request_fd.
>>
>> It's useful to refer to what was discussed in Prague:
>>
>> "A request object is essentially created empty from the point of view of the
>> user. Only values that are changed in the request are part of the request,
>> other values remain unchanged (unless those values change due to a
>> side-effect of setting a request value). When a request completes, make a
>> copy of the volatile controls (since that's the value at that point in
>> time). This is needed for auto- functions that need to know such
>> volatile values."
>>
>> Once a request has been queued it is fairly easy: VIDIOC_G_EXT_CTRLS
>> will return the values as will be applied, either by