cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Wed Jan 17 05:00:17 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   2bd1f1623fbadfdc1026712b3d55141ba164c403
v4l-utils git hash: 1ab08bca182738f934650fc53c60a7ddd4176b47
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

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-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.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: WARNINGS
linux-4.14-x86_64: WARNINGS
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[linux-next:master 3773/9944] drivers/media/dvb-frontends/stb0899_drv.h:151:36: error: weak declaration of 'stb0899_attach' being applied to a already existing, static definition

[kbuild test robot]

Hi Wolfgang,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
master
head:   fdddade65d7b5f8779374eb73d09889185280f60
commit: 6cdeaed3b1420bd2569891be0c4123ff59628e9e [3773/9944] media: 
dvb_usb_pctv452e: module refcount changes were unbalanced
config: x86_64-randconfig-s4-01170712 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
git checkout 6cdeaed3b1420bd2569891be0c4123ff59628e9e
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from drivers/media/usb/dvb-usb/pctv452e.c:20:0:
   drivers/media/usb/dvb-usb/pctv452e.c: In function 'pctv452e_frontend_attach':
>> drivers/media/dvb-frontends/stb0899_drv.h:151:36: error: weak declaration of 
>> 'stb0899_attach' being applied to a already existing, static definition
static inline struct dvb_frontend *stb0899_attach(struct stb0899_config 
*config,
   ^~

vim +/stb0899_attach +151 drivers/media/dvb-frontends/stb0899_drv.h

ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
150  
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08 
@151  static inline struct dvb_frontend *stb0899_attach(struct stb0899_config 
*config,
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
152  struct i2c_adapter *i2c)
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
153  {
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
154printk(KERN_WARNING "%s: Driver disabled by Kconfig\n", __func__);
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
155return NULL;
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
156  }
ae9902da9 drivers/media/dvb/frontends/stb0899_drv.h Manu Abraham 2007-10-08  
157  

:: The code at line 151 was first introduced by commit
:: ae9902da96b4d2d82707706c7fbc93a8e501dde8 V4L/DVB (9417): DVB_ATTACH for 
STB0899, STB6100, TDA8261

:: TO: Manu Abraham 
:: CC: Mauro Carvalho Chehab 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [RFT PATCH v3 6/6] uvcvideo: Move decode processing to process context

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday, 12 January 2018 11:19:27 EET Kieran Bingham wrote:
> Newer high definition cameras, and cameras with multiple lenses such as
> the range of stereo-vision cameras now available have ever increasing
> data rates.
> 
> The inclusion of a variable length packet header in URB packets mean
> that we must memcpy the frame data out to our destination 'manually'.
> This can result in data rates of up to 2 gigabits per second being
> processed.
> 
> To improve efficiency, and maximise throughput, handle the URB decode
> processing through a work queue to move it from interrupt context, and
> allow multiple processors to work on URBs in parallel.
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2:
>  - Lock full critical section of usb_submit_urb()
> 
> v3:
>  - Fix race on submitting uvc_video_decode_data_work() to work queue.
>  - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode)
>  - Rename decodes -> copy_operations
>  - Don't queue work if there is no async task
>  - obtain copy op structure directly in uvc_video_decode_data()
>  - uvc_video_decode_data_work() -> uvc_video_copy_data_work()
> ---
>  drivers/media/usb/uvc/uvc_queue.c |  12 +++-
>  drivers/media/usb/uvc/uvc_video.c | 116 +++
>  drivers/media/usb/uvc/uvcvideo.h  |  24 ++-
>  3 files changed, 138 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index 5a9987e547d3..598087eeb5c2 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -179,10 +179,22 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>   struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> 
> + /* Prevent new buffers coming in. */
> + spin_lock_irq(>irqlock);
> + queue->flags |= UVC_QUEUE_STOPPING;
> + spin_unlock_irq(>irqlock);
> +
> + /*
> +  * All pending work should be completed before disabling the stream, as
> +  * all URBs will be free'd during uvc_video_enable(s, 0).
> +  */
> + flush_workqueue(stream->async_wq);
> +
>   uvc_video_enable(stream, 0);
> 
>   spin_lock_irq(>irqlock);
>   uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> + queue->flags &= ~UVC_QUEUE_STOPPING;
>   spin_unlock_irq(>irqlock);

As discussed today I believe you'll rework this part so I won't comment on it.

