[PATCH] Staging: omap4iss: fix coding style issues

[PATCH] Staging: omap4iss: fix coding style issues

[Avraham Shukron]

This is a patch that fixes checkpatch.pl issues in omap4iss/iss_video.c
Specifically, it fixes "line over 80 characters" issues

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>

---
 drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..cdab053 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
 
 static int iss_video_queue_setup(struct vb2_queue *vq,
 				 unsigned int *count, unsigned int *num_planes,
-				 unsigned int sizes[], struct device *alloc_devs[])
+				 unsigned int sizes[],
+				 struct device *alloc_devs[])
 {
 	struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
 	struct iss_video *video = vfh->video;
@@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 	if (subdev == NULL)
 		return -EINVAL;
 
-	/* Try the get selection operation first and fallback to get format if not
-	 * implemented.
+	/* Try the get selection operation first and fallback to get format if
+	 * not implemented.
 	 */
 	sdsel.pad = pad;
 	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
-- 
2.7.4

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Ozgur Karatas]

28.01.2017, 20:11, "Avraham Shukron" <avraham.shukron@gmail.com>:
> This is a patch that fixes issues in omap4iss/iss_video.c
> Specifically, it fixes "line over 80 characters" issues

Hello,

are you have a sent this changes patch before?
And Greg KH answered you, are you read?

Please send the change once, there is no need for a repeat. 

> Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
>
> ---
>  drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index c16927a..cdab053 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
>
>  static int iss_video_queue_setup(struct vb2_queue *vq,
>                                   unsigned int *count, unsigned int *num_planes,
> - unsigned int sizes[], struct device *alloc_devs[])
> + unsigned int sizes[],
> + struct device *alloc_devs[])

it should be on the same line, maintainer's up to 80 characters allowed.
this "alloc_devs" variable start with int?

Example:

