cron job: media_tree daily build: ERRORS
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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
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
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
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
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
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
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()
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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'
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
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
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
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