>  }
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 3878bec3276e..fb6b5af17380 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1058,21 +1058,74 @@ static int uvc_video_decode_start(struct
> uvc_streaming *stream, return data[0];
>  }
> 
> -static void uvc_video_decode_data(struct uvc_streaming *stream,
> +static void uvc_video_copy_packets(struct uvc_urb *uvc_urb)

Maybe uvc_video_copy_data() as the function doesn't copy the full packets but 
the data only ?

> +{
> + unsigned int i;
> +
> + for (i = 0; i < uvc_urb->async_operations; i++) {
> + struct uvc_copy_op *op = _urb->copy_operations[i];
> +
> + memcpy(op->dst, op->src, op->len);
> +
> + /* Release reference taken on this buffer */
> + uvc_queue_buffer_release(op->buf);
> + }
> +}
> +
> +/*
> + * uvc_video_decode_data_work: Asynchronous memcpy processing
> + *
> + * Perform memcpy tasks in process context, with completion handlers
> + * to return the URB, and buffer handles.
> + *
> + * The work submitter must pre-determine that the work is safe

s/safe/safe./

Could you explain what safe work means ?

> + */
> +static void uvc_video_copy_data_work(struct work_struct *work)
> +{
> + struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
> + struct uvc_streaming *stream = uvc_urb->stream;
> + struct uvc_video_queue *queue = >queue;
> + int ret;
> +
> + uvc_video_copy_packets(uvc_urb);
> +
> + /*
> +  * Prevent resubmitting URBs when shutting down to ensure that no new
> +  * work item will be scheduled after uvc_stop_streaming() flushes the
> +  * work queue.
> +  */
> + spin_lock_irq(>irqlock);
> + if (!(queue->flags & UVC_QUEUE_STOPPING)) {
> + ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
> + if (ret < 0)
> + uvc_printk(KERN_ERR,
> +"Failed to resubmit video URB (%d).\n",
> +ret);
> + }
> + spin_unlock_irq(>irqlock);
> +}
> +
> +static void uvc_video_decode_data(struct uvc_urb *uvc_urb,
>   struct uvc_buffer *buf, const __u8 *data, int len)
>  {
> - unsigned int maxlen, nbytes;
> - void *mem;
> + unsigned int active_op = uvc_urb->async_operations;
> + struct uvc_copy_op *decode = 

[PATCH v2] v4l: async: Protect against double notifier registrations

[Kieran Bingham]

From: Kieran Bingham 

It can be easy to attempt to register the same notifier twice
in mis-handled error cases such as working with -EPROBE_DEFER.

This results in odd kernel crashes where the notifier_list becomes
corrupted due to adding the same entry twice.

Protect against this so that a developer has some sense of the pending
failure, and use a WARN_ON to identify the fault.

Signed-off-by: Kieran Bingham 

---
v2:
 - Reduce verbosity
 - use WARN_ON()
 - Move notifier list initialisation after registration check

 drivers/media/v4l2-core/v4l2-async.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 2b08d03b251d..17a779440ae3 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -374,17 +374,26 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)
struct device *dev =
notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
struct v4l2_async_subdev *asd;
+   struct v4l2_async_notifier *n;
int ret;
int i;
 
if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
 
+   mutex_lock(_lock);
+
+   /* Avoid re-registering a notifier. */
+   list_for_each_entry(n, _list, list) {
+   if (WARN_ON(n == notifier)) {
+   ret = -EEXIST;
+   goto err_unlock;
+   }
+   }
+
INIT_LIST_HEAD(>waiting);
INIT_LIST_HEAD(>done);
 
-   mutex_lock(_lock);
-
for (i = 0; i < notifier->num_subdevs; i++) {
asd = notifier->subdevs[i];
 
-- 
2.7.4

Re: [RFT PATCH v3 5/6] uvcvideo: queue: Support asynchronous buffer handling

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday, 12 January 2018 11:19:26 EET Kieran Bingham wrote:
> The buffer queue interface currently operates sequentially, processing
> buffers after they have fully completed.
> 
> In preparation for supporting parallel tasks operating on the buffers,
> we will need to support buffers being processed on multiple CPUs.
> 
> Adapt the uvc_queue_next_buffer() such that a reference count tracks the
> active use of the buffer, returning the buffer to the VB2 stack at
> completion.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/usb/uvc/uvc_queue.c | 61 ++--
>  drivers/media/usb/uvc/uvcvideo.h  |  4 ++-
>  2 files changed, 54 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index ddac4d89a291..5a9987e547d3 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -131,6 +131,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
> 
>   spin_lock_irqsave(>irqlock, flags);
>   if (likely(!(queue->flags & UVC_QUEUE_DISCONNECTED))) {
> + kref_init(>ref);
>   list_add_tail(>queue, >irqqueue);
>   } else {
>   /* If the device is disconnected return the buffer to userspace
> @@ -424,28 +425,66 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct
> uvc_video_queue *queue) return nextbuf;
>  }
> 
> -struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> +/*
> + * uvc_queue_requeue: Requeue a buffer on our internal irqqueue
> + *
> + * Reuse a buffer through our internal queue without the need to 'prepare'
> + * The buffer will be returned to userspace through the uvc_buffer_queue
> call if
> + * the device has been disconnected
> + */
> +static void uvc_queue_requeue(struct uvc_video_queue *queue,
>   struct uvc_buffer *buf)
>  {
> - struct uvc_buffer *nextbuf;
> - unsigned long flags;
> + buf->error = 0;
> + buf->state = UVC_BUF_STATE_QUEUED;
> + buf->bytesused = 0;
> + vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
> +
> + uvc_buffer_queue(>buf.vb2_buf);
> +}

You could have inlined this in uvc_queue_buffer_complete(), but the above 
documentation is useful, so I'm fine if you prefer keeping it as a separate 
function. Maybe you could call it uvc_queue_buffer_requeue() to be consistent 
with the other functions ?

> +static void uvc_queue_buffer_complete(struct kref *ref)
> +{
> + struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref);
> + struct vb2_buffer *vb = >buf.vb2_buf;
> + struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
> 
>   if ((queue->flags & UVC_QUEUE_DROP_CORRUPTED) && buf->error) {
> - buf->error = 0;
> - buf->state = UVC_BUF_STATE_QUEUED;
> - buf->bytesused = 0;
> - vb2_set_plane_payload(>buf.vb2_buf, 0, 0);
> - return buf;
> + uvc_queue_requeue(queue, buf);
> + return;

This changes the behaviour of the driver. Currently when an erroneous buffer 
is encountered, it will be immediately reused. With this patch applied it will 
be pushed to the back of the queue for later reuse. This will result in 
buffers being reordered, possibly causing issues later when we'll implement 
fences support (or possibly even today).

I think the whole drop corrupted buffers mechanism was a bad idea in the first 
place and I'd like to remove it at some point, buffers in the error state 
should be handled by applications. However, until that's done, I wonder 
whether it would be possible to keep the current order. I unfortunately don't 
see an easy option to do so at the moment, but maybe you would. Otherwise I 
suppose we'll have to leave it as is.

I'm tempted to flip the driver to not drop corrupted buffers by default. I've 
done so on my computer, I'll see if I run into any issue. It could be useful 
if you could set the nodrop parameter to 1 on your systems too when performing 
tests.

>   }
> 
> + buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE;
> + vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused);
> + vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE);
> +}
> +
> +/*
> + * Release a reference on the buffer. Complete the buffer when the last
> + * reference is released
> + */
> +void uvc_queue_buffer_release(struct uvc_buffer *buf)
> +{
> + kref_put(>ref, uvc_queue_buffer_complete);
> +}
> +
> +/*
> + * Remove this buffer from the queue. Lifetime will persist while async
> actions
> + * are still running (if any), and uvc_queue_buffer_release will give the
> buffer
> + * back to VB2 when all users have completed.
> + */
> +struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> + struct uvc_buffer *buf)
> +{
> + struct uvc_buffer *nextbuf;
> + unsigned long flags;
> +
>   

Re: [PATCH] [v4] media: s3c-camif: fix out-of-bounds array access

[Sakari Ailus]

On Tue, Jan 16, 2018 at 10:52:15PM +0100, Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function 
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> As Sylvester explains, we actually need to ensure that the
> value is within the range, so this reworks the loop to be
> easier to parse correctly, and an additional check to fall
> back on the first format value for any unexpected input.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Arnd Bergmann 

Acked-by: Sakari Ailus 

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

Re: [PATCH] [v4] media: s3c-camif: fix out-of-bounds array access

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Tuesday, 16 January 2018 23:52:15 EET Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> As Sylvester explains, we actually need to ensure that the
> value is within the range, so this reworks the loop to be
> easier to parse correctly, and an additional check to fall
> back on the first format value for any unexpected input.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series
> camera interface") Signed-off-by: Arnd Bergmann 

Reviewed-by: Laurent Pinchart 

> ---
> v4: simplify a bit
> v3: fix newly introduced off-by-one bug.
> v2: rework logic rather than removing it.
> ---
>  drivers/media/platform/s3c-camif/camif-capture.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
> b/drivers/media/platform/s3c-camif/camif-capture.c index
> 437395a61065..9ab8e7ee2e1e 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1256,16 +1256,17 @@ static void __camif_subdev_try_format(struct
> camif_dev *camif, {
>   const struct s3c_camif_variant *variant = camif->variant;
>   const struct vp_pix_limits *pix_lim;
> - int i = ARRAY_SIZE(camif_mbus_formats);
> + unsigned int i;
> 
>   /* FIXME: constraints against codec or preview path ? */
>   pix_lim = >vp_pix_limits[VP_CODEC];
> 
> - while (i-- >= 0)
> + for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
>   if (camif_mbus_formats[i] == mf->code)
>   break;
> 
> - mf->code = camif_mbus_formats[i];
> + if (i == ARRAY_SIZE(camif_mbus_formats))
> + mf->code = camif_mbus_formats[0];
> 
>   if (pad == CAMIF_SD_PAD_SINK) {
>   v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,


-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep

[Brad Love]

On 2018-01-16 14:38, Antti Palosaari wrote:
> On 01/16/2018 10:14 PM, Brad Love wrote:
>>
>> On 2018-01-16 13:32, Antti Palosaari wrote:
>>> On 01/16/2018 07:31 PM, Brad Love wrote:

 On 2018-01-15 23:07, Antti Palosaari wrote:
> Hello
> And what is rationale here, is there some use case demod must be
> active and ts set to tristate (disabled)? Just put demod sleep when
> you don't use it.
>
> regards
> Antti

 Hello Antti,

 Perhaps the .ts_bus_ctrl callback does not need to be included in ops,
 but the function is necessary. The demod is already put to sleep when
 not in use, but it leaves the ts bus open. The ts bus has no reason to
 be open when the demod is put to sleep. Leaving the ts bus open during
 sleep affects the other connected demod and nothing is received by it.
 The lgdt3306a driver already tri states its ts bus when put to sleep,
 the si2168 should as well.
>>>
>>> Sounds possible, but unlikely as chip is firmware driven. When you put
>>> chip to sleep you usually want set ts pins to tristate (also other
>>> unused pins) in order to save energy. I haven't never tested it anyway
>>> though, so it could be possible it leaves those pins to some other
>>> state like random output at given time.
>>>
>>> And if you cannot get stream from lgdt3306a, which is connected to
>>> same bus, it really sounds like ts bus pins are left some state
>>> (cannot work if same pin is driven high to other demod whilst other
>>> tries to drive it low.
>>>
>>> Setting ts pins to tri-state during sleep should resolve your issue.
>>
>> Hello Antti,
>>
>> This patch fixes the issue I'm describing, hence why I submitted it. The
>> ts bus must be tristated before putting the chip to sleep for the other
>> demod to get a stream.
>>
>
> I can test tri-state using power meter on some day, but it may be so
> small current that it cannot be seen usb power meter I use (YZXstudio,
> very nice small power meter).
>
> regards
> Antti
>


Nifty looking devices, one just fell into my shopping cart :)

Cheers,

Brad

[PATCH] [v4] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann 
---
v4: simplify a bit
v3: fix newly introduced off-by-one bug.
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..9ab8e7ee2e1e 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,16 +1256,17 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   unsigned int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
-   mf->code = camif_mbus_formats[i];
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
 
if (pad == CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,
-- 
2.9.0

[PATCH v6 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver

[Jacopo Mondi]

Hello,
   new version of CEU after Hans' review.

Added his Acked-by to most patches and closed review comments.
Running v4l2-compliance, I noticed a new failure introduced by the way I now
calculate the plane sizes in set/try_fmt.

This is the function used to update per-plane bytesperline and sizeimage:

static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
   unsigned int bpl, unsigned int szimage)
{
if (plane->bytesperline < bpl)
plane->bytesperline = bpl;
if (plane->sizeimage < szimage)
plane->sizeimage = szimage;
}

I'm seeing a failure as v4l2-compliance requires buffers with both bytesperline
and sizeimage set to MAX_INT . Hans, is this expected from v4l2-compliance?
How should I handle this, if that has to be handled by the single drivers?

Apart from that, here it is the output of v4l2-compliance, with the last tests
failing due to the above stated reason, and two errors in try/set format due to
the fact the driver is not setting ycbcr encoding after it receives an invalid
format. I would set those, but I'm not sure what it the correct value and not
all mainline drivers do that.

---
v4l2-compliance SHA   : 1d3c611dee82090d9456730e24af368b51dcb4a9

Driver Info:
Driver name   : renesas-ceu
Card type : Renesas CEU e821.ceu
Bus info  : platform:renesas-ceu-e821.c
Driver version: 4.14.0
Capabilities  : 0x84201000
Video Capture Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x04201000
Video Capture Multiplanar
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
fail: v4l2-test-controls.cpp(782): subscribe event for control 
'User Controls' failed
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 12 Private Controls: 0

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
fail: v4l2-test-formats.cpp(1162): ret && node->has_frmintervals
test VIDIOC_G/S_PARM: FAIL
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
fail: v4l2-test-formats.cpp(451): 
testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
pix_mp.quantization)
fail: v4l2-test-formats.cpp(736): Video Capture Multiplanar is 
valid, but TRY_FMT failed to return a format
test VIDIOC_TRY_FMT: FAIL
fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
fail: v4l2-test-formats.cpp(451): 
testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
pix_mp.quantization)
fail: v4l2-test-formats.cpp(996): Video Capture Multiplanar is 
valid, but no S_FMT was implemented
test VIDIOC_S_FMT: FAIL
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)

  

[PATCH v6 2/9] include: media: Add Renesas CEU driver interface

[Jacopo Mondi]

Add renesas-ceu header file.

Do not remove the existing sh_mobile_ceu.h one as long as the original
driver does not go away.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 include/media/drv-intf/renesas-ceu.h | 26 ++
 1 file changed, 26 insertions(+)
 create mode 100644 include/media/drv-intf/renesas-ceu.h

diff --git a/include/media/drv-intf/renesas-ceu.h 
b/include/media/drv-intf/renesas-ceu.h
new file mode 100644
index 000..52841d1
--- /dev/null
+++ b/include/media/drv-intf/renesas-ceu.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * renesas-ceu.h - Renesas CEU driver interface
+ *
+ * Copyright 2017-2018 Jacopo Mondi 
+ */
+
+#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__
+#define __MEDIA_DRV_INTF_RENESAS_CEU_H__
+
+#define CEU_MAX_SUBDEVS2
+
+struct ceu_async_subdev {
+   unsigned long flags;
+   unsigned char bus_width;
+   unsigned char bus_shift;
+   unsigned int i2c_adapter_id;
+   unsigned int i2c_address;
+};
+
+struct ceu_platform_data {
+   unsigned int num_subdevs;
+   struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
+};
+
+#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */
-- 
2.7.4

[PATCH v6 1/9] dt-bindings: media: Add Renesas CEU bindings

[Jacopo Mondi]

Add bindings documentation for Renesas Capture Engine Unit (CEU).

Signed-off-by: Jacopo Mondi 
Reviewed-by: Rob Herring 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 .../devicetree/bindings/media/renesas,ceu.txt  | 81 ++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
b/Documentation/devicetree/bindings/media/renesas,ceu.txt
new file mode 100644
index 000..590ee27
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -0,0 +1,81 @@
+Renesas Capture Engine Unit (CEU)
+--
+
+The Capture Engine Unit is the image capture interface found in the Renesas
+SH Mobile and RZ SoCs.
+
+The interface supports a single parallel input with data bus width of 8 or 16
+bits.
+
+Required properties:
+- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1-H
+  and RZ/A1-M SoCs.
+- reg: Registers address base and size.
+- interrupts: The interrupt specifier.
+
+The CEU supports a single parallel input and should contain a single 'port'
+subnode with a single 'endpoint'. Connection to input devices are modeled
+according to the video interfaces OF bindings specified in:
+Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Optional endpoint properties applicable to parallel input bus described in
+the above mentioned "video-interfaces.txt" file are supported.
+
+- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+
+Example:
+
+The example describes the connection between the Capture Engine Unit and an
+OV7670 image sensor connected to i2c1 interface.
+
+ceu: ceu@e821 {
+   reg = <0xe821 0x209c>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   port {
+   ceu_in: endpoint {
+   remote-endpoint = <_out>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+};
+
+i2c1: i2c@fcfee400 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   clock-frequency = <10>;
+
+   ov7670: camera@21 {
+   compatible = "ovti,ov7670";
+   reg = <0x21>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   reset-gpios = < 11 GPIO_ACTIVE_LOW>;
+   powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov7670_out: endpoint {
+   remote-endpoint = <_in>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+   };
+};
-- 
2.7.4

[PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

[Jacopo Mondi]

Add driver for Renesas Capture Engine Unit (CEU).

The CEU interface supports capturing 'data' (YUV422) and 'images'
(NV[12|21|16|61]).

This driver aims to replace the soc_camera-based sh_mobile_ceu one.

Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
platform GR-Peach.

Tested with ov7725 camera sensor on SH4 platform Migo-R.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/platform/Kconfig   |9 +
 drivers/media/platform/Makefile  |1 +
 drivers/media/platform/renesas-ceu.c | 1659 ++
 3 files changed, 1669 insertions(+)
 create mode 100644 drivers/media/platform/renesas-ceu.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index fd0c998..fe7bd26 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
  To compile this driver as a module, choose M here: the module
  will be called stm32-dcmi.
 
+config VIDEO_RENESAS_CEU
+   tristate "Renesas Capture Engine Unit (CEU) driver"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
+   ---help---
+ This is a v4l2 driver for the Renesas CEU Interface
+
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 003b0bb..6580a6b 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o
 obj-$(CONFIG_SOC_CAMERA)   += soc_camera/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF)  += rcar_drif.o
+obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
new file mode 100644
index 000..1b8f0ef
--- /dev/null
+++ b/drivers/media/platform/renesas-ceu.c
@@ -0,0 +1,1659 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
+ * Copyright (C) 2017-2018 Jacopo Mondi 
+ *
+ * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
+ * Copyright (C) 2008 Magnus Damm
+ *
+ * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
+ * Copyright (C) 2006, Sascha Hauer, Pengutronix
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define DRIVER_NAME"renesas-ceu"
+
+/* CEU registers offsets and masks. */
+#define CEU_CAPSR  0x00 /* Capture start register  */
+#define CEU_CAPCR  0x04 /* Capture control register*/
+#define CEU_CAMCR  0x08 /* Capture interface control register  */
+#define CEU_CAMOR  0x10 /* Capture interface offset register   */
+#define CEU_CAPWR  0x14 /* Capture interface width register*/
+#define CEU_CAIFR  0x18 /* Capture interface input format register */
+#define CEU_CRCNTR 0x28 /* CEU register control register   */
+#define CEU_CRCMPR 0x2c /* CEU register forcible control register  */
+#define CEU_CFLCR  0x30 /* Capture filter control register */
+#define CEU_CFSZR  0x34 /* Capture filter size clip register   */
+#define CEU_CDWDR  0x38 /* Capture destination width register  */
+#define CEU_CDAYR  0x3c /* Capture data address Y register */
+#define CEU_CDACR  0x40 /* Capture data address C register */
+#define CEU_CFWCR  0x5c /* Firewall operation control register */
+#define CEU_CDOCR  0x64 /* Capture data output control register*/
+#define CEU_CEIER  0x70 /* Capture event interrupt enable register */
+#define CEU_CETCR  0x74 /* Capture event flag clear register   */
+#define CEU_CSTSR  0x7c /* Capture status register */
+#define CEU_CSRTR  0x80 /* Capture software reset register */
+
+/* Data synchronous fetch mode. */
+#define CEU_CAMCR_JPEG BIT(4)
+
+/* Input components ordering: CEU_CAMCR.DTARY field. */
+#define CEU_CAMCR_DTARY_8_UYVY (0x00 << 8)
+#define CEU_CAMCR_DTARY_8_VYUY (0x01 << 8)
+#define CEU_CAMCR_DTARY_8_YUYV (0x02 << 8)
+#define 

Re: [PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

On Tue, Jan 16, 2018 at 9:17 PM, Laurent Pinchart
 wrote:
> Hi Arnd,
>
> Thank you for the patch.
>
> On Tuesday, 16 January 2018 18:47:24 EET Arnd Bergmann wrote:
>> While experimenting with older compiler versions, I ran
>> into a warning that no longer shows up on gcc-4.8 or newer:
>>
>> drivers/media/platform/s3c-camif/camif-capture.c: In function
>> '__camif_subdev_try_format':
>> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array
>> subscript is below array bounds
>>
>> This is an off-by-one bug, leading to an access before the start of the
>> array, while newer compilers silently assume this undefined behavior
>> cannot happen and leave the loop at index 0 if no other entry matches.
>>
>> As Sylvester explains, we actually need to ensure that the
>> value is within the range, so this reworks the loop to be
>> easier to parse correctly, and an additional check to fall
>> back on the first format value for any unexpected input.
>>
>> I found an existing gcc bug for it and added a reduced version
>> of the function there.
>>
>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
>> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series
>> camera interface") Signed-off-by: Arnd Bergmann 
>> ---
>> v3: fix newly introduced off-by-one bug.
>> v2: rework logic rather than removing it.
>> ---
>>  drivers/media/platform/s3c-camif/camif-capture.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
>> b/drivers/media/platform/s3c-camif/camif-capture.c index
>> 437395a61065..f51b92e94a32 100644
>> --- a/drivers/media/platform/s3c-camif/camif-capture.c
>> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
>> @@ -1256,16 +1256,19 @@ static void __camif_subdev_try_format(struct
>> camif_dev *camif, {
>>   const struct s3c_camif_variant *variant = camif->variant;
>>   const struct vp_pix_limits *pix_lim;
>> - int i = ARRAY_SIZE(camif_mbus_formats);
>> + int i;
>>
>>   /* FIXME: constraints against codec or preview path ? */
>>   pix_lim = >vp_pix_limits[VP_CODEC];
>>
>> - while (i-- >= 0)
>> + for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
>>   if (camif_mbus_formats[i] == mf->code)
>>   break;
>>
>> - mf->code = camif_mbus_formats[i];
>> + if (i == ARRAY_SIZE(camif_mbus_formats))
>> + mf->code = camif_mbus_formats[0];
>> + else
>> + mf->code = camif_mbus_formats[i];
>
> I might be missing something very obvious, but isn't mf->code already ==
> camif_mbus_formats[i] in the else branch ?

Ah, that must be what I was thinking back when I first
discussed it with Sylvester in
https://patchwork.kernel.org/patch/9950041/

Unfortunately, I hadn't given it as much thought today when
I tried to reconstruct the result to send a new version

> How about simply

> unsigned int i;
>
> /* FIXME: constraints against codec or preview path ? */
> pix_lim = >vp_pix_limits[VP_CODEC];
>
> for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
> if (camif_mbus_formats[i] == mf->code)
> break;
>
> if (i == ARRAY_SIZE(camif_mbus_formats))
> mf->code = camif_mbus_formats[0];

Yes, makes sense. I'll send a v4.

  Arnd

[PATCH v6 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Jacopo Mondi]

Remove soc_camera framework dependencies from ov772x sensor driver.
- Handle clock and gpios
- Register async subdevice
- Remove soc_camera specific g/s_mbus_config operations
- Change image format colorspace from JPEG to SRGB as the two use the
  same colorspace information but JPEG makes assumptions on color
  components quantization that do not apply to the sensor
- Remove sizes crop from get_selection as driver can't scale
- Add kernel doc to driver interface header file
- Adjust build system

This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera-based CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/Kconfig  |  11 +++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov772x.c | 169 ++---
 include/media/i2c/ov772x.h |   6 +-
 4 files changed, 130 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..a61d7f4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -645,6 +645,17 @@ config VIDEO_OV5670
  To compile this driver as a module, choose M here: the
  module will be called ov5670.
 
+config VIDEO_OV772X
+   tristate "OmniVision OV772x sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV772x camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov772x.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..fb99293 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
 obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
+obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8063835..912b1b9 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * ov772x Camera Driver
  *
+ * Copyright (C) 2017 Jacopo Mondi 
+ *
  * Copyright (C) 2008 Renesas Solutions Corp.
  * Kuninori Morimoto 
  *
@@ -9,27 +12,25 @@
  * Copyright 2006-7 Jonathan Corbet 
  * Copyright (C) 2008 Magnus Damm
  * Copyright (C) 2008, Guennadi Liakhovetski 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 #include 
 #include 
 
 #include 
-#include 
-#include 
+
 #include 
-#include 
+#include 
 #include 
+#include 
 
 /*
  * register offset
@@ -393,8 +394,10 @@ struct ov772x_win_size {
 struct ov772x_priv {
struct v4l2_subdevsubdev;
struct v4l2_ctrl_handler  hdl;
-   struct v4l2_clk  *clk;
+   struct clk   *clk;
struct ov772x_camera_info*info;
+   struct gpio_desc *pwdn_gpio;
+   struct gpio_desc *rstb_gpio;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
unsigned shortflag_vflip:1;
@@ -409,7 +412,7 @@ struct ov772x_priv {
 static const struct ov772x_color_format ov772x_cfmts[] = {
{
.code   = MEDIA_BUS_FMT_YUYV8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = 0x0,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_YVYU8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = UV_ON,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_UYVY8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = 

[PATCH v6 4/9] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Jacopo Mondi]

Add Capture Engine Unit (CEU) node to device tree.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 arch/arm/boot/dts/r7s72100.dtsi | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ab9645a..5fe62f9 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -135,9 +135,9 @@
#clock-cells = <1>;
compatible = "renesas,r7s72100-mstp-clocks", 
"renesas,cpg-mstp-clocks";
reg = <0xfcfe042c 4>;
-   clocks = <_clk>;
-   clock-indices = ;
-   clock-output-names = "rtc";
+   clocks = <_clk>, <_clk>;
+   clock-indices = ;
+   clock-output-names = "ceu", "rtc";
};
 
mstp7_clks: mstp7_clks@fcfe0430 {
@@ -667,4 +667,13 @@
power-domains = <_clocks>;
status = "disabled";
};
+
+   ceu: ceu@e821 {
+   reg = <0xe821 0x3000>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_CEU>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
 };
-- 
2.7.4

[PATCH v6 5/9] v4l: i2c: Copy ov772x soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/ov772x.c | 1124 
 1 file changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov772x.c

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
new file mode 100644
index 000..8063835
--- /dev/null
+++ b/drivers/media/i2c/ov772x.c
@@ -0,0 +1,1124 @@
+/*
+ * ov772x Camera Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * register offset
+ */
+#define GAIN0x00 /* AGC - Gain control gain setting */
+#define BLUE0x01 /* AWB - Blue channel gain setting */
+#define RED 0x02 /* AWB - Red   channel gain setting */
+#define GREEN   0x03 /* AWB - Green channel gain setting */
+#define COM10x04 /* Common control 1 */
+#define BAVG0x05 /* U/B Average Level */
+#define GAVG0x06 /* Y/Gb Average Level */
+#define RAVG0x07 /* V/R Average Level */
+#define AECH0x08 /* Exposure Value - AEC MSBs */
+#define COM20x09 /* Common control 2 */
+#define PID 0x0A /* Product ID Number MSB */
+#define VER 0x0B /* Product ID Number LSB */
+#define COM30x0C /* Common control 3 */
+#define COM40x0D /* Common control 4 */
+#define COM50x0E /* Common control 5 */
+#define COM60x0F /* Common control 6 */
+#define AEC 0x10 /* Exposure Value */
+#define CLKRC   0x11 /* Internal clock */
+#define COM70x12 /* Common control 7 */
+#define COM80x13 /* Common control 8 */
+#define COM90x14 /* Common control 9 */
+#define COM10   0x15 /* Common control 10 */
+#define REG16   0x16 /* Register 16 */
+#define HSTART  0x17 /* Horizontal sensor size */
+#define HSIZE   0x18 /* Horizontal frame (HREF column) end high 8-bit */
+#define VSTART  0x19 /* Vertical frame (row) start high 8-bit */
+#define VSIZE   0x1A /* Vertical sensor size */
+#define PSHFT   0x1B /* Data format - pixel delay select */
+#define MIDH0x1C /* Manufacturer ID byte - high */
+#define MIDL0x1D /* Manufacturer ID byte - low  */
+#define LAEC0x1F /* Fine AEC value */
+#define COM11   0x20 /* Common control 11 */
+#define BDBASE  0x22 /* Banding filter Minimum AEC value */
+#define DBSTEP  0x23 /* Banding filter Maximum Setp */
+#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) */
+#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) */
+#define VPT 0x26 /* AGC/AEC Fast mode operating region */
+#define REG28   0x28 /* Register 28 */
+#define HOUTSIZE0x29 /* Horizontal data output size MSBs */
+#define EXHCH   0x2A /* Dummy pixel insert MSB */
+#define EXHCL   0x2B /* Dummy pixel insert LSB */
+#define VOUTSIZE0x2C /* Vertical data output size MSBs */
+#define ADVFL   0x2D /* LSB of insert dummy lines in Vertical direction */
+#define ADVFH   0x2E /* MSG of insert dummy lines in Vertical direction */
+#define YAVE0x2F /* Y/G Channel Average value */
+#define LUMHTH  0x30 /* Histogram AEC/AGC Luminance high level threshold */
+#define LUMLTH  0x31 /* Histogram AEC/AGC Luminance low  level threshold */
+#define HREF0x32 /* Image start and size control */
+#define DM_LNL  0x33 /* Dummy line low  8 bits */
+#define DM_LNH  0x34 /* Dummy line high 8 bits */
+#define ADOFF_B 0x35 /* AD offset compensation value for B  channel */
+#define ADOFF_R 0x36 /* AD offset compensation value for R  channel */
+#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */
+#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */
+#define OFF_B   0x39 /* Analog process B  channel offset value */
+#define OFF_R   0x3A /* Analog process R  channel offset value */
+#define OFF_GB  0x3B /* Analog process Gb channel offset value */
+#define OFF_GR  0x3C /* Analog process Gr 

[PATCH v6 8/9] media: i2c: tw9910: Remove soc_camera dependencies

[Jacopo Mondi]

Remove soc_camera framework dependencies from tw9910 sensor driver.
- Handle clock and gpios
- Register async subdevice
- Remove soc_camera specific g/s_mbus_config operations
- Add kernel doc to driver interface header file
- Adjust build system

This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera-based CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/Kconfig  |   9 +++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/tw9910.c | 162 -
 include/media/i2c/tw9910.h |   9 +++
 4 files changed, 120 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index a61d7f4..804a1bf 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -423,6 +423,15 @@ config VIDEO_TW9906
  To compile this driver as a module, choose M here: the
  module will be called tw9906.
 
+config VIDEO_TW9910
+   tristate "Techwell TW9910 video decoder"
+   depends on VIDEO_V4L2 && I2C
+   ---help---
+ Support for Techwell TW9910 NTSC/PAL/SECAM video decoder.
+
+ To compile this driver as a module, choose M here: the
+ module will be called tw9910.
+
 config VIDEO_VPX3220
tristate "vpx3220a, vpx3216b & vpx3214c video decoders"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index fb99293..e26544f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o
 obj-$(CONFIG_VIDEO_TW2804) += tw2804.o
 obj-$(CONFIG_VIDEO_TW9903) += tw9903.o
 obj-$(CONFIG_VIDEO_TW9906) += tw9906.o
+obj-$(CONFIG_VIDEO_TW9910) += tw9910.o
 obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
 obj-$(CONFIG_VIDEO_CS5345) += cs5345.o
 obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index bdb5e0a..96792df 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * tw9910 Video Driver
  *
+ * Copyright (C) 2017 Jacopo Mondi 
+ *
  * Copyright (C) 2008 Renesas Solutions Corp.
  * Kuninori Morimoto 
  *
@@ -10,12 +13,10 @@
  * Copyright 2006-7 Jonathan Corbet 
  * Copyright (C) 2008 Magnus Damm
  * Copyright (C) 2008, Guennadi Liakhovetski 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -25,9 +26,7 @@
 #include 
 #include 
 
-#include 
 #include 
-#include 
 #include 
 
 #define GET_ID(val)  ((val & 0xF8) >> 3)
@@ -228,8 +227,10 @@ struct tw9910_scale_ctrl {
 
 struct tw9910_priv {
struct v4l2_subdev  subdev;
-   struct v4l2_clk *clk;
+   struct clk  *clk;
struct tw9910_video_info*info;
+   struct gpio_desc*pdn_gpio;
+   struct gpio_desc*rstb_gpio;
const struct tw9910_scale_ctrl  *scale;
v4l2_std_id norm;
u32 revision;
@@ -582,13 +583,66 @@ static int tw9910_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
+static int tw9910_power_on(struct tw9910_priv *priv)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   int ret;
+
+   if (priv->clk) {
+   ret = clk_prepare_enable(priv->clk);
+   if (ret)
+   return ret;
+   }
+
+   if (priv->pdn_gpio) {
+   gpiod_set_value(priv->pdn_gpio, 0);
+   usleep_range(500, 1000);
+   }
+
+   /*
+* FIXME: The reset signal is connected to a shared GPIO on some
+* platforms (namely the SuperH Migo-R). Until a framework becomes
+* available to handle this cleanly, request the GPIO temporarily
+* to avoid conflicts.
+*/
+   priv->rstb_gpio = gpiod_get_optional(>dev, "rstb",
+GPIOD_OUT_LOW);
+   if (IS_ERR(priv->rstb_gpio)) {
+   dev_info(>dev, "Unable to get GPIO \"rstb\"");
+   return PTR_ERR(priv->rstb_gpio);
+   }
+
+   if (priv->rstb_gpio) {
+   gpiod_set_value(priv->rstb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(priv->rstb_gpio, 0);
+   usleep_range(500, 1000);
+
+   gpiod_put(priv->rstb_gpio);
+   }
+
+   return 0;
+}
+
+static int tw9910_power_off(struct tw9910_priv *priv)
+{
+   

[PATCH v6 7/9] v4l: i2c: Copy tw9910 soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/tw9910.c | 999 +
 1 file changed, 999 insertions(+)
 create mode 100644 drivers/media/i2c/tw9910.c

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
new file mode 100644
index 000..bdb5e0a
--- /dev/null
+++ b/drivers/media/i2c/tw9910.c
@@ -0,0 +1,999 @@
+/*
+ * tw9910 Video Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov772x driver,
+ *
+ * Copyright (C) 2008 Kuninori Morimoto 
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define GET_ID(val)  ((val & 0xF8) >> 3)
+#define GET_REV(val) (val & 0x07)
+
+/*
+ * register offset
+ */
+#define ID 0x00 /* Product ID Code Register */
+#define STATUS10x01 /* Chip Status Register I */
+#define INFORM 0x02 /* Input Format */
+#define OPFORM 0x03 /* Output Format Control Register */
+#define DLYCTR 0x04 /* Hysteresis and HSYNC Delay Control */
+#define OUTCTR10x05 /* Output Control I */
+#define ACNTL1 0x06 /* Analog Control Register 1 */
+#define CROP_HI0x07 /* Cropping Register, High */
+#define VDELAY_LO  0x08 /* Vertical Delay Register, Low */
+#define VACTIVE_LO 0x09 /* Vertical Active Register, Low */
+#define HDELAY_LO  0x0A /* Horizontal Delay Register, Low */
+#define HACTIVE_LO 0x0B /* Horizontal Active Register, Low */
+#define CNTRL1 0x0C /* Control Register I */
+#define VSCALE_LO  0x0D /* Vertical Scaling Register, Low */
+#define SCALE_HI   0x0E /* Scaling Register, High */
+#define HSCALE_LO  0x0F /* Horizontal Scaling Register, Low */
+#define BRIGHT 0x10 /* BRIGHTNESS Control Register */
+#define CONTRAST   0x11 /* CONTRAST Control Register */
+#define SHARPNESS  0x12 /* SHARPNESS Control Register I */
+#define SAT_U  0x13 /* Chroma (U) Gain Register */
+#define SAT_V  0x14 /* Chroma (V) Gain Register */
+#define HUE0x15 /* Hue Control Register */
+#define CORING10x17
+#define CORING20x18 /* Coring and IF compensation */
+#define VBICNTL0x19 /* VBI Control Register */
+#define ACNTL2 0x1A /* Analog Control 2 */
+#define OUTCTR20x1B /* Output Control 2 */
+#define SDT0x1C /* Standard Selection */
+#define SDTR   0x1D /* Standard Recognition */
+#define TEST   0x1F /* Test Control Register */
+#define CLMPG  0x20 /* Clamping Gain */
+#define IAGC   0x21 /* Individual AGC Gain */
+#define AGCGAIN0x22 /* AGC Gain */
+#define PEAKWT 0x23 /* White Peak Threshold */
+#define CLMPL  0x24 /* Clamp level */
+#define SYNCT  0x25 /* Sync Amplitude */
+#define MISSCNT0x26 /* Sync Miss Count Register */
+#define PCLAMP 0x27 /* Clamp Position Register */
+#define VCNTL1 0x28 /* Vertical Control I */
+#define VCNTL2 0x29 /* Vertical Control II */
+#define CKILL  0x2A /* Color Killer Level Control */
+#define COMB   0x2B /* Comb Filter Control */
+#define LDLY   0x2C /* Luma Delay and H Filter Control */
+#define MISC1  0x2D /* Miscellaneous Control I */
+#define LOOP   0x2E /* LOOP Control Register */
+#define MISC2  0x2F /* Miscellaneous Control II */
+#define MVSN   0x30 /* Macrovision Detection */
+#define STATUS20x31 /* Chip STATUS II */
+#define HFREF  0x32 /* H monitor */
+#define CLMD   0x33 /* CLAMP MODE */
+#define IDCNTL 0x34 /* ID Detection Control */
+#define CLCNTL10x35 /* Clamp Control I */
+#define ANAPLLCTL  0x4C
+#define VBIMIN 0x4D
+#define HSLOWCTL   0x4E
+#define WSS3   0x4F
+#define FILLDATA   0x50
+#define SDID   0x51
+#define DID0x52
+#define WSS1   0x53
+#define WSS2   0x54
+#define VVBI   0x55
+#define LCTL6  0x56
+#define LCTL7  

[PATCH v6 9/9] arch: sh: migor: Use new renesas-ceu camera driver

[Jacopo Mondi]

Migo-R platform uses sh_mobile_ceu camera driver, which is now being
replaced by a proper V4L2 camera driver named 'renesas-ceu'.

Move Migo-R platform to use the v4l2 renesas-ceu camera driver
interface and get rid of soc_camera defined components used to register
sensor drivers and of platform specific enable/disable routines.

Register clock source and GPIOs for sensor drivers, so they can use
clock and gpio APIs.

Also, memory for CEU video buffers is now reserved with membocks APIs,
and need to be declared as dma_coherent during machine initialization to
remove that architecture specific part from CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 arch/sh/boards/mach-migor/setup.c  | 225 +++--
 arch/sh/kernel/cpu/sh4a/clock-sh7722.c |   2 +-
 2 files changed, 101 insertions(+), 126 deletions(-)

diff --git a/arch/sh/boards/mach-migor/setup.c 
b/arch/sh/boards/mach-migor/setup.c
index 0bcbe58..ae4a786 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -1,17 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Renesas System Solutions Asia Pte. Ltd - Migo-R
  *
  * Copyright (C) 2008 Magnus Damm
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
  */
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -23,10 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -45,6 +45,9 @@
  * 0x1800   8GB8   NAND Flash (K9K8G08U0A)
  */
 
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu_dma_membase;
+
 static struct smc91x_platdata smc91x_info = {
.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
 };
@@ -301,65 +304,24 @@ static struct platform_device migor_lcdc_device = {
},
 };
 
-static struct clk *camera_clk;
-static DEFINE_MUTEX(camera_lock);
-
-static void camera_power_on(int is_tw)
-{
-   mutex_lock(_lock);
-
-   /* Use 10 MHz VIO_CKO instead of 24 MHz to work
-* around signal quality issues on Panel Board V2.1.
-*/
-   camera_clk = clk_get(NULL, "video_clk");
-   clk_set_rate(camera_clk, 1000);
-   clk_enable(camera_clk); /* start VIO_CKO */
-
-   /* use VIO_RST to take camera out of reset */
-   mdelay(10);
-   if (is_tw) {
-   gpio_set_value(GPIO_PTT2, 0);
-   gpio_set_value(GPIO_PTT0, 0);
-   } else {
-   gpio_set_value(GPIO_PTT0, 1);
-   }
-   gpio_set_value(GPIO_PTT3, 0);
-   mdelay(10);
-   gpio_set_value(GPIO_PTT3, 1);
-   mdelay(10); /* wait to let chip come out of reset */
-}
-
-static void camera_power_off(void)
-{
-   clk_disable(camera_clk); /* stop VIO_CKO */
-   clk_put(camera_clk);
-
-   gpio_set_value(GPIO_PTT3, 0);
-   mutex_unlock(_lock);
-}
-
-static int ov7725_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(0);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static int tw9910_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(1);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
-   .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static const struct ceu_platform_data ceu_pdata = {
+   .num_subdevs= 2,
+   .subdevs = {
+   { /* [0] = ov772x */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x21,
+   },
+   { /* [1] = tw9910 */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x45,
+   },
+   },
 };
 
 static struct resource migor_ceu_resources[] = {
@@ -373,18 +335,32 @@ static struct resource migor_ceu_resources[] = {
.start  = evt2irq(0x880),
.flags  = IORESOURCE_IRQ,
},
-   [2] = {
-   /* place holder for contiguous memory */
-   },
 };
 
 static struct platform_device migor_ceu_device = {
-   .name   = "sh_mobile_ceu",
-   .id = 0, /* "ceu0" clock */
+   .name   = "renesas-ceu",
+   .id = 0, /* ceu.0 */
.num_resources  = ARRAY_SIZE(migor_ceu_resources),
.resource   = migor_ceu_resources,
.dev= 

[PATCH 1/4] uvcvideo: Drop extern keyword in function declarations

[Laurent Pinchart]

The extern keyword isn't needed to declare functions in header files.
Drop it.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/usb/uvc/uvcvideo.h | 145 +++
 1 file changed, 71 insertions(+), 74 deletions(-)

diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 961d7c13b27b..aba0e74358bd 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -667,40 +667,39 @@ extern unsigned int uvc_hw_timestamps_param;
 /* Core driver */
 extern struct uvc_driver uvc_driver;
 
-extern struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id);
+struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id);
 
 /* Video buffers queue management. */
-extern int uvc_queue_init(struct uvc_video_queue *queue,
-   enum v4l2_buf_type type, int drop_corrupted);
-extern void uvc_queue_release(struct uvc_video_queue *queue);
-extern int uvc_request_buffers(struct uvc_video_queue *queue,
-   struct v4l2_requestbuffers *rb);
-extern int uvc_query_buffer(struct uvc_video_queue *queue,
-   struct v4l2_buffer *v4l2_buf);
-extern int uvc_create_buffers(struct uvc_video_queue *queue,
-   struct v4l2_create_buffers *v4l2_cb);
-extern int uvc_queue_buffer(struct uvc_video_queue *queue,
-   struct v4l2_buffer *v4l2_buf);
-extern int uvc_export_buffer(struct uvc_video_queue *queue,
-   struct v4l2_exportbuffer *exp);
-extern int uvc_dequeue_buffer(struct uvc_video_queue *queue,
-   struct v4l2_buffer *v4l2_buf, int nonblocking);
-extern int uvc_queue_streamon(struct uvc_video_queue *queue,
- enum v4l2_buf_type type);
-extern int uvc_queue_streamoff(struct uvc_video_queue *queue,
-  enum v4l2_buf_type type);
-extern void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect);
-extern struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
-   struct uvc_buffer *buf);
-extern int uvc_queue_mmap(struct uvc_video_queue *queue,
-   struct vm_area_struct *vma);
-extern unsigned int uvc_queue_poll(struct uvc_video_queue *queue,
-   struct file *file, poll_table *wait);
+int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
+ int drop_corrupted);
+void uvc_queue_release(struct uvc_video_queue *queue);
+int uvc_request_buffers(struct uvc_video_queue *queue,
+   struct v4l2_requestbuffers *rb);
+int uvc_query_buffer(struct uvc_video_queue *queue,
+struct v4l2_buffer *v4l2_buf);
+int uvc_create_buffers(struct uvc_video_queue *queue,
+  struct v4l2_create_buffers *v4l2_cb);
+int uvc_queue_buffer(struct uvc_video_queue *queue,
+struct v4l2_buffer *v4l2_buf);
+int uvc_export_buffer(struct uvc_video_queue *queue,
+ struct v4l2_exportbuffer *exp);
+int uvc_dequeue_buffer(struct uvc_video_queue *queue,
+  struct v4l2_buffer *v4l2_buf, int nonblocking);
+int uvc_queue_streamon(struct uvc_video_queue *queue,
+  enum v4l2_buf_type type);
+int uvc_queue_streamoff(struct uvc_video_queue *queue,
+   enum v4l2_buf_type type);
+void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect);
+struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
+struct uvc_buffer *buf);
+int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct *vma);
+unsigned int uvc_queue_poll(struct uvc_video_queue *queue, struct file *file,
+   poll_table *wait);
 #ifndef CONFIG_MMU
-extern unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
-   unsigned long pgoff);
+unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
+ unsigned long pgoff);
 #endif