struct device {
  int (struct device *alloc_devs[);

Check the top lines of the codes.


>  {
>          struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>          struct iss_video *video = vfh->video;
> @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
>          if (subdev == NULL)
>                  return -EINVAL;
>
> - /* Try the get selection operation first and fallback to get format if not
> - * implemented.
> + /* Try the get selection operation first and fallback to get format if
> + * not implemented.
>           */

There is no change here, it opens with comment /* and closes with */.
Please read submittting patch document.

Regards,

>          sdsel.pad = pad;
>          ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
> --
> 2.7.4

~Ozgur

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Ozgur Karatas]

28.01.2017, 20:11, "Avraham Shukron" <avraham.shukron@gmail.com>:
> This is a patch that fixes issues in omap4iss/iss_video.c
> Specifically, it fixes "line over 80 characters" issues

Hello,

are you have a sent this changes patch before?
And Greg KH answered you, are you read?

Please send the change once, there is no need for a repeat. 

> Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
>
> ---
>  drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index c16927a..cdab053 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
>
>  static int iss_video_queue_setup(struct vb2_queue *vq,
>                                   unsigned int *count, unsigned int *num_planes,
> - unsigned int sizes[], struct device *alloc_devs[])
> + unsigned int sizes[],
> + struct device *alloc_devs[])

it should be on the same line, maintainer's up to 80 characters allowed.
this "alloc_devs" variable start with int?

Example:

struct device {
  int (struct device *alloc_devs[);

Check the top lines of the codes.


>  {
>          struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>          struct iss_video *video = vfh->video;
> @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
>          if (subdev == NULL)
>                  return -EINVAL;
>
> - /* Try the get selection operation first and fallback to get format if not
> - * implemented.
> + /* Try the get selection operation first and fallback to get format if
> + * not implemented.
>           */

There is no change here, it opens with comment /* and closes with */.
Please read submittting patch document.

Regards,

>          sdsel.pad = pad;
>          ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
> --
> 2.7.4

~Ozgur

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Avraham Shukron]

On 29/01/2017 9:31, Ozgur Karatas wrote:
> 
> 
> 28.01.2017, 20:11, "Avraham Shukron" <avraham.shukron@gmail.com>:
>> This is a patch that fixes issues in omap4iss/iss_video.c
>> Specifically, it fixes "line over 80 characters" issues
> 
> Hello,
> 
> are you have a sent this changes patch before?
> And Greg KH answered you, are you read?
> 
> Please send the change once, there is no need for a repeat.

Greg asked me to resend because I messed-up the subject line. I did.
Later I figured that I should have sent it as v2, but now its too late.


>> ---
>>  drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
>> index c16927a..cdab053 100644
>> --- a/drivers/staging/media/omap4iss/iss_video.c
>> +++ b/drivers/staging/media/omap4iss/iss_video.c
>> @@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
>>
>>  static int iss_video_queue_setup(struct vb2_queue *vq,
>>                                   unsigned int *count, unsigned int *num_planes,
>> - unsigned int sizes[], struct device *alloc_devs[])
>> + unsigned int sizes[],
>> + struct device *alloc_devs[])
> 
> it should be on the same line, maintainer's up to 80 characters allowed.
> this "alloc_devs" variable start with int?
> 
> Example:
> 
> struct device {
>   int (struct device *alloc_devs[);
> 
> Check the top lines of the codes.
> 
> 

No idea what you mean, but the line was *above* 80 characters, so I broke it.

>>  {
>>          struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>>          struct iss_video *video = vfh->video;
>> @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
>>          if (subdev == NULL)
>>                  return -EINVAL;
>>
>> - /* Try the get selection operation first and fallback to get format if not
>> - * implemented.
>> + /* Try the get selection operation first and fallback to get format if
>> + * not implemented.
>>           */
> 
> There is no change here, it opens with comment /* and closes with */.
> Please read submittting patch document.

The *is* a change. the word "not" is now on the second line of the comment,
putting the first line below 80 characters.

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Avraham Shukron]

On 29/01/2017 9:31, Ozgur Karatas wrote:
> 
> 
> 28.01.2017, 20:11, "Avraham Shukron" <avraham.shukron@gmail.com>:
>> This is a patch that fixes issues in omap4iss/iss_video.c
>> Specifically, it fixes "line over 80 characters" issues
> 
> Hello,
> 
> are you have a sent this changes patch before?
> And Greg KH answered you, are you read?
> 
> Please send the change once, there is no need for a repeat.

Greg asked me to resend because I messed-up the subject line. I did.
Later I figured that I should have sent it as v2, but now its too late.


>> ---
>>  drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
>> index c16927a..cdab053 100644
>> --- a/drivers/staging/media/omap4iss/iss_video.c
>> +++ b/drivers/staging/media/omap4iss/iss_video.c
>> @@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
>>
>>  static int iss_video_queue_setup(struct vb2_queue *vq,
>>                                   unsigned int *count, unsigned int *num_planes,
>> - unsigned int sizes[], struct device *alloc_devs[])
>> + unsigned int sizes[],
>> + struct device *alloc_devs[])
> 
> it should be on the same line, maintainer's up to 80 characters allowed.
> this "alloc_devs" variable start with int?
> 
> Example:
> 
> struct device {
>   int (struct device *alloc_devs[);
> 
> Check the top lines of the codes.
> 
> 

No idea what you mean, but the line was *above* 80 characters, so I broke it.

>>  {
>>          struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>>          struct iss_video *video = vfh->video;
>> @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
>>          if (subdev == NULL)
>>                  return -EINVAL;
>>
>> - /* Try the get selection operation first and fallback to get format if not
>> - * implemented.
>> + /* Try the get selection operation first and fallback to get format if
>> + * not implemented.
>>           */
> 
> There is no change here, it opens with comment /* and closes with */.
> Please read submittting patch document.

The *is* a change. the word "not" is now on the second line of the comment,
putting the first line below 80 characters.

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Laurent Pinchart]

Hello Avraham,

Thank you for the patch.

On Saturday 28 Jan 2017 20:00:08 Avraham Shukron wrote:
> This is a patch that fixes checkpatch.pl issues in omap4iss/iss_video.c
> Specifically, it fixes "line over 80 characters" issues
> 
> Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>

This looks OK to me. I've applied the patch to my tree and will push it to 
v4.11.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/staging/media/omap4iss/iss_video.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index c16927a..cdab053 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -298,7 +298,8 @@ iss_video_check_format(struct iss_video *video, struct
> iss_video_fh *vfh)
> 
>  static int iss_video_queue_setup(struct vb2_queue *vq,
>  				 unsigned int *count, unsigned int 
*num_planes,
> -				 unsigned int sizes[], struct device 
*alloc_devs[])
> +				 unsigned int sizes[],
> +				 struct device *alloc_devs[])
>  {
>  	struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>  	struct iss_video *video = vfh->video;
> @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh,
> struct v4l2_selection *sel) if (subdev == NULL)
>  		return -EINVAL;
> 
> -	/* Try the get selection operation first and fallback to get format if 
not
> -	 * implemented.
> +	/* Try the get selection operation first and fallback to get format if
> +	 * not implemented.
>  	 */
>  	sdsel.pad = pad;
>  	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);

-- 
Regards,

Laurent Pinchart

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Sakari Ailus]

On Mon, Jan 30, 2017 at 07:47:40PM +0200, Laurent Pinchart wrote:
> > @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh,
> > struct v4l2_selection *sel) if (subdev == NULL)
> >  		return -EINVAL;
> > 
> > -	/* Try the get selection operation first and fallback to get format if 
> not
> > -	 * implemented.
> > +	/* Try the get selection operation first and fallback to get format if
> > +	 * not implemented.
> >  	 */

/*
 * Multi line
 * comment.
 */

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] Staging: omap4iss: fix coding style issues

