Improve the error handling of the irq errors. I am not sure why the
retcode was replaced always with -EINVAL, so I have added that fix as a
follow-up patch.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
Ricardo Ribalda (3):
media: bcm2835-unicam: Fix error handling for platform_get_irq
media: bcm2835-unicam: Do not print error when irq not found
media: bcm2835-unicam: Do not replace IRQ retcode during probe
drivers/media/platform/broadcom/bcm2835-unicam.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
---
base-commit: 1c73d0b29d04bf4082e7beb6a508895e118ee30d
change-id: 20240430-fix-broad-490eb1a39754
Best regards,
--
Ricardo Ribalda <[email protected]>
platform_get_irq() cannot return the value 0.
If it returns -EPROBE_DEFER, we need to populate the error code upwards
to retry probing once the irq handler is ready.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/broadcom/bcm2835-unicam.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index bd2bbb53070e..2a3a27ac70ba 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -2660,9 +2660,10 @@ static int unicam_probe(struct platform_device *pdev)
}
ret = platform_get_irq(pdev, 0);
- if (ret <= 0) {
+ if (ret < 0) {
dev_err(&pdev->dev, "No IRQ resource\n");
- ret = -EINVAL;
+ if (ret != -EPROBE_DEFER)
+ ret = -EINVAL;
goto err_unicam_put;
}
--
2.44.0.769.g3c40516874-goog
platform_get_irq() already prints an error for us.
Fixes cocci:
drivers/media/platform/broadcom/bcm2835-unicam.c:2664:2-9: line 2664 is redundant because platform_get_irq() already prints an error
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/broadcom/bcm2835-unicam.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index 2a3a27ac70ba..b2b23d24da19 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -2661,7 +2661,6 @@ static int unicam_probe(struct platform_device *pdev)
ret = platform_get_irq(pdev, 0);
if (ret < 0) {
- dev_err(&pdev->dev, "No IRQ resource\n");
if (ret != -EPROBE_DEFER)
ret = -EINVAL;
goto err_unicam_put;
--
2.44.0.769.g3c40516874-goog
Use the error code generated by platform_get_irq() and
devm_request_irq() as the error code of probe().
It will give a more accurate reason of why it failed.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/platform/broadcom/bcm2835-unicam.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index b2b23d24da19..0b2729bf4a36 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -2660,17 +2660,13 @@ static int unicam_probe(struct platform_device *pdev)
}
ret = platform_get_irq(pdev, 0);
- if (ret < 0) {
- if (ret != -EPROBE_DEFER)
- ret = -EINVAL;
+ if (ret < 0)
goto err_unicam_put;
- }
ret = devm_request_irq(&pdev->dev, ret, unicam_isr, 0,
"unicam_capture0", unicam);
if (ret) {
dev_err(&pdev->dev, "Unable to request interrupt\n");
- ret = -EINVAL;
goto err_unicam_put;
}
--
2.44.0.769.g3c40516874-goog
Hi Ricardo,
Thank you for the patch.
On Tue, Apr 30, 2024 at 07:51:26AM +0000, Ricardo Ribalda wrote:
> platform_get_irq() cannot return the value 0.
>
> If it returns -EPROBE_DEFER, we need to populate the error code upwards
> to retry probing once the irq handler is ready.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/platform/broadcom/bcm2835-unicam.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index bd2bbb53070e..2a3a27ac70ba 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2660,9 +2660,10 @@ static int unicam_probe(struct platform_device *pdev)
> }
>
> ret = platform_get_irq(pdev, 0);
> - if (ret <= 0) {
> + if (ret < 0) {
> dev_err(&pdev->dev, "No IRQ resource\n");
> - ret = -EINVAL;
> + if (ret != -EPROBE_DEFER)
> + ret = -EINVAL;
What's wrong with leaving ret untouched ? I assume it was set to -EINVAL
to avoid returning success in case ret was 0. Now that the test has
changed, I think we can leave the value as-is.
> goto err_unicam_put;
> }
>
>
--
Regards,
Laurent Pinchart
Hi Ricardo,
Thank you for the patch.
On Tue, Apr 30, 2024 at 07:51:27AM +0000, Ricardo Ribalda wrote:
> platform_get_irq() already prints an error for us.
>
> Fixes cocci:
> drivers/media/platform/broadcom/bcm2835-unicam.c:2664:2-9: line 2664 is redundant because platform_get_irq() already prints an error
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/media/platform/broadcom/bcm2835-unicam.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index 2a3a27ac70ba..b2b23d24da19 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2661,7 +2661,6 @@ static int unicam_probe(struct platform_device *pdev)
>
> ret = platform_get_irq(pdev, 0);
> if (ret < 0) {
> - dev_err(&pdev->dev, "No IRQ resource\n");
> if (ret != -EPROBE_DEFER)
> ret = -EINVAL;
> goto err_unicam_put;
--
Regards,
Laurent Pinchart
Hi Ricardo,
Thank you for the patch.
On Tue, Apr 30, 2024 at 07:51:28AM +0000, Ricardo Ribalda wrote:
> Use the error code generated by platform_get_irq() and
> devm_request_irq() as the error code of probe().
>
> It will give a more accurate reason of why it failed.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/platform/broadcom/bcm2835-unicam.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index b2b23d24da19..0b2729bf4a36 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2660,17 +2660,13 @@ static int unicam_probe(struct platform_device *pdev)
> }
>
> ret = platform_get_irq(pdev, 0);
> - if (ret < 0) {
> - if (ret != -EPROBE_DEFER)
> - ret = -EINVAL;
> + if (ret < 0)
> goto err_unicam_put;
> - }
I think you can squash the whole patch with 1/3.
>
> ret = devm_request_irq(&pdev->dev, ret, unicam_isr, 0,
> "unicam_capture0", unicam);
> if (ret) {
> dev_err(&pdev->dev, "Unable to request interrupt\n");
> - ret = -EINVAL;
> goto err_unicam_put;
> }
>
--
Regards,
Laurent Pinchart