-extern int uvc_queue_allocated(struct uvc_video_queue *queue);
+int uvc_queue_allocated(struct uvc_video_queue *queue);
 static inline int uvc_queue_streaming(struct uvc_video_queue *queue)
 {
return vb2_is_streaming(>queue);
@@ -711,18 +710,18 @@ extern const struct v4l2_ioctl_ops uvc_ioctl_ops;
 extern const struct v4l2_file_operations uvc_fops;
 
 /* Media controller */
-extern int uvc_mc_register_entities(struct uvc_video_chain *chain);
-extern void uvc_mc_cleanup_entity(struct uvc_entity *entity);
+int uvc_mc_register_entities(struct uvc_video_chain *chain);
+void uvc_mc_cleanup_entity(struct uvc_entity *entity);
 
 /* Video */
-extern int uvc_video_init(struct uvc_streaming *stream);
-extern int uvc_video_suspend(struct uvc_streaming *stream);
-extern int uvc_video_resume(struct uvc_streaming *stream, int reset);
-extern int uvc_video_enable(struct uvc_streaming *stream, int enable);
-extern int 

[PATCH 4/4] uvcvideo: Use parentheses around sizeof operand

[Laurent Pinchart]

While the sizeof is an operator and not a function, the preferred coding
style in the kernel is to enclose its operand in parentheses. To avoid
mixing multiple coding styles in the driver, use parentheses around all
sizeof operands.

While at it replace a kmalloc() with a kmalloc_array() to silence a
checkpatch warning triggered by this patch.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/usb/uvc/uvc_ctrl.c   |  6 ++---
 drivers/media/usb/uvc/uvc_driver.c | 53 +++---
 drivers/media/usb/uvc/uvc_v4l2.c   | 12 -
 drivers/media/usb/uvc/uvc_video.c  |  2 +-
 4 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 723c517474fc..102594ec3e97 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1019,10 +1019,10 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain 
*chain,
struct uvc_menu_info *menu;
unsigned int i;
 
-   memset(v4l2_ctrl, 0, sizeof *v4l2_ctrl);
+   memset(v4l2_ctrl, 0, sizeof(*v4l2_ctrl));
v4l2_ctrl->id = mapping->id;
v4l2_ctrl->type = mapping->v4l2_type;
-   strlcpy(v4l2_ctrl->name, mapping->name, sizeof v4l2_ctrl->name);
+   strlcpy(v4l2_ctrl->name, mapping->name, sizeof(v4l2_ctrl->name));
v4l2_ctrl->flags = 0;
 
if (!(ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR))
@@ -1182,7 +1182,7 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
}
}
 
-   strlcpy(query_menu->name, menu_info->name, sizeof query_menu->name);
+   strlcpy(query_menu->name, menu_info->name, sizeof(query_menu->name));
 
 done:
mutex_unlock(>ctrl_mutex);
diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 718c3fcde287..2469b49b2b30 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -274,7 +274,7 @@ void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
u32 x, y, r;
unsigned int i, n;
 
-   an = kmalloc(n_terms * sizeof *an, GFP_KERNEL);
+   an = kmalloc_array(n_terms, sizeof(*an), GFP_KERNEL);
if (an == NULL)
return;
 