[Laurent Pinchart]

On Tuesday 31 Jan 2017 12:42:51 Sakari Ailus wrote:
> On Mon, Jan 30, 2017 at 07:47:40PM +0200, Laurent Pinchart wrote:
> > > @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh,
> > > struct v4l2_selection *sel) if (subdev == NULL)
> > > 
> > >  		return -EINVAL;
> > > 
> > > -	/* Try the get selection operation first and fallback to get format if
> > 
> > not
> > 
> > > -	 * implemented.
> > > +	/* Try the get selection operation first and fallback to get format if
> > > +	 * not implemented.
> > > 
> > >  	 */
> 
> /*
>  * Multi line
>  * comment.
>  */

Then let's patch the whole driver in one go :-)

-- 
Regards,

Laurent Pinchart

[PATCH v2] Staging: omap4iss: Fix coding style issues

[Avraham Shukron]

Fixes line-over-80-characters issues as well as multiline comments style.

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>

---
 drivers/staging/media/omap4iss/iss_video.c | 41 ++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..42de513 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,
 	pix->width = mbus->width;
 	pix->height = mbus->height;
 
-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,
 
 	min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;
 
-	/* Clamp the requested bytes per line value. If the maximum bytes per
+	/*
+	 * Clamp the requested bytes per line value. If the maximum bytes per
 	 * line value is zero, the module doesn't support user configurable line
 	 * sizes. Override the requested value with the minimum in that case.
 	 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct v4l2_pix_format *pix,
 	mbus->width = pix->width;
 	mbus->height = pix->height;
 
-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -298,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
 
 static int iss_video_queue_setup(struct vb2_queue *vq,
 				 unsigned int *count, unsigned int *num_planes,
-				 unsigned int sizes[], struct device *alloc_devs[])
+				 unsigned int sizes[],
+				 struct device *alloc_devs[])
 {
 	struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
 	struct iss_video *video = vfh->video;
@@ -360,7 +364,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)
 
 	spin_lock_irqsave(&video->qlock, flags);
 
-	/* Mark the buffer is faulty and give it back to the queue immediately
+	/*
+	 * Mark the buffer is faulty and give it back to the queue immediately
 	 * if the video node has registered an error. vb2 will perform the same
 	 * check when preparing the buffer, but that is inherently racy, so we
 	 * need to handle the race condition with an authoritative check here.
@@ -443,7 +448,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
 
 	buf->vb.vb2_buf.timestamp = ktime_get_ns();
 
-	/* Do frame number propagation only if this is the output video node.
+	/*
+	 * Do frame number propagation only if this is the output video node.
 	 * Frame number either comes from the CSI receivers or it gets
 	 * incremented here if H3A is not active.
 	 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +611,8 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
 
 	mutex_lock(&video->mutex);
 
-	/* Fill the bytesperline and sizeimage fields by converting to media bus
+	/*
+	 * Fill the bytesperline and sizeimage fields by converting to media bus
 	 * format and back to pixel format.
 	 */
 	iss_video_pix_to_mbus(&format->fmt.pix, &fmt);
@@ -678,8 +685,9 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 	if (subdev == NULL)
 		return -EINVAL;
 
-	/* Try the get selection operation first and fallback to get format if not
-	 * implemented.
+	/*
+	 * Try the get selection operation first and fallback to get format if
+	 * not implemented.
 	 */
 	sdsel.pad = pad;
 	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
