Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Laurent Pinchart]

Hi Andrey,

On Monday 03 Oct 2016 22:52:11 Andrey Utkin wrote:
> On Sun, Oct 02, 2016 at 02:30:45AM +0530, Harman Kalra wrote:
> >  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 *count, unsigned int *num_planes,
> > +   unsigned int sizes[], struct device *alloc_devs[])
> 
> 2 spaces + 3 tabs -> 2 spaces + 2 tabs? Am I seeing this correctly?
> Both ways are against CodingStyle.

It's 4 tabs + 1 space -> 3 tabs as far as I can see.

> > -   /* 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.
> > 
> >  */
> 
> This comment style is discouraged so this is at lease not perfect.
> Doesn't checkpatch complain with this change? If it doesn't, could you
> please also check with --strict, as long as you're working on style.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Laurent Pinchart]

Hi Andrey,

On Monday 03 Oct 2016 22:52:11 Andrey Utkin wrote:
> On Sun, Oct 02, 2016 at 02:30:45AM +0530, Harman Kalra wrote:
> >  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 *count, unsigned int *num_planes,
> > +   unsigned int sizes[], struct device *alloc_devs[])
> 
> 2 spaces + 3 tabs -> 2 spaces + 2 tabs? Am I seeing this correctly?
> Both ways are against CodingStyle.

It's 4 tabs + 1 space -> 3 tabs as far as I can see.

> > -   /* 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.
> > 
> >  */
> 
> This comment style is discouraged so this is at lease not perfect.
> Doesn't checkpatch complain with this change? If it doesn't, could you
> please also check with --strict, as long as you're working on style.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Andrey Utkin]

On Sun, Oct 02, 2016 at 02:30:45AM +0530, Harman Kalra wrote:
>  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 *count, unsigned int *num_planes,
> + unsigned int sizes[], struct device *alloc_devs[])

2 spaces + 3 tabs -> 2 spaces + 2 tabs? Am I seeing this correctly?
Both ways are against CodingStyle.

> - /* 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.
>*/

This comment style is discouraged so this is at lease not perfect.
Doesn't checkpatch complain with this change? If it doesn't, could you
please also check with --strict, as long as you're working on style.

Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Andrey Utkin]

On Sun, Oct 02, 2016 at 02:30:45AM +0530, Harman Kalra wrote:
>  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 *count, unsigned int *num_planes,
> + unsigned int sizes[], struct device *alloc_devs[])

2 spaces + 3 tabs -> 2 spaces + 2 tabs? Am I seeing this correctly?
Both ways are against CodingStyle.

> - /* 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.
>*/

This comment style is discouraged so this is at lease not perfect.
Doesn't checkpatch complain with this change? If it doesn't, could you
please also check with --strict, as long as you're working on style.

Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Laurent Pinchart]

Hello Harman,

Thank you for the patch.

The subject of your commit message should at least contain the name of the 
driver. Furthermore, you can mention that the patch originates from warnings 
output by checkpatch.pl, but the subject should describe what you fix (in this 
case what type of warning).

On Sunday 02 Oct 2016 02:30:45 Harman Kalra wrote:
> Removing warnings caught by checkpatch.pl

If the purpose of commit message bodies was to repeat the subject line, we 
wouldn't use them :-) Here you should describe why this change is needed, and 
possibly how it is performed when the patch is complex (which isn't the case 
of this patch).

> Signed-off-by: Harman Kalra 
> ---
>  drivers/staging/media/omap4iss/iss_video.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index c16927a..7cc1691 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -297,8 +297,8 @@ static void iss_video_pix_to_mbus(const struct
> v4l2_pix_format *pix, */
> 
>  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 *count, unsigned int *num_planes,
> + unsigned int sizes[], struct device *alloc_devs[])

This breaks the coding style of the driver which aligns function arguments to 
the opening parenthesis. You can instead wrap the last line to shorten it.

>  {
>   struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>   struct iss_video *video = vfh->video;
> @@ -678,8 +678,8 @@ void omap4iss_video_cancel_stream(struct iss_video
> *video) 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.

Lines can be up to 80 characters long, there's no need to wrap them after 52 
characters only.

>*/
>   sdsel.pad = pad;
>   ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, );
> --
> 1.7.9.5

-- 
Regards,

Laurent Pinchart

Re: [PATCH] [media] : Removing warnings caught by checkpatch.pl

[Laurent Pinchart]

Hello Harman,

Thank you for the patch.

The subject of your commit message should at least contain the name of the 
driver. Furthermore, you can mention that the patch originates from warnings 
output by checkpatch.pl, but the subject should describe what you fix (in this 
case what type of warning).

On Sunday 02 Oct 2016 02:30:45 Harman Kalra wrote:
> Removing warnings caught by checkpatch.pl

If the purpose of commit message bodies was to repeat the subject line, we 
wouldn't use them :-) Here you should describe why this change is needed, and 
possibly how it is performed when the patch is complex (which isn't the case 
of this patch).

> Signed-off-by: Harman Kalra 
> ---
>  drivers/staging/media/omap4iss/iss_video.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index c16927a..7cc1691 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -297,8 +297,8 @@ static void iss_video_pix_to_mbus(const struct
> v4l2_pix_format *pix, */
> 
>  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 *count, unsigned int *num_planes,
> + unsigned int sizes[], struct device *alloc_devs[])

This breaks the coding style of the driver which aligns function arguments to 
the opening parenthesis. You can instead wrap the last line to shorten it.

>  {
>   struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
>   struct iss_video *video = vfh->video;
> @@ -678,8 +678,8 @@ void omap4iss_video_cancel_stream(struct iss_video
> *video) 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.

Lines can be up to 80 characters long, there's no need to wrap them after 52 
characters only.

>*/
>   sdsel.pad = pad;
>   ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, );
> --
> 1.7.9.5

-- 
Regards,

Laurent Pinchart

[PATCH] [media] : Removing warnings caught by checkpatch.pl

[Harman Kalra]

Removing warnings caught by checkpatch.pl

Signed-off-by: Harman Kalra 
---
 drivers/staging/media/omap4iss/iss_video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..7cc1691 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -297,8 +297,8 @@ static void iss_video_pix_to_mbus(const struct 
v4l2_pix_format *pix,
  */

 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 *count, unsigned int *num_planes,
+   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 +678,8 @@ void omap4iss_video_cancel_stream(struct iss_video *video)
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, );
--
1.7.9.5

[PATCH] [media] : Removing warnings caught by checkpatch.pl

[Harman Kalra]

Removing warnings caught by checkpatch.pl

Signed-off-by: Harman Kalra 
---
 drivers/staging/media/omap4iss/iss_video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..7cc1691 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -297,8 +297,8 @@ static void iss_video_pix_to_mbus(const struct 
v4l2_pix_format *pix,
  */

 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 *count, unsigned int *num_planes,
+   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 +678,8 @@ void omap4iss_video_cancel_stream(struct iss_video *video)
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, );
--
1.7.9.5