[PATCH 0/2] staging: vc04_services: bcm2835-camera: Enhanced code readability

[PATCH 0/2] staging: vc04_services: bcm2835-camera: Enhanced code readability

[Narcisa Ana Maria Vasile]

Made a few changes in bcm2835-camera in order to improve code readability.

Narcisa Ana Maria Vasile (2):
  staging: vc04_services: bcm2835-camera: Moved condition outside the
    for     statement
  staging: vc04_services: bcm2835-camera: Removed unnecessary variable

 .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH 1/2] staging: vc04_services: bcm2835-camera: Moved condition outside the for statement

[Narcisa Ana Maria Vasile]

Instead of computing the minimum value in the for loop, I store it in
a variable.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 53e275e..347d8fff 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 	struct mmal_parameter_camera_info_t cam_info = {0};
 	int param_size = sizeof(cam_info);
 	int i;
+	int min_size;
 
 	/* create a camera_info component */
 	ret = vchiq_mmal_component_init(instance, "camera_info",
@@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 					  &param_size)) {
 		pr_info("Failed to get camera info\n");
 	}
-	for (i = 0;
-	     i < (cam_info.num_cameras > num_resolutions ?
-			num_resolutions :
-			cam_info.num_cameras);
-	     i++) {
+
+	min_size = cam_info.num_cameras > num_resolutions ?
+			num_resolutions : cam_info.num_cameras;
+	for (i = 0; i < min_size; i++) {
 		resolutions[i][0] = cam_info.cameras[i].max_width;
 		resolutions[i][1] = cam_info.cameras[i].max_height;
 	}
-- 
1.9.1

Re: [Outreachy kernel] [PATCH 1/2] staging: vc04_services: bcm2835-camera: Moved condition outside the for statement

[Daniel Baluta]

On Wed, Mar 1, 2017 at 1:44 PM, Narcisa Ana Maria Vasile
<narcisaanamaria12@gmail.com> wrote:
> Instead of computing the minimum value in the for loop, I store it in
> a variable.
>
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---

Hi Narcisa,

Please use the imperative mood in the subject line.

e.g:

staging: vc04_services: bcm2835-camera: Moved condition outside the
for statement

also, this could be more specific:

staging: vc04_services: bcm2835-camera: Move min calculation outside the loop



 Also, make commit body
"impersonal".

E.g: ...

 Instead of computing the minimum value in the for loop, store it in a variable.

 https://chris.beams.io/posts/git-commit/
>  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 53e275e..347d8fff 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>         struct mmal_parameter_camera_info_t cam_info = {0};
>         int param_size = sizeof(cam_info);
>         int i;
> +       int min_size;
>
>         /* create a camera_info component */
>         ret = vchiq_mmal_component_init(instance, "camera_info",
> @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>                                           &param_size)) {
>                 pr_info("Failed to get camera info\n");
>         }
> -       for (i = 0;
> -            i < (cam_info.num_cameras > num_resolutions ?
> -                       num_resolutions :
> -                       cam_info.num_cameras);
> -            i++) {
> +
> +       min_size = cam_info.num_cameras > num_resolutions ?
> +                       num_resolutions : cam_info.num_cameras;
> +       for (i = 0; i < min_size; i++) {
>                 resolutions[i][0] = cam_info.cameras[i].max_width;
>                 resolutions[i][1] = cam_info.cameras[i].max_height;
>         }
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/04783e886f993597ef3b14f2602864884e0ac7b7.1488367580.git.narcisaanamaria12%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

[PATCH 2/2] staging: vc04_services: bcm2835-camera: Removed unnecessary variable

[Narcisa Ana Maria Vasile]

Instead of storing the result, now the function returns directly

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 347d8fff..d00e00b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -671,7 +671,6 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
 static int set_overlay_params(struct bm2835_mmal_dev *dev,
 			      struct vchiq_mmal_port *port)
 {
-	int ret;
 	struct mmal_parameter_displayregion prev_config = {
 	.set = MMAL_DISPLAY_SET_LAYER | MMAL_DISPLAY_SET_ALPHA |
 	    MMAL_DISPLAY_SET_DEST_RECT | MMAL_DISPLAY_SET_FULLSCREEN,
@@ -685,11 +684,9 @@ static int set_overlay_params(struct bm2835_mmal_dev *dev,
 		      .height = dev->overlay.w.height,
 		      },
 	};
-	ret = vchiq_mmal_port_parameter_set(dev->instance, port,
+	return vchiq_mmal_port_parameter_set(dev->instance, port,
 					    MMAL_PARAMETER_DISPLAYREGION,
 					    &prev_config, sizeof(prev_config));
-
-	return ret;
 }
 
 /* overlay ioctl */
-- 
1.9.1

Re: [Outreachy kernel] [PATCH 2/2] staging: vc04_services: bcm2835-camera: Removed unnecessary variable

[Julia Lawall]

On Wed, 1 Mar 2017, Narcisa Ana Maria Vasile wrote:

> Instead of storing the result, now the function returns directly
>
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 347d8fff..d00e00b 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -671,7 +671,6 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
>  static int set_overlay_params(struct bm2835_mmal_dev *dev,
>  			      struct vchiq_mmal_port *port)
>  {
> -	int ret;
>  	struct mmal_parameter_displayregion prev_config = {
>  	.set = MMAL_DISPLAY_SET_LAYER | MMAL_DISPLAY_SET_ALPHA |
>  	    MMAL_DISPLAY_SET_DEST_RECT | MMAL_DISPLAY_SET_FULLSCREEN,
> @@ -685,11 +684,9 @@ static int set_overlay_params(struct bm2835_mmal_dev *dev,
>  		      .height = dev->overlay.w.height,
>  		      },
>  	};
> -	ret = vchiq_mmal_port_parameter_set(dev->instance, port,
> +	return vchiq_mmal_port_parameter_set(dev->instance, port,
>  					    MMAL_PARAMETER_DISPLAYREGION,
>  					    &prev_config, sizeof(prev_config));

The arguments needed to be moved one space over.

julia

> -
> -	return ret;
>  }
>
>  /* overlay ioctl */
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/daabcf56391dbfadac95ab36f36afd3bdaeaf423.1488367580.git.narcisaanamaria12%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 2/2] staging: vc04_services: bcm2835-camera: Removed unnecessary variable

[Narcisa Ana Maria Vasile]

On Wed, Mar 01, 2017 at 02:05:09PM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 1 Mar 2017, Narcisa Ana Maria Vasile wrote:
> 
> > Instead of storing the result, now the function returns directly
> >
> > Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> > ---
> >  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > index 347d8fff..d00e00b 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > @@ -671,7 +671,6 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
> >  static int set_overlay_params(struct bm2835_mmal_dev *dev,
> >  			      struct vchiq_mmal_port *port)
> >  {
> > -	int ret;
> >  	struct mmal_parameter_displayregion prev_config = {
> >  	.set = MMAL_DISPLAY_SET_LAYER | MMAL_DISPLAY_SET_ALPHA |
> >  	    MMAL_DISPLAY_SET_DEST_RECT | MMAL_DISPLAY_SET_FULLSCREEN,
> > @@ -685,11 +684,9 @@ static int set_overlay_params(struct bm2835_mmal_dev *dev,
> >  		      .height = dev->overlay.w.height,
> >  		      },
> >  	};
> > -	ret = vchiq_mmal_port_parameter_set(dev->instance, port,
> > +	return vchiq_mmal_port_parameter_set(dev->instance, port,
> >  					    MMAL_PARAMETER_DISPLAYREGION,
> >  					    &prev_config, sizeof(prev_config));
> 
> The arguments needed to be moved one space over.
> 
> julia
>
  Oh, ok.
  I'll resend another version.
  
  Narcisa
> > -
> > -	return ret;
> >  }
> >
> >  /* overlay ioctl */
> > --
> > 1.9.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/daabcf56391dbfadac95ab36f36afd3bdaeaf423.1488367580.git.narcisaanamaria12%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

[PATCH v2 0/2] staging: vc04_services: bcm2835-camera: Enhance code readability

[Narcisa Ana Maria Vasile]

Refactor bcm2835-camera.c for readability

---
Changes in v2:
    - Improve the commit messages 
    - Make commit body impersonal


Narcisa Ana Maria Vasile (2):
  staging: vc04_services: bcm2835-camera: Move min calculation outside
    the loop
  staging: vc04_services: bcm2835-camera: Remove unnecessary variable

 .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Narcisa Ana Maria Vasile]

Instead of computing the minimum value in the for loop, store it in
a variable.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 53e275e..347d8fff 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 	struct mmal_parameter_camera_info_t cam_info = {0};
 	int param_size = sizeof(cam_info);
 	int i;
+	int min_size;
 
 	/* create a camera_info component */
 	ret = vchiq_mmal_component_init(instance, "camera_info",
@@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 					  &param_size)) {
 		pr_info("Failed to get camera info\n");
 	}
-	for (i = 0;
-	     i < (cam_info.num_cameras > num_resolutions ?
-			num_resolutions :
-			cam_info.num_cameras);
-	     i++) {
+
+	min_size = cam_info.num_cameras > num_resolutions ?
+			num_resolutions : cam_info.num_cameras;
+	for (i = 0; i < min_size; i++) {
 		resolutions[i][0] = cam_info.cameras[i].max_width;
 		resolutions[i][1] = cam_info.cameras[i].max_height;
 	}
-- 
1.9.1

[PATCH v2 2/2] staging: vc04_services: bcm2835-camera: Remove unnecessary variable

[Narcisa Ana Maria Vasile]

Instead of storing the result, make the function return directly

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 347d8fff..d00e00b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -671,7 +671,6 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
 static int set_overlay_params(struct bm2835_mmal_dev *dev,
 			      struct vchiq_mmal_port *port)
 {
-	int ret;
 	struct mmal_parameter_displayregion prev_config = {
 	.set = MMAL_DISPLAY_SET_LAYER | MMAL_DISPLAY_SET_ALPHA |
 	    MMAL_DISPLAY_SET_DEST_RECT | MMAL_DISPLAY_SET_FULLSCREEN,
@@ -685,11 +684,9 @@ static int set_overlay_params(struct bm2835_mmal_dev *dev,
 		      .height = dev->overlay.w.height,
 		      },
 	};
-	ret = vchiq_mmal_port_parameter_set(dev->instance, port,
+	return vchiq_mmal_port_parameter_set(dev->instance, port,
 					    MMAL_PARAMETER_DISPLAYREGION,
 					    &prev_config, sizeof(prev_config));
-
-	return ret;
 }
 
 /* overlay ioctl */
-- 
1.9.1

[PATCH v3 0/2] staging: vc04_services: bcm2835-camera: Enhance code readability

[Narcisa Ana Maria Vasile]

Refactor bcm2835-camera.c for readability

---
Changes in v3:
    - Fix coding style issue by moving the arguments one space over

Narcisa Ana Maria Vasile (2):
  staging: vc04_services: bcm2835-camera: Move min calculation outside
    the loop
  staging: vc04_services: bcm2835-camera: Remove unnecessary variable

 .../vc04_services/bcm2835-camera/bcm2835-camera.c     | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

-- 
1.9.1

[PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Narcisa Ana Maria Vasile]

Instead of computing the minimum value in the for loop, store it in a variable.

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 53e275e..7b9d33c 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 	struct mmal_parameter_camera_info_t cam_info = {0};
 	int param_size = sizeof(cam_info);
 	int i;
+	int min_size;
 
 	/* create a camera_info component */
 	ret = vchiq_mmal_component_init(instance, "camera_info",
@@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
 					  &param_size)) {
 		pr_info("Failed to get camera info\n");
 	}
-	for (i = 0;
-	     i < (cam_info.num_cameras > num_resolutions ?
-			num_resolutions :
-			cam_info.num_cameras);
-	     i++) {
+
+	min_size = cam_info.num_cameras > num_resolutions ?
+		   num_resolutions : cam_info.num_cameras;
+	for (i = 0; i < min_size; i++) {
 		resolutions[i][0] = cam_info.cameras[i].max_width;
 		resolutions[i][1] = cam_info.cameras[i].max_height;
 	}
-- 
1.9.1

Re: [PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Greg KH]

On Wed, Mar 01, 2017 at 05:37:13PM +0200, Narcisa Ana Maria Vasile wrote:
> Instead of computing the minimum value in the for loop, store it in a variable.
> 
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 53e275e..7b9d33c 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>  	struct mmal_parameter_camera_info_t cam_info = {0};
>  	int param_size = sizeof(cam_info);
>  	int i;
> +	int min_size;
>  
>  	/* create a camera_info component */
>  	ret = vchiq_mmal_component_init(instance, "camera_info",
> @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>  					  &param_size)) {
>  		pr_info("Failed to get camera info\n");
>  	}
> -	for (i = 0;
> -	     i < (cam_info.num_cameras > num_resolutions ?
> -			num_resolutions :
> -			cam_info.num_cameras);
> -	     i++) {
> +
> +	min_size = cam_info.num_cameras > num_resolutions ?
> +		   num_resolutions : cam_info.num_cameras;

We have a min() macro, why not just use that instead?

thanks,

greg k-h

Re: [PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Narcisa Ana Maria Vasile]

On Mon, Mar 06, 2017 at 01:49:52PM +0100, Greg KH wrote:
> On Wed, Mar 01, 2017 at 05:37:13PM +0200, Narcisa Ana Maria Vasile wrote:
> > Instead of computing the minimum value in the for loop, store it in a variable.
> > 
> > Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> > ---
> >  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > index 53e275e..7b9d33c 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> >  	struct mmal_parameter_camera_info_t cam_info = {0};
> >  	int param_size = sizeof(cam_info);
> >  	int i;
> > +	int min_size;
> >  
> >  	/* create a camera_info component */
> >  	ret = vchiq_mmal_component_init(instance, "camera_info",
> > @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> >  					  &param_size)) {
> >  		pr_info("Failed to get camera info\n");
> >  	}
> > -	for (i = 0;
> > -	     i < (cam_info.num_cameras > num_resolutions ?
> > -			num_resolutions :
> > -			cam_info.num_cameras);
> > -	     i++) {
> > +
> > +	min_size = cam_info.num_cameras > num_resolutions ?
> > +		   num_resolutions : cam_info.num_cameras;
> 
> We have a min() macro, why not just use that instead?
> 
> thanks,
> 
> greg k-h

  I remember I wanted to use the macro, but then decided not to,
  in order to avoid the unsigned int vs int warning.

  Narcisa

Re: [PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Greg KH]

On Mon, Mar 06, 2017 at 02:56:50PM +0200, Narcisa Ana Maria Vasile wrote:
> On Mon, Mar 06, 2017 at 01:49:52PM +0100, Greg KH wrote:
> > On Wed, Mar 01, 2017 at 05:37:13PM +0200, Narcisa Ana Maria Vasile wrote:
> > > Instead of computing the minimum value in the for loop, store it in a variable.
> > > 
> > > Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> > > ---
> > >  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > index 53e275e..7b9d33c 100644
> > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> > >  	struct mmal_parameter_camera_info_t cam_info = {0};
> > >  	int param_size = sizeof(cam_info);
> > >  	int i;
> > > +	int min_size;
> > >  
> > >  	/* create a camera_info component */
> > >  	ret = vchiq_mmal_component_init(instance, "camera_info",
> > > @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> > >  					  &param_size)) {
> > >  		pr_info("Failed to get camera info\n");
> > >  	}
> > > -	for (i = 0;
> > > -	     i < (cam_info.num_cameras > num_resolutions ?
> > > -			num_resolutions :
> > > -			cam_info.num_cameras);
> > > -	     i++) {
> > > +
> > > +	min_size = cam_info.num_cameras > num_resolutions ?
> > > +		   num_resolutions : cam_info.num_cameras;
> > 
> > We have a min() macro, why not just use that instead?
> > 
> > thanks,
> > 
> > greg k-h
> 
>   I remember I wanted to use the macro, but then decided not to,
>   in order to avoid the unsigned int vs int warning.

Then that's a bug you need to fix!  :)

Use min_t() for that.

thanks,

greg k-h

Re: [PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Greg KH]

On Wed, Mar 01, 2017 at 05:37:13PM +0200, Narcisa Ana Maria Vasile wrote:
> Instead of computing the minimum value in the for loop, store it in a variable.
> 
> Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 53e275e..7b9d33c 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>  	struct mmal_parameter_camera_info_t cam_info = {0};
>  	int param_size = sizeof(cam_info);
>  	int i;
> +	int min_size;
>  
>  	/* create a camera_info component */
>  	ret = vchiq_mmal_component_init(instance, "camera_info",
> @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
>  					  &param_size)) {
>  		pr_info("Failed to get camera info\n");
>  	}
> -	for (i = 0;
> -	     i < (cam_info.num_cameras > num_resolutions ?
> -			num_resolutions :
> -			cam_info.num_cameras);
> -	     i++) {
> +
> +	min_size = cam_info.num_cameras > num_resolutions ?
> +		   num_resolutions : cam_info.num_cameras;

Again, just use min().

thanks,

greg k-h

Re: [PATCH v3 1/2] staging: vc04_services: bcm2835-camera: Move min calculation outside the loop

[Greg KH]

On Mon, Mar 06, 2017 at 01:55:37PM +0100, Greg KH wrote:
> On Wed, Mar 01, 2017 at 05:37:13PM +0200, Narcisa Ana Maria Vasile wrote:
> > Instead of computing the minimum value in the for loop, store it in a variable.
> > 
> > Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
> > ---
> >  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > index 53e275e..7b9d33c 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > @@ -1526,6 +1526,7 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> >  	struct mmal_parameter_camera_info_t cam_info = {0};
> >  	int param_size = sizeof(cam_info);
> >  	int i;
> > +	int min_size;
> >  
> >  	/* create a camera_info component */
> >  	ret = vchiq_mmal_component_init(instance, "camera_info",
> > @@ -1541,11 +1542,10 @@ static int get_num_cameras(struct vchiq_mmal_instance *instance,
> >  					  &param_size)) {
> >  		pr_info("Failed to get camera info\n");
> >  	}
> > -	for (i = 0;
> > -	     i < (cam_info.num_cameras > num_resolutions ?
> > -			num_resolutions :
> > -			cam_info.num_cameras);
> > -	     i++) {
> > +
> > +	min_size = cam_info.num_cameras > num_resolutions ?
> > +		   num_resolutions : cam_info.num_cameras;
> 
> Again, just use min().

Argh, duplicate emails on my side, sorry for the noise.

greg k-h

[PATCH v3 2/2] staging: vc04_services: bcm2835-camera: Remove unnecessary variable

[Narcisa Ana Maria Vasile]

Instead of storing the result, make the function return directly

Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 7b9d33c..d5db9e6 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -671,7 +671,6 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
 static int set_overlay_params(struct bm2835_mmal_dev *dev,
 			      struct vchiq_mmal_port *port)
 {
-	int ret;
 	struct mmal_parameter_displayregion prev_config = {
 	.set = MMAL_DISPLAY_SET_LAYER | MMAL_DISPLAY_SET_ALPHA |
 	    MMAL_DISPLAY_SET_DEST_RECT | MMAL_DISPLAY_SET_FULLSCREEN,
@@ -685,11 +684,9 @@ static int set_overlay_params(struct bm2835_mmal_dev *dev,
 		      .height = dev->overlay.w.height,
 		      },
 	};
-	ret = vchiq_mmal_port_parameter_set(dev->instance, port,
-					    MMAL_PARAMETER_DISPLAYREGION,
-					    &prev_config, sizeof(prev_config));
-
-	return ret;
+	return vchiq_mmal_port_parameter_set(dev->instance, port,
+					     MMAL_PARAMETER_DISPLAYREGION,
+					     &prev_config, sizeof(prev_config));
 }
 
 /* overlay ioctl */
-- 
1.9.1