@@ -867,7 +875,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	mutex_lock(&video->stream_lock);
 
-	/* Start streaming on the pipeline. No link touching an entity in the
+	/*
+	 * Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
 	pipe = entity->pipe
@@ -895,7 +904,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	while ((entity = media_entity_graph_walk_next(&graph)))
 		media_entity_enum_set(&pipe->ent_enum, entity);
 
-	/* Verify that the currently configured format matches the output of
+	/*
+	 * Verify that the currently configured format matches the output of
 	 * the connected subdev.
 	 */
 	ret = iss_video_check_format(video, vfh);
@@ -905,7 +915,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	video->bpl_padding = ret;
 	video->bpl_value = vfh->format.fmt.pix.bytesperline;
 
-	/* Find the ISS video node connected at the far end of the pipeline and
+	/*
+	 * Find the ISS video node connected at the far end of the pipeline and
 	 * update the pipeline.
 	 */
 	far_end = iss_video_far_end(video);
@@ -930,7 +941,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	pipe->state |= state;
 	spin_unlock_irqrestore(&pipe->lock, flags);
 
-	/* Set the maximum time per frame as the value requested by userspace.
+	/*
+	 * Set the maximum time per frame as the value requested by userspace.
 	 * This is a soft limit that can be overridden if the hardware doesn't
 	 * support the request limit.
 	 */
@@ -946,7 +958,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (ret < 0)
 		goto err_iss_video_check_format;
 
-	/* In sensor-to-memory mode, the stream can be started synchronously
+	/*
+	 * In sensor-to-memory mode, the stream can be started synchronously
 	 * to the stream on command. In memory-to-memory mode, it will be
 	 * started when buffers are queued on both the input and output.
 	 */
-- 
2.7.4

Re: [PATCH v2] Staging: omap4iss: Fix coding style issues

[Greg KH]

On Mon, Feb 06, 2017 at 07:58:35PM +0200, Avraham Shukron wrote:
> Fixes line-over-80-characters issues as well as multiline comments style.

When you say things like "as well as", that's a hint that this needs to
be broken up into different patches.  Please do so here.

thanks,

greg k-h

[PATCH v3 1/2] staging: omap4iss: fix multiline comment style

[Avraham Shukron]

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
---
 drivers/staging/media/omap4iss/iss_video.c | 38 ++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index bb0e3b4..e21811a 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,
 	pix->width = mbus->width;
 	pix->height = mbus->height;
 
-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,
 
 	min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;
 
-	/* Clamp the requested bytes per line value. If the maximum bytes per
+	/*
+	 * Clamp the requested bytes per line value. If the maximum bytes per
 	 * line value is zero, the module doesn't support user configurable line
 	 * sizes. Override the requested value with the minimum in that case.
 	 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct v4l2_pix_format *pix,
 	mbus->width = pix->width;
 	mbus->height = pix->height;
 
-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -360,7 +363,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)
 
 	spin_lock_irqsave(&video->qlock, flags);
 
-	/* Mark the buffer is faulty and give it back to the queue immediately
+	/*
+	 * Mark the buffer is faulty and give it back to the queue immediately
 	 * if the video node has registered an error. vb2 will perform the same
 	 * check when preparing the buffer, but that is inherently racy, so we
 	 * need to handle the race condition with an authoritative check here.
@@ -443,7 +447,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
 
 	buf->vb.vb2_buf.timestamp = ktime_get_ns();
 
-	/* Do frame number propagation only if this is the output video node.
+	/*
+	 * Do frame number propagation only if this is the output video node.
 	 * Frame number either comes from the CSI receivers or it gets
 	 * incremented here if H3A is not active.
 	 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +610,8 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
 
 	mutex_lock(&video->mutex);
 
-	/* Fill the bytesperline and sizeimage fields by converting to media bus
+	/*
+	 * Fill the bytesperline and sizeimage fields by converting to media bus
 	 * format and back to pixel format.
 	 */
 	iss_video_pix_to_mbus(&format->fmt.pix, &fmt);
@@ -678,8 +684,9 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 	if (subdev == NULL)
 		return -EINVAL;
 
-	/* Try the get selection operation first and fallback to get format if not
-	 * implemented.
+	/*
+	 * Try the get selection operation first and fallback to get format if
+	 * not implemented.
 	 */
 	sdsel.pad = pad;
 	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
