Skip to contentSkip to conversation

staging: vc04_services: bcm2835-camera: Enhanced code readability

PATCH Superseded Narcisa Ana Maria Vasile 4 participants 17 messages Mar 1, 2017 – Mar 6, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover staging: vc04_services: bcm2835-camera: Enhanced code readability

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(-)
Signature · 1 line
1.9.1
#1 lore raw

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

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +5 -5
index 53e275e..347d8fff 100644
@@ -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;
 	}
Signature · 1 line
1.9.1
#2 lore raw

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

Quoted 5 lines · Narcisa Ana Maria Vasile

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

---

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/

Quoted 40 lines
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +5 -6
index 53e275e..347d8fff 100644
@@ -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 [email protected].
To post to this group, send email to [email protected].
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.

#3 lore raw

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

In reply to Narcisa Ana Maria Vasile

Instead of storing the result, now the function returns directly

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -4
index 347d8fff..d00e00b 100644
@@ -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 */
Signature · 1 line
1.9.1
#4 lore raw

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

Quoted 20 lines · Narcisa Ana Maria Vasile

Instead of storing the result, now the function returns directly

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +0 -1
index 347d8fff..d00e00b 100644
@@ -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,

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int set_overlay_params(struct bm2835_mmal_dev *dev, · line 689

Quoted 7 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -1
@@ -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

Quoted 15 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +0 -3
-
-	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 [email protected].
To post to this group, send email to [email protected].
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.

#5 lore raw

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

Quoted 34 lines · Julia Lawall
Quoted 20 lines · Narcisa Ana Maria Vasile

Instead of storing the result, now the function returns directly

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +0 -1
index 347d8fff..d00e00b 100644
@@ -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,

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int set_overlay_params(struct bm2835_mmal_dev *dev, · line 689

Quoted 7 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -1
@@ -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.

Quoted 2 lines

julia

Oh, ok.
I'll resend another version.

Narcisa

Quoted 15 lines
Quoted 15 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +0 -3
-
-	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 [email protected].
To post to this group, send email to [email protected].
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.

#6 lore raw

v2 cover staging: vc04_services: bcm2835-camera: Enhance code readability

In reply to 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(-)
Signature · 1 line
1.9.1
#7 lore raw

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

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +5 -5
index 53e275e..347d8fff 100644
@@ -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;
 	}
Signature · 1 line
1.9.1
#8 lore raw

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

In reply to Narcisa Ana Maria Vasile

Instead of storing the result, make the function return directly

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -4
index 347d8fff..d00e00b 100644
@@ -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 */
Signature · 1 line
1.9.1
#9 lore raw

v3 cover staging: vc04_services: bcm2835-camera: Enhance code readability

In reply to 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(-)
Signature · 1 line
1.9.1
#10 lore raw

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

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +5 -5
index 53e275e..7b9d33c 100644
@@ -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;
 	}
Signature · 1 line
1.9.1
maintainer
#11 lore raw

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

Quoted 21 lines · Narcisa Ana Maria Vasile

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -0
index 53e275e..7b9d33c 100644
@@ -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)) {

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int get_num_cameras(struct vchiq_mmal_instance *instance, · line 1547

Quoted 10 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -5
 		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

#12 lore raw

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

Quoted 35 lines · Greg KH
Quoted 21 lines · Narcisa Ana Maria Vasile

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -0
index 53e275e..7b9d33c 100644
@@ -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)) {

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int get_num_cameras(struct vchiq_mmal_instance *instance, · line 1547

Quoted 10 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -5
 		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?

Quoted 3 lines

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

maintainer
#13 lore raw

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

Quoted 36 lines · Narcisa Ana Maria Vasile
Quoted 35 lines · Greg KH
Quoted 21 lines · Narcisa Ana Maria Vasile

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -0
index 53e275e..7b9d33c 100644
@@ -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)) {

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int get_num_cameras(struct vchiq_mmal_instance *instance, · line 1547

Quoted 10 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -5
 		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?

Quoted 6 lines
Quoted 3 lines

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

maintainer
#14 lore raw

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

In reply to Narcisa Ana Maria Vasile
Quoted 21 lines · Narcisa Ana Maria Vasile

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -0
index 53e275e..7b9d33c 100644
@@ -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)) {

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int get_num_cameras(struct vchiq_mmal_instance *instance, · line 1547

Quoted 10 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -5
 		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

maintainer
#15 lore raw

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

Quoted 18 lines · Greg KH
Quoted 17 lines · Narcisa Ana Maria Vasile

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

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +1 -0
index 53e275e..7b9d33c 100644
@@ -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;
 
Quoted 16 lines
Quoted 4 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +0 -0
 	/* 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)) {

drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c static int get_num_cameras(struct vchiq_mmal_instance *instance, · line 1547

Quoted 10 lines
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -5
 		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

#16 lore raw

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

In reply to Narcisa Ana Maria Vasile

Instead of storing the result, make the function return directly

---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +3 -6
index 7b9d33c..d5db9e6 100644
@@ -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 */
Signature · 1 line
1.9.1