media: rkisp1: Fix unused value issue

PATCH In review Dheeraj Reddy Jonnalagadda 3 participants 5 messages Nov 19, 2024 – Dec 4, 2024

Subsystems (3)

Media (V4L/DVB) Arm Rockchip Rockchip ISP V1 driver

lore

Share & tools

lore

Dheeraj Reddy Jonnalagadda

author

2024-11-19 07:27 UTC #0 lore raw

PATCH v4 1/1 media: rkisp1: Fix unused value issue

This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block.

'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")'

Signed-off-by Dheeraj Reddy Jonnalagadda <[email protected]>

---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +3 -0

index dd114ab77800..9ad5026ab10a 100644
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 			break;
 		}
 
+		if (ret)
+			break;
+
 		/* Parse the endpoint and validate the bus type. */
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (ret) {

Signature · 1 line

2.34.1

Laurent Pinchart

maintainer Reviewed-by on v4 1/1

2024-11-19 08:00 UTC #1 lore raw

Hi Dheeraj,

Thank you for the patch.

Quoted 5 lines · Dheeraj Reddy Jonnalagadda

This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block.

The patch doesn't "return immediately". You can write "Fix this by breaking from the look immediately after the switch block when an error occurs." or something similar.

Quoted 3 lines

'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")'

The Fixes tag should be formatted on a single line, without outer quotes, and without a blank line between it and the Signed-off-by line:

Fixes 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")

Quoted 1 line

Signed-off-by Dheeraj Reddy Jonnalagadda <[email protected]>

I can update the commit message when applying the patch, there's no need to submit a v5, unless if you want to. Please let me know if I should take this version and update the commit message, or if you will send a v5.

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 18 lines

---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +3 -0

index dd114ab77800..9ad5026ab10a 100644
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 			break;
 		}
 
+		if (ret)
+			break;
+
 		/* Parse the endpoint and validate the bus type. */
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (ret) {

Signature · 3 lines

Regards,

Laurent Pinchart

Jacopo Mondi

Reviewed-by

2024-11-19 08:44 UTC #2 lore raw

HI Dheeraj

Quoted 24 lines · Laurent Pinchart

Hi Dheeraj,
Thank you for the patch.
Quoted 5 lines · Dheeraj Reddy Jonnalagadda
This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block.
The patch doesn't "return immediately". You can write "Fix this by breaking from the look immediately after the switch block when an error occurs." or something similar.
Quoted 3 lines
'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")'
The Fixes tag should be formatted on a single line, without outer quotes, and without a blank line between it and the Signed-off-by line:
Fixes 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")

Quoted 8 lines

Quoted 1 line
Signed-off-by Dheeraj Reddy Jonnalagadda <[email protected]>
I can update the commit message when applying the patch, there's no need to submit a v5, unless if you want to. Please let me know if I should take this version and update the commit message, or if you will send a v5.
Reviewed-by Laurent Pinchart <[email protected]>

And when you receive a tag in a previous patch version, please carry it over to the next version you'll send ;)

Reviewed-by Jacopo Mondi <[email protected]>

Thanks
j

Quoted 25 lines

Quoted 10 lines
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +0 -0
index dd114ab77800..9ad5026ab10a 100644
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1) · line 236
Quoted 9 lines
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +3 -0
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 			break;
 		}
 
+		if (ret)
+			break;
+
 		/* Parse the endpoint and validate the bus type. */
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (ret) {
--
Regards,
Laurent Pinchart

Dheeraj Reddy Jonnalagadda

author

2024-11-19 09:34 UTC #3 lore raw

In reply to Laurent Pinchart

Quoted 45 lines · Laurent Pinchart

Hi Dheeraj,
Thank you for the patch.
Quoted 5 lines · Dheeraj Reddy Jonnalagadda
This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block.
The patch doesn't "return immediately". You can write "Fix this by breaking from the look immediately after the switch block when an error occurs." or something similar.
Quoted 3 lines
'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")'
The Fixes tag should be formatted on a single line, without outer quotes, and without a blank line between it and the Signed-off-by line:
Fixes 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")
Quoted 1 line
Signed-off-by Dheeraj Reddy Jonnalagadda <[email protected]>
I can update the commit message when applying the patch, there's no need to submit a v5, unless if you want to. Please let me know if I should take this version and update the commit message, or if you will send a v5.
Reviewed-by Laurent Pinchart <[email protected]>
Quoted 12 lines
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +0 -0
index dd114ab77800..9ad5026ab10a 100644
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 			break;
 		}
 

Quoted 11 lines

Quoted 6 lines

drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +3 -0

+		if (ret)
+			break;
+
 		/* Parse the endpoint and validate the bus type. */
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (ret) {

Signature · 3 lines

Regards,

Laurent Pinchart

Hi Laurent,

Please go ahead and take this version and apply the patch with the updated commit message.

Thank you and Jacopo for your valuable comments. As a new contributor, your feedback is extremely helpful.

-Dheeraj

Laurent Pinchart

maintainer

2024-12-04 17:05 UTC #4 lore raw

In reply to Laurent Pinchart

Quoted 15 lines · Laurent Pinchart

Hi Dheeraj,
Thank you for the patch.
Quoted 5 lines · Dheeraj Reddy Jonnalagadda
This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block.
The patch doesn't "return immediately". You can write "Fix this by breaking from the look immediately after the switch block when an error occurs." or something similar.

Quoted 7 lines

Quoted 2 lines
'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")'
The Fixes tag should be formatted on a single line, without outer quotes, and without a blank line between it and the Signed-off-by line:
Fixes 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")

That commit ID doesn't exist in the git history. The commit corresponding to the message is 7d4f126fde89. I'll update the commit message accordingly.

Quoted 27 lines

Quoted 1 line
Signed-off-by Dheeraj Reddy Jonnalagadda <[email protected]>
I can update the commit message when applying the patch, there's no need to submit a v5, unless if you want to. Please let me know if I should take this version and update the commit message, or if you will send a v5.
Reviewed-by Laurent Pinchart <[email protected]>
Quoted 18 lines
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +3 -0
index dd114ab77800..9ad5026ab10a 100644
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 			break;
 		}
 
+		if (ret)
+			break;
+
 		/* Parse the endpoint and validate the bus type. */
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (ret) {

Signature · 3 lines

Regards,

Laurent Pinchart