@@ -867,7 +874,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	mutex_lock(&video->stream_lock);
 
-	/* Start streaming on the pipeline. No link touching an entity in the
+	/*
+	 * Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
 	pipe = entity->pipe
@@ -895,7 +903,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	while ((entity = media_graph_walk_next(&graph)))
 		media_entity_enum_set(&pipe->ent_enum, entity);
 
-	/* Verify that the currently configured format matches the output of
+	/*
+	 * Verify that the currently configured format matches the output of
 	 * the connected subdev.
 	 */
 	ret = iss_video_check_format(video, vfh);
@@ -905,7 +914,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	video->bpl_padding = ret;
 	video->bpl_value = vfh->format.fmt.pix.bytesperline;
 
-	/* Find the ISS video node connected at the far end of the pipeline and
+	/*
+	 * Find the ISS video node connected at the far end of the pipeline and
 	 * update the pipeline.
 	 */
 	far_end = iss_video_far_end(video);
@@ -930,7 +940,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	pipe->state |= state;
 	spin_unlock_irqrestore(&pipe->lock, flags);
 
-	/* Set the maximum time per frame as the value requested by userspace.
+	/*
+	 * Set the maximum time per frame as the value requested by userspace.
 	 * This is a soft limit that can be overridden if the hardware doesn't
 	 * support the request limit.
 	 */
@@ -946,7 +957,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (ret < 0)
 		goto err_iss_video_check_format;
 
-	/* In sensor-to-memory mode, the stream can be started synchronously
+	/*
+	 * In sensor-to-memory mode, the stream can be started synchronously
 	 * to the stream on command. In memory-to-memory mode, it will be
 	 * started when buffers are queued on both the input and output.
 	 */
-- 
2.7.4

[PATCH v3 2/2] staging: omap4iss: fix coding style issue

[Avraham Shukron]

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index e21811a..0bac582 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -301,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)
 
 static int iss_video_queue_setup(struct vb2_queue *vq,
 				 unsigned int *count, unsigned int *num_planes,
-				 unsigned int sizes[], struct device *alloc_devs[])
+				 unsigned int sizes[],
+				 struct device *alloc_devs[])
 {
 	struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
 	struct iss_video *video = vfh->video;
-- 
2.7.4

Re: [PATCH v3 1/2] staging: omap4iss: fix multiline comment style

[Greg KH]

On Tue, Feb 07, 2017 at 05:40:57PM +0200, Avraham Shukron wrote:
> Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss_video.c | 38 ++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 13 deletions(-)

Again, I don't take patches with no changelog text :(

[PATCH v4 1/2] staging: omap4iss: fix multiline comment style

[Avraham Shukron]

Fixed multi-line comments to their preferred style (First line empty)

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
---
 drivers/staging/media/omap4iss/iss_video.c | 38 ++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index bb0e3b4..e21811a 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,
 	pix->width = mbus->width;
 	pix->height = mbus->height;

-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct iss_video *video,

 	min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;

-	/* Clamp the requested bytes per line value. If the maximum bytes per
+	/*
+	 * Clamp the requested bytes per line value. If the maximum bytes per
 	 * line value is zero, the module doesn't support user configurable line
 	 * sizes. Override the requested value with the minimum in that case.
 	 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct v4l2_pix_format *pix,
 	mbus->width = pix->width;
 	mbus->height = pix->height;

-	/* Skip the last format in the loop so that it will be selected if no
+	/*
+	 * Skip the last format in the loop so that it will be selected if no
 	 * match is found.
 	 */
 	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -360,7 +363,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)

 	spin_lock_irqsave(&video->qlock, flags);

-	/* Mark the buffer is faulty and give it back to the queue immediately
+	/*
+	 * Mark the buffer is faulty and give it back to the queue immediately
 	 * if the video node has registered an error. vb2 will perform the same
 	 * check when preparing the buffer, but that is inherently racy, so we
 	 * need to handle the race condition with an authoritative check here.
@@ -443,7 +447,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)

 	buf->vb.vb2_buf.timestamp = ktime_get_ns();

-	/* Do frame number propagation only if this is the output video node.
+	/*
+	 * Do frame number propagation only if this is the output video node.
 	 * Frame number either comes from the CSI receivers or it gets
 	 * incremented here if H3A is not active.
 	 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +610,8 @@ iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)

 	mutex_lock(&video->mutex);

-	/* Fill the bytesperline and sizeimage fields by converting to media bus
+	/*
+	 * Fill the bytesperline and sizeimage fields by converting to media bus
 	 * format and back to pixel format.
 	 */
 	iss_video_pix_to_mbus(&format->fmt.pix, &fmt);