@@ -423,7 +423,7 @@ static int uvc_parse_format(struct uvc_device *dev,
 
if (fmtdesc != NULL) {
strlcpy(format->name, fmtdesc->name,
-   sizeof format->name);
+   sizeof(format->name));
format->fcc = fmtdesc->fcc;
} else {
uvc_printk(KERN_INFO, "Unknown video format %pUl\n",
@@ -466,7 +466,7 @@ static int uvc_parse_format(struct uvc_device *dev,
return -EINVAL;
}
 
-   strlcpy(format->name, "MJPEG", sizeof format->name);
+   strlcpy(format->name, "MJPEG", sizeof(format->name));
format->fcc = V4L2_PIX_FMT_MJPEG;
format->flags = UVC_FMT_FLAG_COMPRESSED;
format->bpp = 0;
@@ -484,13 +484,13 @@ static int uvc_parse_format(struct uvc_device *dev,
 
switch (buffer[8] & 0x7f) {
case 0:
-   strlcpy(format->name, "SD-DV", sizeof format->name);
+   strlcpy(format->name, "SD-DV", sizeof(format->name));
break;
case 1:
-   strlcpy(format->name, "SDL-DV", sizeof format->name);
+   strlcpy(format->name, "SDL-DV", sizeof(format->name));
break;
case 2:
-   strlcpy(format->name, "HD-DV", sizeof format->name);
+   strlcpy(format->name, "HD-DV", sizeof(format->name));
break;
default:
uvc_trace(UVC_TRACE_DESCR, "device %d videostreaming "
@@ -501,7 +501,7 @@ static int uvc_parse_format(struct uvc_device *dev,
}
 
strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
-   sizeof format->name);
+   sizeof(format->name));
 
format->fcc = V4L2_PIX_FMT_DV;
format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM;
@@ -510,7 +510,7 @@ static int uvc_parse_format(struct uvc_device *dev,
 
/* Create a dummy frame descriptor. */
frame = >frame[0];
-   memset(>frame[0], 0, sizeof format->frame[0]);
+   memset(>frame[0], 0, sizeof(format->frame[0]));
frame->bFrameIntervalType = 1;
frame->dwDefaultFrameInterval = 1;
frame->dwFrameInterval = *intervals;
@@ -677,7 +677,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
return -EINVAL;
}
 
-   streaming = kzalloc(sizeof *streaming, GFP_KERNEL);
+ 

[PATCH 3/4] uvcvideo: Use internal kernel integer types

[Laurent Pinchart]

Replace the __[su]{8,16,32} variant of integer types with the
non-underscored types as the code is internal to the driver, not exposed
to userspace.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/usb/uvc/uvc_ctrl.c   |  56 ++--
 drivers/media/usb/uvc/uvc_driver.c |  36 
 drivers/media/usb/uvc/uvc_isight.c |   6 +-
 drivers/media/usb/uvc/uvc_status.c |   4 +-
 drivers/media/usb/uvc/uvc_v4l2.c   |  62 ++---
 drivers/media/usb/uvc/uvc_video.c  |  40 
 drivers/media/usb/uvc/uvcvideo.h   | 182 ++---
 7 files changed, 193 insertions(+), 193 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 586f0e94061b..723c517474fc 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -366,10 +366,10 @@ static struct uvc_menu_info exposure_auto_controls[] = {
{ 8, "Aperture Priority Mode" },
 };
 
-static __s32 uvc_ctrl_get_zoom(struct uvc_control_mapping *mapping,
-   __u8 query, const __u8 *data)
+static s32 uvc_ctrl_get_zoom(struct uvc_control_mapping *mapping,
+   u8 query, const u8 *data)
 {
-   __s8 zoom = (__s8)data[0];
+   s8 zoom = (s8)data[0];
 
switch (query) {
case UVC_GET_CUR:
@@ -385,17 +385,17 @@ static __s32 uvc_ctrl_get_zoom(struct uvc_control_mapping 
*mapping,
 }
 
 static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping,
-   __s32 value, __u8 *data)
+   s32 value, u8 *data)
 {
data[0] = value == 0 ? 0 : (value > 0) ? 1 : 0xff;
data[2] = min((int)abs(value), 0xff);
 }
 
-static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping,
-   __u8 query, const __u8 *data)
+static s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping,
+   u8 query, const u8 *data)
 {
unsigned int first = mapping->offset / 8;
-   __s8 rel = (__s8)data[first];
+   s8 rel = (s8)data[first];
 
switch (query) {
case UVC_GET_CUR:
@@ -412,7 +412,7 @@ static __s32 uvc_ctrl_get_rel_speed(struct 
uvc_control_mapping *mapping,
 }
 
 static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping,
-   __s32 value, __u8 *data)
+   s32 value, u8 *data)
 {
unsigned int first = mapping->offset / 8;
 
@@ -745,17 +745,17 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
  * Utility functions
  */
 
-static inline __u8 *uvc_ctrl_data(struct uvc_control *ctrl, int id)
+static inline u8 *uvc_ctrl_data(struct uvc_control *ctrl, int id)
 {
return ctrl->uvc_data + id * ctrl->info.size;
 }
 
-static inline int uvc_test_bit(const __u8 *data, int bit)
+static inline int uvc_test_bit(const u8 *data, int bit)
 {
return (data[bit >> 3] >> (bit & 7)) & 1;
 }
 
-static inline void uvc_clear_bit(__u8 *data, int bit)
+static inline void uvc_clear_bit(u8 *data, int bit)
 {
data[bit >> 3] &= ~(1 << (bit & 7));
 }
@@ -765,20 +765,20 @@ static inline void uvc_clear_bit(__u8 *data, int bit)
  * a signed 32bit integer. Sign extension will be performed if the mapping
  * references a signed data type.
  */
-static __s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
-   __u8 query, const __u8 *data)
+static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
+   u8 query, const u8 *data)
 {
int bits = mapping->size;
int offset = mapping->offset;
-   __s32 value = 0;
-   __u8 mask;
+   s32 value = 0;
+   u8 mask;
 
data += offset / 8;
offset &= 7;
mask = ((1LL << bits) - 1) << offset;
 
for (; bits > 0; data++) {
-   __u8 byte = *data & mask;
+   u8 byte = *data & mask;
value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
bits -= 8 - (offset > 0 ? offset : 0);
offset -= 8;
@@ -796,11 +796,11 @@ static __s32 uvc_get_le_value(struct uvc_control_mapping 
*mapping,
  * in the little-endian data stored at 'data' to the value 'value'.
  */
 static void uvc_set_le_value(struct uvc_control_mapping *mapping,
-   __s32 value, __u8 *data)
+   s32 value, u8 *data)
 {
int bits = mapping->size;
int offset = mapping->offset;
-   __u8 mask;
+   u8 mask;
 
/* According to the v4l2 spec, writing any value to a button control
 * should result in the action belonging to the button control being
@@ -826,13 +826,13 @@ static void uvc_set_le_value(struct uvc_control_mapping 
*mapping,
  * Terminal and unit management
  */
 
-static const __u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
-static const __u8 uvc_camera_guid[16] = UVC_GUID_UVC_CAMERA;
-static const __u8 uvc_media_transport_input_guid[16] =
+static const u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
+static const u8 uvc_camera_guid[16] = UVC_GUID_UVC_CAMERA;
+static const u8 uvc_media_transport_input_guid[16] =

[PATCH 2/4] uvcvideo: Use kernel integer types

[Laurent Pinchart]

Replace the uint_{8,16,32} types with the corresponding native kernel
types u{8,16,32}.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/usb/uvc/uvc_driver.c | 16 
 drivers/media/usb/uvc/uvc_v4l2.c   |  2 +-
 drivers/media/usb/uvc/uvcvideo.h   |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index fd387bf3f02d..56d906dd7044 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -267,11 +267,11 @@ static __u32 uvc_colorspace(const __u8 primaries)
  * continued fraction decomposition. Using 8 and 333 for n_terms and threshold
  * respectively seems to give nice results.
  */
-void uvc_simplify_fraction(uint32_t *numerator, uint32_t *denominator,
+void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
unsigned int n_terms, unsigned int threshold)
 {
-   uint32_t *an;
-   uint32_t x, y, r;
+   u32 *an;
+   u32 x, y, r;
unsigned int i, n;
 
an = kmalloc(n_terms * sizeof *an, GFP_KERNEL);
@@ -318,21 +318,21 @@ void uvc_simplify_fraction(uint32_t *numerator, uint32_t 
*denominator,
  * to compute numerator / denominator * 1000 using 32 bit fixed point
  * arithmetic only.
  */
-uint32_t uvc_fraction_to_interval(uint32_t numerator, uint32_t denominator)
+u32 uvc_fraction_to_interval(u32 numerator, u32 denominator)
 {
-   uint32_t multiplier;
+   u32 multiplier;
 
/* Saturate the result if the operation would overflow. */
if (denominator == 0 ||
-   numerator/denominator >= ((uint32_t)-1)/1000)
-   return (uint32_t)-1;
+   numerator/denominator >= ((u32)-1)/1000)
+   return (u32)-1;
 
/* Divide both the denominator and the multiplier by two until
 * numerator * multiplier doesn't overflow. If anyone knows a better
 * algorithm please let me know.
 */
multiplier = 1000;
-   while (numerator > ((uint32_t)-1)/multiplier) {
+   while (numerator > ((u32)-1)/multiplier) {
multiplier /= 2;
denominator /= 2;
}
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index e8db37937571..784796f9f2e1 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -336,7 +336,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming *stream,
 static int uvc_v4l2_get_streamparm(struct uvc_streaming *stream,
struct v4l2_streamparm *parm)
 {
-   uint32_t numerator, denominator;
+   u32 numerator, denominator;
 
if (parm->type != stream->type)
return -EINVAL;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index aba0e74358bd..394c6dcdc85b 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -777,9 +777,9 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
  struct uvc_xu_control_query *xqry);
 
 /* Utility functions */
-void uvc_simplify_fraction(uint32_t *numerator, uint32_t *denominator,
+void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
   unsigned int n_terms, unsigned int threshold);
-uint32_t uvc_fraction_to_interval(uint32_t numerator, uint32_t denominator);
+u32 uvc_fraction_to_interval(u32 numerator, u32 denominator);
 struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
__u8 epaddr);
 
-- 
Regards,

Laurent Pinchart

[PATCH 0/4] uvcvideo style cleanups

[Laurent Pinchart]

Hello,

I was growing tired of explaining during review that, while the uvcvideo.h
header used the extern keyword in front of function declarations, the same
style mistake didn't have to be copied in all patches. I thus decided to write
patch 1/4, and while at it I fixed similar style issues in patch 2/4 to 4/4.

Apart from the kmalloc() to kmalloc_array() change, a byte-to-byte comparison
of the .text sections of the module shows no difference before and after this
series.

Laurent Pinchart (4):
  uvcvideo: Drop extern keyword in function declarations
  uvcvideo: Use kernel integer types
  uvcvideo: Use internal kernel integer types
  uvcvideo: Use parentheses around sizeof operand

 drivers/media/usb/uvc/uvc_ctrl.c   |  62 +++
 drivers/media/usb/uvc/uvc_driver.c |  97 +--
 drivers/media/usb/uvc/uvc_isight.c |   6 +-
 drivers/media/usb/uvc/uvc_status.c |   4 +-
 drivers/media/usb/uvc/uvc_v4l2.c   |  76 -
 drivers/media/usb/uvc/uvc_video.c  |  42 ++---
 drivers/media/usb/uvc/uvcvideo.h   | 321 ++---
 7 files changed, 303 insertions(+), 305 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/7] si2157: Add hybrid tuner support

[Brad Love]

On 2018-01-15 23:05, Antti Palosaari wrote:
> Hello
> So the use case is to share single tuner with multiple demodulators?
> Why don't just register single tuner and pass that info to multiple
> demods?
>
> Antti

Hello Antti,

It was done this way because of lack of knowledge of other ways. The
method I used mirrored that done by the three other drivers I found
which supported *and* included multiple front ends. We had this _attach
function sitting around as part of wip analog support to the si2157, and
it seemed like a nice fit here.

I just perused the tree again and noticed one spot I missed originally,
which does not use an _attach function. I didn't realize I could just
memcpy tuner_ops from fe[0] to fe[1] and call it a done deal, it does
appear to work the same though.

Is this really all that is required? If so, I'll modify patch 7/7 and
put this patch to the side for now.

Cheers,

Brad


>
> On 01/12/2018 06:19 PM, Brad Love wrote:
>> Add ability to share a tuner amongst demodulators. Addtional
>> demods are attached using hybrid_tuner_instance_list.
>>
>> The changes are equivalent to moving all of probe to _attach.
>> Results are backwards compatible with current usage.
>>
>> If the tuner is acquired via attach, then .release cleans state.
>> if the tuner is an i2c driver, then .release is set to NULL, and
>> .remove cleans remaining state.
>>
>> The following file contains a static si2157_attach:
>> - drivers/media/pci/saa7164/saa7164-dvb.c
>> The function name has been appended with _priv to appease
>> the compiler.
>>
>> Signed-off-by: Brad Love 
>> ---
>>   drivers/media/pci/saa7164/saa7164-dvb.c |  11 +-
>>   drivers/media/tuners/si2157.c   | 232
>> +++-
>>   drivers/media/tuners/si2157.h   |  14 ++
>>   drivers/media/tuners/si2157_priv.h  |   5 +
>>   4 files changed, 192 insertions(+), 70 deletions(-)
>>
>> diff --git a/drivers/media/pci/saa7164/saa7164-dvb.c
>> b/drivers/media/pci/saa7164/saa7164-dvb.c
>> index e76d3ba..9522c6c 100644
>> --- a/drivers/media/pci/saa7164/saa7164-dvb.c
>> +++ b/drivers/media/pci/saa7164/saa7164-dvb.c
>> @@ -110,8 +110,9 @@ static struct si2157_config
>> hauppauge_hvr2255_tuner_config = {
>>   .if_port = 1,
>>   };
>>   -static int si2157_attach(struct saa7164_port *port, struct
>> i2c_adapter *adapter,
>> -    struct dvb_frontend *fe, u8 addr8bit, struct si2157_config *cfg)
>> +static int si2157_attach_priv(struct saa7164_port *port,
>> +    struct i2c_adapter *adapter, struct dvb_frontend *fe,
>> +    u8 addr8bit, struct si2157_config *cfg)
>>   {
>>   struct i2c_board_info bi;
>>   struct i2c_client *tuner;
>> @@ -624,11 +625,13 @@ int saa7164_dvb_register(struct saa7164_port
>> *port)
>>   if (port->dvb.frontend != NULL) {
>>     if (port->nr == 0) {
>> -    si2157_attach(port, >i2c_bus[0].i2c_adap,
>> +    si2157_attach_priv(port,
>> +  >i2c_bus[0].i2c_adap,
>>     port->dvb.frontend, 0xc0,
>>     _hvr2255_tuner_config);
>>   } else {
>> -    si2157_attach(port, >i2c_bus[1].i2c_adap,
>> +    si2157_attach_priv(port,
>> +  >i2c_bus[1].i2c_adap,
>>     port->dvb.frontend, 0xc0,
>>     _hvr2255_tuner_config);
>>   }
>> diff --git a/drivers/media/tuners/si2157.c
>> b/drivers/media/tuners/si2157.c
>> index e35b1fa..9121361 100644
>> --- a/drivers/media/tuners/si2157.c
>> +++ b/drivers/media/tuners/si2157.c
>> @@ -18,6 +18,11 @@
>>     static const struct dvb_tuner_ops si2157_ops;
>>   +static DEFINE_MUTEX(si2157_list_mutex);
>> +static LIST_HEAD(hybrid_tuner_instance_list);
>> +
>> +/*-*/
>>
>> +
>>   /* execute firmware command */
>>   static int si2157_cmd_execute(struct i2c_client *client, struct
>> si2157_cmd *cmd)
>>   {
>> @@ -385,6 +390,31 @@ static int si2157_get_if_frequency(struct
>> dvb_frontend *fe, u32 *frequency)
>>   return 0;
>>   }
>>   +static void si2157_release(struct dvb_frontend *fe)
>> +{
>> +    struct i2c_client *client = fe->tuner_priv;
>> +    struct si2157_dev *dev = i2c_get_clientdata(client);
>> +
>> +    dev_dbg(>dev, "%s()\n", __func__);
>> +
>> +    /* only do full cleanup on final instance */
>> +    if (hybrid_tuner_report_instance_count(dev) == 1) {
>> +    /* stop statistics polling */
>> +    cancel_delayed_work_sync(>stat_work);
>> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>> +    if (dev->mdev)
>> +    media_device_unregister_entity(>ent);
>> +#endif
>> +    i2c_set_clientdata(client, NULL);
>> +    }
>> +
>> +    mutex_lock(_list_mutex);
>> +    hybrid_tuner_release_state(dev);
>> +    mutex_unlock(_list_mutex);
>> +
>> +    fe->tuner_priv = NULL;
>> +}
>> +
>>   static const struct dvb_tuner_ops si2157_ops = {
>> 

Re: [PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep

[Antti Palosaari]

On 01/16/2018 10:14 PM, Brad Love wrote:


On 2018-01-16 13:32, Antti Palosaari wrote:

On 01/16/2018 07:31 PM, Brad Love wrote:


On 2018-01-15 23:07, Antti Palosaari wrote:

Hello
And what is rationale here, is there some use case demod must be
active and ts set to tristate (disabled)? Just put demod sleep when
you don't use it.

regards
Antti


Hello Antti,

Perhaps the .ts_bus_ctrl callback does not need to be included in ops,
but the function is necessary. The demod is already put to sleep when
not in use, but it leaves the ts bus open. The ts bus has no reason to
be open when the demod is put to sleep. Leaving the ts bus open during
sleep affects the other connected demod and nothing is received by it.
The lgdt3306a driver already tri states its ts bus when put to sleep,
the si2168 should as well.


Sounds possible, but unlikely as chip is firmware driven. When you put
chip to sleep you usually want set ts pins to tristate (also other
unused pins) in order to save energy. I haven't never tested it anyway
though, so it could be possible it leaves those pins to some other
state like random output at given time.

And if you cannot get stream from lgdt3306a, which is connected to
same bus, it really sounds like ts bus pins are left some state
(cannot work if same pin is driven high to other demod whilst other
tries to drive it low.

Setting ts pins to tri-state during sleep should resolve your issue.


Hello Antti,

This patch fixes the issue I'm describing, hence why I submitted it. The
ts bus must be tristated before putting the chip to sleep for the other
demod to get a stream.



I can test tri-state using power meter on some day, but it may be so 
small current that it cannot be seen usb power meter I use (YZXstudio, 
very nice small power meter).


regards
Antti

--
http://palosaari.fi/

Re: [PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Tuesday, 16 January 2018 18:47:24 EET Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> As Sylvester explains, we actually need to ensure that the
> value is within the range, so this reworks the loop to be
> easier to parse correctly, and an additional check to fall
> back on the first format value for any unexpected input.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series
> camera interface") Signed-off-by: Arnd Bergmann 
> ---
> v3: fix newly introduced off-by-one bug.
> v2: rework logic rather than removing it.
> ---
>  drivers/media/platform/s3c-camif/camif-capture.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
> b/drivers/media/platform/s3c-camif/camif-capture.c index
> 437395a61065..f51b92e94a32 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1256,16 +1256,19 @@ static void __camif_subdev_try_format(struct
> camif_dev *camif, {
>   const struct s3c_camif_variant *variant = camif->variant;
>   const struct vp_pix_limits *pix_lim;
> - int i = ARRAY_SIZE(camif_mbus_formats);
> + int i;
> 
>   /* FIXME: constraints against codec or preview path ? */
>   pix_lim = >vp_pix_limits[VP_CODEC];
> 
> - while (i-- >= 0)
> + for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
>   if (camif_mbus_formats[i] == mf->code)
>   break;
> 
> - mf->code = camif_mbus_formats[i];
> + if (i == ARRAY_SIZE(camif_mbus_formats))
> + mf->code = camif_mbus_formats[0];
> + else
> + mf->code = camif_mbus_formats[i];

I might be missing something very obvious, but isn't mf->code already == 
camif_mbus_formats[i] in the else branch ? How about simply

unsigned int i;

/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];

for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;

if (i == ARRAY_SIZE(camif_mbus_formats))
mf->code = camif_mbus_formats[0];

(I do love the for (...) { ... } else { ... } construct from Python, I miss it 
so much in C.)

>   if (pad == CAMIF_SD_PAD_SINK) {
>   v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep

[Brad Love]

On 2018-01-16 13:32, Antti Palosaari wrote:
> On 01/16/2018 07:31 PM, Brad Love wrote:
>>
>> On 2018-01-15 23:07, Antti Palosaari wrote:
>>> Hello
>>> And what is rationale here, is there some use case demod must be
>>> active and ts set to tristate (disabled)? Just put demod sleep when
>>> you don't use it.
>>>
>>> regards
>>> Antti
>>
>> Hello Antti,
>>
>> Perhaps the .ts_bus_ctrl callback does not need to be included in ops,
>> but the function is necessary. The demod is already put to sleep when
>> not in use, but it leaves the ts bus open. The ts bus has no reason to
>> be open when the demod is put to sleep. Leaving the ts bus open during
>> sleep affects the other connected demod and nothing is received by it.
>> The lgdt3306a driver already tri states its ts bus when put to sleep,
>> the si2168 should as well.
>
> Sounds possible, but unlikely as chip is firmware driven. When you put
> chip to sleep you usually want set ts pins to tristate (also other
> unused pins) in order to save energy. I haven't never tested it anyway
> though, so it could be possible it leaves those pins to some other
> state like random output at given time.
>
> And if you cannot get stream from lgdt3306a, which is connected to
> same bus, it really sounds like ts bus pins are left some state
> (cannot work if same pin is driven high to other demod whilst other
> tries to drive it low.
>
> Setting ts pins to tri-state during sleep should resolve your issue.

Hello Antti,

This patch fixes the issue I'm describing, hence why I submitted it. The
ts bus must be tristated before putting the chip to sleep for the other
demod to get a stream.

Cheers,

Brad



>
>
> regards
> Antti

Re: [PATCH] v4l: async: Protect against double notifier regstrations

[Kieran Bingham]

Hi Sakari,

Thanks for the quick review.

On 16/01/18 15:23, Sakari Ailus wrote:
> Hi Kieran,
> 
> On Tue, Jan 16, 2018 at 02:52:58PM +, Kieran Bingham wrote:
>> From: Kieran Bingham 
>>
>> It can be easy to attempt to register the same notifier twice
>> in mis-handled error cases such as working with -EPROBE_DEFER.
>>
>> This results in odd kernel crashes where the notifier_list becomes
>> corrupted due to adding the same entry twice.
>>
>> Protect against this so that a developer has some sense of the pending
>> failure.
>>
>> Signed-off-by: Kieran Bingham 
>> ---
>>  drivers/media/v4l2-core/v4l2-async.c | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
>> b/drivers/media/v4l2-core/v4l2-async.c
>> index 2b08d03b251d..e8476f0755ca 100644
>> --- a/drivers/media/v4l2-core/v4l2-async.c
>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>> @@ -374,6 +374,7 @@ static int __v4l2_async_notifier_register(struct 
>> v4l2_async_notifier *notifier)
>>  struct device *dev =
>>  notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
>>  struct v4l2_async_subdev *asd;
>> +struct v4l2_async_notifier *n;
>>  int ret;
>>  int i;
>>  
>> @@ -385,6 +386,19 @@ static int __v4l2_async_notifier_register(struct 
>> v4l2_async_notifier *notifier)
>>  
>>  mutex_lock(_lock);
>>  
>> +/*
>> + * Registering the same notifier can occur if a driver incorrectly
>> + * handles a -EPROBE_DEFER for example, and will break in a
>> + * confusing fashion with linked-list corruption.
>> + */
> 
> This would seem fine in the commit message, and it's essentially there
> already. How about simply:
> 
>   /* Avoid re-registering a notifier. */

Yes, I think I was being my usual overly verbose self trying to say why this is 
bad.

Your version is simpler and clear.

>   
> You should actually perform the check before initialising the notifier's
> lists.

I could move the INIT_LIST_HEADs to after the check, and inside the locked
critical section if you think that's appropriate - but I'm not certain it
matters as the driver likely won't work anyway, as it's only going to be here if
it's trying to re-register the device after failing to unregister or some such.

> Although things are likely in a quite bad shape already if this
> happens.

Yes, things are definitely in a bad shape if this happens, and it is not at all
obvious why when you debug :) - That's why I wanted to add this warning.

It's a driver bug - so I don't think it's essential to keep the system running
for long if it happens - just make sure the driver developer knows what has gone
wrong.

I fixed my bug - but it was a real pain to find because of the seemingly random
corruption.

> 
>> +list_for_each_entry(n, _list, list) {
>> +if (n == notifier) {
> 
> if (WARN_ON(n == notifier)) {
> 
> And drop the error message below?
> 

Yes - that sounds reasonable too.

>> +dev_err(dev, "Notifier has already been registered\n");
>> +ret = -EEXIST;
>> +goto err_unlock;
>> +}
>> +}
>> +
>>  for (i = 0; i < notifier->num_subdevs; i++) {
>>  asd = notifier->subdevs[i];
>>  


--
Kieran

Re: [PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep

[Antti Palosaari]

On 01/16/2018 07:31 PM, Brad Love wrote:


On 2018-01-15 23:07, Antti Palosaari wrote:

Hello
And what is rationale here, is there some use case demod must be
active and ts set to tristate (disabled)? Just put demod sleep when
you don't use it.

regards
Antti


Hello Antti,

Perhaps the .ts_bus_ctrl callback does not need to be included in ops,
but the function is necessary. The demod is already put to sleep when
not in use, but it leaves the ts bus open. The ts bus has no reason to
be open when the demod is put to sleep. Leaving the ts bus open during
sleep affects the other connected demod and nothing is received by it.
The lgdt3306a driver already tri states its ts bus when put to sleep,
the si2168 should as well.


Sounds possible, but unlikely as chip is firmware driven. When you put 
chip to sleep you usually want set ts pins to tristate (also other 
unused pins) in order to save energy. I haven't never tested it anyway 
though, so it could be possible it leaves those pins to some other state 
like random output at given time.


And if you cannot get stream from lgdt3306a, which is connected to same 
bus, it really sounds like ts bus pins are left some state (cannot work 
if same pin is driven high to other demod whilst other tries to drive it 
low.


Setting ts pins to tri-state during sleep should resolve your issue.


regards
Antti
--
http://palosaari.fi/

Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

[Tony Luck]

On Sat, Jan 13, 2018 at 10:51 AM, Linus Torvalds
 wrote:
> On Fri, Jan 12, 2018 at 4:15 PM, Tony Luck  wrote:
> So your argument depends on "the uarch will actually run the code in
> order if there are no events that block the pipeline".

And might be bogus ... I'm a software person not a u-arch expert. That
sounded good in my head, but the level of parallelism may be greater
than I can imagine.

> Or at least it depends on a certain latency of the killing of any OoO
> execution being low enough that the cache access doesn't even begin.
>
> I realize that that is very much a particular microarchitectural
> detail, but it's actually a *big* deal. Do we have a set of rules for
> what is not a worry, simply because the speculated accesses get killed
> early enough?
>
> Apparently "test a register value against a constant" is good enough,
> assuming that register is also needed for the address of the access.

People who do understand this are working on what can be guaranteed.
For now don't make big plans based on my ramblings.

-Tony

Re: [PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep

[Brad Love]

On 2018-01-15 23:07, Antti Palosaari wrote:
> Hello
> And what is rationale here, is there some use case demod must be
> active and ts set to tristate (disabled)? Just put demod sleep when
> you don't use it.
>
> regards
> Antti

Hello Antti,

Perhaps the .ts_bus_ctrl callback does not need to be included in ops,
but the function is necessary. The demod is already put to sleep when
not in use, but it leaves the ts bus open. The ts bus has no reason to
be open when the demod is put to sleep. Leaving the ts bus open during
sleep affects the other connected demod and nothing is received by it.
The lgdt3306a driver already tri states its ts bus when put to sleep,
the si2168 should as well.

Cheers,

Brad



>
> On 01/12/2018 06:19 PM, Brad Love wrote:
>> Includes a function to set TS MODE property os si2168. The function
>> either disables the TS output bus, or sets mode to config option.
>>
>> When going to sleep the TS bus is turned off, this makes the driver
>> compatible with multiple frontend usage.
>>
>> Signed-off-by: Brad Love 
>> ---
>>   drivers/media/dvb-frontends/si2168.c | 38
>> 
>>   drivers/media/dvb-frontends/si2168.h |  1 +
>>   2 files changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/dvb-frontends/si2168.c
>> b/drivers/media/dvb-frontends/si2168.c
>> index 539399d..429c03a 100644
>> --- a/drivers/media/dvb-frontends/si2168.c
>> +++ b/drivers/media/dvb-frontends/si2168.c
>> @@ -409,6 +409,30 @@ static int si2168_set_frontend(struct
>> dvb_frontend *fe)
>>   return ret;
>>   }
>>   +static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire)
>> +{
>> +    struct i2c_client *client = fe->demodulator_priv;
>> +    struct si2168_dev *dev = i2c_get_clientdata(client);
>> +    struct si2168_cmd cmd;
>> +    int ret = 0;
>> +
>> +    dev_dbg(>dev, "%s acquire: %d\n", __func__, acquire);
>> +
>> +    /* set TS_MODE property */
>> +    memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
>> +    if (acquire)
>> +    cmd.args[4] |= dev->ts_mode;
>> +    else
>> +    cmd.args[4] |= SI2168_TS_TRISTATE;
>> +    if (dev->ts_clock_gapped)
>> +    cmd.args[4] |= 0x40;
>> +    cmd.wlen = 6;
>> +    cmd.rlen = 4;
>> +    ret = si2168_cmd_execute(client, );
>> +
>> +    return ret;
>> +}
>> +
>>   static int si2168_init(struct dvb_frontend *fe)
>>   {
>>   struct i2c_client *client = fe->demodulator_priv;
>> @@ -540,14 +564,7 @@ static int si2168_init(struct dvb_frontend *fe)
>>    dev->version >> 24 & 0xff, dev->version >> 16 & 0xff,
>>    dev->version >> 8 & 0xff, dev->version >> 0 & 0xff);
>>   -    /* set ts mode */
>> -    memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
>> -    cmd.args[4] |= dev->ts_mode;
>> -    if (dev->ts_clock_gapped)
>> -    cmd.args[4] |= 0x40;
>> -    cmd.wlen = 6;
>> -    cmd.rlen = 4;
>> -    ret = si2168_cmd_execute(client, );
>> +    ret = si2168_ts_bus_ctrl(fe, 1);
>>   if (ret)
>>   goto err;
>>   @@ -584,6 +601,9 @@ static int si2168_sleep(struct dvb_frontend *fe)
>>     dev->active = false;
>>   +    /* tri-state data bus */
>> +    si2168_ts_bus_ctrl(fe, 0);
>> +
>>   /* Firmware B 4.0-11 or later loses warm state during sleep */
>>   if (dev->version > ('B' << 24 | 4 << 16 | 0 << 8 | 11 << 0))
>>   dev->warm = false;
>> @@ -681,6 +701,8 @@ static const struct dvb_frontend_ops si2168_ops = {
>>   .init = si2168_init,
>>   .sleep = si2168_sleep,
>>   +    .ts_bus_ctrl  = si2168_ts_bus_ctrl,
>> +
>>   .set_frontend = si2168_set_frontend,
>>     .read_status = si2168_read_status,
>> diff --git a/drivers/media/dvb-frontends/si2168.h
>> b/drivers/media/dvb-frontends/si2168.h
>> index 3225d0c..f48f0fb 100644
>> --- a/drivers/media/dvb-frontends/si2168.h
>> +++ b/drivers/media/dvb-frontends/si2168.h
>> @@ -38,6 +38,7 @@ struct si2168_config {
>>   /* TS mode */
>>   #define SI2168_TS_PARALLEL    0x06
>>   #define SI2168_TS_SERIAL    0x03
>> +#define SI2168_TS_TRISTATE    0x00
>>   u8 ts_mode;
>>     /* TS clock inverted */
>>
>

Re: [RFT PATCH v3 4/6] uvcvideo: queue: Simplify spin-lock usage

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday, 12 January 2018 11:19:26 EET Kieran Bingham wrote:
> Both uvc_start_streaming(), and uvc_stop_streaming() are called from
> userspace context. As such, they do not need to save the IRQ state, and
> can use spin_lock_irq() and spin_unlock_irq() respectively.

Note that userspace context isn't enough, the key part is that they're called 
with interrupts enabled. If they were called from userspace context but under 
a spin_lock_irq() you couldn't use spin_unlock_irq() in those functions.

> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/usb/uvc/uvc_queue.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index 4a581d631525..ddac4d89a291 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -158,7 +158,6 @@ static int uvc_start_streaming(struct vb2_queue *vq,
> unsigned int count) {
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>   struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> - unsigned long flags;
>   int ret;
> 
>   queue->buf_used = 0;
> @@ -167,9 +166,9 @@ static int uvc_start_streaming(struct vb2_queue *vq,
> unsigned int count) if (ret == 0)
>   return 0;
> 
> - spin_lock_irqsave(>irqlock, flags);
> + spin_lock_irq(>irqlock);
>   uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED);
> - spin_unlock_irqrestore(>irqlock, flags);
> + spin_unlock_irq(>irqlock);
> 
>   return ret;
>  }
> @@ -178,13 +177,12 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
>  {
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>   struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> - unsigned long flags;
> 
>   uvc_video_enable(stream, 0);
> 
> - spin_lock_irqsave(>irqlock, flags);
> + spin_lock_irq(>irqlock);
>   uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> - spin_unlock_irqrestore(>irqlock, flags);
> + spin_unlock_irq(>irqlock);
>  }

Please add a one-line comment above both functions to state

/* Must be called with interrupts enabled. */

With this and the commit message updated,

Reviewed-by: Laurent Pinchart 

>  static const struct vb2_ops uvc_queue_qops = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[jacopo mondi]

Hi Hans,

On Tue, Jan 16, 2018 at 10:46:42AM +0100, Hans Verkuil wrote:
> Hi Jacopo,
>
> Sorry for the late review, but here is finally is.
>
> BTW, can you provide the v4l2-compliance output (ideally with the -f option)
> in the cover letter for v6?

Sure, it was attacched to v3 I guess, since then it has not changed.
I will include that in v6 cover letter.

> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> >
> > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > (NV[12|21|16|61]).
> >
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> >
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> >
> > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> >  drivers/media/platform/Kconfig   |9 +
> >  drivers/media/platform/Makefile  |1 +
> >  drivers/media/platform/renesas-ceu.c | 1648 
> > ++
> >  3 files changed, 1658 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index fd0c998..fe7bd26 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
> >   To compile this driver as a module, choose M here: the module
> >   will be called stm32-dcmi.
> >
> > +config VIDEO_RENESAS_CEU
> > +   tristate "Renesas Capture Engine Unit (CEU) driver"
> > +   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> > +   depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> > +   select VIDEOBUF2_DMA_CONTIG
> > +   select V4L2_FWNODE
> > +   ---help---
> > + This is a v4l2 driver for the Renesas CEU Interface
> > +
> >  source "drivers/media/platform/soc_camera/Kconfig"
> >  source "drivers/media/platform/exynos4-is/Kconfig"
> >  source "drivers/media/platform/am437x/Kconfig"
> > diff --git a/drivers/media/platform/Makefile 
> > b/drivers/media/platform/Makefile
> > index 003b0bb..6580a6b 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o
> >  obj-$(CONFIG_SOC_CAMERA)   += soc_camera/
> >
> >  obj-$(CONFIG_VIDEO_RCAR_DRIF)  += rcar_drif.o
> > +obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o
> >  obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
> >  obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
> >  obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
> > diff --git a/drivers/media/platform/renesas-ceu.c 
> > b/drivers/media/platform/renesas-ceu.c
> > new file mode 100644
> > index 000..ccca838
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas-ceu.c
>
> 
>
> > +/*
> > + * ceu_vb2_setup() - is called to check whether the driver can accept the
> > + *  requested number of buffers and to fill in plane sizes
> > + *  for the current frame format, if required.
> > + */
> > +static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
> > +unsigned int *num_planes, unsigned int sizes[],
> > +struct device *alloc_devs[])
> > +{
> > +   struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> > +   struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> > +   unsigned int i;
> > +
> > +   if (!*count)
> > +   *count = 2;
>
> Don't do this. Instead set the min_buffers_needed field to 2 in the vb2_queue
> struct.
>

I guess setting min_buffers_needed makes this check not required. Will
drop.

> > +
> > +   /* num_planes is set: just check plane sizes. */
> > +   if (*num_planes) {
> > +   for (i = 0; i < pix->num_planes; i++)
> > +   if (sizes[i] < pix->plane_fmt[i].sizeimage)
> > +   return -EINVAL;
> > +
> > +   return 0;
> > +   }
> > +
> > +   /* num_planes not set: called from REQBUFS, just set plane sizes. */
> > +   *num_planes = pix->num_planes;
> > +   for (i = 0; i < pix->num_planes; i++)
> > +   sizes[i] = pix->plane_fmt[i].sizeimage;
> > +
> > +   return 0;
> > +}
> > +
> > +static void ceu_vb2_queue(struct vb2_buffer *vb)
> > +{
> > +   struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> > +   struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > +   struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> > +   struct ceu_buffer *buf = vb2_to_ceu(vbuf);
> > +   unsigned long irqflags;
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < pix->num_planes; i++) {
> > +   if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
> > +   vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> > +   return;
> > +   }
> > +
> > +   

Re: [PATCH 5/8] [media] omap3isp: support 64-bit version of omap3isp_stat_data

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 1:41 AM, Laurent Pinchart
 wrote:
> Hi Arnd,
>
> Thank you for the patch.
>
> I'll try to review this without too much delay. In the meantime, I'm CC'ing
> Sakari Ailus who might be faster than me :-)

Hi Laurent and Sakari,

I stumbled over this while cleaning out my backlog of patches. Any chance one
of you can have a look at this one? It's not needed for the 4.16 merge window,
but we do need it at some point and I would like to not see it again the next
time I clean out my old patches ;-)

  Arnd

Re: [PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Sakari Ailus]

On Tue, Jan 16, 2018 at 05:47:24PM +0100, Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function 
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> As Sylvester explains, we actually need to ensure that the
> value is within the range, so this reworks the loop to be
> easier to parse correctly, and an additional check to fall
> back on the first format value for any unexpected input.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Arnd Bergmann 

Thanks!

Acked-by: Sakari Ailus 

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

Re: [PATCH] [v2] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

On Tue, Jan 16, 2018 at 4:44 PM, Sakari Ailus
 wrote:

>>   if (camif_mbus_formats[i] == mf->code)
>>   break;
>>
>> + if (i == ARRAY_SIZE(camif_mbus_formats))
>> + mf->code = camif_mbus_formats[0];
>> +
>
> Either else here so that the line below is executed only if the condition
> is false, or assign i = 0 above. Otherwise you'll end up with a different
> off-by-one bug. :-)

Oops. Sent v3 now, thanks for the review.

  Arnd

[PATCH] [v3] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann 
---
v3: fix newly introduced off-by-one bug.
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..f51b92e94a32 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,16 +1256,19 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
-   mf->code = camif_mbus_formats[i];
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
+   else
+   mf->code = camif_mbus_formats[i];
 
if (pad == CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(>width, 8, CAMIF_MAX_PIX_WIDTH,
-- 
2.9.0

Re: [PATCH] [v2] media: s3c-camif: fix out-of-bounds array access

[Sakari Ailus]

Hi Arnd,

Thanks for the patch. Please see my comments below.

On Tue, Jan 16, 2018 at 04:30:46PM +0100, Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function 
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> As Sylvester explains, we actually need to ensure that the
> value is within the range, so this reworks the loop to be
> easier to parse correctly, and an additional check to fall
> back on the first format value for any unexpected input.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Arnd Bergmann 
> ---
> v2: rework logic rather than removing it.
> ---
>  drivers/media/platform/s3c-camif/camif-capture.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
> b/drivers/media/platform/s3c-camif/camif-capture.c
> index 437395a61065..002609be1400 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1256,15 +1256,18 @@ static void __camif_subdev_try_format(struct 
> camif_dev *camif,
>  {
>   const struct s3c_camif_variant *variant = camif->variant;
>   const struct vp_pix_limits *pix_lim;
> - int i = ARRAY_SIZE(camif_mbus_formats);
> + int i;
>  
>   /* FIXME: constraints against codec or preview path ? */
>   pix_lim = >vp_pix_limits[VP_CODEC];
>  
> - while (i-- >= 0)
> + for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)

Yeah, that loop was odd...

>   if (camif_mbus_formats[i] == mf->code)
>   break;
>  
> + if (i == ARRAY_SIZE(camif_mbus_formats))
> + mf->code = camif_mbus_formats[0];
> +

Either else here so that the line below is executed only if the condition
is false, or assign i = 0 above. Otherwise you'll end up with a different
off-by-one bug. :-)

i could be unsigned int, too. It'd be nicer that way actually.

>   mf->code = camif_mbus_formats[i];

-- 
Regards,

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

[PATCH] [v2] media: s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

While experimenting with older compiler versions, I ran
into a warning that no longer shows up on gcc-4.8 or newer:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'__camif_subdev_try_format':
drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
subscript is below array bounds

This is an off-by-one bug, leading to an access before the start of the
array, while newer compilers silently assume this undefined behavior
cannot happen and leave the loop at index 0 if no other entry matches.

As Sylvester explains, we actually need to ensure that the
value is within the range, so this reworks the loop to be
easier to parse correctly, and an additional check to fall
back on the first format value for any unexpected input.

I found an existing gcc bug for it and added a reduced version
of the function there.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
camera interface")
Signed-off-by: Arnd Bergmann 
---
v2: rework logic rather than removing it.
---
 drivers/media/platform/s3c-camif/camif-capture.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..002609be1400 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1256,15 +1256,18 @@ static void __camif_subdev_try_format(struct camif_dev 
*camif,
 {
const struct s3c_camif_variant *variant = camif->variant;
const struct vp_pix_limits *pix_lim;
-   int i = ARRAY_SIZE(camif_mbus_formats);
+   int i;
 
/* FIXME: constraints against codec or preview path ? */
pix_lim = >vp_pix_limits[VP_CODEC];
 
-   while (i-- >= 0)
+   for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
break;
 
+   if (i == ARRAY_SIZE(camif_mbus_formats))
+   mf->code = camif_mbus_formats[0];
+
mf->code = camif_mbus_formats[i];
 
if (pad == CAMIF_SD_PAD_SINK) {
-- 
2.9.0

Re: [PATCH] v4l: async: Protect against double notifier regstrations

[Sakari Ailus]

Hi Kieran,

On Tue, Jan 16, 2018 at 02:52:58PM +, Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> It can be easy to attempt to register the same notifier twice
> in mis-handled error cases such as working with -EPROBE_DEFER.
> 
> This results in odd kernel crashes where the notifier_list becomes
> corrupted due to adding the same entry twice.
> 
> Protect against this so that a developer has some sense of the pending
> failure.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 2b08d03b251d..e8476f0755ca 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -374,6 +374,7 @@ static int __v4l2_async_notifier_register(struct 
> v4l2_async_notifier *notifier)
>   struct device *dev =
>   notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
>   struct v4l2_async_subdev *asd;
> + struct v4l2_async_notifier *n;
>   int ret;
>   int i;
>  
> @@ -385,6 +386,19 @@ static int __v4l2_async_notifier_register(struct 
> v4l2_async_notifier *notifier)
>  
>   mutex_lock(_lock);
>  
> + /*
> +  * Registering the same notifier can occur if a driver incorrectly
> +  * handles a -EPROBE_DEFER for example, and will break in a
> +  * confusing fashion with linked-list corruption.
> +  */

This would seem fine in the commit message, and it's essentially there
already. How about simply:

/* Avoid re-registering a notifier. */

You should actually perform the check before initialising the notifier's
lists. Although things are likely in a quite bad shape already if this
happens.

> + list_for_each_entry(n, _list, list) {
> + if (n == notifier) {

if (WARN_ON(n == notifier)) {

And drop the error message below?

> + dev_err(dev, "Notifier has already been registered\n");
> + ret = -EEXIST;
> + goto err_unlock;
> + }
> + }
> +
>   for (i = 0; i < notifier->num_subdevs; i++) {
>   asd = notifier->subdevs[i];
>  

-- 
Regards,

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

Re: [RFT PATCH v3 0/6] Asynchronous UVC

[Kieran Bingham]

Hi Phillip

On 15/01/18 19:35, Philipp Zabel wrote:
> Hi Kieran,
> 
> On Fri, Jan 12, 2018 at 10:19 AM, Kieran Bingham
>  wrote:
>> This series has been tested on both the ZED and BRIO cameras on arm64
>> platforms, however due to the intrinsic changes in the driver I would like to
>> see it tested with other devices and other platforms, so I'd appreciate if
>> anyone can test this on a range of USB cameras.
> 
> FWIW,
> 
> Tested-by: Philipp Zabel 
> 
> with a Lite-On internal Laptop Webcam, Logitech C910 (USB2 isoc),
> Oculus Sensor (USB3 isoc), and Microsoft HoloLens Sensors (USB3 bulk).

Thank you, that is very much appreciated!

I presume that was on x86_64?

--
Regards

Kieran

[PATCH] v4l: async: Protect against double notifier regstrations

[Kieran Bingham]

From: Kieran Bingham 

It can be easy to attempt to register the same notifier twice
in mis-handled error cases such as working with -EPROBE_DEFER.

This results in odd kernel crashes where the notifier_list becomes
corrupted due to adding the same entry twice.

Protect against this so that a developer has some sense of the pending
failure.

Signed-off-by: Kieran Bingham 
---
 drivers/media/v4l2-core/v4l2-async.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 2b08d03b251d..e8476f0755ca 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -374,6 +374,7 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)
struct device *dev =
notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
struct v4l2_async_subdev *asd;
+   struct v4l2_async_notifier *n;
int ret;
int i;
 
@@ -385,6 +386,19 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)
 
mutex_lock(_lock);
 
+   /*
+* Registering the same notifier can occur if a driver incorrectly
+* handles a -EPROBE_DEFER for example, and will break in a
+* confusing fashion with linked-list corruption.
+*/
+   list_for_each_entry(n, _list, list) {
+   if (n == notifier) {
+   dev_err(dev, "Notifier has already been registered\n");
+   ret = -EEXIST;
+   goto err_unlock;
+   }
+   }
+
for (i = 0; i < notifier->num_subdevs; i++) {
asd = notifier->subdevs[i];
 
-- 
2.7.4

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[jacopo mondi]

Hi Hans,

On Tue, Jan 16, 2018 at 11:08:17AM +0100, Hans Verkuil wrote:
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Remove soc_camera framework dependencies from ov772x sensor driver.
> > - Handle clock and gpios
> > - Register async subdevice
> > - Remove soc_camera specific g/s_mbus_config operations
> > - Change image format colorspace from JPEG to SRGB as the two use the
> >   same colorspace information but JPEG makes assumptions on color
> >   components quantization that do not apply to the sensor
> > - Remove sizes crop from get_selection as driver can't scale
> > - Add kernel doc to driver interface header file
> > - Adjust build system
> >
> > This commit does not remove the original soc_camera based driver as long
> > as other platforms depends on soc_camera-based CEU driver.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 

[snip]

> >  static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 
> > height)
> > @@ -855,24 +910,21 @@ static int ov772x_get_selection(struct v4l2_subdev 
> > *sd,
> > struct v4l2_subdev_pad_config *cfg,
> > struct v4l2_subdev_selection *sel)
> >  {
> > +   struct ov772x_priv *priv = to_ov772x(sd);
> > +
> > if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> > return -EINVAL;
> >
> > -   sel->r.left = 0;
> > -   sel->r.top = 0;
>
> Why are these two lines removed?
>
> > switch (sel->target) {
> > case V4L2_SEL_TGT_CROP_BOUNDS:
> > case V4L2_SEL_TGT_CROP_DEFAULT:
> > -   sel->r.width = OV772X_MAX_WIDTH;
> > -   sel->r.height = OV772X_MAX_HEIGHT;
> > -   return 0;
> > case V4L2_SEL_TGT_CROP:
> > -   sel->r.width = VGA_WIDTH;
> > -   sel->r.height = VGA_HEIGHT;
> > -   return 0;
> > -   default:
> > -   return -EINVAL;
>
> Why is this default case removed?
>

Ooops. I have badly addressed your comment on v1.

Will fix in v6.

> > +   sel->r.width = priv->win->rect.width;
> > +   sel->r.height = priv->win->rect.height;
> > +   break;
> > }
> > +
> > +   return 0;
> >  }
> >
> >  static int ov772x_get_fmt(struct v4l2_subdev *sd,
> > @@ -997,24 +1049,8 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev 
> > *sd,
> > return 0;
> >  }
> >
> > -static int ov772x_g_mbus_config(struct v4l2_subdev *sd,
> > -   struct v4l2_mbus_config *cfg)
> > -{
> > -   struct i2c_client *client = v4l2_get_subdevdata(sd);
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -
> > -   cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> > -   V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > -   V4L2_MBUS_DATA_ACTIVE_HIGH;
> > -   cfg->type = V4L2_MBUS_PARALLEL;
> > -   cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> > -
> > -   return 0;
> > -}
> > -
> >  static const struct v4l2_subdev_video_ops ov772x_subdev_video_ops = {
> > .s_stream   = ov772x_s_stream,
> > -   .g_mbus_config  = ov772x_g_mbus_config,
> >  };
> >
> >  static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = {
> > @@ -1038,12 +1074,11 @@ static int ov772x_probe(struct i2c_client *client,
> > const struct i2c_device_id *did)
> >  {
> > struct ov772x_priv  *priv;
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -   struct i2c_adapter  *adapter = to_i2c_adapter(client->dev.parent);
> > +   struct i2c_adapter  *adapter = client->adapter;
> > int ret;
> >
> > -   if (!ssdd || !ssdd->drv_priv) {
> > -   dev_err(>dev, "OV772X: missing platform data!\n");
> > +   if (!client->dev.platform_data) {
> > +   dev_err(>dev, "Missing OV7725 platform data\n");
>
> Nitpick: I'd prefer lowercase in this string: ov7725. It also should be
> ov772x.
>
> > return -EINVAL;
> > }
> >
> > @@ -1059,7 +1094,7 @@ static int ov772x_probe(struct i2c_client *client,
> > if (!priv)
> > return -ENOMEM;
> >
> > -   priv->info = ssdd->drv_priv;
> > +   priv->info = client->dev.platform_data;
> >
> > v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
> > v4l2_ctrl_handler_init(>hdl, 3);
> > @@ -1073,22 +1108,42 @@ static int ov772x_probe(struct i2c_client *client,
> > if (priv->hdl.error)
> > return priv->hdl.error;
> >
> > -   priv->clk = v4l2_clk_get(>dev, "mclk");
> > +   priv->clk = clk_get(>dev, "xclk");
> > if (IS_ERR(priv->clk)) {
> > +   dev_err(>dev, "Unable to get xclk clock\n");
> > ret = PTR_ERR(priv->clk);
> > -   goto eclkget;
> > +   goto error_ctrl_free;
> > }
> >
> > -   ret = ov772x_video_probe(priv);
> > -   if (ret < 0) {
> > -   v4l2_clk_put(priv->clk);
> > -eclkget:
> > -   v4l2_ctrl_handler_free(>hdl);
> > -   } else {
> > -   priv->cfmt = _cfmts[0];

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[Laurent Pinchart]

Hi Hans,

On Tuesday, 16 January 2018 11:46:42 EET Hans Verkuil wrote:
> Hi Jacopo,
> 
> Sorry for the late review, but here is finally is.
> 
> BTW, can you provide the v4l2-compliance output (ideally with the -f option)
> in the cover letter for v6?
> 
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> > 
> > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > (NV[12|21|16|61]).
> > 
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> > 
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> > 
> > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> > 
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> > 
> >  drivers/media/platform/Kconfig   |9 +
> >  drivers/media/platform/Makefile  |1 +
> >  drivers/media/platform/renesas-ceu.c | 1648 +
> >  3 files changed, 1658 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c

[snip]

> > diff --git a/drivers/media/platform/renesas-ceu.c
> > b/drivers/media/platform/renesas-ceu.c new file mode 100644
> > index 000..ccca838
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas-ceu.c

[snip]

> > +static int ceu_s_input(struct file *file, void *priv, unsigned int i)
> > +{
> > +   struct ceu_device *ceudev = video_drvdata(file);
> > +   struct ceu_subdev *ceu_sd_old;
> > +   int ret;
> > +
> 
> Add a check:
> 
>   if (i == ceudev->sd_index)
>   return 0;
> 
> I.e. if the new input == the old input, then that's fine regardless of the
> streaming state.

On a side note this is the kind of checks that the core should handle, but 
that's out of scope for this patch.

> > +   if (vb2_is_streaming(>vb2_vq))
> > +   return -EBUSY;
> > +
> > +   if (i >= ceudev->num_sd)
> > +   return -EINVAL;
> 
> Move this up as the first test.
> 
> > +
> > +   ceu_sd_old = ceudev->sd;
> > +   ceudev->sd = >subdevs[i];
> > +
> > +   /* Make sure we can generate output image formats. */
> > +   ret = ceu_init_formats(ceudev);
> > +   if (ret) {
> > +   ceudev->sd = ceu_sd_old;
> > +   return -EINVAL;
> > +   }
> > +
> > +   /* now that we're sure we can use the sensor, power off the old one. */
> > +   v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0);
> > +   v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1);
> > +
> > +   ceudev->sd_index = i;
> > +
> > +   return 0;
> > +}

[snip]

> > +
> > +static int ceu_notify_complete(struct v4l2_async_notifier *notifier)
> > +{
> > +   struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> > +   struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> > +   struct video_device *vdev = >vdev;
> > +   struct vb2_queue *q = >vb2_vq;
> > +   struct v4l2_subdev *v4l2_sd;
> > +   int ret;
> > +
> > +   /* Initialize vb2 queue. */
> > +   q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +   q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> 
> Don't include VB2_USERPTR. It shouldn't be used with dma_contig.
> 
> You also added a read() fop (vb2_fop_read), so either add VB2_READ here
> or remove the read fop.

Agreed. I'd drop both VB2_USERPTR and vb2_fop_read().

> > +   q->drv_priv = ceudev;
> > +   q->ops  = _vb2_ops;
> > +   q->mem_ops  = _dma_contig_memops;
> > +   q->buf_struct_size  = sizeof(struct ceu_buffer);
> > +   q->timestamp_flags  = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > +   q->lock = >mlock;
> > +   q->dev  = ceudev->v4l2_dev.dev;
> > +
> > +   ret = vb2_queue_init(q);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /*
> > +* Make sure at least one sensor is primary and use it to initialize
> > +* ceu formats.
> > +*/
> > +   if (!ceudev->sd) {
> > +   ceudev->sd = >subdevs[0];
> > +   ceudev->sd_index = 0;
> > +   }
> > +
> > +   v4l2_sd = ceudev->sd->v4l2_sd;
> > +
> > +   ret = ceu_init_formats(ceudev);
> > +   if (ret)
> > +   return ret;
> > +
> > +   ret = ceu_set_default_fmt(ceudev);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /* Register the video device. */
> > +   strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
> > +   vdev->v4l2_dev  = v4l2_dev;
> > +   vdev->lock  = >mlock;
> > +   vdev->queue = >vb2_vq;
> > +   vdev->ctrl_handler  = v4l2_sd->ctrl_handler;
> > +   vdev->fops  = _fops;
> > +   vdev->ioctl_ops = _ioctl_ops;
> > +   vdev->release   = ceu_vdev_release;
> > +   vdev->device_caps   = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> > + V4L2_CAP_STREAMING;
> > +   video_set_drvdata(vdev, ceudev);
> > +
> > +   ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> > +   if (ret < 0) {
> > +   

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Laurent Pinchart]

Hi Hans,

On Tuesday, 16 January 2018 12:08:17 EET Hans Verkuil wrote:
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Remove soc_camera framework dependencies from ov772x sensor driver.
> > - Handle clock and gpios
> > - Register async subdevice
> > - Remove soc_camera specific g/s_mbus_config operations
> > - Change image format colorspace from JPEG to SRGB as the two use the
> > 
> >   same colorspace information but JPEG makes assumptions on color
> >   components quantization that do not apply to the sensor
> > 
> > - Remove sizes crop from get_selection as driver can't scale
> > - Add kernel doc to driver interface header file
> > - Adjust build system
> > 
> > This commit does not remove the original soc_camera based driver as long
> > as other platforms depends on soc_camera-based CEU driver.
> > 
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> > 
> >  drivers/media/i2c/Kconfig  |  11 +++
> >  drivers/media/i2c/Makefile |   1 +
> >  drivers/media/i2c/ov772x.c | 177 
> >  include/media/i2c/ov772x.h |   6 +-
> >  4 files changed, 133 insertions(+), 62 deletions(-)

[snip]

> > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> > index 8063835..df2516c 100644
> > --- a/drivers/media/i2c/ov772x.c
> > +++ b/drivers/media/i2c/ov772x.c

[snip]

> > @@ -1038,12 +1074,11 @@ static int ov772x_probe(struct i2c_client *client,
> > const struct i2c_device_id *did)
> >  {
> > struct ov772x_priv  *priv;
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -   struct i2c_adapter  *adapter = to_i2c_adapter(client->dev.parent);
> > +   struct i2c_adapter  *adapter = client->adapter;
> > int ret;
> > 
> > -   if (!ssdd || !ssdd->drv_priv) {
> > -   dev_err(>dev, "OV772X: missing platform data!\n");
> > +   if (!client->dev.platform_data) {
> > +   dev_err(>dev, "Missing OV7725 platform data\n");
> 
> Nitpick: I'd prefer lowercase in this string: ov7725. It also should be
> ov772x.

Agreed.

> > return -EINVAL;
> > 
> > }

[snip]

> > @@ -1119,6 +1176,6 @@ static struct i2c_driver ov772x_i2c_driver = {
> > 
> >  module_i2c_driver(ov772x_i2c_driver);
> > 
> > -MODULE_DESCRIPTION("SoC Camera driver for ov772x");
> > +MODULE_DESCRIPTION("V4L2 driver for OV772x image sensor");
> 
> Ditto: lower case ov772x.

I'd keep that uppercase. The usual practice (unless I'm mistaken) is to use 
uppercase for chip names and lowercase for driver names. The description 
clearly refers to the chip, so uppercase seems better to me.

> >  MODULE_AUTHOR("Kuninori Morimoto");
> >  MODULE_LICENSE("GPL v2");
> 
> Hmm, shouldn't there be a struct of_device_id as well? So this can be
> used in the device tree?
> 
> I see this sensor was only tested with a non-dt platform. Is it possible
> to test this sensor with the GR-Peach platform (which I gather uses the
> device tree)?
> 
> Making this driver DT compliant can be done as a follow-up patch.

I think it's a good idea, but I'd prefer having that in a separate patch. We 
will also need DT bindings, it's a bit out of scope for this series.

Jacopo, you can keep my ack after addressing Hans' comments.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] imx074/mt9t031: deprecate soc_camera sensors

[Sakari Ailus]

On Tue, Jan 16, 2018 at 12:02:43PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> These two sensor drivers in drivers/media/i2c/soc_camera are not
> used anymore. Move them to staging in preparation of being deleted
> once the soc_camera framework is removed. Unless someone steps up
> to do the conversion to a proper V4L2 subdev driver.

For both:

Acked-by: Sakari Ailus 

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

[PATCH 2/2] mt9t031: deprecate, move to staging

[Hans Verkuil]

From: Hans Verkuil 

This driver is unused and depends on the deprecated soc-camera framework.
Move it to staging in preparation for being removed unless someone does
the work to convert it to a proper V4L2 subdev driver.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/soc_camera/Kconfig  |  6 --
 drivers/media/i2c/soc_camera/Makefile |  1 -
 drivers/staging/media/Kconfig |  2 ++
 drivers/staging/media/Makefile|  1 +
 drivers/staging/media/mt9t031/Kconfig | 11 +++
 drivers/staging/media/mt9t031/Makefile|  1 +
 drivers/staging/media/mt9t031/TODO|  5 +
 .../{media/i2c/soc_camera => staging/media/mt9t031}/mt9t031.c |  0
 8 files changed, 20 insertions(+), 7 deletions(-)
 create mode 100644 drivers/staging/media/mt9t031/Kconfig
 create mode 100644 drivers/staging/media/mt9t031/Makefile
 create mode 100644 drivers/staging/media/mt9t031/TODO
 rename drivers/{media/i2c/soc_camera => staging/media/mt9t031}/mt9t031.c (100%)

diff --git a/drivers/media/i2c/soc_camera/Kconfig 
b/drivers/media/i2c/soc_camera/Kconfig
index d7136f2c2b20..7c2aabc8a3f6 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -17,12 +17,6 @@ config SOC_CAMERA_MT9M111
  This is the legacy configuration which shouldn't be used anymore,
  while VIDEO_MT9M111 should be used instead.
 
-config SOC_CAMERA_MT9T031
-   tristate "mt9t031 support"
-   depends on SOC_CAMERA && I2C
-   help
- This driver supports MT9T031 cameras from Micron.
-
 config SOC_CAMERA_MT9T112
tristate "mt9t112 support"
depends on SOC_CAMERA && I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile 
b/drivers/media/i2c/soc_camera/Makefile
index a489b00e43b5..8c7770f62997 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_SOC_CAMERA_MT9M001)   += mt9m001.o
-obj-$(CONFIG_SOC_CAMERA_MT9T031)   += mt9t031.o
 obj-$(CONFIG_SOC_CAMERA_MT9T112)   += mt9t112.o
 obj-$(CONFIG_SOC_CAMERA_MT9V022)   += mt9v022.o
 obj-$(CONFIG_SOC_CAMERA_OV5642)+= ov5642.o
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index 9afdb2e279cc..f99287e58402 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -31,6 +31,8 @@ source "drivers/staging/media/imx/Kconfig"
 
 source "drivers/staging/media/imx074/Kconfig"
 
+source "drivers/staging/media/mt9t031/Kconfig"
+
 source "drivers/staging/media/omap4iss/Kconfig"
 
 source "drivers/staging/media/tegra-vde/Kconfig"
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index 9958466524ed..a98efd52a185 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_I2C_BCM2048)   += bcm2048/
 obj-$(CONFIG_DVB_CXD2099)  += cxd2099/
 obj-$(CONFIG_VIDEO_IMX_MEDIA)  += imx/
 obj-$(CONFIG_SOC_CAMERA_IMX074)+= imx074/
+obj-$(CONFIG_SOC_CAMERA_MT9T031)   += mt9t031/
 obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/
 obj-$(CONFIG_VIDEO_OMAP4)  += omap4iss/
 obj-$(CONFIG_INTEL_ATOMISP) += atomisp/
diff --git a/drivers/staging/media/mt9t031/Kconfig 
b/drivers/staging/media/mt9t031/Kconfig
new file mode 100644
index ..f48e06a03cdb
--- /dev/null
+++ b/drivers/staging/media/mt9t031/Kconfig
@@ -0,0 +1,11 @@
+config SOC_CAMERA_IMX074
+   tristate "imx074 support (DEPRECATED)"
+   depends on SOC_CAMERA && I2C
+   help
+ This driver supports IMX074 cameras from Sony
+
+config SOC_CAMERA_MT9T031
+   tristate "mt9t031 support (DEPRECATED)"
+   depends on SOC_CAMERA && I2C
+   help
+ This driver supports MT9T031 cameras from Micron.
diff --git a/drivers/staging/media/mt9t031/Makefile 
b/drivers/staging/media/mt9t031/Makefile
new file mode 100644
index ..bfd24c442b33
--- /dev/null
+++ b/drivers/staging/media/mt9t031/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SOC_CAMERA_MT9T031)   += mt9t031.o
diff --git a/drivers/staging/media/mt9t031/TODO 
b/drivers/staging/media/mt9t031/TODO
new file mode 100644
index ..15580a4f950c
--- /dev/null
+++ b/drivers/staging/media/mt9t031/TODO
@@ -0,0 +1,5 @@
+This sensor driver needs to be converted to a regular
+v4l2 subdev driver. The soc_camera framework is deprecated and
+will be removed in the future. Unless someone does this work this
+sensor driver will be deleted when the soc_camera framework is
+deleted.
diff --git a/drivers/media/i2c/soc_camera/mt9t031.c 
b/drivers/staging/media/mt9t031/mt9t031.c
similarity index 100%
rename from drivers/media/i2c/soc_camera/mt9t031.c
rename to drivers/staging/media/mt9t031/mt9t031.c
-- 
2.15.1

[PATCH 1/2] imx074: deprecate, move to staging

[Hans Verkuil]

From: Hans Verkuil 

This driver is unused and depends on the deprecated soc-camera framework.
Move it to staging in preparation for being removed unless someone does
the work to convert it to a proper V4L2 subdev driver.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/soc_camera/Kconfig| 6 --
 drivers/media/i2c/soc_camera/Makefile   | 1 -
 drivers/staging/media/Kconfig   | 2 ++
 drivers/staging/media/Makefile  | 1 +
 drivers/staging/media/imx074/Kconfig| 5 +
 drivers/staging/media/imx074/Makefile   | 1 +
 drivers/staging/media/imx074/TODO   | 5 +
 drivers/{media/i2c/soc_camera => staging/media/imx074}/imx074.c | 0
 8 files changed, 14 insertions(+), 7 deletions(-)
 create mode 100644 drivers/staging/media/imx074/Kconfig
 create mode 100644 drivers/staging/media/imx074/Makefile
 create mode 100644 drivers/staging/media/imx074/TODO
 rename drivers/{media/i2c/soc_camera => staging/media/imx074}/imx074.c (100%)

diff --git a/drivers/media/i2c/soc_camera/Kconfig 
b/drivers/media/i2c/soc_camera/Kconfig
index 72b369895b37..d7136f2c2b20 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -1,11 +1,5 @@
 comment "soc_camera sensor drivers"
 
-config SOC_CAMERA_IMX074
-   tristate "imx074 support"
-   depends on SOC_CAMERA && I2C
-   help
- This driver supports IMX074 cameras from Sony
-
 config SOC_CAMERA_MT9M001
tristate "mt9m001 support"
depends on SOC_CAMERA && I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile 
b/drivers/media/i2c/soc_camera/Makefile
index faa2df8901d2..a489b00e43b5 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -1,5 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_SOC_CAMERA_IMX074)+= imx074.o
 obj-$(CONFIG_SOC_CAMERA_MT9M001)   += mt9m001.o
 obj-$(CONFIG_SOC_CAMERA_MT9T031)   += mt9t031.o
 obj-$(CONFIG_SOC_CAMERA_MT9T112)   += mt9t112.o
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index e68e1d343d53..9afdb2e279cc 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -29,6 +29,8 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
 
 source "drivers/staging/media/imx/Kconfig"
 
+source "drivers/staging/media/imx074/Kconfig"
+
 source "drivers/staging/media/omap4iss/Kconfig"
 
 source "drivers/staging/media/tegra-vde/Kconfig"
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index 59a47f69884f..9958466524ed 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_I2C_BCM2048)  += bcm2048/
 obj-$(CONFIG_DVB_CXD2099)  += cxd2099/
 obj-$(CONFIG_VIDEO_IMX_MEDIA)  += imx/
+obj-$(CONFIG_SOC_CAMERA_IMX074)+= imx074/
 obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/
 obj-$(CONFIG_VIDEO_OMAP4)  += omap4iss/
 obj-$(CONFIG_INTEL_ATOMISP) += atomisp/
diff --git a/drivers/staging/media/imx074/Kconfig 
b/drivers/staging/media/imx074/Kconfig
new file mode 100644
index ..229cbeea580b
--- /dev/null
+++ b/drivers/staging/media/imx074/Kconfig
@@ -0,0 +1,5 @@
+config SOC_CAMERA_IMX074
+   tristate "imx074 support (DEPRECATED)"
+   depends on SOC_CAMERA && I2C
+   help
+ This driver supports IMX074 cameras from Sony
diff --git a/drivers/staging/media/imx074/Makefile 
b/drivers/staging/media/imx074/Makefile
new file mode 100644
index ..7d183574aa84
--- /dev/null
+++ b/drivers/staging/media/imx074/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SOC_CAMERA_IMX074)+= imx074.o
diff --git a/drivers/staging/media/imx074/TODO 
b/drivers/staging/media/imx074/TODO
new file mode 100644
index ..15580a4f950c
--- /dev/null
+++ b/drivers/staging/media/imx074/TODO
@@ -0,0 +1,5 @@
+This sensor driver needs to be converted to a regular
+v4l2 subdev driver. The soc_camera framework is deprecated and
+will be removed in the future. Unless someone does this work this
+sensor driver will be deleted when the soc_camera framework is
+deleted.
diff --git a/drivers/media/i2c/soc_camera/imx074.c 
b/drivers/staging/media/imx074/imx074.c
similarity index 100%
rename from drivers/media/i2c/soc_camera/imx074.c
rename to drivers/staging/media/imx074/imx074.c
-- 
2.15.1

[PATCH 0/2] imx074/mt9t031: deprecate soc_camera sensors

[Hans Verkuil]

From: Hans Verkuil 

These two sensor drivers in drivers/media/i2c/soc_camera are not
used anymore. Move them to staging in preparation of being deleted
once the soc_camera framework is removed. Unless someone steps up
to do the conversion to a proper V4L2 subdev driver.

Regards,

Hans

Hans Verkuil (2):
  imx074: deprecate, move to staging
  mt9t031: deprecate, move to staging

 drivers/media/i2c/soc_camera/Kconfig | 12 
 drivers/media/i2c/soc_camera/Makefile|  2 --
 drivers/staging/media/Kconfig|  4 
 drivers/staging/media/Makefile   |  2 ++
 drivers/staging/media/imx074/Kconfig |  5 +
 drivers/staging/media/imx074/Makefile|  1 +
 drivers/staging/media/imx074/TODO|  5 +
 .../{media/i2c/soc_camera => staging/media/imx074}/imx074.c  |  0
 drivers/staging/media/mt9t031/Kconfig| 11 +++
 drivers/staging/media/mt9t031/Makefile   |  1 +
 drivers/staging/media/mt9t031/TODO   |  5 +
 .../i2c/soc_camera => staging/media/mt9t031}/mt9t031.c   |  0
 12 files changed, 34 insertions(+), 14 deletions(-)
 create mode 100644 drivers/staging/media/imx074/Kconfig
 create mode 100644 drivers/staging/media/imx074/Makefile
 create mode 100644 drivers/staging/media/imx074/TODO
 rename drivers/{media/i2c/soc_camera => staging/media/imx074}/imx074.c (100%)
 create mode 100644 drivers/staging/media/mt9t031/Kconfig
 create mode 100644 drivers/staging/media/mt9t031/Makefile
 create mode 100644 drivers/staging/media/mt9t031/TODO
 rename drivers/{media/i2c/soc_camera => staging/media/mt9t031}/mt9t031.c (100%)

-- 
2.15.1

Re: [RFC PATCH 5/9] media: vb2: add support for requests

[Hans Verkuil]

On 01/16/2018 10:39 AM, Alexandre Courbot wrote:
> On Mon, Jan 15, 2018 at 6:07 PM, Hans Verkuil  wrote:
>> On 01/15/2018 09:24 AM, Alexandre Courbot wrote:
>>> On Fri, Jan 12, 2018 at 7:49 PM, Hans Verkuil  wrote:
 On 12/15/17 08:56, Alexandre Courbot wrote:
> Add throttling support for buffers when requests are in use on a given
> queue. Buffers associated to a request are kept into the vb2 queue until
> the request becomes active, at which point all the buffers are passed to
> the driver. The queue can also signal that is has processed all of a
> request's buffers.
>
> Also add support for the request parameter when handling the QBUF ioctl.
>
> Signed-off-by: Alexandre Courbot 
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 59 
> 
>  drivers/media/v4l2-core/videobuf2-v4l2.c | 29 +++-
>  include/media/videobuf2-core.h   | 25 +-
>  3 files changed, 104 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index cb115ba6a1d2..c01038b7962a 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -898,6 +898,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
> vb2_buffer_state state)
>   state != VB2_BUF_STATE_REQUEUEING))
>   state = VB2_BUF_STATE_ERROR;
>
> + WARN_ON(vb->request != q->cur_req);

 What's the reason for this WARN_ON? It's not immediately obvious to me.
>>>
>>> This is a safeguard against driver bugs: a buffer should not complete
>>> unless it is part of the request being currently processed.
>>>

> +
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>   /*
>* Although this is not a callback, it still does have to balance
> @@ -920,6 +922,13 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
> vb2_buffer_state state)
>   /* Add the buffer to the done buffers list */
>   list_add_tail(>done_entry, >done_list);
>   vb->state = state;
> +
> + if (q->cur_req) {
> + WARN_ON(q->req_buf_cnt < 1);
> +
> + if (--q->req_buf_cnt == 0)
> + q->cur_req = NULL;
> + }
>   }
>   atomic_dec(>owned_by_drv_count);
>   spin_unlock_irqrestore(>done_lock, flags);
> @@ -1298,6 +1307,16 @@ int vb2_core_prepare_buf(struct vb2_queue *q, 
> unsigned int index, void *pb)
>  }
>  EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
>
> +static void vb2_queue_enqueue_current_buffers(struct vb2_queue *q)
> +{
> + struct vb2_buffer *vb;
> +
> + list_for_each_entry(vb, >queued_list, queued_entry) {
> + if (vb->request == q->cur_req)
> + __enqueue_in_driver(vb);
> + }
> +}

 I think this will clash big time with the v4l2 fence patch series...
>>>
>>> Indeed, but on the other hand I was not a big fan of going through the
>>> whole list. :) So I welcome the extra throttling introduced by the
>>> fence series.
>>
>> There is only throttling if fences are used by userspace. Otherwise there
>> is no change.
>>
>>>

> +
>  /**
>   * vb2_start_streaming() - Attempt to start streaming.
>   * @q:   videobuf2 queue
> @@ -1318,8 +1337,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>* If any buffers were queued before streamon,
>* we can now pass them to driver for processing.
>*/
> - list_for_each_entry(vb, >queued_list, queued_entry)
> - __enqueue_in_driver(vb);
> + vb2_queue_enqueue_current_buffers(q);
>
>   /* Tell the driver to start streaming */
>   q->start_streaming_called = 1;
> @@ -1361,7 +1379,8 @@ static int vb2_start_streaming(struct vb2_queue *q)
>   return ret;
>  }
>
> -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
> +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index,
> +   struct media_request *req, void *pb)
>  {
>   struct vb2_buffer *vb;
>   int ret;
> @@ -1392,6 +1411,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
> index, void *pb)
>   q->queued_count++;
>   q->waiting_for_buffers = false;
>   vb->state = VB2_BUF_STATE_QUEUED;
> + vb->request = req;
>
>   if (pb)
>   call_void_bufop(q, copy_timestamp, vb, pb);
> @@ -1401,8 +1421,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned 
> int index, void *pb)
>   /*
>* If already streaming, give the buffer to driver 

Pomoc dla Zosi

[Karina i Jakub Bieckowie]

Szanowni Państwo,

Nazywamy się Karina i Jakub Bieckowie. Jesteśmy młodym małżeństwem oraz 
rodzicami cudownych bliźniaczek – Zuzi i Zosi. Dziewczynki w grudniu skończyły 
roczek. To niewątpliwie piękny czas, choć w naszym przypadku 
zakłócił go wielki niepokój.

O ile Zuzia jest zdrowym szkrabem, o tyle Zosia urodziła się z bardzo poważną 
wadą - bilateral fibular hemimelia. Oznacza to, że jej organizm nie wykształcił 
kości strzałkowych, a co za tym idzie, jej nóżki są 
kompletnie zdeformowane. Brak leczenia nieodwołalnie skazuje Zosię na wózek 
inwalidzki. Niestety, próżno szukać dla niej szansy w Polsce – ani w ramach 
państwowej opieki zdrowotnej, ani prywatnie. Dość powiedzieć, 
że jeszcze kilka lat temu procedura postępowania wobec dzieci z taką wadą 
zakładała amputację kończyn. 

Zjeździliśmy dziesiątki szpitali, rozmawialiśmy z setkami lekarzy. Wniosek jest 
następujący - tylko jeden szpital na świecie jest w stanie dać Zosi gwarancję, 
że stanie na nogi, a nawet, że będzie chodziła. To 
Paley Institute w Stanach Zjednoczonych, klinika założona przez światowej sławy 
ortopedę dziecięcego, doktora Drora Paleya. Będąc w Polsce, doktor Paley badał 
Zosię. Uznał jej wadę za arcyrzadką, stwierdził nawet, 
że prawdopodobnie nigdy dotąd nie została ona opisana w literaturze 
specjalistycznej. Jednocześnie wie, jak przywrócić zdrowie Zosi. Nakreślił już, 
etap po etapie, jak przebiegałaby operacja dziewczynki.

A zatem - jest nadzieja! Równocześnie, są też bariery. Bardzo poważne bariery. 
Pierwsza to czas - operacja ma sens tylko do 24 miesiąca życia dziecka, a Zosia 
dwa latka skończy w grudniu 2018 roku. Drugą barierą są 
pieniądze - ogromna kwota 600 tysięcy złotych. Wszystko, co mamy to wciąż za 
mało, żeby pokryć te koszty. Nawet z pomocą bliskich daleko nam od realizacji 
celu.

Jesteśmy zwykłą rodziną z Częstochowy, taką jak dziesiątki innych, czyli żyjącą 
raczej skromnie, ale dającą sobie radę w polskich realiach... o ile nie 
spotkają jej poważne kłopoty zdrowotne. Trafiło na naszą 
Zosię. Dlatego pragniemy poprosić Państwa o pomoc. Każda złotówka daje szansę 
Zosi na normalne życie. Za każdą będziemy dozgonnie wdzięczni. Adres, pod 
którym można dokonywać wpłat znajdą Państwo tutaj: 


https://dzieciom.pl/podopieczni/31298 


Wierzymy w ludzi, wierzymy w empatię i bezinteresowną pomoc. I nie wyobrażamy 
sobie, że moglibyśmy zaprzepaścić tę daną nam szansę – jakkolwiek jest ona 
niełatwa do realizacji. Dlatego jeszcze raz prosimy Państwa o 
wsparcie -  wspólnymi siłami zdołamy postawić Zosię na nogi!

Z poważaniem
Karina i Jakub Bieckowie - rodzice Zosi i Zuzi

Re: [PATCH 1/3] dma-buf: make returning the exclusive fence optional

[Christian König]

Ping? Daniel you requested the patch with its user.

Would be nice when I can commit this cause we need it for debugging and 
cleaning up a bunch of other things as well.


Regards,
Christian.

Am 12.01.2018 um 10:47 schrieb Christian König:

Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.

If not specified the exclusive fence is put into the fence array as
well.

This is helpful for a couple of cases where we need all fences in a
single array.

Signed-off-by: Christian König 
---
  drivers/dma-buf/reservation.c | 31 ++-
  1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
   * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
   * the required size, and must be freed by caller)
   *
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
   */
  int reservation_object_get_fences_rcu(struct reservation_object *obj,
  struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
  
  	do {

struct reservation_object_list *fobj;
-   unsigned seq;
-   unsigned int i;
+   unsigned int i, seq;
+   size_t sz = 0;
  
  		shared_count = i = 0;
  
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,

goto unlock;
  
  		fobj = rcu_dereference(obj->fence);

-   if (fobj) {
+   if (fobj)
+   sz += sizeof(*shared) * fobj->shared_max;
+
+   if (!pfence_excl && fence_excl)
+   sz += sizeof(*shared);
+
+   if (sz) {
struct dma_fence **nshared;
-   size_t sz = sizeof(*shared) * fobj->shared_max;
  
  			nshared = krealloc(shared, sz,

   GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
break;
}
shared = nshared;
-   shared_count = fobj->shared_count;
-
+   shared_count = fobj ? fobj->shared_count : 0;
for (i = 0; i < shared_count; ++i) {
shared[i] = rcu_dereference(fobj->shared[i]);
if (!dma_fence_get_rcu(shared[i]))
break;
}
+
+   if (!pfence_excl && fence_excl) {
+   shared[i] = fence_excl;
+   fence_excl = NULL;
+   ++i;
+   ++shared_count;
+   }
}
  
  		if (i != shared_count || read_seqcount_retry(>seq, seq)) {

@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
  
  	*pshared_count = shared_count;

*pshared = shared;
-   *pfence_excl = fence_excl;
+   if (pfence_excl)
+   *pfence_excl = fence_excl;
  
  	return ret;

  }

Re: [PATCH v5 9/9] arch: sh: migor: Use new renesas-ceu camera driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Migo-R platform uses sh_mobile_ceu camera driver, which is now being
> replaced by a proper V4L2 camera driver named 'renesas-ceu'.
> 
> Move Migo-R platform to use the v4l2 renesas-ceu camera driver
> interface and get rid of soc_camera defined components used to register
> sensor drivers and of platform specific enable/disable routines.
> 
> Register clock source and GPIOs for sensor drivers, so they can use
> clock and gpio APIs.
> 
> Also, memory for CEU video buffers is now reserved with membocks APIs,
> and need to be declared as dma_coherent during machine initialization to
> remove that architecture specific part from CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 8/9] media: i2c: tw9910: Remove soc_camera dependencies

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from tw9910 sensor driver.
> - Handle clock and gpios
> - Register async subdevice
> - Remove soc_camera specific g/s_mbus_config operations
> - Add kernel doc to driver interface header file
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver as long
> as other platforms depends on soc_camera-based CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from ov772x sensor driver.
> - Handle clock and gpios
> - Register async subdevice
> - Remove soc_camera specific g/s_mbus_config operations
> - Change image format colorspace from JPEG to SRGB as the two use the
>   same colorspace information but JPEG makes assumptions on color
>   components quantization that do not apply to the sensor
> - Remove sizes crop from get_selection as driver can't scale
> - Add kernel doc to driver interface header file
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver as long
> as other platforms depends on soc_camera-based CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/i2c/Kconfig  |  11 +++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ov772x.c | 177 
> ++---
>  include/media/i2c/ov772x.h |   6 +-
>  4 files changed, 133 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cb5d7ff..a61d7f4 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -645,6 +645,17 @@ config VIDEO_OV5670
> To compile this driver as a module, choose M here: the
> module will be called ov5670.
>  
> +config VIDEO_OV772X
> + tristate "OmniVision OV772x sensor support"
> + depends on I2C && VIDEO_V4L2
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV772x camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov772x.
> +
>  config VIDEO_OV7640
>   tristate "OmniVision OV7640 sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 548a9ef..fb99293 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
>  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 8063835..df2516c 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -1,6 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * ov772x Camera Driver
>   *
> + * Copyright (C) 2017 Jacopo Mondi 
> + *
>   * Copyright (C) 2008 Renesas Solutions Corp.
>   * Kuninori Morimoto 
>   *
> @@ -9,27 +12,25 @@
>   * Copyright 2006-7 Jonathan Corbet 
>   * Copyright (C) 2008 Magnus Damm
>   * Copyright (C) 2008, Guennadi Liakhovetski 
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
>   */
>  
> +#include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
>  #include 
> -#include 
> -#include 
> +
>  #include 
> -#include 
> +#include 
>  #include 
> +#include 
>  
>  /*
>   * register offset
> @@ -393,8 +394,10 @@ struct ov772x_win_size {
>  struct ov772x_priv {
>   struct v4l2_subdevsubdev;
>   struct v4l2_ctrl_handler  hdl;
> - struct v4l2_clk  *clk;
> + struct clk   *clk;
>   struct ov772x_camera_info*info;
> + struct gpio_desc *pwdn_gpio;
> + struct gpio_desc *rstb_gpio;
>   const struct ov772x_color_format *cfmt;
>   const struct ov772x_win_size *win;
>   unsigned shortflag_vflip:1;
> @@ -409,7 +412,7 @@ struct ov772x_priv {
>  static const struct ov772x_color_format ov772x_cfmts[] = {
>   {
>   .code   = MEDIA_BUS_FMT_YUYV8_2X8,
> - .colorspace = V4L2_COLORSPACE_JPEG,
> + .colorspace = V4L2_COLORSPACE_SRGB,
>   .dsp3   = 0x0,
>   .dsp4   = DSP_OFMT_YUV,
>   .com3   = SWAP_YUV,
> @@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
>   },
>   {
>   .code   = MEDIA_BUS_FMT_YVYU8_2X8,
> - .colorspace = V4L2_COLORSPACE_JPEG,
> + .colorspace = V4L2_COLORSPACE_SRGB,
>   .dsp3   = UV_ON,
>   .dsp4   = DSP_OFMT_YUV,
>   .com3   = SWAP_YUV,
> @@ -425,7 

Re: [PATCH v5 1/9] dt-bindings: media: Add Renesas CEU bindings

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Rob Herring 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  .../devicetree/bindings/media/renesas,ceu.txt  | 81 
> ++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
> b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> new file mode 100644
> index 000..590ee27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> @@ -0,0 +1,81 @@
> +Renesas Capture Engine Unit (CEU)
> +--
> +
> +The Capture Engine Unit is the image capture interface found in the Renesas
> +SH Mobile and RZ SoCs.
> +
> +The interface supports a single parallel input with data bus width of 8 or 16
> +bits.
> +
> +Required properties:
> +- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1-H
> +  and RZ/A1-M SoCs.
> +- reg: Registers address base and size.
> +- interrupts: The interrupt specifier.
> +
> +The CEU supports a single parallel input and should contain a single 'port'
> +subnode with a single 'endpoint'. Connection to input devices are modeled
> +according to the video interfaces OF bindings specified in:
> +Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Optional endpoint properties applicable to parallel input bus described in
> +the above mentioned "video-interfaces.txt" file are supported.
> +
> +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH 
> respectively.
> +  If property is not present, default is active high.
> +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH 
> respectively.
> +  If property is not present, default is active high.
> +
> +Example:
> +
> +The example describes the connection between the Capture Engine Unit and an
> +OV7670 image sensor connected to i2c1 interface.
> +
> +ceu: ceu@e821 {
> + reg = <0xe821 0x209c>;
> + compatible = "renesas,r7s72100-ceu";
> + interrupts = ;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + port {
> + ceu_in: endpoint {
> + remote-endpoint = <_out>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> +};
> +
> +i2c1: i2c@fcfee400 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + clock-frequency = <10>;
> +
> + ov7670: camera@21 {
> + compatible = "ovti,ov7670";
> + reg = <0x21>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + reset-gpios = < 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
> 

Re: [PATCH v5 4/9] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add Capture Engine Unit (CEU) node to device tree.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index ab9645a..5fe62f9 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -135,9 +135,9 @@
>   #clock-cells = <1>;
>   compatible = "renesas,r7s72100-mstp-clocks", 
> "renesas,cpg-mstp-clocks";
>   reg = <0xfcfe042c 4>;
> - clocks = <_clk>;
> - clock-indices = ;
> - clock-output-names = "rtc";
> + clocks = <_clk>, <_clk>;
> + clock-indices = ;
> + clock-output-names = "ceu", "rtc";
>   };
>  
>   mstp7_clks: mstp7_clks@fcfe0430 {
> @@ -667,4 +667,13 @@
>   power-domains = <_clocks>;
>   status = "disabled";
>   };
> +
> + ceu: ceu@e821 {
> + reg = <0xe821 0x3000>;
> + compatible = "renesas,r7s72100-ceu";
> + interrupts = ;
> + clocks = <_clks R7S72100_CLK_CEU>;
> + power-domains = <_clocks>;
> + status = "disabled";
> + };
>  };
> 

Re: [PATCH v5 2/9] include: media: Add Renesas CEU driver interface

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add renesas-ceu header file.
> 
> Do not remove the existing sh_mobile_ceu.h one as long as the original
> driver does not go away.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  include/media/drv-intf/renesas-ceu.h | 26 ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 include/media/drv-intf/renesas-ceu.h
> 
> diff --git a/include/media/drv-intf/renesas-ceu.h 
> b/include/media/drv-intf/renesas-ceu.h
> new file mode 100644
> index 000..52841d1
> --- /dev/null
> +++ b/include/media/drv-intf/renesas-ceu.h
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * renesas-ceu.h - Renesas CEU driver interface
> + *
> + * Copyright 2017-2018 Jacopo Mondi 
> + */
> +
> +#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__
> +#define __MEDIA_DRV_INTF_RENESAS_CEU_H__
> +
> +#define CEU_MAX_SUBDEVS  2
> +
> +struct ceu_async_subdev {
> + unsigned long flags;
> + unsigned char bus_width;
> + unsigned char bus_shift;
> + unsigned int i2c_adapter_id;
> + unsigned int i2c_address;
> +};
> +
> +struct ceu_platform_data {
> + unsigned int num_subdevs;
> + struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
> +};
> +
> +#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */
> 

Re: [PATCH v5 7/9] v4l: i2c: Copy tw9910 soc_camera sensor driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
> No modification to KConfig and Makefile as soc_camera framework
> dependencies need to be removed first in next commit.
> 
> Signed-off-by: Jacopo Mondi 
> Acked-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 5/9] v4l: i2c: Copy ov772x soc_camera sensor driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
> No modification to KConfig and Makefile as soc_camera framework
> dependencies need to be removed first in next commit.
> 
> Signed-off-by: Jacopo Mondi 
> Acked-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[Hans Verkuil]

Hi Jacopo,

Sorry for the late review, but here is finally is.

BTW, can you provide the v4l2-compliance output (ideally with the -f option)
in the cover letter for v6?

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |1 +
>  drivers/media/platform/renesas-ceu.c | 1648 
> ++
>  3 files changed, 1658 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd0c998..fe7bd26 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
> To compile this driver as a module, choose M here: the module
> will be called stm32-dcmi.
>  
> +config VIDEO_RENESAS_CEU
> + tristate "Renesas Capture Engine Unit (CEU) driver"
> + depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> + depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_FWNODE
> + ---help---
> +   This is a v4l2 driver for the Renesas CEU Interface
> +
>  source "drivers/media/platform/soc_camera/Kconfig"
>  source "drivers/media/platform/exynos4-is/Kconfig"
>  source "drivers/media/platform/am437x/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 003b0bb..6580a6b 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)  += sh_vou.o
>  obj-$(CONFIG_SOC_CAMERA) += soc_camera/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF)+= rcar_drif.o
> +obj-$(CONFIG_VIDEO_RENESAS_CEU)  += renesas-ceu.o
>  obj-$(CONFIG_VIDEO_RENESAS_FCP)  += rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
>  obj-$(CONFIG_VIDEO_RENESAS_JPU)  += rcar_jpu.o
> diff --git a/drivers/media/platform/renesas-ceu.c 
> b/drivers/media/platform/renesas-ceu.c
> new file mode 100644
> index 000..ccca838
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c



> +/*
> + * ceu_vb2_setup() - is called to check whether the driver can accept the
> + *requested number of buffers and to fill in plane sizes
> + *for the current frame format, if required.
> + */
> +static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
> +  unsigned int *num_planes, unsigned int sizes[],
> +  struct device *alloc_devs[])
> +{
> + struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> + struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> + unsigned int i;
> +
> + if (!*count)
> + *count = 2;

Don't do this. Instead set the min_buffers_needed field to 2 in the vb2_queue
struct.

> +
> + /* num_planes is set: just check plane sizes. */
> + if (*num_planes) {
> + for (i = 0; i < pix->num_planes; i++)
> + if (sizes[i] < pix->plane_fmt[i].sizeimage)
> + return -EINVAL;
> +
> + return 0;
> + }
> +
> + /* num_planes not set: called from REQBUFS, just set plane sizes. */
> + *num_planes = pix->num_planes;
> + for (i = 0; i < pix->num_planes; i++)
> + sizes[i] = pix->plane_fmt[i].sizeimage;
> +
> + return 0;
> +}
> +
> +static void ceu_vb2_queue(struct vb2_buffer *vb)
> +{
> + struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> + struct ceu_buffer *buf = vb2_to_ceu(vbuf);
> + unsigned long irqflags;
> + unsigned int i;
> +
> + for (i = 0; i < pix->num_planes; i++) {
> + if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> + return;
> + }
> +
> + vb2_set_plane_payload(vb, i, pix->plane_fmt[i].sizeimage);

This is not the right vb2 op for this test, this belongs in the buf_prepare
op. There you can just return an error and you don't need to call buffer_done.

> + }
> +
> + spin_lock_irqsave(>lock, irqflags);
> + list_add_tail(>queue, >capture);
> + spin_unlock_irqrestore(>lock, irqflags);
> +}
> +
> +static int ceu_start_streaming(struct vb2_queue *vq, 

Re: [RFC PATCH 6/9] media: vb2: add support for requests in QBUF ioctl

[Alexandre Courbot]

On Mon, Jan 15, 2018 at 6:19 PM, Hans Verkuil  wrote:
> On 01/15/2018 09:24 AM, Alexandre Courbot wrote:
>> On Fri, Jan 12, 2018 at 8:37 PM, Hans Verkuil  wrote:
>>> On 12/15/17 08:56, Alexandre Courbot wrote:
 Support the request argument of the QBUF ioctl.

 Signed-off-by: Alexandre Courbot 
 ---
  drivers/media/v4l2-core/v4l2-ioctl.c | 93 
 +++-
  1 file changed, 92 insertions(+), 1 deletion(-)

 diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
 b/drivers/media/v4l2-core/v4l2-ioctl.c
 index 8d041247e97f..28f9c368563e 100644
 --- a/drivers/media/v4l2-core/v4l2-ioctl.c
 +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
 @@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
 +#include 

  #include 

 @@ -965,6 +966,81 @@ static int check_fmt(struct file *file, enum 
 v4l2_buf_type type)
   return -EINVAL;
  }

 +/*
 + * Validate that a given request can be used during an ioctl.
 + *
 + * When using the request API, request file descriptors must be matched 
 against
 + * the actual request object. User-space can pass any file descriptor, so 
 we
 + * need to make sure the call is valid before going further.
 + *
 + * This function looks up the request and associated data and performs the
 + * following sanity checks:
 + *
 + * * Make sure that the entity supports requests,
 + * * Make sure that the entity belongs to the media_device managing the 
 passed
 + *   request,
 + * * Make sure that the entity data (if any) is associated to the current 
 file
 + *   handler.
 + *
 + * This function returns a pointer to the valid request, or and error 
 code in
 + * case of failure. When successful, a reference to the request is 
 acquired and
 + * must be properly released.
 + */
 +#ifdef CONFIG_MEDIA_CONTROLLER
 +static struct media_request *
 +check_request(int request, struct file *file, void *fh)
 +{
 + struct media_request *req = NULL;
 + struct video_device *vfd = video_devdata(file);
 + struct v4l2_fh *vfh =
 + test_bit(V4L2_FL_USES_V4L2_FH, >flags) ? fh : NULL;
 + struct media_entity *entity = >entity;
 + const struct media_entity *ent;
 + struct media_request_entity_data *data;
 + bool found = false;
 +
 + if (!entity)
 + return ERR_PTR(-EINVAL);
 +
 + /* Check that the entity supports requests */
 + if (!entity->req_ops)
 + return ERR_PTR(-ENOTSUPP);
 +
 + req = media_request_get_from_fd(request);
>>>
>>> You can get the media_device from vfd->v4l2_dev->mdev. So it is much easier
>>> to just pass the media_device as an argument to 
>>> media_request_get_from_fd()...
>>>
 + if (!req)
 + return ERR_PTR(-EINVAL);
 +
 + /* Validate that the entity belongs to the media_device managing
 +  * the request queue */
 + media_device_for_each_entity(ent, req->queue->mdev) {
 + if (entity == ent) {
 + found = true;
 + break;
 + }
 + }
 + if (!found) {
 + media_request_put(req);
 + return ERR_PTR(-EINVAL);
 + }
>>>
>>> ...and then you don't need to do this ^^^ extra validation check.
>>
>> Ah right, all you need to do is check that req->queue->mdev ==
>> vfd->v4l2_dev->mdev and you can get rid of this whole block.
>
> Correct.
>
>  I don't
>> think we can do that in media_request_get_from_fd() though, since it
>> is called from other places where (IIUC) we don't have access to the
>> media_device.
>
> Yes, sorry about that. Ignore my comment about media_request_get_from_fd().
> I misunderstood what that function did.
>
>>
>>>
 +
 + /* Validate that the entity's data belongs to the correct fh */
 + data = media_request_get_entity_data(req, entity, vfh);
 + if (IS_ERR(data)) {
 + media_request_put(req);
 + return ERR_PTR(PTR_ERR(data));
 + }
>>>
>>> This assumes that each filehandle has its own state. That's true for codecs,
>>> but not for most (all?) other devices. There the state is per device 
>>> instance.
>>>
>>> I'm not sure if we have a unique identifying mark for such drivers. The 
>>> closest
>>> is checking if fh->m2m_ctx is non-NULL, but I don't know if all drivers with
>>> per-filehandle state use that field. An alternative might be to check if
>>> fh->ctrl_handler is non-NULL. But again, I'm not sure if that's a 100% valid
>>> check.
>>
>> I think the current code already takes that case into account: if the
>> device does not uses v4l2_fh, then the fh argument passed to
>> 

Re: [RFC PATCH 5/9] media: vb2: add support for requests

[Alexandre Courbot]

On Mon, Jan 15, 2018 at 6:07 PM, Hans Verkuil  wrote:
> On 01/15/2018 09:24 AM, Alexandre Courbot wrote:
>> On Fri, Jan 12, 2018 at 7:49 PM, Hans Verkuil  wrote:
>>> On 12/15/17 08:56, Alexandre Courbot wrote:
 Add throttling support for buffers when requests are in use on a given
 queue. Buffers associated to a request are kept into the vb2 queue until
 the request becomes active, at which point all the buffers are passed to
 the driver. The queue can also signal that is has processed all of a
 request's buffers.

 Also add support for the request parameter when handling the QBUF ioctl.

 Signed-off-by: Alexandre Courbot 
 ---
  drivers/media/v4l2-core/videobuf2-core.c | 59 
 
  drivers/media/v4l2-core/videobuf2-v4l2.c | 29 +++-
  include/media/videobuf2-core.h   | 25 +-
  3 files changed, 104 insertions(+), 9 deletions(-)

 diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
 b/drivers/media/v4l2-core/videobuf2-core.c
 index cb115ba6a1d2..c01038b7962a 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -898,6 +898,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
 vb2_buffer_state state)
   state != VB2_BUF_STATE_REQUEUEING))
   state = VB2_BUF_STATE_ERROR;

 + WARN_ON(vb->request != q->cur_req);
>>>
>>> What's the reason for this WARN_ON? It's not immediately obvious to me.
>>
>> This is a safeguard against driver bugs: a buffer should not complete
>> unless it is part of the request being currently processed.
>>
>>>
 +
  #ifdef CONFIG_VIDEO_ADV_DEBUG
   /*
* Although this is not a callback, it still does have to balance
 @@ -920,6 +922,13 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
 vb2_buffer_state state)
   /* Add the buffer to the done buffers list */
   list_add_tail(>done_entry, >done_list);
   vb->state = state;
 +
 + if (q->cur_req) {
 + WARN_ON(q->req_buf_cnt < 1);
 +
 + if (--q->req_buf_cnt == 0)
 + q->cur_req = NULL;
 + }
   }
   atomic_dec(>owned_by_drv_count);
   spin_unlock_irqrestore(>done_lock, flags);
 @@ -1298,6 +1307,16 @@ int vb2_core_prepare_buf(struct vb2_queue *q, 
 unsigned int index, void *pb)
  }
  EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);

 +static void vb2_queue_enqueue_current_buffers(struct vb2_queue *q)
 +{
 + struct vb2_buffer *vb;
 +
 + list_for_each_entry(vb, >queued_list, queued_entry) {
 + if (vb->request == q->cur_req)
 + __enqueue_in_driver(vb);
 + }
 +}
>>>
>>> I think this will clash big time with the v4l2 fence patch series...
>>
>> Indeed, but on the other hand I was not a big fan of going through the
>> whole list. :) So I welcome the extra throttling introduced by the
>> fence series.
>
> There is only throttling if fences are used by userspace. Otherwise there
> is no change.
>
>>
>>>
 +
  /**
   * vb2_start_streaming() - Attempt to start streaming.
   * @q:   videobuf2 queue
 @@ -1318,8 +1337,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
* If any buffers were queued before streamon,
* we can now pass them to driver for processing.
*/
 - list_for_each_entry(vb, >queued_list, queued_entry)
 - __enqueue_in_driver(vb);
 + vb2_queue_enqueue_current_buffers(q);

   /* Tell the driver to start streaming */
   q->start_streaming_called = 1;
 @@ -1361,7 +1379,8 @@ static int vb2_start_streaming(struct vb2_queue *q)
   return ret;
  }

 -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index,
 +   struct media_request *req, void *pb)
  {
   struct vb2_buffer *vb;
   int ret;
 @@ -1392,6 +1411,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
 index, void *pb)
   q->queued_count++;
   q->waiting_for_buffers = false;
   vb->state = VB2_BUF_STATE_QUEUED;
 + vb->request = req;

   if (pb)
   call_void_bufop(q, copy_timestamp, vb, pb);
 @@ -1401,8 +1421,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
 index, void *pb)
   /*
* If already streaming, give the buffer to driver for processing.
* If not, the buffer will be given to driver on next streamon.
 +  *
 +  * If using the request API, the buffer will be given to 

[PATCH v6 2/4] media: ov5695: add support for OV5695 sensor

[Shunqian Zheng]

This patch adds driver for Omnivision's ov5695 sensor,
the driver supports following features:
 - supported resolutions
   + 2592x1944 at 30fps
   + 1920x1080 at 30fps
   + 1296x972 at 60fps
   + 1280x720 at 30fps
   + 640x480 at 120fps
 - test patterns
 - manual exposure/gain(analog and digital) control
 - vblank and hblank
 - media controller
 - runtime pm

Signed-off-by: Shunqian Zheng 
Reviewed-by: Jacopo Mondi 
---
 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov5695.c | 1399 
 4 files changed, 1418 insertions(+)
 create mode 100644 drivers/media/i2c/ov5695.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 85773bf..cca5b1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10046,6 +10046,13 @@ T: git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/ov5647.c
 
+OMNIVISION OV5695 SENSOR DRIVER
+M: Shunqian Zheng 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov5695.c
+
 OMNIVISION OV7670 SENSOR DRIVER
 M: Jonathan Corbet 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 3c6d642..55b37c8 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -645,6 +645,17 @@ config VIDEO_OV5670
  To compile this driver as a module, choose M here: the
  module will be called ov5670.
 
+config VIDEO_OV5695
+   tristate "OmniVision OV5695 sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV5695 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov5695.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..a063030 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 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_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
new file mode 100644
index 000..2db7d2e
--- /dev/null
+++ b/drivers/media/i2c/ov5695.c
@@ -0,0 +1,1399 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ov5695 driver
+ *
+ * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#ifndef V4L2_CID_DIGITAL_GAIN
+#define V4L2_CID_DIGITAL_GAIN  V4L2_CID_GAIN
+#endif
+
+/* 45Mhz * 4 Binning */
+#define OV5695_PIXEL_RATE  (45 * 1000 * 1000 * 4)
+#define OV5695_XVCLK_FREQ  2400
+
+#define CHIP_ID0x005695
+#define OV5695_REG_CHIP_ID 0x300a
+
+#define OV5695_REG_CTRL_MODE   0x0100
+#define OV5695_MODE_SW_STANDBY 0x0
+#define OV5695_MODE_STREAMING  BIT(0)
+
+#define OV5695_REG_EXPOSURE0x3500
+#defineOV5695_EXPOSURE_MIN 4
+#defineOV5695_EXPOSURE_STEP1
+#define OV5695_VTS_MAX 0x7fff
+
+#define OV5695_REG_ANALOG_GAIN 0x3509
+#defineANALOG_GAIN_MIN 0x10
+#defineANALOG_GAIN_MAX 0xf8
+#defineANALOG_GAIN_STEP1
+#defineANALOG_GAIN_DEFAULT 0xf8
+
+#define OV5695_REG_DIGI_GAIN_H 0x350a
+#define OV5695_REG_DIGI_GAIN_L 0x350b
+#define OV5695_DIGI_GAIN_L_MASK0x3f
+#define OV5695_DIGI_GAIN_H_SHIFT   6
+#define OV5695_DIGI_GAIN_MIN   0
+#define OV5695_DIGI_GAIN_MAX   (0x4000 - 1)
+#define OV5695_DIGI_GAIN_STEP  1
+#define OV5695_DIGI_GAIN_DEFAULT   1024
+
+#define OV5695_REG_TEST_PATTERN0x4503
+#defineOV5695_TEST_PATTERN_ENABLE  0x80
+#defineOV5695_TEST_PATTERN_DISABLE 0x0
+
+#define OV5695_REG_VTS 0x380e
+
+#define REG_NULL   0x
+
+#define OV5695_REG_VALUE_08BIT 1
+#define OV5695_REG_VALUE_16BIT 2
+#define OV5695_REG_VALUE_24BIT 3
+
+#define OV5695_LANES   2
+#define OV5695_BITS_PER_SAMPLE 10
+
+static const char * const ov5695_supply_names[] = {
+   "avdd", /* Analog power */
+   "dovdd",/* Digital I/O power */
+   

[PATCH v6 4/4] media: ov2685: add support for OV2685 sensor

[Shunqian Zheng]

This patch adds driver for Omnivision's ov2685 sensor.
Though the ov2685 can output yuv data, this driver only
supports the raw bayer format, including the following features:
  - output 1600x1200 at 30fps
  - test patterns
  - manual exposure/gain control
  - vblank and hblank
  - media controller
  - runtime pm

Signed-off-by: Shunqian Zheng 
Reviewed-by: Jacopo Mondi 
---
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  12 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov2685.c | 845 +
 4 files changed, 865 insertions(+)
 create mode 100644 drivers/media/i2c/ov2685.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cca5b1c..32afc69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10032,6 +10032,13 @@ T: git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/ov13858.c
 
+OMNIVISION OV2685 SENSOR DRIVER
+M: Shunqian Zheng 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov2685.c
+
 OMNIVISION OV5640 SENSOR DRIVER
 M: Steve Longerbeam 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 55b37c8..63a175d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2685
+   tristate "OmniVision OV2685 sensor support"
+   depends on VIDEO_V4L2 && I2C && MEDIA_CONTROLLER
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2685 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2685.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index a063030..3054c69 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
new file mode 100644
index 000..6940984
--- /dev/null
+++ b/drivers/media/i2c/ov2685.c
@@ -0,0 +1,845 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ov2685 driver
+ *
+ * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CHIP_ID0x2685
+#define OV2685_REG_CHIP_ID 0x300a
+
+#define OV2685_XVCLK_FREQ  2400
+
+#define REG_SC_CTRL_MODE   0x0100
+#define SC_CTRL_MODE_STANDBY   0x0
+#define SC_CTRL_MODE_STREAMING BIT(0)
+
+#define OV2685_REG_EXPOSURE0x3500
+#defineOV2685_EXPOSURE_MIN 4
+#defineOV2685_EXPOSURE_STEP1
+
+#define OV2685_REG_VTS 0x380e
+#define OV2685_VTS_MAX 0x7fff
+
+#define OV2685_REG_GAIN0x350a
+#define OV2685_GAIN_MIN0
+#define OV2685_GAIN_MAX0x07ff
+#define OV2685_GAIN_STEP   0x1
+#define OV2685_GAIN_DEFAULT0x0036
+
+#define OV2685_REG_TEST_PATTERN0x5080
+#define OV2685_TEST_PATTERN_DISABLED   0x00
+#define OV2685_TEST_PATTERN_COLOR_BAR  0x80
+#define OV2685_TEST_PATTERN_RANDOM 0x81
+#define OV2685_TEST_PATTERN_COLOR_BAR_FADE 0x88
+#define OV2685_TEST_PATTERN_BW_SQUARE  0x92
+#define OV2685_TEST_PATTERN_COLOR_SQUARE   0x82
+
+#define REG_NULL   0x
+
+#define OV2685_REG_VALUE_08BIT 1
+#define OV2685_REG_VALUE_16BIT 2
+#define OV2685_REG_VALUE_24BIT 3
+
+#define OV2685_LANES   1
+#define OV2685_BITS_PER_SAMPLE 10
+
+static const char * const ov2685_supply_names[] = {
+   "avdd", /* Analog power */
+   "dovdd",/* Digital I/O power */
+   "dvdd", /* Digital core power */
+};
+
+#define OV2685_NUM_SUPPLIES ARRAY_SIZE(ov2685_supply_names)
+
+struct regval {
+   u16 addr;
+   u8 val;
+};
+
+struct ov2685_mode {
+   u32 width;
+   u32 height;
+   u32 exp_def;
+   u32 hts_def;
+   u32 vts_def;
+   const struct regval *reg_list;
+};
+

[PATCH v6 3/4] dt-bindings: media: Add bindings for OV2685

[Shunqian Zheng]

Add device tree binding documentation for the OV2685 sensor.

Signed-off-by: Shunqian Zheng 
Reviewed-by: Jacopo Mondi 
---
 .../devicetree/bindings/media/i2c/ov2685.txt   | 41 ++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2685.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2685.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2685.txt
new file mode 100644
index 000..625c4a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2685.txt
@@ -0,0 +1,41 @@
+* Omnivision OV2685 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: shall be "ovti,ov2685"
+- clocks: reference to the xvclk input clock
+- clock-names: shall be "xvclk"
+- avdd-supply: Analog voltage supply, 2.8 volts
+- dovdd-supply: Digital I/O voltage supply, 1.8 volts
+- dvdd-supply: Digital core voltage supply, 1.8 volts
+- reset-gpios: Low active reset gpio
+
+The device node shall contain one 'port' child node with an
+'endpoint' subnode for its digital output video port,
+in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+The endpoint optional property 'data-lanes' shall be "<1>".
+
+Example:
+ {
+   ov2685: camera-sensor@3c {
+   compatible = "ovti,ov2685";
+   reg = <0x3c>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_24m_cam>;
+
+   clocks = < SCLK_TESTCLKOUT1>;
+   clock-names = "xvclk";
+
+   avdd-supply = <_cam>;
+   dovdd-supply = <>;
+   dvdd-supply = <>;
+   reset-gpios = < 3 GPIO_ACTIVE_LOW>;
+
+   port {
+   ucam_out: endpoint {
+   remote-endpoint = <_in_ucam>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
1.9.1

[PATCH v6 0/4] Add supports for OV2685 and OV5695 sensors

[Shunqian Zheng]

This adds the OV2685 and OV5695 sensor supports.

Changes of v6 are mainly addressing comments from Jacopo@
 - Fix the node/lable name inverted in Doc.
 - Guard the source code with IS_ENABLED(CONFIG_OF)
 - Move the __v4l2_ctrl_handler_setup() into ov2685_s_stream()

Changes of v5,
 - Squash the MAINTAINERS entry to driver patch

Mainly changes of v4 are addressing the comments from Sakari,
including,
 - Put dt binding before driver in series
 - Add MAINTAINERS entries
 - Use regulator_bulk_*()
 - Fix the pm_runtime_* in probe()
 - Fix the typo of 2685 0x3008/0x3010 regs

Shunqian Zheng (4):
  dt-bindings: media: Add bindings for OV5695
  media: ov5695: add support for OV5695 sensor
  dt-bindings: media: Add bindings for OV2685
  media: ov2685: add support for OV2685 sensor

 .../devicetree/bindings/media/i2c/ov2685.txt   |   41 +
 .../devicetree/bindings/media/i2c/ov5695.txt   |   41 +
 MAINTAINERS|   14 +
 drivers/media/i2c/Kconfig  |   23 +
 drivers/media/i2c/Makefile |2 +
 drivers/media/i2c/ov2685.c |  845 
 drivers/media/i2c/ov5695.c | 1399 
 7 files changed, 2365 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2685.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
 create mode 100644 drivers/media/i2c/ov2685.c
 create mode 100644 drivers/media/i2c/ov5695.c

-- 
1.9.1

[PATCH v6 1/4] dt-bindings: media: Add bindings for OV5695

[Shunqian Zheng]

Add device tree binding documentation for the OV5695 sensor.

Signed-off-by: Shunqian Zheng 
Reviewed-by: Jacopo Mondi 
---
 .../devicetree/bindings/media/i2c/ov5695.txt   | 41 ++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
new file mode 100644
index 000..640a637
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
@@ -0,0 +1,41 @@
+* Omnivision OV5695 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: shall be "ovti,ov5695"
+- clocks: reference to the xvclk input clock
+- clock-names: shall be "xvclk"
+- avdd-supply: Analog voltage supply, 2.8 volts
+- dovdd-supply: Digital I/O voltage supply, 1.8 volts
+- dvdd-supply: Digital core voltage supply, 1.2 volts
+- reset-gpios: Low active reset gpio
+
+The device node shall contain one 'port' child node with an
+'endpoint' subnode for its digital output video port,
+in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+The endpoint optional property 'data-lanes' shall be "<1 2>".
+
+Example:
+ {
+   ov5695: camera-sensor@36 {
+   compatible = "ovti,ov5695";
+   reg = <0x36>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_24m_cam>;
+
+   clocks = < SCLK_TESTCLKOUT1>;
+   clock-names = "xvclk";
+
+   avdd-supply = <_cam>;
+   dovdd-supply = <>;
+   dvdd-supply = <_cam>;
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>;
+
+   port {
+   wcam_out: endpoint {
+   remote-endpoint = <_in_wcam>;
+   data-lanes = <1 2>;
+   };
+   };
+   };
+};
-- 
1.9.1

[ANN] Prague Media summit report October 2017

[Sakari Ailus]

Hello everyone,

Here's the Prague Media summit report from 26th and 27th October 2017. It
took long but finally it's here!

If you feel something important is missing or incorrect, feel free to
reply.


First day
*

The first day was mostly concentrated on discussing the media development
process. No notes were done at the time of the meeting, so any report here
is based on my own recollection may be unrepresentative of the entire
meeting. 

Attendees

Sakari Ailus
  - Mauro Carvalho Chehab   
  
Shuah Khan  
  
Mike Krufky 
  
Gustavo Padovan 
  
Laurent Pinchart
  
Niklas Söderlund
  
Hans Verkuil
  


Media development statistics


Mauro gave a presentation on media tree development statistics he's been
collecting. The slides can be found here:




Media device sharing


Media device sharing was discussed during the first day as well. The use
case is related to a USB device that in Linux has several interfaces (?)
which in Linux are devices. The purpose of this would be to show entities
backed by different devices from the same USB device, without either of the
drivers being necessarily aware of each other. The USB device is also hot
unpluggable, meaning that the devices backing the media entities in the
media graph may disappear, and do so in any order as they could appear.

Media device sharing requires improvements to the Media controller
framework. Especially:

- The Media controller framework assumes the media device is backed by a
  single struct device. There's also an ops struct, also related to a
  single driver. There may no longer be 1:1 relation between these two and
  the media device.

- Object lifetime management. Object lifetime management has been found to
  be not designed well enough in Media controller in general. The original
  use case was non-hotpluggable devices and the early efforts address the
  problem were focussed in preventing unbinding of drivers from devices in
  various ways. This need to be properly addressed, with refcounts, so that
  the necessary memory objects for various operations will stay where
  needed as long as needed. (I'm not going to details here. Work has been
  done to this end but it is not complete yet.)


Second day
**

The report on the second day is based on the notes written during the
meeting. The raw notes can be found here:



Attendees

Jacopo Mondi
Mauro Chehab
Niklas Söderlund
Benoit Parrot
Pavel Machek
Ricardo Ribalda
Dimitrios Katasaros
Laurent Pinchart
Hans Verkuil
Gustavo Padovan
Alexandre Courbot
Sakari Ailus
Mike Krufky
Brad Love
Tomas Dratva


CEC status
==

Hans gave a presentation on CEC status. The slides are available here:



Latest CEC status is always available here:



Side note: should we use the new serial bus (serdev) for the RC serial
driver?


Explicit synchronization in V4L2


Gustavo gave a presentation on explicit synchronisation in V4L2. It's
available here:



In-fences are easy, there is no large issue in the RFC.

Out-fences, however, are subject to buffer reordering issues. A new event
has been created to report buffer order to userspace. Fences are created at
QBUF time, but can't be used by userspace until an event reports which
buffer will be available next.

Not all drivers perform reordering. As an optimization a capability flag
could tell userspace that buffers are always ordered and that events are
not needed. The capability flag should likely happen at/after setting
format, as out-of-order depends on the video format

Reordering is mostly useful for codecs, and can also occur from buffer
recycling internally in drivers when capture errors happen. While recycling
is allowed by V4L2, we could forbid it when fences are used. In that case
all buffers would be returned to userspace in order, and errors be reported
with the error flag set.

We may need to make capture queues not 

Dear Friend,

[Mrs. Aisha Gaddafi]

Dear Friend,

I came across your e-mail contact prior a private search while in need
of your assistance. My name is Aisha  Gaddafi a single Mother and a
Widow with three Children. I am the only biological Daughter of late
Libyan President (Late Colonel Muammar Gaddafi).

I have an investment funds worth Twenty Seven Million Five Hundred
Thousand United State Dollar ($27.500.000.00 ) and i need an
investment Manager/Partner and because of the asylum status i will
authorize you the ownership of the funds, however, I am interested in
you for investment project assistance in your country, may be from
there, we can build a business relationship in the near future.

I am willing to negotiate investment/business profit sharing ratio
with you base on the future investment earning profits. If you are
willing to handle this project kindly reply urgent to enable me
provide you more information about the investment funds. Your Urgent
Reply Will Be Appreciated Please Reply me in my box.
(mrs.aishagad...@gmail.com)

Best Regards
Mrs Aisha Gaddafi