@@ -678,8 +684,9 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 	if (subdev == NULL)
 		return -EINVAL;

-	/* Try the get selection operation first and fallback to get format if not
-	 * implemented.
+	/*
+	 * Try the get selection operation first and fallback to get format if
+	 * not implemented.
 	 */
 	sdsel.pad = pad;
 	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
@@ -867,7 +874,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)

 	mutex_lock(&video->stream_lock);

-	/* Start streaming on the pipeline. No link touching an entity in the
+	/*
+	 * Start streaming on the pipeline. No link touching an entity in the
 	 * pipeline can be activated or deactivated once streaming is started.
 	 */
 	pipe = entity->pipe
@@ -895,7 +903,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	while ((entity = media_graph_walk_next(&graph)))
 		media_entity_enum_set(&pipe->ent_enum, entity);

-	/* Verify that the currently configured format matches the output of
+	/*
+	 * Verify that the currently configured format matches the output of
 	 * the connected subdev.
 	 */
 	ret = iss_video_check_format(video, vfh);
@@ -905,7 +914,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	video->bpl_padding = ret;
 	video->bpl_value = vfh->format.fmt.pix.bytesperline;

-	/* Find the ISS video node connected at the far end of the pipeline and
+	/*
+	 * Find the ISS video node connected at the far end of the pipeline and
 	 * update the pipeline.
 	 */
 	far_end = iss_video_far_end(video);
@@ -930,7 +940,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	pipe->state |= state;
 	spin_unlock_irqrestore(&pipe->lock, flags);

-	/* Set the maximum time per frame as the value requested by userspace.
+	/*
+	 * Set the maximum time per frame as the value requested by userspace.
 	 * This is a soft limit that can be overridden if the hardware doesn't
 	 * support the request limit.
 	 */
@@ -946,7 +957,8 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	if (ret < 0)
 		goto err_iss_video_check_format;

-	/* In sensor-to-memory mode, the stream can be started synchronously
+	/*
+	 * In sensor-to-memory mode, the stream can be started synchronously
 	 * to the stream on command. In memory-to-memory mode, it will be
 	 * started when buffers are queued on both the input and output.
 	 */
-- 
2.7.4

[PATCH v4 2/2] staging: omap4iss: fix coding style issue

[Avraham Shukron]

Broke argument list so that it won't exceed 80 characters

Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index e21811a..0bac582 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -301,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct iss_video_fh *vfh)

 static int iss_video_queue_setup(struct vb2_queue *vq,
 				 unsigned int *count, unsigned int *num_planes,
-				 unsigned int sizes[], struct device *alloc_devs[])
+				 unsigned int sizes[],
+				 struct device *alloc_devs[])
 {
 	struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
 	struct iss_video *video = vfh->video;
-- 
2.7.4

Re: [PATCH v4 1/2] staging: omap4iss: fix multiline comment style

[Laurent Pinchart]

Hi Avraham,

Thank you for the patches.

On Thursday 09 Feb 2017 18:57:55 Avraham Shukron wrote:
> Fixed multi-line comments to their preferred style (First line empty)
> 
> Signed-off-by: Avraham Shukron <avraham.shukron@gmail.com>

For both of them,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I've applied the patches to my tree and will send a pull request for v4.12.

> ---
>  drivers/staging/media/omap4iss/iss_video.c | 38 ++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index bb0e3b4..e21811a 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct
> iss_video *video, pix->width = mbus->width;
>  	pix->height = mbus->height;
> 
> -	/* Skip the last format in the loop so that it will be selected if no
> +	/*
> +	 * Skip the last format in the loop so that it will be selected if no
>  	 * match is found.
>  	 */
>  	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
> @@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct
> iss_video *video,
> 
>  	min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;
> 
> -	/* Clamp the requested bytes per line value. If the maximum bytes per
> +	/*
> +	 * Clamp the requested bytes per line value. If the maximum bytes per
>  	 * line value is zero, the module doesn't support user configurable 
line
>  	 * sizes. Override the requested value with the minimum in that case.
>  	 */
> @@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct
> v4l2_pix_format *pix, mbus->width = pix->width;
>  	mbus->height = pix->height;
> 
> -	/* Skip the last format in the loop so that it will be selected if no
> +	/*
> +	 * Skip the last format in the loop so that it will be selected if no
>  	 * match is found.
>  	 */
>  	for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
> @@ -360,7 +363,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)
> 
>  	spin_lock_irqsave(&video->qlock, flags);
> 
> -	/* Mark the buffer is faulty and give it back to the queue immediately
> +	/*
> +	 * Mark the buffer is faulty and give it back to the queue immediately
>  	 * if the video node has registered an error. vb2 will perform the 
same
>  	 * check when preparing the buffer, but that is inherently racy, so we
>  	 * need to handle the race condition with an authoritative check here.
> @@ -443,7 +447,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video)
> 
>  	buf->vb.vb2_buf.timestamp = ktime_get_ns();
> 
> -	/* Do frame number propagation only if this is the output video node.
> +	/*
> +	 * Do frame number propagation only if this is the output video node.
>  	 * Frame number either comes from the CSI receivers or it gets
>  	 * incremented here if H3A is not active.
>  	 * Note: There is no guarantee that the output buffer will finish
> @@ -605,7 +610,8 @@ iss_video_set_format(struct file *file, void *fh, struct
> v4l2_format *format)
> 
>  	mutex_lock(&video->mutex);
> 
> -	/* Fill the bytesperline and sizeimage fields by converting to media 
bus
> +	/*
> +	 * Fill the bytesperline and sizeimage fields by converting to media 
bus
>  	 * format and back to pixel format.
>  	 */
>  	iss_video_pix_to_mbus(&format->fmt.pix, &fmt);
> @@ -678,8 +684,9 @@ iss_video_get_selection(struct file *file, void *fh,
> struct v4l2_selection *sel) if (subdev == NULL)
>  		return -EINVAL;
> 
> -	/* Try the get selection operation first and fallback to get format if 
not
> -	 * implemented.
> +	/*
> +	 * Try the get selection operation first and fallback to get format if
> +	 * not implemented.
>  	 */
>  	sdsel.pad = pad;
>  	ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel);
> @@ -867,7 +874,8 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> 
>  	mutex_lock(&video->stream_lock);
> 
> -	/* Start streaming on the pipeline. No link touching an entity in the
> +	/*
> +	 * Start streaming on the pipeline. No link touching an entity in the
>  	 * pipeline can be activated or deactivated once streaming is started.
>  	 */
>  	pipe = entity->pipe
> @@ -895,7 +903,8 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) while ((entity = media_graph_walk_next(&graph)))
>  		media_entity_enum_set(&pipe->ent_enum, entity);
> 
> -	/* Verify that the currently configured format matches the output of
> +	/*
> +	 * Verify that the currently configured format matches the output of
>  	 * the connected subdev.
>  	 */
>  	ret = iss_video_check_format(video, vfh);
> @@ -905,7 +914,8 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) video->bpl_padding = ret;
>  	video->bpl_value = vfh->format.fmt.pix.bytesperline;
> 
> -	/* Find the ISS video node connected at the far end of the pipeline 
and
> +	/*
> +	 * Find the ISS video node connected at the far end of the pipeline 
and
>  	 * update the pipeline.
>  	 */
>  	far_end = iss_video_far_end(video);
> @@ -930,7 +940,8 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) pipe->state |= state;
>  	spin_unlock_irqrestore(&pipe->lock, flags);
> 
> -	/* Set the maximum time per frame as the value requested by userspace.
> +	/*
> +	 * Set the maximum time per frame as the value requested by userspace.
>  	 * This is a soft limit that can be overridden if the hardware doesn't
>  	 * support the request limit.
>  	 */
> @@ -946,7 +957,8 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) if (ret < 0)
>  		goto err_iss_video_check_format;
> 
> -	/* In sensor-to-memory mode, the stream can be started synchronously
> +	/*
> +	 * In sensor-to-memory mode, the stream can be started synchronously
>  	 * to the stream on command. In memory-to-memory mode, it will be
>  	 * started when buffers are queued on both the input and output.
>  	 */

-- 
Regards,

Laurent Pinchart